[PATCH] x86/kexec: Use noreturn attributes, and drop unreachable code

[PATCH] x86/kexec: Use noreturn attributes, and drop unreachable code

[Andrew Cooper]

kexec_reloc() does not return.  Plumbing this property upwards lets us mark
machine_kexec() and machine_reboot_kexec() noreturn too.  This in turn lets us
drop some unreachable BUG()/return statements.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
 xen/arch/x86/include/asm/machine_kexec.h | 7 ++++---
 xen/arch/x86/machine_kexec.c             | 5 ++---
 xen/common/kexec.c                       | 5 -----
 xen/include/xen/kexec.h                  | 4 ++--
 4 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/include/asm/machine_kexec.h b/xen/arch/x86/include/asm/machine_kexec.h
index d4880818c1d9..81fe95e091d1 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 __ASSEMBLY__
+#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 d83aa4e7e93b..d9b839f46401 100644
--- a/xen/arch/x86/machine_kexec.c
+++ b/xen/arch/x86/machine_kexec.c
@@ -138,15 +138,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 a2ffb6530cff..0c85f6171a98 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -391,8 +391,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 cf_check kexec_reboot(void *_image)
@@ -403,9 +401,6 @@ static long cf_check kexec_reboot(void *_image)
 
     kexec_common_shutdown();
     machine_reboot_kexec(image);
-
-    BUG();
-    return 0;
 }
 
 static void cf_check do_crashdump_trigger(unsigned char key)
diff --git a/xen/include/xen/kexec.h b/xen/include/xen/kexec.h
index e66eb6a8e593..ff3531586a21 100644
--- a/xen/include/xen/kexec.h
+++ b/xen/include/xen/kexec.h
@@ -48,8 +48,8 @@ int machine_kexec_add_page(struct kexec_image *image, unsigned long vaddr,
 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 machine_reboot_kexec(struct kexec_image *image);
-void machine_kexec(struct kexec_image *image);
+void noreturn machine_reboot_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.11.0

Re: [PATCH] x86/kexec: Use noreturn attributes, and drop unreachable code

[Jan Beulich]

On 07.03.2022 22:02, Andrew Cooper wrote:
> kexec_reloc() does not return.  Plumbing this property upwards lets us mark
> machine_kexec() and machine_reboot_kexec() noreturn too.  This in turn lets us
> drop some unreachable BUG()/return statements.

I'm certainly fine with the added attributes. I'm less convinced of the
removal of BUG() - I'd prefer to leave to the compiler elimination of
these when found to be dead, just to be on the safe side. And I'm pretty
uncertain about the removal of "return", when it comes to old enough
compilers.

> @@ -403,9 +401,6 @@ static long cf_check kexec_reboot(void *_image)
>  
>      kexec_common_shutdown();
>      machine_reboot_kexec(image);
> -
> -    BUG();
> -    return 0;
>  }

In principle this function now is also "noreturn", but I think I
vaguely recall some compilers warning about "noreturn" when a function
returns other than void.

Jan