Re: PATCH: nVMX: Better MSR_IA32_FEATURE_CONTROL handling

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

On Wed, Mar 07, 2012, Avi Kivity wrote about "Re: PATCH: nVMX: Better MSR_IA32_FEATURE_CONTROL handling":
> >  	struct page *apic_access_page;
> > +	u64 msr_ia32_feature_control;
> >  };
>...
> (msrs_to_save).  The variable itself should live in vcpu->arch, even if
> some bits are vendor specific.

Does this MSR exist in AMD? I was under the impression that it is an
Intel-only MSR, and that AMD has something different, the VM_CR MSR,
so it didn't make sense to put this in vcpu->arch. Is my impression
wrong?

I seems, by the way, that svm.c has vm_cr_msr in svm->nested, basically the
same what I did, not in vcpu->arch. Why is this bad?

Also, it seems that VM_CR is also not on the list on msrs_to_save.
A bug?

> > @@ -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.

I agree, I'll move it. But if it's a VMX-only MSR, I want to leave it
in vmx.c, and not move it to x86.c.

-- 
Nadav Har'El                        |                  Thursday, Mar 15 2012, 
nyh@math.technion.ac.il             |-----------------------------------------
Phone +972-523-790466, ICQ 13349191 |If I am not for myself, who will be for
http://nadav.harel.org.il           |me? If I am only for myself, who am I?