Re: [PATCH 1/4] KVM: nVMX: support restore of VMX capability MSRs

[David Matlack]

On Tue, Nov 29, 2016 at 12:01 AM, Paolo Bonzini  wrote:
>> On Mon, Nov 28, 2016 at 2:48 PM, Paolo Bonzini  wrote:
>> > On 28/11/2016 22:11, David Matlack wrote:
>> >> > PINBASED_CTLS, PROCBASED_CTLS, EXIT_CTLS and ENTRY_CTLS can be derived
>> >> > from their "true" counterparts, so I think it's better to remove the
>> >> > "non-true" ones from struct nested_vmx (and/or add the "true" ones when
>> >> > missing) and make them entirely computed.  But it can be done on top.
>> >>
>> >> Good point. And that would mean userspace does not need to restore the
>> >> non-true MSRs, right?
>> >
>> > Yes, sorry for being a bit too concise. :)
>>
>> I'll include this cleanup in the next version of the patchset since it
>> affects which MSRs userspace will restore. It looks like a pretty
>> simple patch.
>
> Don't bother removing the "non-true" registers from nested_vmx; you only
> need to adjust the userspace API.

I already wrote the patch, so unless there's an argument against
removing them I'll include it in the next patchset. Thanks!

>
>> >
>> >> KVM does not emulate MSR_IA32_VMX_BASIC[55]=0,
>> >> and will probably never want to.
>> >
>> > That's a separate question, MSR_IA32_VMX_BASIC[55]=0 basically means
>> > that the "true" capabilities are the same as the "default" capabilities.
>> >  If userspace wanted to set it that way, KVM right now would not hide
>> > the "true" capability MSR, but on the other hand the nested hypervisor
>> > should not even notice the difference.
>>
>> KVM would also need to use the non-true MSR in place of the true MSRs
>> when checking VMCS12 during VM-entry.
>
> It's not necessary, userspace would set the relevant bits to 1 in the true
> MSRs, for both the low and high parts.  If it doesn't, it's garbage in
> garbage out.
>
> Paolo

Re: [PATCH 1/4] KVM: nVMX: support restore of VMX capability MSRs

[David Matlack]

On Tue, Nov 29, 2016 at 12:01 AM, Paolo Bonzini  wrote:
>> On Mon, Nov 28, 2016 at 2:48 PM, Paolo Bonzini  wrote:
>> > On 28/11/2016 22:11, David Matlack wrote:
>> >> > PINBASED_CTLS, PROCBASED_CTLS, EXIT_CTLS and ENTRY_CTLS can be derived
>> >> > from their "true" counterparts, so I think it's better to remove the
>> >> > "non-true" ones from struct nested_vmx (and/or add the "true" ones when
>> >> > missing) and make them entirely computed.  But it can be done on top.
>> >>
>> >> Good point. And that would mean userspace does not need to restore the
>> >> non-true MSRs, right?
>> >
>> > Yes, sorry for being a bit too concise. :)
>>
>> I'll include this cleanup in the next version of the patchset since it
>> affects which MSRs userspace will restore. It looks like a pretty
>> simple patch.
>
> Don't bother removing the "non-true" registers from nested_vmx; you only
> need to adjust the userspace API.

I already wrote the patch, so unless there's an argument against
removing them I'll include it in the next patchset. Thanks!

>
>> >
>> >> KVM does not emulate MSR_IA32_VMX_BASIC[55]=0,
>> >> and will probably never want to.
>> >
>> > That's a separate question, MSR_IA32_VMX_BASIC[55]=0 basically means
>> > that the "true" capabilities are the same as the "default" capabilities.
>> >  If userspace wanted to set it that way, KVM right now would not hide
>> > the "true" capability MSR, but on the other hand the nested hypervisor
>> > should not even notice the difference.
>>
>> KVM would also need to use the non-true MSR in place of the true MSRs
>> when checking VMCS12 during VM-entry.
>
> It's not necessary, userspace would set the relevant bits to 1 in the true
> MSRs, for both the low and high parts.  If it doesn't, it's garbage in
> garbage out.
>
> Paolo

Re: [PATCH 1/4] KVM: nVMX: support restore of VMX capability MSRs

[Paolo Bonzini]

> On Mon, Nov 28, 2016 at 2:48 PM, Paolo Bonzini  wrote:
> > On 28/11/2016 22:11, David Matlack wrote:
> >> > PINBASED_CTLS, PROCBASED_CTLS, EXIT_CTLS and ENTRY_CTLS can be derived
> >> > from their "true" counterparts, so I think it's better to remove the
> >> > "non-true" ones from struct nested_vmx (and/or add the "true" ones when
> >> > missing) and make them entirely computed.  But it can be done on top.
> >>
> >> Good point. And that would mean userspace does not need to restore the
> >> non-true MSRs, right?
> >
> > Yes, sorry for being a bit too concise. :)
> 
> I'll include this cleanup in the next version of the patchset since it
> affects which MSRs userspace will restore. It looks like a pretty
> simple patch.

Don't bother removing the "non-true" registers from nested_vmx; you only
need to adjust the userspace API.

> >
> >> KVM does not emulate MSR_IA32_VMX_BASIC[55]=0,
> >> and will probably never want to.
> >
> > That's a separate question, MSR_IA32_VMX_BASIC[55]=0 basically means
> > that the "true" capabilities are the same as the "default" capabilities.
> >  If userspace wanted to set it that way, KVM right now would not hide
> > the "true" capability MSR, but on the other hand the nested hypervisor
> > should not even notice the difference.
> 
> KVM would also need to use the non-true MSR in place of the true MSRs
> when checking VMCS12 during VM-entry.

It's not necessary, userspace would set the relevant bits to 1 in the true
MSRs, for both the low and high parts.  If it doesn't, it's garbage in
garbage out.

Paolo

Re: [PATCH 1/4] KVM: nVMX: support restore of VMX capability MSRs

[Paolo Bonzini]

> On Mon, Nov 28, 2016 at 2:48 PM, Paolo Bonzini  wrote:
> > On 28/11/2016 22:11, David Matlack wrote:
> >> > PINBASED_CTLS, PROCBASED_CTLS, EXIT_CTLS and ENTRY_CTLS can be derived
> >> > from their "true" counterparts, so I think it's better to remove the
> >> > "non-true" ones from struct nested_vmx (and/or add the "true" ones when
> >> > missing) and make them entirely computed.  But it can be done on top.
> >>
> >> Good point. And that would mean userspace does not need to restore the
> >> non-true MSRs, right?
> >
> > Yes, sorry for being a bit too concise. :)
> 
> I'll include this cleanup in the next version of the patchset since it
> affects which MSRs userspace will restore. It looks like a pretty
> simple patch.

Don't bother removing the "non-true" registers from nested_vmx; you only
need to adjust the userspace API.

> >
> >> KVM does not emulate MSR_IA32_VMX_BASIC[55]=0,
> >> and will probably never want to.
> >
> > That's a separate question, MSR_IA32_VMX_BASIC[55]=0 basically means
> > that the "true" capabilities are the same as the "default" capabilities.
> >  If userspace wanted to set it that way, KVM right now would not hide
> > the "true" capability MSR, but on the other hand the nested hypervisor
> > should not even notice the difference.
> 
> KVM would also need to use the non-true MSR in place of the true MSRs
> when checking VMCS12 during VM-entry.

It's not necessary, userspace would set the relevant bits to 1 in the true
MSRs, for both the low and high parts.  If it doesn't, it's garbage in
garbage out.

Paolo

Re: [PATCH 1/4] KVM: nVMX: support restore of VMX capability MSRs

[David Matlack]

On Mon, Nov 28, 2016 at 2:48 PM, Paolo Bonzini  wrote:
> On 28/11/2016 22:11, David Matlack wrote:
>> > PINBASED_CTLS, PROCBASED_CTLS, EXIT_CTLS and ENTRY_CTLS can be derived
>> > from their "true" counterparts, so I think it's better to remove the
>> > "non-true" ones from struct nested_vmx (and/or add the "true" ones when
>> > missing) and make them entirely computed.  But it can be done on top.
>>
>> Good point. And that would mean userspace does not need to restore the
>> non-true MSRs, right?
>
> Yes, sorry for being a bit too concise. :)

I'll include this cleanup in the next version of the patchset since it
affects which MSRs userspace will restore. It looks like a pretty
simple patch.

>
>> KVM does not emulate MSR_IA32_VMX_BASIC[55]=0,
>> and will probably never want to.
>
> That's a separate question, MSR_IA32_VMX_BASIC[55]=0 basically means
> that the "true" capabilities are the same as the "default" capabilities.
>  If userspace wanted to set it that way, KVM right now would not hide
> the "true" capability MSR, but on the other hand the nested hypervisor
> should not even notice the difference.

KVM would also need to use the non-true MSR in place of the true MSRs
when checking VMCS12 during VM-entry.

>
> Paolo

Re: [PATCH 1/4] KVM: nVMX: support restore of VMX capability MSRs

[David Matlack]

On Mon, Nov 28, 2016 at 2:48 PM, Paolo Bonzini  wrote:
> On 28/11/2016 22:11, David Matlack wrote:
>> > PINBASED_CTLS, PROCBASED_CTLS, EXIT_CTLS and ENTRY_CTLS can be derived
>> > from their "true" counterparts, so I think it's better to remove the
>> > "non-true" ones from struct nested_vmx (and/or add the "true" ones when
>> > missing) and make them entirely computed.  But it can be done on top.
>>
>> Good point. And that would mean userspace does not need to restore the
>> non-true MSRs, right?
>
> Yes, sorry for being a bit too concise. :)

I'll include this cleanup in the next version of the patchset since it
affects which MSRs userspace will restore. It looks like a pretty
simple patch.

>
>> KVM does not emulate MSR_IA32_VMX_BASIC[55]=0,
>> and will probably never want to.
>
> That's a separate question, MSR_IA32_VMX_BASIC[55]=0 basically means
> that the "true" capabilities are the same as the "default" capabilities.
>  If userspace wanted to set it that way, KVM right now would not hide
> the "true" capability MSR, but on the other hand the nested hypervisor
> should not even notice the difference.

KVM would also need to use the non-true MSR in place of the true MSRs
when checking VMCS12 during VM-entry.

>
> Paolo

Re: [PATCH 1/4] KVM: nVMX: support restore of VMX capability MSRs

[Paolo Bonzini]

On 28/11/2016 22:11, David Matlack wrote:
> > PINBASED_CTLS, PROCBASED_CTLS, EXIT_CTLS and ENTRY_CTLS can be derived
> > from their "true" counterparts, so I think it's better to remove the
> > "non-true" ones from struct nested_vmx (and/or add the "true" ones when
> > missing) and make them entirely computed.  But it can be done on top.
>
> Good point. And that would mean userspace does not need to restore the
> non-true MSRs, right?

Yes, sorry for being a bit too concise. :)

> KVM does not emulate MSR_IA32_VMX_BASIC[55]=0,
> and will probably never want to.

That's a separate question, MSR_IA32_VMX_BASIC[55]=0 basically means
that the "true" capabilities are the same as the "default" capabilities.
 If userspace wanted to set it that way, KVM right now would not hide
the "true" capability MSR, but on the other hand the nested hypervisor
should not even notice the difference.

Paolo

Re: [PATCH 1/4] KVM: nVMX: support restore of VMX capability MSRs

[Paolo Bonzini]

On 28/11/2016 22:11, David Matlack wrote:
> > PINBASED_CTLS, PROCBASED_CTLS, EXIT_CTLS and ENTRY_CTLS can be derived
> > from their "true" counterparts, so I think it's better to remove the
> > "non-true" ones from struct nested_vmx (and/or add the "true" ones when
> > missing) and make them entirely computed.  But it can be done on top.
>
> Good point. And that would mean userspace does not need to restore the
> non-true MSRs, right?

Yes, sorry for being a bit too concise. :)

> KVM does not emulate MSR_IA32_VMX_BASIC[55]=0,
> and will probably never want to.

That's a separate question, MSR_IA32_VMX_BASIC[55]=0 basically means
that the "true" capabilities are the same as the "default" capabilities.
 If userspace wanted to set it that way, KVM right now would not hide
the "true" capability MSR, but on the other hand the nested hypervisor
should not even notice the difference.

Paolo

Re: [PATCH 1/4] KVM: nVMX: support restore of VMX capability MSRs

[David Matlack]

On Wed, Nov 23, 2016 at 3:44 AM, Paolo Bonzini  wrote:
> On 23/11/2016 02:14, David Matlack wrote:
>>   switch (msr_index) {
>>   case MSR_IA32_VMX_BASIC:
>> + return vmx_restore_vmx_basic(vmx, data);
>> + case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
>> + case MSR_IA32_VMX_PINBASED_CTLS:
>> + case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
>> + case MSR_IA32_VMX_PROCBASED_CTLS:
>> + case MSR_IA32_VMX_TRUE_EXIT_CTLS:
>> + case MSR_IA32_VMX_EXIT_CTLS:
>> + case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
>> + case MSR_IA32_VMX_ENTRY_CTLS:
>
> PINBASED_CTLS, PROCBASED_CTLS, EXIT_CTLS and ENTRY_CTLS can be derived
> from their "true" counterparts, so I think it's better to remove the
> "non-true" ones from struct nested_vmx (and/or add the "true" ones when
> missing) and make them entirely computed.  But it can be done on top.

Good point. And that would mean userspace does not need to restore the
non-true MSRs, right? KVM does not emulate MSR_IA32_VMX_BASIC[55]=0,
and will probably never want to.

Re: [PATCH 1/4] KVM: nVMX: support restore of VMX capability MSRs

[David Matlack]

On Wed, Nov 23, 2016 at 3:44 AM, Paolo Bonzini  wrote:
> On 23/11/2016 02:14, David Matlack wrote:
>>   switch (msr_index) {
>>   case MSR_IA32_VMX_BASIC:
>> + return vmx_restore_vmx_basic(vmx, data);
>> + case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
>> + case MSR_IA32_VMX_PINBASED_CTLS:
>> + case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
>> + case MSR_IA32_VMX_PROCBASED_CTLS:
>> + case MSR_IA32_VMX_TRUE_EXIT_CTLS:
>> + case MSR_IA32_VMX_EXIT_CTLS:
>> + case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
>> + case MSR_IA32_VMX_ENTRY_CTLS:
>
> PINBASED_CTLS, PROCBASED_CTLS, EXIT_CTLS and ENTRY_CTLS can be derived
> from their "true" counterparts, so I think it's better to remove the
> "non-true" ones from struct nested_vmx (and/or add the "true" ones when
> missing) and make them entirely computed.  But it can be done on top.

Good point. And that would mean userspace does not need to restore the
non-true MSRs, right? KVM does not emulate MSR_IA32_VMX_BASIC[55]=0,
and will probably never want to.

Re: [PATCH 1/4] KVM: nVMX: support restore of VMX capability MSRs

[Paolo Bonzini]

On 23/11/2016 02:14, David Matlack wrote:
> The VMX capability MSRs advertise the set of features the KVM virtual
> CPU can support. This set of features vary across different host CPUs
> and KVM versions. This patch aims to addresses both sources of
> differences, allowing VMs to be migrated across CPUs and KVM versions
> without guest-visible changes to these MSRs. Note that cross-KVM-
> version migration is only supported from this point forward.
> 
> When the VMX capability MSRs are restored, they are audited to check
> that the set of features advertised are a subset of what KVM and the
> CPU support.
> 
> Since the VMX capability MSRs are read-only, they do not need to be on
> the default MSR save/restore lists. The userspace hypervisor can set
> the values of these MSRs or read them from KVM at VCPU creation time,
> and restore the same value after every save/restore.
> 
> Signed-off-by: David Matlack 
> ---
>  arch/x86/include/asm/vmx.h |  31 +
>  arch/x86/kvm/vmx.c | 317 
> +
>  2 files changed, 324 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index a002b07..a4ca897 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -25,6 +25,7 @@
>  #define VMX_H
>  
>  
> +#include 
>  #include 
>  #include 
>  
> @@ -110,6 +111,36 @@
>  #define VMX_MISC_SAVE_EFER_LMA   0x0020
>  #define VMX_MISC_ACTIVITY_HLT0x0040
>  
> +static inline u32 vmx_basic_vmcs_revision_id(u64 vmx_basic)
> +{
> + return vmx_basic & GENMASK_ULL(30, 0);
> +}
> +
> +static inline u32 vmx_basic_vmcs_size(u64 vmx_basic)
> +{
> + return (vmx_basic & GENMASK_ULL(44, 32)) >> 32;
> +}
> +
> +static inline int vmx_misc_preemption_timer_rate(u64 vmx_misc)
> +{
> + return vmx_misc & VMX_MISC_PREEMPTION_TIMER_RATE_MASK;
> +}
> +
> +static inline int vmx_misc_cr3_count(u64 vmx_misc)
> +{
> + return (vmx_misc & GENMASK_ULL(24, 16)) >> 16;
> +}
> +
> +static inline int vmx_misc_max_msr(u64 vmx_misc)
> +{
> + return (vmx_misc & GENMASK_ULL(27, 25)) >> 25;
> +}
> +
> +static inline int vmx_misc_mseg_revid(u64 vmx_misc)
> +{
> + return (vmx_misc & GENMASK_ULL(63, 32)) >> 32;
> +}
> +
>  /* VMCS Encodings */
>  enum vmcs_field {
>   VIRTUAL_PROCESSOR_ID= 0x,
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 5382b82..6ec3832 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -463,6 +463,12 @@ struct nested_vmx {
>   u32 nested_vmx_misc_high;
>   u32 nested_vmx_ept_caps;

> +/*
> + * Called when userspace is restoring VMX MSRs.
> + *
> + * Returns 0 on success, non-0 otherwise.
> + */
> +static int vmx_set_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data)
>  {
>   struct vcpu_vmx *vmx = to_vmx(vcpu);
>  
>   switch (msr_index) {
>   case MSR_IA32_VMX_BASIC:
> + return vmx_restore_vmx_basic(vmx, data);
> + case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
> + case MSR_IA32_VMX_PINBASED_CTLS:
> + case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
> + case MSR_IA32_VMX_PROCBASED_CTLS:
> + case MSR_IA32_VMX_TRUE_EXIT_CTLS:
> + case MSR_IA32_VMX_EXIT_CTLS:
> + case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
> + case MSR_IA32_VMX_ENTRY_CTLS:

PINBASED_CTLS, PROCBASED_CTLS, EXIT_CTLS and ENTRY_CTLS can be derived
from their "true" counterparts, so I think it's better to remove the
"non-true" ones from struct nested_vmx (and/or add the "true" ones when
missing) and make them entirely computed.  But it can be done on top.

Paolo

> + case MSR_IA32_VMX_PROCBASED_CTLS2:
> + return vmx_restore_control_msr(vmx, msr_index, data);
> + case MSR_IA32_VMX_MISC:
> + return vmx_restore_vmx_misc(vmx, data);
> + case MSR_IA32_VMX_CR0_FIXED0:
> + case MSR_IA32_VMX_CR4_FIXED0:
> + return vmx_restore_fixed0_msr(vmx, msr_index, data);
> + case MSR_IA32_VMX_CR0_FIXED1:
> + case MSR_IA32_VMX_CR4_FIXED1:
> + return vmx_restore_fixed1_msr(vmx, msr_index, data);
> + case MSR_IA32_VMX_EPT_VPID_CAP:
> + return vmx_restore_vmx_ept_vpid_cap(vmx, data);
> + case MSR_IA32_VMX_VMCS_ENUM:
> + vmx->nested.nested_vmx_vmcs_enum = data;
> + return 0;
> + default:
>   /*
> -  * This MSR reports some information about VMX support. We
> -  * should return information about the VMX we emulate for the
> -  * guest, and the VMCS structure we give it - not about the
> -  * VMX support of the underlying hardware.
> +  * The rest of the VMX capability MSRs do not support restore.
>*/
> - *pdata = VMCS12_REVISION | VMX_BASIC_TRUE_CTLS |
> -((u64)VMCS12_SIZE << VMX_BASIC_VMCS_SIZE_SHIFT) |
> -(VMX_BASIC_MEM_TYPE_WB << 

Re: [PATCH 1/4] KVM: nVMX: support restore of VMX capability MSRs

[Paolo Bonzini]

On 23/11/2016 02:14, David Matlack wrote:
> The VMX capability MSRs advertise the set of features the KVM virtual
> CPU can support. This set of features vary across different host CPUs
> and KVM versions. This patch aims to addresses both sources of
> differences, allowing VMs to be migrated across CPUs and KVM versions
> without guest-visible changes to these MSRs. Note that cross-KVM-
> version migration is only supported from this point forward.
> 
> When the VMX capability MSRs are restored, they are audited to check
> that the set of features advertised are a subset of what KVM and the
> CPU support.
> 
> Since the VMX capability MSRs are read-only, they do not need to be on
> the default MSR save/restore lists. The userspace hypervisor can set
> the values of these MSRs or read them from KVM at VCPU creation time,
> and restore the same value after every save/restore.
> 
> Signed-off-by: David Matlack 
> ---
>  arch/x86/include/asm/vmx.h |  31 +
>  arch/x86/kvm/vmx.c | 317 
> +
>  2 files changed, 324 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index a002b07..a4ca897 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -25,6 +25,7 @@
>  #define VMX_H
>  
>  
> +#include 
>  #include 
>  #include 
>  
> @@ -110,6 +111,36 @@
>  #define VMX_MISC_SAVE_EFER_LMA   0x0020
>  #define VMX_MISC_ACTIVITY_HLT0x0040
>  
> +static inline u32 vmx_basic_vmcs_revision_id(u64 vmx_basic)
> +{
> + return vmx_basic & GENMASK_ULL(30, 0);
> +}
> +
> +static inline u32 vmx_basic_vmcs_size(u64 vmx_basic)
> +{
> + return (vmx_basic & GENMASK_ULL(44, 32)) >> 32;
> +}
> +
> +static inline int vmx_misc_preemption_timer_rate(u64 vmx_misc)
> +{
> + return vmx_misc & VMX_MISC_PREEMPTION_TIMER_RATE_MASK;
> +}
> +
> +static inline int vmx_misc_cr3_count(u64 vmx_misc)
> +{
> + return (vmx_misc & GENMASK_ULL(24, 16)) >> 16;
> +}
> +
> +static inline int vmx_misc_max_msr(u64 vmx_misc)
> +{
> + return (vmx_misc & GENMASK_ULL(27, 25)) >> 25;
> +}
> +
> +static inline int vmx_misc_mseg_revid(u64 vmx_misc)
> +{
> + return (vmx_misc & GENMASK_ULL(63, 32)) >> 32;
> +}
> +
>  /* VMCS Encodings */
>  enum vmcs_field {
>   VIRTUAL_PROCESSOR_ID= 0x,
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 5382b82..6ec3832 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -463,6 +463,12 @@ struct nested_vmx {
>   u32 nested_vmx_misc_high;
>   u32 nested_vmx_ept_caps;

> +/*
> + * Called when userspace is restoring VMX MSRs.
> + *
> + * Returns 0 on success, non-0 otherwise.
> + */
> +static int vmx_set_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data)
>  {
>   struct vcpu_vmx *vmx = to_vmx(vcpu);
>  
>   switch (msr_index) {
>   case MSR_IA32_VMX_BASIC:
> + return vmx_restore_vmx_basic(vmx, data);
> + case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
> + case MSR_IA32_VMX_PINBASED_CTLS:
> + case MSR_IA32_VMX_TRUE_PROCBASED_CTLS:
> + case MSR_IA32_VMX_PROCBASED_CTLS:
> + case MSR_IA32_VMX_TRUE_EXIT_CTLS:
> + case MSR_IA32_VMX_EXIT_CTLS:
> + case MSR_IA32_VMX_TRUE_ENTRY_CTLS:
> + case MSR_IA32_VMX_ENTRY_CTLS:

PINBASED_CTLS, PROCBASED_CTLS, EXIT_CTLS and ENTRY_CTLS can be derived
from their "true" counterparts, so I think it's better to remove the
"non-true" ones from struct nested_vmx (and/or add the "true" ones when
missing) and make them entirely computed.  But it can be done on top.

Paolo

> + case MSR_IA32_VMX_PROCBASED_CTLS2:
> + return vmx_restore_control_msr(vmx, msr_index, data);
> + case MSR_IA32_VMX_MISC:
> + return vmx_restore_vmx_misc(vmx, data);
> + case MSR_IA32_VMX_CR0_FIXED0:
> + case MSR_IA32_VMX_CR4_FIXED0:
> + return vmx_restore_fixed0_msr(vmx, msr_index, data);
> + case MSR_IA32_VMX_CR0_FIXED1:
> + case MSR_IA32_VMX_CR4_FIXED1:
> + return vmx_restore_fixed1_msr(vmx, msr_index, data);
> + case MSR_IA32_VMX_EPT_VPID_CAP:
> + return vmx_restore_vmx_ept_vpid_cap(vmx, data);
> + case MSR_IA32_VMX_VMCS_ENUM:
> + vmx->nested.nested_vmx_vmcs_enum = data;
> + return 0;
> + default:
>   /*
> -  * This MSR reports some information about VMX support. We
> -  * should return information about the VMX we emulate for the
> -  * guest, and the VMCS structure we give it - not about the
> -  * VMX support of the underlying hardware.
> +  * The rest of the VMX capability MSRs do not support restore.
>*/
> - *pdata = VMCS12_REVISION | VMX_BASIC_TRUE_CTLS |
> -((u64)VMCS12_SIZE << VMX_BASIC_VMCS_SIZE_SHIFT) |
> -(VMX_BASIC_MEM_TYPE_WB <<