| Message ID | 3c641433fa7cfe1f7fdc918ab32086835a2e13eb.1723806405.git.Sergiy_Kibrik@epam.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/CPU: optional build of Intel/AMD CPUs support | expand |
On 16.08.2024 13:19, Sergiy Kibrik wrote: > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -919,7 +919,8 @@ static void cf_check svm_ctxt_switch_from(struct vcpu *v) > * Possibly clear previous guest selection of SSBD if set. Note that > * SPEC_CTRL.SSBD is already handled by svm_vmexit_spec_ctrl. > */ > - if ( v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD ) > + if ( IS_ENABLED(CONFIG_AMD) && > + v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD ) > { > ASSERT(v->domain->arch.cpuid->extd.virt_ssbd); > amd_set_legacy_ssbd(false); > @@ -953,7 +954,8 @@ static void cf_check svm_ctxt_switch_to(struct vcpu *v) > wrmsr_tsc_aux(v->arch.msrs->tsc_aux); > > /* Load SSBD if set by the guest. */ > - if ( v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD ) > + if ( IS_ENABLED(CONFIG_AMD) && > + v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD ) > { > ASSERT(v->domain->arch.cpuid->extd.virt_ssbd); > amd_set_legacy_ssbd(true); Instead of these changes, shouldn't AMD_SVM become dependent upon AMD in Kconfig? > +#ifdef CONFIG_AMD > +extern bool amd_acpi_c1e_quirk; > +extern bool amd_virt_spec_ctrl; > +#else > + > +#define amd_acpi_c1e_quirk (false) > +#define amd_virt_spec_ctrl (false) As a remark, while there's nothing wrong with parenthesizing "false" here, it also isn't really necessary. Omitting unnecessary parentheses generally aids readability imo. Jan
19.08.24 15:36, Jan Beulich: > On 16.08.2024 13:19, Sergiy Kibrik wrote: >> --- a/xen/arch/x86/hvm/svm/svm.c >> +++ b/xen/arch/x86/hvm/svm/svm.c >> @@ -919,7 +919,8 @@ static void cf_check svm_ctxt_switch_from(struct vcpu *v) >> * Possibly clear previous guest selection of SSBD if set. Note that >> * SPEC_CTRL.SSBD is already handled by svm_vmexit_spec_ctrl. >> */ >> - if ( v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD ) >> + if ( IS_ENABLED(CONFIG_AMD) && >> + v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD ) >> { >> ASSERT(v->domain->arch.cpuid->extd.virt_ssbd); >> amd_set_legacy_ssbd(false); >> @@ -953,7 +954,8 @@ static void cf_check svm_ctxt_switch_to(struct vcpu *v) >> wrmsr_tsc_aux(v->arch.msrs->tsc_aux); >> >> /* Load SSBD if set by the guest. */ >> - if ( v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD ) >> + if ( IS_ENABLED(CONFIG_AMD) && >> + v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD ) >> { >> ASSERT(v->domain->arch.cpuid->extd.virt_ssbd); >> amd_set_legacy_ssbd(true); > Instead of these changes, shouldn't AMD_SVM become dependent upon AMD in > Kconfig? It could be done earlier, yet I haven't done so since we briefly touched this before and decided not to link {AMD,INTEL} with {AMD_SVM,INTEL_VMX} then: https://lore.kernel.org/xen-devel/9a973f18-e0af-456c-8b07-6869f044519e@citrix.com/ -Sergiy
On 29.08.2024 11:48, Sergiy Kibrik wrote: > 19.08.24 15:36, Jan Beulich: >> On 16.08.2024 13:19, Sergiy Kibrik wrote: >>> --- a/xen/arch/x86/hvm/svm/svm.c >>> +++ b/xen/arch/x86/hvm/svm/svm.c >>> @@ -919,7 +919,8 @@ static void cf_check svm_ctxt_switch_from(struct vcpu *v) >>> * Possibly clear previous guest selection of SSBD if set. Note that >>> * SPEC_CTRL.SSBD is already handled by svm_vmexit_spec_ctrl. >>> */ >>> - if ( v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD ) >>> + if ( IS_ENABLED(CONFIG_AMD) && >>> + v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD ) >>> { >>> ASSERT(v->domain->arch.cpuid->extd.virt_ssbd); >>> amd_set_legacy_ssbd(false); >>> @@ -953,7 +954,8 @@ static void cf_check svm_ctxt_switch_to(struct vcpu *v) >>> wrmsr_tsc_aux(v->arch.msrs->tsc_aux); >>> >>> /* Load SSBD if set by the guest. */ >>> - if ( v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD ) >>> + if ( IS_ENABLED(CONFIG_AMD) && >>> + v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD ) >>> { >>> ASSERT(v->domain->arch.cpuid->extd.virt_ssbd); >>> amd_set_legacy_ssbd(true); >> Instead of these changes, shouldn't AMD_SVM become dependent upon AMD in >> Kconfig? > > It could be done earlier, yet I haven't done so since we briefly touched > this before and decided not to link {AMD,INTEL} with {AMD_SVM,INTEL_VMX} > then: > > https://lore.kernel.org/xen-devel/9a973f18-e0af-456c-8b07-6869f044519e@citrix.com/ Yet that only suggests that e.g HYGON also ought to select AMD_SVM. Which will happen transitively with HYGON selecting AMD (in the earlier patch). Jan
diff --git a/xen/arch/x86/cpu/Makefile b/xen/arch/x86/cpu/Makefile index eeb9ebe562..2c34597136 100644 --- a/xen/arch/x86/cpu/Makefile +++ b/xen/arch/x86/cpu/Makefile @@ -2,7 +2,7 @@ obj-y += mcheck/ obj-y += microcode/ obj-y += mtrr/ -obj-y += amd.o +obj-$(CONFIG_AMD) += amd.o obj-$(CONFIG_CENTAUR) += centaur.o obj-y += common.o obj-$(CONFIG_HYGON) += hygon.o diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c index 580b01d6d5..5930b712bf 100644 --- a/xen/arch/x86/cpu/common.c +++ b/xen/arch/x86/cpu/common.c @@ -194,7 +194,7 @@ void ctxt_switch_levelling(const struct vcpu *next) if (cpu_has_cpuid_faulting) set_cpuid_faulting(enable_cpuid_faulting); - else + else if ( IS_ENABLED(CONFIG_AMD) ) amd_set_cpuid_user_dis(enable_cpuid_faulting); return; @@ -340,7 +340,9 @@ void __init early_cpu_init(bool verbose) case X86_VENDOR_INTEL: intel_unlock_cpuid_leaves(c); actual_cpu = intel_cpu_dev; break; #endif +#ifdef CONFIG_AMD case X86_VENDOR_AMD: actual_cpu = amd_cpu_dev; break; +#endif #ifdef CONFIG_CENTAUR case X86_VENDOR_CENTAUR: actual_cpu = centaur_cpu_dev; break; #endif diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index 92bb10c504..88902e2d3a 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -919,7 +919,8 @@ static void cf_check svm_ctxt_switch_from(struct vcpu *v) * Possibly clear previous guest selection of SSBD if set. Note that * SPEC_CTRL.SSBD is already handled by svm_vmexit_spec_ctrl. */ - if ( v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD ) + if ( IS_ENABLED(CONFIG_AMD) && + v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD ) { ASSERT(v->domain->arch.cpuid->extd.virt_ssbd); amd_set_legacy_ssbd(false); @@ -953,7 +954,8 @@ static void cf_check svm_ctxt_switch_to(struct vcpu *v) wrmsr_tsc_aux(v->arch.msrs->tsc_aux); /* Load SSBD if set by the guest. */ - if ( v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD ) + if ( IS_ENABLED(CONFIG_AMD) && + v->arch.msrs->virt_spec_ctrl.raw & SPEC_CTRL_SSBD ) { ASSERT(v->domain->arch.cpuid->extd.virt_ssbd); amd_set_legacy_ssbd(true); diff --git a/xen/arch/x86/include/asm/amd.h b/xen/arch/x86/include/asm/amd.h index fa4e0fc766..da35b82d5a 100644 --- a/xen/arch/x86/include/asm/amd.h +++ b/xen/arch/x86/include/asm/amd.h @@ -158,20 +158,36 @@ #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 0; +} +#endif extern s8 opt_allow_unsafe; void fam10h_check_enable_mmcfg(void); void check_enable_amd_mmconf_dmi(void); -extern bool amd_acpi_c1e_quirk; void amd_check_disable_c1e(unsigned int port, u8 value); extern bool amd_legacy_ssbd; -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); +#ifdef CONFIG_AMD +extern bool amd_acpi_c1e_quirk; +extern bool amd_virt_spec_ctrl; +#else + +#define amd_acpi_c1e_quirk (false) +#define amd_virt_spec_ctrl (false) + +#endif + #endif /* __AMD_H__ */ diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c index ba6c3e80d2..1964a417de 100644 --- a/xen/arch/x86/spec_ctrl.c +++ b/xen/arch/x86/spec_ctrl.c @@ -1893,10 +1893,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> CC: Jan Beulich <jbeulich@suse.com> --- changes in v2: - drop routines-stubs in amd.h, handle call sites instead - cpu_has_amd_erratum() return int instead of bool --- xen/arch/x86/cpu/Makefile | 2 +- xen/arch/x86/cpu/common.c | 4 +++- xen/arch/x86/hvm/svm/svm.c | 6 ++++-- xen/arch/x86/include/asm/amd.h | 20 ++++++++++++++++++-- xen/arch/x86/spec_ctrl.c | 2 ++ 5 files changed, 28 insertions(+), 6 deletions(-)