| Message ID | 1468797499-8715-2-git-send-email-jhugo@codeaurora.org |
|---|---|
| State | Rejected |
| Headers | show |
On 07/17/2016 05:18 PM, Jeffrey Hugo wrote: > ACPI 6.1 introduces a new FADT/MADT version combination. > > Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org> > --- > src/acpi/madt/madt.c | 50 +++++++++++++++++++++++++++++--------------------- > 1 file changed, 29 insertions(+), 21 deletions(-) > > diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c > index 0926f64..9218a75 100644 > --- a/src/acpi/madt/madt.c > +++ b/src/acpi/madt/madt.c > @@ -84,27 +84,27 @@ > * subtable is doing okay. And that's what our tests below check. > * > * > - * FADT Major Version -> 1 3 4 4 5 5 6 6 > - * FADT Minor Version -> x x x x x 1 0 0 > - * MADT revision -> 1 1 2 3 3 3 3 4 > - * Spec Version -> 1.0 2.0 3.0b 4.0a 5.0b 5.1a 6.0 6.0a > + * FADT Major Version -> 1 3 4 4 5 5 6 6 6 > + * FADT Minor Version -> x x x x x 1 0 0 1 > + * MADT revision -> 1 1 2 3 3 3 3 4 4 > + * Spec Version -> 1.0 2.0 3.0b 4.0a 5.0b 5.1a 6.0 6.0a 6.1 > * Subtable Name Type Expected Length -> > - * Processor Local APIC 0x0 8 8 8 8 8 8 8 8 > - * IO APIC 0x1 12 12 12 12 12 12 12 12 > - * Int Src Override 0x2 10 10 10 10 10 10 10 10 > - * NMI Src 0x3 8 8 8 8 8 8 8 8 > - * Local APIC NMI Struct 0x4 6 6 6 6 6 6 6 6 > - * Local APIC Addr Ovrrd 0x5 16 12 12 12 12 12 12 > - * IO SAPIC 0x6 20 16 16 16 16 16 16 > - * Local SAPIC 0x7 8 >16 >16 >16 >16 >16 >16 > - * Platform Int Src 0x8 16 16 16 16 16 16 16 > - * Proc Local x2APIC 0x9 16 16 16 16 16 > - * Local x2APIC NMI 0xa 12 12 12 12 12 > - * GICC CPU I/F 0xb 40 76 80 80 > - * GICD 0xc 24 24 24 24 > - * GICv2m MSI 0xd 24 24 24 > - * GICR 0xe 16 16 16 > - * GIC ITS 0xf 16 20 > + * Processor Local APIC 0x0 8 8 8 8 8 8 8 8 8 > + * IO APIC 0x1 12 12 12 12 12 12 12 12 12 > + * Int Src Override 0x2 10 10 10 10 10 10 10 10 10 > + * NMI Src 0x3 8 8 8 8 8 8 8 8 8 > + * Local APIC NMI Struct 0x4 6 6 6 6 6 6 6 6 6 > + * Local APIC Addr Ovrrd 0x5 16 12 12 12 12 12 12 12 > + * IO SAPIC 0x6 20 16 16 16 16 16 16 16 > + * Local SAPIC 0x7 8 >16 >16 >16 >16 >16 >16 >16 > + * Platform Int Src 0x8 16 16 16 16 16 16 16 16 > + * Proc Local x2APIC 0x9 16 16 16 16 16 16 > + * Local x2APIC NMI 0xa 12 12 12 12 12 12 > + * GICC CPU I/F 0xb 40 76 80 80 80 > + * GICD 0xc 24 24 24 24 24 > + * GICv2m MSI 0xd 24 24 24 24 > + * GICR 0xe 16 16 16 16 > + * GIC ITS 0xf 16 20 20 Again, as mentioned before, I would recommend making the ITS length 20 for the ACPI 6.0 entry. Otherwise, this all looks good. And thanks again for submitting these! > * > * In the table, each length entry is what should be in the length > * field of the subtable, and -- in general -- it should match the > @@ -115,7 +115,7 @@ > */ > > #define FADT_MAX_MAJOR_REVISION ((uint8_t)6) > -#define FADT_MAX_MINOR_REVISION ((uint8_t)0) > +#define FADT_MAX_MINOR_REVISION ((uint8_t)1) > #define MADT_MAX_REVISION ((uint8_t)4) > > #define SUBTABLE_UNDEFINED 0x00 > @@ -193,6 +193,14 @@ static struct acpi_madt_subtable_lengths spec_info[] = { > .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, > 16, 16, 12, 80, 24, 24, 16, 20 } > }, > + { /* for ACPI 6.1 */ > + .major_version = 6, > + .minor_version = 1, > + .madt_version = 4, > + .num_types = 16, > + .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, > + 16, 16, 12, 80, 24, 24, 16, 20 } > + }, > { /* terminator */ > .major_version = 0, > .minor_version = 0, >
On 7/19/2016 3:19 PM, Al Stone wrote: > On 07/17/2016 05:18 PM, Jeffrey Hugo wrote: >> ACPI 6.1 introduces a new FADT/MADT version combination. >> >> Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org> >> --- >> src/acpi/madt/madt.c | 50 +++++++++++++++++++++++++++++--------------------- >> 1 file changed, 29 insertions(+), 21 deletions(-) >> >> diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c >> index 0926f64..9218a75 100644 >> --- a/src/acpi/madt/madt.c >> +++ b/src/acpi/madt/madt.c >> @@ -84,27 +84,27 @@ >> * subtable is doing okay. And that's what our tests below check. >> * >> * >> - * FADT Major Version -> 1 3 4 4 5 5 6 6 >> - * FADT Minor Version -> x x x x x 1 0 0 >> - * MADT revision -> 1 1 2 3 3 3 3 4 >> - * Spec Version -> 1.0 2.0 3.0b 4.0a 5.0b 5.1a 6.0 6.0a >> + * FADT Major Version -> 1 3 4 4 5 5 6 6 6 >> + * FADT Minor Version -> x x x x x 1 0 0 1 >> + * MADT revision -> 1 1 2 3 3 3 3 4 4 >> + * Spec Version -> 1.0 2.0 3.0b 4.0a 5.0b 5.1a 6.0 6.0a 6.1 >> * Subtable Name Type Expected Length -> >> - * Processor Local APIC 0x0 8 8 8 8 8 8 8 8 >> - * IO APIC 0x1 12 12 12 12 12 12 12 12 >> - * Int Src Override 0x2 10 10 10 10 10 10 10 10 >> - * NMI Src 0x3 8 8 8 8 8 8 8 8 >> - * Local APIC NMI Struct 0x4 6 6 6 6 6 6 6 6 >> - * Local APIC Addr Ovrrd 0x5 16 12 12 12 12 12 12 >> - * IO SAPIC 0x6 20 16 16 16 16 16 16 >> - * Local SAPIC 0x7 8 >16 >16 >16 >16 >16 >16 >> - * Platform Int Src 0x8 16 16 16 16 16 16 16 >> - * Proc Local x2APIC 0x9 16 16 16 16 16 >> - * Local x2APIC NMI 0xa 12 12 12 12 12 >> - * GICC CPU I/F 0xb 40 76 80 80 >> - * GICD 0xc 24 24 24 24 >> - * GICv2m MSI 0xd 24 24 24 >> - * GICR 0xe 16 16 16 >> - * GIC ITS 0xf 16 20 >> + * Processor Local APIC 0x0 8 8 8 8 8 8 8 8 8 >> + * IO APIC 0x1 12 12 12 12 12 12 12 12 12 >> + * Int Src Override 0x2 10 10 10 10 10 10 10 10 10 >> + * NMI Src 0x3 8 8 8 8 8 8 8 8 8 >> + * Local APIC NMI Struct 0x4 6 6 6 6 6 6 6 6 6 >> + * Local APIC Addr Ovrrd 0x5 16 12 12 12 12 12 12 12 >> + * IO SAPIC 0x6 20 16 16 16 16 16 16 16 >> + * Local SAPIC 0x7 8 >16 >16 >16 >16 >16 >16 >16 >> + * Platform Int Src 0x8 16 16 16 16 16 16 16 16 >> + * Proc Local x2APIC 0x9 16 16 16 16 16 16 >> + * Local x2APIC NMI 0xa 12 12 12 12 12 12 >> + * GICC CPU I/F 0xb 40 76 80 80 80 >> + * GICD 0xc 24 24 24 24 24 >> + * GICv2m MSI 0xd 24 24 24 24 >> + * GICR 0xe 16 16 16 16 >> + * GIC ITS 0xf 16 20 20 > > Again, as mentioned before, I would recommend making the ITS length 20 for > the ACPI 6.0 entry. Maybe not clear from the diff, but the documentation was 20, and the implementation was 16. I decided that the documentation here should match the implementation, since it seemed like the desire was to keep the implementation to match the spec. I think that making the implementation match the intent of the spec (length 20) per your comment on "Re: [PATCH] acpi: madt: Fix ITS subtable length check" makes sense, perhaps with a short note about why fwts diverged from the spec. I'll add that to this series and repost. > > Otherwise, this all looks good. And thanks again for submitting these! > >> * >> * In the table, each length entry is what should be in the length >> * field of the subtable, and -- in general -- it should match the >> @@ -115,7 +115,7 @@ >> */ >> >> #define FADT_MAX_MAJOR_REVISION ((uint8_t)6) >> -#define FADT_MAX_MINOR_REVISION ((uint8_t)0) >> +#define FADT_MAX_MINOR_REVISION ((uint8_t)1) >> #define MADT_MAX_REVISION ((uint8_t)4) >> >> #define SUBTABLE_UNDEFINED 0x00 >> @@ -193,6 +193,14 @@ static struct acpi_madt_subtable_lengths spec_info[] = { >> .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, >> 16, 16, 12, 80, 24, 24, 16, 20 } >> }, >> + { /* for ACPI 6.1 */ >> + .major_version = 6, >> + .minor_version = 1, >> + .madt_version = 4, >> + .num_types = 16, >> + .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, >> + 16, 16, 12, 80, 24, 24, 16, 20 } >> + }, >> { /* terminator */ >> .major_version = 0, >> .minor_version = 0, >> > >
On 07/19/2016 03:34 PM, Jeffrey Hugo wrote: > On 7/19/2016 3:19 PM, Al Stone wrote: >> On 07/17/2016 05:18 PM, Jeffrey Hugo wrote: >>> ACPI 6.1 introduces a new FADT/MADT version combination. >>> >>> Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org> >>> --- >>> src/acpi/madt/madt.c | 50 +++++++++++++++++++++++++++++--------------------- >>> 1 file changed, 29 insertions(+), 21 deletions(-) >>> >>> diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c >>> index 0926f64..9218a75 100644 >>> --- a/src/acpi/madt/madt.c >>> +++ b/src/acpi/madt/madt.c >>> @@ -84,27 +84,27 @@ >>> * subtable is doing okay. And that's what our tests below check. >>> * >>> * >>> - * FADT Major Version -> 1 3 4 4 5 5 6 6 >>> - * FADT Minor Version -> x x x x x 1 0 0 >>> - * MADT revision -> 1 1 2 3 3 3 3 4 >>> - * Spec Version -> 1.0 2.0 3.0b 4.0a 5.0b 5.1a 6.0 6.0a >>> + * FADT Major Version -> 1 3 4 4 5 5 6 6 6 >>> + * FADT Minor Version -> x x x x x 1 0 0 1 >>> + * MADT revision -> 1 1 2 3 3 3 3 4 4 >>> + * Spec Version -> 1.0 2.0 3.0b 4.0a 5.0b 5.1a 6.0 6.0a 6.1 >>> * Subtable Name Type Expected Length -> >>> - * Processor Local APIC 0x0 8 8 8 8 8 8 8 8 >>> - * IO APIC 0x1 12 12 12 12 12 12 12 12 >>> - * Int Src Override 0x2 10 10 10 10 10 10 10 10 >>> - * NMI Src 0x3 8 8 8 8 8 8 8 8 >>> - * Local APIC NMI Struct 0x4 6 6 6 6 6 6 6 6 >>> - * Local APIC Addr Ovrrd 0x5 16 12 12 12 12 12 12 >>> - * IO SAPIC 0x6 20 16 16 16 16 16 16 >>> - * Local SAPIC 0x7 8 >16 >16 >16 >16 >16 >16 >>> - * Platform Int Src 0x8 16 16 16 16 16 16 16 >>> - * Proc Local x2APIC 0x9 16 16 16 16 16 >>> - * Local x2APIC NMI 0xa 12 12 12 12 12 >>> - * GICC CPU I/F 0xb 40 76 80 80 >>> - * GICD 0xc 24 24 24 24 >>> - * GICv2m MSI 0xd 24 24 24 >>> - * GICR 0xe 16 16 16 >>> - * GIC ITS 0xf 16 20 >>> + * Processor Local APIC 0x0 8 8 8 8 8 8 8 8 8 >>> + * IO APIC 0x1 12 12 12 12 12 12 12 12 12 >>> + * Int Src Override 0x2 10 10 10 10 10 10 10 10 10 >>> + * NMI Src 0x3 8 8 8 8 8 8 8 8 8 >>> + * Local APIC NMI Struct 0x4 6 6 6 6 6 6 6 6 6 >>> + * Local APIC Addr Ovrrd 0x5 16 12 12 12 12 12 12 12 >>> + * IO SAPIC 0x6 20 16 16 16 16 16 16 16 >>> + * Local SAPIC 0x7 8 >16 >16 >16 >16 >16 >16 >16 >>> + * Platform Int Src 0x8 16 16 16 16 16 16 16 16 >>> + * Proc Local x2APIC 0x9 16 16 16 16 16 16 >>> + * Local x2APIC NMI 0xa 12 12 12 12 12 12 >>> + * GICC CPU I/F 0xb 40 76 80 80 80 >>> + * GICD 0xc 24 24 24 24 24 >>> + * GICv2m MSI 0xd 24 24 24 24 >>> + * GICR 0xe 16 16 16 16 >>> + * GIC ITS 0xf 16 20 20 >> >> Again, as mentioned before, I would recommend making the ITS length 20 for >> the ACPI 6.0 entry. > > Maybe not clear from the diff, but the documentation was 20, and the > implementation was 16. I decided that the documentation here should match the > implementation, since it seemed like the desire was to keep the implementation > to match the spec. Oh, I see. I wasn't paying close enough attention; sorry about that. Agreed, though -- the documentation and implementation should match. > I think that making the implementation match the intent of the spec (length 20) > per your comment on "Re: [PATCH] acpi: madt: Fix ITS subtable length check" > makes sense, perhaps with a short note about why fwts diverged from the spec. That would be most excellent. Thanks. > I'll add that to this series and repost. > >> >> Otherwise, this all looks good. And thanks again for submitting these! >> >>> * >>> * In the table, each length entry is what should be in the length >>> * field of the subtable, and -- in general -- it should match the >>> @@ -115,7 +115,7 @@ >>> */ >>> >>> #define FADT_MAX_MAJOR_REVISION ((uint8_t)6) >>> -#define FADT_MAX_MINOR_REVISION ((uint8_t)0) >>> +#define FADT_MAX_MINOR_REVISION ((uint8_t)1) >>> #define MADT_MAX_REVISION ((uint8_t)4) >>> >>> #define SUBTABLE_UNDEFINED 0x00 >>> @@ -193,6 +193,14 @@ static struct acpi_madt_subtable_lengths spec_info[] = { >>> .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, >>> 16, 16, 12, 80, 24, 24, 16, 20 } >>> }, >>> + { /* for ACPI 6.1 */ >>> + .major_version = 6, >>> + .minor_version = 1, >>> + .madt_version = 4, >>> + .num_types = 16, >>> + .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, >>> + 16, 16, 12, 80, 24, 24, 16, 20 } >>> + }, >>> { /* terminator */ >>> .major_version = 0, >>> .minor_version = 0, >>> >> >> > >
diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c index 0926f64..9218a75 100644 --- a/src/acpi/madt/madt.c +++ b/src/acpi/madt/madt.c @@ -84,27 +84,27 @@ * subtable is doing okay. And that's what our tests below check. * * - * FADT Major Version -> 1 3 4 4 5 5 6 6 - * FADT Minor Version -> x x x x x 1 0 0 - * MADT revision -> 1 1 2 3 3 3 3 4 - * Spec Version -> 1.0 2.0 3.0b 4.0a 5.0b 5.1a 6.0 6.0a + * FADT Major Version -> 1 3 4 4 5 5 6 6 6 + * FADT Minor Version -> x x x x x 1 0 0 1 + * MADT revision -> 1 1 2 3 3 3 3 4 4 + * Spec Version -> 1.0 2.0 3.0b 4.0a 5.0b 5.1a 6.0 6.0a 6.1 * Subtable Name Type Expected Length -> - * Processor Local APIC 0x0 8 8 8 8 8 8 8 8 - * IO APIC 0x1 12 12 12 12 12 12 12 12 - * Int Src Override 0x2 10 10 10 10 10 10 10 10 - * NMI Src 0x3 8 8 8 8 8 8 8 8 - * Local APIC NMI Struct 0x4 6 6 6 6 6 6 6 6 - * Local APIC Addr Ovrrd 0x5 16 12 12 12 12 12 12 - * IO SAPIC 0x6 20 16 16 16 16 16 16 - * Local SAPIC 0x7 8 >16 >16 >16 >16 >16 >16 - * Platform Int Src 0x8 16 16 16 16 16 16 16 - * Proc Local x2APIC 0x9 16 16 16 16 16 - * Local x2APIC NMI 0xa 12 12 12 12 12 - * GICC CPU I/F 0xb 40 76 80 80 - * GICD 0xc 24 24 24 24 - * GICv2m MSI 0xd 24 24 24 - * GICR 0xe 16 16 16 - * GIC ITS 0xf 16 20 + * Processor Local APIC 0x0 8 8 8 8 8 8 8 8 8 + * IO APIC 0x1 12 12 12 12 12 12 12 12 12 + * Int Src Override 0x2 10 10 10 10 10 10 10 10 10 + * NMI Src 0x3 8 8 8 8 8 8 8 8 8 + * Local APIC NMI Struct 0x4 6 6 6 6 6 6 6 6 6 + * Local APIC Addr Ovrrd 0x5 16 12 12 12 12 12 12 12 + * IO SAPIC 0x6 20 16 16 16 16 16 16 16 + * Local SAPIC 0x7 8 >16 >16 >16 >16 >16 >16 >16 + * Platform Int Src 0x8 16 16 16 16 16 16 16 16 + * Proc Local x2APIC 0x9 16 16 16 16 16 16 + * Local x2APIC NMI 0xa 12 12 12 12 12 12 + * GICC CPU I/F 0xb 40 76 80 80 80 + * GICD 0xc 24 24 24 24 24 + * GICv2m MSI 0xd 24 24 24 24 + * GICR 0xe 16 16 16 16 + * GIC ITS 0xf 16 20 20 * * In the table, each length entry is what should be in the length * field of the subtable, and -- in general -- it should match the @@ -115,7 +115,7 @@ */ #define FADT_MAX_MAJOR_REVISION ((uint8_t)6) -#define FADT_MAX_MINOR_REVISION ((uint8_t)0) +#define FADT_MAX_MINOR_REVISION ((uint8_t)1) #define MADT_MAX_REVISION ((uint8_t)4) #define SUBTABLE_UNDEFINED 0x00 @@ -193,6 +193,14 @@ static struct acpi_madt_subtable_lengths spec_info[] = { .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, 16, 16, 12, 80, 24, 24, 16, 20 } }, + { /* for ACPI 6.1 */ + .major_version = 6, + .minor_version = 1, + .madt_version = 4, + .num_types = 16, + .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, + 16, 16, 12, 80, 24, 24, 16, 20 } + }, { /* terminator */ .major_version = 0, .minor_version = 0,
ACPI 6.1 introduces a new FADT/MADT version combination. Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org> --- src/acpi/madt/madt.c | 50 +++++++++++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 21 deletions(-)