1. Patchew
  2. linux
  3. [PATCH 0/2] nVMX: Ensure host's PAT stays sane
  4. View patch

[PATCH 1/2] nVMX: Ensure host's PAT is loaded at the end of all VMX tests

Sean Christopherson posted 2 patches 1 year, 8 months ago
[PATCH 1/2] nVMX: Ensure host's PAT is loaded at the end of all VMX tests
Posted by Sean Christopherson 1 year, 8 months ago 
Load the host's original PAT on VM-Exit by default in all VMX tests, and
manually write PAT with the original value in the test that verifies all
legal PAT values can be loaded via HOST_PAT.  Failure to (re)load the
correct host PAT results in all tests that run after test_load_host_pat()
using UC memtype for all memory.

Opportunistically fix a message goof for the ENT_LOAD_PAT=0 case.

Reported-by: Xiangfei Ma <xiangfeix.ma@intel.com>
Cc: Yan Zhao <yan.y.zhao@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/vmx.c       | 4 +++-
 x86/vmx_tests.c | 5 ++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 9f08c096..c803eaa6 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1126,6 +1126,8 @@ static void init_vmcs_host(void)
 	vmcs_write(HOST_CR4, read_cr4());
 	vmcs_write(HOST_SYSENTER_EIP, (u64)(&entry_sysenter));
 	vmcs_write(HOST_SYSENTER_CS,  KERNEL_CS);
+	if (ctrl_exit_rev.clr & EXI_LOAD_PAT)
+		vmcs_write(HOST_PAT, rdmsr(MSR_IA32_CR_PAT));
 
 	/* 26.2.3 */
 	vmcs_write(HOST_SEL_CS, KERNEL_CS);
@@ -1247,7 +1249,7 @@ int init_vmcs(struct vmcs **vmcs)
 	/* All settings to pin/exit/enter/cpu
 	   control fields should be placed here */
 	ctrl_pin |= PIN_EXTINT | PIN_NMI | PIN_VIRT_NMI;
-	ctrl_exit = EXI_LOAD_EFER | EXI_HOST_64;
+	ctrl_exit = EXI_LOAD_EFER | EXI_HOST_64 | EXI_LOAD_PAT;
 	ctrl_enter = (ENT_LOAD_EFER | ENT_GUEST_64);
 	/* DIsable IO instruction VMEXIT now */
 	ctrl_cpu[0] &= (~(CPU_IO | CPU_IO_BITMAP));
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 22e8812a..8a17dd90 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -7233,6 +7233,7 @@ static void test_guest_efer(void)
 static void test_pat(u32 field, const char * field_name, u32 ctrl_field,
 		     u64 ctrl_bit)
 {
+	u64 pat_msr_saved = rdmsr(MSR_IA32_CR_PAT);
 	u32 ctrl_saved = vmcs_read(ctrl_field);
 	u64 pat_saved = vmcs_read(field);
 	u64 i, val;
@@ -7252,7 +7253,7 @@ static void test_pat(u32 field, const char * field_name, u32 ctrl_field,
 				report_prefix_pop();
 
 			} else {	// GUEST_PAT
-				test_guest_state("ENT_LOAD_PAT enabled", false,
+				test_guest_state("ENT_LOAD_PAT disabled", false,
 						 val, "GUEST_PAT");
 			}
 		}
@@ -7274,6 +7275,8 @@ static void test_pat(u32 field, const char * field_name, u32 ctrl_field,
 					error = 0;
 
 				test_vmx_vmlaunch(error);
+				wrmsr(MSR_IA32_CR_PAT, pat_msr_saved);
+
 				report_prefix_pop();
 
 			} else {	// GUEST_PAT
-- 
2.45.1.467.gbab1589fc0-goog
Re: [PATCH 1/2] nVMX: Ensure host's PAT is loaded at the end of all VMX tests
Posted by Yan Zhao 1 year, 8 months ago 
On Wed, Jun 05, 2024 at 03:45:26PM -0700, Sean Christopherson wrote:
> Load the host's original PAT on VM-Exit by default in all VMX tests, and
> manually write PAT with the original value in the test that verifies all
> legal PAT values can be loaded via HOST_PAT.  Failure to (re)load the
> correct host PAT results in all tests that run after test_load_host_pat()
> using UC memtype for all memory.
> 
> Opportunistically fix a message goof for the ENT_LOAD_PAT=0 case.
> 
> Reported-by: Xiangfei Ma <xiangfeix.ma@intel.com>
> Cc: Yan Zhao <yan.y.zhao@intel.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  x86/vmx.c       | 4 +++-
>  x86/vmx_tests.c | 5 ++++-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/x86/vmx.c b/x86/vmx.c
> index 9f08c096..c803eaa6 100644
> --- a/x86/vmx.c
> +++ b/x86/vmx.c
> @@ -1126,6 +1126,8 @@ static void init_vmcs_host(void)
>  	vmcs_write(HOST_CR4, read_cr4());
>  	vmcs_write(HOST_SYSENTER_EIP, (u64)(&entry_sysenter));
>  	vmcs_write(HOST_SYSENTER_CS,  KERNEL_CS);
> +	if (ctrl_exit_rev.clr & EXI_LOAD_PAT)
> +		vmcs_write(HOST_PAT, rdmsr(MSR_IA32_CR_PAT));
>  
>  	/* 26.2.3 */
>  	vmcs_write(HOST_SEL_CS, KERNEL_CS);
> @@ -1247,7 +1249,7 @@ int init_vmcs(struct vmcs **vmcs)
>  	/* All settings to pin/exit/enter/cpu
>  	   control fields should be placed here */
>  	ctrl_pin |= PIN_EXTINT | PIN_NMI | PIN_VIRT_NMI;
> -	ctrl_exit = EXI_LOAD_EFER | EXI_HOST_64;
> +	ctrl_exit = EXI_LOAD_EFER | EXI_HOST_64 | EXI_LOAD_PAT;
>  	ctrl_enter = (ENT_LOAD_EFER | ENT_GUEST_64);
>  	/* DIsable IO instruction VMEXIT now */
>  	ctrl_cpu[0] &= (~(CPU_IO | CPU_IO_BITMAP));
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 22e8812a..8a17dd90 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -7233,6 +7233,7 @@ static void test_guest_efer(void)
>  static void test_pat(u32 field, const char * field_name, u32 ctrl_field,
>  		     u64 ctrl_bit)
>  {
> +	u64 pat_msr_saved = rdmsr(MSR_IA32_CR_PAT);
>  	u32 ctrl_saved = vmcs_read(ctrl_field);
>  	u64 pat_saved = vmcs_read(field);
>  	u64 i, val;
> @@ -7252,7 +7253,7 @@ static void test_pat(u32 field, const char * field_name, u32 ctrl_field,
>  				report_prefix_pop();
>  
>  			} else {	// GUEST_PAT
> -				test_guest_state("ENT_LOAD_PAT enabled", false,
> +				test_guest_state("ENT_LOAD_PAT disabled", false,
>  						 val, "GUEST_PAT");
>  			}
>  		}
> @@ -7274,6 +7275,8 @@ static void test_pat(u32 field, const char * field_name, u32 ctrl_field,
>  					error = 0;
>  
>  				test_vmx_vmlaunch(error);
> +				wrmsr(MSR_IA32_CR_PAT, pat_msr_saved);
> +
>  				report_prefix_pop();
>  
>  			} else {	// GUEST_PAT

Is it possible that ENT_LOAD_PAT of GUEST_PAT is tested when there's no support of
EXI_LOAD_PAT of HOST_PAT?
Then
	wrmsr(MSR_IA32_CR_PAT, pat_msr_saved);
is also required in this case?

> -- 
> 2.45.1.467.gbab1589fc0-goog
>
Re: [PATCH 1/2] nVMX: Ensure host's PAT is loaded at the end of all VMX tests
Posted by Sean Christopherson 1 year, 8 months ago 
On Tue, Jun 11, 2024, Yan Zhao wrote:
> On Wed, Jun 05, 2024 at 03:45:26PM -0700, Sean Christopherson wrote:
> > @@ -7274,6 +7275,8 @@ static void test_pat(u32 field, const char * field_name, u32 ctrl_field,
> >  					error = 0;
> >  
> >  				test_vmx_vmlaunch(error);
> > +				wrmsr(MSR_IA32_CR_PAT, pat_msr_saved);
> > +
> >  				report_prefix_pop();
> >  
> >  			} else {	// GUEST_PAT
> 
> Is it possible that ENT_LOAD_PAT of GUEST_PAT is tested when there's no support of
> EXI_LOAD_PAT of HOST_PAT?
> Then
> 	wrmsr(MSR_IA32_CR_PAT, pat_msr_saved);
> is also required in this case?

Heh, in theory, yeah, a nested setup could create such a monstrosity.  It's easy
enough to handle, so I guess why not?  Though I'm tempted to assert instead,
because practically speaking this code will never be hit, and thus never validated.

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 2063ee90..ffe7064c 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -7288,6 +7288,9 @@ static void test_pat(u32 field, const char * field_name, u32 ctrl_field,
                                error = (i == 0x2 || i == 0x3 || i >= 0x8);
                                test_guest_state("ENT_LOAD_PAT enabled", !!error,
                                                 val, "GUEST_PAT");
+
+                               if (!(ctrl_exit_rev.clr & EXI_LOAD_PAT))
+                                       wrmsr(MSR_IA32_CR_PAT, pat_msr_saved);
                        }
 
                }
Re: [PATCH 1/2] nVMX: Ensure host's PAT is loaded at the end of all VMX tests
Posted by Yan Zhao 1 year, 8 months ago 
On Mon, Jun 10, 2024 at 06:47:55PM -0700, Sean Christopherson wrote:
> On Tue, Jun 11, 2024, Yan Zhao wrote:
> > On Wed, Jun 05, 2024 at 03:45:26PM -0700, Sean Christopherson wrote:
> > > @@ -7274,6 +7275,8 @@ static void test_pat(u32 field, const char * field_name, u32 ctrl_field,
> > >  					error = 0;
> > >  
> > >  				test_vmx_vmlaunch(error);
> > > +				wrmsr(MSR_IA32_CR_PAT, pat_msr_saved);
> > > +
> > >  				report_prefix_pop();
> > >  
> > >  			} else {	// GUEST_PAT
> > 
> > Is it possible that ENT_LOAD_PAT of GUEST_PAT is tested when there's no support of
> > EXI_LOAD_PAT of HOST_PAT?
> > Then
> > 	wrmsr(MSR_IA32_CR_PAT, pat_msr_saved);
> > is also required in this case?
> 
> Heh, in theory, yeah, a nested setup could create such a monstrosity.  It's easy
> enough to handle, so I guess why not?  Though I'm tempted to assert instead,
> because practically speaking this code will never be hit, and thus never validated.
> 
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 2063ee90..ffe7064c 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -7288,6 +7288,9 @@ static void test_pat(u32 field, const char * field_name, u32 ctrl_field,
>                                 error = (i == 0x2 || i == 0x3 || i >= 0x8);
>                                 test_guest_state("ENT_LOAD_PAT enabled", !!error,
>                                                  val, "GUEST_PAT");
> +
> +                               if (!(ctrl_exit_rev.clr & EXI_LOAD_PAT))
> +                                       wrmsr(MSR_IA32_CR_PAT, pat_msr_saved);
>                         }
>  
>                 }
Tested passed with EXI_LOAD_PAT intentionally cleared in ctrl_exit_rev.clr.

Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.