x86/EFI: allow reboot= overrides when running under EFI

By default we will always use EFI reboot mechanism when running under EFI platforms. However some EFI platforms are buggy and need to use the ACPI mechanism to reboot (such as Lenovo ThinkCentre M57). As such respect the 'reboot=' override and DMI overrides for EFI platforms.

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

- BOOT_INVALID is just zero
- also consider acpi_disabled in BOOT_INVALID resolution
- duplicate BOOT_INVALID resolution in machine_restart()
- don't fall back from BOOT_ACPI to BOOT_EFI (if it was overridden, it surely was for a reason)
- adjust doc change formatting

Signed-off-by Jan Beulich <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

---
v2: Update reboot_type prior to calling efi_reset_system(), as
requested by Andrew.

docs/misc/xen-command-line.markdown +4 -1

@@ -1145,7 +1145,7 @@ The following resources are available:
   Technology.
 
 ### reboot
-> `= t[riple] | k[bd] | a[cpi] | p[ci] | n[o] [, [w]arm | [c]old]`
+> `= t[riple] | k[bd] | a[cpi] | p[ci] | e[fi] | n[o] [, [w]arm | [c]old]`
 
 > Default: `0`
 

@@ -1165,6 +1165,9 @@ Specify the host reboot method.
 
 `pci` instructs Xen to reboot the host using PCI reset register (port CF9).
 
+'efi' instructs Xen to reboot using the EFI reboot call (in EFI mode by
+ default it will use that method first).
+
 ### ro-hpet
 > `= <boolean>`
 

xen/arch/x86/shutdown.c +28 -5

@@ -28,16 +28,18 @@
 #include <asm/apic.h>
 
 enum reboot_type {
+        BOOT_INVALID,
         BOOT_TRIPLE = 't',
         BOOT_KBD = 'k',
         BOOT_ACPI = 'a',
         BOOT_CF9 = 'p',
+        BOOT_EFI = 'e',
 };
 
 static int reboot_mode;
 
 /*
- * reboot=t[riple] | k[bd] | a[cpi] | p[ci] | n[o] [, [w]arm | [c]old]
+ * reboot=t[riple] | k[bd] | a[cpi] | p[ci] | n[o] | [e]fi [, [w]arm | [c]old]
  * warm   Don't set the cold reboot flag
  * cold   Set the cold reboot flag
  * no     Suppress automatic reboot after panics or crashes

@@ -45,8 +47,9 @@ static int reboot_mode;
  * kbd    Use the keyboard controller. cold reset (default)
  * acpi   Use the RESET_REG in the FADT
  * pci    Use the so-called "PCI reset register", CF9
+ * efi    Use the EFI reboot (if running under EFI)
  */
-static enum reboot_type reboot_type = BOOT_ACPI;
+static enum reboot_type reboot_type = BOOT_INVALID;
 static void __init set_reboot_type(char *str)
 {
     for ( ; ; )

@@ -63,6 +66,7 @@ static void __init set_reboot_type(char 
             reboot_mode = 0x0;
             break;
         case 'a':
+        case 'e':
         case 'k':
         case 't':
         case 'p':

@@ -106,6 +110,14 @@ void machine_halt(void)
     __machine_halt(NULL);
 }
 
+static void default_reboot_type(void)
+{
+    if ( reboot_type == BOOT_INVALID )
+        reboot_type = efi_enabled ? BOOT_EFI
+                                  : acpi_disabled ? BOOT_KBD
+                                                  : BOOT_ACPI;
+}
+
 static int __init override_reboot(struct dmi_system_id *d)
 {
     enum reboot_type type = (long)d->driver_data;

@@ -452,6 +464,7 @@ static struct dmi_system_id __initdata r
 
 static int __init reboot_init(void)
 {
+    default_reboot_type();
     dmi_check_system(reboot_dmi_table);
     return 0;
 }

@@ -465,7 +478,7 @@ static void noreturn __machine_restart(v
 void machine_restart(unsigned int delay_millisecs)
 {
     unsigned int i, attempt;
-    enum reboot_type orig_reboot_type = reboot_type;
+    enum reboot_type orig_reboot_type;
     const struct desc_ptr no_idt = { 0 };
 
     watchdog_disable();

@@ -504,15 +517,20 @@ void machine_restart(unsigned int delay_
         tboot_shutdown(TB_SHUTDOWN_REBOOT);
     }
 
-    efi_reset_system(reboot_mode != 0);
+    /* Just in case reboot_init() didn't run yet. */
+    default_reboot_type();
+    orig_reboot_type = reboot_type;
 
     /* Rebooting needs to touch the page at absolute address 0. */
-    *((unsigned short *)__va(0x472)) = reboot_mode;
+    if ( reboot_type != BOOT_EFI )
+        *((unsigned short *)__va(0x472)) = reboot_mode;
 
     for ( attempt = 0; ; attempt++ )
     {
         switch ( reboot_type )
         {
+        case BOOT_INVALID:
+            ASSERT_UNREACHABLE();
         case BOOT_KBD:
             /* Pulse the keyboard reset line. */
             for ( i = 0; i < 100; i++ )

@@ -532,6 +550,11 @@ void machine_restart(unsigned int delay_
             reboot_type = (((attempt == 1) && (orig_reboot_type == BOOT_ACPI))
                            ? BOOT_ACPI : BOOT_TRIPLE);
             break;
+        case BOOT_EFI:
+            reboot_type = acpi_disabled ? BOOT_KBD : BOOT_ACPI;
+            efi_reset_system(reboot_mode != 0);
+            *((unsigned short *)__va(0x472)) = reboot_mode;
+            break;
         case BOOT_TRIPLE:
             asm volatile ("lidt %0; int3" : : "m" (no_idt));
             reboot_type = BOOT_KBD;

Quoted 1 line

Quoted 1 line · On 12.03.15 at 17:43, <[email protected]> wrote:

Quoted 16 lines · Jan Beulich

By default we will always use EFI reboot mechanism when running under EFI platforms. However some EFI platforms are buggy and need to use the ACPI mechanism to reboot (such as Lenovo ThinkCentre M57). As such respect the 'reboot=' override and DMI overrides for EFI platforms.

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

- BOOT_INVALID is just zero
- also consider acpi_disabled in BOOT_INVALID resolution
- duplicate BOOT_INVALID resolution in machine_restart()
- don't fall back from BOOT_ACPI to BOOT_EFI (if it was overridden, it surely was for a reason)
- adjust doc change formatting

Quoted 2 lines

Signed-off-by Jan Beulich <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

Reviewed-by Andrew Cooper <[email protected]>

Are you happy merging this with Ross's v2 patch (my review still withstanding), with which this conflicts?

Quoted 18 lines

Quoted 1 line

Quoted 1 line · On 12.03.15 at 17:43, <[email protected]> wrote:

Quoted 16 lines · Jan Beulich

By default we will always use EFI reboot mechanism when running under EFI platforms. However some EFI platforms are buggy and need to use the ACPI mechanism to reboot (such as Lenovo ThinkCentre M57). As such respect the 'reboot=' override and DMI overrides for EFI platforms.

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

- BOOT_INVALID is just zero
- also consider acpi_disabled in BOOT_INVALID resolution
- duplicate BOOT_INVALID resolution in machine_restart()
- don't fall back from BOOT_ACPI to BOOT_EFI (if it was overridden, it surely was for a reason)
- adjust doc change formatting

Quoted 6 lines

Quoted 2 lines

Signed-off-by Jan Beulich <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

Reviewed-by Andrew Cooper <[email protected]>

Are you happy merging this with Ross's v2 patch (my review still withstanding), with which this conflicts?

And which allows this to become even less of a change:

x86: don't apply reboot quirks if reboot set by user

If reboot= is specified on the command-line, don't apply reboot quirks to allow the command-line option to take precedence.

This is a port of Linux commit 5955633e91bf ("x86/reboot: Skip DMI checks if reboot set by user").

Signed-off-by Ross Lagerwall <[email protected]>

Leverage (and make apply on top of) c643fb110a ("x86/EFI: allow reboot= overrides when running under EFI").

Signed-off-by Jan Beulich <[email protected]>

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

@@ -464,6 +464,13 @@ static struct dmi_system_id __initdata r
 
 static int __init reboot_init(void)
 {
+    /*
+     * Only do the DMI check if reboot_type hasn't been overridden
+     * on the command line
+     */
+    if ( reboot_type != BOOT_INVALID )
+        return 0;
+
     default_reboot_type();
     dmi_check_system(reboot_dmi_table);
     return 0;

Quoted 34 lines · Jan Beulich

Quoted 18 lines

Quoted 1 line

Quoted 1 line · On 12.03.15 at 17:43, <[email protected]> wrote:

Quoted 16 lines · Jan Beulich

By default we will always use EFI reboot mechanism when running under EFI platforms. However some EFI platforms are buggy and need to use the ACPI mechanism to reboot (such as Lenovo ThinkCentre M57). As such respect the 'reboot=' override and DMI overrides for EFI platforms.

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

- BOOT_INVALID is just zero
- also consider acpi_disabled in BOOT_INVALID resolution
- duplicate BOOT_INVALID resolution in machine_restart()
- don't fall back from BOOT_ACPI to BOOT_EFI (if it was overridden, it surely was for a reason)
- adjust doc change formatting

Quoted 6 lines

Quoted 2 lines

Signed-off-by Jan Beulich <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

Reviewed-by Andrew Cooper <[email protected]>

Are you happy merging this with Ross's v2 patch (my review still withstanding), with which this conflicts?

And which allows this to become even less of a change:

x86: don't apply reboot quirks if reboot set by user

If reboot= is specified on the command-line, don't apply reboot quirks to allow the command-line option to take precedence.

This is a port of Linux commit 5955633e91bf ("x86/reboot: Skip DMI checks if reboot set by user").

Quoted 6 lines

Signed-off-by Ross Lagerwall <[email protected]>

Leverage (and make apply on top of) c643fb110a ("x86/EFI: allow reboot= overrides when running under EFI").

Signed-off-by Jan Beulich <[email protected]>

Reviewed-by Andrew Cooper <[email protected]>

Quoted 21 lines

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

@@ -464,6 +464,13 @@ static struct dmi_system_id __initdata r
 
 static int __init reboot_init(void)
 {
+    /*
+     * Only do the DMI check if reboot_type hasn't been overridden
+     * on the command line
+     */
+    if ( reboot_type != BOOT_INVALID )
+        return 0;
+
     default_reboot_type();
     dmi_check_system(reboot_dmi_table);
     return 0;

By default we will always use EFI reboot mechanism when running under EFI platforms. However some EFI platforms are buggy and need to use the ACPI mechanism to reboot (such as Lenovo ThinkCentre M57). As such respect the 'reboot=' override and DMI overrides for EFI platforms.

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

- BOOT_INVALID is just zero
- also consider acpi_disabled in BOOT_INVALID resolution
- duplicate BOOT_INVALID resolution in machine_restart()
- don't fall back from BOOT_ACPI to BOOT_EFI (if it was overridden, it surely was for a reason)
- adjust doc change formatting

Signed-off-by Jan Beulich <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

---
v2: Update reboot_type prior to calling efi_reset_system(), as
requested by Andrew.

docs/misc/xen-command-line.markdown +4 -1

@@ -1145,7 +1145,7 @@ The following resources are available:
    Technology.
 
  ### reboot
-> `= t[riple] | k[bd] | a[cpi] | p[ci] | n[o] [, [w]arm | [c]old]`
+> `= t[riple] | k[bd] | a[cpi] | p[ci] | e[fi] | n[o] [, [w]arm | [c]old]`
 
  > Default: `0`
 

@@ -1165,6 +1165,9 @@ Specify the host reboot method.
 
  `pci` instructs Xen to reboot the host using PCI reset register (port CF9).
 
+'efi' instructs Xen to reboot using the EFI reboot call (in EFI mode by
+ default it will use that method first).
+
  ### ro-hpet
  > `= <boolean>`
 

xen/arch/x86/shutdown.c +23 -5

@@ -28,16 +28,18 @@
  #include <asm/apic.h>
 
  enum reboot_type {
+        BOOT_INVALID,
          BOOT_TRIPLE = 't',
          BOOT_KBD = 'k',
          BOOT_ACPI = 'a',
          BOOT_CF9 = 'p',
+        BOOT_EFI = 'e',
  };
 
  static int reboot_mode;
 
  /*
- * reboot=t[riple] | k[bd] | a[cpi] | p[ci] | n[o] [, [w]arm | [c]old]
+ * reboot=t[riple] | k[bd] | a[cpi] | p[ci] | n[o] | [e]fi [, [w]arm | [c]old]
   * warm   Don't set the cold reboot flag
   * cold   Set the cold reboot flag
   * no     Suppress automatic reboot after panics or crashes

@@ -45,8 +47,9 @@ static int reboot_mode;
   * kbd    Use the keyboard controller. cold reset (default)
   * acpi   Use the RESET_REG in the FADT
   * pci    Use the so-called "PCI reset register", CF9
+ * efi    Use the EFI reboot (if running under EFI)
   */
-static enum reboot_type reboot_type = BOOT_ACPI;
+static enum reboot_type reboot_type = BOOT_INVALID;
  static void __init set_reboot_type(char *str)
  {
      for ( ; ; )

@@ -63,6 +66,7 @@ static void __init set_reboot_type(char
              reboot_mode = 0x0;
              break;
          case 'a':
+        case 'e':
          case 'k':
          case 't':
          case 'p':

@@ -106,6 +110,14 @@ void machine_halt(void)
      __machine_halt(NULL);
  }
 
+static void default_reboot_type(void)
+{
+    if ( reboot_type == BOOT_INVALID )
+        reboot_type = efi_enabled ? BOOT_EFI
+                                  : acpi_disabled ? BOOT_KBD
+                                                  : BOOT_ACPI;
+}
+
  static int __init override_reboot(struct dmi_system_id *d)
  {
      enum reboot_type type = (long)d->driver_data;

@@ -452,6 +464,7 @@ static struct dmi_system_id __initdata r
 
  static int __init reboot_init(void)
  {
+    default_reboot_type();
      dmi_check_system(reboot_dmi_table);
      return 0;
  }

@@ -465,7 +478,7 @@ static void noreturn __machine_restart(v
  void machine_restart(unsigned int delay_millisecs)
  {
      unsigned int i, attempt;
-    enum reboot_type orig_reboot_type = reboot_type;
+    enum reboot_type orig_reboot_type;
      const struct desc_ptr no_idt = { 0 };
 
      watchdog_disable();

@@ -504,15 +517,20 @@ void machine_restart(unsigned int delay_
          tboot_shutdown(TB_SHUTDOWN_REBOOT);
      }
 
-    efi_reset_system(reboot_mode != 0);
+    /* Just in case reboot_init() didn't run yet. */
+    default_reboot_type();
+    orig_reboot_type = reboot_type;
 
      /* Rebooting needs to touch the page at absolute address 0. */
-    *((unsigned short *)__va(0x472)) = reboot_mode;
+    if ( reboot_type != BOOT_EFI )
+        *((unsigned short *)__va(0x472)) = reboot_mode;
 
      for ( attempt = 0; ; attempt++ )
      {
          switch ( reboot_type )
          {
+        case BOOT_INVALID:
+            ASSERT_UNREACHABLE();
          case BOOT_KBD:
              /* Pulse the keyboard reset line. */
              for ( i = 0; i < 100; i++ )

Quoted 140 lines · Jan Beulich

By default we will always use EFI reboot mechanism when running under EFI platforms. However some EFI platforms are buggy and need to use the ACPI mechanism to reboot (such as Lenovo ThinkCentre M57). As such respect the 'reboot=' override and DMI overrides for EFI platforms.

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

- BOOT_INVALID is just zero
- also consider acpi_disabled in BOOT_INVALID resolution
- duplicate BOOT_INVALID resolution in machine_restart()
- don't fall back from BOOT_ACPI to BOOT_EFI (if it was overridden, it surely was for a reason)
- adjust doc change formatting

Signed-off-by Jan Beulich <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

---
v2: Update reboot_type prior to calling efi_reset_system(), as
requested by Andrew.

docs/misc/xen-command-line.markdown +4 -1

@@ -1145,7 +1145,7 @@ The following resources are available:
   Technology.
 
 ### reboot
-> `= t[riple] | k[bd] | a[cpi] | p[ci] | n[o] [, [w]arm | [c]old]`
+> `= t[riple] | k[bd] | a[cpi] | p[ci] | e[fi] | n[o] [, [w]arm | [c]old]`
 
 > Default: `0`
 

@@ -1165,6 +1165,9 @@ Specify the host reboot method.
 
 `pci` instructs Xen to reboot the host using PCI reset register (port CF9).
 
+'efi' instructs Xen to reboot using the EFI reboot call (in EFI mode by
+ default it will use that method first).
+
 ### ro-hpet
 > `= <boolean>`
 

xen/arch/x86/shutdown.c +23 -5

@@ -28,16 +28,18 @@
 #include <asm/apic.h>
 
 enum reboot_type {
+        BOOT_INVALID,
         BOOT_TRIPLE = 't',
         BOOT_KBD = 'k',
         BOOT_ACPI = 'a',
         BOOT_CF9 = 'p',
+        BOOT_EFI = 'e',
 };
 
 static int reboot_mode;
 
 /*
- * reboot=t[riple] | k[bd] | a[cpi] | p[ci] | n[o] [, [w]arm | [c]old]
+ * reboot=t[riple] | k[bd] | a[cpi] | p[ci] | n[o] | [e]fi [, [w]arm | [c]old]
  * warm   Don't set the cold reboot flag
  * cold   Set the cold reboot flag
  * no     Suppress automatic reboot after panics or crashes

@@ -45,8 +47,9 @@ static int reboot_mode;
  * kbd    Use the keyboard controller. cold reset (default)
  * acpi   Use the RESET_REG in the FADT
  * pci    Use the so-called "PCI reset register", CF9
+ * efi    Use the EFI reboot (if running under EFI)
  */
-static enum reboot_type reboot_type = BOOT_ACPI;
+static enum reboot_type reboot_type = BOOT_INVALID;
 static void __init set_reboot_type(char *str)
 {
     for ( ; ; )

@@ -63,6 +66,7 @@ static void __init set_reboot_type(char
             reboot_mode = 0x0;
             break;
         case 'a':
+        case 'e':
         case 'k':
         case 't':
         case 'p':

@@ -106,6 +110,14 @@ void machine_halt(void)
     __machine_halt(NULL);
 }
 
+static void default_reboot_type(void)
+{
+    if ( reboot_type == BOOT_INVALID )
+        reboot_type = efi_enabled ? BOOT_EFI
+                                  : acpi_disabled ? BOOT_KBD
+                                                  : BOOT_ACPI;
+}
+
 static int __init override_reboot(struct dmi_system_id *d)
 {
     enum reboot_type type = (long)d->driver_data;

@@ -452,6 +464,7 @@ static struct dmi_system_id __initdata r
 
 static int __init reboot_init(void)
 {
+    default_reboot_type();
     dmi_check_system(reboot_dmi_table);
     return 0;
 }

@@ -465,7 +478,7 @@ static void noreturn __machine_restart(v
 void machine_restart(unsigned int delay_millisecs)
 {
     unsigned int i, attempt;
-    enum reboot_type orig_reboot_type = reboot_type;
+    enum reboot_type orig_reboot_type;
     const struct desc_ptr no_idt = { 0 };
 
     watchdog_disable();

@@ -504,15 +517,20 @@ void machine_restart(unsigned int delay_
         tboot_shutdown(TB_SHUTDOWN_REBOOT);
     }
 
-    efi_reset_system(reboot_mode != 0);
+    /* Just in case reboot_init() didn't run yet. */
+    default_reboot_type();
+    orig_reboot_type = reboot_type;
 
     /* Rebooting needs to touch the page at absolute address 0. */
-    *((unsigned short *)__va(0x472)) = reboot_mode;
+    if ( reboot_type != BOOT_EFI )
+        *((unsigned short *)__va(0x472)) = reboot_mode;
 
     for ( attempt = 0; ; attempt++ )
     {
         switch ( reboot_type )
         {
+        case BOOT_INVALID:
+            ASSERT_UNREACHABLE();
         case BOOT_KBD:
             /* Pulse the keyboard reset line. */
             for ( i = 0; i < 100; i++ )

xen/arch/x86/shutdown.c void machine_restart(unsigned int delay_ · line 556

Quoted 7 lines

xen/arch/x86/shutdown.c +4 -0

@@ -532,6 +550,11 @@ void machine_restart(unsigned int delay_
             reboot_type = (((attempt == 1) && (orig_reboot_type == BOOT_ACPI))
                            ? BOOT_ACPI : BOOT_TRIPLE);
             break;
+        case BOOT_EFI:
+            reboot_type = acpi_disabled ? BOOT_KBD : BOOT_ACPI;
+            efi_reset_system(reboot_mode != 0);
+            *((unsigned short *)__va(0x472)) = reboot_mode;

This has broken rebooting on EFI systems because acpi_disabled is marked as __init_data so it causes a page fault trying to access it.

acpi_disabled is also used in default_reboot_type() which is called from machine_restart().

xen/arch/x86/shutdown.c void machine_restart(unsigned int delay_ · line 560

Quoted 7 lines

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

+            break;
         case BOOT_TRIPLE:
             asm volatile ("lidt %0; int3" : : "m" (no_idt));
             reboot_type = BOOT_KBD;

Regards

Signature · 1 line

Ross Lagerwall

Quoted 152 lines · Ross Lagerwall

Quoted 140 lines · Jan Beulich

By default we will always use EFI reboot mechanism when running under EFI platforms. However some EFI platforms are buggy and need to use the ACPI mechanism to reboot (such as Lenovo ThinkCentre M57). As such respect the 'reboot=' override and DMI overrides for EFI platforms.

Signed-off-by Konrad Rzeszutek Wilk <[email protected]>

- BOOT_INVALID is just zero
- also consider acpi_disabled in BOOT_INVALID resolution
- duplicate BOOT_INVALID resolution in machine_restart()
- don't fall back from BOOT_ACPI to BOOT_EFI (if it was overridden, it surely was for a reason)
- adjust doc change formatting

Signed-off-by Jan Beulich <[email protected]>

Reviewed-by Konrad Rzeszutek Wilk <[email protected]>

---
v2: Update reboot_type prior to calling efi_reset_system(), as
requested by Andrew.

docs/misc/xen-command-line.markdown +4 -1

@@ -1145,7 +1145,7 @@ The following resources are available:
    Technology.
 
  ### reboot
-> `= t[riple] | k[bd] | a[cpi] | p[ci] | n[o] [, [w]arm | [c]old]`
+> `= t[riple] | k[bd] | a[cpi] | p[ci] | e[fi] | n[o] [, [w]arm | [c]old]`
 
  > Default: `0`
 

@@ -1165,6 +1165,9 @@ Specify the host reboot method.
 
  `pci` instructs Xen to reboot the host using PCI reset register (port CF9).
 
+'efi' instructs Xen to reboot using the EFI reboot call (in EFI mode by
+ default it will use that method first).
+
  ### ro-hpet
  > `= <boolean>`
 

xen/arch/x86/shutdown.c +23 -5

@@ -28,16 +28,18 @@
  #include <asm/apic.h>
 
  enum reboot_type {
+        BOOT_INVALID,
          BOOT_TRIPLE = 't',
          BOOT_KBD = 'k',
          BOOT_ACPI = 'a',
          BOOT_CF9 = 'p',
+        BOOT_EFI = 'e',
  };
 
  static int reboot_mode;
 
  /*
- * reboot=t[riple] | k[bd] | a[cpi] | p[ci] | n[o] [, [w]arm | [c]old]
+ * reboot=t[riple] | k[bd] | a[cpi] | p[ci] | n[o] | [e]fi [, [w]arm | [c]old]
   * warm   Don't set the cold reboot flag
   * cold   Set the cold reboot flag
   * no     Suppress automatic reboot after panics or crashes

@@ -45,8 +47,9 @@ static int reboot_mode;
   * kbd    Use the keyboard controller. cold reset (default)
   * acpi   Use the RESET_REG in the FADT
   * pci    Use the so-called "PCI reset register", CF9
+ * efi    Use the EFI reboot (if running under EFI)
   */
-static enum reboot_type reboot_type = BOOT_ACPI;
+static enum reboot_type reboot_type = BOOT_INVALID;
  static void __init set_reboot_type(char *str)
  {
      for ( ; ; )

@@ -63,6 +66,7 @@ static void __init set_reboot_type(char
              reboot_mode = 0x0;
              break;
          case 'a':
+        case 'e':
          case 'k':
          case 't':
          case 'p':

@@ -106,6 +110,14 @@ void machine_halt(void)
      __machine_halt(NULL);
  }
 
+static void default_reboot_type(void)
+{
+    if ( reboot_type == BOOT_INVALID )
+        reboot_type = efi_enabled ? BOOT_EFI
+                                  : acpi_disabled ? BOOT_KBD
+                                                  : BOOT_ACPI;
+}
+
  static int __init override_reboot(struct dmi_system_id *d)
  {
      enum reboot_type type = (long)d->driver_data;

@@ -452,6 +464,7 @@ static struct dmi_system_id __initdata r
 
  static int __init reboot_init(void)
  {
+    default_reboot_type();
      dmi_check_system(reboot_dmi_table);
      return 0;
  }

@@ -465,7 +478,7 @@ static void noreturn __machine_restart(v
  void machine_restart(unsigned int delay_millisecs)
  {
      unsigned int i, attempt;
-    enum reboot_type orig_reboot_type = reboot_type;
+    enum reboot_type orig_reboot_type;
      const struct desc_ptr no_idt = { 0 };
 
      watchdog_disable();

@@ -504,15 +517,20 @@ void machine_restart(unsigned int delay_
          tboot_shutdown(TB_SHUTDOWN_REBOOT);
      }
 
-    efi_reset_system(reboot_mode != 0);
+    /* Just in case reboot_init() didn't run yet. */
+    default_reboot_type();
+    orig_reboot_type = reboot_type;
 
      /* Rebooting needs to touch the page at absolute address 0. */
-    *((unsigned short *)__va(0x472)) = reboot_mode;
+    if ( reboot_type != BOOT_EFI )
+        *((unsigned short *)__va(0x472)) = reboot_mode;
 
      for ( attempt = 0; ; attempt++ )
      {
          switch ( reboot_type )
          {
+        case BOOT_INVALID:
+            ASSERT_UNREACHABLE();
          case BOOT_KBD:
              /* Pulse the keyboard reset line. */
              for ( i = 0; i < 100; i++ )

xen/arch/x86/shutdown.c void machine_restart(unsigned int delay_ · line 556

Quoted 7 lines

xen/arch/x86/shutdown.c +4 -0

@@ -532,6 +550,11 @@ void machine_restart(unsigned int delay_
              reboot_type = (((attempt == 1) && (orig_reboot_type == BOOT_ACPI))
                             ? BOOT_ACPI : BOOT_TRIPLE);
              break;
+        case BOOT_EFI:
+            reboot_type = acpi_disabled ? BOOT_KBD : BOOT_ACPI;
+            efi_reset_system(reboot_mode != 0);
+            *((unsigned short *)__va(0x472)) = reboot_mode;

This has broken rebooting on EFI systems because acpi_disabled is marked as __init_data so it causes a page fault trying to access it.

Quoted 2 lines

acpi_disabled is also used in default_reboot_type() which is called from machine_restart().

Bummer! I presume this patch fixes it for you?

xen/arch/x86/setup.c +1 -1

index 384df03..b4b7b91 100644
@@ -106,7 +106,7 @@ struct cpuinfo_x86 __read_mostly boot_cpu_data = { 0, 0, 0, 0, -1 };
 
  unsigned long __read_mostly mmu_cr4_features = XEN_MINIMAL_CR4;
 
-bool_t __initdata acpi_disabled;
+bool_t acpi_disabled;
  bool_t __initdata acpi_force;
  static char __initdata acpi_param[10] = "";
  static void __init parse_acpi_param(char *s)

Quoted 1 line