| Message ID | 20250306233519.3006560-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/e820: Remove opencoded vendor/feature checks | expand |
On 07.03.2025 00:35, Andrew Cooper wrote: > We've already scanned features by the time init_e820() is called. Remove the > cpuid() calls. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> > Backporting. Not sure it's worth backporing, but it is safe (just) to > backport past commit 365f408339d3 ("x86/boot: Load microcode much earlier on > boot"). That commit was the last one to reposition early_cpu_init(). At least I wouldn't consider such cleanup to be an obvious backporting candidate. > I'm pretty sure that all 64bit CPUs have MTRR, but I'm less certain if > dropping the check is wise given the variety of VM configurations that exist. We did consider exposing PAT-only configurations to guests, so I don't think we should be implying MTRR from being 64-bit (unless we know cpu_has_hypervisor is false). Jan
On 07.03.2025 09:48, Jan Beulich wrote: > On 07.03.2025 00:35, Andrew Cooper wrote: >> We've already scanned features by the time init_e820() is called. Remove the >> cpuid() calls. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > > Reviewed-by: Jan Beulich <jbeulich@suse.com> > >> Backporting. Not sure it's worth backporing, but it is safe (just) to >> backport past commit 365f408339d3 ("x86/boot: Load microcode much earlier on >> boot"). That commit was the last one to reposition early_cpu_init(). > > At least I wouldn't consider such cleanup to be an obvious backporting > candidate. > >> I'm pretty sure that all 64bit CPUs have MTRR, but I'm less certain if >> dropping the check is wise given the variety of VM configurations that exist. > > We did consider exposing PAT-only configurations to guests, so I don't think > we should be implying MTRR from being 64-bit (unless we know cpu_has_hypervisor > is false). Except that - we do: #define cpu_has_mtrr 1 I guess that wants undoing (pre-dating the consideration of Xen running virtualized itself). Jan
diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c index e052e84de75c..ca577c0bde0f 100644 --- a/xen/arch/x86/e820.c +++ b/xen/arch/x86/e820.c @@ -421,21 +421,12 @@ static void __init clip_to_limit(uint64_t limit, const char *warnmsg) /* Conservative estimate of top-of-RAM by looking for MTRR WB regions. */ static uint64_t __init mtrr_top_of_ram(void) { - uint32_t eax, ebx, ecx, edx; uint64_t mtrr_cap, mtrr_def, addr_mask, base, mask, top; unsigned int i; /* By default we check only Intel systems. */ if ( e820_mtrr_clip == -1 ) - { - char vendor[13]; - cpuid(0x00000000, &eax, - (uint32_t *)&vendor[0], - (uint32_t *)&vendor[8], - (uint32_t *)&vendor[4]); - vendor[12] = '\0'; - e820_mtrr_clip = !strcmp(vendor, "GenuineIntel"); - } + e820_mtrr_clip = boot_cpu_data.x86_vendor == X86_VENDOR_INTEL; if ( !e820_mtrr_clip ) return 0; @@ -444,8 +435,7 @@ static uint64_t __init mtrr_top_of_ram(void) printk("Checking MTRR ranges...\n"); /* Does the CPU support architectural MTRRs? */ - cpuid(0x00000001, &eax, &ebx, &ecx, &edx); - if ( !test_bit(X86_FEATURE_MTRR & 31, &edx) ) + if ( !cpu_has_mtrr ) return 0; /* paddr_bits must have been set at this point */
We've already scanned features by the time init_e820() is called. Remove the cpuid() calls. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> Backporting. Not sure it's worth backporing, but it is safe (just) to backport past commit 365f408339d3 ("x86/boot: Load microcode much earlier on boot"). That commit was the last one to reposition early_cpu_init(). I'm pretty sure that all 64bit CPUs have MTRR, but I'm less certain if dropping the check is wise given the variety of VM configurations that exist. --- xen/arch/x86/e820.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-)