[PATCH 04/11] KVM: selftests: Rework OSXSAVE CR4=>CPUID test to play nice with AVX insns

Sean Christopherson posted 11 patches 1 month, 3 weeks ago
[PATCH 04/11] KVM: selftests: Rework OSXSAVE CR4=>CPUID test to play nice with AVX insns
Posted by Sean Christopherson 1 month, 3 weeks ago
Rework the CR4/CPUID sync test to clear CR4.OSXSAVE, do CPUID, and restore
CR4.OSXSAVE in assembly, so that there is zero chance of AVX instructions
being executed while CR4.OSXSAVE is disabled.  This will allow enabling
CR4.OSXSAVE by default for selftests vCPUs as a general means of playing
nice with AVX instructions.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 .../kvm/x86_64/cr4_cpuid_sync_test.c          | 46 +++++++++++++------
 1 file changed, 32 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
index 624dc725e14d..da818afb7031 100644
--- a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
+++ b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
@@ -19,15 +19,14 @@
 #include "kvm_util.h"
 #include "processor.h"
 
-static inline bool cr4_cpuid_is_sync(void)
-{
-	uint64_t cr4 = get_cr4();
-
-	return (this_cpu_has(X86_FEATURE_OSXSAVE) == !!(cr4 & X86_CR4_OSXSAVE));
-}
+#define MAGIC_HYPERCALL_PORT	0x80
 
 static void guest_code(void)
 {
+	u32 regs[4] = {
+		[KVM_CPUID_EAX] = X86_FEATURE_OSXSAVE.function,
+		[KVM_CPUID_ECX] = X86_FEATURE_OSXSAVE.index,
+	};
 	uint64_t cr4;
 
 	/* turn on CR4.OSXSAVE */
@@ -36,13 +35,29 @@ static void guest_code(void)
 	set_cr4(cr4);
 
 	/* verify CR4.OSXSAVE == CPUID.OSXSAVE */
-	GUEST_ASSERT(cr4_cpuid_is_sync());
+	GUEST_ASSERT(this_cpu_has(X86_FEATURE_OSXSAVE));
 
-	/* notify hypervisor to change CR4 */
-	GUEST_SYNC(0);
+	/*
+	 * Notify hypervisor to clear CR4.0SXSAVE, do CPUID and save output,
+	 * and then restore CR4.  Do this all in  assembly to ensure no AVX
+	 * instructions are executed while OSXSAVE=0.
+	 */
+	asm volatile (
+		"out %%al, $" __stringify(MAGIC_HYPERCALL_PORT) "\n\t"
+		"cpuid\n\t"
+		"mov %%rdi, %%cr4\n\t"
+		: "+a" (regs[KVM_CPUID_EAX]),
+		  "=b" (regs[KVM_CPUID_EBX]),
+		  "+c" (regs[KVM_CPUID_ECX]),
+		  "=d" (regs[KVM_CPUID_EDX])
+		: "D" (get_cr4())
+	);
 
-	/* check again */
-	GUEST_ASSERT(cr4_cpuid_is_sync());
+	/* Verify KVM cleared OSXSAVE in CPUID when it was cleared in CR4. */
+	GUEST_ASSERT(!(regs[X86_FEATURE_OSXSAVE.reg] & BIT(X86_FEATURE_OSXSAVE.bit)));
+
+	/* Verify restoring CR4 also restored OSXSAVE in CPUID. */
+	GUEST_ASSERT(this_cpu_has(X86_FEATURE_OSXSAVE));
 
 	GUEST_DONE();
 }
@@ -62,13 +77,16 @@ int main(int argc, char *argv[])
 		vcpu_run(vcpu);
 		TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
 
-		switch (get_ucall(vcpu, &uc)) {
-		case UCALL_SYNC:
+		if (vcpu->run->io.port == MAGIC_HYPERCALL_PORT &&
+		    vcpu->run->io.direction == KVM_EXIT_IO_OUT) {
 			/* emulate hypervisor clearing CR4.OSXSAVE */
 			vcpu_sregs_get(vcpu, &sregs);
 			sregs.cr4 &= ~X86_CR4_OSXSAVE;
 			vcpu_sregs_set(vcpu, &sregs);
-			break;
+			continue;
+		}
+
+		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
 			REPORT_GUEST_ASSERT(uc);
 			break;
-- 
2.47.0.rc0.187.ge670bccf7e-goog
Re: [PATCH 04/11] KVM: selftests: Rework OSXSAVE CR4=>CPUID test to play nice with AVX insns
Posted by Vitaly Kuznetsov 1 month, 3 weeks ago
Sean Christopherson <seanjc@google.com> writes:

> Rework the CR4/CPUID sync test to clear CR4.OSXSAVE, do CPUID, and restore
> CR4.OSXSAVE in assembly, so that there is zero chance of AVX instructions
> being executed while CR4.OSXSAVE is disabled.  This will allow enabling
> CR4.OSXSAVE by default for selftests vCPUs as a general means of playing
> nice with AVX instructions.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  .../kvm/x86_64/cr4_cpuid_sync_test.c          | 46 +++++++++++++------
>  1 file changed, 32 insertions(+), 14 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
> index 624dc725e14d..da818afb7031 100644
> --- a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
> @@ -19,15 +19,14 @@
>  #include "kvm_util.h"
>  #include "processor.h"
>  
> -static inline bool cr4_cpuid_is_sync(void)
> -{
> -	uint64_t cr4 = get_cr4();
> -
> -	return (this_cpu_has(X86_FEATURE_OSXSAVE) == !!(cr4 & X86_CR4_OSXSAVE));
> -}
> +#define MAGIC_HYPERCALL_PORT	0x80
>  
>  static void guest_code(void)
>  {
> +	u32 regs[4] = {
> +		[KVM_CPUID_EAX] = X86_FEATURE_OSXSAVE.function,
> +		[KVM_CPUID_ECX] = X86_FEATURE_OSXSAVE.index,
> +	};
>  	uint64_t cr4;
>  
>  	/* turn on CR4.OSXSAVE */
> @@ -36,13 +35,29 @@ static void guest_code(void)
>  	set_cr4(cr4);
>  
>  	/* verify CR4.OSXSAVE == CPUID.OSXSAVE */
> -	GUEST_ASSERT(cr4_cpuid_is_sync());
> +	GUEST_ASSERT(this_cpu_has(X86_FEATURE_OSXSAVE));
>  
> -	/* notify hypervisor to change CR4 */
> -	GUEST_SYNC(0);
> +	/*
> +	 * Notify hypervisor to clear CR4.0SXSAVE, do CPUID and save output,
> +	 * and then restore CR4.  Do this all in  assembly to ensure no AVX
> +	 * instructions are executed while OSXSAVE=0.
> +	 */
> +	asm volatile (
> +		"out %%al, $" __stringify(MAGIC_HYPERCALL_PORT) "\n\t"
> +		"cpuid\n\t"
> +		"mov %%rdi, %%cr4\n\t"
> +		: "+a" (regs[KVM_CPUID_EAX]),
> +		  "=b" (regs[KVM_CPUID_EBX]),
> +		  "+c" (regs[KVM_CPUID_ECX]),
> +		  "=d" (regs[KVM_CPUID_EDX])
> +		: "D" (get_cr4())
> +	);
>  
> -	/* check again */
> -	GUEST_ASSERT(cr4_cpuid_is_sync());
> +	/* Verify KVM cleared OSXSAVE in CPUID when it was cleared in CR4. */
> +	GUEST_ASSERT(!(regs[X86_FEATURE_OSXSAVE.reg] & BIT(X86_FEATURE_OSXSAVE.bit)));
> +
> +	/* Verify restoring CR4 also restored OSXSAVE in CPUID. */
> +	GUEST_ASSERT(this_cpu_has(X86_FEATURE_OSXSAVE));
>  
>  	GUEST_DONE();
>  }
> @@ -62,13 +77,16 @@ int main(int argc, char *argv[])
>  		vcpu_run(vcpu);
>  		TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
>  
> -		switch (get_ucall(vcpu, &uc)) {
> -		case UCALL_SYNC:
> +		if (vcpu->run->io.port == MAGIC_HYPERCALL_PORT &&
> +		    vcpu->run->io.direction == KVM_EXIT_IO_OUT) {
>  			/* emulate hypervisor clearing CR4.OSXSAVE */
>  			vcpu_sregs_get(vcpu, &sregs);
>  			sregs.cr4 &= ~X86_CR4_OSXSAVE;
>  			vcpu_sregs_set(vcpu, &sregs);
> -			break;
> +			continue;
> +		}
> +
> +		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
>  			REPORT_GUEST_ASSERT(uc);
>  			break;

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>

-- 
Vitaly