xen/arch/x86/include/asm/machine_kexec.h | 7 ++++--- xen/arch/x86/machine_kexec.c | 5 ++--- xen/common/kexec.c | 2 -- xen/include/xen/kexec.h | 2 +- 4 files changed, 7 insertions(+), 9 deletions(-)
kexec_reloc() does not return. Plumbing this property upwards lets us mark
machine_kexec() and machine_reboot_kexec() noreturn too.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <jbeulich@suse.com>
CC: Roger Pau Monné <roger@xenproject.org>
CC: Teddy Astie <teddy.astie@vates.tech>
Rebase forward many years.
---
xen/arch/x86/include/asm/machine_kexec.h | 7 ++++---
xen/arch/x86/machine_kexec.c | 5 ++---
xen/common/kexec.c | 2 --
xen/include/xen/kexec.h | 2 +-
4 files changed, 7 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/include/asm/machine_kexec.h b/xen/arch/x86/include/asm/machine_kexec.h
index 3e189acf2464..f0bdc2f44109 100644
--- a/xen/arch/x86/include/asm/machine_kexec.h
+++ b/xen/arch/x86/include/asm/machine_kexec.h
@@ -4,10 +4,11 @@
#define KEXEC_RELOC_FLAG_COMPAT 0x1 /* 32-bit image */
#ifndef __ASSEMBLER__
+#include <xen/compiler.h>
-extern void kexec_reloc(unsigned long reloc_code, unsigned long reloc_pt,
- unsigned long ind_maddr, unsigned long entry_maddr,
- unsigned long flags);
+extern void noreturn kexec_reloc(
+ unsigned long reloc_code, unsigned long reloc_pt, unsigned long ind_maddr,
+ unsigned long entry_maddr, unsigned long flags);
extern const char kexec_reloc_end[];
diff --git a/xen/arch/x86/machine_kexec.c b/xen/arch/x86/machine_kexec.c
index 038670922d47..0fb9ed50e780 100644
--- 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;
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 8ccaef1e0258..da3bb95d0852 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -397,8 +397,6 @@ void kexec_crash(enum crash_reason reason)
kexec_crash_save_cpu();
machine_crash_shutdown();
machine_kexec(kexec_image[KEXEC_IMAGE_CRASH_BASE + pos]);
-
- BUG();
}
static long noreturn cf_check kexec_reboot(void *_image)
diff --git a/xen/include/xen/kexec.h b/xen/include/xen/kexec.h
index e0d793b04316..6bb3fde629a6 100644
--- 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);
void kexec_crash(enum crash_reason reason);
void kexec_crash_save_cpu(void);
struct crash_xen_info *kexec_crash_save_info(void);
--
2.39.5
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.
> --- 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.
> --- 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@suse.com>
Jan
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.
>
>> --- 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.
>
>> --- 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@suse.com>
I am not inclined to make changes.
~Andrew
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@suse.com>
>
> 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
© 2016 - 2026 Red Hat, Inc.