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

Reply via email to