Re: [PATCH v2 3/3] xen/x86: add missing noreturn attributes

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.06.2025 18:16, Nicola Vetrini wrote:
> On 2025-06-18 17:18, Roger Pau Monné wrote:
>> On Fri, Jun 06, 2025 at 02:27:09PM -0700, victorm.lira@xxxxxxx wrote:
>>> --- a/xen/arch/x86/traps.c
>>> +++ b/xen/arch/x86/traps.c
>>> @@ -805,7 +805,7 @@ void fatal_trap(const struct cpu_user_regs *regs, 
>>> bool show_remote)
>>>            (regs->eflags & X86_EFLAGS_IF) ? "" : " IN INTERRUPT 
>>> CONTEXT");
>>>  }
>>>
>>> -void asmlinkage do_unhandled_trap(struct cpu_user_regs *regs)
>>> +void asmlinkage noreturn do_unhandled_trap(struct cpu_user_regs 
>>> *regs)
>>>  {
>>>      fatal_trap(regs, false);
>>>  }
>>> diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
>>> index c77f304bb0..8460a4a1ae 100644
>>> --- a/xen/arch/x86/x86_64/traps.c
>>> +++ b/xen/arch/x86/x86_64/traps.c
>>> @@ -293,7 +293,7 @@ void show_page_walk(unsigned long addr)
>>>             l1_table_offset(addr), l1e_get_intpte(l1e), pfn);
>>>  }
>>>
>>> -void asmlinkage do_double_fault(struct cpu_user_regs *regs)
>>> +void asmlinkage noreturn do_double_fault(struct cpu_user_regs *regs)
>>
>> Does noreturn matter for functions called from assembly (asmlinkage
>> ones)?  In that case the hint is not useful for code generation, since
>> it's hand written assembly already?  (it's arguably useful for the
>> developer writing the code)
>>
>> Might be worth mentioning in the commit message if the above is
>> accurate.  For example by adding to the commit message: "noreturn is
>> not relevant for functions called from assembly, but can be used as a
>> hint for the developers writing the code".
> 
> Yes, it is relevant because the rule considers only the single function, 
> not the context where it is called (that is orders of magnitude more 
> difficult to check automatically). For my part, I'm ok with your 
> suggestion.

Right. In fact for non-static functions the attribute normally would go
on the declaration. The need to have it on the definition in the two
cases above is an aspect that may also want to go into the amended
description.

Jan