Re: [Xen-devel] [PATCH] x86: cap address bits CPUID output

[Jan Beulich]

>>> On 09.05.16 at 14:35,  wrote:
> On 02/05/16 16:11, Jan Beulich wrote:
>> --- a/xen/arch/x86/e820.c
>> +++ b/xen/arch/x86/e820.c
>> @@ -451,11 +451,11 @@ static uint64_t __init mtrr_top_of_ram(v
>>   return 0;
>>  
>>  /* Find the physical address size for this CPU. */
>> -cpuid(0x8000, , , , );
>> -if ( eax >= 0x8008 )
>> +if ( cpuid_eax(0x8000) >= 0x8008 )
> 
> You indicated in review of my cpuid series that this check is buggy, and
> the upper half needs comparing exactly to 0x8000.

Oh, indeed. How did I forget?

> It occurs to me that early_cpu_detect() really should be callled
> earlier, to avoid needing to recalculate this information.

Well, I had (briefly) looked at that option, but iirc it didn't
seem feasible to move it up as early as we'd need it to be.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86: cap address bits CPUID output

[Andrew Cooper]

On 02/05/16 16:11, Jan Beulich wrote:
> --- a/xen/arch/x86/e820.c
> +++ b/xen/arch/x86/e820.c
> @@ -451,11 +451,11 @@ static uint64_t __init mtrr_top_of_ram(v
>   return 0;
>  
>  /* Find the physical address size for this CPU. */
> -cpuid(0x8000, , , , );
> -if ( eax >= 0x8008 )
> +if ( cpuid_eax(0x8000) >= 0x8008 )

You indicated in review of my cpuid series that this check is buggy, and
the upper half needs comparing exactly to 0x8000.

It occurs to me that early_cpu_detect() really should be callled
earlier, to avoid needing to recalculate this information.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH] x86: cap address bits CPUID output

[Jan Beulich]

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

At once simplify the code in hvm_cpuid() and mtrr_top_of_ram().

Signed-off-by: Jan Beulich 

--- 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.
@@ -240,7 +241,14 @@ static void __init early_cpu_detect(void
if ( cpuid_eax(0x8000) >= 0x8008 ) {
eax = cpuid_eax(0x8008);
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,11 @@ static uint64_t __init mtrr_top_of_ram(v
  return 0;
 
 /* Find the physical address size for this CPU. */
-cpuid(0x8000, , , , );
-if ( eax >= 0x8008 )
+if ( cpuid_eax(0x8000) >= 0x8008 )
 {
-cpuid(0x8008, , , , );
-phys_bits = (uint8_t)eax;
+phys_bits = (uint8_t)cpuid_eax(0x8008);
+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 0x8008:
+*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(0x8001, NULL, NULL, NULL, &_edx);
-*eax = (*eax & ~0x00) | (_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 0x8008:
+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[]);
 



x86: cap address bits CPUID output

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

At once simplify the code in hvm_cpuid() and mtrr_top_of_ram().

Signed-off-by: Jan Beulich 

--- 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.
@@ -240,7 +241,14 @@ static void __init early_cpu_detect(void
if ( cpuid_eax(0x8000) >= 0x8008 ) {
eax = cpuid_eax(0x8008);
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
@@