Am Dienstag, den 06.03.2012, 17:47 +0200 schrieb Nadav Har'El: > On Tue, Mar 06, 2012, Avi Kivity wrote about "Re: [PATCH] KVM: Enable VMX-related bits in MSR_IA32_FEATURE_CONTROL.": > > > case MSR_IA32_FEATURE_CONTROL: > > > - *pdata = 0; > > > + /* > > > + * If nested VMX is enabled, set the lock bit (bit 0) > > > + * and the "Enable VMX outside SMX" bit (bit 2) in the > > > + * FEATURE_CONTROL MSR. > > > + */ > > > + *pdata = nested_vmx_allowed(vcpu) ? 0x5 : 0; > > 0x5 can be written as FEATURE_CONTROL_LOCKED | > FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX Nice. Didn't know those constants. Next time I'll try harder to find those. :) > > > > break; > > > case MSR_IA32_VMX_BASIC: > > > /* > > > > The way I read it, it should be done by the guest, not the host. > > This is also how I understand it. Check out KVM's own hardware_enable() > to see how a guest does turn on these bits before using VMXON - it > doesn't need to rely on the BIOS to have done it earlier (the BIOS, can, > however, prevent the guest from doing this by setting only the lock bit). After looking through the code (vmx_disabled_by_bios), it seems that KVM doesn't bother with FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX if FEATURE_CONTROL_LOCKED is not set. It seems like our kernel should do the same. Sorry for the noise. > What is true, however, is that the existing code is probably incomplete > in three ways (see section 20.7 of the SDM): > > 1. It always returns 0 for this MSR, even if the guest previously set it > to something else. > > 2. handle_vmon() does not check the previous setting of this MSR. > If the guest (or its BIOS) doesn't set both FEATURE_CONTROL_LOCKED > and FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX, it should get a > #GP on an attempt to VMXON. This will allow L1's BIOS to disable > nested VMX if it wishes (not that I think this is a very useful > usecase...). > > 3. vmx_set_vmx_msr to MSR_IA32_FEATURE_CONTROL should throw a #GP if > FEATURE_CONTROL_LOCKED is on. > > I'll create a patch to do this shortly. This is greatly appreciated! Regards, Julian
Attachment:
signature.asc
Description: This is a digitally signed message part