| Message ID | 57278A4D02000078000E7B8B@prv-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 02.05.16 at 17:11, <JBeulich@suse.com> wrote: > 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 <jbeulich@suse.com> > > --- 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(0x80000000) >= 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,11 @@ 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 ) > + if ( cpuid_eax(0x80000000) >= 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[]); >
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(0x80000000, &eax, &ebx, &ecx, &edx); > - if ( eax >= 0x80000008 ) > + if ( cpuid_eax(0x80000000) >= 0x80000008 ) 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
>>> On 09.05.16 at 14:35, <andrew.cooper3@citrix.com> 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(0x80000000, &eax, &ebx, &ecx, &edx); >> - if ( eax >= 0x80000008 ) >> + if ( cpuid_eax(0x80000000) >= 0x80000008 ) > > 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
--- 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(0x80000000) >= 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,11 @@ 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 ) + if ( cpuid_eax(0x80000000) >= 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[]);