2014-02-28 04:12:00

by Li, Aubrey

[permalink] [raw]
Subject: [patch] x86: Introduce BOOT_EFI and BOOT_CF9 into the reboot sequence loop

This patch is to introduce BOOT_EFI and BOOT_CF9 in the reboot sequence
loop, to fix the reboot problem on the known Intel Bay Trail-T based
platform, for example, ASUS-T100 and Dell Venue 8/11 Pro. These
platforms don't support ACPI reboot, we expect to call EFI runtime
service to handle this case, and CF9 is an alternate after EFI.

We don't need any dmidecode based quirks any more, we don't need extra
"reboot=efi" or "reboot=p" in the command line. The machine can be
rebooted automatically with the patch.

References: https://bugzilla.kernel.org/show_bug.cgi?id=70931
Signed-off-by Aubrey Li <[email protected]>
---
arch/x86/kernel/reboot.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index c752cb4..1b58e00 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -462,11 +462,12 @@ void __attribute__((weak)) mach_reboot_fixups(void)
*
* 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) If still alive, write to the keyboard controller again
+ * 3) If still alive, call EFI runtime service
+ * 4) If still alive, write to the PCI IO port 0xCF9
+ * 5) If still alive, try (1)~(4) one time 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
+ * following the same pattern, except that if we're still alive after
(5) we'll
* try to force a triple fault and then cycle between hitting the keyboard
* controller and doing that
*/
@@ -500,7 +501,7 @@ static void native_machine_emergency_restart(void)
}
if (attempt == 0 && orig_reboot_type == BOOT_ACPI) {
attempt = 1;
- reboot_type = BOOT_ACPI;
+ reboot_type = BOOT_EFI;
} else {
reboot_type = BOOT_TRIPLE;
}
@@ -530,7 +531,7 @@ static void native_machine_emergency_restart(void)
EFI_RESET_WARM :
EFI_RESET_COLD,
EFI_SUCCESS, 0, NULL);
- reboot_type = BOOT_KBD;
+ reboot_type = BOOT_CF9;
break;

case BOOT_CF9:
@@ -548,7 +549,7 @@ static void native_machine_emergency_restart(void)
outb(cf9|reboot_code, 0xcf9);
udelay(50);
}
- reboot_type = BOOT_KBD;
+ reboot_type = BOOT_ACPI;
break;
}
}
--
1.7.10.4


2014-02-28 04:56:46

by Matthew Garrett

[permalink] [raw]
Subject: Re: [patch] x86: Introduce BOOT_EFI and BOOT_CF9 into the reboot sequence loop

On Fri, Feb 28, 2014 at 12:11:57PM +0800, Li, Aubrey wrote:
> This patch is to introduce BOOT_EFI and BOOT_CF9 in the reboot sequence
> loop, to fix the reboot problem on the known Intel Bay Trail-T based
> platform, for example, ASUS-T100 and Dell Venue 8/11 Pro. These
> platforms don't support ACPI reboot, we expect to call EFI runtime
> service to handle this case, and CF9 is an alternate after EFI.

EFI reboot is still somewhat unreliable - it may be safe after the
recent patches to provide a 1:1 mapping. CF9 is, as far as I know, not
part of any spec, so it seems like a bad idea to put it in the default
list.

What do the ACPI reboot vectors look like on these systems?

> - * 3) If still alive, write to the ACPI reboot register again
> - * 4) If still alive, write to the keyboard controller again
> + * 3) If still alive, call EFI runtime service
> + * 4) If still alive, write to the PCI IO port 0xCF9

This is definitely incorrect. The ACPI write *must* occur twice in order
to be effective on various systems. EFI shouldn't be attempted until
after the second ACPI write.

--
Matthew Garrett | [email protected]

2014-02-28 05:22:40

by Li, Aubrey

[permalink] [raw]
Subject: Re: [patch] x86: Introduce BOOT_EFI and BOOT_CF9 into the reboot sequence loop

On 2014/2/28 12:56, Matthew Garrett wrote:
> On Fri, Feb 28, 2014 at 12:11:57PM +0800, Li, Aubrey wrote:
>> This patch is to introduce BOOT_EFI and BOOT_CF9 in the reboot sequence
>> loop, to fix the reboot problem on the known Intel Bay Trail-T based
>> platform, for example, ASUS-T100 and Dell Venue 8/11 Pro. These
>> platforms don't support ACPI reboot, we expect to call EFI runtime
>> service to handle this case, and CF9 is an alternate after EFI.
>
> EFI reboot is still somewhat unreliable - it may be safe after the
> recent patches to provide a 1:1 mapping.

So it's acceptable to put EFI in the default list.


> CF9 is, as far as I know, not part of any spec, so it seems like a bad
> idea to put it in the default list.

Any hurt known if put it in the default list?

>
> What do the ACPI reboot vectors look like on these systems?

Reset register address: 0xCF9
Value to cause reset: 0x6

>
>> - * 3) If still alive, write to the ACPI reboot register again
>> - * 4) If still alive, write to the keyboard controller again
>> + * 3) If still alive, call EFI runtime service
>> + * 4) If still alive, write to the PCI IO port 0xCF9
>
> This is definitely incorrect. The ACPI write *must* occur twice in order
> to be effective on various systems. EFI shouldn't be attempted until
> after the second ACPI write.
>

Do we have any spec mentioned that?

Thanks,
-Aubrey

2014-02-28 05:56:34

by Matthew Garrett

[permalink] [raw]
Subject: Re: [patch] x86: Introduce BOOT_EFI and BOOT_CF9 into the reboot sequence loop

On Fri, Feb 28, 2014 at 01:22:37PM +0800, Li, Aubrey wrote:
> On 2014/2/28 12:56, Matthew Garrett wrote:
> > EFI reboot is still somewhat unreliable - it may be safe after the
> > recent patches to provide a 1:1 mapping.
>
> So it's acceptable to put EFI in the default list.

Probably, once we've got those patches landed (I've lost track of
whether they're in 3.13 or aimed at 3.14)

> > CF9 is, as far as I know, not part of any spec, so it seems like a bad
> > idea to put it in the default list.
>
> Any hurt known if put it in the default list?

Mm. Not all x86 platforms support cf8/cf9 (Moorestown, for instance) and
so it's theoretically possible that they'd put some different hardware
there instead. But then, Moorestown probably has its own reboot code, so
that may not matter?

> >
> > What do the ACPI reboot vectors look like on these systems?
>
> Reset register address: 0xCF9
> Value to cause reset: 0x6

Huh. But that's almost exactly what the PCI reboot code would do. Why
does the PCI method work but the ACPI one fail? Does it really depend on
ORing the original value with the reset value? Or is the timing just
somehow marginal?

> > This is definitely incorrect. The ACPI write *must* occur twice in order
> > to be effective on various systems. EFI shouldn't be attempted until
> > after the second ACPI write.
> >
>
> Do we have any spec mentioned that?

Nope. This is entirely unspecified, it's just how things work - several
vendors use cf9 for the ACPI reboot vector, and there have to be two
writes to cf9 to trigger the reboot. Windows attempts the write twice,
and as a result things work.

--
Matthew Garrett | [email protected]

2014-02-28 06:08:01

by Li, Aubrey

[permalink] [raw]
Subject: Re: [patch] x86: Introduce BOOT_EFI and BOOT_CF9 into the reboot sequence loop

On 2014/2/28 13:56, Matthew Garrett wrote:
> On Fri, Feb 28, 2014 at 01:22:37PM +0800, Li, Aubrey wrote:
>> On 2014/2/28 12:56, Matthew Garrett wrote:
>>> EFI reboot is still somewhat unreliable - it may be safe after the
>>> recent patches to provide a 1:1 mapping.
>>
>> So it's acceptable to put EFI in the default list.
>
> Probably, once we've got those patches landed (I've lost track of
> whether they're in 3.13 or aimed at 3.14)

You didn't look the reference I quoted in the patch.

It's stable if 32/64 bit linux call the corresponding 32/64bit EFI
runtime service. Matt Fleming's mixed mode is aiming at 3.15:

http://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/log/?h=mixed-mode

>
>>> CF9 is, as far as I know, not part of any spec, so it seems like a bad
>>> idea to put it in the default list.
>>
>> Any hurt known if put it in the default list?
>
> Mm. Not all x86 platforms support cf8/cf9 (Moorestown, for instance) and
> so it's theoretically possible that they'd put some different hardware
> there instead. But then, Moorestown probably has its own reboot code, so
> that may not matter?

Yes, Moorestown has its own machine_ops. Instead of the system hanging
after issue "reboot" command, I think and suggest CF9 is worth to have a
try.

>
>>>
>>> What do the ACPI reboot vectors look like on these systems?
>>
>> Reset register address: 0xCF9
>> Value to cause reset: 0x6
>
> Huh. But that's almost exactly what the PCI reboot code would do. Why
> does the PCI method work but the ACPI one fail? Does it really depend on
> ORing the original value with the reset value? Or is the timing just
> somehow marginal?

reboot returns at:

if (!(acpi_gbl_FADT.flags & ACPI_FADT_RESET_REGISTER))
return;

This is a ACPI bug or intention, who knows.

>
>>> This is definitely incorrect. The ACPI write *must* occur twice in order
>>> to be effective on various systems. EFI shouldn't be attempted until
>>> after the second ACPI write.
>>>
>>
>> Do we have any spec mentioned that?
>
> Nope. This is entirely unspecified, it's just how things work - several
> vendors use cf9 for the ACPI reboot vector, and there have to be two
> writes to cf9 to trigger the reboot. Windows attempts the write twice,
> and as a result things work.
>

Thanks to clarify this, I'll refine the patch, including CF9 if you
don't have more concern.

-Aubrey

2014-02-28 06:12:59

by Matthew Garrett

[permalink] [raw]
Subject: Re: [patch] x86: Introduce BOOT_EFI and BOOT_CF9 into the reboot sequence loop

On Fri, Feb 28, 2014 at 02:07:58PM +0800, Li, Aubrey wrote:
> On 2014/2/28 13:56, Matthew Garrett wrote:
> > Probably, once we've got those patches landed (I've lost track of
> > whether they're in 3.13 or aimed at 3.14)
>
> You didn't look the reference I quoted in the patch.
>
> It's stable if 32/64 bit linux call the corresponding 32/64bit EFI
> runtime service. Matt Fleming's mixed mode is aiming at 3.15:
>
> http://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/log/?h=mixed-mode

It's stable as long as you have the 1:1 mapping patches, which are
different to the mixed mode patches. Otherwise it'll work on some
hardware and crash on others.

> > Mm. Not all x86 platforms support cf8/cf9 (Moorestown, for instance) and
> > so it's theoretically possible that they'd put some different hardware
> > there instead. But then, Moorestown probably has its own reboot code, so
> > that may not matter?
>
> Yes, Moorestown has its own machine_ops. Instead of the system hanging
> after issue "reboot" command, I think and suggest CF9 is worth to have a
> try.

Writing to arbitrary register addresses isn't a good plan if we're on a
platform that might have different hardware there.

> >> Reset register address: 0xCF9
> >> Value to cause reset: 0x6
> >
> > Huh. But that's almost exactly what the PCI reboot code would do. Why
> > does the PCI method work but the ACPI one fail? Does it really depend on
> > ORing the original value with the reset value? Or is the timing just
> > somehow marginal?
>
> reboot returns at:
>
> if (!(acpi_gbl_FADT.flags & ACPI_FADT_RESET_REGISTER))
> return;
>
> This is a ACPI bug or intention, who knows.

Well, how about we figure that out? Is there a full acpi dump of one of
these machines somewhere?

--
Matthew Garrett | [email protected]

2014-02-28 06:20:52

by Li, Aubrey

[permalink] [raw]
Subject: Re: [patch] x86: Introduce BOOT_EFI and BOOT_CF9 into the reboot sequence loop

On 2014/2/28 14:12, Matthew Garrett wrote:
> On Fri, Feb 28, 2014 at 02:07:58PM +0800, Li, Aubrey wrote:
>> On 2014/2/28 13:56, Matthew Garrett wrote:
>>> Probably, once we've got those patches landed (I've lost track of
>>> whether they're in 3.13 or aimed at 3.14)
>>
>> You didn't look the reference I quoted in the patch.
>>
>> It's stable if 32/64 bit linux call the corresponding 32/64bit EFI
>> runtime service. Matt Fleming's mixed mode is aiming at 3.15:
>>
>> http://git.kernel.org/cgit/linux/kernel/git/mfleming/efi.git/log/?h=mixed-mode
>
> It's stable as long as you have the 1:1 mapping patches, which are
> different to the mixed mode patches. Otherwise it'll work on some
> hardware and crash on others.
>
>>> Mm. Not all x86 platforms support cf8/cf9 (Moorestown, for instance) and
>>> so it's theoretically possible that they'd put some different hardware
>>> there instead. But then, Moorestown probably has its own reboot code, so
>>> that may not matter?
>>
>> Yes, Moorestown has its own machine_ops. Instead of the system hanging
>> after issue "reboot" command, I think and suggest CF9 is worth to have a
>> try.
>
> Writing to arbitrary register addresses isn't a good plan if we're on a
> platform that might have different hardware there.
>

Do we have one actually? if we have, I'll remove CF9, if no, I persist
in keeping it, because without it my box can't reboot now, :)

>>>> Reset register address: 0xCF9
>>>> Value to cause reset: 0x6
>>>
>>> Huh. But that's almost exactly what the PCI reboot code would do. Why
>>> does the PCI method work but the ACPI one fail? Does it really depend on
>>> ORing the original value with the reset value? Or is the timing just
>>> somehow marginal?
>>
>> reboot returns at:
>>
>> if (!(acpi_gbl_FADT.flags & ACPI_FADT_RESET_REGISTER))
>> return;
>>
>> This is a ACPI bug or intention, who knows.
>
> Well, how about we figure that out? Is there a full acpi dump of one of
> these machines somewhere?
>

Well, I already figured that out. Reset Register Supported flag is ZERO
in FACP table. I attached this table for your interesting.

When I said "this is a ACPI bug or intention", I actually meant it's a
bug or intention created by OEM.

Thanks,
-Aubrey


Attachments:
facp.dsl (8.92 kB)

2014-02-28 06:23:29

by Matthew Garrett

[permalink] [raw]
Subject: Re: [patch] x86: Introduce BOOT_EFI and BOOT_CF9 into the reboot sequence loop

On Fri, Feb 28, 2014 at 02:20:41PM +0800, Li, Aubrey wrote:

> Well, I already figured that out. Reset Register Supported flag is ZERO
> in FACP table. I attached this table for your interesting.

Ok, in that case we need to check how Windows deals with a FACP that has
this flag set to 0 but valid looking registers defined. I'll see if I
can do that tomorrow.

--
Matthew Garrett | [email protected]

2014-02-28 06:40:24

by Li, Aubrey

[permalink] [raw]
Subject: Re: [patch] x86: Introduce BOOT_EFI and BOOT_CF9 into the reboot sequence loop

On 2014/2/28 14:23, Matthew Garrett wrote:
> On Fri, Feb 28, 2014 at 02:20:41PM +0800, Li, Aubrey wrote:
>
>> Well, I already figured that out. Reset Register Supported flag is ZERO
>> in FACP table. I attached this table for your interesting.
>
> Ok, in that case we need to check how Windows deals with a FACP that has
> this flag set to 0 but valid looking registers defined. I'll see if I
> can do that tomorrow.
>

Just let you know, Windows8.1 calls EFI on these boxes for reboot/shutdown.

I'll be surprise if other windows version ignores RESET_REG_SUP to use
ACPI reset registers.

=========ACPI spec ==============================
RESET_REG_SUP: If set, indicates the system supports system reset via
the FADT RESET_REG as described in Section 4.8.3.6
=================================================

Thanks for checking that, please keep me posted. I'll re-submit the
patch after you have an update.

-Aubrey

2014-02-28 06:44:19

by Matthew Garrett

[permalink] [raw]
Subject: Re: [patch] x86: Introduce BOOT_EFI and BOOT_CF9 into the reboot sequence loop

On Fri, Feb 28, 2014 at 02:39:56PM +0800, Li, Aubrey wrote:

> Just let you know, Windows8.1 calls EFI on these boxes for reboot/shutdown.

Ok, in that case we should add EFI reboot to the list once Matt's 1:1
mapping support has landed. The right place to try it is probably after
the second attempt to perform an ACPI reboot. I'm still not enthusiastic
about adding cf9 by default.

--
Matthew Garrett | [email protected]

2014-02-28 06:54:27

by Li, Aubrey

[permalink] [raw]
Subject: Re: [patch] x86: Introduce BOOT_EFI and BOOT_CF9 into the reboot sequence loop

On 2014/2/28 14:44, Matthew Garrett wrote:
> On Fri, Feb 28, 2014 at 02:39:56PM +0800, Li, Aubrey wrote:
>
>> Just let you know, Windows8.1 calls EFI on these boxes for reboot/shutdown.
>
> Ok, in that case we should add EFI reboot to the list once Matt's 1:1
> mapping support has landed. The right place to try it is probably after
> the second attempt to perform an ACPI reboot. I'm still not enthusiastic
> about adding cf9 by default.
>

I'll defer to you if no one on the list supports me to add cf9.

Thanks,
-Aubrey

2014-02-28 17:47:42

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [patch] x86: Introduce BOOT_EFI and BOOT_CF9 into the reboot sequence loop

On 02/27/2014 10:54 PM, Li, Aubrey wrote:
> On 2014/2/28 14:44, Matthew Garrett wrote:
>> On Fri, Feb 28, 2014 at 02:39:56PM +0800, Li, Aubrey wrote:
>>
>>> Just let you know, Windows8.1 calls EFI on these boxes for reboot/shutdown.
>>
>> Ok, in that case we should add EFI reboot to the list once Matt's 1:1
>> mapping support has landed. The right place to try it is probably after
>> the second attempt to perform an ACPI reboot. I'm still not enthusiastic
>> about adding cf9 by default.
>>
>
> I'll defer to you if no one on the list supports me to add cf9.
>

I believe we *had* it in the default list for a while, and it caused
hangs on some systems.

-hpa

2014-02-28 22:11:07

by Li, Aubrey

[permalink] [raw]
Subject: Re: [patch] x86: Introduce BOOT_EFI and BOOT_CF9 into the reboot sequence loop

On 2014/3/1 1:47, H. Peter Anvin wrote:
> On 02/27/2014 10:54 PM, Li, Aubrey wrote:
>> On 2014/2/28 14:44, Matthew Garrett wrote:
>>> On Fri, Feb 28, 2014 at 02:39:56PM +0800, Li, Aubrey wrote:
>>>
>>>> Just let you know, Windows8.1 calls EFI on these boxes for reboot/shutdown.
>>>
>>> Ok, in that case we should add EFI reboot to the list once Matt's 1:1
>>> mapping support has landed. The right place to try it is probably after
>>> the second attempt to perform an ACPI reboot. I'm still not enthusiastic
>>> about adding cf9 by default.
>>>
>>
>> I'll defer to you if no one on the list supports me to add cf9.
>>
>
> I believe we *had* it in the default list for a while, and it caused
> hangs on some systems.

So this can be finalized, add EFI after twice ACPI reboot attempt. I'll
update the bugzilla and the patch.

Thanks,
-Aubrey
>
> -hpa
>
>

2014-02-28 22:17:04

by Adam Williamson

[permalink] [raw]
Subject: Re: [patch] x86: Introduce BOOT_EFI and BOOT_CF9 into the reboot sequence loop

On Sat, 2014-03-01 at 06:11 +0800, Li, Aubrey wrote:
> On 2014/3/1 1:47, H. Peter Anvin wrote:
> > On 02/27/2014 10:54 PM, Li, Aubrey wrote:
> >> On 2014/2/28 14:44, Matthew Garrett wrote:
> >>> On Fri, Feb 28, 2014 at 02:39:56PM +0800, Li, Aubrey wrote:
> >>>
> >>>> Just let you know, Windows8.1 calls EFI on these boxes for reboot/shutdown.
> >>>
> >>> Ok, in that case we should add EFI reboot to the list once Matt's 1:1
> >>> mapping support has landed. The right place to try it is probably after
> >>> the second attempt to perform an ACPI reboot. I'm still not enthusiastic
> >>> about adding cf9 by default.
> >>>
> >>
> >> I'll defer to you if no one on the list supports me to add cf9.
> >>
> >
> > I believe we *had* it in the default list for a while, and it caused
> > hangs on some systems.
>
> So this can be finalized, add EFI after twice ACPI reboot attempt. I'll
> update the bugzilla and the patch.
>
> Thanks,
> -Aubrey

I believe Matt still wanted to check the status of "the recent patches
to provide a 1:1 mapping" and only commit this after those. I was trying
to figure out what patches he meant exactly so I could check that status
(I'm helping! I'm helping!) but I couldn't.
--
Adam Williamson
Fedora QA Community Monkey
IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net
http://www.happyassassin.net