| Message ID | 1458056124-8024-2-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Mar 15, 2016 at 03:34:57PM +0000, Andrew Cooper wrote: > None of these features are interesting for Xen to use, or to be advertised to > guests. Doing so identifies further areas of code which can be removed now > that 32bit support has been dropped. > > IA64 has a sole user in microcode_intel.c. While it is plausible for a 32bit > x86 hypervisor to get there via IA64's x86 emulation, a 64bit x86 hypervisor > most certainly won't. > > MP proves to be more complicated. It is only advertised on some K7 > processors, not on K8 or newer, and now listed as reserved in the AMD manual. > Cleaning this up reveals two chunks of common SMP code which was only > applicable to K7 processors, which are 32bit only. > > While cleaning this area up, remove the incosistent use of newlines in the s/incosistent/inconsistent/ > cpu_has_* definition block. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > > v3: New > > This cleanup removes the only users of TAINT_UNSAFE_SMP, but I have left the > infrastructure incase it needs to be used in the future. > --- > xen/arch/x86/microcode_intel.c | 3 +-- > xen/arch/x86/smpboot.c | 46 ---------------------------------------- > xen/include/asm-x86/cpufeature.h | 18 ---------------- > 3 files changed, 1 insertion(+), 66 deletions(-) Aaah, can't beat those diffstats. > > diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c > index 0a5f403..6949c25 100644 > --- a/xen/arch/x86/microcode_intel.c > +++ b/xen/arch/x86/microcode_intel.c > @@ -99,8 +99,7 @@ static int collect_cpu_info(unsigned int cpu_num, struct cpu_signature *csig) > > memset(csig, 0, sizeof(*csig)); > > - if ( (c->x86_vendor != X86_VENDOR_INTEL) || (c->x86 < 6) || > - cpu_has(c, X86_FEATURE_IA64) ) > + if ( (c->x86_vendor != X86_VENDOR_INTEL) || (c->x86 < 6) ) > { > printk(KERN_ERR "microcode: CPU%d not a capable Intel " > "processor\n", cpu_num); > diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c > index 1eb16cb..1c02681 100644 > --- a/xen/arch/x86/smpboot.c > +++ b/xen/arch/x86/smpboot.c > @@ -98,39 +98,6 @@ static void smp_store_cpu_info(int id) > secondary_socket_cpumask = NULL; > } > } > - > - /* > - * Certain Athlons might work (for various values of 'work') in SMP > - * but they are not certified as MP capable. > - */ > - if ( (c->x86_vendor == X86_VENDOR_AMD) && (c->x86 == 6) ) > - { > - /* Athlon 660/661 is valid. */ > - if ( (c->x86_model==6) && ((c->x86_mask==0) || (c->x86_mask==1)) ) > - goto valid_k7; > - > - /* Duron 670 is valid */ > - if ( (c->x86_model==7) && (c->x86_mask==0) ) > - goto valid_k7; > - > - /* > - * Athlon 662, Duron 671, and Athlon >model 7 have capability bit. > - * It's worth noting that the A5 stepping (662) of some Athlon XP's > - * have the MP bit set. > - * See http://www.heise.de/newsticker/data/jow-18.10.01-000 for more. > - */ > - if ( ((c->x86_model==6) && (c->x86_mask>=2)) || > - ((c->x86_model==7) && (c->x86_mask>=1)) || > - (c->x86_model> 7) ) > - if (cpu_has_mp) > - goto valid_k7; > - > - /* If we get here, it's not a certified SMP capable AMD system. */ > - add_taint(TAINT_UNSAFE_SMP); > - } > - > - valid_k7: > - ; > } > > /* > @@ -1042,19 +1009,6 @@ int __cpu_up(unsigned int cpu) > > void __init smp_cpus_done(void) > { > - /* > - * Don't taint if we are running SMP kernel on a single non-MP > - * approved Athlon > - */ > - if ( tainted & TAINT_UNSAFE_SMP ) > - { > - if ( num_online_cpus() > 1 ) > - printk(KERN_INFO "WARNING: This combination of AMD " > - "processors is not suitable for SMP.\n"); > - else > - tainted &= ~TAINT_UNSAFE_SMP; > - } > - > if ( nmi_watchdog == NMI_LOCAL_APIC ) > check_nmi_watchdog(); > > diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h > index 021427c..4ed36fe 100644 > --- a/xen/include/asm-x86/cpufeature.h > +++ b/xen/include/asm-x86/cpufeature.h > @@ -31,7 +31,6 @@ > #define X86_FEATURE_CMOV (0*32+15) /* CMOV instruction (FCMOVCC and FCOMI too if FPU present) */ > #define X86_FEATURE_PAT (0*32+16) /* Page Attribute Table */ > #define X86_FEATURE_PSE36 (0*32+17) /* 36-bit PSEs */ > -#define X86_FEATURE_PN (0*32+18) /* Processor serial number */ > #define X86_FEATURE_CLFLUSH (0*32+19) /* Supports the CLFLUSH instruction */ > #define X86_FEATURE_DS (0*32+21) /* Debug Store */ > #define X86_FEATURE_ACPI (0*32+22) /* ACPI via MSR */ > @@ -40,16 +39,13 @@ > /* of FPU context), and CR4.OSFXSR available */ > #define X86_FEATURE_SSE (0*32+25) /* Streaming SIMD Extensions */ > #define X86_FEATURE_SSE2 (0*32+26) /* Streaming SIMD Extensions-2 */ > -#define X86_FEATURE_SELFSNOOP (0*32+27) /* CPU self snoop */ > #define X86_FEATURE_HT (0*32+28) /* Hyper-Threading */ > #define X86_FEATURE_ACC (0*32+29) /* Automatic clock control */ > -#define X86_FEATURE_IA64 (0*32+30) /* IA-64 processor */ > #define X86_FEATURE_PBE (0*32+31) /* Pending Break Enable */ > > /* AMD-defined CPU features, CPUID level 0x80000001, word 1 */ > /* Don't duplicate feature flags which are redundant with Intel! */ > #define X86_FEATURE_SYSCALL (1*32+11) /* SYSCALL/SYSRET */ > -#define X86_FEATURE_MP (1*32+19) /* MP Capable. */ > #define X86_FEATURE_NX (1*32+20) /* Execute Disable */ > #define X86_FEATURE_MMXEXT (1*32+22) /* AMD MMX extensions */ > #define X86_FEATURE_FFXSR (1*32+25) /* FFXSR instruction optimizations */ > @@ -88,7 +84,6 @@ > #define X86_FEATURE_EST (4*32+ 7) /* Enhanced SpeedStep */ > #define X86_FEATURE_TM2 (4*32+ 8) /* Thermal Monitor 2 */ > #define X86_FEATURE_SSSE3 (4*32+ 9) /* Supplemental Streaming SIMD Extensions-3 */ > -#define X86_FEATURE_CID (4*32+10) /* Context ID */ > #define X86_FEATURE_FMA (4*32+12) /* Fused Multiply Add */ > #define X86_FEATURE_CX16 (4*32+13) /* CMPXCHG16B */ > #define X86_FEATURE_XTPR (4*32+14) /* Send Task Priority Messages */ > @@ -187,41 +182,28 @@ > #define cpu_has_sse2 boot_cpu_has(X86_FEATURE_SSE2) > #define cpu_has_sse3 boot_cpu_has(X86_FEATURE_SSE3) > #define cpu_has_ht boot_cpu_has(X86_FEATURE_HT) > -#define cpu_has_mp 1 > #define cpu_has_nx boot_cpu_has(X86_FEATURE_NX) > #define cpu_has_clflush boot_cpu_has(X86_FEATURE_CLFLUSH) > #define cpu_has_page1gb boot_cpu_has(X86_FEATURE_PAGE1GB) > #define cpu_has_fsgsbase boot_cpu_has(X86_FEATURE_FSGSBASE) > #define cpu_has_aperfmperf boot_cpu_has(X86_FEATURE_APERFMPERF) > - > #define cpu_has_smep boot_cpu_has(X86_FEATURE_SMEP) > #define cpu_has_smap boot_cpu_has(X86_FEATURE_SMAP) > #define cpu_has_fpu_sel (!boot_cpu_has(X86_FEATURE_NO_FPU_SEL)) > - > #define cpu_has_ffxsr ((boot_cpu_data.x86_vendor == X86_VENDOR_AMD) \ > && boot_cpu_has(X86_FEATURE_FFXSR)) > - > #define cpu_has_x2apic boot_cpu_has(X86_FEATURE_X2APIC) > - > #define cpu_has_pcid boot_cpu_has(X86_FEATURE_PCID) > - > #define cpu_has_xsave boot_cpu_has(X86_FEATURE_XSAVE) > #define cpu_has_avx boot_cpu_has(X86_FEATURE_AVX) > #define cpu_has_lwp boot_cpu_has(X86_FEATURE_LWP) > #define cpu_has_mpx boot_cpu_has(X86_FEATURE_MPX) > - > #define cpu_has_arch_perfmon boot_cpu_has(X86_FEATURE_ARCH_PERFMON) > - > #define cpu_has_rdtscp boot_cpu_has(X86_FEATURE_RDTSCP) > - > #define cpu_has_svm boot_cpu_has(X86_FEATURE_SVM) > - > #define cpu_has_vmx boot_cpu_has(X86_FEATURE_VMXE) > - > #define cpu_has_cpuid_faulting boot_cpu_has(X86_FEATURE_CPUID_FAULTING) > - > #define cpu_has_cx16 boot_cpu_has(X86_FEATURE_CX16) > - > #define cpu_has_xsaveopt boot_cpu_has(X86_FEATURE_XSAVEOPT) > #define cpu_has_xsavec boot_cpu_has(X86_FEATURE_XSAVEC) > #define cpu_has_xgetbv1 boot_cpu_has(X86_FEATURE_XGETBV1) > -- > 2.1.4 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
On 16/03/16 07:53, Konrad Rzeszutek Wilk wrote: > On Tue, Mar 15, 2016 at 03:34:57PM +0000, Andrew Cooper wrote: >> None of these features are interesting for Xen to use, or to be advertised to >> guests. Doing so identifies further areas of code which can be removed now >> that 32bit support has been dropped. >> >> IA64 has a sole user in microcode_intel.c. While it is plausible for a 32bit >> x86 hypervisor to get there via IA64's x86 emulation, a 64bit x86 hypervisor >> most certainly won't. >> >> MP proves to be more complicated. It is only advertised on some K7 >> processors, not on K8 or newer, and now listed as reserved in the AMD manual. >> Cleaning this up reveals two chunks of common SMP code which was only >> applicable to K7 processors, which are 32bit only. >> >> While cleaning this area up, remove the incosistent use of newlines in the > s/incosistent/inconsistent/ Fixed. > >> cpu_has_* definition block. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > >> --- >> CC: Jan Beulich <JBeulich@suse.com> >> >> v3: New >> >> This cleanup removes the only users of TAINT_UNSAFE_SMP, but I have left the >> infrastructure incase it needs to be used in the future. >> --- >> xen/arch/x86/microcode_intel.c | 3 +-- >> xen/arch/x86/smpboot.c | 46 ---------------------------------------- >> xen/include/asm-x86/cpufeature.h | 18 ---------------- >> 3 files changed, 1 insertion(+), 66 deletions(-) > Aaah, can't beat those diffstats. Absolutely not. Going into this, I was only expecting the -18 from cpufeature.h ~Andrew
On 3/15/16 10:34 AM, Andrew Cooper wrote: > None of these features are interesting for Xen to use, or to be advertised to > guests. Doing so identifies further areas of code which can be removed now > that 32bit support has been dropped. > > IA64 has a sole user in microcode_intel.c. While it is plausible for a 32bit > x86 hypervisor to get there via IA64's x86 emulation, a 64bit x86 hypervisor > most certainly won't. > > MP proves to be more complicated. It is only advertised on some K7 > processors, not on K8 or newer, and now listed as reserved in the AMD manual. > Cleaning this up reveals two chunks of common SMP code which was only > applicable to K7 processors, which are 32bit only. > > While cleaning this area up, remove the incosistent use of newlines in the Konrad already pointed this out. > cpu_has_* definition block. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- Reviewed-by: Doug Goldstein <cardoe@cardoe.com> This patch seems to be a general improvement and doesn't cause other regressions without the rest of the series so can we merged this one.
>>> On 22.03.16 at 15:06, <cardoe@cardoe.com> wrote: > On 3/15/16 10:34 AM, Andrew Cooper wrote: >> None of these features are interesting for Xen to use, or to be advertised > to >> guests. Doing so identifies further areas of code which can be removed now >> that 32bit support has been dropped. >> >> IA64 has a sole user in microcode_intel.c. While it is plausible for a > 32bit >> x86 hypervisor to get there via IA64's x86 emulation, a 64bit x86 hypervisor >> most certainly won't. >> >> MP proves to be more complicated. It is only advertised on some K7 >> processors, not on K8 or newer, and now listed as reserved in the AMD > manual. >> Cleaning this up reveals two chunks of common SMP code which was only >> applicable to K7 processors, which are 32bit only. >> >> While cleaning this area up, remove the incosistent use of newlines in the > > Konrad already pointed this out. > >> cpu_has_* definition block. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> --- > > Reviewed-by: Doug Goldstein <cardoe@cardoe.com> > > This patch seems to be a general improvement and doesn't cause other > regressions without the rest of the series so can we merged this one. Please check the staging tree before making such requests. Jan
diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c index 0a5f403..6949c25 100644 --- a/xen/arch/x86/microcode_intel.c +++ b/xen/arch/x86/microcode_intel.c @@ -99,8 +99,7 @@ static int collect_cpu_info(unsigned int cpu_num, struct cpu_signature *csig) memset(csig, 0, sizeof(*csig)); - if ( (c->x86_vendor != X86_VENDOR_INTEL) || (c->x86 < 6) || - cpu_has(c, X86_FEATURE_IA64) ) + if ( (c->x86_vendor != X86_VENDOR_INTEL) || (c->x86 < 6) ) { printk(KERN_ERR "microcode: CPU%d not a capable Intel " "processor\n", cpu_num); diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c index 1eb16cb..1c02681 100644 --- a/xen/arch/x86/smpboot.c +++ b/xen/arch/x86/smpboot.c @@ -98,39 +98,6 @@ static void smp_store_cpu_info(int id) secondary_socket_cpumask = NULL; } } - - /* - * Certain Athlons might work (for various values of 'work') in SMP - * but they are not certified as MP capable. - */ - if ( (c->x86_vendor == X86_VENDOR_AMD) && (c->x86 == 6) ) - { - /* Athlon 660/661 is valid. */ - if ( (c->x86_model==6) && ((c->x86_mask==0) || (c->x86_mask==1)) ) - goto valid_k7; - - /* Duron 670 is valid */ - if ( (c->x86_model==7) && (c->x86_mask==0) ) - goto valid_k7; - - /* - * Athlon 662, Duron 671, and Athlon >model 7 have capability bit. - * It's worth noting that the A5 stepping (662) of some Athlon XP's - * have the MP bit set. - * See http://www.heise.de/newsticker/data/jow-18.10.01-000 for more. - */ - if ( ((c->x86_model==6) && (c->x86_mask>=2)) || - ((c->x86_model==7) && (c->x86_mask>=1)) || - (c->x86_model> 7) ) - if (cpu_has_mp) - goto valid_k7; - - /* If we get here, it's not a certified SMP capable AMD system. */ - add_taint(TAINT_UNSAFE_SMP); - } - - valid_k7: - ; } /* @@ -1042,19 +1009,6 @@ int __cpu_up(unsigned int cpu) void __init smp_cpus_done(void) { - /* - * Don't taint if we are running SMP kernel on a single non-MP - * approved Athlon - */ - if ( tainted & TAINT_UNSAFE_SMP ) - { - if ( num_online_cpus() > 1 ) - printk(KERN_INFO "WARNING: This combination of AMD " - "processors is not suitable for SMP.\n"); - else - tainted &= ~TAINT_UNSAFE_SMP; - } - if ( nmi_watchdog == NMI_LOCAL_APIC ) check_nmi_watchdog(); diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h index 021427c..4ed36fe 100644 --- a/xen/include/asm-x86/cpufeature.h +++ b/xen/include/asm-x86/cpufeature.h @@ -31,7 +31,6 @@ #define X86_FEATURE_CMOV (0*32+15) /* CMOV instruction (FCMOVCC and FCOMI too if FPU present) */ #define X86_FEATURE_PAT (0*32+16) /* Page Attribute Table */ #define X86_FEATURE_PSE36 (0*32+17) /* 36-bit PSEs */ -#define X86_FEATURE_PN (0*32+18) /* Processor serial number */ #define X86_FEATURE_CLFLUSH (0*32+19) /* Supports the CLFLUSH instruction */ #define X86_FEATURE_DS (0*32+21) /* Debug Store */ #define X86_FEATURE_ACPI (0*32+22) /* ACPI via MSR */ @@ -40,16 +39,13 @@ /* of FPU context), and CR4.OSFXSR available */ #define X86_FEATURE_SSE (0*32+25) /* Streaming SIMD Extensions */ #define X86_FEATURE_SSE2 (0*32+26) /* Streaming SIMD Extensions-2 */ -#define X86_FEATURE_SELFSNOOP (0*32+27) /* CPU self snoop */ #define X86_FEATURE_HT (0*32+28) /* Hyper-Threading */ #define X86_FEATURE_ACC (0*32+29) /* Automatic clock control */ -#define X86_FEATURE_IA64 (0*32+30) /* IA-64 processor */ #define X86_FEATURE_PBE (0*32+31) /* Pending Break Enable */ /* AMD-defined CPU features, CPUID level 0x80000001, word 1 */ /* Don't duplicate feature flags which are redundant with Intel! */ #define X86_FEATURE_SYSCALL (1*32+11) /* SYSCALL/SYSRET */ -#define X86_FEATURE_MP (1*32+19) /* MP Capable. */ #define X86_FEATURE_NX (1*32+20) /* Execute Disable */ #define X86_FEATURE_MMXEXT (1*32+22) /* AMD MMX extensions */ #define X86_FEATURE_FFXSR (1*32+25) /* FFXSR instruction optimizations */ @@ -88,7 +84,6 @@ #define X86_FEATURE_EST (4*32+ 7) /* Enhanced SpeedStep */ #define X86_FEATURE_TM2 (4*32+ 8) /* Thermal Monitor 2 */ #define X86_FEATURE_SSSE3 (4*32+ 9) /* Supplemental Streaming SIMD Extensions-3 */ -#define X86_FEATURE_CID (4*32+10) /* Context ID */ #define X86_FEATURE_FMA (4*32+12) /* Fused Multiply Add */ #define X86_FEATURE_CX16 (4*32+13) /* CMPXCHG16B */ #define X86_FEATURE_XTPR (4*32+14) /* Send Task Priority Messages */ @@ -187,41 +182,28 @@ #define cpu_has_sse2 boot_cpu_has(X86_FEATURE_SSE2) #define cpu_has_sse3 boot_cpu_has(X86_FEATURE_SSE3) #define cpu_has_ht boot_cpu_has(X86_FEATURE_HT) -#define cpu_has_mp 1 #define cpu_has_nx boot_cpu_has(X86_FEATURE_NX) #define cpu_has_clflush boot_cpu_has(X86_FEATURE_CLFLUSH) #define cpu_has_page1gb boot_cpu_has(X86_FEATURE_PAGE1GB) #define cpu_has_fsgsbase boot_cpu_has(X86_FEATURE_FSGSBASE) #define cpu_has_aperfmperf boot_cpu_has(X86_FEATURE_APERFMPERF) - #define cpu_has_smep boot_cpu_has(X86_FEATURE_SMEP) #define cpu_has_smap boot_cpu_has(X86_FEATURE_SMAP) #define cpu_has_fpu_sel (!boot_cpu_has(X86_FEATURE_NO_FPU_SEL)) - #define cpu_has_ffxsr ((boot_cpu_data.x86_vendor == X86_VENDOR_AMD) \ && boot_cpu_has(X86_FEATURE_FFXSR)) - #define cpu_has_x2apic boot_cpu_has(X86_FEATURE_X2APIC) - #define cpu_has_pcid boot_cpu_has(X86_FEATURE_PCID) - #define cpu_has_xsave boot_cpu_has(X86_FEATURE_XSAVE) #define cpu_has_avx boot_cpu_has(X86_FEATURE_AVX) #define cpu_has_lwp boot_cpu_has(X86_FEATURE_LWP) #define cpu_has_mpx boot_cpu_has(X86_FEATURE_MPX) - #define cpu_has_arch_perfmon boot_cpu_has(X86_FEATURE_ARCH_PERFMON) - #define cpu_has_rdtscp boot_cpu_has(X86_FEATURE_RDTSCP) - #define cpu_has_svm boot_cpu_has(X86_FEATURE_SVM) - #define cpu_has_vmx boot_cpu_has(X86_FEATURE_VMXE) - #define cpu_has_cpuid_faulting boot_cpu_has(X86_FEATURE_CPUID_FAULTING) - #define cpu_has_cx16 boot_cpu_has(X86_FEATURE_CX16) - #define cpu_has_xsaveopt boot_cpu_has(X86_FEATURE_XSAVEOPT) #define cpu_has_xsavec boot_cpu_has(X86_FEATURE_XSAVEC) #define cpu_has_xgetbv1 boot_cpu_has(X86_FEATURE_XGETBV1)
None of these features are interesting for Xen to use, or to be advertised to guests. Doing so identifies further areas of code which can be removed now that 32bit support has been dropped. IA64 has a sole user in microcode_intel.c. While it is plausible for a 32bit x86 hypervisor to get there via IA64's x86 emulation, a 64bit x86 hypervisor most certainly won't. MP proves to be more complicated. It is only advertised on some K7 processors, not on K8 or newer, and now listed as reserved in the AMD manual. Cleaning this up reveals two chunks of common SMP code which was only applicable to K7 processors, which are 32bit only. While cleaning this area up, remove the incosistent use of newlines in the cpu_has_* definition block. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> v3: New This cleanup removes the only users of TAINT_UNSAFE_SMP, but I have left the infrastructure incase it needs to be used in the future. --- xen/arch/x86/microcode_intel.c | 3 +-- xen/arch/x86/smpboot.c | 46 ---------------------------------------- xen/include/asm-x86/cpufeature.h | 18 ---------------- 3 files changed, 1 insertion(+), 66 deletions(-)