| Message ID | 5632b05e993cca78a58b800dd37165ccd80b944f.1723196909.git.Sergiy_Kibrik@epam.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86/CPU: optional build of Intel/AMD CPUs support | expand |
On 09.08.2024 12:11, Sergiy Kibrik wrote: > --- a/xen/arch/x86/include/asm/amd.h > +++ b/xen/arch/x86/include/asm/amd.h > @@ -158,13 +158,21 @@ > #define is_zen4_uarch() boot_cpu_has(X86_FEATURE_AUTO_IBRS) > > struct cpuinfo_x86; > +#ifdef CONFIG_AMD > int cpu_has_amd_erratum(const struct cpuinfo_x86 *cpu, int osvw_id, ...); > +#else > +static inline int cpu_has_amd_erratum(const struct cpuinfo_x86 *cpu, int osvw_id, ...) Nit: Too long line. > +{ > + return false; I wouldn't mind if you consistently changed the function to return bool, but as long as it returns int I don't think it should be returning false. > @@ -173,5 +181,19 @@ extern bool amd_virt_spec_ctrl; > bool amd_setup_legacy_ssbd(void); > void amd_set_legacy_ssbd(bool enable); > void amd_set_cpuid_user_dis(bool enable); > +#else > +static inline void amd_set_cpuid_user_dis(bool enable) {} > +static inline void amd_set_legacy_ssbd(bool enable) {} > +static inline bool amd_setup_legacy_ssbd(void) > +{ > + return false; > +} Nit: Would be a little nicer if these were in the same order as their corresponding declarations. However, along the lines of one of my comments on the Intel counterpart patch ... > +#define amd_acpi_c1e_quirk (false) > +#define amd_virt_spec_ctrl (false) > +#define amd_legacy_ssbd (false) > + > +static inline void amd_check_disable_c1e(unsigned int port, u8 value) {} > +#endif ... question overall is how many of these stubs are really required, once clearly AMD-only code is properly taken care of (perhaps not just in spec_ctrl.c). Jan
13.08.24 10:50, Jan Beulich: > On 09.08.2024 12:11, Sergiy Kibrik wrote: >> --- a/xen/arch/x86/include/asm/amd.h >> +++ b/xen/arch/x86/include/asm/amd.h >> @@ -158,13 +158,21 @@ >> #define is_zen4_uarch() boot_cpu_has(X86_FEATURE_AUTO_IBRS) >> >> struct cpuinfo_x86; >> +#ifdef CONFIG_AMD >> int cpu_has_amd_erratum(const struct cpuinfo_x86 *cpu, int osvw_id, ...); >> +#else >> +static inline int cpu_has_amd_erratum(const struct cpuinfo_x86 *cpu, int osvw_id, ...) > > Nit: Too long line. > >> +{ >> + return false; > > I wouldn't mind if you consistently changed the function to return > bool, but as long as it returns int I don't think it should be returning > false. > indeed, it should return 0 >> @@ -173,5 +181,19 @@ extern bool amd_virt_spec_ctrl; >> bool amd_setup_legacy_ssbd(void); >> void amd_set_legacy_ssbd(bool enable); >> void amd_set_cpuid_user_dis(bool enable); >> +#else >> +static inline void amd_set_cpuid_user_dis(bool enable) {} >> +static inline void amd_set_legacy_ssbd(bool enable) {} >> +static inline bool amd_setup_legacy_ssbd(void) >> +{ >> + return false; >> +} > > Nit: Would be a little nicer if these were in the same order as their > corresponding declarations. However, along the lines of one of my > comments on the Intel counterpart patch ... > >> +#define amd_acpi_c1e_quirk (false) >> +#define amd_virt_spec_ctrl (false) >> +#define amd_legacy_ssbd (false) >> + >> +static inline void amd_check_disable_c1e(unsigned int port, u8 value) {} >> +#endif > > ... question overall is how many of these stubs are really required, > once clearly AMD-only code is properly taken care of (perhaps not just > in spec_ctrl.c). > most of these functions-stubs can go away, though it'll require more CONFIG_AMD checks at call sites, and more patches probably. -Sergiy
diff --git a/xen/arch/x86/cpu/Makefile b/xen/arch/x86/cpu/Makefile index 020c86bda3..5efd87be38 100644 --- a/xen/arch/x86/cpu/Makefile +++ b/xen/arch/x86/cpu/Makefile @@ -2,10 +2,10 @@ obj-y += mcheck/ obj-y += microcode/ obj-y += mtrr/ -obj-y += amd.o +obj-$(CONFIG_AMD) += amd.o obj-y += centaur.o obj-y += common.o -obj-y += hygon.o +obj-$(CONFIG_AMD) += hygon.o obj-$(CONFIG_INTEL) += intel.o obj-$(CONFIG_INTEL) += intel_cacheinfo.o obj-y += mwait-idle.o diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c index 50ce13f81c..7be689c2e3 100644 --- a/xen/arch/x86/cpu/common.c +++ b/xen/arch/x86/cpu/common.c @@ -341,9 +341,11 @@ void __init early_cpu_init(bool verbose) actual_cpu = intel_cpu_dev; break; case X86_VENDOR_SHANGHAI: actual_cpu = shanghai_cpu_dev; break; #endif +#ifdef CONFIG_AMD case X86_VENDOR_AMD: actual_cpu = amd_cpu_dev; break; - case X86_VENDOR_CENTAUR: actual_cpu = centaur_cpu_dev; break; case X86_VENDOR_HYGON: actual_cpu = hygon_cpu_dev; break; +#endif + case X86_VENDOR_CENTAUR: actual_cpu = centaur_cpu_dev; break; default: actual_cpu = default_cpu; if (!verbose) diff --git a/xen/arch/x86/include/asm/amd.h b/xen/arch/x86/include/asm/amd.h index fa4e0fc766..a2481eddc7 100644 --- a/xen/arch/x86/include/asm/amd.h +++ b/xen/arch/x86/include/asm/amd.h @@ -158,13 +158,21 @@ #define is_zen4_uarch() boot_cpu_has(X86_FEATURE_AUTO_IBRS) struct cpuinfo_x86; +#ifdef CONFIG_AMD int cpu_has_amd_erratum(const struct cpuinfo_x86 *cpu, int osvw_id, ...); +#else +static inline int cpu_has_amd_erratum(const struct cpuinfo_x86 *cpu, int osvw_id, ...) +{ + return false; +} +#endif extern s8 opt_allow_unsafe; void fam10h_check_enable_mmcfg(void); void check_enable_amd_mmconf_dmi(void); +#ifdef CONFIG_AMD extern bool amd_acpi_c1e_quirk; void amd_check_disable_c1e(unsigned int port, u8 value); @@ -173,5 +181,19 @@ extern bool amd_virt_spec_ctrl; bool amd_setup_legacy_ssbd(void); void amd_set_legacy_ssbd(bool enable); void amd_set_cpuid_user_dis(bool enable); +#else +static inline void amd_set_cpuid_user_dis(bool enable) {} +static inline void amd_set_legacy_ssbd(bool enable) {} +static inline bool amd_setup_legacy_ssbd(void) +{ + return false; +} + +#define amd_acpi_c1e_quirk (false) +#define amd_virt_spec_ctrl (false) +#define amd_legacy_ssbd (false) + +static inline void amd_check_disable_c1e(unsigned int port, u8 value) {} +#endif #endif /* __AMD_H__ */ diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c index 92405b8be7..8231515c80 100644 --- a/xen/arch/x86/spec_ctrl.c +++ b/xen/arch/x86/spec_ctrl.c @@ -1884,10 +1884,12 @@ void __init init_speculation_mitigations(void) setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM); } +#ifdef CONFIG_AMD /* Support VIRT_SPEC_CTRL.SSBD if AMD_SSBD is not available. */ if ( opt_msr_sc_hvm && !cpu_has_amd_ssbd && (cpu_has_virt_ssbd || (amd_legacy_ssbd && amd_setup_legacy_ssbd())) ) amd_virt_spec_ctrl = true; +#endif /* Figure out default_xen_spec_ctrl. */ if ( has_spec_ctrl && ibrs )
Similar to making Intel CPU support optional -- as we've got CONFIG_AMD option now, we can put arch/x86/cpu/amd.c under it and make it possible to build Xen without AMD CPU support. One possible use case is to dispose of dead code in Intel-only systems. Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> --- xen/arch/x86/cpu/Makefile | 4 ++-- xen/arch/x86/cpu/common.c | 4 +++- xen/arch/x86/include/asm/amd.h | 22 ++++++++++++++++++++++ xen/arch/x86/spec_ctrl.c | 2 ++ 4 files changed, 29 insertions(+), 3 deletions(-)