[PATCH 0/3] kvm-unit-tests: Check CPUID level/xlevel before using CPUID data

[PATCH 0/3] kvm-unit-tests: Check CPUID level/xlevel before using CPUID data

[Eduardo Habkost]

Change the kvm-unit-tests x86 code to always check CPUID level/xlevel before
looking at CPUID data. Otherwise, the test code will be looking at bogus data.

Eduardo Habkost (3):
  x86: apic: Look up MAXPHYADDR on CPUID correctly
  x86: vmx: Use cpuid_maxphyaddr()
  x86: Check level, xlevel before returning CPUID data

 lib/x86/processor.h | 18 +++++++++++++++++-
 x86/apic.c          |  2 +-
 x86/vmx.c           |  6 +++---
 3 files changed, 21 insertions(+), 5 deletions(-)

-- 
1.9.3

[PATCH 1/3] x86: apic: Look up MAXPHYADDR on CPUID correctly

[Eduardo Habkost]

When the CPUID xlevel on QEMU is < 0x80000008, we get the following:

    $ ./x86-run x86/apic.flat -smp 2 -cpu qemu64,xlevel=0x80000007
    [...]
    FAIL: apicbase: reserved physaddr bits

That happens because CPUID[0x80000008].EAX won't have the expected data
if xlevel < 0x80000008.

When the CPUID physical address bits information is not available on CPUID,
assume it is 36, as documented on Intel SDM, Volume 3A, section
10.4.4 "Local APIC Status and Location":

> Bits 0 through 7, bits 9 and 10, and bits MAXPHYADDR[1] through 63 in the
> IA32_APIC_BASE MSR are reserved.
>
> [1] The MAXPHYADDR is 36 bits for processors that do not support CPUID
> leaf 80000008H, or indicated by CPUID.80000008H:EAX[bits 7:0] for
> processors that support CPUID leaf 80000008H."

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 lib/x86/processor.h | 8 ++++++++
 x86/apic.c          | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index f5f1c82..d4e295b 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -294,6 +294,14 @@ static inline struct cpuid cpuid(u32 function)
     return cpuid_indexed(function, 0);
 }
 
+static inline u8 cpuid_maxphyaddr(void)
+{
+    if (cpuid(0x80000000).a < 0x80000008)
+        return 36;
+    return cpuid(0x80000008).a & 0xff;
+}
+
+
 static inline void pause(void)
 {
     asm volatile ("pause");
diff --git a/x86/apic.c b/x86/apic.c
index 3f463a5..2619d85 100644
--- a/x86/apic.c
+++ b/x86/apic.c
@@ -124,7 +124,7 @@ static void test_apicbase(void)
     report("relocate apic",
            *(volatile u32 *)(ALTERNATE_APIC_BASE + APIC_LVR) == lvr);
 
-    value = orig_apicbase | (1UL << (cpuid(0x80000008).a & 0xff));
+    value = orig_apicbase | (1UL << cpuid_maxphyaddr());
     report("apicbase: reserved physaddr bits",
            test_for_exception(GP_VECTOR, do_write_apicbase, &value));
 
-- 
1.9.3

[PATCH 2/3] x86: vmx: Use cpuid_maxphyaddr()

[Eduardo Habkost]

The vmx test code calls cpuid(0x80000008) without checking xlevel first.
Change it to use cpuid_maxphyaddr() instead.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 x86/vmx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index ef9caca..132e5f0 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -161,7 +161,7 @@ void print_vmexit_info()
 static void test_vmclear(void)
 {
 	struct vmcs *tmp_root;
-	int width = cpuid(0x80000008).a & 0xff;
+	int width = cpuid_maxphyaddr();
 
 	/*
 	 * Note- The tests below do not necessarily have a
@@ -652,7 +652,7 @@ static int test_vmxon(void)
 {
 	int ret, ret1;
 	u64 *tmp_region = vmxon_region;
-	int width = cpuid(0x80000008).a & 0xff;
+	int width = cpuid_maxphyaddr();
 
 	/* Unaligned page access */
 	vmxon_region = (u64 *)((intptr_t)vmxon_region + 1);
@@ -694,7 +694,7 @@ out:
 static void test_vmptrld(void)
 {
 	struct vmcs *vmcs, *tmp_root;
-	int width = cpuid(0x80000008).a & 0xff;
+	int width = cpuid_maxphyaddr();
 
 	vmcs = alloc_page();
 	vmcs->revision_id = basic.revision;
-- 
1.9.3

[PATCH 3/3] x86: Check level, xlevel before returning CPUID data

[Eduardo Habkost]

None of the existing code using cpuid checks level or xlevel before
running it. Instead of changing all callers, make the cpuid() function
check if the requested leaf is available, before returning any data.

All existing callers of cpuid() and cpuid_indexed() are checks for the
presence of feature bits, so it is safe to return all zeroes when the
requested CPUID leaf is not available.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 lib/x86/processor.h | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index d4e295b..7973879 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -280,7 +280,7 @@ static inline ulong read_dr7(void)
 
 struct cpuid { u32 a, b, c, d; };
 
-static inline struct cpuid cpuid_indexed(u32 function, u32 index)
+static inline struct cpuid raw_cpuid(u32 function, u32 index)
 {
     struct cpuid r;
     asm volatile ("cpuid"
@@ -289,6 +289,14 @@ static inline struct cpuid cpuid_indexed(u32 function, u32 index)
     return r;
 }
 
+static inline struct cpuid cpuid_indexed(u32 function, u32 index)
+{
+    u32 level = raw_cpuid(function & 0xf0000000, 0).a;
+    if (level < function)
+        return (struct cpuid) { 0, 0, 0, 0 };
+    return raw_cpuid(function, index);
+}
+
 static inline struct cpuid cpuid(u32 function)
 {
     return cpuid_indexed(function, 0);
@@ -296,9 +304,9 @@ static inline struct cpuid cpuid(u32 function)
 
 static inline u8 cpuid_maxphyaddr(void)
 {
-    if (cpuid(0x80000000).a < 0x80000008)
+    if (raw_cpuid(0x80000000, 0).a < 0x80000008)
         return 36;
-    return cpuid(0x80000008).a & 0xff;
+    return raw_cpuid(0x80000008, 0).a & 0xff;
 }
 
 
-- 
1.9.3

Re: [PATCH 0/3] kvm-unit-tests: Check CPUID level/xlevel before using CPUID data

[Paolo Bonzini]

Il 28/08/2014 20:02, Eduardo Habkost ha scritto:
> Change the kvm-unit-tests x86 code to always check CPUID level/xlevel before
> looking at CPUID data. Otherwise, the test code will be looking at bogus data.
> 
> Eduardo Habkost (3):
>   x86: apic: Look up MAXPHYADDR on CPUID correctly
>   x86: vmx: Use cpuid_maxphyaddr()
>   x86: Check level, xlevel before returning CPUID data
> 
>  lib/x86/processor.h | 18 +++++++++++++++++-
>  x86/apic.c          |  2 +-
>  x86/vmx.c           |  6 +++---
>  3 files changed, 21 insertions(+), 5 deletions(-)
> 

Applying this series, thanks.

Paolo