intended behaviour of noreboot

RFC PATCH In review Muli Ben-Yehuda 4 participants 6 messages Aug 6, 2006 – Aug 7, 2006

Share & tools

Muli Ben-Yehuda

author

2006-08-06 06:50 UTC #0 lore raw

Hi,

Setting 'noreboot' on the Xen command line stops reboot(1) from working in dom0. Is this the intended behaviour? I would expect noreboot to only apply to reboots that are trigerred as a result of kernel panis or oopses.

Cheers,
Muli

Keir Fraser

2006-08-07 07:17 UTC #1 lore raw

Quoted 4 lines · Muli Ben-Yehuda

Setting 'noreboot' on the Xen command line stops reboot(1) from working in dom0. Is this the intended behaviour? I would expect noreboot to only apply to reboots that are trigerred as a result of kernel panis or oopses.

Clean shutdown of domain0 could clear the noreboot flag in Xen.

-- Keir

Muli Ben-Yehuda

author

2006-08-07 07:34 UTC #2 lore raw

Quoted 8 lines · Keir Fraser

Quoted 4 lines · Muli Ben-Yehuda
Setting 'noreboot' on the Xen command line stops reboot(1) from working in dom0. Is this the intended behaviour? I would expect noreboot to only apply to reboots that are trigerred as a result of kernel panis or oopses.
Clean shutdown of domain0 could clear the noreboot flag in Xen.

Ok, good. I'll whip up a patch later today.

Cheers,
Muli

Muli Ben-Yehuda

author

2006-08-07 12:13 UTC #3 lore raw

RFC PATCH 1/1 fix noreboot behaviour on clean reboot (was: intended behaviour of noreboot)

In reply to Keir Fraser

Quoted 8 lines · Keir Fraser

Quoted 4 lines · Muli Ben-Yehuda
Setting 'noreboot' on the Xen command line stops reboot(1) from working in dom0. Is this the intended behaviour? I would expect noreboot to only apply to reboots that are trigerred as a result of kernel panis or oopses.
Clean shutdown of domain0 could clear the noreboot flag in Xen.

Itt's a bit too hacky that machine_restart() either restarts or halts, depends on opt_noreboot. How about something like this:

- move code specific to dom0 out of domain_shutdown() into dom0_shutdown()
- dom0_shutdown() has the following semantics:
if reason is SHUTDOWN_poweroff: halt
if reason is SHUTDOWN_crash: halt if opt_noreboot is set, reboot otherwise.
if reason is SHUTDOWN_reboot: reboot (regardless of noreboot) if reason is anything else: halt if opt_noreboot is set, reboot otherwise (preserve backward compatible behaviour).
- I checked and PV Linux i386/x86-64 uses SHUTDOWN_crash/SHUTDOWN_reboot appropriately to give the above semantics.
- since opt_noreboot is now referenced in common code, add definitions for it for ia64 and ppc as well. Hooking it in in arch code is simple - just parse the command line options and set opt_noreboot appropriately.

Tested on x86-64, comments appreciated!

Cheers,
Muli

Signed-off-by Muli Ben-Yehuda <[email protected]>

xen/arch/ia64/xen/domain.c +2 -0

diff -r ffa5b2975dff xen/arch/ia64/xen/domain.c
@@ -77,6 +77,8 @@ DEFINE_PER_CPU(uint8_t *, current_psr_i_
 DEFINE_PER_CPU(uint8_t *, current_psr_i_addr);
 DEFINE_PER_CPU(int *, current_psr_ic_addr);
 
+int opt_noreboot = 0;
+
 #include <xen/sched-if.h>
 
 static void flush_vtlb_for_context_switch(struct vcpu* vcpu)

xen/arch/powerpc/domain.c +2 -0

diff -r ffa5b2975dff xen/arch/powerpc/domain.c
@@ -31,6 +31,8 @@
 #include <asm/current.h>
 #include <asm/hcalls.h>
 
+int opt_noreboot = 0;
+
 extern void idle_loop(void);
 
 #define next_arg(fmt, args) ({                                              \

xen/arch/x86/shutdown.c +1 -7

diff -r ffa5b2975dff xen/arch/x86/shutdown.c
@@ -22,7 +22,7 @@
 #include <asm/msr.h>
 
 /* opt_noreboot: If true, machine will need manual reset on error. */
-static int opt_noreboot = 0;
+int opt_noreboot = 0;
 boolean_param("noreboot", opt_noreboot);
 
 /* reboot_str: comma-separated list of reboot options. */

@@ -204,12 +204,6 @@ void machine_restart(char * __unused)
 {
     int i;
 
-    if ( opt_noreboot )
-    {
-        printk("Reboot disabled on cmdline: require manual reset\n");
-        machine_halt();
-    }
-
     watchdog_disable();
     console_start_sync();

xen/common/domain.c +44 ~~-20~~

85 lines not on this page · raw message

xen/drivers/char/console.c +5 -1

diff -r ffa5b2975dff xen/drivers/char/console.c
@@ -21,6 +21,7 @@
 #include <xen/mm.h>
 #include <xen/delay.h>
 #include <xen/guest_access.h>
+#include <xen/reboot.h>
 #include <asm/current.h>
 #include <asm/debugger.h>
 #include <asm/io.h>

@@ -737,7 +738,10 @@ void panic(const char *fmt, ...)
 
     watchdog_disable();
     mdelay(5000);
-    machine_restart(0);
+    if ( opt_noreboot )
+        machine_halt();
+    else
+        machine_restart(NULL);
 }
 
 void __bug(char *file, int line)

xen/include/xen/reboot.h +4 -0

diff -r ffa5b2975dff xen/include/xen/reboot.h
@@ -1,5 +1,9 @@
 #ifndef _LINUX_REBOOT_H
 #define _LINUX_REBOOT_H
+
+/* opt_noreboot: If true, machine will need manual reset on error. */
+/* only implemented on i386/x86-64 for the moment */
+extern int opt_noreboot;
 
 extern void machine_restart(char *cmd);
 extern void machine_halt(void);

Jan Beulich

2006-08-07 12:42 UTC #4 lore raw

Re: [PATCH RFC] fix noreboot behaviour on clean reboot (was: intended behaviour of noreb

Quoted 31 lines

Quoted 1 line
Quoted 1 line
Muli Ben-Yehuda <[email protected]> 07.08.06 14:13 >>>
Quoted 8 lines · Keir Fraser
Quoted 4 lines · Muli Ben-Yehuda
Setting 'noreboot' on the Xen command line stops reboot(1) from working in dom0. Is this the intended behaviour? I would expect noreboot to only apply to reboots that are trigerred as a result of kernel panis or oopses.
Clean shutdown of domain0 could clear the noreboot flag in Xen.
Itt's a bit too hacky that machine_restart() either restarts or halts, depends on opt_noreboot. How about something like this:
- move code specific to dom0 out of domain_shutdown() into dom0_shutdown()
- dom0_shutdown() has the following semantics:
if reason is SHUTDOWN_poweroff: halt
if reason is SHUTDOWN_crash: halt if opt_noreboot is set, reboot otherwise.
if reason is SHUTDOWN_reboot: reboot (regardless of noreboot) if reason is anything else: halt if opt_noreboot is set, reboot otherwise (preserve backward compatible behaviour).
- I checked and PV Linux i386/x86-64 uses SHUTDOWN_crash/SHUTDOWN_reboot appropriately to give the above semantics.
- since opt_noreboot is now referenced in common code, add definitions for it for ia64 and ppc as well. Hooking it in in arch code is simple - just parse the command line options and set opt_noreboot appropriately.

Quoted 1 line

Tested on x86-64, comments appreciated!

Yes, please!

Jan

Akio Takebe

2006-08-07 21:30 UTC #5 lore raw

Re: [Xen-devel] [PATCH RFC] fix noreboot behaviour on clean reboot(was: intended behaviour of noreboot)

In reply to Muli Ben-Yehuda

Hi,

I tested Muil's patch on ia64.

The below is test results.
- Before patch

    reboot  ----> reboot
    halt    ----> reboot
    panic   ----> reboot

- After patch

    reboot  ----> reboot
    halt    ----> reboot
    panic   ----> reboot

This results is caused by calling EFI_RESET_SHUTDOWN in ia64's machine_halt(). Ia64's machine_halt require system reboot because EFI_RESET_SHUTDOWN (used by ia64's machine_halt()) enter ACPI G2/S5
or G3 state.

This is ia64 issue, I think Muli's patch is good. :-)

Best Regards,

Akio Takebe

Quoted 35 lines · Keir Fraser

Quoted 8 lines · Muli Ben-Yehuda
Quoted 5 lines
Setting 'noreboot' on the Xen command line stops reboot(1) from working in dom0. Is this the intended behaviour? I would expect noreboot to only apply to reboots that are trigerred as a result of kernel panis or oopses.
Clean shutdown of domain0 could clear the noreboot flag in Xen.
Itt's a bit too hacky that machine_restart() either restarts or halts, depends on opt_noreboot. How about something like this:
- move code specific to dom0 out of domain_shutdown() into dom0_shutdown()
- dom0_shutdown() has the following semantics:
if reason is SHUTDOWN_poweroff: halt
if reason is SHUTDOWN_crash: halt if opt_noreboot is set, reboot otherwise.
if reason is SHUTDOWN_reboot: reboot (regardless of noreboot) if reason is anything else: halt if opt_noreboot is set, reboot otherwise (preserve backward compatible behaviour).
- I checked and PV Linux i386/x86-64 uses SHUTDOWN_crash/SHUTDOWN_reboot appropriately to give the above semantics.
- since opt_noreboot is now referenced in common code, add definitions for it for ia64 and ppc as well. Hooking it in in arch code is simple - just parse the command line options and set opt_noreboot appropriately.
Tested on x86-64, comments appreciated!
Cheers,
Muli