Re: PATCH: nVMX: Better MSR_IA32_FEATURE_CONTROL handling

[Nadav Har'El <nyh@math.technion.ac.il>]

Hi, in a minute I'll send a new version of the MSR_IA32_FEATURE_CONTROL
patch for nested VMX; I just wanted to reply first to your comments so
you'll know what to expect:

On Wed, Mar 07, 2012, Avi Kivity wrote about "Re: PATCH: nVMX: Better MSR_IA32_FEATURE_CONTROL handling":
> On 03/07/2012 05:58 PM, Nadav Har'El wrote:
> > +	u64 msr_ia32_feature_control;
> >  };
> 
> Need to add to the list of exported MSRs so it can be live migrated
> (msrs_to_save).

Did this.

> The variable itself should live in vcpu->arch, even if
> some bits are vendor specific.

But not this. I understand what you explained about vmx.c being for
Intel *hosts*, not about emulating Intel *guests*, but I do think that
since none of the bits in this MSR are relevant on AMD hosts (which
don't do nested VMX), it isn't useful to support this MSR outside vmx.c.

So I left this variable it in vmx->nested. As I noted earlier, svm.c
did exactly the same thing (nested.vm_cr_msr), so at least there's
symmetry here.

> > @@ -1999,7 +2000,7 @@ static int vmx_get_vmx_msr(struct kvm_vc
> >  
> >  	switch (msr_index) {
> >  	case MSR_IA32_FEATURE_CONTROL:
> > -		*pdata = 0;
> > +		*pdata = to_vmx(vcpu)->nested.msr_ia32_feature_control;
> >  		break;
> 
> In a separate patch, please move this outside vmx_get_vmx_msr().  It's
> not a vmx msr.

Done, but not split into two patches: The patch removes the old case in
vmx_get_vmx_msr() (and also removes vmx_set_vmx_msr() entirely) and
instead adds the case in vmx_get_msr() and vmx_set_msr().

> > +#define VMXON_NEEDED_FEATURES \
> > +	  (FEATURE_CONTROL_LOCKED | FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX)
> 
> Use const u64 instead of #define please, it jars my eyes.

I would, if Linux coding style allowed to declare variables in the
middle of blocks. Unfortunately it doesn't, so I left this #define.
I don't think it's that bad.

-- 
Nadav Har'El                        |                    Monday, Mar 19 2012, 
nyh@math.technion.ac.il             |-----------------------------------------
Phone +972-523-790466, ICQ 13349191 |A conscience does not prevent sin. It
http://nadav.harel.org.il           |only prevents you from enjoying it.