Re: [PATCH] x86/kexec: Annotate more noreturns

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.10.2026 21:01, Andrew Cooper wrote:
> On 06/10/2026 12:26 pm, Jan Beulich wrote:
>> On 06.10.2026 12:54, Andrew Cooper wrote:
>>> kexec_reloc() does not return.  Plumbing this property upwards lets us mark
>>> machine_kexec() and machine_reboot_kexec() noreturn too.
>> The latter already is annotated such (see also next remark) as of 
>> 4d2ed2f88a0c.
> 
> You missed half of it.  I'm surprised that it didn't create a MISRA
> violation.

Please clarify what it is that I missed.

>>> --- a/xen/arch/x86/machine_kexec.c
>>> +++ b/xen/arch/x86/machine_kexec.c
>>> @@ -142,15 +142,14 @@ void machine_kexec_unload(struct kexec_image *image)
>>>      /* no-op. kimage_free() frees all control pages. */
>>>  }
>>>  
>>> -void machine_reboot_kexec(struct kexec_image *image)
>>> +void noreturn machine_reboot_kexec(struct kexec_image *image)
>>>  {
>>>      BUG_ON(smp_processor_id() != 0);
>>>      smp_send_stop();
>>>      machine_kexec(image);
>>> -    BUG();
>>>  }
>>>  
>>> -void machine_kexec(struct kexec_image *image)
>>> +void noreturn machine_kexec(struct kexec_image *image)
>>>  {
>>>      int i;
>>>      unsigned long reloc_flags = 0;
>> The annotations here aren't needed, are they? I.e. this hunk can shrink
>> to just the removal of the BUG() invocation.
> 
> Readers of this code are not psychic.  They shouldn't need to be in
> order to follow what's going on.

"Psychic"? What are you trying to tell me? There's nothing wrong with
repeating this kind of annotation on the definition, but there also is
no need for doing so. Hence as far as I can judge, the change I made
was self-consistent. If you feel like repeating the annotation is
wanted, then that's different from you (half implicitly, half
explicitly) claiming it's _needed_.

>>> --- a/xen/include/xen/kexec.h
>>> +++ b/xen/include/xen/kexec.h
>>> @@ -49,7 +49,7 @@ int machine_kexec_load(struct kexec_image *image);
>>>  void machine_kexec_unload(struct kexec_image *image);
>>>  void machine_kexec_reserved(xen_kexec_reserve_t *reservation);
>>>  void noreturn machine_reboot_kexec(struct kexec_image *image);
>>> -void machine_kexec(struct kexec_image *image);
>>> +void noreturn machine_kexec(struct kexec_image *image);
>> The annotations on the declarations are sufficient, with another Misra
>> rule making sure we don't omit the inclusion of the header in the CU
>> having the definitions.
>>
>> Preferably with the redundant annotations omitted, and with the description
>> slightly adjusted:
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> I am not inclined to make changes.

Well, so be it then. Yet it again indicates that in certain situations
you're simply not willing to accept that there may be (valid)
viewpoints other than yours. Or, if you think those aren't valid, make
clear what's objectively wrong.

Jan