|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2/2] CA-162192: Fix rebooting on some EFI-booted systems
On 12/03/15 11:44, Jan Beulich wrote:
>>>> On 11.03.15 at 19:49, <konrad.wilk@xxxxxxxxxx> wrote:
>> From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>> 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@xxxxxxxxxx>
> 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@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>
>
> --- 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;
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |