Re: [XEN PATCH v1 7/7] x86/MCE: optional build of AMD/Intel MCE code

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 23 Apr 2024, Sergiy Kibrik wrote:
> Separate Intel/AMD-specific MCE code using CONFIG_{INTEL,AMD} config options.
> Now we can avoid build of mcheck code if support for specific platform is
> intentionally disabled by configuration.
> 
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>
> ---
>  xen/arch/x86/cpu/mcheck/Makefile    | 6 ++----
>  xen/arch/x86/cpu/mcheck/non-fatal.c | 6 ++++++
>  xen/arch/x86/cpu/mcheck/vmce.h      | 1 +
>  3 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/x86/cpu/mcheck/Makefile 
> b/xen/arch/x86/cpu/mcheck/Makefile
> index f927f10b4d..5b3f6d875c 100644
> --- a/xen/arch/x86/cpu/mcheck/Makefile
> +++ b/xen/arch/x86/cpu/mcheck/Makefile
> @@ -1,12 +1,10 @@
> -obj-y += amd_nonfatal.o
> -obj-y += mce_amd.o
>  obj-y += mcaction.o
>  obj-y += barrier.o
> -obj-y += intel-nonfatal.o
>  obj-y += mctelem.o
>  obj-y += mce.o
>  obj-y += mce-apei.o
> -obj-y += mce_intel.o
> +obj-$(CONFIG_AMD) += mce_amd.o amd_nonfatal.o
> +obj-$(CONFIG_INTEL) += mce_intel.o intel-nonfatal.o
>  obj-y += non-fatal.o
>  obj-y += util.o
>  obj-y += vmce.o

Awesome!

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> diff --git a/xen/arch/x86/cpu/mcheck/non-fatal.c 
> b/xen/arch/x86/cpu/mcheck/non-fatal.c
> index 33cacd15c2..2d91a3b1e0 100644
> --- a/xen/arch/x86/cpu/mcheck/non-fatal.c
> +++ b/xen/arch/x86/cpu/mcheck/non-fatal.c
> @@ -24,14 +24,20 @@ static int __init cf_check init_nonfatal_mce_checker(void)
>        * Check for non-fatal errors every MCE_RATE s
>        */
>       switch (c->x86_vendor) {
> +#ifdef CONFIG_AMD
>       case X86_VENDOR_AMD:
>       case X86_VENDOR_HYGON:
>               /* Assume we are on K8 or newer AMD or Hygon CPU here */
>               amd_nonfatal_mcheck_init(c);
>               break;
> +#endif
> +#ifdef CONFIG_INTEL
>       case X86_VENDOR_INTEL:
>               intel_nonfatal_mcheck_init(c);
>               break;
> +#endif
> +     default:
> +             return -ENODEV;
>       }
>       printk(KERN_INFO "mcheck_poll: Machine check polling timer started.\n");
>       return 0;

For consistency in all other cases this patch series uses IS_ENABLED
checks. They could be used here as well.