[PATCH 02/11] KVM: selftests: Precisely mask off dynamic fields in CPUID test

Sean Christopherson posted 11 patches 1 month, 3 weeks ago
[PATCH 02/11] KVM: selftests: Precisely mask off dynamic fields in CPUID test
Posted by Sean Christopherson 1 month, 3 weeks ago
When comparing vCPU CPUID entries against KVM's supported CPUID, mask off
only the dynamic fields/bits instead of skipping the entire entry.
Precisely masking bits isn't meaningfully more difficult than skipping
entire entries, and will be necessary to maintain test coverage when a
future commit enables OSXSAVE by default, i.e. makes one bit in all of
CPUID.0x1 dynamic.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 .../testing/selftests/kvm/x86_64/cpuid_test.c | 61 +++++++++++--------
 1 file changed, 36 insertions(+), 25 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86_64/cpuid_test.c b/tools/testing/selftests/kvm/x86_64/cpuid_test.c
index fec03b11b059..f7fdcef5fa59 100644
--- a/tools/testing/selftests/kvm/x86_64/cpuid_test.c
+++ b/tools/testing/selftests/kvm/x86_64/cpuid_test.c
@@ -12,17 +12,16 @@
 #include "kvm_util.h"
 #include "processor.h"
 
-/* CPUIDs known to differ */
-struct {
-	u32 function;
-	u32 index;
-} mangled_cpuids[] = {
-	/*
-	 * These entries depend on the vCPU's XCR0 register and IA32_XSS MSR,
-	 * which are not controlled for by this test.
-	 */
-	{.function = 0xd, .index = 0},
-	{.function = 0xd, .index = 1},
+struct cpuid_mask {
+	union {
+		struct {
+			u32 eax;
+			u32 ebx;
+			u32 ecx;
+			u32 edx;
+		};
+		u32 regs[4];
+	};
 };
 
 static void test_guest_cpuids(struct kvm_cpuid2 *guest_cpuid)
@@ -56,17 +55,23 @@ static void guest_main(struct kvm_cpuid2 *guest_cpuid)
 	GUEST_DONE();
 }
 
-static bool is_cpuid_mangled(const struct kvm_cpuid_entry2 *entrie)
+static struct cpuid_mask get_const_cpuid_mask(const struct kvm_cpuid_entry2 *entry)
 {
-	int i;
+	struct cpuid_mask mask;
 
-	for (i = 0; i < ARRAY_SIZE(mangled_cpuids); i++) {
-		if (mangled_cpuids[i].function == entrie->function &&
-		    mangled_cpuids[i].index == entrie->index)
-			return true;
+	memset(&mask, 0xff, sizeof(mask));
+
+	switch (entry->function) {
+	case 0xd:
+		/*
+		 * CPUID.0xD.{0,1}.EBX enumerate XSAVE size based on the current
+		 * XCR0 and IA32_XSS MSR values.
+		 */
+		if (entry->index < 2)
+			mask.ebx = 0;
+		break;
 	}
-
-	return false;
+	return mask;
 }
 
 static void compare_cpuids(const struct kvm_cpuid2 *cpuid1,
@@ -79,6 +84,8 @@ static void compare_cpuids(const struct kvm_cpuid2 *cpuid1,
 		    "CPUID nent mismatch: %d vs. %d", cpuid1->nent, cpuid2->nent);
 
 	for (i = 0; i < cpuid1->nent; i++) {
+		struct cpuid_mask mask;
+
 		e1 = &cpuid1->entries[i];
 		e2 = &cpuid2->entries[i];
 
@@ -88,15 +95,19 @@ static void compare_cpuids(const struct kvm_cpuid2 *cpuid1,
 			    i, e1->function, e1->index, e1->flags,
 			    e2->function, e2->index, e2->flags);
 
-		if (is_cpuid_mangled(e1))
-			continue;
+		/* Mask off dynamic bits, e.g. OSXSAVE, when comparing entries. */
+		mask = get_const_cpuid_mask(e1);
 
-		TEST_ASSERT(e1->eax == e2->eax && e1->ebx == e2->ebx &&
-			    e1->ecx == e2->ecx && e1->edx == e2->edx,
+		TEST_ASSERT((e1->eax & mask.eax) == (e2->eax & mask.eax) &&
+			    (e1->ebx & mask.ebx) == (e2->ebx & mask.ebx) &&
+			    (e1->ecx & mask.ecx) == (e2->ecx & mask.ecx) &&
+			    (e1->edx & mask.edx) == (e2->edx & mask.edx),
 			    "CPUID 0x%x.%x differ: 0x%x:0x%x:0x%x:0x%x vs 0x%x:0x%x:0x%x:0x%x",
 			    e1->function, e1->index,
-			    e1->eax, e1->ebx, e1->ecx, e1->edx,
-			    e2->eax, e2->ebx, e2->ecx, e2->edx);
+			    e1->eax & mask.eax, e1->ebx & mask.ebx,
+			    e1->ecx & mask.ecx, e1->edx & mask.edx,
+			    e2->eax & mask.eax, e2->ebx & mask.ebx,
+			    e2->ecx & mask.ecx, e2->edx & mask.edx);
 	}
 }
 
-- 
2.47.0.rc0.187.ge670bccf7e-goog
Re: [PATCH 02/11] KVM: selftests: Precisely mask off dynamic fields in CPUID test
Posted by Vitaly Kuznetsov 1 month, 3 weeks ago
Sean Christopherson <seanjc@google.com> writes:

> When comparing vCPU CPUID entries against KVM's supported CPUID, mask off
> only the dynamic fields/bits instead of skipping the entire entry.
> Precisely masking bits isn't meaningfully more difficult than skipping
> entire entries, and will be necessary to maintain test coverage when a
> future commit enables OSXSAVE by default, i.e. makes one bit in all of
> CPUID.0x1 dynamic.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  .../testing/selftests/kvm/x86_64/cpuid_test.c | 61 +++++++++++--------
>  1 file changed, 36 insertions(+), 25 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/x86_64/cpuid_test.c b/tools/testing/selftests/kvm/x86_64/cpuid_test.c
> index fec03b11b059..f7fdcef5fa59 100644
> --- a/tools/testing/selftests/kvm/x86_64/cpuid_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/cpuid_test.c
> @@ -12,17 +12,16 @@
>  #include "kvm_util.h"
>  #include "processor.h"
>  
> -/* CPUIDs known to differ */
> -struct {
> -	u32 function;
> -	u32 index;
> -} mangled_cpuids[] = {
> -	/*
> -	 * These entries depend on the vCPU's XCR0 register and IA32_XSS MSR,
> -	 * which are not controlled for by this test.
> -	 */
> -	{.function = 0xd, .index = 0},
> -	{.function = 0xd, .index = 1},
> +struct cpuid_mask {
> +	union {
> +		struct {
> +			u32 eax;
> +			u32 ebx;
> +			u32 ecx;
> +			u32 edx;
> +		};
> +		u32 regs[4];
> +	};
>  };
>  
>  static void test_guest_cpuids(struct kvm_cpuid2 *guest_cpuid)
> @@ -56,17 +55,23 @@ static void guest_main(struct kvm_cpuid2 *guest_cpuid)
>  	GUEST_DONE();
>  }
>  
> -static bool is_cpuid_mangled(const struct kvm_cpuid_entry2 *entrie)
> +static struct cpuid_mask get_const_cpuid_mask(const struct kvm_cpuid_entry2 *entry)
>  {
> -	int i;
> +	struct cpuid_mask mask;
>  
> -	for (i = 0; i < ARRAY_SIZE(mangled_cpuids); i++) {
> -		if (mangled_cpuids[i].function == entrie->function &&
> -		    mangled_cpuids[i].index == entrie->index)
> -			return true;
> +	memset(&mask, 0xff, sizeof(mask));
> +
> +	switch (entry->function) {
> +	case 0xd:
> +		/*
> +		 * CPUID.0xD.{0,1}.EBX enumerate XSAVE size based on the current
> +		 * XCR0 and IA32_XSS MSR values.
> +		 */
> +		if (entry->index < 2)
> +			mask.ebx = 0;
> +		break;
>  	}
> -
> -	return false;
> +	return mask;
>  }
>  
>  static void compare_cpuids(const struct kvm_cpuid2 *cpuid1,
> @@ -79,6 +84,8 @@ static void compare_cpuids(const struct kvm_cpuid2 *cpuid1,
>  		    "CPUID nent mismatch: %d vs. %d", cpuid1->nent, cpuid2->nent);
>  
>  	for (i = 0; i < cpuid1->nent; i++) {
> +		struct cpuid_mask mask;
> +
>  		e1 = &cpuid1->entries[i];
>  		e2 = &cpuid2->entries[i];
>  
> @@ -88,15 +95,19 @@ static void compare_cpuids(const struct kvm_cpuid2 *cpuid1,
>  			    i, e1->function, e1->index, e1->flags,
>  			    e2->function, e2->index, e2->flags);
>  
> -		if (is_cpuid_mangled(e1))
> -			continue;
> +		/* Mask off dynamic bits, e.g. OSXSAVE, when comparing entries. */
> +		mask = get_const_cpuid_mask(e1);
>  
> -		TEST_ASSERT(e1->eax == e2->eax && e1->ebx == e2->ebx &&
> -			    e1->ecx == e2->ecx && e1->edx == e2->edx,
> +		TEST_ASSERT((e1->eax & mask.eax) == (e2->eax & mask.eax) &&
> +			    (e1->ebx & mask.ebx) == (e2->ebx & mask.ebx) &&
> +			    (e1->ecx & mask.ecx) == (e2->ecx & mask.ecx) &&
> +			    (e1->edx & mask.edx) == (e2->edx & mask.edx),
>  			    "CPUID 0x%x.%x differ: 0x%x:0x%x:0x%x:0x%x vs 0x%x:0x%x:0x%x:0x%x",
>  			    e1->function, e1->index,
> -			    e1->eax, e1->ebx, e1->ecx, e1->edx,
> -			    e2->eax, e2->ebx, e2->ecx, e2->edx);
> +			    e1->eax & mask.eax, e1->ebx & mask.ebx,
> +			    e1->ecx & mask.ecx, e1->edx & mask.edx,
> +			    e2->eax & mask.eax, e2->ebx & mask.ebx,
> +			    e2->ecx & mask.ecx, e2->edx & mask.edx);
>  	}
>  }

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

-- 
Vitaly