diff mbox series

KVM: VMX: Get rid of hard-coded value around IA32_VMX_BASIC

Message ID 20230330092149.101047-1-kai.huang@intel.com (mailing list archive)
State New, archived
Headers show
Series KVM: VMX: Get rid of hard-coded value around IA32_VMX_BASIC | expand

Commit Message

Huang, Kai March 30, 2023, 9:21 a.m. UTC 
Currently, setup_vmcs_config() uses hard-coded values when performing
sanity check on the value of IA32_VMX_BASIC MSR and setting up the three
members of 'vmcs_config': size, revison_id and basic_cap.  However, the
kernel actually already has macro definitions for those relevant bits in
asm/msr-index.h and functions to get revision_id and size in asm/vmx.h.

Add the missing helper function to get the basic_cap, and use those
macros and helper functions in setup_vmcs_config() to get rid of those
hard-coded values.

No functional change intended.

Signed-off-by: Kai Huang <kai.huang@intel.com>
---
 arch/x86/include/asm/vmx.h |  5 +++++
 arch/x86/kvm/vmx/vmx.c     | 17 +++++++++--------
 2 files changed, 14 insertions(+), 8 deletions(-)


base-commit: 99b30869804ea59d9596cdbefa5cc3aabd588521

Comments

Sean Christopherson April 5, 2023, 12:18 a.m. UTC | #1 
On Thu, Mar 30, 2023, Kai Huang wrote:
> Currently, setup_vmcs_config() uses hard-coded values when performing
> sanity check on the value of IA32_VMX_BASIC MSR and setting up the three
> members of 'vmcs_config': size, revison_id and basic_cap.  However, the
> kernel actually already has macro definitions for those relevant bits in
> asm/msr-index.h and functions to get revision_id and size in asm/vmx.h.
> 
> Add the missing helper function to get the basic_cap, and use those
> macros and helper functions in setup_vmcs_config() to get rid of those
> hard-coded values.
> 
> No functional change intended.
> 
> Signed-off-by: Kai Huang <kai.huang@intel.com>
> ---
>  arch/x86/include/asm/vmx.h |  5 +++++
>  arch/x86/kvm/vmx/vmx.c     | 17 +++++++++--------
>  2 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index 498dc600bd5c..2b488895fe0e 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -141,6 +141,11 @@ static inline u32 vmx_basic_vmcs_size(u64 vmx_basic)
>  	return (vmx_basic & GENMASK_ULL(44, 32)) >> 32;
>  }
>  
> +static inline u32 vmx_basic_cap(u64 vmx_basic)

I would omit this helper and just open code the ugliness, purely because what
it's tracking is completely random.

> +{
> +	return (vmx_basic & ~GENMASK_ULL(44, 32)) >> 32;
> +}
> +

...

>  #ifdef CONFIG_X86_64
>  	/* IA-32 SDM Vol 3B: 64-bit CPUs always have VMX_BASIC_MSR[48]==0. */
> -	if (vmx_msr_high & (1u<<16))
> +	if (basic_msr & VMX_BASIC_64)
>  		return -EIO;
>  #endif
>  
>  	/* Require Write-Back (WB) memory type for VMCS accesses. */
> -	if (((vmx_msr_high >> 18) & 15) != 6)
> +	if (((basic_msr & VMX_BASIC_MEM_TYPE_MASK) >> VMX_BASIC_MEM_TYPE_SHIFT)
> +			!= VMX_BASIC_MEM_TYPE_WB)

This one is worthy of a helper, e.g. 

	if (vmx_basic_vmcs_memtype(basic_msr) != VMX_BASIC_MEM_TYPE_WB)
		return -EIO;


>  		return -EIO;
>  
>  	rdmsrl(MSR_IA32_VMX_MISC, misc_msr);
>  
> -	vmcs_conf->size = vmx_msr_high & 0x1fff;
> -	vmcs_conf->basic_cap = vmx_msr_high & ~0x1fff;
> +	vmcs_conf->size = vmx_basic_vmcs_size(basic_msr);
> +	vmcs_conf->basic_cap = vmx_basic_cap(basic_msr);

Actually, looking at this more closely, splitting up the basic MSR during setup
is silly.  None of the usage is in hot paths, and cpu_has_vmx_basic_inout() is
downright bizarre with the current code.

Rather than do all of these weird dances, what about saving the full/raw MSR in
the config, and then using the helpers to extract info as needed?  E.g. the below
over a few patches.  As a bonus (maybe), KVM will sanity check the entire MSR
across CPUs.

---
 arch/x86/include/asm/vmx.h      |  5 +++++
 arch/x86/kvm/vmx/capabilities.h |  6 ++----
 arch/x86/kvm/vmx/vmx.c          | 22 +++++++++-------------
 3 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 498dc600bd5c..b21dcd128add 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -141,6 +141,11 @@ static inline u32 vmx_basic_vmcs_size(u64 vmx_basic)
 	return (vmx_basic & GENMASK_ULL(44, 32)) >> 32;
 }
 
+static inline u32 vmx_basic_vmcs_memtype(u64 vmx_basic)
+{
+	return (vmx_basic & VMX_BASIC_MEM_TYPE_MASK) >> VMX_BASIC_MEM_TYPE_SHIFT;
+}
+
 static inline int vmx_misc_preemption_timer_rate(u64 vmx_misc)
 {
 	return vmx_misc & VMX_MISC_PREEMPTION_TIMER_RATE_MASK;
diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h
index 45162c1bcd8f..d8849e42ca80 100644
--- a/arch/x86/kvm/vmx/capabilities.h
+++ b/arch/x86/kvm/vmx/capabilities.h
@@ -54,9 +54,7 @@ struct nested_vmx_msrs {
 };
 
 struct vmcs_config {
-	int size;
-	u32 basic_cap;
-	u32 revision_id;
+	u64 basic;
 	u32 pin_based_exec_ctrl;
 	u32 cpu_based_exec_ctrl;
 	u32 cpu_based_2nd_exec_ctrl;
@@ -76,7 +74,7 @@ extern struct vmx_capability vmx_capability __ro_after_init;
 
 static inline bool cpu_has_vmx_basic_inout(void)
 {
-	return	(((u64)vmcs_config.basic_cap << 32) & VMX_BASIC_INOUT);
+	return vmcs_config.basic & VMX_BASIC_INOUT;
 }
 
 static inline bool cpu_has_virtual_nmis(void)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index d7bf14abdba1..64a43c2d72e9 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2597,13 +2597,13 @@ static u64 adjust_vmx_controls64(u64 ctl_opt, u32 msr)
 static int setup_vmcs_config(struct vmcs_config *vmcs_conf,
 			     struct vmx_capability *vmx_cap)
 {
-	u32 vmx_msr_low, vmx_msr_high;
 	u32 _pin_based_exec_control = 0;
 	u32 _cpu_based_exec_control = 0;
 	u32 _cpu_based_2nd_exec_control = 0;
 	u64 _cpu_based_3rd_exec_control = 0;
 	u32 _vmexit_control = 0;
 	u32 _vmentry_control = 0;
+	u64 basic_msr;
 	u64 misc_msr;
 	int i;
 
@@ -2722,29 +2722,25 @@ static int setup_vmcs_config(struct vmcs_config *vmcs_conf,
 		_vmexit_control &= ~x_ctrl;
 	}
 
-	rdmsr(MSR_IA32_VMX_BASIC, vmx_msr_low, vmx_msr_high);
+	rdmsrl(MSR_IA32_VMX_BASIC, basic_msr);
 
 	/* IA-32 SDM Vol 3B: VMCS size is never greater than 4kB. */
-	if ((vmx_msr_high & 0x1fff) > PAGE_SIZE)
+	if (vmx_basic_vmcs_size(basic_msr) > PAGE_SIZE)
 		return -EIO;
 
 #ifdef CONFIG_X86_64
 	/* IA-32 SDM Vol 3B: 64-bit CPUs always have VMX_BASIC_MSR[48]==0. */
-	if (vmx_msr_high & (1u<<16))
+	if (basic_msr & VMX_BASIC_64)
 		return -EIO;
 #endif
 
 	/* Require Write-Back (WB) memory type for VMCS accesses. */
-	if (((vmx_msr_high >> 18) & 15) != 6)
+	if (vmx_basic_vmcs_memtype(basic_msr) != VMX_BASIC_MEM_TYPE_WB)
 		return -EIO;
 
 	rdmsrl(MSR_IA32_VMX_MISC, misc_msr);
 
-	vmcs_conf->size = vmx_msr_high & 0x1fff;
-	vmcs_conf->basic_cap = vmx_msr_high & ~0x1fff;
-
-	vmcs_conf->revision_id = vmx_msr_low;
-
+	vmcs_conf->basic = basic_msr;
 	vmcs_conf->pin_based_exec_ctrl = _pin_based_exec_control;
 	vmcs_conf->cpu_based_exec_ctrl = _cpu_based_exec_control;
 	vmcs_conf->cpu_based_2nd_exec_ctrl = _cpu_based_2nd_exec_control;
@@ -2883,13 +2879,13 @@ struct vmcs *alloc_vmcs_cpu(bool shadow, int cpu, gfp_t flags)
 	if (!pages)
 		return NULL;
 	vmcs = page_address(pages);
-	memset(vmcs, 0, vmcs_config.size);
+	memset(vmcs, 0, vmx_basic_vmcs_size(vmcs_config.basic));
 
 	/* KVM supports Enlightened VMCS v1 only */
 	if (kvm_is_using_evmcs())
 		vmcs->hdr.revision_id = KVM_EVMCS_VERSION;
 	else
-		vmcs->hdr.revision_id = vmcs_config.revision_id;
+		vmcs->hdr.revision_id = vmx_basic_vmcs_revision_id(vmcs_config.basic);
 
 	if (shadow)
 		vmcs->hdr.shadow_vmcs = 1;
@@ -2982,7 +2978,7 @@ static __init int alloc_kvm_area(void)
 		 * physical CPU.
 		 */
 		if (kvm_is_using_evmcs())
-			vmcs->hdr.revision_id = vmcs_config.revision_id;
+			vmcs->hdr.revision_id = vmx_basic_vmcs_revision_id(vmcs_config.basic);
 
 		per_cpu(vmxarea, cpu) = vmcs;
 	}

base-commit: f6cde92083dec5cf424504d7029acdffbe5beed8
--
Huang, Kai April 5, 2023, 3:10 a.m. UTC | #2 
On Tue, 2023-04-04 at 17:18 -0700, Sean Christopherson wrote:
> On Thu, Mar 30, 2023, Kai Huang wrote:
> > Currently, setup_vmcs_config() uses hard-coded values when performing
> > sanity check on the value of IA32_VMX_BASIC MSR and setting up the three
> > members of 'vmcs_config': size, revison_id and basic_cap.  However, the
> > kernel actually already has macro definitions for those relevant bits in
> > asm/msr-index.h and functions to get revision_id and size in asm/vmx.h.
> > 
> > Add the missing helper function to get the basic_cap, and use those
> > macros and helper functions in setup_vmcs_config() to get rid of those
> > hard-coded values.
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Kai Huang <kai.huang@intel.com>
> > ---
> >  arch/x86/include/asm/vmx.h |  5 +++++
> >  arch/x86/kvm/vmx/vmx.c     | 17 +++++++++--------
> >  2 files changed, 14 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> > index 498dc600bd5c..2b488895fe0e 100644
> > --- a/arch/x86/include/asm/vmx.h
> > +++ b/arch/x86/include/asm/vmx.h
> > @@ -141,6 +141,11 @@ static inline u32 vmx_basic_vmcs_size(u64 vmx_basic)
> >  	return (vmx_basic & GENMASK_ULL(44, 32)) >> 32;
> >  }
> >  
> > +static inline u32 vmx_basic_cap(u64 vmx_basic)
> 
> I would omit this helper and just open code the ugliness, purely because what
> it's tracking is completely random.
> 
> > +{
> > +	return (vmx_basic & ~GENMASK_ULL(44, 32)) >> 32;
> > +}
> > +
> 
> ...
> 
> >  #ifdef CONFIG_X86_64
> >  	/* IA-32 SDM Vol 3B: 64-bit CPUs always have VMX_BASIC_MSR[48]==0. */
> > -	if (vmx_msr_high & (1u<<16))
> > +	if (basic_msr & VMX_BASIC_64)
> >  		return -EIO;
> >  #endif
> >  
> >  	/* Require Write-Back (WB) memory type for VMCS accesses. */
> > -	if (((vmx_msr_high >> 18) & 15) != 6)
> > +	if (((basic_msr & VMX_BASIC_MEM_TYPE_MASK) >> VMX_BASIC_MEM_TYPE_SHIFT)
> > +			!= VMX_BASIC_MEM_TYPE_WB)
> 
> This one is worthy of a helper, e.g. 
> 
> 	if (vmx_basic_vmcs_memtype(basic_msr) != VMX_BASIC_MEM_TYPE_WB)
> 		return -EIO;
> 
> 
> >  		return -EIO;
> >  
> >  	rdmsrl(MSR_IA32_VMX_MISC, misc_msr);
> >  
> > -	vmcs_conf->size = vmx_msr_high & 0x1fff;
> > -	vmcs_conf->basic_cap = vmx_msr_high & ~0x1fff;
> > +	vmcs_conf->size = vmx_basic_vmcs_size(basic_msr);
> > +	vmcs_conf->basic_cap = vmx_basic_cap(basic_msr);
> 
> Actually, looking at this more closely, splitting up the basic MSR during setup
> is silly.  None of the usage is in hot paths, and cpu_has_vmx_basic_inout() is
> downright bizarre with the current code.
> 
> Rather than do all of these weird dances, what about saving the full/raw MSR in
> the config, and then using the helpers to extract info as needed?  E.g. the below
> over a few patches.  As a bonus (maybe), KVM will sanity check the entire MSR
> across CPUs.

Fine to me.  I'll rewrite this in one or more patches as you suggested and post
again.  Thanks!

> 
> ---
>  arch/x86/include/asm/vmx.h      |  5 +++++
>  arch/x86/kvm/vmx/capabilities.h |  6 ++----
>  arch/x86/kvm/vmx/vmx.c          | 22 +++++++++-------------
>  3 files changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index 498dc600bd5c..b21dcd128add 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -141,6 +141,11 @@ static inline u32 vmx_basic_vmcs_size(u64 vmx_basic)
>  	return (vmx_basic & GENMASK_ULL(44, 32)) >> 32;
>  }
>  
> +static inline u32 vmx_basic_vmcs_memtype(u64 vmx_basic)
> +{
> +	return (vmx_basic & VMX_BASIC_MEM_TYPE_MASK) >> VMX_BASIC_MEM_TYPE_SHIFT;
> +}
> +
>  static inline int vmx_misc_preemption_timer_rate(u64 vmx_misc)
>  {
>  	return vmx_misc & VMX_MISC_PREEMPTION_TIMER_RATE_MASK;
> diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h
> index 45162c1bcd8f..d8849e42ca80 100644
> --- a/arch/x86/kvm/vmx/capabilities.h
> +++ b/arch/x86/kvm/vmx/capabilities.h
> @@ -54,9 +54,7 @@ struct nested_vmx_msrs {
>  };
>  
>  struct vmcs_config {
> -	int size;
> -	u32 basic_cap;
> -	u32 revision_id;
> +	u64 basic;
>  	u32 pin_based_exec_ctrl;
>  	u32 cpu_based_exec_ctrl;
>  	u32 cpu_based_2nd_exec_ctrl;
> @@ -76,7 +74,7 @@ extern struct vmx_capability vmx_capability __ro_after_init;
>  
>  static inline bool cpu_has_vmx_basic_inout(void)
>  {
> -	return	(((u64)vmcs_config.basic_cap << 32) & VMX_BASIC_INOUT);
> +	return vmcs_config.basic & VMX_BASIC_INOUT;
>  }
>  
>  static inline bool cpu_has_virtual_nmis(void)
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index d7bf14abdba1..64a43c2d72e9 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2597,13 +2597,13 @@ static u64 adjust_vmx_controls64(u64 ctl_opt, u32 msr)
>  static int setup_vmcs_config(struct vmcs_config *vmcs_conf,
>  			     struct vmx_capability *vmx_cap)
>  {
> -	u32 vmx_msr_low, vmx_msr_high;
>  	u32 _pin_based_exec_control = 0;
>  	u32 _cpu_based_exec_control = 0;
>  	u32 _cpu_based_2nd_exec_control = 0;
>  	u64 _cpu_based_3rd_exec_control = 0;
>  	u32 _vmexit_control = 0;
>  	u32 _vmentry_control = 0;
> +	u64 basic_msr;
>  	u64 misc_msr;
>  	int i;
>  
> @@ -2722,29 +2722,25 @@ static int setup_vmcs_config(struct vmcs_config *vmcs_conf,
>  		_vmexit_control &= ~x_ctrl;
>  	}
>  
> -	rdmsr(MSR_IA32_VMX_BASIC, vmx_msr_low, vmx_msr_high);
> +	rdmsrl(MSR_IA32_VMX_BASIC, basic_msr);
>  
>  	/* IA-32 SDM Vol 3B: VMCS size is never greater than 4kB. */
> -	if ((vmx_msr_high & 0x1fff) > PAGE_SIZE)
> +	if (vmx_basic_vmcs_size(basic_msr) > PAGE_SIZE)
>  		return -EIO;
>  
>  #ifdef CONFIG_X86_64
>  	/* IA-32 SDM Vol 3B: 64-bit CPUs always have VMX_BASIC_MSR[48]==0. */
> -	if (vmx_msr_high & (1u<<16))
> +	if (basic_msr & VMX_BASIC_64)
>  		return -EIO;
>  #endif
>  
>  	/* Require Write-Back (WB) memory type for VMCS accesses. */
> -	if (((vmx_msr_high >> 18) & 15) != 6)
> +	if (vmx_basic_vmcs_memtype(basic_msr) != VMX_BASIC_MEM_TYPE_WB)
>  		return -EIO;
>  
>  	rdmsrl(MSR_IA32_VMX_MISC, misc_msr);
>  
> -	vmcs_conf->size = vmx_msr_high & 0x1fff;
> -	vmcs_conf->basic_cap = vmx_msr_high & ~0x1fff;
> -
> -	vmcs_conf->revision_id = vmx_msr_low;
> -
> +	vmcs_conf->basic = basic_msr;
>  	vmcs_conf->pin_based_exec_ctrl = _pin_based_exec_control;
>  	vmcs_conf->cpu_based_exec_ctrl = _cpu_based_exec_control;
>  	vmcs_conf->cpu_based_2nd_exec_ctrl = _cpu_based_2nd_exec_control;
> @@ -2883,13 +2879,13 @@ struct vmcs *alloc_vmcs_cpu(bool shadow, int cpu, gfp_t flags)
>  	if (!pages)
>  		return NULL;
>  	vmcs = page_address(pages);
> -	memset(vmcs, 0, vmcs_config.size);
> +	memset(vmcs, 0, vmx_basic_vmcs_size(vmcs_config.basic));
>  
>  	/* KVM supports Enlightened VMCS v1 only */
>  	if (kvm_is_using_evmcs())
>  		vmcs->hdr.revision_id = KVM_EVMCS_VERSION;
>  	else
> -		vmcs->hdr.revision_id = vmcs_config.revision_id;
> +		vmcs->hdr.revision_id = vmx_basic_vmcs_revision_id(vmcs_config.basic);
>  
>  	if (shadow)
>  		vmcs->hdr.shadow_vmcs = 1;
> @@ -2982,7 +2978,7 @@ static __init int alloc_kvm_area(void)
>  		 * physical CPU.
>  		 */
>  		if (kvm_is_using_evmcs())
> -			vmcs->hdr.revision_id = vmcs_config.revision_id;
> +			vmcs->hdr.revision_id = vmx_basic_vmcs_revision_id(vmcs_config.basic);
>  
>  		per_cpu(vmxarea, cpu) = vmcs;
>  	}
> 
> base-commit: f6cde92083dec5cf424504d7029acdffbe5beed8
diff mbox series

Patch

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 498dc600bd5c..2b488895fe0e 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -141,6 +141,11 @@  static inline u32 vmx_basic_vmcs_size(u64 vmx_basic)
 	return (vmx_basic & GENMASK_ULL(44, 32)) >> 32;
 }
 
+static inline u32 vmx_basic_cap(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;
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index c3ef74562158..8f4982ec3c8c 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2597,13 +2597,13 @@  static u64 adjust_vmx_controls64(u64 ctl_opt, u32 msr)
 static int setup_vmcs_config(struct vmcs_config *vmcs_conf,
 			     struct vmx_capability *vmx_cap)
 {
-	u32 vmx_msr_low, vmx_msr_high;
 	u32 _pin_based_exec_control = 0;
 	u32 _cpu_based_exec_control = 0;
 	u32 _cpu_based_2nd_exec_control = 0;
 	u64 _cpu_based_3rd_exec_control = 0;
 	u32 _vmexit_control = 0;
 	u32 _vmentry_control = 0;
+	u64 basic_msr;
 	u64 misc_msr;
 	int i;
 
@@ -2722,28 +2722,29 @@  static int setup_vmcs_config(struct vmcs_config *vmcs_conf,
 		_vmexit_control &= ~x_ctrl;
 	}
 
-	rdmsr(MSR_IA32_VMX_BASIC, vmx_msr_low, vmx_msr_high);
+	rdmsrl(MSR_IA32_VMX_BASIC, basic_msr);
 
 	/* IA-32 SDM Vol 3B: VMCS size is never greater than 4kB. */
-	if ((vmx_msr_high & 0x1fff) > PAGE_SIZE)
+	if (vmx_basic_vmcs_size(basic_msr) > PAGE_SIZE)
 		return -EIO;
 
 #ifdef CONFIG_X86_64
 	/* IA-32 SDM Vol 3B: 64-bit CPUs always have VMX_BASIC_MSR[48]==0. */
-	if (vmx_msr_high & (1u<<16))
+	if (basic_msr & VMX_BASIC_64)
 		return -EIO;
 #endif
 
 	/* Require Write-Back (WB) memory type for VMCS accesses. */
-	if (((vmx_msr_high >> 18) & 15) != 6)
+	if (((basic_msr & VMX_BASIC_MEM_TYPE_MASK) >> VMX_BASIC_MEM_TYPE_SHIFT)
+			!= VMX_BASIC_MEM_TYPE_WB)
 		return -EIO;
 
 	rdmsrl(MSR_IA32_VMX_MISC, misc_msr);
 
-	vmcs_conf->size = vmx_msr_high & 0x1fff;
-	vmcs_conf->basic_cap = vmx_msr_high & ~0x1fff;
+	vmcs_conf->size = vmx_basic_vmcs_size(basic_msr);
+	vmcs_conf->basic_cap = vmx_basic_cap(basic_msr);
 
-	vmcs_conf->revision_id = vmx_msr_low;
+	vmcs_conf->revision_id = vmx_basic_vmcs_revision_id(basic_msr);
 
 	vmcs_conf->pin_based_exec_ctrl = _pin_based_exec_control;
 	vmcs_conf->cpu_based_exec_ctrl = _cpu_based_exec_control;