Re: [XEN PATCH v3 5/6] x86/MCE: add default switch case in init_nonfatal_mce_checker()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.05.2024 11:44, Jan Beulich wrote:
> On 14.05.2024 10:26, Sergiy Kibrik wrote:
>> The default switch case block is likely wanted here, to handle situation
>> e.g. of unexpected c->x86_vendor value -- then no mcheck init is done, but
>> misleading message still gets logged anyway.
> 
> With "likely" dropped I'm okay with this as far as the addition of the default
> label goes. However, ...
> 
>> --- a/xen/arch/x86/cpu/mcheck/non-fatal.c
>> +++ b/xen/arch/x86/cpu/mcheck/non-fatal.c
>> @@ -29,9 +29,13 @@ static int __init cf_check init_nonfatal_mce_checker(void)
>>              /* Assume we are on K8 or newer AMD or Hygon CPU here */
>>              amd_nonfatal_mcheck_init(c);
>>              break;
>> +
>>      case X86_VENDOR_INTEL:
>>              intel_nonfatal_mcheck_init(c);
>>              break;
>> +
>> +    default:
>> +            return -ENODEV;
>>      }
>>      printk(KERN_INFO "mcheck_poll: Machine check polling timer started.\n");
>>      return 0;
> 
> ... converting this to an error return still isn't justified. On one hand it's
> benign because we still don't check return values from initcalls. Yet otoh it
> isn't really an error, is it?

I realize earlier in the function there are error returns, too. I question at
least the former of them as well. And the latter shouldn't be an error at least
when the vendor isn't going to be handled in the switch().

Jan