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(-)

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 )
-- 
2.25.1

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