2013-08-21 01:54:44

by Li Fei

[permalink] [raw]
Subject: [PATCH] reboot: perform warm/cold reset correctly for CF9 type

In current implementation for reboot type CF9 and CF9_COND,
warm and cold reset are not differentiated, and both are
performed by writing 0x06 to port 0xCF9 as warm reset. It's not
correct.

This commit will differentiate warm and cold reset, and perform
them correctly as below:
For warm reset, write 0x06 to port 0xCF9;
For cold reset, write 0x0E to port 0xCF9.

From: Liu Chuansheng <[email protected]>
Signed-off-by: Li Fei <[email protected]>
---
arch/x86/kernel/reboot.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 563ed91..6e06299 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -511,10 +511,15 @@ static void native_machine_emergency_restart(void)

case BOOT_CF9_COND:
if (port_cf9_safe) {
- u8 cf9 = inb(0xcf9) & ~6;
+ u8 cf9 = inb(0xcf9) &
+ ~(reboot_mode == REBOOT_WARM ?
+ 0x06 : 0x0E);
outb(cf9|2, 0xcf9); /* Request hard reset */
udelay(50);
- outb(cf9|6, 0xcf9); /* Actually do the reset */
+ /* Actually do the reset */
+ outb(cf9|(reboot_mode == REBOOT_WARM ?
+ 0x06 : 0x0E),
+ 0xcf9);
udelay(50);
}
reboot_type = BOOT_KBD;
--
1.7.4.1



2013-08-21 07:29:45

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] reboot: perform warm/cold reset correctly for CF9 type


* Li Fei <[email protected]> wrote:

> In current implementation for reboot type CF9 and CF9_COND,
> warm and cold reset are not differentiated, and both are
> performed by writing 0x06 to port 0xCF9 as warm reset. It's not
> correct.
>
> This commit will differentiate warm and cold reset, and perform
> them correctly as below:
> For warm reset, write 0x06 to port 0xCF9;
> For cold reset, write 0x0E to port 0xCF9.
>
> From: Liu Chuansheng <[email protected]>
> Signed-off-by: Li Fei <[email protected]>
> ---
> arch/x86/kernel/reboot.c | 9 +++++++--
> 1 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> index 563ed91..6e06299 100644
> --- a/arch/x86/kernel/reboot.c
> +++ b/arch/x86/kernel/reboot.c
> @@ -511,10 +511,15 @@ static void native_machine_emergency_restart(void)
>
> case BOOT_CF9_COND:
> if (port_cf9_safe) {
> - u8 cf9 = inb(0xcf9) & ~6;
> + u8 cf9 = inb(0xcf9) &
> + ~(reboot_mode == REBOOT_WARM ?
> + 0x06 : 0x0E);
> outb(cf9|2, 0xcf9); /* Request hard reset */
> udelay(50);
> - outb(cf9|6, 0xcf9); /* Actually do the reset */
> + /* Actually do the reset */
> + outb(cf9|(reboot_mode == REBOOT_WARM ?
> + 0x06 : 0x0E),
> + 0xcf9);
> udelay(50);

Looks good, but please introduce a reboot_val intermediate
variable instead of duplicating that ugly line-broken
construct twice.

Thanks,

Ingo

2013-08-21 07:34:39

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] reboot: perform warm/cold reset correctly for CF9 type

Careful. This is a very different definition of warm vs cold boot used elsewhere.

Ingo Molnar <[email protected]> wrote:
>
>* Li Fei <[email protected]> wrote:
>
>> In current implementation for reboot type CF9 and CF9_COND,
>> warm and cold reset are not differentiated, and both are
>> performed by writing 0x06 to port 0xCF9 as warm reset. It's not
>> correct.
>>
>> This commit will differentiate warm and cold reset, and perform
>> them correctly as below:
>> For warm reset, write 0x06 to port 0xCF9;
>> For cold reset, write 0x0E to port 0xCF9.
>>
>> From: Liu Chuansheng <[email protected]>
>> Signed-off-by: Li Fei <[email protected]>
>> ---
>> arch/x86/kernel/reboot.c | 9 +++++++--
>> 1 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
>> index 563ed91..6e06299 100644
>> --- a/arch/x86/kernel/reboot.c
>> +++ b/arch/x86/kernel/reboot.c
>> @@ -511,10 +511,15 @@ static void
>native_machine_emergency_restart(void)
>>
>> case BOOT_CF9_COND:
>> if (port_cf9_safe) {
>> - u8 cf9 = inb(0xcf9) & ~6;
>> + u8 cf9 = inb(0xcf9) &
>> + ~(reboot_mode == REBOOT_WARM ?
>> + 0x06 : 0x0E);
>> outb(cf9|2, 0xcf9); /* Request hard reset */
>> udelay(50);
>> - outb(cf9|6, 0xcf9); /* Actually do the reset */
>> + /* Actually do the reset */
>> + outb(cf9|(reboot_mode == REBOOT_WARM ?
>> + 0x06 : 0x0E),
>> + 0xcf9);
>> udelay(50);
>
>Looks good, but please introduce a reboot_val intermediate
>variable instead of duplicating that ugly line-broken
>construct twice.
>
>Thanks,
>
> Ingo

--
Sent from my mobile phone. Please excuse brevity and lack of formatting.

2013-08-21 08:08:29

by Li Fei

[permalink] [raw]
Subject: RE: [PATCH] reboot: perform warm/cold reset correctly for CF9 type

> Careful. This is a very different definition of warm vs cold boot used elsewhere.

We have tested both warm and cold reboot on our x86 platform, and it works well.
Besides with commit, the expected warm and reboot type can be specified through
command line.

> >Looks good, but please introduce a reboot_val intermediate
> >variable instead of duplicating that ugly line-broken
> >construct twice.

Accepted and will update it in later version.

> >
> >Thanks,
> >
> > Ingo
>

Thanks,
Fei
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-08-21 08:15:40

by Li Fei

[permalink] [raw]
Subject: [PATCH V2] reboot: perform warm/cold reset correctly for CF9 type

In current implementation for reboot type CF9 and CF9_COND,
warm and cold reset are not differentiated, and both are
performed by writing 0x06 to port 0xCF9 as warm reset. It's not
correct.

This commit will differentiate warm and cold reset, and perform
them correctly as below:
For warm reset, write 0x06 to port 0xCF9;
For cold reset, write 0x0E to port 0xCF9.

From: Liu Chuansheng <[email protected]>
Signed-off-by: Li Fei <[email protected]>
---
arch/x86/kernel/reboot.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 563ed91..ca42f63 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -511,10 +511,13 @@ static void native_machine_emergency_restart(void)

case BOOT_CF9_COND:
if (port_cf9_safe) {
- u8 cf9 = inb(0xcf9) & ~6;
+ u8 reboot_code = reboot_mode == REBOOT_WARM ?
+ 0x06 : 0x0E;
+ u8 cf9 = inb(0xcf9) & ~reboot_code;
outb(cf9|2, 0xcf9); /* Request hard reset */
udelay(50);
- outb(cf9|6, 0xcf9); /* Actually do the reset */
+ /* Actually do the reset */
+ outb(cf9|reboot_code, 0xcf9);
udelay(50);
}
reboot_type = BOOT_KBD;
--
1.7.4.1


2013-08-21 09:08:40

by H. Peter Anvin

[permalink] [raw]
Subject: RE: [PATCH] reboot: perform warm/cold reset correctly for CF9 type

The thing is that the existing "warm" and "cold" means something different, I believe (skip post vs do post.) It is possible it just works, but it would be good to know which platforms or works on.

Also, why do you need the cf9 boot method at all? All current systems * should * use the ACPI method.

"Li, Fei" <[email protected]> wrote:
>> Careful. This is a very different definition of warm vs cold boot
>used elsewhere.
>
>We have tested both warm and cold reboot on our x86 platform, and it
>works well.
>Besides with commit, the expected warm and reboot type can be specified
>through
>command line.
>
>> >Looks good, but please introduce a reboot_val intermediate
>> >variable instead of duplicating that ugly line-broken
>> >construct twice.
>
>Accepted and will update it in later version.
>
>> >
>> >Thanks,
>> >
>> > Ingo
>>
>
>Thanks,
>Fei

--
Sent from my mobile phone. Please excuse brevity and lack of formatting.

2013-08-22 03:33:10

by Li Fei

[permalink] [raw]
Subject: RE: [PATCH] reboot: perform warm/cold reset correctly for CF9 type

> Subject: RE: [PATCH] reboot: perform warm/cold reset correctly for CF9 type
>
> The thing is that the existing "warm" and "cold" means something different, I
> believe (skip post vs do post.) It is possible it just works, but it would be good to
> know which platforms or works on.
>
> Also, why do you need the cf9 boot method at all? All current systems * should
> * use the ACPI method.
>

In fact, ACPI method also implements reset by writing RESET_VALUE into RESET_REG
port. In our platform, it also uses port 0xCF9. The difference between cold and wart reset
is whether power cycle is involved or not. After all, this is another topic.

>From the source code, CF9 boot method are still used by some apple MAC and dell
platforms if I understand correctly.

Back to our patch, if you really concern the effect of cold reboot 0x0E to current existed
platforms, we can set the default value of reboot_mode as REBOOT_WARM.

Does it make sense?

Thanks,
Fei
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-03 17:35:56

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] reboot: perform warm/cold reset correctly for CF9 type

On 08/21/2013 08:32 PM, Li, Fei wrote:
>
> In fact, ACPI method also implements reset by writing RESET_VALUE into RESET_REG
> port. In our platform, it also uses port 0xCF9. The difference between cold and wart reset
> is whether power cycle is involved or not. After all, this is another topic.
>
> From the source code, CF9 boot method are still used by some apple MAC and dell
> platforms if I understand correctly.
>
> Back to our patch, if you really concern the effect of cold reboot 0x0E to current existed
> platforms, we can set the default value of reboot_mode as REBOOT_WARM.
>
> Does it make sense?
>

Sort-of-kind-of.

The problem here is that "cold" versus "warm" reboot seems to have
slightly different meanings for BIOS (bypass POST) and the CF9 register
(where I presume it is a "deeper" platform reset of some kind ... is it
even well-defined what the semantics are?)

I could be wrong and the cold/warm reset values in CF9 end up having
exactly the same function as the magic BIOS signature does; if so, then
I would like to be told so explicitly, ideally with an explanation about
how it works on the hw level (or a pointer to relevant documentation,
Intel internal docs are fine.)

Finally, again, does this solve a real problem?

-hpa

2013-09-09 01:54:28

by Liu, Chuansheng

[permalink] [raw]
Subject: RE: [PATCH] reboot: perform warm/cold reset correctly for CF9 type

Hello hpa,

> I could be wrong and the cold/warm reset values in CF9 end up having
> exactly the same function as the magic BIOS signature does; if so, then
> I would like to be told so explicitly, ideally with an explanation about
> how it works on the hw level (or a pointer to relevant documentation,
> Intel internal docs are fine.)
The Intel link is here for SandyBridge platform:
https://houston.fm.intel.com/wiki/doku.php?id=svwiki:projects:sandybridge:testplans:reset:start
And we have one internal baytrail platform doc refered it also.

>
> Finally, again, does this solve a real problem?
Yes, we already have the 0xCF9 rebooting function, why not support the COLD/WARM reset both?
Thanks.


For safety and compatibility, we prepared the below patch, do you think is it making sense?



From: liu chuansheng <[email protected]>
Subject: [PATCH] X86, reboot: supporting COLD reset thru 0xcf9 port

Current X86 rebooting function supports the platform reset thru
port 0xcf9, but currently it hardcoded only for WARM reset that
writing 0x6 into port 0xcf9.

Here we added the support the COLD reset that writing 0xe thru
port 0xcf9, and the actual reset type is determined by reboot_mode
variable.

Also for safety and compatibility, we will set the default reboot
mode as WARM reset for several Apple MacBooks.

Signed-off-by: Li Fei <[email protected]>
Signed-off-by: liu chuansheng <[email protected]>
---
arch/x86/kernel/reboot.c | 20 ++++++++++++++++++--
1 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 563ed91..b4a1cb4 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -116,6 +116,12 @@ static int __init set_pci_reboot(const struct dmi_system_id *d)
{
if (reboot_type != BOOT_CF9) {
reboot_type = BOOT_CF9;
+
+ /* The default reboot_mode value is COLD reset(0) for X86 platform,
+ * here for safety and compatibility, we set the default reboot
+ * mode as WARM reset for several Apple MacBooks.
+ */
+ reboot_mode = REBOOT_WARM;
pr_info("%s series board detected. Selecting %s-method for reboots.\n",
"PCI", d->ident);
}
@@ -511,10 +517,20 @@ static void native_machine_emergency_restart(void)

case BOOT_CF9_COND:
if (port_cf9_safe) {
- u8 cf9 = inb(0xcf9) & ~6;
+ u8 reboot_val;
+ u8 cf9;
+
+ if (reboot_mode == REBOOT_WARM)
+ reboot_val = 0x6;
+ else
+ reboot_val = 0xe;
+
+ cf9 = inb(0xcf9) & ~reboot_val;
outb(cf9|2, 0xcf9); /* Request hard reset */
udelay(50);
- outb(cf9|6, 0xcf9); /* Actually do the reset */
+
+ /* Actually do the reset */
+ outb(cf9|reboot_val, 0xcf9);
udelay(50);
}
reboot_type = BOOT_KBD;
--
1.7.0.4

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-09-09 02:57:10

by H. Peter Anvin

[permalink] [raw]
Subject: RE: [PATCH] reboot: perform warm/cold reset correctly for CF9 type

Please answer the questions that I asked, not something else entirely.

"Liu, Chuansheng" <[email protected]> wrote:
>Hello hpa,
>
>> I could be wrong and the cold/warm reset values in CF9 end up having
>> exactly the same function as the magic BIOS signature does; if so,
>then
>> I would like to be told so explicitly, ideally with an explanation
>about
>> how it works on the hw level (or a pointer to relevant documentation,
>> Intel internal docs are fine.)
>The Intel link is here for SandyBridge platform:
>https://houston.fm.intel.com/wiki/doku.php?id=svwiki:projects:sandybridge:testplans:reset:start
>And we have one internal baytrail platform doc refered it also.
>
>>
>> Finally, again, does this solve a real problem?
>Yes, we already have the 0xCF9 rebooting function, why not support the
>COLD/WARM reset both?
>Thanks.
>
>
>For safety and compatibility, we prepared the below patch, do you think
>is it making sense?
>
>
>
>From: liu chuansheng <[email protected]>
>Subject: [PATCH] X86, reboot: supporting COLD reset thru 0xcf9 port
>
>Current X86 rebooting function supports the platform reset thru
>port 0xcf9, but currently it hardcoded only for WARM reset that
>writing 0x6 into port 0xcf9.
>
>Here we added the support the COLD reset that writing 0xe thru
>port 0xcf9, and the actual reset type is determined by reboot_mode
>variable.
>
>Also for safety and compatibility, we will set the default reboot
>mode as WARM reset for several Apple MacBooks.
>
>Signed-off-by: Li Fei <[email protected]>
>Signed-off-by: liu chuansheng <[email protected]>
>---
> arch/x86/kernel/reboot.c | 20 ++++++++++++++++++--
> 1 files changed, 18 insertions(+), 2 deletions(-)
>
>diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
>index 563ed91..b4a1cb4 100644
>--- a/arch/x86/kernel/reboot.c
>+++ b/arch/x86/kernel/reboot.c
>@@ -116,6 +116,12 @@ static int __init set_pci_reboot(const struct
>dmi_system_id *d)
> {
> if (reboot_type != BOOT_CF9) {
> reboot_type = BOOT_CF9;
>+
>+ /* The default reboot_mode value is COLD reset(0) for
>X86 platform,
>+ * here for safety and compatibility, we set the
>default reboot
>+ * mode as WARM reset for several Apple MacBooks.
>+ */
>+ reboot_mode = REBOOT_WARM;
>pr_info("%s series board detected. Selecting %s-method for reboots.\n",
> "PCI", d->ident);
> }
>@@ -511,10 +517,20 @@ static void
>native_machine_emergency_restart(void)
>
> case BOOT_CF9_COND:
> if (port_cf9_safe) {
>- u8 cf9 = inb(0xcf9) & ~6;
>+ u8 reboot_val;
>+ u8 cf9;
>+
>+ if (reboot_mode == REBOOT_WARM)
>+ reboot_val = 0x6;
>+ else
>+ reboot_val = 0xe;
>+
>+ cf9 = inb(0xcf9) & ~reboot_val;
> outb(cf9|2, 0xcf9); /* Request hard reset */
> udelay(50);
>- outb(cf9|6, 0xcf9); /* Actually do the
>reset */
>+
>+ /* Actually do the reset */
>+ outb(cf9|reboot_val, 0xcf9);
> udelay(50);
> }
> reboot_type = BOOT_KBD;

--
Sent from my mobile phone. Please pardon brevity and lack of formatting.

Subject: [tip:x86/reboot] reboot: Allow specifying warm/ cold reset for CF9 boot type

Commit-ID: 02020dae966ae71e8010ecbd04eb847ad0d53669
Gitweb: http://git.kernel.org/tip/02020dae966ae71e8010ecbd04eb847ad0d53669
Author: Li Fei <[email protected]>
AuthorDate: Wed, 21 Aug 2013 16:13:57 +0800
Committer: H. Peter Anvin <[email protected]>
CommitDate: Tue, 10 Sep 2013 08:56:02 -0700

reboot: Allow specifying warm/cold reset for CF9 boot type

In current implementation for reboot type CF9 and CF9_COND,
warm and cold reset are not differentiated, and both are
performed by writing 0x06 to port 0xCF9.

This commit will differentiate warm and cold reset:

For warm reset, write 0x06 to port 0xCF9;
For cold reset, write 0x0E to port 0xCF9.

[ hpa: This meaning of "cold" and "warm" reset is different from other
reboot types use, where "warm" means "bypass BIOS POST". It is also
not entirely clear that it actually solves any actual problem. However,
it would seem fairly harmless to offer this additional option.

Also note that we do not mask bit 3 in the "warm reset" case. This
preserves the behavior on existing systems, including ones quirked
to use CF9. It seems reasonable that on any system where the
warm/cold distinction actually matters that bit 3 would be read as
zero. ]

From: Liu Chuansheng <[email protected]>
Signed-off-by: Li Fei <[email protected]>
Link: http://lkml.kernel.org/r/1377072837.24556.2.camel@fli24-HP-Compaq-8100-Elite-CMT-PC
Signed-off-by: H. Peter Anvin <[email protected]>
---
arch/x86/kernel/reboot.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 563ed91..ca42f63 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -511,10 +511,13 @@ static void native_machine_emergency_restart(void)

case BOOT_CF9_COND:
if (port_cf9_safe) {
- u8 cf9 = inb(0xcf9) & ~6;
+ u8 reboot_code = reboot_mode == REBOOT_WARM ?
+ 0x06 : 0x0E;
+ u8 cf9 = inb(0xcf9) & ~reboot_code;
outb(cf9|2, 0xcf9); /* Request hard reset */
udelay(50);
- outb(cf9|6, 0xcf9); /* Actually do the reset */
+ /* Actually do the reset */
+ outb(cf9|reboot_code, 0xcf9);
udelay(50);
}
reboot_type = BOOT_KBD;

Subject: [tip:x86/reboot] reboot: Allow specifying warm/ cold reset for CF9 boot type

Commit-ID: 16c21ae5ca636cfd38e581ebcf709c49d78ea56d
Gitweb: http://git.kernel.org/tip/16c21ae5ca636cfd38e581ebcf709c49d78ea56d
Author: Li Fei <[email protected]>
AuthorDate: Wed, 21 Aug 2013 16:13:57 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 25 Sep 2013 19:33:22 +0200

reboot: Allow specifying warm/cold reset for CF9 boot type

In current implementation for reboot type CF9 and CF9_COND,
warm and cold reset are not differentiated, and both are
performed by writing 0x06 to port 0xCF9.

This commit will differentiate warm and cold reset:

For warm reset, write 0x06 to port 0xCF9;
For cold reset, write 0x0E to port 0xCF9.

[ hpa: This meaning of "cold" and "warm" reset is different from other
reboot types use, where "warm" means "bypass BIOS POST". It is also
not entirely clear that it actually solves any actual problem. However,
it would seem fairly harmless to offer this additional option.

Also note that we do not mask bit 3 in the "warm reset" case. This
preserves the behavior on existing systems, including ones quirked
to use CF9. It seems reasonable that on any system where the
warm/cold distinction actually matters that bit 3 would be read as
zero. ]

From: Liu Chuansheng <[email protected]>
Signed-off-by: Li Fei <[email protected]>
Link: http://lkml.kernel.org/r/1377072837.24556.2.camel@fli24-HP-Compaq-8100-Elite-CMT-PC
Signed-off-by: H. Peter Anvin <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/reboot.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 563ed91..ca42f63 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -511,10 +511,13 @@ static void native_machine_emergency_restart(void)

case BOOT_CF9_COND:
if (port_cf9_safe) {
- u8 cf9 = inb(0xcf9) & ~6;
+ u8 reboot_code = reboot_mode == REBOOT_WARM ?
+ 0x06 : 0x0E;
+ u8 cf9 = inb(0xcf9) & ~reboot_code;
outb(cf9|2, 0xcf9); /* Request hard reset */
udelay(50);
- outb(cf9|6, 0xcf9); /* Actually do the reset */
+ /* Actually do the reset */
+ outb(cf9|reboot_code, 0xcf9);
udelay(50);
}
reboot_type = BOOT_KBD;