Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   "Hu, Robert" <robert.hu@intel.com>
To:     Sean Christopherson <seanjc@google.com>,
        Paolo Bonzini <pbonzini@redhat.com>
CC:     Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Robert Hoo <robert.hu@linux.intel.com>
Subject: RE: [PATCH] KVM: nVMX: Dynamically compute max VMCS index for vmcs12
Thread-Topic: [PATCH] KVM: nVMX: Dynamically compute max VMCS index for vmcs12
Thread-Index: AQHXZIuB9zPySR3qkk2RxIcrlGLzAKseKLEAgAAIJYCALy/4cA==
Date:   Wed, 21 Jul 2021 10:02:03 +0000
Message-ID: <DM4PR11MB5453A57DAAC025417C22BCA4E0E39@DM4PR11MB5453.namprd11.prod.outlook.com>
References: <20210618214658.2700765-1-seanjc@google.com>
 <c847e00a-e422-cdc9-3317-fbbd82b6e418@redhat.com>
 <YNDHfX0cntj72sk6@google.com>
In-Reply-To: <YNDHfX0cntj72sk6@google.com>
Accept-Language: en-US
Content-Language: en-US
dlp-version: 11.5.1.3
dlp-reaction: no-action
dlp-product: dlpe-windows
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?tu+6aGt0+DQwt0K4atvtuMnGSnqvyGn7Ba7pHY2z4TiBjXs+qR5VmdBAmRk6?=
 =?us-ascii?Q?Z8DIgg8W3bkrwMiNlaiNXm83Aj+w+W+LvIqxDH/ZjM2Om+W8YITG9zL0KlsE?=
 =?us-ascii?Q?PR2Mh/N+gazQb6y/tQWcpkVxu801vm0ap3qcEZjw75FGpuQpVG1EutahJ596?=
 =?us-ascii?Q?+QE2Gv+slyWN/XpIZ1PTBKuuFDLgzVg/BH/um3tP+2QJQ/eHbwl1BlR/KHCn?=
 =?us-ascii?Q?XkkuXtnaeZ7+Zd7ln/o+6eKhpjy9Ip/dHZyUVNg7l7WpU1MwcyRwX7IoYHJ1?=
 =?us-ascii?Q?89RAVpoeWOLHpokXLRjlA1dLKpUStl1V8+uMhtP8Mo8TysL5IFOISeN3dGTg?=
 =?us-ascii?Q?pebWc9TaQ+u9KAN342yc5rgN1/rRRjDw6Q79OzuiXJZnk17ZlvWr61T/OPcp?=
 =?us-ascii?Q?ZHvJY0k2c+IPbVuDwDORQO7bwSZNmT+o2Y6wUAryGdbiMQiqAlCD28alRAZ7?=
 =?us-ascii?Q?0H/TD3ydUfSoqR8kNgx4xifmWKVt6Vf66BfZatUwsF4TjirXK1OPcL5wxtru?=
 =?us-ascii?Q?BksICNnO//F0y+GU9fmRsZC8OxDgsP8nLYpUMyOLkJJb0govJBfbffdF3Xjf?=
 =?us-ascii?Q?Mqmdg+7SZhzCYRC49NyMmlPg4WJ5Eglwl34A2SGZzTcDkMyX755TQcgl8u3k?=
 =?us-ascii?Q?iE8Srw608nj8CpWyVZ6rXi3HXI/2GRXp8gCPX1OiKJ+6x/Ck6nSJHnK07SuH?=
 =?us-ascii?Q?G/WXIuYtYPauRNRTlfU2uyXeaJ/gbbEfpwV3zskLHHYrXQWvW7BL3iRexVgG?=
 =?us-ascii?Q?v9uKYqrfmE5xB+XBMX5EhQmYhiIAnu0TaK/3342ZHCtNb6SywDcZ9IhcPQ6m?=
 =?us-ascii?Q?aEJTmBWxOym+13kU6LTpnXJRX/wVUwk/CysO69Of1l3k7nvcIIFiUMwnatmU?=
 =?us-ascii?Q?8bMpkk0kQJ903hhxH8GMzc6ruF/BPta7h6U60TS71Y/67+zHhOhhKum27wo8?=
 =?us-ascii?Q?5Q6+bE0n6vYcUTJajRZDoxFrjdXKrvS5MPTIwwRbvMggjIrX1s/LW5GhChxJ?=
 =?us-ascii?Q?5+6pIZ3JUVDRzHmnSmvghyV2NpJMfgQ4QI4wukxqD+3ijOYnvAbH29khVsMT?=
 =?us-ascii?Q?HPYA2tnn6Ri207DZ8Z5tD+VHGcqN9AZWpBbT0gqWxdeBMAbDjaiTqCLvvXI1?=
 =?us-ascii?Q?e8dqZYBHrjaQGHwNxZbxYFKSSVRlZ8eul4/7PtJmhzTNnGCL83hUorfrUhef?=
 =?us-ascii?Q?7XWaO6IhNYJ1bmlLMncEqAV7nBWbt2MonLHa3uEsMHgceuqbeiVXg4YZDvBD?=
 =?us-ascii?Q?FY6EqZCYpLuDeJeB4BGrJBaRghd6VTCeQL3zobiEOJ2lGrP3DDnRcK0OTLJm?=
 =?us-ascii?Q?p+cR6iBdrwuwjlpuaUjYZOnE?=
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB5453.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4be4a210-c5f7-49b2-d3fb-08d94c2e9745
X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Jul 2021 10:02:03.8513
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: /e6MpeGZ+cI+mZ/rXVQ//TPjz8ALt2LbMibJomJJVkICEugJoi91zNCZRHDOAnrCpMHLW6cCGu2RQLHFxxs3MQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5551
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> -----Original Message-----
> From: Sean Christopherson <seanjc@google.com>
> Sent: Tuesday, June 22, 2021 01:08
> To: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>; Wanpeng Li
> <wanpengli@tencent.com>; Jim Mattson <jmattson@google.com>; Joerg
> Roedel <joro@8bytes.org>; kvm@vger.kernel.org; linux-kernel@vger.kernel.o=
rg
> Subject: Re: [PATCH] KVM: nVMX: Dynamically compute max VMCS index for
> vmcs12
>=20
> On Mon, Jun 21, 2021, Paolo Bonzini wrote:
> > On 18/06/21 23:46, Sean Christopherson wrote:
> > > Calculate the max VMCS index for vmcs12 by walking the array to find
> > > the actual max index.  Hardcoding the index is prone to bitrot, and
> > > the calculation is only done on KVM bringup (albeit on every CPU,
> > > but there aren't _that_ many null entries in the array).
> > >
> > > Fixes: 3c0f99366e34 ("KVM: nVMX: Add a TSC multiplier field in
> > > VMCS12")
> > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > > ---
> > >
> > > Note, the vmx test in kvm-unit-tests will still fail using stock
> > > QEMU, as QEMU also hardcodes and overwrites the MSR.  The test
> > > passes if I hack KVM to ignore userspace (it was easier than rebuildi=
ng
> QEMU).
> >
> > Queued, thanks.  Without having checked the kvm-unit-tests sources
> > very thoroughly, this might be a configuration issue in
> > kvm-unit-tests; in theory "-cpu host" (unlike "-cpu
> > host,migratable=3Dno") should not enable TSC scaling.
>=20
> As noted in the code comments, KVM allows VMREAD/VMWRITE to all defined
> fields, whether or not the field should actually exist for the vCPU model=
 doesn't
> enter into the equation.  That's technically wrong as there are a number =
of
> fields that the SDM explicitly states exist iff a certain feature is supp=
orted. =20
[Hu, Robert]=20
It's right that a number of fields' existence depends on some certain featu=
re.
Meanwhile, "IA32_VMX_VMCS_ENUM indicates to software the highest index
value used in the encoding of any field *supported* by the processor", rath=
er than
*existed*.
So my understanding is no matter what VMCS exec control field's value is se=
t, value
of IA32_VMX_VMCS_ENUM shall not be affected, as it reports the physical CPU=
's capability
rather than runtime VMCS configuration.
Back to nested case, L1's VMCS configuration lays the "physical" capability=
 for L2, right?
nested_vmx_msrs or at least nested_vmx_msrs.vmcs_enum shall be put to vcpu
scope, rather than current kvm global.
Current nested_vmx_calc_vmcs_enum_msr() is invoked at early stage, before v=
cpu features
are settled. I think should be moved to later stage as well.
=20
> To fix that we'd need to add a "feature flag" to vmcs_field_to_offset_tab=
le that is
> checked against the vCPU model, though updating the MSR would probably fa=
ll
> onto userspace's shoulders?
>=20
> And FWIW, this is the QEMU code:
>=20
>   #define VMCS12_MAX_FIELD_INDEX (0x17)
>=20
>   static void kvm_msr_entry_add_vmx(X86CPU *cpu, FeatureWordArray f)
>   {
>       ...
>=20
>       /*
>        * Just to be safe, write these with constant values.  The CRn_FIXE=
D1
>        * MSRs are generated by KVM based on the vCPU's CPUID.
>        */
>       kvm_msr_entry_add(cpu, MSR_IA32_VMX_CR0_FIXED0,
>                         CR0_PE_MASK | CR0_PG_MASK | CR0_NE_MASK);
>       kvm_msr_entry_add(cpu, MSR_IA32_VMX_CR4_FIXED0,
>                         CR4_VMXE_MASK);
>       kvm_msr_entry_add(cpu, MSR_IA32_VMX_VMCS_ENUM,
>                         VMCS12_MAX_FIELD_INDEX << 1);
>   }