diff mbox

x86: correct remaining extended CPUID level checks

Message ID 5730AA9C02000078000E98CF@prv-mh.provo.novell.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Beulich May 9, 2016, 1:19 p.m. UTC 
We should consistently check the upper 16 bits to be equal 0x8000 and
only then the full value to be >= the desired level.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
As to inclusion in 4.7 - I think this would be desirable, but it's not
a must: I'm unaware of real world environments where
CPUID[0x80000000].EAX would yield a value not having the upper 16 bits
set to 0x8000.
x86: correct remaining extended CPUID level checks

We should consistently check the upper 16 bits to be equal 0x8000 and
only then the full value to be >= the desired level.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
As to inclusion in 4.7 - I think this would be desirable, but it's not
a must: I'm unaware of real world environments where
CPUID[0x80000000].EAX would yield a value not having the upper 16 bits
set to 0x8000.

--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -133,7 +133,10 @@ __start:
         /* Interrogate CPU extended features via CPUID. */
         mov     $0x80000000,%eax
         cpuid
+        shld    $16,%eax,%ecx
         xor     %edx,%edx
+        cmp     $0x8000,%cx         # any function @ 0x8000xxxx?
+        jne     1f
         cmp     $0x80000000,%eax    # any function > 0x80000000?
         jbe     1f
         mov     $0x80000001,%eax
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -297,9 +297,12 @@ static void generic_identify(struct cpui
 
 	/* AMD-defined flags: level 0x80000001 */
 	c->extended_cpuid_level = cpuid_eax(0x80000000);
-	cpuid(0x80000001, &tmp, &tmp,
-	      &c->x86_capability[cpufeat_word(X86_FEATURE_LAHF_LM)],
-	      &c->x86_capability[cpufeat_word(X86_FEATURE_SYSCALL)]);
+	if ((c->extended_cpuid_level >> 16) != 0x8000)
+		c->extended_cpuid_level = 0;
+	if (c->extended_cpuid_level > 0x80000000)
+		cpuid(0x80000001, &tmp, &tmp,
+		      &c->x86_capability[cpufeat_word(X86_FEATURE_LAHF_LM)],
+		      &c->x86_capability[cpufeat_word(X86_FEATURE_SYSCALL)]);
 	if (c == &boot_cpu_data)
 		bootsym(cpuid_ext_features) =
 			c->x86_capability[cpufeat_word(X86_FEATURE_NX)];
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -605,7 +605,9 @@ static void __init efi_arch_handle_modul
 
 static void __init efi_arch_cpu(void)
 {
-    if ( cpuid_eax(0x80000000) > 0x80000000 )
+    uint32_t eax = cpuid_eax(0x80000000);
+
+    if ( (eax >> 16) == 0x8000 && eax > 0x80000000 )
     {
         cpuid_ext_features = cpuid_edx(0x80000001);
         boot_cpu_data.x86_capability[cpufeat_word(X86_FEATURE_SYSCALL)]
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -884,7 +884,7 @@ const char *hvm_efer_valid(const struct
 
         ASSERT(v->domain == current->domain);
         hvm_cpuid(0x80000000, &level, NULL, NULL, NULL);
-        if ( level >= 0x80000001 )
+        if ( (level >> 16) == 0x8000 && level > 0x80000000 )
         {
             unsigned int dummy;
 
--- a/xen/arch/x86/hvm/mtrr.c
+++ b/xen/arch/x86/hvm/mtrr.c
@@ -455,7 +455,7 @@ bool_t mtrr_var_range_msr_set(
     {
         phys_addr = 36;
         hvm_cpuid(0x80000000, &eax, NULL, NULL, NULL);
-        if ( eax >= 0x80000008 )
+        if ( (eax >> 16) == 0x8000 && eax >= 0x80000008 )
         {
             hvm_cpuid(0x80000008, &eax, NULL, NULL, NULL);
             phys_addr = (uint8_t)eax;

Comments

Andrew Cooper May 9, 2016, 1:25 p.m. UTC | #1 
On 09/05/16 14:19, Jan Beulich wrote:
> We should consistently check the upper 16 bits to be equal 0x8000 and
> only then the full value to be >= the desired level.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> As to inclusion in 4.7 - I think this would be desirable, but it's not
> a must: I'm unaware of real world environments where
> CPUID[0x80000000].EAX would yield a value not having the upper 16 bits
> set to 0x8000.

Either way, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Wei Liu May 9, 2016, 1:34 p.m. UTC | #2 
On Mon, May 09, 2016 at 07:19:56AM -0600, Jan Beulich wrote:
> We should consistently check the upper 16 bits to be equal 0x8000 and
> only then the full value to be >= the desired level.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> As to inclusion in 4.7 - I think this would be desirable, but it's not
> a must: I'm unaware of real world environments where
> CPUID[0x80000000].EAX would yield a value not having the upper 16 bits
> set to 0x8000.
> 

We're early in our RC so:

Release-acked-by: Wei Liu <wei.liu2@citrix.com>

Wei.
diff mbox

Patch

--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -133,7 +133,10 @@  __start:
         /* Interrogate CPU extended features via CPUID. */
         mov     $0x80000000,%eax
         cpuid
+        shld    $16,%eax,%ecx
         xor     %edx,%edx
+        cmp     $0x8000,%cx         # any function @ 0x8000xxxx?
+        jne     1f
         cmp     $0x80000000,%eax    # any function > 0x80000000?
         jbe     1f
         mov     $0x80000001,%eax
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -297,9 +297,12 @@  static void generic_identify(struct cpui
 
 	/* AMD-defined flags: level 0x80000001 */
 	c->extended_cpuid_level = cpuid_eax(0x80000000);
-	cpuid(0x80000001, &tmp, &tmp,
-	      &c->x86_capability[cpufeat_word(X86_FEATURE_LAHF_LM)],
-	      &c->x86_capability[cpufeat_word(X86_FEATURE_SYSCALL)]);
+	if ((c->extended_cpuid_level >> 16) != 0x8000)
+		c->extended_cpuid_level = 0;
+	if (c->extended_cpuid_level > 0x80000000)
+		cpuid(0x80000001, &tmp, &tmp,
+		      &c->x86_capability[cpufeat_word(X86_FEATURE_LAHF_LM)],
+		      &c->x86_capability[cpufeat_word(X86_FEATURE_SYSCALL)]);
 	if (c == &boot_cpu_data)
 		bootsym(cpuid_ext_features) =
 			c->x86_capability[cpufeat_word(X86_FEATURE_NX)];
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -605,7 +605,9 @@  static void __init efi_arch_handle_modul
 
 static void __init efi_arch_cpu(void)
 {
-    if ( cpuid_eax(0x80000000) > 0x80000000 )
+    uint32_t eax = cpuid_eax(0x80000000);
+
+    if ( (eax >> 16) == 0x8000 && eax > 0x80000000 )
     {
         cpuid_ext_features = cpuid_edx(0x80000001);
         boot_cpu_data.x86_capability[cpufeat_word(X86_FEATURE_SYSCALL)]
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -884,7 +884,7 @@  const char *hvm_efer_valid(const struct
 
         ASSERT(v->domain == current->domain);
         hvm_cpuid(0x80000000, &level, NULL, NULL, NULL);
-        if ( level >= 0x80000001 )
+        if ( (level >> 16) == 0x8000 && level > 0x80000000 )
         {
             unsigned int dummy;
 
--- a/xen/arch/x86/hvm/mtrr.c
+++ b/xen/arch/x86/hvm/mtrr.c
@@ -455,7 +455,7 @@  bool_t mtrr_var_range_msr_set(
     {
         phys_addr = 36;
         hvm_cpuid(0x80000000, &eax, NULL, NULL, NULL);
-        if ( eax >= 0x80000008 )
+        if ( (eax >> 16) == 0x8000 && eax >= 0x80000008 )
         {
             hvm_cpuid(0x80000008, &eax, NULL, NULL, NULL);
             phys_addr = (uint8_t)eax;