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

Zhang Rui linux-thinkpad@linux-thinkpad.org
Thu, 13 Nov 2008 10:21:00 +0800


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