LinuxLists.cc - Endless ACPI errors on Linus tree (5b664cb235)

[permalink] [raw]

Subject: Re: Endless ACPI errors on Linus tree (5b664cb235)

Takashi Iwai <[email protected]> writes:

> the boot with the latest Linus git tree fails on x86-64 due to the
> endless kernel messages like below:
>
> ACPI Error (evpge-0710): No handler or method for GPE[10],
> disabling event[20080609]
>
> It happens on today's tree and also on yesterday (33af79d12e).
> The config is below.

That was after the ACPI merge I assume? Do you have a full boot log?

Revert candidates to test would be e38e8a0743b0e996a8a3fbea8908fe75a84f02c7
and c91d924e3af08d4f98eab6ebf81f2b8ce132448f (Bob, that were both
changes from you for evgpe.c). Can you see if reverting
those helps? If yes which?

Thanks.

-Andi

2008-07-18 13:15:36

[permalink] [raw]

Subject: Re: Endless ACPI errors on Linus tree (5b664cb235)

At Fri, 18 Jul 2008 13:35:48 +0200,
Andi Kleen wrote:
>
> Takashi Iwai <[email protected]> writes:
>
> > the boot with the latest Linus git tree fails on x86-64 due to the
> > endless kernel messages like below:
> >
> > ACPI Error (evpge-0710): No handler or method for GPE[10],
> > disabling event[20080609]
> >
> > It happens on today's tree and also on yesterday (33af79d12e).
> > The config is below.
>
> That was after the ACPI merge I assume?

Yes.

> Do you have a full boot log?

Sorry, no. The kernel shows the error message (the number after GPE
constantly changing as 1x) endlessly, and couldn't boot up properly to
get a log.

> Revert candidates to test would be e38e8a0743b0e996a8a3fbea8908fe75a84f02c7
> and c91d924e3af08d4f98eab6ebf81f2b8ce132448f (Bob, that were both
> changes from you for evgpe.c). Can you see if reverting
> those helps? If yes which?

Will give it a spin.

thanks,

Takashi

2008-07-18 13:34:32

[permalink] [raw]

Subject: Re: Endless ACPI errors on Linus tree (5b664cb235)

At Fri, 18 Jul 2008 15:15:25 +0200,
I wrote:
>
> At Fri, 18 Jul 2008 13:35:48 +0200,
> Andi Kleen wrote:
> >
> > Takashi Iwai <[email protected]> writes:
> >
> > > the boot with the latest Linus git tree fails on x86-64 due to the
> > > endless kernel messages like below:
> > >
> > > ACPI Error (evpge-0710): No handler or method for GPE[10],
> > > disabling event[20080609]
> > >
> > > It happens on today's tree and also on yesterday (33af79d12e).
> > > The config is below.
> >
> > That was after the ACPI merge I assume?
>
> Yes.
>
> > Do you have a full boot log?
>
> Sorry, no. The kernel shows the error message (the number after GPE
> constantly changing as 1x) endlessly, and couldn't boot up properly to
> get a log.
>
> > Revert candidates to test would be e38e8a0743b0e996a8a3fbea8908fe75a84f02c7
> > and c91d924e3af08d4f98eab6ebf81f2b8ce132448f (Bob, that were both
> > changes from you for evgpe.c). Can you see if reverting
> > those helps? If yes which?
>
> Will give it a spin.

I reverting both, but it doesn't fix the problem.

Another finding is that the boot reaches to the exec of init, at
least. So I could get a sane state with init=/bin/sh. The message
appears after the init script running udevd.

Also, the machine could boot fine with the recent linux-next kernel,
at least, 20080711-0714. (It failed for last couple of days, but it
can be irrelevant.)

I'll investigate a bit. Any hints are appreciated.

thanks,

Takashi

2008-07-18 15:48:48

[permalink] [raw]

Subject: Re: Endless ACPI errors on Linus tree (5b664cb235)

At Fri, 18 Jul 2008 15:34:13 +0200,
I wrote:
>
> At Fri, 18 Jul 2008 15:15:25 +0200,
> I wrote:
> >
> > At Fri, 18 Jul 2008 13:35:48 +0200,
> > Andi Kleen wrote:
> > >
> > > Takashi Iwai <[email protected]> writes:
> > >
> > > > the boot with the latest Linus git tree fails on x86-64 due to the
> > > > endless kernel messages like below:
> > > >
> > > > ACPI Error (evpge-0710): No handler or method for GPE[10],
> > > > disabling event[20080609]
> > > >
> > > > It happens on today's tree and also on yesterday (33af79d12e).
> > > > The config is below.
> > >
> > > That was after the ACPI merge I assume?
> >
> > Yes.
> >
> > > Do you have a full boot log?
> >
> > Sorry, no. The kernel shows the error message (the number after GPE
> > constantly changing as 1x) endlessly, and couldn't boot up properly to
> > get a log.
> >
> > > Revert candidates to test would be e38e8a0743b0e996a8a3fbea8908fe75a84f02c7
> > > and c91d924e3af08d4f98eab6ebf81f2b8ce132448f (Bob, that were both
> > > changes from you for evgpe.c). Can you see if reverting
> > > those helps? If yes which?
> >
> > Will give it a spin.
>
> I reverting both, but it doesn't fix the problem.
>
> Another finding is that the boot reaches to the exec of init, at
> least. So I could get a sane state with init=/bin/sh. The message
> appears after the init script running udevd.
>
> Also, the machine could boot fine with the recent linux-next kernel,
> at least, 20080711-0714. (It failed for last couple of days, but it
> can be irrelevant.)
>
> I'll investigate a bit. Any hints are appreciated.

OK, found out the bad commit via bisect.
Reverting below fixes the boot problem.

commit 01a5bba576b9364b33f61f0cd9fa70c2cf5535e2
Author: Jan Beulich <[email protected]>
Date: Wed Jul 16 23:27:08 2008 +0200

Fix FADT parsing

The (1.0 inherited) separate length fields in the FADT are byte granular.
Further, PM1a/b may have distinct lengths and live in distinct address spaces.
acpi_tb_convert_fadt() should account for all of these conditions.

Apart from these changes I'm puzzled by the fact that, not just for
acpi_gbl_xpm1{a,b}_enable, acpi_hw_low_level_{read,write}() get an explicit
size passed rather than using the size found in the passed GAS. What happens
on a platform that defines PM1{a,b} wider than 16 bits? Of course,
acpi_hw_low_level_{read,write}() at present are entirely un-prepared to deal
with sizes other than 8, 16, or 32, not to speak of a non-zero bit_offset or
access_width...

Takashi

2008-07-18 15:52:24

[permalink] [raw]

Subject: Re: Endless ACPI errors on Linus tree (5b664cb235)

> I'll investigate a bit. Any hints are appreciated.

Don't know what causes it then. Could you please bisect?

To cut it short you might want to start with the ACPI merge
only (d442cc44c0db56e84ef6aa244a88427d2efe06cd) and first check if that
kernel didn't do it and the patch after it did
(4314652bb41df08ad65bd25176ba1dfd24b14a51)

Then you can bisect between the two relatively quickly.

Thanks,

-Andi

2008-07-18 15:53:54

[permalink] [raw]

Subject: Re: Endless ACPI errors on Linus tree (5b664cb235)

On Fri, Jul 18, 2008 at 05:48:39PM +0200, Takashi Iwai wrote:
> At Fri, 18 Jul 2008 15:34:13 +0200,
> I wrote:
> >
> > At Fri, 18 Jul 2008 15:15:25 +0200,
> > I wrote:
> > >
> > > At Fri, 18 Jul 2008 13:35:48 +0200,
> > > Andi Kleen wrote:
> > > >
> > > > Takashi Iwai <[email protected]> writes:
> > > >
> > > > > the boot with the latest Linus git tree fails on x86-64 due to the
> > > > > endless kernel messages like below:
> > > > >
> > > > > ACPI Error (evpge-0710): No handler or method for GPE[10],
> > > > > disabling event[20080609]
> > > > >
> > > > > It happens on today's tree and also on yesterday (33af79d12e).
> > > > > The config is below.
> > > >
> > > > That was after the ACPI merge I assume?
> > >
> > > Yes.
> > >
> > > > Do you have a full boot log?
> > >
> > > Sorry, no. The kernel shows the error message (the number after GPE
> > > constantly changing as 1x) endlessly, and couldn't boot up properly to
> > > get a log.
> > >
> > > > Revert candidates to test would be e38e8a0743b0e996a8a3fbea8908fe75a84f02c7
> > > > and c91d924e3af08d4f98eab6ebf81f2b8ce132448f (Bob, that were both
> > > > changes from you for evgpe.c). Can you see if reverting
> > > > those helps? If yes which?
> > >
> > > Will give it a spin.
> >
> > I reverting both, but it doesn't fix the problem.
> >
> > Another finding is that the boot reaches to the exec of init, at
> > least. So I could get a sane state with init=/bin/sh. The message
> > appears after the init script running udevd.
> >
> > Also, the machine could boot fine with the recent linux-next kernel,
> > at least, 20080711-0714. (It failed for last couple of days, but it
> > can be irrelevant.)
> >
> > I'll investigate a bit. Any hints are appreciated.
>
> OK, found out the bad commit via bisect.
> Reverting below fixes the boot problem.

Ah thanks. That one was on the revert list anyways becase
it caused some other problems.

-Andi

2008-07-18 15:56:30

[permalink] [raw]

Subject: Re: Endless ACPI errors on Linus tree (5b664cb235)

At Fri, 18 Jul 2008 17:52:44 +0200,
Andi Kleen wrote:
>
> On Fri, Jul 18, 2008 at 05:48:39PM +0200, Takashi Iwai wrote:
> > At Fri, 18 Jul 2008 15:34:13 +0200,
> > I wrote:
> > >
> > > At Fri, 18 Jul 2008 15:15:25 +0200,
> > > I wrote:
> > > >
> > > > At Fri, 18 Jul 2008 13:35:48 +0200,
> > > > Andi Kleen wrote:
> > > > >
> > > > > Takashi Iwai <[email protected]> writes:
> > > > >
> > > > > > the boot with the latest Linus git tree fails on x86-64 due to the
> > > > > > endless kernel messages like below:
> > > > > >
> > > > > > ACPI Error (evpge-0710): No handler or method for GPE[10],
> > > > > > disabling event[20080609]
> > > > > >
> > > > > > It happens on today's tree and also on yesterday (33af79d12e).
> > > > > > The config is below.
> > > > >
> > > > > That was after the ACPI merge I assume?
> > > >
> > > > Yes.
> > > >
> > > > > Do you have a full boot log?
> > > >
> > > > Sorry, no. The kernel shows the error message (the number after GPE
> > > > constantly changing as 1x) endlessly, and couldn't boot up properly to
> > > > get a log.
> > > >
> > > > > Revert candidates to test would be e38e8a0743b0e996a8a3fbea8908fe75a84f02c7
> > > > > and c91d924e3af08d4f98eab6ebf81f2b8ce132448f (Bob, that were both
> > > > > changes from you for evgpe.c). Can you see if reverting
> > > > > those helps? If yes which?
> > > >
> > > > Will give it a spin.
> > >
> > > I reverting both, but it doesn't fix the problem.
> > >
> > > Another finding is that the boot reaches to the exec of init, at
> > > least. So I could get a sane state with init=/bin/sh. The message
> > > appears after the init script running udevd.
> > >
> > > Also, the machine could boot fine with the recent linux-next kernel,
> > > at least, 20080711-0714. (It failed for last couple of days, but it
> > > can be irrelevant.)
> > >
> > > I'll investigate a bit. Any hints are appreciated.
> >
> > OK, found out the bad commit via bisect.
> > Reverting below fixes the boot problem.
>
> Ah thanks. That one was on the revert list anyways becase
> it caused some other problems.

OK, good to hear :)

thanks,

Takashi

2008-07-18 16:16:42

[permalink] [raw]

Subject: Re: Endless ACPI errors on Linus tree (5b664cb235)

>OK, found out the bad commit via bisect.
>Reverting below fixes the boot problem.
>
>
>commit 01a5bba576b9364b33f61f0cd9fa70c2cf5535e2
>Author: Jan Beulich <[email protected]>
>Date: Wed Jul 16 23:27:08 2008 +0200
>
> Fix FADT parsing

After someone else observed some problem with this patch earlier, I had
already sent out attached patch as a replacement one. Any chance you
could also try this with your box?

Thanks, Jan

Attachments:

(No filename) (438.00 B)
linux-2.6.26-acpi-fadt-parse.patch (3.76 kB)
Download all attachments

2008-07-18 16:39:05

[permalink] [raw]

Subject: Re: Endless ACPI errors on Linus tree (5b664cb235)

At Fri, 18 Jul 2008 17:16:53 +0100,
Jan Beulich wrote:
>
> >OK, found out the bad commit via bisect.
> >Reverting below fixes the boot problem.
> >
> >
> >commit 01a5bba576b9364b33f61f0cd9fa70c2cf5535e2
> >Author: Jan Beulich <[email protected]>
> >Date: Wed Jul 16 23:27:08 2008 +0200
> >
> > Fix FADT parsing
>
> After someone else observed some problem with this patch earlier, I had
> already sent out attached patch as a replacement one. Any chance you
> could also try this with your box?

It resulted in the same error, unfortunately.

thanks,

Takashi

>
> Thanks, Jan
>
> [2 linux-2.6.26-acpi-fadt-parse.patch <text/plain (quoted-printable)>]
> The (1.0 inherited) separate length fields in the FADT are byte
> granular. Further, PM1a/b may have distinct lengths and live in
> distinct address spaces. acpi_tb_convert_fadt() should account for
> all of these conditions.
>
> Signed-off-by: Jan Beulich <[email protected]>
>
> ---
> drivers/acpi/tables/tbfadt.c | 39 ++++++++++++++++++++++++++++++++-------
> 1 file changed, 32 insertions(+), 7 deletions(-)
>
> --- linux-2.6.26/drivers/acpi/tables/tbfadt.c 2008-07-13 23:51:29.000000000 +0200
> +++ 2.6.26-acpi-fadt-parse/drivers/acpi/tables/tbfadt.c 2008-07-18 10:43:55.000000000 +0200
> @@ -50,7 +50,7 @@ ACPI_MODULE_NAME("tbfadt")
> /* Local prototypes */
> static void inline
> acpi_tb_init_generic_address(struct acpi_generic_address *generic_address,
> - u8 bit_width, u64 address);
> + u8 byte_width, u64 address);
>
> static void acpi_tb_convert_fadt(void);
>
> @@ -111,7 +111,7 @@ static struct acpi_fadt_info fadt_info_t
> * FUNCTION: acpi_tb_init_generic_address
> *
> * PARAMETERS: generic_address - GAS struct to be initialized
> - * bit_width - Width of this register
> + * byte_width - Width of this register
> * Address - Address of the register
> *
> * RETURN: None
> @@ -124,7 +124,7 @@ static struct acpi_fadt_info fadt_info_t
>
> static void inline
> acpi_tb_init_generic_address(struct acpi_generic_address *generic_address,
> - u8 bit_width, u64 address)
> + u8 byte_width, u64 address)
> {
>
> /*
> @@ -136,7 +136,7 @@ acpi_tb_init_generic_address(struct acpi
> /* All other fields are byte-wide */
>
> generic_address->space_id = ACPI_ADR_SPACE_SYSTEM_IO;
> - generic_address->bit_width = bit_width;
> + generic_address->bit_width = byte_width << 3;
> generic_address->bit_offset = 0;
> generic_address->access_width = 0;
> }
> @@ -343,9 +343,21 @@ static void acpi_tb_convert_fadt(void)
> *
> * The PM event blocks are split into two register blocks, first is the
> * PM Status Register block, followed immediately by the PM Enable Register
> - * block. Each is of length (pm1_event_length/2)
> + * block. Each is of length (xpm1x_event_block.bit_width/2)
> */
> - pm1_register_length = (u8) ACPI_DIV_2(acpi_gbl_FADT.pm1_event_length);
> + if (!ACPI_MOD_16(acpi_gbl_FADT.xpm1a_event_block.bit_width))
> + pm1_register_length = (u8) ACPI_DIV_16(acpi_gbl_FADT
> + .xpm1a_event_block
> + .bit_width);
> + else {
> + printk(KERN_WARNING "FADT: "
> + "X_PM1a_EVT_BLK.bit_width=%u is invalid,"
> + " falling back to PM1_EVT_LEN=%u\n",
> + acpi_gbl_FADT.xpm1a_event_block.bit_width,
> + acpi_gbl_FADT.pm1_event_length);
> + pm1_register_length = (u8) ACPI_DIV_2(acpi_gbl_FADT
> + .pm1_event_length);
> + }
>
> /* The PM1A register block is required */
>
> @@ -360,13 +372,26 @@ static void acpi_tb_convert_fadt(void)
> /* The PM1B register block is optional, ignore if not present */
>
> if (acpi_gbl_FADT.xpm1b_event_block.address) {
> + if (!ACPI_MOD_16(acpi_gbl_FADT.xpm1b_event_block.bit_width))
> + pm1_register_length = (u8) ACPI_DIV_16(acpi_gbl_FADT
> + .xpm1b_event_block
> + .bit_width);
> + else {
> + printk(KERN_WARNING "FADT: "
> + "X_PM1b_EVT_BLK.bit_width=%u is invalid,"
> + " falling back to PM1_EVT_LEN=%u\n",
> + acpi_gbl_FADT.xpm1b_event_block.bit_width,
> + acpi_gbl_FADT.pm1_event_length);
> + pm1_register_length = (u8) ACPI_DIV_2(acpi_gbl_FADT
> + .pm1_event_length);
> + }
> acpi_tb_init_generic_address(&acpi_gbl_xpm1b_enable,
> pm1_register_length,
> (acpi_gbl_FADT.xpm1b_event_block.
> address + pm1_register_length));
> /* Don't forget to copy space_id of the GAS */
> acpi_gbl_xpm1b_enable.space_id =
> - acpi_gbl_FADT.xpm1a_event_block.space_id;
> + acpi_gbl_FADT.xpm1b_event_block.space_id;
>
> }
> }

2008-07-18 16:59:08

[permalink] [raw]

Subject: Re: Endless ACPI errors on Linus tree (5b664cb235)

On Fri, Jul 18, 2008 at 06:38:53PM +0200, Takashi Iwai wrote:
> At Fri, 18 Jul 2008 17:16:53 +0100,
> Jan Beulich wrote:
> >
> > >OK, found out the bad commit via bisect.
> > >Reverting below fixes the boot problem.
> > >
> > >
> > >commit 01a5bba576b9364b33f61f0cd9fa70c2cf5535e2
> > >Author: Jan Beulich <[email protected]>
> > >Date: Wed Jul 16 23:27:08 2008 +0200
> > >
> > > Fix FADT parsing
> >
> > After someone else observed some problem with this patch earlier, I had
> > already sent out attached patch as a replacement one. Any chance you
> > could also try this with your box?
>
> It resulted in the same error, unfortunately.

Can you send a acpi table dump please?

-Andi

2008-07-19 09:24:17

[permalink] [raw]

Subject: Re: Endless ACPI errors on Linus tree (5b664cb235)

At Fri, 18 Jul 2008 18:58:14 +0200,
Andi Kleen wrote:
>
> On Fri, Jul 18, 2008 at 06:38:53PM +0200, Takashi Iwai wrote:
> > At Fri, 18 Jul 2008 17:16:53 +0100,
> > Jan Beulich wrote:
> > >
> > > >OK, found out the bad commit via bisect.
> > > >Reverting below fixes the boot problem.
> > > >
> > > >
> > > >commit 01a5bba576b9364b33f61f0cd9fa70c2cf5535e2
> > > >Author: Jan Beulich <[email protected]>
> > > >Date: Wed Jul 16 23:27:08 2008 +0200
> > > >
> > > > Fix FADT parsing
> > >
> > > After someone else observed some problem with this patch earlier, I had
> > > already sent out attached patch as a replacement one. Any chance you
> > > could also try this with your box?
> >
> > It resulted in the same error, unfortunately.
>
> Can you send a acpi table dump please?

Here it is.

BTW, I'll be on vacation for the next week. Will test after that if
needed.

thanks,

Takashi

Attachments:

acpi.out.gz (51.87 kB)

2008-07-21 08:11:37

[permalink] [raw]

Subject: Re: Endless ACPI errors on Linus tree (5b664cb235)

>> > > After someone else observed some problem with this patch earlier, I had
>> > > already sent out attached patch as a replacement one. Any chance you
>> > > could also try this with your box?
>> >
>> > It resulted in the same error, unfortunately.
>>
>> Can you send a acpi table dump please?

The situation is even worse here - the (v2) bit width now is divisible by 16,
but still isn't in sync with the v1 field (as is the gpe0 one):

v1 v2
PM1a_EVT 4 0x10
PM1a_CNT 2 0x10
PM1b_CNT 2 0x10
PM_TMR 4 0x20
GPE0 8 0x20

That would basically mean there's no way to use the v2 fields here - all
the code could do is report the inconsitency, but it would have to use
the v1 field in any case. That's really a shame, and as before I'm
somewhat hesitant to do so. The only way I could easily find myself
doing this would be to introduce a command line option controlling the
behavior here, but that would need to be done outside of the ACPICA
code I'd assume.

Andi, Bob, Len?

Jan

2008-07-21 08:44:13

[permalink] [raw]

Subject: Re: Endless ACPI errors on Linus tree (5b664cb235)

> That would basically mean there's no way to use the v2 fields here - all
> the code could do is report the inconsitency, but it would have to use
> the v1 field in any case. That's really a shame, and as before I'm
> somewhat hesitant to do so. The only way I could easily find myself
> doing this would be to introduce a command line option controlling the
> behavior here, but that would need to be done outside of the ACPICA
> code I'd assume.
>
> Andi, Bob, Len?

I think we should only use v1 FADT fields for now and ignore v2.

-Andi

2008-07-23 11:27:39

[permalink] [raw]

Subject: Re: Endless ACPI errors on Linus tree (5b664cb235)

>>> Andi Kleen <[email protected]> 21.07.08 10:43 >>>
>> That would basically mean there's no way to use the v2 fields here - all
>> the code could do is report the inconsitency, but it would have to use
>> the v1 field in any case. That's really a shame, and as before I'm
>> somewhat hesitant to do so. The only way I could easily find myself
>> doing this would be to introduce a command line option controlling the
>> behavior here, but that would need to be done outside of the ACPICA
>> code I'd assume.
>>
>> Andi, Bob, Len?
>
>I think we should only use v1 FADT fields for now and ignore v2.

Here we go:

Subject: ACPI: fix FADT parsing (v3)

The (1.0 inherited) separate length fields in the FADT are byte
granular. Further, PM1a/b may live in distinct address spaces, and they
also could have different widths (if using the respective v2 fields was
okay, which in practice it apparently isn't). acpi_tb_convert_fadt()
should account for all of these conditions.

Signed-off-by: Jan Beulich <[email protected]>

---
drivers/acpi/tables/tbfadt.c | 32 +++++++++++++++++++++++++-------
1 file changed, 25 insertions(+), 7 deletions(-)

--- linux-2.6.26/drivers/acpi/tables/tbfadt.c 2008-07-13 23:51:29.000000000 +0200
+++ 2.6.26-acpi-fadt-parse/drivers/acpi/tables/tbfadt.c 2008-07-23 13:21:22.000000000 +0200
@@ -50,7 +50,7 @@ ACPI_MODULE_NAME("tbfadt")
/* Local prototypes */
static void inline
acpi_tb_init_generic_address(struct acpi_generic_address *generic_address,
- u8 bit_width, u64 address);
+ u8 byte_width, u64 address);

static void acpi_tb_convert_fadt(void);

@@ -111,7 +111,7 @@ static struct acpi_fadt_info fadt_info_t
* FUNCTION: acpi_tb_init_generic_address
*
* PARAMETERS: generic_address - GAS struct to be initialized
- * bit_width - Width of this register
+ * byte_width - Width of this register
* Address - Address of the register
*
* RETURN: None
@@ -124,7 +124,7 @@ static struct acpi_fadt_info fadt_info_t

static void inline
acpi_tb_init_generic_address(struct acpi_generic_address *generic_address,
- u8 bit_width, u64 address)
+ u8 byte_width, u64 address)
{

/*
@@ -136,7 +136,7 @@ acpi_tb_init_generic_address(struct acpi
/* All other fields are byte-wide */

generic_address->space_id = ACPI_ADR_SPACE_SYSTEM_IO;
- generic_address->bit_width = bit_width;
+ generic_address->bit_width = byte_width << 3;
generic_address->bit_offset = 0;
generic_address->access_width = 0;
}
@@ -342,9 +342,20 @@ static void acpi_tb_convert_fadt(void)
* useful to calculate them once, here.
*
* The PM event blocks are split into two register blocks, first is the
- * PM Status Register block, followed immediately by the PM Enable Register
- * block. Each is of length (pm1_event_length/2)
+ * PM Status Register block, followed immediately by the PM Enable
+ * Register block. Each is of length (xpm1x_event_block.bit_width/2).
+ *
+ * On various systems the v2 fields (and particularly the bit widths)
+ * cannot be relied upon, though. Hence resort to using the v1 length
+ * here (and warn about the inconsistency).
*/
+ if (acpi_gbl_FADT.xpm1a_event_block.bit_width
+ != acpi_gbl_FADT.pm1_event_length * 8)
+ printk(KERN_WARNING "FADT: "
+ "X_PM1a_EVT_BLK.bit_width (%u) does not match"
+ " PM1_EVT_LEN (%u)\n",
+ acpi_gbl_FADT.xpm1a_event_block.bit_width,
+ acpi_gbl_FADT.pm1_event_length);
pm1_register_length = (u8) ACPI_DIV_2(acpi_gbl_FADT.pm1_event_length);

/* The PM1A register block is required */
@@ -360,13 +371,20 @@ static void acpi_tb_convert_fadt(void)
/* The PM1B register block is optional, ignore if not present */

if (acpi_gbl_FADT.xpm1b_event_block.address) {
+ if (acpi_gbl_FADT.xpm1b_event_block.bit_width
+ != acpi_gbl_FADT.pm1_event_length * 8)
+ printk(KERN_WARNING "FADT: "
+ "X_PM1b_EVT_BLK.bit_width (%u) does not match"
+ " PM1_EVT_LEN (%u)\n",
+ acpi_gbl_FADT.xpm1b_event_block.bit_width,
+ acpi_gbl_FADT.pm1_event_length);
acpi_tb_init_generic_address(&acpi_gbl_xpm1b_enable,
pm1_register_length,
(acpi_gbl_FADT.xpm1b_event_block.
address + pm1_register_length));
/* Don't forget to copy space_id of the GAS */
acpi_gbl_xpm1b_enable.space_id =
- acpi_gbl_FADT.xpm1a_event_block.space_id;
+ acpi_gbl_FADT.xpm1b_event_block.space_id;

}
}

2008-07-23 15:45:34

by Moore, Robert

[permalink] [raw]

Subject: RE: Endless ACPI errors on Linus tree (5b664cb235)

Couple of thoughts.

I think we must use the V2 field if it is present and the address value
is > 32 bits.

What if the V2 address is < 32 bits but does not match the V1 address?

>-----Original Message-----
>From: Jan Beulich [mailto:[email protected]]
>Sent: Wednesday, July 23, 2008 4:28 AM
>To: Andi Kleen; Len Brown
>Cc: Moore, Robert; Andrew Paprocki; Takashi Iwai; LKML
>Subject: Re: Endless ACPI errors on Linus tree (5b664cb235)
>
>>>> Andi Kleen <[email protected]> 21.07.08 10:43 >>>
>>> That would basically mean there's no way to use the v2 fields here -
all
>>> the code could do is report the inconsitency, but it would have to
use
>>> the v1 field in any case. That's really a shame, and as before I'm
>>> somewhat hesitant to do so. The only way I could easily find myself
>>> doing this would be to introduce a command line option controlling
the
>>> behavior here, but that would need to be done outside of the ACPICA
>>> code I'd assume.
>>>
>>> Andi, Bob, Len?
>>
>>I think we should only use v1 FADT fields for now and ignore v2.
>
>Here we go:
>
>Subject: ACPI: fix FADT parsing (v3)
>
>The (1.0 inherited) separate length fields in the FADT are byte
>granular. Further, PM1a/b may live in distinct address spaces, and they
>also could have different widths (if using the respective v2 fields was
>okay, which in practice it apparently isn't). acpi_tb_convert_fadt()
>should account for all of these conditions.
>
>Signed-off-by: Jan Beulich <[email protected]>
>
>---
> drivers/acpi/tables/tbfadt.c | 32 +++++++++++++++++++++++++-------
> 1 file changed, 25 insertions(+), 7 deletions(-)
>
>--- linux-2.6.26/drivers/acpi/tables/tbfadt.c 2008-07-13
>23:51:29.000000000 +0200
>+++ 2.6.26-acpi-fadt-parse/drivers/acpi/tables/tbfadt.c
2008-07-23
>13:21:22.000000000 +0200
>@@ -50,7 +50,7 @@ ACPI_MODULE_NAME("tbfadt")
> /* Local prototypes */
> static void inline
> acpi_tb_init_generic_address(struct acpi_generic_address
*generic_address,
>- u8 bit_width, u64 address);
>+ u8 byte_width, u64 address);
>
> static void acpi_tb_convert_fadt(void);
>
>@@ -111,7 +111,7 @@ static struct acpi_fadt_info fadt_info_t
> * FUNCTION: acpi_tb_init_generic_address
> *
> * PARAMETERS: generic_address - GAS struct to be initialized
>- * bit_width - Width of this register
>+ * byte_width - Width of this register
> * Address - Address of the register
> *
> * RETURN: None
>@@ -124,7 +124,7 @@ static struct acpi_fadt_info fadt_info_t
>
> static void inline
> acpi_tb_init_generic_address(struct acpi_generic_address
*generic_address,
>- u8 bit_width, u64 address)
>+ u8 byte_width, u64 address)
> {
>
> /*
>@@ -136,7 +136,7 @@ acpi_tb_init_generic_address(struct acpi
> /* All other fields are byte-wide */
>
> generic_address->space_id = ACPI_ADR_SPACE_SYSTEM_IO;
>- generic_address->bit_width = bit_width;
>+ generic_address->bit_width = byte_width << 3;
> generic_address->bit_offset = 0;
> generic_address->access_width = 0;
> }
>@@ -342,9 +342,20 @@ static void acpi_tb_convert_fadt(void)
> * useful to calculate them once, here.
> *
> * The PM event blocks are split into two register blocks, first
is
>the
>- * PM Status Register block, followed immediately by the PM
Enable
>Register
>- * block. Each is of length (pm1_event_length/2)
>+ * PM Status Register block, followed immediately by the PM
Enable
>+ * Register block. Each is of length
(xpm1x_event_block.bit_width/2).
>+ *
>+ * On various systems the v2 fields (and particularly the bit
widths)
>+ * cannot be relied upon, though. Hence resort to using the v1
length
>+ * here (and warn about the inconsistency).
> */
>+ if (acpi_gbl_FADT.xpm1a_event_block.bit_width
>+ != acpi_gbl_FADT.pm1_event_length * 8)
>+ printk(KERN_WARNING "FADT: "
>+ "X_PM1a_EVT_BLK.bit_width (%u) does not match"
>+ " PM1_EVT_LEN (%u)\n",
>+ acpi_gbl_FADT.xpm1a_event_block.bit_width,
>+ acpi_gbl_FADT.pm1_event_length);
> pm1_register_length = (u8)
>ACPI_DIV_2(acpi_gbl_FADT.pm1_event_length);
>
> /* The PM1A register block is required */
>@@ -360,13 +371,20 @@ static void acpi_tb_convert_fadt(void)
> /* The PM1B register block is optional, ignore if not present */
>
> if (acpi_gbl_FADT.xpm1b_event_block.address) {
>+ if (acpi_gbl_FADT.xpm1b_event_block.bit_width
>+ != acpi_gbl_FADT.pm1_event_length * 8)
>+ printk(KERN_WARNING "FADT: "
>+ "X_PM1b_EVT_BLK.bit_width (%u) does not
match"
>+ " PM1_EVT_LEN (%u)\n",
>+
acpi_gbl_FADT.xpm1b_event_block.bit_width,
>+ acpi_gbl_FADT.pm1_event_length);
> acpi_tb_init_generic_address(&acpi_gbl_xpm1b_enable,
> pm1_register_length,
>
(acpi_gbl_FADT.xpm1b_event_block.
> address +
pm1_register_length));
> /* Don't forget to copy space_id of the GAS */
> acpi_gbl_xpm1b_enable.space_id =
>- acpi_gbl_FADT.xpm1a_event_block.space_id;
>+ acpi_gbl_FADT.xpm1b_event_block.space_id;
>
> }
> }
>

2008-07-23 16:13:54