On 27.04.2024 01:16, Stefano Stabellini wrote:
> On Tue, 23 Apr 2024, Sergiy Kibrik wrote:
>> --- 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!
Almost. I'd appreciate if the ordering of files would be retained. It's
not quite alphabetic, but still. Moving mce_amd.o and mcaction.o to their
designated slots may or may not be done right here.
>> --- 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;
This, while perhaps desirable, doesn't fit ...
>> }
>> printk(KERN_INFO "mcheck_poll: Machine check polling timer started.\n");
>> return 0;
... earlier behavior, and hence is somewhat unexpected in a change which, by
its description, looks like a "no functional change" one.
> For consistency in all other cases this patch series uses IS_ENABLED
> checks. They could be used here as well.
Hmm, I think for switch() statements like this (see also comments elsewhere
on this series) using #ifdef is overall better.
Jan
