diff mbox

[v2] x86: cap address bits CPUID output

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

Commit Message

Jan Beulich May 9, 2016, 1:15 p.m. UTC 
Don't use more or report more to guests than we are capable of
handling.

At once
- correct the involved extended CPUID level checks,
- simplify the code in hvm_cpuid() and mtrr_top_of_ram().

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Also correct extended CPUID level range checks.
x86: cap address bits CPUID output

Don't use more or report more to guests than we are capable of
handling.

At once
- correct the involved extended CPUID level checks,
- simplify the code in hvm_cpuid() and mtrr_top_of_ram().

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Also correct extended CPUID level range checks.

--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -46,6 +46,7 @@ const struct cpu_dev *__read_mostly cpu_
 
 unsigned int paddr_bits __read_mostly = 36;
 unsigned int hap_paddr_bits __read_mostly = 36;
+unsigned int vaddr_bits __read_mostly = VADDR_BITS;
 
 /*
  * Default host IA32_CR_PAT value to cover all memory types.
@@ -237,10 +238,18 @@ static void __init early_cpu_detect(void
 	c->x86_capability[cpufeat_word(X86_FEATURE_FPU)] = edx;
 	c->x86_capability[cpufeat_word(X86_FEATURE_SSE3)] = ecx;
 
-	if ( cpuid_eax(0x80000000) >= 0x80000008 ) {
+	eax = cpuid_eax(0x80000000);
+	if ((eax >> 16) == 0x8000 && eax >= 0x80000008) {
 		eax = cpuid_eax(0x80000008);
 		paddr_bits = eax & 0xff;
+		if (paddr_bits > PADDR_BITS)
+			paddr_bits = PADDR_BITS;
+		vaddr_bits = (eax >> 8) & 0xff;
+		if (vaddr_bits > VADDR_BITS)
+			vaddr_bits = VADDR_BITS;
 		hap_paddr_bits = ((eax >> 16) & 0xff) ?: paddr_bits;
+		if (hap_paddr_bits > PADDR_BITS)
+			hap_paddr_bits = PADDR_BITS;
 	}
 }
 
--- a/xen/arch/x86/e820.c
+++ b/xen/arch/x86/e820.c
@@ -451,11 +451,12 @@ static uint64_t __init mtrr_top_of_ram(v
          return 0;
 
     /* Find the physical address size for this CPU. */
-    cpuid(0x80000000, &eax, &ebx, &ecx, &edx);
-    if ( eax >= 0x80000008 )
+    eax = cpuid_eax(0x80000000);
+    if ( (eax >> 16) == 0x8000 && eax >= 0x80000008 )
     {
-        cpuid(0x80000008, &eax, &ebx, &ecx, &edx);
-        phys_bits = (uint8_t)eax;
+        phys_bits = (uint8_t)cpuid_eax(0x80000008);
+        if ( phys_bits > PADDR_BITS )
+            phys_bits = PADDR_BITS;
     }
     addr_mask = ((1ull << phys_bits) - 1) & ~((1ull << 12) - 1);
 
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3504,19 +3504,19 @@ void hvm_cpuid(unsigned int input, unsig
         break;
 
     case 0x80000008:
+        *eax &= 0xff;
         count = d->arch.paging.gfn_bits + PAGE_SHIFT;
-        if ( (*eax & 0xff) > count )
-            *eax = (*eax & ~0xff) | count;
+        if ( *eax > count )
+            *eax = count;
 
         hvm_cpuid(1, NULL, NULL, NULL, &_edx);
         count = _edx & (cpufeat_mask(X86_FEATURE_PAE) |
                         cpufeat_mask(X86_FEATURE_PSE36)) ? 36 : 32;
-        if ( (*eax & 0xff) < count )
-            *eax = (*eax & ~0xff) | count;
+        if ( *eax < count )
+            *eax = count;
 
         hvm_cpuid(0x80000001, NULL, NULL, NULL, &_edx);
-        *eax = (*eax & ~0xffff00) | (_edx & cpufeat_mask(X86_FEATURE_LM)
-                                     ? 0x3000 : 0x2000);
+        *eax |= _edx & cpufeat_mask(X86_FEATURE_LM) ? vaddr_bits << 8 : 0x2000;
 
         *ebx &= hvm_featureset[FEATURESET_e8b];
         break;
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1146,6 +1146,7 @@ void pv_cpuid(struct cpu_user_regs *regs
         break;
 
     case 0x80000008:
+        a = paddr_bits | (vaddr_bits << 8);
         b &= pv_featureset[FEATURESET_e8b];
         break;
 
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -216,10 +216,12 @@ extern bool_t opt_cpu_info;
 extern u32 cpuid_ext_features;
 extern u64 trampoline_misc_enable_off;
 
-/* Maximum width of physical addresses supported by the hardware */
+/* Maximum width of physical addresses supported by the hardware. */
 extern unsigned int paddr_bits;
-/* Max physical address width supported within HAP guests */
+/* Max physical address width supported within HAP guests. */
 extern unsigned int hap_paddr_bits;
+/* Maximum width of virtual addresses supported by the hardware. */
+extern unsigned int vaddr_bits;
 
 extern const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id table[]);

Comments

Andrew Cooper May 9, 2016, 1:21 p.m. UTC | #1 
On 09/05/16 14:15, Jan Beulich wrote:
> Don't use more or report more to guests than we are capable of
> handling.
>
> At once
> - correct the involved extended CPUID level checks,
> - simplify the code in hvm_cpuid() and mtrr_top_of_ram().
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, with perhaps one
tweak

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3504,19 +3504,19 @@ void hvm_cpuid(unsigned int input, unsig
>          break;
>  
>      case 0x80000008:
> +        *eax &= 0xff;
>          count = d->arch.paging.gfn_bits + PAGE_SHIFT;
> -        if ( (*eax & 0xff) > count )
> -            *eax = (*eax & ~0xff) | count;
> +        if ( *eax > count )
> +            *eax = count;
>  
>          hvm_cpuid(1, NULL, NULL, NULL, &_edx);
>          count = _edx & (cpufeat_mask(X86_FEATURE_PAE) |
>                          cpufeat_mask(X86_FEATURE_PSE36)) ? 36 : 32;
> -        if ( (*eax & 0xff) < count )
> -            *eax = (*eax & ~0xff) | count;
> +        if ( *eax < count )
> +            *eax = count;
>  
>          hvm_cpuid(0x80000001, NULL, NULL, NULL, &_edx);
> -        *eax = (*eax & ~0xffff00) | (_edx & cpufeat_mask(X86_FEATURE_LM)
> -                                     ? 0x3000 : 0x2000);
> +        *eax |= _edx & cpufeat_mask(X86_FEATURE_LM) ? vaddr_bits << 8 : 0x2000;

Using (32 << 8) would be clearer than 0x2000.

~Andrew
Jan Beulich May 9, 2016, 1:26 p.m. UTC | #2 
>>> On 09.05.16 at 15:21, <andrew.cooper3@citrix.com> wrote:
> On 09/05/16 14:15, Jan Beulich wrote:
>> Don't use more or report more to guests than we are capable of
>> handling.
>>
>> At once
>> - correct the involved extended CPUID level checks,
>> - simplify the code in hvm_cpuid() and mtrr_top_of_ram().
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, with perhaps one
> tweak
> 
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -3504,19 +3504,19 @@ void hvm_cpuid(unsigned int input, unsig
>>          break;
>>  
>>      case 0x80000008:
>> +        *eax &= 0xff;
>>          count = d->arch.paging.gfn_bits + PAGE_SHIFT;
>> -        if ( (*eax & 0xff) > count )
>> -            *eax = (*eax & ~0xff) | count;
>> +        if ( *eax > count )
>> +            *eax = count;
>>  
>>          hvm_cpuid(1, NULL, NULL, NULL, &_edx);
>>          count = _edx & (cpufeat_mask(X86_FEATURE_PAE) |
>>                          cpufeat_mask(X86_FEATURE_PSE36)) ? 36 : 32;
>> -        if ( (*eax & 0xff) < count )
>> -            *eax = (*eax & ~0xff) | count;
>> +        if ( *eax < count )
>> +            *eax = count;
>>  
>>          hvm_cpuid(0x80000001, NULL, NULL, NULL, &_edx);
>> -        *eax = (*eax & ~0xffff00) | (_edx & cpufeat_mask(X86_FEATURE_LM)
>> -                                     ? 0x3000 : 0x2000);
>> +        *eax |= _edx & cpufeat_mask(X86_FEATURE_LM) ? vaddr_bits << 8 : 0x2000;

In that case I'd prefer

        *eax |= (_edx & cpufeat_mask(X86_FEATURE_LM) ? vaddr_bits : 32) << 8;

Jan
Andrew Cooper May 9, 2016, 1:27 p.m. UTC | #3 
On 09/05/16 14:26, Jan Beulich wrote:
>>>> On 09.05.16 at 15:21, <andrew.cooper3@citrix.com> wrote:
>> On 09/05/16 14:15, Jan Beulich wrote:
>>> Don't use more or report more to guests than we are capable of
>>> handling.
>>>
>>> At once
>>> - correct the involved extended CPUID level checks,
>>> - simplify the code in hvm_cpuid() and mtrr_top_of_ram().
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, with perhaps one
>> tweak
>>
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -3504,19 +3504,19 @@ void hvm_cpuid(unsigned int input, unsig
>>>          break;
>>>  
>>>      case 0x80000008:
>>> +        *eax &= 0xff;
>>>          count = d->arch.paging.gfn_bits + PAGE_SHIFT;
>>> -        if ( (*eax & 0xff) > count )
>>> -            *eax = (*eax & ~0xff) | count;
>>> +        if ( *eax > count )
>>> +            *eax = count;
>>>  
>>>          hvm_cpuid(1, NULL, NULL, NULL, &_edx);
>>>          count = _edx & (cpufeat_mask(X86_FEATURE_PAE) |
>>>                          cpufeat_mask(X86_FEATURE_PSE36)) ? 36 : 32;
>>> -        if ( (*eax & 0xff) < count )
>>> -            *eax = (*eax & ~0xff) | count;
>>> +        if ( *eax < count )
>>> +            *eax = count;
>>>  
>>>          hvm_cpuid(0x80000001, NULL, NULL, NULL, &_edx);
>>> -        *eax = (*eax & ~0xffff00) | (_edx & cpufeat_mask(X86_FEATURE_LM)
>>> -                                     ? 0x3000 : 0x2000);
>>> +        *eax |= _edx & cpufeat_mask(X86_FEATURE_LM) ? vaddr_bits << 8 : 0x2000;
> In that case I'd prefer
>
>         *eax |= (_edx & cpufeat_mask(X86_FEATURE_LM) ? vaddr_bits : 32) << 8;

That looks better.

~Andrew
Wei Liu May 9, 2016, 1:31 p.m. UTC | #4 
On Mon, May 09, 2016 at 07:15:47AM -0600, Jan Beulich wrote:
> Don't use more or report more to guests than we are capable of
> handling.
> 
> At once
> - correct the involved extended CPUID level checks,
> - simplify the code in hvm_cpuid() and mtrr_top_of_ram().
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

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

Patch

--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -46,6 +46,7 @@  const struct cpu_dev *__read_mostly cpu_
 
 unsigned int paddr_bits __read_mostly = 36;
 unsigned int hap_paddr_bits __read_mostly = 36;
+unsigned int vaddr_bits __read_mostly = VADDR_BITS;
 
 /*
  * Default host IA32_CR_PAT value to cover all memory types.
@@ -237,10 +238,18 @@  static void __init early_cpu_detect(void
 	c->x86_capability[cpufeat_word(X86_FEATURE_FPU)] = edx;
 	c->x86_capability[cpufeat_word(X86_FEATURE_SSE3)] = ecx;
 
-	if ( cpuid_eax(0x80000000) >= 0x80000008 ) {
+	eax = cpuid_eax(0x80000000);
+	if ((eax >> 16) == 0x8000 && eax >= 0x80000008) {
 		eax = cpuid_eax(0x80000008);
 		paddr_bits = eax & 0xff;
+		if (paddr_bits > PADDR_BITS)
+			paddr_bits = PADDR_BITS;
+		vaddr_bits = (eax >> 8) & 0xff;
+		if (vaddr_bits > VADDR_BITS)
+			vaddr_bits = VADDR_BITS;
 		hap_paddr_bits = ((eax >> 16) & 0xff) ?: paddr_bits;
+		if (hap_paddr_bits > PADDR_BITS)
+			hap_paddr_bits = PADDR_BITS;
 	}
 }
 
--- a/xen/arch/x86/e820.c
+++ b/xen/arch/x86/e820.c
@@ -451,11 +451,12 @@  static uint64_t __init mtrr_top_of_ram(v
          return 0;
 
     /* Find the physical address size for this CPU. */
-    cpuid(0x80000000, &eax, &ebx, &ecx, &edx);
-    if ( eax >= 0x80000008 )
+    eax = cpuid_eax(0x80000000);
+    if ( (eax >> 16) == 0x8000 && eax >= 0x80000008 )
     {
-        cpuid(0x80000008, &eax, &ebx, &ecx, &edx);
-        phys_bits = (uint8_t)eax;
+        phys_bits = (uint8_t)cpuid_eax(0x80000008);
+        if ( phys_bits > PADDR_BITS )
+            phys_bits = PADDR_BITS;
     }
     addr_mask = ((1ull << phys_bits) - 1) & ~((1ull << 12) - 1);
 
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3504,19 +3504,19 @@  void hvm_cpuid(unsigned int input, unsig
         break;
 
     case 0x80000008:
+        *eax &= 0xff;
         count = d->arch.paging.gfn_bits + PAGE_SHIFT;
-        if ( (*eax & 0xff) > count )
-            *eax = (*eax & ~0xff) | count;
+        if ( *eax > count )
+            *eax = count;
 
         hvm_cpuid(1, NULL, NULL, NULL, &_edx);
         count = _edx & (cpufeat_mask(X86_FEATURE_PAE) |
                         cpufeat_mask(X86_FEATURE_PSE36)) ? 36 : 32;
-        if ( (*eax & 0xff) < count )
-            *eax = (*eax & ~0xff) | count;
+        if ( *eax < count )
+            *eax = count;
 
         hvm_cpuid(0x80000001, NULL, NULL, NULL, &_edx);
-        *eax = (*eax & ~0xffff00) | (_edx & cpufeat_mask(X86_FEATURE_LM)
-                                     ? 0x3000 : 0x2000);
+        *eax |= _edx & cpufeat_mask(X86_FEATURE_LM) ? vaddr_bits << 8 : 0x2000;
 
         *ebx &= hvm_featureset[FEATURESET_e8b];
         break;
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1146,6 +1146,7 @@  void pv_cpuid(struct cpu_user_regs *regs
         break;
 
     case 0x80000008:
+        a = paddr_bits | (vaddr_bits << 8);
         b &= pv_featureset[FEATURESET_e8b];
         break;
 
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -216,10 +216,12 @@  extern bool_t opt_cpu_info;
 extern u32 cpuid_ext_features;
 extern u64 trampoline_misc_enable_off;
 
-/* Maximum width of physical addresses supported by the hardware */
+/* Maximum width of physical addresses supported by the hardware. */
 extern unsigned int paddr_bits;
-/* Max physical address width supported within HAP guests */
+/* Max physical address width supported within HAP guests. */
 extern unsigned int hap_paddr_bits;
+/* Maximum width of virtual addresses supported by the hardware. */
+extern unsigned int vaddr_bits;
 
 extern const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id table[]);