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
