[RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Thomas Renninger]

Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

References: http://bugzilla.kernel.org/show_bug.cgi?id=8246

Blacklist R40e, R51e and T40, T40p, T41, T41p, T42, T42p, R50 and R50p
ThinkPads to use the RSDT instead of the XSDT.

R40e (c-states freeze machine) and R51e (slow booting) have known problems.
The rest share one BIOS and have been added in hope that this bug gets solved,
but this is not proven:
  T41p shutting down due to "temperature critical"
  https://bugzilla.novell.com/show_bug.cgi?id=378327
I saw a kind of fixed temperature offset with the exact same machine models
and exact same BIOS. Short after cold boot the one machine showed about 15
degree C more than the other. This looked like wrong, too high temperature
values read from EC on the one machine. I never found out why, this could
explain and fix it, but I did not double check.

It does not matter to have this BIOS also use 32 bit addresses, XP and 
OpenSUSE 11.0 does that and it works fine.

Signed-off-by: Thomas Renninger <trenn@suse.de>
Tested-by: Mark Doughty <me@markdoughty.co.uk>
CC: Yakui Zhao <yakui.zhao@intel.com>

---
 Documentation/kernel-parameters.txt |    5 ++++
 drivers/acpi/tables.c               |   37 ++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -237,6 +237,11 @@ and is between 256 and 4096 characters.
 			to assume that this machine's pmtimer latches its value
 			and always returns good values.
 
+	acpi_root_table= [X86,ACPI]
+			{ rsdt }
+			rsdt: Take RSDT address for fetching
+			ACPI tables (instead of XSDT)
+
 	agp=		[AGP]
 			{ off | try_unsupported }
 			off: disable AGP support
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -32,6 +32,7 @@
 #include <linux/errno.h>
 #include <linux/acpi.h>
 #include <linux/bootmem.h>
+#include <linux/dmi.h>
 
 #define PREFIX			"ACPI: "
 
@@ -282,6 +283,37 @@ static void __init check_multiple_madt(v
 	return;
 }
 
+static struct dmi_system_id __initdata acpi_rsdt_dmi_table[] = {
+	{
+	    .ident = "ThinkPad ", /* R40e, broken C-states */
+	    .matches = {
+		DMI_MATCH(DMI_BIOS_VENDOR, "IBM"),
+		DMI_MATCH(DMI_BIOS_VERSION, "1SET")},
+	},
+	{
+	    .ident = "ThinkPad ", /* R50e, slow booting */
+	    .matches = {
+		DMI_MATCH(DMI_BIOS_VENDOR, "IBM"),
+		DMI_MATCH(DMI_BIOS_VERSION, "1WET")},
+	},
+	{
+	    .ident = "ThinkPad ", /* T40, T40p, T41, T41p, T42, T42p
+				     R50, R50p */
+	    .matches = {
+		DMI_MATCH(DMI_BIOS_VENDOR, "IBM"),
+		DMI_MATCH(DMI_BIOS_VERSION, "1RET")},
+	},
+	{}
+};
+
+static int __init acpi_force_rsdt(char *opt)
+{
+	if (!strcmp(opt, "rsdt"))
+		acpi_gbl_force_rsdt = 1;
+	return 0;
+}
+early_param("acpi_root_table", acpi_force_rsdt);
+
 /*
  * acpi_table_init()
  *
@@ -293,6 +325,11 @@ static void __init check_multiple_madt(v
 
 int __init acpi_table_init(void)
 {
+	if (dmi_check_system(acpi_rsdt_dmi_table))
+		acpi_gbl_force_rsdt = 1;
+	if (acpi_gbl_force_rsdt)
+		printk(KERN_INFO "Using RSDT as ACPI root table\n");
+
 	acpi_initialize_tables(initial_tables, ACPI_MAX_TABLES, 0);
 	check_multiple_madt();
 	return 0;

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Sun, Oct 19, 2008 at 11:50:55PM +0200, Thomas Renninger wrote:
> Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt
> 
> References: http://bugzilla.kernel.org/show_bug.cgi?id=8246
> 
> Blacklist R40e, R51e and T40, T40p, T41, T41p, T42, T42p, R50 and R50p
> ThinkPads to use the RSDT instead of the XSDT.

I'm very sceptical about this. All of these machines were released years 
before Vista, which makes me unsure about the diagnosis that this is 
something to do with Vista readyness.

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

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Henrique de Moraes Holschuh]

On Mon, 20 Oct 2008, Matthew Garrett wrote:
> On Sun, Oct 19, 2008 at 11:50:55PM +0200, Thomas Renninger wrote:
> > Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt
> > 
> > References: http://bugzilla.kernel.org/show_bug.cgi?id=8246
> > 
> > Blacklist R40e, R51e and T40, T40p, T41, T41p, T42, T42p, R50 and R50p
> > ThinkPads to use the RSDT instead of the XSDT.
> 
> I'm very sceptical about this. All of these machines were released years 
> before Vista, which makes me unsure about the diagnosis that this is 
> something to do with Vista readyness.

I believe you will find stuff broken due to Vista Readyness badly
retro-fitted only on the Z-series and also the T60.

The R51e (BIOS TP-78) does NOT share a BIOS with the T40-T42 or any of the
two R51 base models. The bugzilla bug report really cannot be used against
them.  Thomas, do you have extra data not in the bug report that causes you
to doubt the TP-1R BIOS?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Henrique de Moraes Holschuh]

On Sun, 19 Oct 2008, Thomas Renninger wrote:
> The rest share one BIOS and have been added in hope that this bug gets solved,
> but this is not proven:
>   T41p shutting down due to "temperature critical"
>   https://bugzilla.novell.com/show_bug.cgi?id=378327
> I saw a kind of fixed temperature offset with the exact same machine models
> and exact same BIOS. Short after cold boot the one machine showed about 15
> degree C more than the other. This looked like wrong, too high temperature
> values read from EC on the one machine. I never found out why, this could
> explain and fix it, but I did not double check.

How is using the RSDT instead of XSDT going to help with that?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Thomas Renninger]

On Monday 20 October 2008 17:31:53 Henrique de Moraes Holschuh wrote:
> On Mon, 20 Oct 2008, Matthew Garrett wrote:
> > On Sun, Oct 19, 2008 at 11:50:55PM +0200, Thomas Renninger wrote:
> > > Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt
> > >
> > > References: http://bugzilla.kernel.org/show_bug.cgi?id=8246
> > >
> > > Blacklist R40e, R51e and T40, T40p, T41, T41p, T42, T42p, R50 and R50p
> > > ThinkPads to use the RSDT instead of the XSDT.
> >
> > I'm very sceptical about this. All of these machines were released years
> > before Vista, which makes me unsure about the diagnosis that this is
> > something to do with Vista readyness.
>
> I believe you will find stuff broken due to Vista Readyness badly
> retro-fitted only on the Z-series and also the T60.
>
> The R51e (BIOS TP-78) does NOT share a BIOS with the T40-T42 or any of the
> two R51 base models. The bugzilla bug report really cannot be used against
> them.  Thomas, do you have extra data not in the bug report that causes you
> to doubt the TP-1R BIOS?

Len, can you remove this part of the dmi entry then:
+       {
+           .ident = "ThinkPad ", /* T40, T40p, T41, T41p, T42, T42p
+                                    R50, R50p */
+           .matches = {
+               DMI_MATCH(DMI_BIOS_VENDOR, "IBM"),
+               DMI_MATCH(DMI_BIOS_VERSION, "1RET")},
+       },

The others (these BIOSes seem to not be shared between that many machines) 
were tested that this patchset fixes sever bugs:
  - system freeze when using C-states on R40e
  - much faster booting on R50e

Thanks,

     Thomas

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Mon, Oct 20, 2008 at 06:23:27PM +0200, Thomas Renninger wrote:

> The others (these BIOSes seem to not be shared between that many machines) 
> were tested that this patchset fixes sever bugs:
>   - system freeze when using C-states on R40e
>   - much faster booting on R50e

I'm still not happy with just DMIing this. If Windows is behaving 
differently in terms of choosing which values to use, we should be 
duplicating that behaviour - there may well be other affected systems. 
Working around it for these specific machines just means it'll take 
longer to fix properly. Thomas, can Lenovo provide any information about 
why this hardware works in Windows?

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

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Thomas Renninger]

On Monday 20 October 2008 03:01:42 Matthew Garrett wrote:
> On Sun, Oct 19, 2008 at 11:50:55PM +0200, Thomas Renninger wrote:
> > Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt
> >
> > References: http://bugzilla.kernel.org/show_bug.cgi?id=8246
> >
> > Blacklist R40e, R51e and T40, T40p, T41, T41p, T42, T42p, R50 and R50p
> > ThinkPads to use the RSDT instead of the XSDT.
>
> I'm very sceptical about this. All of these machines were released years
> before Vista, which makes me unsure about the diagnosis that this is
> something to do with Vista readyness.

I sent you a link to BIOS updates where the last changelog entry stated VISTA 
support. Hmm, I double checked R40e and R50e and there is nothing about VISTA 
support.
Nevertheless, this would mean the addresses used by Linux are not supported.

> I'm very sceptical about this
Could you please explain why? Or better suggest another solution if you have a 
better one.
XP uses the 32 bit adresses. Vista the 64 bit adresses.
For some reason, the 64 bit addresses are not 0 as demanded by the spec if 
they should get ignored, therefore Linux takes them. There is no way of 
automated detection, because all this happens before the DSDT is parsed.

Could you please point to a specific problem you have with the patches, these 
are safe, tested and verified fixes.

     Thomas

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Thomas Renninger]

On Monday 20 October 2008 18:27:32 Matthew Garrett wrote:
> On Mon, Oct 20, 2008 at 06:23:27PM +0200, Thomas Renninger wrote:
> > The others (these BIOSes seem to not be shared between that many
> > machines) were tested that this patchset fixes sever bugs:
> >   - system freeze when using C-states on R40e
> >   - much faster booting on R50e
>
> I'm still not happy with just DMIing this. If Windows is behaving
> differently in terms of choosing which values to use, we should be
> duplicating that behaviour - there may well be other affected systems.
> Working around it for these specific machines just means it'll take
> longer to fix properly. Thomas, can Lenovo provide any information about
> why this hardware works in Windows?

As said, probably because Vista is not supported at all or not showing the 
symptoms.
There is no other way than blacklisting.
If you know one, please tell us or these will stay broken for ever.
> I'm still not happy...
does not really help.
It's also very unlikely other machines show that. It looks like they started 
Vista support and left it half broken. If others show that, they now at least 
have a boot param to test if it helps.

     Thomas

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Mon, Oct 20, 2008 at 06:48:41PM +0200, Thomas Renninger wrote:

> As said, probably because Vista is not supported at all or not showing the 
> symptoms.

But *why* is Vista not showing the symptoms?

> It's also very unlikely other machines show that. It looks like they started 
> Vista support and left it half broken. If others show that, they now at least 
> have a boot param to test if it helps.

The R40e was released in 2003. The idea that it has initial Vista 
support is just horrifically unlikely. The issue is that the invalid 
values in the table were never caught because, for some reason, Windows 
never used them. The bug is that we're not using the same heuristic as 
Windows. Applying a static list of hardware just results in it being 
likely that we'll never fix that bug.

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

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Henrique de Moraes Holschuh]

On Mon, 20 Oct 2008, Matthew Garrett wrote:
> On Mon, Oct 20, 2008 at 06:48:41PM +0200, Thomas Renninger wrote:
> > As said, probably because Vista is not supported at all or not showing the 
> > symptoms.
> 
> But *why* is Vista not showing the symptoms?

I very much doubt anyone ever attempted to run Vista on a R40e or a R50e.
It probably won't work well on the T4x/R51/R52 either, but I haven't tried.

> values in the table were never caught because, for some reason, Windows 
> never used them. The bug is that we're not using the same heuristic as 
> Windows. Applying a static list of hardware just results in it being 
> likely that we'll never fix that bug.

I am a bit lost, now.  Are you talking about newer ThinkPads with
Vista-capable BIOSes, or the older ones that can't handle it and thus are
only subject to some ACPI access patterns when running Linux?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Mon, Oct 20, 2008 at 03:51:24PM -0200, Henrique de Moraes Holschuh wrote:
> On Mon, 20 Oct 2008, Matthew Garrett wrote:
> > But *why* is Vista not showing the symptoms?
> 
> I very much doubt anyone ever attempted to run Vista on a R40e or a R50e.
> It probably won't work well on the T4x/R51/R52 either, but I haven't tried.

I'd be surprised if nobody has done it. The hardware is above the 
minimum spec.

> > values in the table were never caught because, for some reason, Windows 
> > never used them. The bug is that we're not using the same heuristic as 
> > Windows. Applying a static list of hardware just results in it being 
> > likely that we'll never fix that bug.
> 
> I am a bit lost, now.  Are you talking about newer ThinkPads with
> Vista-capable BIOSes, or the older ones that can't handle it and thus are
> only subject to some ACPI access patterns when running Linux?

The R40e and R50e have differing values in the RSDT and XSDT tables. 
Linux uses the values from the XSDT, which apparently results in a 
system hang whenever you attempt to perform C-state manipulation. Using 
the RSDT values instead avoids this. Windows works fine, so is clearly 
using the RSDT values. However, we have no knowledge of what mechanism 
Windows uses to choose between the two sets of values.

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

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Henrique de Moraes Holschuh]

On Mon, 20 Oct 2008, Thomas Renninger wrote:
> On Monday 20 October 2008 03:01:42 Matthew Garrett wrote:
> > On Sun, Oct 19, 2008 at 11:50:55PM +0200, Thomas Renninger wrote:
> > > Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt
> > >
> > > References: http://bugzilla.kernel.org/show_bug.cgi?id=8246
> > >
> > > Blacklist R40e, R51e and T40, T40p, T41, T41p, T42, T42p, R50 and R50p
> > > ThinkPads to use the RSDT instead of the XSDT.
> >
> > I'm very sceptical about this. All of these machines were released years
> > before Vista, which makes me unsure about the diagnosis that this is
> > something to do with Vista readyness.
> 
> I sent you a link to BIOS updates where the last changelog entry stated VISTA 
> support. Hmm, I double checked R40e and R50e and there is nothing about VISTA 
> support.
> Nevertheless, this would mean the addresses used by Linux are not supported.

Correct.  They're not, and latent ACPI BIOS bugs make this painful for Linux
(and probably would cause trouble for a modern Windows if you even could
install it on those boxes).

So, it might well be that issues don't show up on Windows in the R40e and
R50e because it is an *OLD* version of Windows that just plain doesn't touch
the broken stuff, AFAIK.  Heck, I am not sure if the R40e even can run XP,
let alone Vista...

> XP uses the 32 bit adresses. Vista the 64 bit adresses.

Don't touch 64bit addresses on ACPI BIOSes that are too old to support Vista
is about the only heuristic I can imagine that would work, here.  But that's
likely going to cover too many machines that didn't need the quirk.

> Could you please point to a specific problem you have with the patches, these 
> are safe, tested and verified fixes.

Well, *I* don't have a problem with them now that you removed the T4x/R51
from the DMI list.  BTW: it could very well be that the T4x/R51 needs to be
in there, but it should be tested directly, as they use a different BIOS.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Henrique de Moraes Holschuh]

On Mon, 20 Oct 2008, Matthew Garrett wrote:
> On Mon, Oct 20, 2008 at 03:51:24PM -0200, Henrique de Moraes Holschuh wrote:
> > On Mon, 20 Oct 2008, Matthew Garrett wrote:
> > > But *why* is Vista not showing the symptoms?
> > 
> > I very much doubt anyone ever attempted to run Vista on a R40e or a R50e.
> > It probably won't work well on the T4x/R51/R52 either, but I haven't tried.
> 
> I'd be surprised if nobody has done it. The hardware is above the 
> minimum spec.

I'd ask someone that did before I assume it worked fine (i.e. "does it
suspend and resume to RAM properly when running XP?  when running
Vista?")...

The R40e is **OLD**.  The R50e is not that newer, either.

> > > values in the table were never caught because, for some reason, Windows 
> > > never used them. The bug is that we're not using the same heuristic as 
> > > Windows. Applying a static list of hardware just results in it being 
> > > likely that we'll never fix that bug.
> > 
> > I am a bit lost, now.  Are you talking about newer ThinkPads with
> > Vista-capable BIOSes, or the older ones that can't handle it and thus are
> > only subject to some ACPI access patterns when running Linux?
> 
> The R40e and R50e have differing values in the RSDT and XSDT tables. 
> Linux uses the values from the XSDT, which apparently results in a 
> system hang whenever you attempt to perform C-state manipulation. Using 
> the RSDT values instead avoids this. Windows works fine, so is clearly 
> using the RSDT values. However, we have no knowledge of what mechanism 
> Windows uses to choose between the two sets of values.

Err... WHICH versions of Windows "work fine"?  And HOW do those versions
behave?

One cannot assume "Vista" for "Windows" on such old boxes.  You need
explicit confirmation that an user ran Vista and tried suspend/resume,
really.  Because nobody would be surprised if Vista crashed hard on a R50e,
let alone if it worked moderately right but couldn't survive a
suspend/resume cycle.  It wouldn't be "newsworthy".

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Mon, Oct 20, 2008 at 04:16:22PM -0200, Henrique de Moraes Holschuh wrote:
> On Mon, 20 Oct 2008, Matthew Garrett wrote:
> > I'd be surprised if nobody has done it. The hardware is above the 
> > minimum spec.
> 
> I'd ask someone that did before I assume it worked fine (i.e. "does it
> suspend and resume to RAM properly when running XP?  when running
> Vista?")...

C state transitions, not S state. Working properly here means not 
hanging the moment ACPI processor power management starts.

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

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Henrique de Moraes Holschuh]

On Mon, 20 Oct 2008, Matthew Garrett wrote:
> On Mon, Oct 20, 2008 at 04:16:22PM -0200, Henrique de Moraes Holschuh wrote:
> > On Mon, 20 Oct 2008, Matthew Garrett wrote:
> > > I'd be surprised if nobody has done it. The hardware is above the 
> > > minimum spec.
> > 
> > I'd ask someone that did before I assume it worked fine (i.e. "does it
> > suspend and resume to RAM properly when running XP?  when running
> > Vista?")...
> 
> C state transitions, not S state. Working properly here means not 
> hanging the moment ACPI processor power management starts.

I see, but my point remains.  Does a version of Windows that was tested to
do C-state transitions on the R50e and R40e even use the XSDT?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Mon, Oct 20, 2008 at 04:47:54PM -0200, Henrique de Moraes Holschuh wrote:
> On Mon, 20 Oct 2008, Matthew Garrett wrote:
> > C state transitions, not S state. Working properly here means not 
> > hanging the moment ACPI processor power management starts.
> 
> I see, but my point remains.  Does a version of Windows that was tested to
> do C-state transitions on the R50e and R40e even use the XSDT?

Vista is known to run on the R50e. See 
http://forums.lenovo.com/lnv/board/message?board.id=R_Series_Thinkpads&thread.id=3467 
, for instance.
-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Henrique de Moraes Holschuh]

On Mon, 20 Oct 2008, Matthew Garrett wrote:
> On Mon, Oct 20, 2008 at 04:47:54PM -0200, Henrique de Moraes Holschuh wrote:
> > On Mon, 20 Oct 2008, Matthew Garrett wrote:
> > > C state transitions, not S state. Working properly here means not 
> > > hanging the moment ACPI processor power management starts.
> > 
> > I see, but my point remains.  Does a version of Windows that was tested to
> > do C-state transitions on the R50e and R40e even use the XSDT?
> 
> Vista is known to run on the R50e. See 
> http://forums.lenovo.com/lnv/board/message?board.id=R_Series_Thinkpads&thread.id=3467 
> , for instance.

Ok, now one has to wonder whether C-states work, but still the chances that
we are doing something differently than Windows XP/Vista here have gone up a
great deal in my book :-(

If you can describe how to test C-states in Windows, one could try to find
an R50e user to do that test, either in the linux-thinkpad ML, or in the
ThinkPad forums.

I still think a date cutoff line would explain a lot.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Thomas Renninger]

On Monday 20 October 2008 20:52:41 Matthew Garrett wrote:
> On Mon, Oct 20, 2008 at 04:47:54PM -0200, Henrique de Moraes Holschuh wrote:
> > On Mon, 20 Oct 2008, Matthew Garrett wrote:
> > > C state transitions, not S state. Working properly here means not
> > > hanging the moment ACPI processor power management starts.
> >
> > I see, but my point remains.  Does a version of Windows that was tested
> > to do C-state transitions on the R50e and R40e even use the XSDT?
>
> Vista is known to run on the R50e. See
> http://forums.lenovo.com/lnv/board/message?board.id=R_Series_Thinkpads&thre
>ad.id=3467 , for instance.

The R50e also works on Linux. See:
http://bugzilla.kernel.org/show_bug.cgi?id=8246
Problem Description: Since kernel 2.6.20 the acpi modules take an unacceptable
time to load.

It is the R40e which cannot do C-state switching.

Both were verified to work with using 32 bit values and both are broken with 
64 bit addresses.

    Thomas

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Thomas Renninger]

On Monday 20 October 2008 20:52:41 Matthew Garrett wrote:
> On Mon, Oct 20, 2008 at 04:47:54PM -0200, Henrique de Moraes Holschuh wrote:
> > On Mon, 20 Oct 2008, Matthew Garrett wrote:
> > > C state transitions, not S state. Working properly here means not
> > > hanging the moment ACPI processor power management starts.
> >
> > I see, but my point remains.  Does a version of Windows that was tested
> > to do C-state transitions on the R50e and R40e even use the XSDT?
>
> Vista is known to run on the R50e. See
> http://forums.lenovo.com/lnv/board/message?board.id=R_Series_Thinkpads&thre
>ad.id=3467 , for instance.

Linux is also known to run with the R50e:
Hardware Environment: IBM Thinkpad R51e
Problem Description: Since kernel 2.6.20 the acpi modules take an unacceptable
time to load. 

Later:
Ok I can confirm this patch works. Finally after using 2.6.22 for ages i can
update. Thank you all for your effort.

The C-state problem which gets fixed by these is the R40e.

    Thomas

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Thomas Renninger]

On Monday 20 October 2008 19:58:14 Matthew Garrett wrote:
> On Mon, Oct 20, 2008 at 03:51:24PM -0200, Henrique de Moraes Holschuh wrote:
> > On Mon, 20 Oct 2008, Matthew Garrett wrote:
> > > But *why* is Vista not showing the symptoms?
> >
> > I very much doubt anyone ever attempted to run Vista on a R40e or a R50e.
> > It probably won't work well on the T4x/R51/R52 either, but I haven't
> > tried.
>
> I'd be surprised if nobody has done it. The hardware is above the
> minimum spec.
>
> > > values in the table were never caught because, for some reason, Windows
> > > never used them. The bug is that we're not using the same heuristic as
> > > Windows. Applying a static list of hardware just results in it being
> > > likely that we'll never fix that bug.
> >
> > I am a bit lost, now.  Are you talking about newer ThinkPads with
> > Vista-capable BIOSes, or the older ones that can't handle it and thus are
> > only subject to some ACPI access patterns when running Linux?
>
> The R40e and R50e have differing values in the RSDT and XSDT tables.
> Linux uses the values from the XSDT, which apparently results in a
> system hang whenever you attempt to perform C-state manipulation. Using
> the RSDT values instead avoids this. Windows works fine, so is clearly
> using the RSDT values. However, we have no knowledge of what mechanism
> Windows uses to choose between the two sets of values.

Yes we have:
 - XP uses 32 bit addresses
 - Vista uses the newer FADT revision and there the 64 bit addresses.

This information is from BIOS engineers who should know Windows behaviour much 
better than we ever could.

You may still want to install Windows and do some tests, I won't.
Until you find a better solution (I really tried and I doubt there is one), 
please don't block these patches again.

The problem is known for a year now, the patches are posted the third time and 
still the machines are broken mainline, because some people don't like the 
blacklist, want to do it more generic, but have no idea how to do that. Maybe 
you have, then please speak up.

The problem you try to solve is:
Can it be detected whether the system supports Vista at FADT parse time.
Do you have a solution or at least an idea for that?
Do you have another (unintrusive for old machines) idea?

IMO:
  - This is not possible.
  - This shouldn't be done anyway. You would change the behaviour of all
    old machines (Be aware that there may be server BIOSes which
    explicitly did it the right thing for Linux, not caring about Windows XP).
    All this for two BIOSes for which Lenovo admits that they are broken.

The right thing to do (at least for now) is to blacklist these two broken 
BIOSes.
If there really should be other machines having such an issue you now have the 
boot param to test easily, but I doubt you find more machines with this 
problem.

Thanks,

      Thomas

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Thomas Renninger]

On Monday 20 October 2008 17:46:28 Henrique de Moraes Holschuh wrote:
> On Sun, 19 Oct 2008, Thomas Renninger wrote:
> > The rest share one BIOS and have been added in hope that this bug gets
> > solved, but this is not proven:
> >   T41p shutting down due to "temperature critical"
> >   https://bugzilla.novell.com/show_bug.cgi?id=378327
> > I saw a kind of fixed temperature offset with the exact same machine
> > models and exact same BIOS. Short after cold boot the one machine showed
> > about 15 degree C more than the other. This looked like wrong, too high
> > temperature values read from EC on the one machine. I never found out
> > why, this could explain and fix it, but I did not double check.
>
> How is using the RSDT instead of XSDT going to help with that?

RSDT -> FADT (early rev)
XSDT -> other FADT (later rev)

Broken IO addresses are taken, this can theoretically cause everything.
E.g. the slow boot of the R50e caused by wrong IO FADT adresses
is related to the EC:
http://bugzilla.kernel.org/show_bug.cgi?id=8246#c26
Ok I tested 2.6.20.3 with ec_intr=0 and it works fine.

But as said, this is not proven for the temperature problem for the non R40e 
and non R50e. I added them, because I know that the 32 bit address for those 
is used on XP and it's safe.
As soon as these patchseries is in, people can easily try things out using the 
boot param.

   Thomas

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Tue, Oct 21, 2008 at 11:57:44AM +0200, Thomas Renninger wrote:
> Yes we have:
>  - XP uses 32 bit addresses
>  - Vista uses the newer FADT revision and there the 64 bit addresses.
> 
> This information is from BIOS engineers who should know Windows behaviour much 
> better than we ever could.

If Vista is absolutely known to *always* use the 64-bit addresses under 
every single circumstance where they're present, then yes, I agree that 
this is probably the least bad situation. This is trivial to test - 
install Vista on an R40e and see if it hangs on boot. If it does, I 
withdraw my objection. If not, then we're not fixing it properly.

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

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Thomas Renninger]

On Tuesday 21 October 2008 14:46:04 Matthew Garrett wrote:
> On Tue, Oct 21, 2008 at 11:57:44AM +0200, Thomas Renninger wrote:
> > Yes we have:
> >  - XP uses 32 bit addresses
> >  - Vista uses the newer FADT revision and there the 64 bit addresses.
> >
> > This information is from BIOS engineers who should know Windows behaviour
> > much better than we ever could.
>
> If Vista is absolutely known to *always* use the 64-bit addresses under
> every single circumstance where they're present, then yes, I agree that
> this is probably the least bad situation. This is trivial to test -
> install Vista on an R40e and see if it hangs on boot. If it does, I
> withdraw my objection. If not, then we're not fixing it properly.

a) I don't have a R40e
b) I don't have a Vista
c) I wouldn't try anyway. I spend enough time on this one and I trust the
   people who told me Vista is using the 64 bit addresses
d) The commit Window is closing

If you have the time and knowledge to verify:
> If Vista is absolutely known to *always* use the 64-bit addresses under
> every single circumstance where they're present, then yes I agree that ...
then pls, go ahead.
Even if this is not proven, the patchset should still go in to get the 
machines fixed for now and to easily test others, there was enough discussion 
around it.

Thanks,

    Thomas

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Tue, Oct 21, 2008 at 03:05:33PM +0200, Thomas Renninger wrote:

> Even if this is not proven, the patchset should still go in to get the 
> machines fixed for now and to easily test others, there was enough discussion 
> around it.

What do you mean, "fixed"? They all already work, even if it's in a 
suboptimal way.

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

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Thomas Renninger]

On Tuesday 21 October 2008 15:08:36 Matthew Garrett wrote:
> On Tue, Oct 21, 2008 at 03:05:33PM +0200, Thomas Renninger wrote:
> > Even if this is not proven, the patchset should still go in to get the
> > machines fixed for now and to easily test others, there was enough
> > discussion around it.
>
> What do you mean, "fixed"? They all already work, even if it's in a
> suboptimal way.

If C-states don't work and do work with a patch. I call the patch a fix.
I also call the machine broken (even if it does boot) before and fixed 
afterwards.
I also call a machine broken which takes minutes to boot and fixed as soon as 
the problem is solved and the machine is booting in a reasonable amount of 
time.

   Thomas

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Tue, Oct 21, 2008 at 04:01:55PM +0200, Rafael J. Wysocki wrote:

> Moreover, as soon as the real fix is developed we can remove the DMI list.
> Quite frankly, I don't see any downsides.

The downside is that with the DMI list there's significantly less 
incentive to produce a "real" fix (assuming there is one). We've no idea 
how many other systems may be affected in one way or another.

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

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Rafael J. Wysocki]

On Tuesday, 21 of October 2008, Thomas Renninger wrote:
> On Tuesday 21 October 2008 15:08:36 Matthew Garrett wrote:
> > On Tue, Oct 21, 2008 at 03:05:33PM +0200, Thomas Renninger wrote:
> > > Even if this is not proven, the patchset should still go in to get the
> > > machines fixed for now and to easily test others, there was enough
> > > discussion around it.
> >
> > What do you mean, "fixed"? They all already work, even if it's in a
> > suboptimal way.
> 
> If C-states don't work and do work with a patch. I call the patch a fix.
> I also call the machine broken (even if it does boot) before and fixed 
> afterwards.

If not a fix, this certainly is improvement.

> I also call a machine broken which takes minutes to boot and fixed as soon as 
> the problem is solved and the machine is booting in a reasonable amount of 
> time.

I haven't followed the discussion closely, but IMO it's better to have the
machines in a usable state right now than wait for a "real fix" forever.

Moreover, as soon as the real fix is developed we can remove the DMI list.
Quite frankly, I don't see any downsides.

Thanks,
Rafael

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Thomas Renninger]

On Tuesday 21 October 2008 16:00:14 Matthew Garrett wrote:
> On Tue, Oct 21, 2008 at 04:01:55PM +0200, Rafael J. Wysocki wrote:
> > Moreover, as soon as the real fix is developed we can remove the DMI
> > list. Quite frankly, I don't see any downsides.
>
> The downside is that with the DMI list there's significantly less
> incentive to produce a "real" fix (assuming there is one). We've no idea
> how many other systems may be affected in one way or another.

With the boot param you at least get an idea how many other sytems are 
affected.

If you run the linuxfirmwarekit, AFAIK there is a test: it extracts both FADTs 
and compares the 32 and 64 bit addresses in case both are not 0 and throws a 
warning it they differ.
If people see this and they have some mysterious problem, they should give 
this: acpi_root_table=rsdt boot param a try (after applying the patches) and 
report back.

Thanks,

    Thomas

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Tue, Oct 21, 2008 at 04:12:14PM +0200, Thomas Renninger wrote:
> On Tuesday 21 October 2008 16:00:14 Matthew Garrett wrote:
> > The downside is that with the DMI list there's significantly less
> > incentive to produce a "real" fix (assuming there is one). We've no idea
> > how many other systems may be affected in one way or another.
> 
> With the boot param you at least get an idea how many other sytems are 
> affected.

No, not really. Our bug reporting underrepresents reality in the general 
case. We have no idea how many subtle issues on other hardware may be 
caused by this kind of BIOS breakage. And the sheer number of people 
still running with pci=noacpi irqpoll and the like indicates that many 
users will just run with boot options rather than ever reporting the 
issue that caused them to.

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

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Rafael J. Wysocki]

On Tuesday, 21 of October 2008, Matthew Garrett wrote:
> On Tue, Oct 21, 2008 at 04:01:55PM +0200, Rafael J. Wysocki wrote:
> 
> > Moreover, as soon as the real fix is developed we can remove the DMI list.
> > Quite frankly, I don't see any downsides.
> 
> The downside is that with the DMI list there's significantly less 
> incentive to produce a "real" fix (assuming there is one).

I really don't think it works this way.

As long as there only are a few machines in the list, the incentive is very
small in either case.  However, as the number of machines in the list grows, it
will produce pressure to fix things.  [That's exactly what happened in the
case of the Turion-based HP BIOS issue.] 

> We've no idea how many other systems may be affected in one way or another.

Yes, and the list may help us to get an idea IMO.

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Tue, Oct 21, 2008 at 04:25:46PM +0200, Rafael J. Wysocki wrote:
> On Tuesday, 21 of October 2008, Matthew Garrett wrote:
> > We've no idea how many other systems may be affected in one way or another.
> 
> Yes, and the list may help us to get an idea IMO.

How? We *know* we're deviating from the behaviour of Windows here. What 
we don't know is how that will affect different machines. I suspect 
we'll end up with a bunch of "Well, I added this boot option and then my 
system booted slightly faster" and have no ability to work out whether 
the problem's actually related. See the number of people who reported 
that acpi_apic_instance made a difference, or even the fact that Thomas 
included a bunch of systems with no real assurance that they were hit by 
this.

A static list will eventually end up either filled with false positives 
or missing several machines that should be there.
-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Thomas Renninger]

On Tuesday 21 October 2008 16:29:56 Matthew Garrett wrote:
> On Tue, Oct 21, 2008 at 04:25:46PM +0200, Rafael J. Wysocki wrote:
> > On Tuesday, 21 of October 2008, Matthew Garrett wrote:
> > > We've no idea how many other systems may be affected in one way or
> > > another.
> >
> > Yes, and the list may help us to get an idea IMO.
>
> How? We *know* we're deviating from the behaviour of Windows here.
Matthew, there is no solution to stick to the Windows behaviour.
Even if hundreds of machines are affected, you have still not yet come up with 
a generic solution, but try to block the fix.
> What 
> we don't know is how that will affect different machines.
> we'll end up with a bunch of "Well, I added this boot option and then my
> system booted slightly faster" and have no ability to work out whether
> the problem's actually related. See the number of people who reported
> that acpi_apic_instance made a difference, or even the fact that Thomas
> included a bunch of systems with no real assurance that they were hit by
> this.
This is simply a fact of missing resources.
It cost me hours to debug it that far down and I still have to argue getting 
it finally fixed, even there is no alternative (presented yet) (there is no 
generic way to fix it). And yes the bugs are anoying, but not critical so I 
will probably not look at it any further.

> A static list will eventually end up either filled with false positives
> or missing several machines that should be there.

This is a problem of two broken ThinkPad BIOSes.
Go ahead and install some Windowses to prove the opposite. Until you do (and 
then still have no solution for a generic fix), pls do not further try to 
block getting this fixed.

   Thomas

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Tue, Oct 21, 2008 at 04:45:44PM +0200, Thomas Renninger wrote:
> On Tuesday 21 October 2008 16:29:56 Matthew Garrett wrote:
> > How? We *know* we're deviating from the behaviour of Windows here.
> Matthew, there is no solution to stick to the Windows behaviour.
> Even if hundreds of machines are affected, you have still not yet come up with 
> a generic solution, but try to block the fix.

You're the one asserting that Windows behaves in a certain way. I'm 
asking you to justify that assertion. The information from that test 
allows us to determine whether a static table is the best we can do or 
not.

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

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Rafael J. Wysocki]

On Tuesday, 21 of October 2008, Matthew Garrett wrote:
> On Tue, Oct 21, 2008 at 04:25:46PM +0200, Rafael J. Wysocki wrote:
> > On Tuesday, 21 of October 2008, Matthew Garrett wrote:
> > > We've no idea how many other systems may be affected in one way or another.
> > 
> > Yes, and the list may help us to get an idea IMO.
> 
> How? We *know* we're deviating from the behaviour of Windows here. What 
> we don't know is how that will affect different machines. I suspect 
> we'll end up with a bunch of "Well, I added this boot option and then my 
> system booted slightly faster" and have no ability to work out whether 
> the problem's actually related.

Well, this is similar to suspend problems where many different issues may
give the same symptom.  In these cases we also often have very limited
possibility to figure out why some particular workaround actually works on
given machine type, but with no access to the machine and with a bug reporter
who can't compile the kernel himself all we can do is to verify that it sort of
works.  By putting the machine into a blacklist we can at least make Linux more
usable to the user in question, which also is important.

Technically, this doesn't _fix_ the problem, but it vastly improves user
experience.

> See the number of people who reported that acpi_apic_instance made a
> difference, or even the fact that Thomas included a bunch of systems with no
> real assurance that they were hit by this.

Hm, this is not a good thing.  Is there any reliable way to verify that?

> A static list will eventually end up either filled with false positives 
> or missing several machines that should be there.

While there may be false positives and missing items in the list, at least we
can learn from it which machines _may_ be affected and what's the number of
such machines (the order of magnitude).

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Tue, Oct 21, 2008 at 05:10:31PM +0200, Rafael J. Wysocki wrote:
> On Tuesday, 21 of October 2008, Matthew Garrett wrote:
> > How? We *know* we're deviating from the behaviour of Windows here. What 
> > we don't know is how that will affect different machines. I suspect 
> > we'll end up with a bunch of "Well, I added this boot option and then my 
> > system booted slightly faster" and have no ability to work out whether 
> > the problem's actually related.
> 
> Well, this is similar to suspend problems where many different issues may
> give the same symptom.  In these cases we also often have very limited
> possibility to figure out why some particular workaround actually works on
> given machine type, but with no access to the machine and with a bug reporter
> who can't compile the kernel himself all we can do is to verify that it sort of
> works.  By putting the machine into a blacklist we can at least make Linux more
> usable to the user in question, which also is important.

Right, but in this case we understand the root cause of the problem - we 
behave differently to Windows in a very specific way. What we don't 
understand are the precise circumstances in which Windows behaves that 
way. It could be that Vista always uses the 64-bit addresses if 
available, and in that case this is the best possible solution. But it 
could also be that Vista swaps addresses based on whether _INI requests 
the Vista OSI or not. It could be that Vista uses the 32-bit addresses 
on 32-bit CPUs. Or where the addresses differ and there's a valid 32-bit 
entry, perhaps they use that. When there's a simple test to perform, we 
should do that before adding a static list.

> > See the number of people who reported that acpi_apic_instance made a
> > difference, or even the fact that Thomas included a bunch of systems with no
> > real assurance that they were hit by this.
> 
> Hm, this is not a good thing.  Is there any reliable way to verify that?

We can verify whether the addresses are actually different, but it's 
possible that that's harmless on some machines. However, history 
suggests that there's a placebo effect in adding boot options...

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

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Henrique de Moraes Holschuh]

On Tue, 21 Oct 2008, Matthew Garrett wrote:
> Right, but in this case we understand the root cause of the problem - we 
> behave differently to Windows in a very specific way. What we don't 
> understand are the precise circumstances in which Windows behaves that 
> way. It could be that Vista always uses the 64-bit addresses if 
> available, and in that case this is the best possible solution. But it 
> could also be that Vista swaps addresses based on whether _INI requests 
> the Vista OSI or not. It could be that Vista uses the 32-bit addresses 
> on 32-bit CPUs. Or where the addresses differ and there's a valid 32-bit 
> entry, perhaps they use that. When there's a simple test to perform, we 
> should do that before adding a static list.
> 
> > > See the number of people who reported that acpi_apic_instance made a
> > > difference, or even the fact that Thomas included a bunch of systems with no
> > > real assurance that they were hit by this.
> > 
> > Hm, this is not a good thing.  Is there any reliable way to verify that?
> 
> We can verify whether the addresses are actually different, but it's 
> possible that that's harmless on some machines. However, history 
> suggests that there's a placebo effect in adding boot options...

This is all nice and true, HOWEVER apparently all of us lack the hardware
and lab setup to perform the testing.   And I haven't seen anyone publish
clear instructions we could ask a Vista user to do, either.

So, can we please commit these patches, with the T4x BIOSes removed from the
DMI list already?  We already know the R40e and R50e are borked, we have a
patch that fixes them *cleanly* and completely safely (no side-effects on
any other box).  There is no excuse not to apply it since we do NOT have
anything better on hand, NOR are we going to be able to produce better
patches anytime soon.

If someone manages to set up a lab to do testing, or manages to ask someone
else who has deep knowledge of how Microsoft made Vista behave on border
conditions, he can send us better information, and a general solution can be
deployed and the band-aid DMI table, removed.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Tue, Oct 21, 2008 at 01:46:15PM -0200, Henrique de Moraes Holschuh wrote:

> This is all nice and true, HOWEVER apparently all of us lack the hardware
> and lab setup to perform the testing.   And I haven't seen anyone publish
> clear instructions we could ask a Vista user to do, either.

Boot Vista on R40e. If machine hangs after ACPI is brought up, Vista 
uses 64-bit addresses under all circumstances.
-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [ltp] Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Henrique de Moraes Holschuh]

On Tue, 21 Oct 2008, Matthew Garrett wrote:
> On Tue, Oct 21, 2008 at 01:46:15PM -0200, Henrique de Moraes Holschuh wrote:
> > This is all nice and true, HOWEVER apparently all of us lack the hardware
> > and lab setup to perform the testing.   And I haven't seen anyone publish
> > clear instructions we could ask a Vista user to do, either.
> 
> Boot Vista on R40e. If machine hangs after ACPI is brought up, Vista 
> uses 64-bit addresses under all circumstances.

Is it hanging on Linux?

Anyway, I just asked if anyone managed to install Vista on the R40e in the
thinkpad forums.  Let's hope some kind soul with a R40e will reply.  But I
am against holding these patches any further.  If we find a better way to do
things in the future, we deal with it then.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [ltp] Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Tue, Oct 21, 2008 at 02:58:04PM -0200, Henrique de Moraes Holschuh wrote:
> On Tue, 21 Oct 2008, Matthew Garrett wrote:
> > Boot Vista on R40e. If machine hangs after ACPI is brought up, Vista 
> > uses 64-bit addresses under all circumstances.
> 
> Is it hanging on Linux?

No, because we disable C-state transitions on them. Otherwise it hangs 
once the ACPI processor module is initialised.

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

Re: [ltp] Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Rafael J. Wysocki]

On Tuesday, 21 of October 2008, Henrique de Moraes Holschuh wrote:
> On Tue, 21 Oct 2008, Matthew Garrett wrote:
> > On Tue, Oct 21, 2008 at 01:46:15PM -0200, Henrique de Moraes Holschuh wrote:
> > > This is all nice and true, HOWEVER apparently all of us lack the hardware
> > > and lab setup to perform the testing.   And I haven't seen anyone publish
> > > clear instructions we could ask a Vista user to do, either.
> > 
> > Boot Vista on R40e. If machine hangs after ACPI is brought up, Vista 
> > uses 64-bit addresses under all circumstances.
> 
> Is it hanging on Linux?
> 
> Anyway, I just asked if anyone managed to install Vista on the R40e in the
> thinkpad forums.  Let's hope some kind soul with a R40e will reply.  But I
> am against holding these patches any further.  If we find a better way to do
> things in the future, we deal with it then.

FWIW, I agree.

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

I've now had confirmation from multiple sources that Vista still uses 
the 32-bit addresses for the GPE blocks. We're actually seeing the same 
bug on some currently shipping machines, so again I'm going to suggest 
that the blacklist model isn't going to scale and we should just behave 
like Windows. How about this patch instead? It does some sanity 
checking, so I doubt that there's any way it could break a legitimate 
system. I've left IA64 as-is because it seems more likely that there'd 
be a requirement for 64-bit setups there.

Signed-off-by: Matthew Garrett <mjg@redhat.com>

---

Are there even any ACPI platforms where a system io address can be 
greater than 32 bits? It's limited to 16 bits on x86, so I *really* 
don't think this is going to break anything. The FADTs I've checked from 
Thinkpads all seem to have valid 32-bit addresses even using the one 
obtained from the XSDT.

diff --git a/drivers/acpi/events/evgpeblk.c b/drivers/acpi/events/evgpeblk.c
index 73c058e..eed35d7 100644
--- a/drivers/acpi/events/evgpeblk.c
+++ b/drivers/acpi/events/evgpeblk.c
@@ -1107,6 +1107,32 @@ acpi_status acpi_ev_gpe_initialize(void)
 	 */
 
 	/*
+	 * The ACPI specification says that we should use the 64-bit
+	 * address offset for the GPE blocks if it exists. However,
+	 * Windows uses the legacy address. Various vendors have left
+	 * incorrect values in the 64-bit field, which then causes
+	 * problems later. Since the vast majority of machines have
+	 * never been tested with an OS that uses the 64-bit value by
+	 * default, we should behave like Windows and ignore the spec
+	 * by only using the 64-bit address if it contains something
+	 * that can't be represented in the legacy field. Since system
+	 * io space is only 16 bits on x86, this should be entirely
+	 * safe.
+	 */
+
+#ifdef CONFIG_X86
+	if (acpi_gbl_FADT.gpe0_block &&
+	    acpi_gbl_FADT.xgpe0_block.space_id == ACPI_ADR_SPACE_SYSTEM_IO)
+		acpi_gbl_FADT.xgpe0_block.address =
+			(u64)acpi_gbl_FADT.gpe0_block;
+
+	if (acpi_gbl_FADT.gpe1_block &&
+	    acpi_gbl_FADT.xgpe1_block.space_id == ACPI_ADR_SPACE_SYSTEM_IO)
+		acpi_gbl_FADT.xgpe1_block.address =
+			(u64)acpi_gbl_FADT.gpe1_block;
+#endif
+
+	/*
 	 * Determine the maximum GPE number for this machine.
 	 *
 	 * Note: both GPE0 and GPE1 are optional, and either can exist without

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

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Thomas Renninger]

On Monday 10 November 2008 06:58:56 pm Matthew Garrett wrote:
> I've now had confirmation from multiple sources that Vista still uses
> the 32-bit addresses for the GPE blocks.
Are you sure that Windows Server implementations also use 32-bit addresses?
Are you sure that upcoming Windows implementations will always use 32-bit 
addresses?
Do all X86 machines support Windows or could there be machines, especially 
servers which only support Mac OS, Solaris/Linux or other OSes which stick to 
the spec which you break with this change?
> We're actually seeing the same 
> bug on some currently shipping machines
Which ones?
> , so again I'm going to suggest 
> that the blacklist model isn't going to scale and we should just behave
> like Windows. How about this patch instead? It does some sanity
> checking, so I doubt that there's any way it could break a legitimate
> system. I've left IA64 as-is because it seems more likely that there'd
> be a requirement for 64-bit setups there.

If at all, this should not be added for .28, but for .29 and stay in 
linux-next for a while.
I wonder why all (hmm, hard to say, at least a lot) recent machines have valid 
64 bit addresses.
I also disagree with violating the spec unconditionally, breaking machines 
which would stick to it. It's likely that machines do not get a latest 
mainline kernel tests. Once this change is in distributions and machines do 
break, people are busted. There should at least be a boot param to switch 
back.

We might come away with it. But I have the strong feeling that there are 
machines running better using 32-bit and machines running better with 64-bit 
addresses used.

       Thomas

> Signed-off-by: Matthew Garrett <mjg@redhat.com>
>
> ---
>
> Are there even any ACPI platforms where a system io address can be
> greater than 32 bits? It's limited to 16 bits on x86, so I *really*
> don't think this is going to break anything. The FADTs I've checked from
> Thinkpads all seem to have valid 32-bit addresses even using the one
> obtained from the XSDT.
>
> diff --git a/drivers/acpi/events/evgpeblk.c
> b/drivers/acpi/events/evgpeblk.c index 73c058e..eed35d7 100644
> --- a/drivers/acpi/events/evgpeblk.c
> +++ b/drivers/acpi/events/evgpeblk.c
> @@ -1107,6 +1107,32 @@ acpi_status acpi_ev_gpe_initialize(void)
>  	 */
>
>  	/*
> +	 * The ACPI specification says that we should use the 64-bit
> +	 * address offset for the GPE blocks if it exists. However,
> +	 * Windows uses the legacy address. Various vendors have left
> +	 * incorrect values in the 64-bit field, which then causes
> +	 * problems later. Since the vast majority of machines have
> +	 * never been tested with an OS that uses the 64-bit value by
> +	 * default, we should behave like Windows and ignore the spec
> +	 * by only using the 64-bit address if it contains something
> +	 * that can't be represented in the legacy field. Since system
> +	 * io space is only 16 bits on x86, this should be entirely
> +	 * safe.
> +	 */
> +
> +#ifdef CONFIG_X86
> +	if (acpi_gbl_FADT.gpe0_block &&
> +	    acpi_gbl_FADT.xgpe0_block.space_id == ACPI_ADR_SPACE_SYSTEM_IO)
> +		acpi_gbl_FADT.xgpe0_block.address =
> +			(u64)acpi_gbl_FADT.gpe0_block;
> +
> +	if (acpi_gbl_FADT.gpe1_block &&
> +	    acpi_gbl_FADT.xgpe1_block.space_id == ACPI_ADR_SPACE_SYSTEM_IO)
> +		acpi_gbl_FADT.xgpe1_block.address =
> +			(u64)acpi_gbl_FADT.gpe1_block;
> +#endif
> +
> +	/*
>  	 * Determine the maximum GPE number for this machine.
>  	 *
>  	 * Note: both GPE0 and GPE1 are optional, and either can exist without

Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Wed, Nov 12, 2008 at 05:58:37PM -0600, Thomas Renninger wrote:
> On Monday 10 November 2008 06:58:56 pm Matthew Garrett wrote:
> > I've now had confirmation from multiple sources that Vista still uses
> > the 32-bit addresses for the GPE blocks.
> Are you sure that Windows Server implementations also use 32-bit addresses?

Yes.

> Are you sure that upcoming Windows implementations will always use 32-bit 
> addresses?

I'm sure that Microsoft will not break widespread hardware, yes.

> Do all X86 machines support Windows or could there be machines, especially 
> servers which only support Mac OS, Solaris/Linux or other OSes which stick to 
> the spec which you break with this change?

Do you have any examples of machines that this would break?

> > We're actually seeing the same 
> > bug on some currently shipping machines
> Which ones?

Various workstations from HP.

> I also disagree with violating the spec unconditionally, breaking machines 
> which would stick to it. It's likely that machines do not get a latest 
> mainline kernel tests. Once this change is in distributions and machines do 
> break, people are busted. There should at least be a boot param to switch 
> back.

*Which* machines would this break? Who would put a good value in the 
64-bit field and a bad one in the 32-bit field? Note that the patch 
checks whether the 32-bit field is unset and whether the 64-bit field 
refers to system IO space.

> We might come away with it. But I have the strong feeling that there are 
> machines running better using 32-bit and machines running better with 64-bit 
> addresses used.

If you can find a single example of the latter case, I'll be 
astonishingly surprised.

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

Re: [Devel] Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Zhang Rui]

On Thu, 2008-11-13 at 07:58 +0800, Thomas Renninger wrote:
> On Monday 10 November 2008 06:58:56 pm Matthew Garrett wrote:
> > I've now had confirmation from multiple sources that Vista still uses
> > the 32-bit addresses for the GPE blocks.
> Are you sure that Windows Server implementations also use 32-bit addresses?
> Are you sure that upcoming Windows implementations will always use 32-bit 
> addresses?
> Do all X86 machines support Windows or could there be machines, especially 
> servers which only support Mac OS, Solaris/Linux or other OSes which stick to 
> the spec which you break with this change?
> > We're actually seeing the same 
> > bug on some currently shipping machines
> Which ones?
> > , so again I'm going to suggest 
> > that the blacklist model isn't going to scale and we should just behave
> > like Windows. How about this patch instead? It does some sanity
> > checking, so I doubt that there's any way it could break a legitimate
> > system. I've left IA64 as-is because it seems more likely that there'd
> > be a requirement for 64-bit setups there.
> 
> If at all, this should not be added for .28, but for .29 and stay in 
> linux-next for a while.
> I wonder why all (hmm, hard to say, at least a lot) recent machines have valid 
> 64 bit addresses.
> I also disagree with violating the spec unconditionally, breaking machines 
> which would stick to it. It's likely that machines do not get a latest 
> mainline kernel tests. Once this change is in distributions and machines do 
> break, people are busted. There should at least be a boot param to switch 
> back.
> 
> We might come away with it. But I have the strong feeling that there are 
> machines running better using 32-bit and machines running better with 64-bit 
> addresses used.
here is an example,
I have a test box which suspends well, but always reboots instead of
resuming when pressing the power button.
I found that there are two FACS tables on this platform,
XSDT-->FADT1-->Xfacs------>FACS1
	|----->facs-----|
			|->FACS2
RSDT-->FADT2-->facs-----|
Linux uses XSDT on this platform and sets the waking vector in FACS1
when suspending. But it seems that the BIOS only cares for the waking
vector in FACS2, thus it reboots when resuming because the waking vector
is not set at all.
My original proposal is to install both FADT1.Xfacs and FADT1.facs in to
the global table list and set the waking vectors on both of them.
now it seems that switching back to RSDT if multiple valid FACS tables
exists is also a solution for this issue.

anyway, the boot option is needed, which is much more flexible and make
it easier to verify/workaround such kind of issues.

thanks,
rui
> 
>        Thomas
> 
> > Signed-off-by: Matthew Garrett <mjg@redhat.com>
> >
> > ---
> >
> > Are there even any ACPI platforms where a system io address can be
> > greater than 32 bits? It's limited to 16 bits on x86, so I *really*
> > don't think this is going to break anything. The FADTs I've checked from
> > Thinkpads all seem to have valid 32-bit addresses even using the one
> > obtained from the XSDT.
> >
> > diff --git a/drivers/acpi/events/evgpeblk.c
> > b/drivers/acpi/events/evgpeblk.c index 73c058e..eed35d7 100644
> > --- a/drivers/acpi/events/evgpeblk.c
> > +++ b/drivers/acpi/events/evgpeblk.c
> > @@ -1107,6 +1107,32 @@ acpi_status acpi_ev_gpe_initialize(void)
> >  	 */
> >
> >  	/*
> > +	 * The ACPI specification says that we should use the 64-bit
> > +	 * address offset for the GPE blocks if it exists. However,
> > +	 * Windows uses the legacy address. Various vendors have left
> > +	 * incorrect values in the 64-bit field, which then causes
> > +	 * problems later. Since the vast majority of machines have
> > +	 * never been tested with an OS that uses the 64-bit value by
> > +	 * default, we should behave like Windows and ignore the spec
> > +	 * by only using the 64-bit address if it contains something
> > +	 * that can't be represented in the legacy field. Since system
> > +	 * io space is only 16 bits on x86, this should be entirely
> > +	 * safe.
> > +	 */
> > +
> > +#ifdef CONFIG_X86
> > +	if (acpi_gbl_FADT.gpe0_block &&
> > +	    acpi_gbl_FADT.xgpe0_block.space_id == ACPI_ADR_SPACE_SYSTEM_IO)
> > +		acpi_gbl_FADT.xgpe0_block.address =
> > +			(u64)acpi_gbl_FADT.gpe0_block;
> > +
> > +	if (acpi_gbl_FADT.gpe1_block &&
> > +	    acpi_gbl_FADT.xgpe1_block.space_id == ACPI_ADR_SPACE_SYSTEM_IO)
> > +		acpi_gbl_FADT.xgpe1_block.address =
> > +			(u64)acpi_gbl_FADT.gpe1_block;
> > +#endif
> > +
> > +	/*
> >  	 * Determine the maximum GPE number for this machine.
> >  	 *
> >  	 * Note: both GPE0 and GPE1 are optional, and either can exist without
> 
> 
> _______________________________________________
> Devel mailing list
> Devel@lists.acpica.org
> https://lists.acpica.org/mailman/listinfo/devel

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Devel] Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Thu, Nov 13, 2008 at 10:21:00AM +0800, Zhang Rui wrote:
> I have a test box which suspends well, but always reboots instead of
> resuming when pressing the power button.
> I found that there are two FACS tables on this platform,
> XSDT-->FADT1-->Xfacs------>FACS1
> 	|----->facs-----|
> 			|->FACS2
> RSDT-->FADT2-->facs-----|
> Linux uses XSDT on this platform and sets the waking vector in FACS1
> when suspending. But it seems that the BIOS only cares for the waking
> vector in FACS2, thus it reboots when resuming because the waking vector
> is not set at all.

That makes it sound like we should be using the 32-bit value in this 
case as well.

> My original proposal is to install both FADT1.Xfacs and FADT1.facs in to
> the global table list and set the waking vectors on both of them.
> now it seems that switching back to RSDT if multiple valid FACS tables
> exists is also a solution for this issue.

Why? If there's a FADT entry in the XSDT we might as well use it - all 
the machines I've checked contain valid information in the 32-bit entry 
of the FADT there.

-- 
Matthew Garrett | mjg59@srcf.ucam.org
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Devel] Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Zhang Rui]

On Thu, 2008-11-13 at 10:24 +0800, Matthew Garrett wrote:
> On Thu, Nov 13, 2008 at 10:21:00AM +0800, Zhang Rui wrote:
> > I have a test box which suspends well, but always reboots instead of
> > resuming when pressing the power button.
> > I found that there are two FACS tables on this platform,
> > XSDT-->FADT1-->Xfacs------>FACS1
> > 	|----->facs-----|
> > 			|->FACS2
> > RSDT-->FADT2-->facs-----|
> > Linux uses XSDT on this platform and sets the waking vector in FACS1
> > when suspending. But it seems that the BIOS only cares for the waking
> > vector in FACS2, thus it reboots when resuming because the waking vector
> > is not set at all.
> 
> That makes it sound like we should be using the 32-bit value in this 
> case as well.

> 
> > My original proposal is to install both FADT1.Xfacs and FADT1.facs in to
> > the global table list and set the waking vectors on both of them.
> > now it seems that switching back to RSDT if multiple valid FACS tables
> > exists is also a solution for this issue.
> 
> Why? If there's a FADT entry in the XSDT we might as well use it - all 
> the machines I've checked contain valid information in the 32-bit entry 
> of the FADT there.
> 
do you mean always using FADT1.facs and ignore the FADT.Xfacs in Linux?

thanks,
rui

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Devel] Re: [RESEND] [PATCH 2/3] Introduce acpi_root_table=rsdt boot param and dmi list to force rsdt

[Matthew Garrett]

On Thu, Nov 13, 2008 at 04:27:09PM +0800, Zhang Rui wrote:
> On Thu, 2008-11-13 at 10:24 +0800, Matthew Garrett wrote:
> > Why? If there's a FADT entry in the XSDT we might as well use it - all 
> > the machines I've checked contain valid information in the 32-bit entry 
> > of the FADT there.
> > 
> do you mean always using FADT1.facs and ignore the FADT.Xfacs in Linux?

If the value is below 4GB and uses the same resource type as the legacy 
value, I don't see why not.
-- 
Matthew Garrett | mjg59@srcf.ucam.org
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html