Re: PATCH: nVMX: Better MSR_IA32_FEATURE_CONTROL handling

[Avi Kivity <avi@redhat.com>]

On 03/15/2012 07:40 PM, Nadav Har'El wrote:
> 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?

vmx.c is not about guest-side Intel features, it's about vmx specific
implementation of x86 features.  Whenever possible we put things in
x86.c so if the other vendor picks up the feature, it will just work.

Nested vmx is of course impossible to put in x86.c.  This MSR is
somewhat on the borderline, there's nothing vendor specific about the
_implementation_, but it all the features it controls are very vendor
specific.  So I guess it can stay in vmx.c, if you can make it available
for save/restore.

> 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?

The same reasoning applies.

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

Yes.

-- 
error compiling committee.c: too many arguments to function