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

[Andrew Cooper <andrew.cooper3@citrix.com>]

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;