Re: [XEN PATCH v1 1/2] x86/intel: optional build of intel.c

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.08.2024 12:09, Sergiy Kibrik wrote:
> With specific config option INTEL in place and most of the code that depends
> on intel.c now can be optionally enabled/disabled it's now possible to put
> the whole intel.c under INTEL option as well. This will allow for a Xen build
> without Intel CPU support.
> 
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>
> ---
>  xen/arch/x86/cpu/Makefile            | 6 +++---
>  xen/arch/x86/cpu/common.c            | 4 +++-
>  xen/arch/x86/include/asm/processor.h | 7 ++++---
>  3 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/x86/cpu/Makefile b/xen/arch/x86/cpu/Makefile
> index eafce5f204..020c86bda3 100644
> --- a/xen/arch/x86/cpu/Makefile
> +++ b/xen/arch/x86/cpu/Makefile
> @@ -6,10 +6,10 @@ obj-y += amd.o
>  obj-y += centaur.o
>  obj-y += common.o
>  obj-y += hygon.o
> -obj-y += intel.o
> -obj-y += intel_cacheinfo.o
> +obj-$(CONFIG_INTEL) += intel.o
> +obj-$(CONFIG_INTEL) += intel_cacheinfo.o
>  obj-y += mwait-idle.o
> -obj-y += shanghai.o
> +obj-$(CONFIG_INTEL) += shanghai.o
>  obj-y += vpmu.o
>  obj-$(CONFIG_AMD) += vpmu_amd.o
>  obj-$(CONFIG_INTEL) += vpmu_intel.o
> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
> index ff4cd22897..50ce13f81c 100644
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -336,11 +336,13 @@ void __init early_cpu_init(bool verbose)
>  
>       c->x86_vendor = x86_cpuid_lookup_vendor(ebx, ecx, edx);
>       switch (c->x86_vendor) {
> +#ifdef CONFIG_INTEL
>       case X86_VENDOR_INTEL:    intel_unlock_cpuid_leaves(c);
>                                 actual_cpu = intel_cpu_dev;    break;
> +     case X86_VENDOR_SHANGHAI: actual_cpu = shanghai_cpu_dev; break;
> +#endif
>       case X86_VENDOR_AMD:      actual_cpu = amd_cpu_dev;      break;
>       case X86_VENDOR_CENTAUR:  actual_cpu = centaur_cpu_dev;  break;
> -     case X86_VENDOR_SHANGHAI: actual_cpu = shanghai_cpu_dev; break;
>       case X86_VENDOR_HYGON:    actual_cpu = hygon_cpu_dev;    break;
>       default:
>               actual_cpu = default_cpu;
> diff --git a/xen/arch/x86/include/asm/processor.h 
> b/xen/arch/x86/include/asm/processor.h
> index 66463f6a6d..a88d45252b 100644
> --- a/xen/arch/x86/include/asm/processor.h
> +++ b/xen/arch/x86/include/asm/processor.h
> @@ -507,15 +507,16 @@ static inline uint8_t get_cpu_family(uint32_t raw, 
> uint8_t *model,
>  extern int8_t opt_tsx;
>  extern bool rtm_disabled;
>  void tsx_init(void);
> +void update_mcu_opt_ctrl(void);
> +void set_in_mcu_opt_ctrl(uint32_t mask, uint32_t val);
>  #else
>  #define opt_tsx      0     /* explicitly indicate TSX is off */
>  #define rtm_disabled false /* RTM was not force-disabled */
>  static inline void tsx_init(void) {}
> +static inline void update_mcu_opt_ctrl(void) {}
> +static inline void set_in_mcu_opt_ctrl(uint32_t mask, uint32_t val) {}
>  #endif
>  
> -void update_mcu_opt_ctrl(void);
> -void set_in_mcu_opt_ctrl(uint32_t mask, uint32_t val);

I'm okay-ish with the simple stubbing out for update_mcu_opt_ctrl(), but
set_in_mcu_opt_ctrl() imo requires more work. The call sites in spec_ctrl.c
shouldn't give the wrong impression of having some effect. Imo in
init_speculation_mitigations() an #endif wants to move down, while all of
gds_calculations() wants to be compiled out. And that likely extends to
further Intel-only functions there (with an early bail-out keyed to
boot_cpu_data.x86_vendor != X86_VENDOR_INTEL). Which overall likely means
there wants to be another separate patch dealing with that. (And then
maybe a counterpart one for AMD.)

Jan