Re: [Xen-devel] [PATCH v2] x86/EFI: allow reboot= overrides when running under EFI

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 12/03/15 16:32, Jan Beulich wrote:
> 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@xxxxxxxxxx>
>
> - 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@xxxxxxxx>
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>

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

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

> ---
> v2: Update reboot_type prior to calling efi_reset_system(), as
>     requested by Andrew.
>
> --- 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:
> +            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;
>
>


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