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

[Yan Zhao]

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@xxxxxxxxx>
> Cc: Yan Zhao <yan.y.zhao@xxxxxxxxx>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
>  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
>