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@xxxxxxxxxx ----------------------------------- -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html