Re: [PATCH] KVM: Enable VMX-related bits in MSR_IA32_FEATURE_CONTROL.

[Julian Stecklina <js@alien8.de>]

[-- Attachment #1: Type: text/plain, Size: 2291 bytes --]

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 #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]