Re: [PATCH] Add DMI quirk for Intel DP55KG mainboard

Re: [PATCH] Add DMI quirk for Intel DP55KG mainboard

[Len Brown]

On Mon, 4 Jan 2010, Christian Hofstaedtler wrote:

> From: Christian Hofstaedtler <ch+git@zeha.at>
> 
> The Intel DP55KG mainboard cannot reboot using the keyboard controller,
> but ACPI works fine. Auto-select ACPI for this board.
> 
> Signed-off-by: Christian Hofstaedtler <ch@zeha.at>
> ---
>  arch/x86/kernel/reboot.c |   21 +++++++++++++++++++++
>  1 files changed, 21 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
> index 1545bc0..b7fcf59 100644
> --- a/arch/x86/kernel/reboot.c
> +++ b/arch/x86/kernel/reboot.c
> @@ -427,6 +427,19 @@ static int __init set_pci_reboot(const struct dmi_system_id *d)
>  	return 0;
>  }
>  
> +/*
> + * Some Intel mainboards need reboot=a to actually reboot
> + */
> +static int __init set_acpi_reboot(const struct dmi_system_id *d)
> +{
> +	if (reboot_type != BOOT_ACPI) {
> +		reboot_type = BOOT_ACPI;
> +		printk(KERN_INFO "%s hardware detected. "
> +		       "Selecting ACPI-method for reboots.\n", d->ident);
> +	}
> +	return 0;
> +}
> +
>  static struct dmi_system_id __initdata pci_reboot_dmi_table[] = {
>  	{	/* Handle problems with rebooting on Apple MacBook5 */
>  		.callback = set_pci_reboot,
> @@ -452,6 +465,14 @@ static struct dmi_system_id __initdata pci_reboot_dmi_table[] = {
>  			DMI_MATCH(DMI_PRODUCT_NAME, "Macmini3,1"),
>  		},
>  	},
> +	{	/* Handle problems with rebooting on Intel DP55KG mainboard */
> +		.callback = set_acpi_reboot,
> +		.ident = "Intel DP55KG Mainboard",
> +		.matches = {
> +			DMI_MATCH(DMI_BOARD_VENDOR, "Intel Corporation"),
> +			DMI_MATCH(DMI_BOARD_NAME, "DP55KG"),
> +		},
> +	},
>  	{ }
>  };
>  
> -- 
> 1.6.4.4


I believe that Venki is sending a patch to make all recent
motherboards use acpi reset by default; and I'd rather
see that kind of patch than this one.

I've had the patch below patch in acpi-test, and thus linus-next,
and -mm since Nov 2008 to change the default for
all systems w/ no issues.

commit 2cb758c617833c18c533e1c4c31167e59d092235
Author: Len Brown <len.brown@intel.com>
Date:   Thu Nov 6 22:43:21 2008 -0500

    x86, ACPI: default to reboot via ACPI (again)
    
    We've run into systems which do not reboot properly
    without using the ACPI reset mechanism.  So lets
    try this in linux-next for a while and see
    how many existing machines stop rebooting
    because they can't handle ACPI reboot.
    
    Signed-off-by: Len Brown <len.brown@intel.com>

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index d2d1ce8..4c53aeb 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -32,7 +32,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)

Re: [PATCH] Add DMI quirk for Intel DP55KG mainboard

[H. Peter Anvin]

On 01/04/2010 09:15 AM, Len Brown wrote:
> 
> I believe that Venki is sending a patch to make all recent
> motherboards use acpi reset by default; and I'd rather
> see that kind of patch than this one.
> 
> I've had the patch below patch in acpi-test, and thus linus-next,
> and -mm since Nov 2008 to change the default for
> all systems w/ no issues.
> 

Maybe I'm misremembering, but didn't we have to pull this patch after it
hit mainstream the last time?

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] Add DMI quirk for Intel DP55KG mainboard

[Len Brown]

> > I believe that Venki is sending a patch to make all recent
> > motherboards use acpi reset by default; and I'd rather
> > see that kind of patch than this one.
> > 
> > I've had the patch below patch in acpi-test, and thus linus-next,
> > and -mm since Nov 2008 to change the default for
> > all systems w/ no issues.
> > 
> 
> Maybe I'm misremembering, but didn't we have to pull this patch after it
> hit mainstream the last time?


c7ffa6c26277b403920e2255d10df849bd613380
"x86: default to reboot via ACPI"

set ACPI as the default in 2.6.28-rc1

8d00450d296dedec9ada38d43b83e79cca6fd5a3
Revert "x86: default to reboot via ACPI"

pulled it in v2.6.28-rc4

The original patch was justified by VMX support,
and the revert was in response to Andrey Borzenkov's
DMI patch to usee BIOS reset on his Toshiba Porgege 4000:

http://lkml.org/lkml/2008/10/31/242

The Toshiba Portege 4000 shipped in early 2002.

The patch to restore ACPI reset to default has been in Linux-next
(with occasional drops) since the day it was dropped from 2.6.28-rc4.

As we have an existence proof, I have no doubt that using
ACPI reset by default will un-earth some regressions.  However,
my guess is that the DMI list of old machines that will fail
with ACPI reset will be smaller than by the DMI list of
brand-new machines that will fail without ACPI reset.

We couuld use a DMI year cut-off as we do with
ACPI itself CONFIG_ACPI_BLACKLIST_YEAR, though that
doesn't work on systems that don't have DMI...
If I were to propose a year, I'd say 2006 and newer,
since XP was "Windows 2006".

thanks,
Len Brown, Intel Open Source Technology Center

Re: [PATCH] Add DMI quirk for Intel DP55KG mainboard

[Arjan van de Ven]

On Mon, 04 Jan 2010 09:30:27 -0800
"H. Peter Anvin" <hpa@zytor.com> wrote:

> On 01/04/2010 09:15 AM, Len Brown wrote:
> > 
> > I believe that Venki is sending a patch to make all recent
> > motherboards use acpi reset by default; and I'd rather
> > see that kind of patch than this one.
> > 
> > I've had the patch below patch in acpi-test, and thus linus-next,
> > and -mm since Nov 2008 to change the default for
> > all systems w/ no issues.
> > 
> 
> Maybe I'm misremembering, but didn't we have to pull this patch after
> it hit mainstream the last time?

can we do it based on the dmi/acpi year?
Eg for all 2008 and later machines or something, while leaving the
older ones as is



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH] Add DMI quirk for Intel DP55KG mainboard

[Robert Hancock]

On 01/04/2010 04:03 PM, Len Brown wrote:
>>> I believe that Venki is sending a patch to make all recent
>>> motherboards use acpi reset by default; and I'd rather
>>> see that kind of patch than this one.
>>>
>>> I've had the patch below patch in acpi-test, and thus linus-next,
>>> and -mm since Nov 2008 to change the default for
>>> all systems w/ no issues.
>>>
>>
>> Maybe I'm misremembering, but didn't we have to pull this patch after it
>> hit mainstream the last time?
>
>
> c7ffa6c26277b403920e2255d10df849bd613380
> "x86: default to reboot via ACPI"
>
> set ACPI as the default in 2.6.28-rc1
>
> 8d00450d296dedec9ada38d43b83e79cca6fd5a3
> Revert "x86: default to reboot via ACPI"
>
> pulled it in v2.6.28-rc4
>
> The original patch was justified by VMX support,
> and the revert was in response to Andrey Borzenkov's
> DMI patch to usee BIOS reset on his Toshiba Porgege 4000:
>
> http://lkml.org/lkml/2008/10/31/242
>
> The Toshiba Portege 4000 shipped in early 2002.
>
> The patch to restore ACPI reset to default has been in Linux-next
> (with occasional drops) since the day it was dropped from 2.6.28-rc4.
>
> As we have an existence proof, I have no doubt that using
> ACPI reset by default will un-earth some regressions.  However,
> my guess is that the DMI list of old machines that will fail
> with ACPI reset will be smaller than by the DMI list of
> brand-new machines that will fail without ACPI reset.
>
> We couuld use a DMI year cut-off as we do with
> ACPI itself CONFIG_ACPI_BLACKLIST_YEAR, though that
> doesn't work on systems that don't have DMI...
> If I were to propose a year, I'd say 2006 and newer,
> since XP was "Windows 2006".

Think you mean 2001, Vista was 2006..

I'm assuming that Windows is using ACPI reset these days, so presumably 
that's what we should be doing by default, and blacklisting machines 
where that doesn't work, rather than the reverse..

Re: [PATCH] Add DMI quirk for Intel DP55KG mainboard

[H. Peter Anvin]

On 01/04/2010 06:15 PM, Robert Hancock wrote:
> 
> Think you mean 2001, Vista was 2006..
> 
> I'm assuming that Windows is using ACPI reset these days, so presumably
> that's what we should be doing by default, and blacklisting machines
> where that doesn't work, rather than the reverse..

Using a DMI cutoff year in the mid-2000's seems like it would make sense.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

[PATCH] Default to ACPI reboots on newish X86 hardware

[Christian Hofstaedtler]

Newer hardware is assumed to no longer reboot succesfully using the
keyboard controller, but needs to use ACPI instead.
To not cause problems with older hardware, only hardware with a BIOS
date 2006 or newer is considered for this choice.

Also unifiy reboot_type selection code.

Signed-off-by: Christian Hofstaedtler <ch@zeha.at>
---
 arch/x86/include/asm/emergency-restart.h |    1 +
 arch/x86/kernel/reboot.c                 |   71 ++++++++++++++++++++++++-----
 2 files changed, 59 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/emergency-restart.h b/arch/x86/include/asm/emergency-restart.h
index cc70c1c..72ee23a 100644
--- a/arch/x86/include/asm/emergency-restart.h
+++ b/arch/x86/include/asm/emergency-restart.h
@@ -2,6 +2,7 @@
 #define _ASM_X86_EMERGENCY_RESTART_H
 
 enum reboot_type {
+	BOOT_UNDECIDED = '?',
 	BOOT_TRIPLE = 't',
 	BOOT_KBD = 'k',
 #ifdef CONFIG_X86_32
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 1545bc0..2768416 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_UNDECIDED;
 int reboot_force;
 
 #if defined(CONFIG_X86_32) && defined(CONFIG_SMP)
@@ -134,7 +134,11 @@ static int __init set_bios_reboot(const struct dmi_system_id *d)
 	return 0;
 }
 
-static struct dmi_system_id __initdata reboot_dmi_table[] = {
+/*
+ * This table only gets used on x86_32, so only use with
+ * set_bios_reboot.
+ */
+static struct dmi_system_id __initdata reboot_dmi_table_x86_32[] = {
 	{	/* Handle problems with rebooting on Dell E520's */
 		.callback = set_bios_reboot,
 		.ident = "Dell E520",
@@ -270,13 +274,6 @@ static struct dmi_system_id __initdata reboot_dmi_table[] = {
 	{ }
 };
 
-static int __init reboot_init(void)
-{
-	dmi_check_system(reboot_dmi_table);
-	return 0;
-}
-core_initcall(reboot_init);
-
 /* The following code and data reboots the machine by switching to real
    mode and jumping to the BIOS reset entry point, as if the CPU has
    really been reset.  The previous version asked the keyboard
@@ -427,7 +424,8 @@ static int __init set_pci_reboot(const struct dmi_system_id *d)
 	return 0;
 }
 
-static struct dmi_system_id __initdata pci_reboot_dmi_table[] = {
+/* This table gets used on x86_32 AND x86_64. */
+static struct dmi_system_id __initdata reboot_dmi_table_all[] = {
 	{	/* Handle problems with rebooting on Apple MacBook5 */
 		.callback = set_pci_reboot,
 		.ident = "Apple MacBook5",
@@ -455,12 +453,59 @@ static struct dmi_system_id __initdata pci_reboot_dmi_table[] = {
 	{ }
 };
 
-static int __init pci_reboot_init(void)
+/* See if the Hardware is new enough to support ACPI reboots. */
+static int __init reboot_acpi_likey_supported(void)
 {
-	dmi_check_system(pci_reboot_dmi_table);
+        int year;
+
+	/* Doesn't exist? Likely an old system */
+	if (!dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL)) {
+		return 0;
+	}
+	/* 0? Likely a buggy new BIOS */
+	if (year == 0) {
+		printk(KERN_ERR "reboot: DMI BIOS year==0, "
+			"assuming ACPI-reboot-capable machine\n");
+		return 1;
+	}
+        /* 2006 was decided as the cut-off year. */
+	if (year < 2006) {
+		return 0;
+	}
+	return 1;
+}
+
+/* Decide how we will reboot:
+ * - Check the X86_32-only quirks table.
+ * - Check the generic quirks table.
+ * - Check if we could use ACPI-based reboot.
+ * - Fall back to old-style Keyboard Controller reboot.
+ *
+ * In any case, don't override user decisions. (reboot=...)
+ */
+static int __init reboot_init(void)
+{
+	if (reboot_type != BOOT_UNDECIDED)
+		return 0;
+
+#ifdef CONFIG_X86_32
+	dmi_check_system(reboot_dmi_table_x86_32);
+#endif /* CONFIG_X86_32 */
+	dmi_check_system(reboot_dmi_table_all);
+
+	if (reboot_type != BOOT_UNDECIDED)
+		return 0;
+
+	if (reboot_acpi_likey_supported()) {
+		reboot_type = BOOT_ACPI;
+	} else {
+		printk(KERN_INFO "Selecting old-style reboot for older hardware\n");
+		reboot_type = BOOT_KBD;
+	}
+
 	return 0;
 }
-core_initcall(pci_reboot_init);
+core_initcall(reboot_init);
 
 static inline void kb_wait(void)
 {
-- 
1.6.4.4

Re: [PATCH] Default to ACPI reboots on newish X86 hardware

[H. Peter Anvin]

On 01/05/2010 04:30 AM, Christian Hofstaedtler wrote:
> Newer hardware is assumed to no longer reboot succesfully using the
> keyboard controller, but needs to use ACPI instead.
> To not cause problems with older hardware, only hardware with a BIOS
> date 2006 or newer is considered for this choice.
> 
> Also unifiy reboot_type selection code.
> 
> +	}
> +	/* 0? Likely a buggy new BIOS */
> +	if (year == 0) {
> +		printk(KERN_ERR "reboot: DMI BIOS year==0, "
> +			"assuming ACPI-reboot-capable machine\n");
> +		return 1;
> +	}
> +        /* 2006 was decided as the cut-off year. */
> +	if (year < 2006) {
> +		return 0;
> +	}
> +	return 1;
> +}
> +

Why assume it is a buggy *new* BIOS rather than a buggy *old* BIOS?

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] Default to ACPI reboots on newish X86 hardware

[Christian Hofstaedtler]

On Tuesday 05 January 2010 18:04:39 H. Peter Anvin wrote:
> On 01/05/2010 04:30 AM, Christian Hofstaedtler wrote:
> > Newer hardware is assumed to no longer reboot succesfully using the
> > keyboard controller, but needs to use ACPI instead.
> > To not cause problems with older hardware, only hardware with a BIOS
> > date 2006 or newer is considered for this choice.
> >
> > Also unifiy reboot_type selection code.
> >
> > +	}
> > +	/* 0? Likely a buggy new BIOS */
> > +	if (year == 0) {
> > +		printk(KERN_ERR "reboot: DMI BIOS year==0, "
> > +			"assuming ACPI-reboot-capable machine\n");
> > +		return 1;
> > +	}
> > +        /* 2006 was decided as the cut-off year. */
> > +	if (year < 2006) {
> > +		return 0;
> > +	}
> > +	return 1;
> > +}
> > +
> 
> Why assume it is a buggy *new* BIOS rather than a buggy *old* BIOS?

Well, this is the same as with CONFIG_ACPI_BLACKLIST_YEAR. But yeah, I've also 
thought about it, but then *I* really don't know how many such broken BIOSes 
are there in the wild. And how many of them are new enough.

Also, I think we could push the year even more up, say, to 2009. On the 
assumption that such new hardware which can't do BOOT_KBD is only being 
produced now (late 2009). Better err on the safe side?

  Christian

Re: [PATCH] Default to ACPI reboots on newish X86 hardware

[H. Peter Anvin]

On 01/05/2010 10:26 AM, Christian Hofstaedtler wrote:
>>
>> Why assume it is a buggy *new* BIOS rather than a buggy *old* BIOS?
> 
> Well, this is the same as with CONFIG_ACPI_BLACKLIST_YEAR. But yeah, I've also 
> thought about it, but then *I* really don't know how many such broken BIOSes 
> are there in the wild. And how many of them are new enough.
> 
> Also, I think we could push the year even more up, say, to 2009. On the 
> assumption that such new hardware which can't do BOOT_KBD is only being 
> produced now (late 2009). Better err on the safe side?
> 

2006-ish seems like a reasonable cutoff to me.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] Add DMI quirk for Intel DP55KG mainboard

[Pavel Machek]

> commit 2cb758c617833c18c533e1c4c31167e59d092235
> Author: Len Brown <len.brown@intel.com>
> Date:   Thu Nov 6 22:43:21 2008 -0500
> 
>     x86, ACPI: default to reboot via ACPI (again)
>     
>     We've run into systems which do not reboot properly
>     without using the ACPI reset mechanism.  So lets
>     try this in linux-next for a while and see
>     how many existing machines stop rebooting
>     because they can't handle ACPI reboot.

>     Signed-off-by: Len Brown <len.brown@intel.com>

Nak.

> @@ -32,7 +32,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;
>  

KBD is present on all PCs, not so with acpi. If machine can't handle
kbd reboot, it is not IBM PC compatible.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Add DMI quirk for Intel DP55KG mainboard

[Matthew Garrett]

On Mon, Jan 04, 2010 at 05:45:58PM -0800, Arjan van de Ven wrote:

> can we do it based on the dmi/acpi year?
> Eg for all 2008 and later machines or something, while leaving the
> older ones as is

I think a saner model would be to use a similar method to Windows - 
that is, change behaviour based on the OSI calls that the firmware 
makes. If the firmware supports Vista, it's probably safe to use the 
ACPI reboot method.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] Add DMI quirk for Intel DP55KG mainboard

[Alan Cox]

> KBD is present on all PCs, not so with acpi. If machine can't handle
> kbd reboot, it is not IBM PC compatible.

Most modern PC systems are not "IBM PC compatible" in various respects.

Defaulting to ACPI reboot is fine as it can fall back to non ACPI reboot
if there is no ACPI or ACPI doesn't provide the needed info for the
system in question

Re: [PATCH] Add DMI quirk for Intel DP55KG mainboard

[Len Brown]

> > can we do it based on the dmi/acpi year?

> I think a saner model would be to use a similar method to Windows - 
> that is, change behaviour based on the OSI calls that the firmware 
> makes. If the firmware supports Vista, it's probably safe to use the 
> ACPI reboot method.

Andrey Borzenkov's Toshiba Portege 4000 failed to reset with ACPI,
which is why we reverted ACPI reset as default in 2008.
Toshiba shipped the Portege 4000 in 2002 running with Windows 2000,
suggesting that Windows 2000 does not use ACPI reset -- at least
not always.

When debugging the reset failure in:
http://bugzilla.kernel.org/show_bug.cgi?id=11942
Yakui Zhao found that Windows XP writes the ACPI reset register
no matter if the FADT says it is valid or not:
http://lkml.indiana.edu/hypermail/linux/kernel/0811.0/02272.html
At the end of the day, we decided that Linux should honor that bit,
but what we learned is that Windows XP apparently uses ACPI reset by 
default.

So it is likely that our DMI exception lists will be minimized
by using legacy reset for W2K generation and before; and
using ACPI reset for Windows XP generation and after.
As Robert Hancock corrected me, XP is _OSI(Windows 2001).

But...
using _OSI is not "a similar method to Windows".
The BIOS does not need to invoke _OSI to determine if
it should expose a properly functioning ACPI reset or not.
Windows XP simply demanded it, and the box failed WHQL
if it did not work.

Further, there is no _guarantee_ that a BIOS will invoke _OSI
at all, let alone a _rule_ for what _OSI() strings the BIOS
will choose to query to trigger its Windows specific
compatibility hooks -- even if common practice is for
a desktop BIOS to evaluate _OSI strings in sequence
up throught he most recent version of Windows it
knows about...

So I'm more comfortable with DMI, and any year after
Windows 2000 is gone is probably reasonable, say 2003.

I have no doubt that no matter what we do, we will
end up with some exceptions on a blacklist, the goal
is simply to minimize those lists.

cheers,
Len Brown, Intel Open Source Technology Center

ps.
For CONFIG_ACPI_BLACKLIST_YEAR we chose not to strive
for consensus on a hard-coded year, but made it a config option.
I'm not a fan of more config options, but that one allowed
us to avoid thrashing about what year ACPI should kick in
and defer that to the distros.  I think it has probably
served its purpose now, as Fedora for the last few years
has shipped with this option disabled -- enabling ACPI
on all systems that present an ACPI BIOS.

Re: [PATCH] Add DMI quirk for Intel DP55KG mainboard

[Matthew Garrett]

On Wed, Jan 06, 2010 at 02:26:44PM -0500, Len Brown wrote:

> But...
> using _OSI is not "a similar method to Windows".
> The BIOS does not need to invoke _OSI to determine if
> it should expose a properly functioning ACPI reset or not.
> Windows XP simply demanded it, and the box failed WHQL
> if it did not work.

http://download.microsoft.com/download/7/E/7/7E7662CF-CBEA-470B-A97E-CE7CE0D98DC2/WinACPI_OSI.docx 
was what I was referring to:

"By using the _OSI method, ASL writers can easily determine the version 
of the ACPI interfaces that the host operating system supports. This 
versioning method provides a solution for creating firmware that can 
support future operating systems and enable the operating system to 
change behavior based on the requested interface levels."

We know that this is used for deciding whether or not to block system IO 
accesses, but it wouldn't surprise me if it's also used to determine 
other functionality like whether or not the ACPI interface is used for 
rebooting.

> Further, there is no _guarantee_ that a BIOS will invoke _OSI
> at all, let alone a _rule_ for what _OSI() strings the BIOS
> will choose to query to trigger its Windows specific
> compatibility hooks -- even if common practice is for
> a desktop BIOS to evaluate _OSI strings in sequence
> up throught he most recent version of Windows it
> knows about...

It's effectively guaranteed if the system is validated with Windows.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] Default to ACPI reboots on newish X86 hardware

[Len Brown]

> Also, I think we could push the year even more up, say, to 2009. On the 
> assumption that such new hardware which can't do BOOT_KBD is only being 
> produced now (late 2009). Better err on the safe side?

I'd prefer sticking with 2006 so we don't need a white-list for
brand-new machines.

Indeed, I think that 2006 is already conservative,
and I'd be willing to debug failures on machines back as far as 2003.

thanks,
-Len Brown, Intel Open Source Technology Center

Re: [PATCH] Add DMI quirk for Intel DP55KG mainboard

[Len Brown]

> > using _OSI is not "a similar method to Windows".
> > The BIOS does not need to invoke _OSI to determine if
> > it should expose a properly functioning ACPI reset or not.
> > Windows XP simply demanded it, and the box failed WHQL
> > if it did not work.
> 
> http://download.microsoft.com/download/7/E/7/7E7662CF-CBEA-470B-A97E-CE7CE0D98DC2/WinACPI_OSI.docx 
> was what I was referring to:
> 
> "By using the _OSI method, ASL writers can easily determine the version 
> of the ACPI interfaces that the host operating system supports. This 
> versioning method provides a solution for creating firmware that can 
> support future operating systems and enable the operating system to 
> change behavior based on the requested interface levels."
> 
> We know that this is used for deciding whether or not to block system IO 
> accesses, but it wouldn't surprise me if it's also used to determine 
> other functionality like whether or not the ACPI interface is used for 
> rebooting.

I've looked at _OSI use in over a hundred DSDTs and never
seen run-time re-configuration of reset support.

I do not think the BIOS has a run-time decision to make here.
If a box is designed to support Windows XP and newer, it is
likely that ACPI_RESET is simply valid and XP blindly uses it.
If reset fails, the box doesn't pass WHQL and the box is fixed.
If W2K is run on that box, ACPI_RESET is still valid, just that
W2K chooses to not write to it.

> > Further, there is no _guarantee_ that a BIOS will invoke _OSI
> > at all, let alone a _rule_ for what _OSI() strings the BIOS
> > will choose to query to trigger its Windows specific
> > compatibility hooks -- even if common practice is for
> > a desktop BIOS to evaluate _OSI strings in sequence
> > up throught he most recent version of Windows it
> > knows about...
> 
> It's effectively guaranteed if the system is validated with Windows.

today's common industry practice != future guarantee

We can't rely on blind use of _OSI to mean "new enough", since
it was supported back in W2K era.  That means we have to parse
the OSI strings.  But what happens when a BIOS writer decides to
evaluate _OSI("Windows Future") without evaluating any of the
old strings we know about?  We would disable ACPI reset on such
a future box?

thanks,
-Len

Re: [PATCH] Add DMI quirk for Intel DP55KG mainboard

[Matthew Garrett]

On Wed, Jan 06, 2010 at 03:22:30PM -0500, Len Brown wrote:

> I've looked at _OSI use in over a hundred DSDTs and never
> seen run-time re-configuration of reset support.

The point isn't that the firmware changes its behaviour - the point is 
that the OS does.

> I do not think the BIOS has a run-time decision to make here.
> If a box is designed to support Windows XP and newer, it is
> likely that ACPI_RESET is simply valid and XP blindly uses it.
> If reset fails, the box doesn't pass WHQL and the box is fixed.
> If W2K is run on that box, ACPI_RESET is still valid, just that
> W2K chooses to not write to it.

And if ACPI_RESET is set but untested (because 2000 never used it)?

> We can't rely on blind use of _OSI to mean "new enough", since
> it was supported back in W2K era.  That means we have to parse
> the OSI strings.  But what happens when a BIOS writer decides to
> evaluate _OSI("Windows Future") without evaluating any of the
> old strings we know about?  We would disable ACPI reset on such
> a future box?

Potentially, yes. But since such a machine would clearly expect to be 
treated as "Windows Future", we'd be running it in an untested 
configuration anyway.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] Add DMI quirk for Intel DP55KG mainboard

[H. Peter Anvin]

On 01/06/2010 12:22 PM, Len Brown wrote:
>>
>> It's effectively guaranteed if the system is validated with Windows.
> 
> today's common industry practice != future guarantee
> 
> We can't rely on blind use of _OSI to mean "new enough", since
> it was supported back in W2K era.  That means we have to parse
> the OSI strings.  But what happens when a BIOS writer decides to
> evaluate _OSI("Windows Future") without evaluating any of the
> old strings we know about?  We would disable ACPI reset on such
> a future box?
> 

What about using _OSI() with a *blacklist*, i.e. treat _OSI("Unknown
String") as "new enough", whereas _OSI("Windows 2000") is ignored?

	-hpa

Re: [PATCH] Add DMI quirk for Intel DP55KG mainboard

[Robert Hancock]

On 01/06/2010 02:22 PM, Len Brown wrote:
>>> using _OSI is not "a similar method to Windows".
>>> The BIOS does not need to invoke _OSI to determine if
>>> it should expose a properly functioning ACPI reset or not.
>>> Windows XP simply demanded it, and the box failed WHQL
>>> if it did not work.
>>
>> http://download.microsoft.com/download/7/E/7/7E7662CF-CBEA-470B-A97E-CE7CE0D98DC2/WinACPI_OSI.docx
>> was what I was referring to:
>>
>> "By using the _OSI method, ASL writers can easily determine the version
>> of the ACPI interfaces that the host operating system supports. This
>> versioning method provides a solution for creating firmware that can
>> support future operating systems and enable the operating system to
>> change behavior based on the requested interface levels."
>>
>> We know that this is used for deciding whether or not to block system IO
>> accesses, but it wouldn't surprise me if it's also used to determine
>> other functionality like whether or not the ACPI interface is used for
>> rebooting.
>
> I've looked at _OSI use in over a hundred DSDTs and never
> seen run-time re-configuration of reset support.
>
> I do not think the BIOS has a run-time decision to make here.
> If a box is designed to support Windows XP and newer, it is
> likely that ACPI_RESET is simply valid and XP blindly uses it.
> If reset fails, the box doesn't pass WHQL and the box is fixed.
> If W2K is run on that box, ACPI_RESET is still valid, just that
> W2K chooses to not write to it.
>
>>> Further, there is no _guarantee_ that a BIOS will invoke _OSI
>>> at all, let alone a _rule_ for what _OSI() strings the BIOS
>>> will choose to query to trigger its Windows specific
>>> compatibility hooks -- even if common practice is for
>>> a desktop BIOS to evaluate _OSI strings in sequence
>>> up throught he most recent version of Windows it
>>> knows about...
>>
>> It's effectively guaranteed if the system is validated with Windows.
>
> today's common industry practice != future guarantee
>
> We can't rely on blind use of _OSI to mean "new enough", since
> it was supported back in W2K era.  That means we have to parse
> the OSI strings.  But what happens when a BIOS writer decides to
> evaluate _OSI("Windows Future") without evaluating any of the
> old strings we know about?  We would disable ACPI reset on such
> a future box?

Well, the use of such OSI calls to indicate the BIOS supports 
OS-specific features is explicitly recommended by Microsoft, i.e. see 
the document linked at 
http://www.microsoft.com/whdc/system/pnppwr/powermgmt/WinACPI_OSI.mspx :

"Place the routine that identifies the operating system in an _INI 
method under the \_SB scope so that _OSI can run as early as possible. 
This placement is important because the operating system makes features 
available based on the string argument to the _OSI method."

Also from http://www.microsoft.com/taiwan/whdc/archive/BIOSAML.mspx :

"By passing the string "Windows 2001" into the _OSI method, the BIOS 
indicates to the operating system that the BIOS is aware of and 
compatible with the ACPI implementation and feature set of Windows XP."

They don't explicitly say how to do it, but the implication is that one 
should use the sort of method I've seen in various BIOSes, where it 
starts with checking _OSI on the newest Windows version and tries all 
known older Windows versions, and stops when it finds the first one that 
succeeds..

[PATCH] Default to ACPI reboots on newish X86 hardware

[Christian Hofstaedtler]

Newer hardware is assumed to no longer reboot succesfully using the
keyboard controller, but needs to use ACPI instead.
To not cause problems with older hardware, only hardware with a BIOS
date 2006 or newer is considered for this choice. Broken BIOSes
reporting a BIOS date of 0 are not specially considered, and therefore
get the KBD reboot behaviour.

Also unifiy reboot_type selection code.

Signed-off-by: Christian Hofstaedtler <ch@zeha.at>
---
 arch/x86/include/asm/emergency-restart.h |    1 +
 arch/x86/kernel/reboot.c                 |   65 ++++++++++++++++++++++++------
 2 files changed, 53 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/emergency-restart.h b/arch/x86/include/asm/emergency-restart.h
index cc70c1c..72ee23a 100644
--- a/arch/x86/include/asm/emergency-restart.h
+++ b/arch/x86/include/asm/emergency-restart.h
@@ -2,6 +2,7 @@
 #define _ASM_X86_EMERGENCY_RESTART_H
 
 enum reboot_type {
+	BOOT_UNDECIDED = '?',
 	BOOT_TRIPLE = 't',
 	BOOT_KBD = 'k',
 #ifdef CONFIG_X86_32
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 1545bc0..5e78483 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_UNDECIDED;
 int reboot_force;
 
 #if defined(CONFIG_X86_32) && defined(CONFIG_SMP)
@@ -134,7 +134,11 @@ static int __init set_bios_reboot(const struct dmi_system_id *d)
 	return 0;
 }
 
-static struct dmi_system_id __initdata reboot_dmi_table[] = {
+/*
+ * This table only gets used on x86_32, so only use with
+ * set_bios_reboot.
+ */
+static struct dmi_system_id __initdata reboot_dmi_table_x86_32[] = {
 	{	/* Handle problems with rebooting on Dell E520's */
 		.callback = set_bios_reboot,
 		.ident = "Dell E520",
@@ -270,13 +274,6 @@ static struct dmi_system_id __initdata reboot_dmi_table[] = {
 	{ }
 };
 
-static int __init reboot_init(void)
-{
-	dmi_check_system(reboot_dmi_table);
-	return 0;
-}
-core_initcall(reboot_init);
-
 /* The following code and data reboots the machine by switching to real
    mode and jumping to the BIOS reset entry point, as if the CPU has
    really been reset.  The previous version asked the keyboard
@@ -427,7 +424,8 @@ static int __init set_pci_reboot(const struct dmi_system_id *d)
 	return 0;
 }
 
-static struct dmi_system_id __initdata pci_reboot_dmi_table[] = {
+/* This table gets used on x86_32 AND x86_64. */
+static struct dmi_system_id __initdata reboot_dmi_table_all[] = {
 	{	/* Handle problems with rebooting on Apple MacBook5 */
 		.callback = set_pci_reboot,
 		.ident = "Apple MacBook5",
@@ -455,12 +453,53 @@ static struct dmi_system_id __initdata pci_reboot_dmi_table[] = {
 	{ }
 };
 
-static int __init pci_reboot_init(void)
+/* See if the Hardware is new enough to support ACPI reboots. */
+static int __init reboot_acpi_likey_supported(void)
+{
+        int year;
+
+	/* Doesn't exist? Likely an old system */
+	if (!dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL)) {
+		return 0;
+	}
+        /* 2006 was decided as the cut-off year. */
+	if (year < 2006) {
+		return 0;
+	}
+	return 1;
+}
+
+/* Decide how we will reboot:
+ * - Check the X86_32-only quirks table.
+ * - Check the generic quirks table.
+ * - Check if we could use ACPI-based reboot.
+ * - Fall back to old-style Keyboard Controller reboot.
+ *
+ * In any case, don't override user decisions. (reboot=...)
+ */
+static int __init reboot_init(void)
 {
-	dmi_check_system(pci_reboot_dmi_table);
+	if (reboot_type != BOOT_UNDECIDED)
+		return 0;
+
+#ifdef CONFIG_X86_32
+	dmi_check_system(reboot_dmi_table_x86_32);
+#endif /* CONFIG_X86_32 */
+	dmi_check_system(reboot_dmi_table_all);
+
+	if (reboot_type != BOOT_UNDECIDED)
+		return 0;
+
+	if (reboot_acpi_likey_supported()) {
+		reboot_type = BOOT_ACPI;
+	} else {
+		printk(KERN_INFO "Selecting old-style reboot for older hardware\n");
+		reboot_type = BOOT_KBD;
+	}
+
 	return 0;
 }
-core_initcall(pci_reboot_init);
+core_initcall(reboot_init);
 
 static inline void kb_wait(void)
 {
-- 
1.6.4.4

Re: [PATCH] Default to ACPI reboots on newish X86 hardware

[Christian Hofstaedtler]

On Thursday 07 January 2010 21:03:13 Christian Hofstaedtler wrote:
> Newer hardware is assumed to no longer reboot succesfully using the
> keyboard controller, but needs to use ACPI instead.
> To not cause problems with older hardware, only hardware with a BIOS
> date 2006 or newer is considered for this choice. Broken BIOSes
> reporting a BIOS date of 0 are not specially considered, and therefore
> get the KBD reboot behaviour.
> 
> Also unifiy reboot_type selection code.

So this is probably the patch with all comments addressed, except for doing 
some _OSI ACPI call detection. If someone wants to do something based on that 
idea, please do - I know too little about this stuff to be useful here.

  Christian

Re: [PATCH] Default to ACPI reboots on newish X86 hardware

[H. Peter Anvin]

On 01/07/2010 12:03 PM, Christian Hofstaedtler wrote:
> Newer hardware is assumed to no longer reboot succesfully using the
> keyboard controller, but needs to use ACPI instead.
> To not cause problems with older hardware, only hardware with a BIOS
> date 2006 or newer is considered for this choice. Broken BIOSes
> reporting a BIOS date of 0 are not specially considered, and therefore
> get the KBD reboot behaviour.
> 
> Also unifiy reboot_type selection code.
> 
> Signed-off-by: Christian Hofstaedtler <ch@zeha.at>

Looks good to me.  Len: do you want to take it or should I?

	-hpa

Re: [PATCH] Add DMI quirk for Intel DP55KG mainboard

[Len Brown]

> What about using _OSI() with a *blacklist*, i.e. treat _OSI("Unknown
> String") as "new enough", whereas _OSI("Windows 2000") is ignored?

Sound reasoning, but I think that parsing the _OSI strings presented
by the BIOS so that the OS can deving whether it is a Windows XP
box or newer is more complicated than this problem merits.

I recommend that we compare DMI date with 2003 and go from there.
If we find that doesn't work, we can apply additional cleverness.

-Len Brown
Intel Open Source Technology Center

Re: [PATCH] Default to ACPI reboots on newish X86 hardware

[Len Brown]

On Thu, 7 Jan 2010, Christian Hofstaedtler wrote:

Thanks for writing this patch Christian,
it is something that has been unsettled for
a long time and it will be great to close the issue.

> Newer hardware is assumed to no longer reboot succesfully using the
> keyboard controller, but needs to use ACPI instead.
> To not cause problems with older hardware, only hardware with a BIOS
> date 2006 or newer is considered for this choice. Broken BIOSes
> reporting a BIOS date of 0 are not specially considered, and therefore
> get the KBD reboot behaviour.
> 
> Also unifiy reboot_type selection code.

Please split the patch in two patches:

1. cleanup w/o policy change
2. policy change w/o cleanup

better if the policy change is #2, so if we need to revert it
we don't have to revert the cleanup too.

> Signed-off-by: Christian Hofstaedtler <ch@zeha.at>
> ---
>  arch/x86/include/asm/emergency-restart.h |    1 +
>  arch/x86/kernel/reboot.c                 |   65 ++++++++++++++++++++++++------
>  2 files changed, 53 insertions(+), 13 deletions(-)


> +/* See if the Hardware is new enough to support ACPI reboots. */
> +static int __init reboot_acpi_likey_supported(void)
> +{
> +        int year;
> +
> +	/* Doesn't exist? Likely an old system */
> +	if (!dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL)) {
> +		return 0;
> +	}

I think it may be better to simply return 1 in this case.

While we have seen dmi_get_date() fail in practice on "modern" machines,
if CONFIG_ACPI_BLACKLIST_YEAR is set, we will already punish users by
disabling ACPI and making them invoke acpi=force.

So the effect of this check is to disable ACPI-reset
on systems where the user has likely already invoked acpi=force --
which seems somewhat counter-intuitive.

> +        /* 2006 was decided as the cut-off year. */
> +	if (year < 2006) {
> +		return 0;
> +	}

I'd rather see 2003.
If we run into trouble, it is a 1-liner to move it forward.
But I think we'll probably do fine with anything newer than 2001.

thanks,
Len Brown, Intel Open Source Technolgy Center

[PATCH 1/2] x86: Unify reboot_type selection

[Christian Hofstaedtler]

Unify x86_32-only and x86_32+x86_64 reboot quirks selection functions,
so the code path is a bit easier to understand and gets a predefined
execution order.

Signed-off-by: Christian Hofstaedtler <ch@zeha.at>
---
 arch/x86/include/asm/emergency-restart.h |    1 +
 arch/x86/kernel/reboot.c                 |   43 +++++++++++++++++++++---------
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/emergency-restart.h b/arch/x86/include/asm/emergency-restart.h
index cc70c1c..72ee23a 100644
--- a/arch/x86/include/asm/emergency-restart.h
+++ b/arch/x86/include/asm/emergency-restart.h
@@ -2,6 +2,7 @@
 #define _ASM_X86_EMERGENCY_RESTART_H
 
 enum reboot_type {
+	BOOT_UNDECIDED = '?',
 	BOOT_TRIPLE = 't',
 	BOOT_KBD = 'k',
 #ifdef CONFIG_X86_32
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 1545bc0..36f0c86 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_UNDECIDED;
 int reboot_force;
 
 #if defined(CONFIG_X86_32) && defined(CONFIG_SMP)
@@ -134,7 +134,11 @@ static int __init set_bios_reboot(const struct dmi_system_id *d)
 	return 0;
 }
 
-static struct dmi_system_id __initdata reboot_dmi_table[] = {
+/*
+ * This table only gets used on x86_32, so only use with
+ * set_bios_reboot.
+ */
+static struct dmi_system_id __initdata reboot_dmi_table_x86_32[] = {
 	{	/* Handle problems with rebooting on Dell E520's */
 		.callback = set_bios_reboot,
 		.ident = "Dell E520",
@@ -270,13 +274,6 @@ static struct dmi_system_id __initdata reboot_dmi_table[] = {
 	{ }
 };
 
-static int __init reboot_init(void)
-{
-	dmi_check_system(reboot_dmi_table);
-	return 0;
-}
-core_initcall(reboot_init);
-
 /* The following code and data reboots the machine by switching to real
    mode and jumping to the BIOS reset entry point, as if the CPU has
    really been reset.  The previous version asked the keyboard
@@ -427,7 +424,8 @@ static int __init set_pci_reboot(const struct dmi_system_id *d)
 	return 0;
 }
 
-static struct dmi_system_id __initdata pci_reboot_dmi_table[] = {
+/* This table gets used on x86_32 AND x86_64. */
+static struct dmi_system_id __initdata reboot_dmi_table_all[] = {
 	{	/* Handle problems with rebooting on Apple MacBook5 */
 		.callback = set_pci_reboot,
 		.ident = "Apple MacBook5",
@@ -455,12 +453,30 @@ static struct dmi_system_id __initdata pci_reboot_dmi_table[] = {
 	{ }
 };
 
-static int __init pci_reboot_init(void)
+/* Decide how we will reboot:
+ * - Check the X86_32-only quirks table.
+ * - Check the generic quirks table.
+ * - Default to old-style Keyboard Controller reboot.
+ */
+static int __init reboot_init(void)
 {
-	dmi_check_system(pci_reboot_dmi_table);
+	/* don't override user decisions (reboot=...) */
+	if (reboot_type != BOOT_UNDECIDED)
+		return 0;
+
+#ifdef CONFIG_X86_32
+	dmi_check_system(reboot_dmi_table_x86_32);
+#endif /* CONFIG_X86_32 */
+	dmi_check_system(reboot_dmi_table_all);
+
+	if (reboot_type != BOOT_UNDECIDED)
+		return 0;
+
+	reboot_type = BOOT_KBD;
+
 	return 0;
 }
-core_initcall(pci_reboot_init);
+core_initcall(reboot_init);
 
 static inline void kb_wait(void)
 {
@@ -534,6 +550,7 @@ static void native_machine_emergency_restart(void)
 	for (;;) {
 		/* Could also try the reset bit in the Hammer NB */
 		switch (reboot_type) {
+		case BOOT_UNDECIDED:
 		case BOOT_KBD:
 			mach_reboot_fixups(); /* for board specific fixups */
 
-- 
1.6.4.4

[PATCH 2/2] Default to ACPI reboots on newish X86 hardware

[Christian Hofstaedtler]

Newer hardware reportedly can no longer successfully reboot using the
keyboard controller, but needs to use ACPI instead.

To not cause problems with older hardware, only hardware with a BIOS
date 2003 or newer is considered for this choice.
Broken BIOSes (no DMI BIOS date, or BIOS date of 0) also get ACPI
reboots, but those will auto fallback to KBD reboots, as ACPI won't get
enabled without acpi=force.
---
 arch/x86/kernel/reboot.c |   32 ++++++++++++++++++++++++++++++--
 1 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 36f0c86..919453a 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -453,10 +453,33 @@ static struct dmi_system_id __initdata reboot_dmi_table_all[] = {
 	{ }
 };
 
+/* See if the Hardware is new enough to support ACPI reboots. */
+static int __init reboot_acpi_likey_supported(void)
+{
+	int year;
+
+	/* No BIOS date? We can safely say "Yes" here, because ACPI-reboot
+	 * will only work when acpi=force was specified, else it falls back
+	 * to KBD-reboots anyway. */
+	if (!dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL)) {
+		return 1;
+	}
+	if (year == 0) {
+		return 1;
+	}
+
+	/* 2003 was decided as the cut-off year. */
+	if (year < 2003) {
+		return 0;
+	}
+	return 1;
+}
+
 /* Decide how we will reboot:
  * - Check the X86_32-only quirks table.
  * - Check the generic quirks table.
- * - Default to old-style Keyboard Controller reboot.
+ * - Check if we could use ACPI-based reboot.
+ * - Fall back to old-style Keyboard Controller reboot.
  */
 static int __init reboot_init(void)
 {
@@ -472,7 +495,12 @@ static int __init reboot_init(void)
 	if (reboot_type != BOOT_UNDECIDED)
 		return 0;
 
-	reboot_type = BOOT_KBD;
+	if (reboot_acpi_likey_supported()) {
+		reboot_type = BOOT_ACPI;
+	} else {
+		printk(KERN_INFO "Selecting old-style reboot for older hardware\n");
+		reboot_type = BOOT_KBD;
+	}
 
 	return 0;
 }
-- 
1.6.4.4

Re: [PATCH] Default to ACPI reboots on newish X86 hardware

[Christian Hofstaedtler]

Len,

* Len Brown <lenb@kernel.org> [100120 06:21]:
> On Thu, 7 Jan 2010, Christian Hofstaedtler wrote:
> 
> Thanks for writing this patch Christian,
> it is something that has been unsettled for
> a long time and it will be great to close the issue.
> 
> > Newer hardware is assumed to no longer reboot succesfully using the
> > keyboard controller, but needs to use ACPI instead.
> > To not cause problems with older hardware, only hardware with a BIOS
> > date 2006 or newer is considered for this choice. Broken BIOSes
> > reporting a BIOS date of 0 are not specially considered, and therefore
> > get the KBD reboot behaviour.
> > 
> > Also unifiy reboot_type selection code.
> 
> Please split the patch in two patches:
> 
> 1. cleanup w/o policy change
> 2. policy change w/o cleanup
> 
> better if the policy change is #2, so if we need to revert it
> we don't have to revert the cleanup too.

Please find the splitted patch in reply to your mail.
 
> > Signed-off-by: Christian Hofstaedtler <ch@zeha.at>
> > ---
> >  arch/x86/include/asm/emergency-restart.h |    1 +
> >  arch/x86/kernel/reboot.c                 |   65 ++++++++++++++++++++++++------
> >  2 files changed, 53 insertions(+), 13 deletions(-)
> 
> 
> > +/* See if the Hardware is new enough to support ACPI reboots. */
> > +static int __init reboot_acpi_likey_supported(void)
> > +{
> > +        int year;
> > +
> > +	/* Doesn't exist? Likely an old system */
> > +	if (!dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL)) {
> > +		return 0;
> > +	}
> 
> I think it may be better to simply return 1 in this case.
> 
> While we have seen dmi_get_date() fail in practice on "modern" machines,
> if CONFIG_ACPI_BLACKLIST_YEAR is set, we will already punish users by
> disabling ACPI and making them invoke acpi=force.
> 
> So the effect of this check is to disable ACPI-reset
> on systems where the user has likely already invoked acpi=force --
> which seems somewhat counter-intuitive.

Okay; I've also re-added the check for year==0, so these machines
get the same behaviour.

> > +        /* 2006 was decided as the cut-off year. */
> > +	if (year < 2006) {
> > +		return 0;
> > +	}
> 
> I'd rather see 2003.
> If we run into trouble, it is a 1-liner to move it forward.
> But I think we'll probably do fine with anything newer than 2001.

Made this 2003.

> thanks,
> Len Brown, Intel Open Source Technolgy Center


  Christian

-- 
christian hofstaedtler

Re: [PATCH 1/2] x86: Unify reboot_type selection

[H. Peter Anvin]

On 01/21/2010 09:18 AM, Christian Hofstaedtler wrote:
> Unify x86_32-only and x86_32+x86_64 reboot quirks selection functions,
> so the code path is a bit easier to understand and gets a predefined
> execution order.

>  
> -static struct dmi_system_id __initdata reboot_dmi_table[] = {
> +/*
> + * This table only gets used on x86_32, so only use with
> + * set_bios_reboot.
> + */
> +static struct dmi_system_id __initdata reboot_dmi_table_x86_32[] = {
>  	{	/* Handle problems with rebooting on Dell E520's */
>  		.callback = set_bios_reboot,
>  		.ident = "Dell E520",
> @@ -270,13 +274,6 @@ static struct dmi_system_id __initdata reboot_dmi_table[] = {
>  	{ }
>  };
>  

I think it would make more sense to just #ifdef off a section of a
single table, instead of having an #ifdef for a separate table and an
#ifdef for a table entry.

I don't know how big these tables are -- this is initdata after all, so
unless the tables are really small, we could just make set_bios_reboot a
noop on x86-64.

	-hpa

Re: [PATCH 2/2] Default to ACPI reboots on newish X86 hardware

[Len Brown]

Hi Christian,
I've replaced your previous patch in the acpi-test tree with this 
refreshed pair.

Note that the 2nd patch had some checkpatch issues.
Perhaps you can run checkpatch before sending future patches.

WARNING: braces {} are not necessary for single statement blocks
#35: FILE: arch/x86/kernel/reboot.c:464:
+	if (!dmi_get_date(DMI_BIOS_DATE, &year, NULL, NULL)) {
+		return 1;
+	}

WARNING: braces {} are not necessary for single statement blocks
#38: FILE: arch/x86/kernel/reboot.c:467:
+	if (year == 0) {
+		return 1;
+	}

WARNING: braces {} are not necessary for single statement blocks
#43: FILE: arch/x86/kernel/reboot.c:472:
+	if (year < 2003) {
+		return 0;
+	}

ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 3 warnings, 47 lines checked


thanks,
Len Brown, Intel Open Source Technology Center