Re: [PATCH 04/12] x86: add noreturn in a few more places

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.08.2026 21:13, Andrew Cooper wrote:
> On 28/08/2026 8:01 am, Jan Beulich wrote:
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -2304,7 +2304,7 @@ void asmlinkage entry_from_pv(struct cpu
>>      case X86_ET_HW_EXC:
>>          switch ( vec )
>>          {
>> -        case X86_EXC_DF: return do_double_fault(regs);
>> +        case X86_EXC_DF: do_double_fault(regs); /* noreturn */
>>          case X86_EXC_MC: return do_machine_check(regs);
>>          }
>>          break;
>> @@ -2615,7 +2615,7 @@ void asmlinkage entry_from_xen(struct cp
>>      case X86_ET_HW_EXC:
>>          switch ( regs->fred_ss.vector )
>>          {
>> -        case X86_EXC_DF: return do_double_fault(regs);
>> +        case X86_EXC_DF: do_double_fault(regs); /* noreturn */
>>          case X86_EXC_MC: return do_machine_check(regs);
>>          }
>>          break;
>>
> 
> For starters you're missing a break, and the only reason this isn't a
> compile error is the trailing comment.

"break" there would again be unreachable, though.

>  Second, it's a tailcall anyway. 
> There really is nothing unreachable anywhere in this construct.

Just that the concept of "tailcall" is an optimization, not something
inherent to the language.

> But by far the most important, it the singular noreturn attribute on
> do_double_fault() (elsewhere, and not visible when reading these two
> functions) which is preventing #DF falling into #MC.   This introduces
> fragility which did not exist previously.

I realized that when making the patch, yet what do you do when the rule
is as it is? Hence why I added the comment, really.

> do_double_fault() would conditionally return if we ever got around to
> fixing espfix64.

And hence would have to lose its "noreturn". At which point call sites
would need inspecting. (As said - yes, I do realize the fragility.)

> So no - I'm going to insist that Eclair is taught to accept "return
> some_noreturn_fn();" as intentional.  It is objectively less fragile
> than the MISRA-preferred option.

Nicola, thoughts?

Jan