2010-12-09 21:47:26

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 1/3] X86: Revamp reboot behaviour to match Windows more closely

Windows reboots by hitting the ACPI reboot vector (if available), trying
the keyboard controller, hitting the ACPI reboot vector again and then
giving the keyboard controller one last go. Rework our reboot process a
little to default to matching this behaviour, although we'll fall through
to attempting a triple fault if nothing else works.

Signed-off-by: Matthew Garrett <[email protected]>
---
arch/x86/kernel/reboot.c | 23 ++++++++++++++++++++++-
1 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index c495aa8..c770e66 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -34,7 +34,7 @@ EXPORT_SYMBOL(pm_power_off);

static const struct desc_ptr no_idt = {};
static int reboot_mode;
-enum reboot_type reboot_type = BOOT_KBD;
+enum reboot_type reboot_type = BOOT_ACPI;
int reboot_force;

#if defined(CONFIG_X86_32) && defined(CONFIG_SMP)
@@ -538,9 +538,23 @@ void __attribute__((weak)) mach_reboot_fixups(void)
{
}

+/*
+ * Windows does the following on reboot:
+ * 1) If the FADT has the ACPI reboot register flag set, try it
+ * 2) If still alive, write to the keyboard controller
+ * 3) If still alive, write to the ACPI reboot register again
+ * 4) Ig still alive, write to the keyboard controller again
+ *
+ * If the machine is still alive at this stage, it gives up. We default to
+ * following the same pattern, except that if we're still alive after (4) we'll
+ * try to force a triple fault and then cycle between hitting the keyboard
+ * controller and doing that
+ */
static void native_machine_emergency_restart(void)
{
int i;
+ int attempt = 0;
+ int orig_reboot_type = reboot_type;

if (reboot_emergency)
emergency_vmx_disable_all();
@@ -562,6 +576,13 @@ static void native_machine_emergency_restart(void)
outb(0xfe, 0x64); /* pulse reset low */
udelay(50);
}
+ if (attempt == 0 && orig_reboot_type == BOOT_ACPI) {
+ attempt = 1;
+ reboot_type = BOOT_ACPI;
+ } else {
+ reboot_type = BOOT_TRIPLE;
+ }
+ break;

case BOOT_TRIPLE:
load_idt(&no_idt);
--
1.7.3.2


2010-12-09 21:47:35

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 3/3] ACPI: Bug compatibility for Windows on the ACPI reboot vector

Windows ignores the bit_offset and bit_width, despite the spec requiring
that they be validated. Drop the checks so that we match this behaviour.
Windows also goes straight for the keyboard controller if the ACPI reboot
fails, so we shouldn't sleep if we're still alive.

Signed-off-by: Matthew Garrett <[email protected]>
---
drivers/acpi/reboot.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/reboot.c b/drivers/acpi/reboot.c
index 93f9114..4870aaa 100644
--- a/drivers/acpi/reboot.c
+++ b/drivers/acpi/reboot.c
@@ -15,9 +15,10 @@ void acpi_reboot(void)

rr = &acpi_gbl_FADT.reset_register;

- /* Is the reset register supported? */
- if (!(acpi_gbl_FADT.flags & ACPI_FADT_RESET_REGISTER) ||
- rr->bit_width != 8 || rr->bit_offset != 0)
+ /* Is the reset register supported? The spec says we should be
+ * checking the bit width and bit offset, but Windows ignores
+ * these fields */
+ if (!(acpi_gbl_FADT.flags & ACPI_FADT_RESET_REGISTER))
return;

reset_value = acpi_gbl_FADT.reset_value;
@@ -45,6 +46,4 @@ void acpi_reboot(void)
acpi_reset();
break;
}
- /* Wait ten seconds */
- acpi_os_stall(10000000);
}
--
1.7.3.2

2010-12-09 21:47:30

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 2/3] ACPICA: Fix access width for reset vector

Section 4.7.3.6 of the ACPI specification requires that the register width
of the reset vector be 8 bits. Windows simply hardcodes the access to be
a byte and ignores the width provided in the FADT, so make sure that we
do the same.

Signed-off-by: Matthew Garrett <[email protected]>
---
drivers/acpi/acpica/hwxface.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/acpica/hwxface.c b/drivers/acpi/acpica/hwxface.c
index 50cc3be..c6a4e63 100644
--- a/drivers/acpi/acpica/hwxface.c
+++ b/drivers/acpi/acpica/hwxface.c
@@ -82,12 +82,11 @@ acpi_status acpi_reset(void)
/*
* For I/O space, write directly to the OSL. This bypasses the port
* validation mechanism, which may block a valid write to the reset
- * register.
+ * register. Spec section 4.7.3.6 requires register width to be 8.
*/
status =
acpi_os_write_port((acpi_io_address) reset_reg->address,
- acpi_gbl_FADT.reset_value,
- reset_reg->bit_width);
+ acpi_gbl_FADT.reset_value, 8)
} else {
/* Write the reset value to the reset register */

--
1.7.3.2

2010-12-09 22:25:25

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH 1/3] X86: Revamp reboot behaviour to match Windows more closely

On 12/09/2010 01:46 PM, Matthew Garrett wrote:
> Windows reboots by hitting the ACPI reboot vector (if available), trying
> the keyboard controller, hitting the ACPI reboot vector again and then
> giving the keyboard controller one last go. Rework our reboot process a
> little to default to matching this behaviour, although we'll fall through
> to attempting a triple fault if nothing else works.
>
> Signed-off-by: Matthew Garrett<[email protected]>

When this was discussed before we agreed to use ACPI reboot by default
with an ACPI cutoff date; this doesn't have any such cutoff.

-hpa

2010-12-09 22:32:12

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 1/3] X86: Revamp reboot behaviour to match Windows more closely

On Thu, Dec 09, 2010 at 02:25:05PM -0800, H. Peter Anvin wrote:
> On 12/09/2010 01:46 PM, Matthew Garrett wrote:
>> Windows reboots by hitting the ACPI reboot vector (if available), trying
>> the keyboard controller, hitting the ACPI reboot vector again and then
>> giving the keyboard controller one last go. Rework our reboot process a
>> little to default to matching this behaviour, although we'll fall through
>> to attempting a triple fault if nothing else works.
>>
>> Signed-off-by: Matthew Garrett<[email protected]>
>
> When this was discussed before we agreed to use ACPI reboot by default
> with an ACPI cutoff date; this doesn't have any such cutoff.

Windows doesn't either. Older machines simply won't have the appropriate
flag set in their FADT.

--
Matthew Garrett | [email protected]

2010-12-09 22:32:59

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH 1/3] X86: Revamp reboot behaviour to match Windows more closely

On 12/09/2010 02:32 PM, Matthew Garrett wrote:
> On Thu, Dec 09, 2010 at 02:25:05PM -0800, H. Peter Anvin wrote:
>> On 12/09/2010 01:46 PM, Matthew Garrett wrote:
>>> Windows reboots by hitting the ACPI reboot vector (if available), trying
>>> the keyboard controller, hitting the ACPI reboot vector again and then
>>> giving the keyboard controller one last go. Rework our reboot process a
>>> little to default to matching this behaviour, although we'll fall through
>>> to attempting a triple fault if nothing else works.
>>>
>>> Signed-off-by: Matthew Garrett<[email protected]>
>>
>> When this was discussed before we agreed to use ACPI reboot by default
>> with an ACPI cutoff date; this doesn't have any such cutoff.
>
> Windows doesn't either. Older machines simply won't have the appropriate
> flag set in their FADT.
>

Windows doesn't get validated on old hardware.

-hpa

2010-12-09 22:36:43

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH 3/3] ACPI: Bug compatibility for Windows on the ACPI reboot vector

On 12/09/2010 01:46 PM, Matthew Garrett wrote:
> Windows ignores the bit_offset and bit_width, despite the spec requiring
> that they be validated. Drop the checks so that we match this behaviour.
> Windows also goes straight for the keyboard controller if the ACPI reboot
> fails, so we shouldn't sleep if we're still alive.
>
> Signed-off-by: Matthew Garrett<[email protected]>

Be careful: the goal isn't bug-compatibility with Windows, the goal is
to work on the maximum number of systems. Bug-compatibility with
Windows *may* be a way to that goal, but it is not a guarantee, since
Windows doesn't get validated against older systems that went through
WHQL under an earlier version of Windows.

-hpa

2010-12-09 22:38:57

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 1/3] X86: Revamp reboot behaviour to match Windows more closely

On Thu, Dec 09, 2010 at 02:32:52PM -0800, H. Peter Anvin wrote:
> On 12/09/2010 02:32 PM, Matthew Garrett wrote:
>> On Thu, Dec 09, 2010 at 02:25:05PM -0800, H. Peter Anvin wrote:
>>> When this was discussed before we agreed to use ACPI reboot by default
>>> with an ACPI cutoff date; this doesn't have any such cutoff.
>>
>> Windows doesn't either. Older machines simply won't have the appropriate
>> flag set in their FADT.
>>
>
> Windows doesn't get validated on old hardware.

XP does this, and older hardware predates the version of the ACPI spec
that introduced this flag.

--
Matthew Garrett | [email protected]

2010-12-09 22:40:22

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH 1/3] X86: Revamp reboot behaviour to match Windows more closely

On 12/09/2010 02:38 PM, Matthew Garrett wrote:
> On Thu, Dec 09, 2010 at 02:32:52PM -0800, H. Peter Anvin wrote:
>> On 12/09/2010 02:32 PM, Matthew Garrett wrote:
>>> On Thu, Dec 09, 2010 at 02:25:05PM -0800, H. Peter Anvin wrote:
>>>> When this was discussed before we agreed to use ACPI reboot by default
>>>> with an ACPI cutoff date; this doesn't have any such cutoff.
>>>
>>> Windows doesn't either. Older machines simply won't have the appropriate
>>> flag set in their FADT.
>>>
>>
>> Windows doesn't get validated on old hardware.
>
> XP does this, and older hardware predates the version of the ACPI spec
> that introduced this flag.
>

ACPI is very buggy on early implementations, presumably because WHQL had
not yet been extended to include it. That is exactly why we combine
these kinds of things with a date check.

-hpa

2010-12-09 22:41:52

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 2/3] ACPICA: Fix access width for reset vector

On Thursday, December 09, 2010, Matthew Garrett wrote:
> Section 4.7.3.6 of the ACPI specification requires that the register width
> of the reset vector be 8 bits. Windows simply hardcodes the access to be
> a byte and ignores the width provided in the FADT, so make sure that we
> do the same.
>
> Signed-off-by: Matthew Garrett <[email protected]>

Acked-by: Rafael J. Wysocki <[email protected]>

> ---
> drivers/acpi/acpica/hwxface.c | 5 ++---
> 1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/acpica/hwxface.c b/drivers/acpi/acpica/hwxface.c
> index 50cc3be..c6a4e63 100644
> --- a/drivers/acpi/acpica/hwxface.c
> +++ b/drivers/acpi/acpica/hwxface.c
> @@ -82,12 +82,11 @@ acpi_status acpi_reset(void)
> /*
> * For I/O space, write directly to the OSL. This bypasses the port
> * validation mechanism, which may block a valid write to the reset
> - * register.
> + * register. Spec section 4.7.3.6 requires register width to be 8.
> */
> status =
> acpi_os_write_port((acpi_io_address) reset_reg->address,
> - acpi_gbl_FADT.reset_value,
> - reset_reg->bit_width);
> + acpi_gbl_FADT.reset_value, 8)
> } else {
> /* Write the reset value to the reset register */
>
>

2010-12-09 22:42:12

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 3/3] ACPI: Bug compatibility for Windows on the ACPI reboot vector

On Thursday, December 09, 2010, H. Peter Anvin wrote:
> On 12/09/2010 01:46 PM, Matthew Garrett wrote:
> > Windows ignores the bit_offset and bit_width, despite the spec requiring
> > that they be validated. Drop the checks so that we match this behaviour.
> > Windows also goes straight for the keyboard controller if the ACPI reboot
> > fails, so we shouldn't sleep if we're still alive.
> >
> > Signed-off-by: Matthew Garrett<[email protected]>
>
> Be careful: the goal isn't bug-compatibility with Windows, the goal is
> to work on the maximum number of systems. Bug-compatibility with
> Windows *may* be a way to that goal, but it is not a guarantee, since
> Windows doesn't get validated against older systems that went through
> WHQL under an earlier version of Windows.

Agreed.

Rafael

2010-12-09 22:51:53

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 1/3] X86: Revamp reboot behaviour to match Windows more closely

On Thu, Dec 09, 2010 at 02:40:15PM -0800, H. Peter Anvin wrote:
> On 12/09/2010 02:38 PM, Matthew Garrett wrote:
>>> Windows doesn't get validated on old hardware.
>>
>> XP does this, and older hardware predates the version of the ACPI spec
>> that introduced this flag.
>>
>
> ACPI is very buggy on early implementations, presumably because WHQL had
> not yet been extended to include it. That is exactly why we combine
> these kinds of things with a date check.

I'm afraid I don't understand your argument. The date cutoff would be on
the order of 2001 (anything after this will have been tested with XP).
The spec that defines this behaviour only came into existence in August
2000, and any older hardware will be missing the flag that indicates
that this feature is supported. It doesn't seem realistic to believe
that there's any real body of hardware that sets the flag but otherwise
has a broken implementation.

--
Matthew Garrett | [email protected]

2010-12-09 22:53:43

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH 1/3] X86: Revamp reboot behaviour to match Windows more closely

On 12/09/2010 02:51 PM, Matthew Garrett wrote:
> On Thu, Dec 09, 2010 at 02:40:15PM -0800, H. Peter Anvin wrote:
>> On 12/09/2010 02:38 PM, Matthew Garrett wrote:
>>>> Windows doesn't get validated on old hardware.
>>>
>>> XP does this, and older hardware predates the version of the ACPI spec
>>> that introduced this flag.
>>>
>>
>> ACPI is very buggy on early implementations, presumably because WHQL had
>> not yet been extended to include it. That is exactly why we combine
>> these kinds of things with a date check.
>
> I'm afraid I don't understand your argument. The date cutoff would be on
> the order of 2001 (anything after this will have been tested with XP).
> The spec that defines this behaviour only came into existence in August
> 2000, and any older hardware will be missing the flag that indicates
> that this feature is supported. It doesn't seem realistic to believe
> that there's any real body of hardware that sets the flag but otherwise
> has a broken implementation.
>

2001 is probably a good date, then.

It's pretty safe you'll see the bit being set on systems which are older
than that, even if it was not defined at the time it was created -- just
being garbage. That's par for the course in BIOS land.

-hpa

2010-12-09 22:58:44

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 1/3] X86: Revamp reboot behaviour to match Windows more closely

On Thu, Dec 09, 2010 at 02:53:35PM -0800, H. Peter Anvin wrote:
> On 12/09/2010 02:51 PM, Matthew Garrett wrote:
>> I'm afraid I don't understand your argument. The date cutoff would be on
>> the order of 2001 (anything after this will have been tested with XP).
>> The spec that defines this behaviour only came into existence in August
>> 2000, and any older hardware will be missing the flag that indicates
>> that this feature is supported. It doesn't seem realistic to believe
>> that there's any real body of hardware that sets the flag but otherwise
>> has a broken implementation.
>>
>
> 2001 is probably a good date, then.
>
> It's pretty safe you'll see the bit being set on systems which are older
> than that, even if it was not defined at the time it was created -- just
> being garbage. That's par for the course in BIOS land.

There's a revision field in the FADT. They'd need to simultaneously
provide an incorrect revision *and* by pure luck set the 10th bit of a
32-bit register. Is it possible? Yes. Is it likely? No, and I don't see
a benefit in adding extra code to force hardware into a less-tested
configuration.

--
Matthew Garrett | [email protected]

2010-12-09 23:12:18

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH 2/3] ACPICA: Fix access width for reset vector

On 12/09/2010 01:46 PM, Matthew Garrett wrote:
> Section 4.7.3.6 of the ACPI specification requires that the register width
> of the reset vector be 8 bits. Windows simply hardcodes the access to be
> a byte and ignores the width provided in the FADT, so make sure that we
> do the same.
>
> Signed-off-by: Matthew Garrett<[email protected]>

Does anyone have an acpidump from a machine with an ALi 1535 southbridge?

-hpa

2010-12-09 23:26:12

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 2/3] ACPICA: Fix access width for reset vector

On Thu, Dec 09, 2010 at 03:12:01PM -0800, H. Peter Anvin wrote:
> On 12/09/2010 01:46 PM, Matthew Garrett wrote:
>> Section 4.7.3.6 of the ACPI specification requires that the register width
>> of the reset vector be 8 bits. Windows simply hardcodes the access to be
>> a byte and ignores the width provided in the FADT, so make sure that we
>> do the same.
>>
>> Signed-off-by: Matthew Garrett<[email protected]>
>
> Does anyone have an acpidump from a machine with an ALi 1535 southbridge?

I've got one from an HP nx9005. It's only a revision 1 FADT, so there's
no reset register defined.

--
Matthew Garrett | [email protected]