[PATCH 1/2] x86: Don't apply reboot quirks if reboot set by user

[PATCH 1/2] x86: Don't apply reboot quirks if reboot set by user

[Ross Lagerwall]

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 <ross.lagerwall@citrix.com>
---
 xen/arch/x86/shutdown.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
index 21f6cf5..236b4a1 100644
--- a/xen/arch/x86/shutdown.c
+++ b/xen/arch/x86/shutdown.c
@@ -35,6 +35,7 @@ enum reboot_type {
 };
 
 static int reboot_mode;
+static bool_t reboot_default = 1;
 
 /*
  * reboot=t[riple] | k[bd] | a[cpi] | p[ci] | n[o] [, [w]arm | [c]old]
@@ -51,6 +52,8 @@ static void __init set_reboot_type(char *str)
 {
     for ( ; ; )
     {
+        reboot_default = 0;
+
         switch ( *str )
         {
         case 'n': /* no reboot */
@@ -452,6 +455,13 @@ static struct dmi_system_id __initdata reboot_dmi_table[] = {
 
 static int __init reboot_init(void)
 {
+    /*
+     * Only do the DMI check if reboot_type hasn't been overridden
+     * on the command line
+     */
+    if ( !reboot_default )
+        return 0;
+
     dmi_check_system(reboot_dmi_table);
     return 0;
 }
-- 
2.1.0

[PATCH 2/2] CA-162192: Fix rebooting on some EFI-booted systems

[Ross Lagerwall]

On some systems, the ResetSystem EFI runtime service is broken.  Follow the
Linux (and Windows) way by preferring ACPI reboot over EFI reboot.  This is
done by making BOOT_EFI a reboot mode similar to BOOT_ACPI and BOOT_KBD.

This was seen on an Intel S1200RP_SE with firmware
S1200RP.86B.02.02.0005.102320140911, version 4.6, date 2014-10-23.

Based on the following Linux commits:
de18c850af70 ("x86: EFI runtime service support: EFI runtime services")
a4f1987e4c54 ("x86, reboot: Add EFI and CF9 reboot methods into the
default list")
44be28e9dd98 ("x86/reboot: Add EFI reboot quirk for ACPI Hardware
Reduced flag")

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 docs/misc/xen-command-line.markdown |  5 ++++-
 xen/arch/x86/shutdown.c             | 26 +++++++++++++++++++-------
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 63871cb..95ea5f4 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1113,7 +1113,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] | e[fi] | p[ci] | n[o] [, [w]arm | [c]old]`
 
 > Default: `0`
 
@@ -1131,6 +1131,9 @@ Specify the host reboot method.
 
 `acpi` instructs Xen to reboot the host using RESET_REG in the ACPI FADT.
 
+`efi` instructs Xen to reboot the host using the ResetSystem EFI runtime
+service.
+
 `pci` instructs Xen to reboot the host using PCI reset register (port CF9).
 
 ### ro-hpet
diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
index 236b4a1..1ffbc88 100644
--- a/xen/arch/x86/shutdown.c
+++ b/xen/arch/x86/shutdown.c
@@ -31,6 +31,7 @@ enum reboot_type {
         BOOT_TRIPLE = 't',
         BOOT_KBD = 'k',
         BOOT_ACPI = 'a',
+        BOOT_EFI = 'e',
         BOOT_CF9 = 'p',
 };
 
@@ -38,13 +39,14 @@ static int reboot_mode;
 static bool_t reboot_default = 1;
 
 /*
- * reboot=t[riple] | k[bd] | a[cpi] | p[ci] | n[o] [, [w]arm | [c]old]
+ * reboot=t[riple] | k[bd] | a[cpi] | e[fi] | p[ci] | n[o] [, [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
  * triple Force a triple fault (init)
  * kbd    Use the keyboard controller. cold reset (default)
  * acpi   Use the RESET_REG in the FADT
+ * efi    Use the ResetSystem EFI runtime service
  * pci    Use the so-called "PCI reset register", CF9
  */
 static enum reboot_type reboot_type = BOOT_ACPI;
@@ -66,6 +68,7 @@ static void __init set_reboot_type(char *str)
             reboot_mode = 0x0;
             break;
         case 'a':
+        case 'e':
         case 'k':
         case 't':
         case 'p':
@@ -462,7 +465,14 @@ static int __init reboot_init(void)
     if ( !reboot_default )
         return 0;
 
-    dmi_check_system(reboot_dmi_table);
+    /*
+     * If no quirks apply and the ACPI Hardware Reduced bit is set, prefer EFI
+     * runtime services over ACPI.
+     */
+    if ( !dmi_check_system(reboot_dmi_table) &&
+         (acpi_gbl_FADT.flags & ACPI_FADT_HW_REDUCED) )
+        reboot_type = BOOT_EFI;
+
     return 0;
 }
 __initcall(reboot_init);
@@ -514,8 +524,6 @@ void machine_restart(unsigned int delay_millisecs)
         tboot_shutdown(TB_SHUTDOWN_REBOOT);
     }
 
-    efi_reset_system(reboot_mode != 0);
-
     /* Rebooting needs to touch the page at absolute address 0. */
     *((unsigned short *)__va(0x472)) = reboot_mode;
 
@@ -535,12 +543,12 @@ void machine_restart(unsigned int delay_millisecs)
             /*
              * If this platform supports ACPI reset, we follow a Windows-style
              * reboot attempt sequence:
-             *   ACPI -> KBD -> ACPI -> KBD
+             *   ACPI -> KBD -> ACPI -> KBD -> EFI
              * After this we revert to our usual sequence:
-             *   KBD -> TRIPLE -> KBD -> TRIPLE -> KBD -> ...
+             *   TRIPLE -> KBD -> TRIPLE -> KBD -> ...
              */
             reboot_type = (((attempt == 1) && (orig_reboot_type == BOOT_ACPI))
-                           ? BOOT_ACPI : BOOT_TRIPLE);
+                           ? BOOT_ACPI : BOOT_EFI);
             break;
         case BOOT_TRIPLE:
             asm volatile ("lidt %0; int3" : : "m" (no_idt));
@@ -550,6 +558,10 @@ void machine_restart(unsigned int delay_millisecs)
             acpi_reboot();
             reboot_type = BOOT_KBD;
             break;
+        case BOOT_EFI:
+            efi_reset_system(reboot_mode != 0);
+            reboot_type = BOOT_TRIPLE;
+            break;
         case BOOT_CF9:
             {
                 u8 cf9 = inb(0xcf9) & ~6;
-- 
2.1.0

Re: [PATCH 1/2] x86: Don't apply reboot quirks if reboot set by user

[Andrew Cooper]

On 11/03/15 11:44, Ross Lagerwall wrote:
> 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 <ross.lagerwall@citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> ---
>  xen/arch/x86/shutdown.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
> index 21f6cf5..236b4a1 100644
> --- a/xen/arch/x86/shutdown.c
> +++ b/xen/arch/x86/shutdown.c
> @@ -35,6 +35,7 @@ enum reboot_type {
>  };
>  
>  static int reboot_mode;
> +static bool_t reboot_default = 1;
>  
>  /*
>   * reboot=t[riple] | k[bd] | a[cpi] | p[ci] | n[o] [, [w]arm | [c]old]
> @@ -51,6 +52,8 @@ static void __init set_reboot_type(char *str)
>  {
>      for ( ; ; )
>      {
> +        reboot_default = 0;
> +
>          switch ( *str )
>          {
>          case 'n': /* no reboot */
> @@ -452,6 +455,13 @@ static struct dmi_system_id __initdata reboot_dmi_table[] = {
>  
>  static int __init reboot_init(void)
>  {
> +    /*
> +     * Only do the DMI check if reboot_type hasn't been overridden
> +     * on the command line
> +     */
> +    if ( !reboot_default )
> +        return 0;
> +
>      dmi_check_system(reboot_dmi_table);
>      return 0;
>  }

Re: [PATCH 2/2] CA-162192: Fix rebooting on some EFI-booted systems

[Andrew Cooper]

On 11/03/15 11:44, Ross Lagerwall wrote:
> On some systems, the ResetSystem EFI runtime service is broken.  Follow the
> Linux (and Windows) way by preferring ACPI reboot over EFI reboot.  This is
> done by making BOOT_EFI a reboot mode similar to BOOT_ACPI and BOOT_KBD.
>
> This was seen on an Intel S1200RP_SE with firmware
> S1200RP.86B.02.02.0005.102320140911, version 4.6, date 2014-10-23.
>
> Based on the following Linux commits:
> de18c850af70 ("x86: EFI runtime service support: EFI runtime services")
> a4f1987e4c54 ("x86, reboot: Add EFI and CF9 reboot methods into the
> default list")
> 44be28e9dd98 ("x86/reboot: Add EFI reboot quirk for ACPI Hardware
> Reduced flag")
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>

You should avoid the internal ticket reference in the subject.

If Jan is happy, this could be fixed up on commit.

For the patch itself, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> ---
>  docs/misc/xen-command-line.markdown |  5 ++++-
>  xen/arch/x86/shutdown.c             | 26 +++++++++++++++++++-------
>  2 files changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index 63871cb..95ea5f4 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1113,7 +1113,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] | e[fi] | p[ci] | n[o] [, [w]arm | [c]old]`
>  
>  > Default: `0`
>  
> @@ -1131,6 +1131,9 @@ Specify the host reboot method.
>  
>  `acpi` instructs Xen to reboot the host using RESET_REG in the ACPI FADT.
>  
> +`efi` instructs Xen to reboot the host using the ResetSystem EFI runtime
> +service.
> +
>  `pci` instructs Xen to reboot the host using PCI reset register (port CF9).
>  
>  ### ro-hpet
> diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
> index 236b4a1..1ffbc88 100644
> --- a/xen/arch/x86/shutdown.c
> +++ b/xen/arch/x86/shutdown.c
> @@ -31,6 +31,7 @@ enum reboot_type {
>          BOOT_TRIPLE = 't',
>          BOOT_KBD = 'k',
>          BOOT_ACPI = 'a',
> +        BOOT_EFI = 'e',
>          BOOT_CF9 = 'p',
>  };
>  
> @@ -38,13 +39,14 @@ static int reboot_mode;
>  static bool_t reboot_default = 1;
>  
>  /*
> - * reboot=t[riple] | k[bd] | a[cpi] | p[ci] | n[o] [, [w]arm | [c]old]
> + * reboot=t[riple] | k[bd] | a[cpi] | e[fi] | p[ci] | n[o] [, [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
>   * triple Force a triple fault (init)
>   * kbd    Use the keyboard controller. cold reset (default)
>   * acpi   Use the RESET_REG in the FADT
> + * efi    Use the ResetSystem EFI runtime service
>   * pci    Use the so-called "PCI reset register", CF9
>   */
>  static enum reboot_type reboot_type = BOOT_ACPI;
> @@ -66,6 +68,7 @@ static void __init set_reboot_type(char *str)
>              reboot_mode = 0x0;
>              break;
>          case 'a':
> +        case 'e':
>          case 'k':
>          case 't':
>          case 'p':
> @@ -462,7 +465,14 @@ static int __init reboot_init(void)
>      if ( !reboot_default )
>          return 0;
>  
> -    dmi_check_system(reboot_dmi_table);
> +    /*
> +     * If no quirks apply and the ACPI Hardware Reduced bit is set, prefer EFI
> +     * runtime services over ACPI.
> +     */
> +    if ( !dmi_check_system(reboot_dmi_table) &&
> +         (acpi_gbl_FADT.flags & ACPI_FADT_HW_REDUCED) )
> +        reboot_type = BOOT_EFI;
> +
>      return 0;
>  }
>  __initcall(reboot_init);
> @@ -514,8 +524,6 @@ void machine_restart(unsigned int delay_millisecs)
>          tboot_shutdown(TB_SHUTDOWN_REBOOT);
>      }
>  
> -    efi_reset_system(reboot_mode != 0);
> -
>      /* Rebooting needs to touch the page at absolute address 0. */
>      *((unsigned short *)__va(0x472)) = reboot_mode;
>  
> @@ -535,12 +543,12 @@ void machine_restart(unsigned int delay_millisecs)
>              /*
>               * If this platform supports ACPI reset, we follow a Windows-style
>               * reboot attempt sequence:
> -             *   ACPI -> KBD -> ACPI -> KBD
> +             *   ACPI -> KBD -> ACPI -> KBD -> EFI
>               * After this we revert to our usual sequence:
> -             *   KBD -> TRIPLE -> KBD -> TRIPLE -> KBD -> ...
> +             *   TRIPLE -> KBD -> TRIPLE -> KBD -> ...
>               */
>              reboot_type = (((attempt == 1) && (orig_reboot_type == BOOT_ACPI))
> -                           ? BOOT_ACPI : BOOT_TRIPLE);
> +                           ? BOOT_ACPI : BOOT_EFI);
>              break;
>          case BOOT_TRIPLE:
>              asm volatile ("lidt %0; int3" : : "m" (no_idt));
> @@ -550,6 +558,10 @@ void machine_restart(unsigned int delay_millisecs)
>              acpi_reboot();
>              reboot_type = BOOT_KBD;
>              break;
> +        case BOOT_EFI:
> +            efi_reset_system(reboot_mode != 0);
> +            reboot_type = BOOT_TRIPLE;
> +            break;
>          case BOOT_CF9:
>              {
>                  u8 cf9 = inb(0xcf9) & ~6;

Re: [PATCH 1/2] x86: Don't apply reboot quirks if reboot set by user

[Jan Beulich]

>>> On 11.03.15 at 12:44, <ross.lagerwall@citrix.com> wrote:
> --- a/xen/arch/x86/shutdown.c
> +++ b/xen/arch/x86/shutdown.c
> @@ -35,6 +35,7 @@ enum reboot_type {
>  };
>  
>  static int reboot_mode;
> +static bool_t reboot_default = 1;

__initdata

> @@ -51,6 +52,8 @@ static void __init set_reboot_type(char *str)
>  {
>      for ( ; ; )
>      {
> +        reboot_default = 0;

'n', 'w', and 'c' shouldn't have this effect.

Jan

Re: [PATCH 2/2] CA-162192: Fix rebooting on some EFI-booted systems

[Jan Beulich]

>>> On 11.03.15 at 12:44, <ross.lagerwall@citrix.com> wrote:
> On some systems, the ResetSystem EFI runtime service is broken.  Follow the
> Linux (and Windows) way by preferring ACPI reboot over EFI reboot.  This is
> done by making BOOT_EFI a reboot mode similar to BOOT_ACPI and BOOT_KBD.

No. Just because Linux and/or Windows do things a certain way
doesn't mean we should follow suit. Rebooting via EFI runtime
services should still be the default when running on EFI. Quirks
to overcome certain systems' limitations will of course be
acceptable.

Jan

Re: [PATCH 2/2] CA-162192: Fix rebooting on some EFI-booted systems

[Konrad Rzeszutek Wilk]

On Wed, Mar 11, 2015 at 02:08:37PM +0000, Jan Beulich wrote:
> >>> On 11.03.15 at 12:44, <ross.lagerwall@citrix.com> wrote:
> > On some systems, the ResetSystem EFI runtime service is broken.  Follow the
> > Linux (and Windows) way by preferring ACPI reboot over EFI reboot.  This is
> > done by making BOOT_EFI a reboot mode similar to BOOT_ACPI and BOOT_KBD.
> 
> No. Just because Linux and/or Windows do things a certain way
> doesn't mean we should follow suit. Rebooting via EFI runtime
> services should still be the default when running on EFI. Quirks
> to overcome certain systems' limitations will of course be
> acceptable.

You might just use this patch (that I had forgotten to post) - and which
needs testing since the last posting.:

>From 891f6df52dd9586a43cd76b1052a0caee35ee3e5 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Tue, 3 Feb 2015 11:18:04 -0500
Subject: [PATCH] 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 <konrad.wilk@oracle.com>

---
 docs/misc/xen-command-line.markdown |  5 ++++-
 xen/arch/x86/shutdown.c             | 26 ++++++++++++++++++++------
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 9b458e1..b1f74a1 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1110,7 +1110,7 @@ The following resources are available:
   * `rmid_max` indicates the max value for rmid.
 
 ### reboot
-> `= t[riple] | k[bd] | a[cpi] | p[ci] | n[o] [, [w]arm | [c]old]`
+> `= t[riple] | k[bd] | a[cpi] | p[ci] | n[o] | [e]fi [, [w]arm | [c]old]`
 
 > Default: `0`
 
@@ -1130,6 +1130,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>`
 
diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
index 21f6cf5..021998e 100644
--- a/xen/arch/x86/shutdown.c
+++ b/xen/arch/x86/shutdown.c
@@ -28,16 +28,18 @@
 #include <asm/apic.h>
 
 enum reboot_type {
+        BOOT_INVALID = 'i',
         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 ( ; ; )
@@ -66,6 +69,7 @@ static void __init set_reboot_type(char *str)
         case 'k':
         case 't':
         case 'p':
+        case 'e':
             reboot_type = *str;
             break;
         }
@@ -452,6 +456,9 @@ static struct dmi_system_id __initdata reboot_dmi_table[] = {
 
 static int __init reboot_init(void)
 {
+    if ( reboot_type == BOOT_INVALID )
+        reboot_type = efi_enabled ? BOOT_EFI : BOOT_ACPI;
+
     dmi_check_system(reboot_dmi_table);
     return 0;
 }
@@ -504,10 +511,9 @@ void machine_restart(unsigned int delay_millisecs)
         tboot_shutdown(TB_SHUTDOWN_REBOOT);
     }
 
-    efi_reset_system(reboot_mode != 0);
-
     /* Rebooting needs to touch the page at absolute address 0. */
-    *((unsigned short *)__va(0x472)) = reboot_mode;
+    if ( !efi_enabled )
+        *((unsigned short *)__va(0x472)) = reboot_mode;
 
     for ( attempt = 0; ; attempt++ )
     {
@@ -532,13 +538,21 @@ void machine_restart(unsigned int delay_millisecs)
             reboot_type = (((attempt == 1) && (orig_reboot_type == BOOT_ACPI))
                            ? BOOT_ACPI : BOOT_TRIPLE);
             break;
+        case BOOT_EFI:
+            efi_reset_system(reboot_mode != 0);
+            reboot_type = BOOT_ACPI;
+            break;
         case BOOT_TRIPLE:
             asm volatile ("lidt %0; int3" : : "m" (no_idt));
             reboot_type = BOOT_KBD;
             break;
+        case BOOT_INVALID:
         case BOOT_ACPI:
             acpi_reboot();
-            reboot_type = BOOT_KBD;
+            if ( efi_enabled && attempt == 0 )
+                reboot_type = BOOT_EFI;
+            else
+                reboot_type = BOOT_KBD;
             break;
         case BOOT_CF9:
             {
-- 
2.1.0

> 
> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 1/2] x86: Don't apply reboot quirks if reboot set by user

[Konrad Rzeszutek Wilk]

On Wed, Mar 11, 2015 at 11:44:51AM +0000, Ross Lagerwall wrote:
> 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 <ross.lagerwall@citrix.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  xen/arch/x86/shutdown.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
> index 21f6cf5..236b4a1 100644
> --- a/xen/arch/x86/shutdown.c
> +++ b/xen/arch/x86/shutdown.c
> @@ -35,6 +35,7 @@ enum reboot_type {
>  };
>  
>  static int reboot_mode;
> +static bool_t reboot_default = 1;
>  
>  /*
>   * reboot=t[riple] | k[bd] | a[cpi] | p[ci] | n[o] [, [w]arm | [c]old]
> @@ -51,6 +52,8 @@ static void __init set_reboot_type(char *str)
>  {
>      for ( ; ; )
>      {
> +        reboot_default = 0;
> +
>          switch ( *str )
>          {
>          case 'n': /* no reboot */
> @@ -452,6 +455,13 @@ static struct dmi_system_id __initdata reboot_dmi_table[] = {
>  
>  static int __init reboot_init(void)
>  {
> +    /*
> +     * Only do the DMI check if reboot_type hasn't been overridden
> +     * on the command line
> +     */
> +    if ( !reboot_default )
> +        return 0;
> +
>      dmi_check_system(reboot_dmi_table);
>      return 0;
>  }
> -- 
> 2.1.0
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 2/2] CA-162192: Fix rebooting on some EFI-booted systems

[Jan Beulich]

>>> On 11.03.15 at 15:36, <konrad.wilk@oracle.com> wrote:
> @@ -504,10 +511,9 @@ void machine_restart(unsigned int delay_millisecs)
>          tboot_shutdown(TB_SHUTDOWN_REBOOT);
>      }
>  
> -    efi_reset_system(reboot_mode != 0);
> -
>      /* Rebooting needs to touch the page at absolute address 0. */
> -    *((unsigned short *)__va(0x472)) = reboot_mode;
> +    if ( !efi_enabled )
> +        *((unsigned short *)__va(0x472)) = reboot_mode;

This shouldn't depend on efi_enabled, but on reboot type not being
BOOT_EFI (which means it may need to be duplicated in the "case
BOOT_EFI:" code).

Jan

Re: [PATCH 2/2] CA-162192: Fix rebooting on some EFI-booted systems

[Konrad Rzeszutek Wilk]

On Wed, Mar 11, 2015 at 03:19:28PM +0000, Jan Beulich wrote:
> >>> On 11.03.15 at 15:36, <konrad.wilk@oracle.com> wrote:
> > @@ -504,10 +511,9 @@ void machine_restart(unsigned int delay_millisecs)
> >          tboot_shutdown(TB_SHUTDOWN_REBOOT);
> >      }
> >  
> > -    efi_reset_system(reboot_mode != 0);
> > -
> >      /* Rebooting needs to touch the page at absolute address 0. */
> > -    *((unsigned short *)__va(0x472)) = reboot_mode;
> > +    if ( !efi_enabled )
> > +        *((unsigned short *)__va(0x472)) = reboot_mode;
> 
> This shouldn't depend on efi_enabled, but on reboot type not being
> BOOT_EFI (which means it may need to be duplicated in the "case
> BOOT_EFI:" code).

<nods>

>From da22ca80c2c1570636b5c8f2382e053b63c14a0e Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Tue, 3 Feb 2015 11:18:04 -0500
Subject: [PATCH] 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 <konrad.wilk@oracle.com>
---
 docs/misc/xen-command-line.markdown |  5 ++++-
 xen/arch/x86/shutdown.c             | 27 +++++++++++++++++++++------
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 5044da1..7685aa0 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -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] | n[o] | [e]fi [, [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>`
 
diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
index 21f6cf5..9c93b64 100644
--- a/xen/arch/x86/shutdown.c
+++ b/xen/arch/x86/shutdown.c
@@ -28,16 +28,18 @@
 #include <asm/apic.h>
 
 enum reboot_type {
+        BOOT_INVALID = 'i',
         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 ( ; ; )
@@ -66,6 +69,7 @@ static void __init set_reboot_type(char *str)
         case 'k':
         case 't':
         case 'p':
+        case 'e':
             reboot_type = *str;
             break;
         }
@@ -452,6 +456,9 @@ static struct dmi_system_id __initdata reboot_dmi_table[] = {
 
 static int __init reboot_init(void)
 {
+    if ( reboot_type == BOOT_INVALID )
+        reboot_type = efi_enabled ? BOOT_EFI : BOOT_ACPI;
+
     dmi_check_system(reboot_dmi_table);
     return 0;
 }
@@ -504,10 +511,9 @@ void machine_restart(unsigned int delay_millisecs)
         tboot_shutdown(TB_SHUTDOWN_REBOOT);
     }
 
-    efi_reset_system(reboot_mode != 0);
-
     /* 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++ )
     {
@@ -532,13 +538,22 @@ void machine_restart(unsigned int delay_millisecs)
             reboot_type = (((attempt == 1) && (orig_reboot_type == BOOT_ACPI))
                            ? BOOT_ACPI : BOOT_TRIPLE);
             break;
+        case BOOT_EFI:
+            efi_reset_system(reboot_mode != 0);
+            reboot_type = BOOT_ACPI;
+            *((unsigned short *)__va(0x472)) = reboot_mode;
+            break;
         case BOOT_TRIPLE:
             asm volatile ("lidt %0; int3" : : "m" (no_idt));
             reboot_type = BOOT_KBD;
             break;
+        case BOOT_INVALID:
         case BOOT_ACPI:
             acpi_reboot();
-            reboot_type = BOOT_KBD;
+            if ( efi_enabled && attempt == 0 )
+                reboot_type = BOOT_EFI;
+            else
+                reboot_type = BOOT_KBD;
             break;
         case BOOT_CF9:
             {
-- 
2.1.0

Re: [PATCH 2/2] CA-162192: Fix rebooting on some EFI-booted systems

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 5680 bytes --]

>>> On 11.03.15 at 19:49, <konrad.wilk@oracle.com> wrote:
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Tue, 3 Feb 2015 11:18:04 -0500
> Subject: [PATCH] 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 <konrad.wilk@oracle.com>

Further re-worked (hopefully we're reaching something
everyone is okay with):

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 <konrad.wilk@oracle.com>

- 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 <jbeulich@suse.com>

--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -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>`
 
--- a/xen/arch/x86/shutdown.c
+++ b/xen/arch/x86/shutdown.c
@@ -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:
+            efi_reset_system(reboot_mode != 0);
+            reboot_type = acpi_disabled ? BOOT_KBD : BOOT_ACPI;
+            *((unsigned short *)__va(0x472)) = reboot_mode;
+            break;
         case BOOT_TRIPLE:
             asm volatile ("lidt %0; int3" : : "m" (no_idt));
             reboot_type = BOOT_KBD;



[-- Attachment #2: x86-EFI-reboot-override.patch --]
[-- Type: text/plain, Size: 4997 bytes --]

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 <konrad.wilk@oracle.com>

- 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 <jbeulich@suse.com>

--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -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>`
 
--- a/xen/arch/x86/shutdown.c
+++ b/xen/arch/x86/shutdown.c
@@ -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:
+            efi_reset_system(reboot_mode != 0);
+            reboot_type = acpi_disabled ? BOOT_KBD : BOOT_ACPI;
+            *((unsigned short *)__va(0x472)) = reboot_mode;
+            break;
         case BOOT_TRIPLE:
             asm volatile ("lidt %0; int3" : : "m" (no_idt));
             reboot_type = BOOT_KBD;

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 2/2] CA-162192: Fix rebooting on some EFI-booted systems

[Konrad Rzeszutek Wilk]

On March 12, 2015 7:44:58 AM EDT, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 11.03.15 at 19:49, <konrad.wilk@oracle.com> wrote:
>> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Date: Tue, 3 Feb 2015 11:18:04 -0500
>> Subject: [PATCH] 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 <konrad.wilk@oracle.com>
>
>Further re-worked (hopefully we're reaching something
>everyone is okay with):
>
>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 <konrad.wilk@oracle.com>

And Reviewed-by as well.
>
>- 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 <jbeulich@suse.com>
>
>--- a/docs/misc/xen-command-line.markdown
>+++ b/docs/misc/xen-command-line.markdown
>@@ -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>`
> 
>--- a/xen/arch/x86/shutdown.c
>+++ b/xen/arch/x86/shutdown.c
>@@ -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:
>+            efi_reset_system(reboot_mode != 0);
>+            reboot_type = acpi_disabled ? BOOT_KBD : BOOT_ACPI;
>+            *((unsigned short *)__va(0x472)) = reboot_mode;
>+            break;
>         case BOOT_TRIPLE:
>             asm volatile ("lidt %0; int3" : : "m" (no_idt));
>             reboot_type = BOOT_KBD;

Re: [PATCH 2/2] CA-162192: Fix rebooting on some EFI-booted systems

[Andrew Cooper]

On 12/03/15 11:44, Jan Beulich wrote:
>>>> On 11.03.15 at 19:49, <konrad.wilk@oracle.com> wrote:
>> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> Date: Tue, 3 Feb 2015 11:18:04 -0500
>> Subject: [PATCH] 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 <konrad.wilk@oracle.com>
> Further re-worked (hopefully we're reaching something
> everyone is okay with):
>
> 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 <konrad.wilk@oracle.com>
>
> - 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 <jbeulich@suse.com>
>
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -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>`
>  
> --- a/xen/arch/x86/shutdown.c
> +++ b/xen/arch/x86/shutdown.c
> @@ -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();

This still suffers from the bug Ross fixed in patch 1.  If the user
provides an override, the dmi quirks should be skipped so the request on
the command line is honoured.

>      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:
> +            efi_reset_system(reboot_mode != 0);
> +            reboot_type = acpi_disabled ? BOOT_KBD : BOOT_ACPI;
> +            *((unsigned short *)__va(0x472)) = reboot_mode;
> +            break;

Update the reboot type before calling efi_reset_system()

That way, if efi_reset_system() does fault, we will reenter
machine_restart() and take an alternate route.

The exact symptoms with the reference code is that efi_reset_system()
causes a #GP fault which breaks back into Xen, and the panic() path
reenters efi_reset_system() at which point the system wedges and needs a
hard power cycle.

~Andrew

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

Re: [PATCH 2/2] CA-162192: Fix rebooting on some EFI-booted systems

[Jan Beulich]

>>> On 12.03.15 at 15:07, <andrew.cooper3@citrix.com> wrote:
> On 12/03/15 11:44, Jan Beulich wrote:
>> @@ -452,6 +464,7 @@ static struct dmi_system_id __initdata r
>>  
>>  static int __init reboot_init(void)
>>  {
>> +    default_reboot_type();
> 
> This still suffers from the bug Ross fixed in patch 1.  If the user
> provides an override, the dmi quirks should be skipped so the request on
> the command line is honoured.

And I assumed he'd resubmit that patch anyway, i.e. I didn't even
mean to fix this here.

>> @@ -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:
>> +            efi_reset_system(reboot_mode != 0);
>> +            reboot_type = acpi_disabled ? BOOT_KBD : BOOT_ACPI;
>> +            *((unsigned short *)__va(0x472)) = reboot_mode;
>> +            break;
> 
> Update the reboot type before calling efi_reset_system()
> 
> That way, if efi_reset_system() does fault, we will reenter
> machine_restart() and take an alternate route.
> 
> The exact symptoms with the reference code is that efi_reset_system()
> causes a #GP fault which breaks back into Xen, and the panic() path
> reenters efi_reset_system() at which point the system wedges and needs a
> hard power cycle.

Good point, flipped.

Jan