Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1177929
| From | Al Stone <ahs3@redhat.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v3 2/3] ACPI / ARM64: add BAD_MADT_GICC_ENTRY() macro |
| Date | 2015-07-06 23:30 +0200 |
| Message-ID | <pJm7F-59T-27@gated-at.bofh.it> (permalink) |
| References | <pHWoV-X2-1@gated-at.bofh.it> <pI9Pc-13T-7@gated-at.bofh.it> <pIfhU-4ha-11@gated-at.bofh.it> <pIiz8-6oZ-5@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On 07/03/2015 05:54 PM, Rafael J. Wysocki wrote:
> On Friday, July 03, 2015 01:51:36 PM Al Stone wrote:
>> On 07/03/2015 08:06 AM, Catalin Marinas wrote:
>>> On Thu, Jul 02, 2015 at 05:48:35PM -0600, Al Stone wrote:
>>>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>>>> index 39248d3..a3c26a4 100644
>>>> --- a/arch/arm64/include/asm/acpi.h
>>>> +++ b/arch/arm64/include/asm/acpi.h
>>>> @@ -19,6 +19,17 @@
>>>> #include <asm/psci.h>
>>>> #include <asm/smp_plat.h>
>>>>
>>>> +/* Macros for consistency checks of the GICC subtable of MADT */
>>>> +#define ACPI_MADT_GICC_51_LENGTH 76
>>>> +#define ACPI_MADT_GICC_60_LENGTH 80
>>>> +
>>>> +#define BAD_MADT_GICC_ENTRY(entry, end) ( \
>>>> + (!entry) || (unsigned long)entry + sizeof(*entry) > end || \
>>>> + ((ACPI_FADT_SPEC_VERSION == ACPI_FADT_SPEC_VERSION_51) && \
>>>> + (entry->header.length != ACPI_MADT_GICC_51_LENGTH)) || \
>>>> + ((ACPI_FADT_SPEC_VERSION == ACPI_FADT_SPEC_VERSION_60) && \
>>>> + (entry->header.length != ACPI_MADT_GICC_60_LENGTH)))
>>>
>>> This looks ugly but, well, we could live with this.
>>
>> Nod. It's right at the hairy edge of becoming a function, I think.
>>
>>> However, I'd like to avoid having to extend this macro every time we get
>>> a new spec released, like 6.1 defining another 80 or 84 etc. So, how
>>> about we only update this when there is an actual change in the length?
>>> Something like:
>>>
>>> #define ACPI_MADT_GICC_LENGTH ({ \
>>> u8 length; \
>>> if (ACPI_FADT_SPEC_VERSION < ACPI_FADT_SPEC_VERSION_6_0) \
>>> length = 76; \
>>> else \
>>> length = 80; \
>>> length; \
>>> })
>>>
>>> or just:
>>>
>>> #define ACPI_MADT_GICC_LENGTH \
>>> (ACPI_FADT_SPEC_VERSION < ACPI_FADT_SPEC_VERSION_6_0 ? 76 : 80)
>>>
>>> (the latter is simpler but may not look nice if we change it again in
>>> 6.1; though we could re-write this macro when needed, not a problem)
>>>
>>
>> Perhaps the sanity checking for the MADT subtables needs to be revisited
>> and a more general solution provided -- this is not the only MADT subtable
>> with this problem and it may occur again.
>>
>> Even the versions above are not technically compliant with the spec. If
>> we implement what the spec currently says, it might look something like
>> this:
>>
>> #define ACPI_MADT_GICC_LENGTH ({ \
>> u8 length; \
>> switch (ACPI_FADT_SPEC_VERSION) { \
>> case ACPI_FADT_SPEC_VERSION_5_0: \
>> length = 40; \
>> break; \
>> case ACPI_FADT_SPEC_VERSION_5_1: \
>> length = 76; \
>> break; \
>> default: /* use 6.0 size */ \
>> length = 80; \
>> } \
>> length; \
>> })
>>
>> So it's just messy and there will be a need for change. Let me think about
>> making this a function instead of a macro; it may make sense to really fix
>> BAD_MADT_ENTRY in general instead of just dealing with the GICC subtable,
>> but it could also be overkill.
>
> So here's my suggestion.
>
> First, make ARM64 boot with 4.2+ in the simplest way possible.
ACK.
> Second, set out to fix BAD_MADT_ENTRY() etc. Start with fixing ACPICA to
> distinguish between the different formats depending on the spec version and
> follow up from there.
>
> Thanks,
> Rafael
Yup, that's what I was thinking.
--
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3@redhat.com
-----------------------------------
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v3 0/3] Correct for ACPI 5.1->6.0 spec changes in MADT GICC entries Al Stone <al.stone@linaro.org> - 2015-07-03 01:50 +0200
[PATCH v3 3/3] ACPI / ARM64 : use the new BAD_MADT_GICC_ENTRY macro Al Stone <al.stone@linaro.org> - 2015-07-03 01:50 +0200
[PATCH v3 2/3] ACPI / ARM64: add BAD_MADT_GICC_ENTRY() macro Al Stone <al.stone@linaro.org> - 2015-07-03 01:50 +0200
Re: [PATCH v3 2/3] ACPI / ARM64: add BAD_MADT_GICC_ENTRY() macro Catalin Marinas <catalin.marinas@arm.com> - 2015-07-03 16:10 +0200
Re: [PATCH v3 2/3] ACPI / ARM64: add BAD_MADT_GICC_ENTRY() macro Al Stone <ahs3@redhat.com> - 2015-07-06 23:30 +0200
[PATCH v3 1/3] ACPI : introduce macros for using the ACPI specification version Al Stone <al.stone@linaro.org> - 2015-07-03 01:50 +0200
Re: [PATCH v3 1/3] ACPI : introduce macros for using the ACPI specification version "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-07-03 02:00 +0200
Re: [PATCH v3 1/3] ACPI : introduce macros for using the ACPI specification version Al Stone <ahs3@redhat.com> - 2015-07-07 00:00 +0200
Re: [PATCH v3 1/3] ACPI : introduce macros for using the ACPI specification version "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-07-07 00:20 +0200
csiph-web