All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Stone <ahs3@redhat.com>
To: Hanjun Guo <hanjun.guo@linaro.org>,
	Al Stone <al.stone@linaro.org>,
	linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Cc: rjw@rjwysocki.net, lenb@kernel.org, catalin.marinas@arm.com,
	will.deacon@arm.com, tglx@linutronix.de, jason@lakedaemon.net,
	linux-kernel@vger.kernel.org, linaro-acpi@lists.linaro.org,
	linaro-kernel@lists.linaro.org, patches@linaro.org
Subject: Re: [PATCH v2 0/3] Correct for ACPI 5.1->6.0 spec changes in MADT GICC entries
Date: Fri, 19 Jun 2015 14:05:31 -0600	[thread overview]
Message-ID: <5584760B.5040201@redhat.com> (raw)
In-Reply-To: <5583F4F7.1000002@linaro.org>

On 06/19/2015 04:54 AM, Hanjun Guo wrote:
> On 06/19/2015 06:36 AM, Al Stone wrote:
>> In the ACPI 5.1 version of the spec, the struct for the GICC subtable
>> (struct acpi_madt_generic_interrupt) of the MADT is 76 bytes long; in
>> ACPI 6.0, the struct is 80 bytes long.  But, there is only one definition
>> in ACPICA for this struct -- and that is the 6.0 version.  Hence, when
>> BAD_MADT_ENTRY() compares the struct size to the length in the GICC
>> subtable, it fails if 5.1 structs are in use, and there are systems in
>> the wild that have them.
>>
>> Note that this was found in linux-next and these patches apply against
>> that tree and the arm64 kernel tree; 4.1-rc8 does not appear to have this
>> problem since it still has the 5.1 struct definition.
>>
>> Even though there is precendent in ia64 code for ignoring the changes in
>> size, this patch set instead tries to verify correctness.  The first patch
>> in the set adds macros for easily using the ACPI spec version.  The second
>> patch adds the BAD_MADT_GICC_ENTRY() macro that uses the version macros to
>> check the GICC subtable only, accounting for the difference in specification
>> versions that are possible.  The final patch replaces BAD_MADT_ENTRY usage
>> with the BAD_MADT_GICC_ENTRY macro in arm64 code, which is currently the
>> only architecture affected.  The BAD_MADT_ENTRY() will continue to work as
>> is for all other MADT subtables.
>>
>> I have tested these patches on an APM Mustang with version 1.15 firmware,
>> where the problem was found, and they fix the problem.
> 
> I also tested on QEMU, it fixed the problem when I was using ACPICA 6.0
> updates for MADT table,
> 
> Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
> 
> Thanks
> Hanjun

Cool.  The patches did what they should.  Looks like they work on AMD
Seattle with older firmware, also.  Thanks.

>>
>> Changes for v2:
>>    -- Replace magic constants with proper defines (Lorenzo)
>>    -- Minor syntax clean-up noted by checkpatch
>>    -- Send out CCs properly this time
>>    -- Minor clean-up of the paragraphs in this cover letter
>>
>>
>> Al Stone (3):
>>    ACPI : introduce macros for using the ACPI specification version
>>    ACPI: add BAD_MADT_GICC_ENTRY() macro
>>    ACPI / ARM64 : use the new BAD_MADT_GICC_ENTRY macro
>>
>>   arch/arm64/kernel/smp.c   |  2 +-
>>   drivers/irqchip/irq-gic.c |  2 +-
>>   include/linux/acpi.h      | 15 +++++++++++++++
>>   3 files changed, 17 insertions(+), 2 deletions(-)
>>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in


-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3@redhat.com
-----------------------------------

WARNING: multiple messages have this Message-ID (diff)
From: ahs3@redhat.com (Al Stone)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 0/3] Correct for ACPI 5.1->6.0 spec changes in MADT GICC entries
Date: Fri, 19 Jun 2015 14:05:31 -0600	[thread overview]
Message-ID: <5584760B.5040201@redhat.com> (raw)
In-Reply-To: <5583F4F7.1000002@linaro.org>

On 06/19/2015 04:54 AM, Hanjun Guo wrote:
> On 06/19/2015 06:36 AM, Al Stone wrote:
>> In the ACPI 5.1 version of the spec, the struct for the GICC subtable
>> (struct acpi_madt_generic_interrupt) of the MADT is 76 bytes long; in
>> ACPI 6.0, the struct is 80 bytes long.  But, there is only one definition
>> in ACPICA for this struct -- and that is the 6.0 version.  Hence, when
>> BAD_MADT_ENTRY() compares the struct size to the length in the GICC
>> subtable, it fails if 5.1 structs are in use, and there are systems in
>> the wild that have them.
>>
>> Note that this was found in linux-next and these patches apply against
>> that tree and the arm64 kernel tree; 4.1-rc8 does not appear to have this
>> problem since it still has the 5.1 struct definition.
>>
>> Even though there is precendent in ia64 code for ignoring the changes in
>> size, this patch set instead tries to verify correctness.  The first patch
>> in the set adds macros for easily using the ACPI spec version.  The second
>> patch adds the BAD_MADT_GICC_ENTRY() macro that uses the version macros to
>> check the GICC subtable only, accounting for the difference in specification
>> versions that are possible.  The final patch replaces BAD_MADT_ENTRY usage
>> with the BAD_MADT_GICC_ENTRY macro in arm64 code, which is currently the
>> only architecture affected.  The BAD_MADT_ENTRY() will continue to work as
>> is for all other MADT subtables.
>>
>> I have tested these patches on an APM Mustang with version 1.15 firmware,
>> where the problem was found, and they fix the problem.
> 
> I also tested on QEMU, it fixed the problem when I was using ACPICA 6.0
> updates for MADT table,
> 
> Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
> 
> Thanks
> Hanjun

Cool.  The patches did what they should.  Looks like they work on AMD
Seattle with older firmware, also.  Thanks.

>>
>> Changes for v2:
>>    -- Replace magic constants with proper defines (Lorenzo)
>>    -- Minor syntax clean-up noted by checkpatch
>>    -- Send out CCs properly this time
>>    -- Minor clean-up of the paragraphs in this cover letter
>>
>>
>> Al Stone (3):
>>    ACPI : introduce macros for using the ACPI specification version
>>    ACPI: add BAD_MADT_GICC_ENTRY() macro
>>    ACPI / ARM64 : use the new BAD_MADT_GICC_ENTRY macro
>>
>>   arch/arm64/kernel/smp.c   |  2 +-
>>   drivers/irqchip/irq-gic.c |  2 +-
>>   include/linux/acpi.h      | 15 +++++++++++++++
>>   3 files changed, 17 insertions(+), 2 deletions(-)
>>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in


-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3 at redhat.com
-----------------------------------

WARNING: multiple messages have this Message-ID (diff)
From: Al Stone <ahs3@redhat.com>
To: Hanjun Guo <hanjun.guo@linaro.org>,
	Al Stone <al.stone@linaro.org>,
	linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Cc: rjw@rjwysocki.net, lenb@kernel.org, catalin.marinas@arm.com,
	will.deacon@arm.com, tglx@linutronix.de, jason@lakedaemon.net,
	linux-kernel@vger.kernel.org, linaro-acpi@lists.linaro.org,
	linaro-kernel@lists.linaro.org, patches@linaro.org
Subject: Re: [PATCH v2 0/3] Correct for ACPI 5.1->6.0 spec changes in MADT GICC entries
Date: Fri, 19 Jun 2015 14:05:31 -0600	[thread overview]
Message-ID: <5584760B.5040201@redhat.com> (raw)
In-Reply-To: <5583F4F7.1000002@linaro.org>

On 06/19/2015 04:54 AM, Hanjun Guo wrote:
> On 06/19/2015 06:36 AM, Al Stone wrote:
>> In the ACPI 5.1 version of the spec, the struct for the GICC subtable
>> (struct acpi_madt_generic_interrupt) of the MADT is 76 bytes long; in
>> ACPI 6.0, the struct is 80 bytes long.  But, there is only one definition
>> in ACPICA for this struct -- and that is the 6.0 version.  Hence, when
>> BAD_MADT_ENTRY() compares the struct size to the length in the GICC
>> subtable, it fails if 5.1 structs are in use, and there are systems in
>> the wild that have them.
>>
>> Note that this was found in linux-next and these patches apply against
>> that tree and the arm64 kernel tree; 4.1-rc8 does not appear to have this
>> problem since it still has the 5.1 struct definition.
>>
>> Even though there is precendent in ia64 code for ignoring the changes in
>> size, this patch set instead tries to verify correctness.  The first patch
>> in the set adds macros for easily using the ACPI spec version.  The second
>> patch adds the BAD_MADT_GICC_ENTRY() macro that uses the version macros to
>> check the GICC subtable only, accounting for the difference in specification
>> versions that are possible.  The final patch replaces BAD_MADT_ENTRY usage
>> with the BAD_MADT_GICC_ENTRY macro in arm64 code, which is currently the
>> only architecture affected.  The BAD_MADT_ENTRY() will continue to work as
>> is for all other MADT subtables.
>>
>> I have tested these patches on an APM Mustang with version 1.15 firmware,
>> where the problem was found, and they fix the problem.
> 
> I also tested on QEMU, it fixed the problem when I was using ACPICA 6.0
> updates for MADT table,
> 
> Tested-by: Hanjun Guo <hanjun.guo@linaro.org>
> 
> Thanks
> Hanjun

Cool.  The patches did what they should.  Looks like they work on AMD
Seattle with older firmware, also.  Thanks.

>>
>> Changes for v2:
>>    -- Replace magic constants with proper defines (Lorenzo)
>>    -- Minor syntax clean-up noted by checkpatch
>>    -- Send out CCs properly this time
>>    -- Minor clean-up of the paragraphs in this cover letter
>>
>>
>> Al Stone (3):
>>    ACPI : introduce macros for using the ACPI specification version
>>    ACPI: add BAD_MADT_GICC_ENTRY() macro
>>    ACPI / ARM64 : use the new BAD_MADT_GICC_ENTRY macro
>>
>>   arch/arm64/kernel/smp.c   |  2 +-
>>   drivers/irqchip/irq-gic.c |  2 +-
>>   include/linux/acpi.h      | 15 +++++++++++++++
>>   3 files changed, 17 insertions(+), 2 deletions(-)
>>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in


-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3@redhat.com
-----------------------------------
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

  reply	other threads:[~2015-06-19 20:05 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-18 22:36 [PATCH v2 0/3] Correct for ACPI 5.1->6.0 spec changes in MADT GICC entries Al Stone
2015-06-18 22:36 ` Al Stone
2015-06-18 22:36 ` [PATCH v2 1/3] ACPI : introduce macros for using the ACPI specification version Al Stone
2015-06-18 22:36   ` Al Stone
2015-06-19 10:49   ` Hanjun Guo
2015-06-19 10:49     ` Hanjun Guo
2015-06-19 10:49     ` Hanjun Guo
2015-06-19 20:01     ` Al Stone
2015-06-19 20:01       ` Al Stone
2015-06-19 20:01       ` Al Stone
2015-06-30 20:12   ` Rafael J. Wysocki
2015-06-30 20:12     ` Rafael J. Wysocki
2015-06-30 21:15     ` Al Stone
2015-06-30 21:15       ` Al Stone
2015-07-01  2:30       ` Hanjun Guo
2015-07-01  2:30         ` Hanjun Guo
2015-06-18 22:36 ` [PATCH v2 2/3] ACPI: add BAD_MADT_GICC_ENTRY() macro Al Stone
2015-06-18 22:36   ` Al Stone
2015-06-19 10:49   ` Hanjun Guo
2015-06-19 10:49     ` Hanjun Guo
2015-06-19 10:49     ` Hanjun Guo
2015-06-19 20:02     ` Al Stone
2015-06-19 20:02       ` Al Stone
2015-06-19 20:02       ` Al Stone
2015-06-18 22:36 ` [PATCH v2 3/3] ACPI / ARM64 : use the new BAD_MADT_GICC_ENTRY macro Al Stone
2015-06-18 22:36   ` Al Stone
2015-06-19  9:46   ` Will Deacon
2015-06-19  9:46     ` Will Deacon
2015-06-19 20:03     ` Al Stone
2015-06-19 20:03       ` Al Stone
2015-06-19 20:03       ` Al Stone
2015-06-19 10:52   ` Hanjun Guo
2015-06-19 10:52     ` Hanjun Guo
2015-06-19 10:52     ` Hanjun Guo
2015-06-19 10:54 ` [PATCH v2 0/3] Correct for ACPI 5.1->6.0 spec changes in MADT GICC entries Hanjun Guo
2015-06-19 10:54   ` Hanjun Guo
2015-06-19 10:54   ` Hanjun Guo
2015-06-19 20:05   ` Al Stone [this message]
2015-06-19 20:05     ` Al Stone
2015-06-19 20:05     ` Al Stone
2015-06-30 17:07 ` Sudeep Holla
2015-06-30 17:07   ` Sudeep Holla
2015-06-30 17:29   ` Al Stone
2015-06-30 17:29     ` Al Stone
2015-06-30 18:25     ` Rafael J. Wysocki
2015-06-30 18:25       ` Rafael J. Wysocki
2015-06-30 18:35       ` Rafael J. Wysocki
2015-06-30 18:35         ` Rafael J. Wysocki
2015-07-01  2:06         ` Hanjun Guo
2015-07-01  2:06           ` Hanjun Guo
2015-07-02 18:25           ` Al Stone
2015-07-02 18:25             ` Al Stone
2015-06-30 18:39       ` Al Stone
2015-06-30 18:39         ` Al Stone
2015-06-30 19:05         ` Rafael J. Wysocki
2015-06-30 19:05           ` Rafael J. Wysocki
2015-06-30 19:57           ` Al Stone
2015-06-30 19:57             ` Al Stone
2015-06-30 19:45       ` Al Stone
2015-06-30 19:45         ` Al Stone
2015-06-30 19:58         ` Rafael J. Wysocki
2015-06-30 19:58           ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5584760B.5040201@redhat.com \
    --to=ahs3@redhat.com \
    --cc=al.stone@linaro.org \
    --cc=catalin.marinas@arm.com \
    --cc=hanjun.guo@linaro.org \
    --cc=jason@lakedaemon.net \
    --cc=lenb@kernel.org \
    --cc=linaro-acpi@lists.linaro.org \
    --cc=linaro-kernel@lists.linaro.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@linaro.org \
    --cc=rjw@rjwysocki.net \
    --cc=tglx@linutronix.de \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.