From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ross Lagerwall Subject: Re: [PATCH v2] x86/EFI: allow reboot= overrides when running under EFI Date: Mon, 23 Mar 2015 09:21:14 +0000 Message-ID: <550FDB0A.9040809@citrix.com> References: <5501CD9302000078000694AB@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YZyXv-0007cL-Ff for xen-devel@lists.xenproject.org; Mon, 23 Mar 2015 09:21:19 +0000 In-Reply-To: <5501CD9302000078000694AB@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel , Andrew Cooper List-Id: xen-devel@lists.xenproject.org On 03/12/2015 04:32 PM, 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 > > - 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 > Reviewed-by: Konrad Rzeszutek Wilk > --- > 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 > > `= ` > > --- a/xen/arch/x86/shutdown.c > +++ b/xen/arch/x86/shutdown.c > @@ -28,16 +28,18 @@ > #include > > 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; 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(). > + break; > case BOOT_TRIPLE: > asm volatile ("lidt %0; int3" : : "m" (no_idt)); > reboot_type = BOOT_KBD; > > Regards -- Ross Lagerwall