| Message ID | 1468435305-24896-1-git-send-email-jhugo@codeaurora.org |
|---|---|
| State | Superseded |
| Headers | show |
On 2016-07-14 02:41 AM, Jeffrey Hugo wrote: > The ACPI spec defines the MADT GIC ITS subtable length as 20 bytes. The > test is checking to see if the subtable is 16 bytes in length which causes > an invalid test failure for MADT tables with GIC ITS subtables that follow > the spec. > > Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org> > --- > src/acpi/madt/madt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c > index b65b89e..65d3962 100644 > --- a/src/acpi/madt/madt.c > +++ b/src/acpi/madt/madt.c > @@ -183,7 +183,7 @@ static struct acpi_madt_subtable_lengths spec_info[] = { > .madt_version = 3, > .num_types = 16, > .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, > - 16, 16, 12, 80, 24, 24, 16, 16 } > + 16, 16, 12, 80, 24, 24, 16, 20 } > }, Hi Jeffrey, Thanks a lot for pointing this out. I noticed the ITS's length is 16 in ACPI 6.0 and it is updated to 20 in ACPI 6.1 (See revision history "1487 The Length of GIC ITS Structure is wrong"). However, the madt's revision also changed from 3 to 4 1. talk about to this change in this patch 2. keep the original structure and add a new structure for revision 4, ex. { /* for ACPI 6.0 */ .major_version = 6, .minor_version = 0, .madt_version = 3, .num_types = 16, .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, 16, 16, 12, 80, 24, 24, 16, 16 } }, { /* for ACPI 6.1 */ .major_version = 6, .minor_version = 0, .madt_version = 4, .num_types = 16, .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, 16, 16, 12, 80, 24, 24, 16, 20 } Hi Al, Will you be able to give some suggestions? > { /* terminator */ > .major_version = 0, >
On 7/14/2016 12:50 AM, Alex Hung wrote: > On 2016-07-14 02:41 AM, Jeffrey Hugo wrote: >> The ACPI spec defines the MADT GIC ITS subtable length as 20 bytes. The >> test is checking to see if the subtable is 16 bytes in length which >> causes >> an invalid test failure for MADT tables with GIC ITS subtables that >> follow >> the spec. >> >> Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org> >> --- >> src/acpi/madt/madt.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c >> index b65b89e..65d3962 100644 >> --- a/src/acpi/madt/madt.c >> +++ b/src/acpi/madt/madt.c >> @@ -183,7 +183,7 @@ static struct acpi_madt_subtable_lengths >> spec_info[] = { >> .madt_version = 3, >> .num_types = 16, >> .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, >> - 16, 16, 12, 80, 24, 24, 16, 16 } >> + 16, 16, 12, 80, 24, 24, 16, 20 } >> }, > > Hi Jeffrey, > > Thanks a lot for pointing this out. > > I noticed the ITS's length is 16 in ACPI 6.0 and it is updated to 20 in > ACPI 6.1 (See revision history "1487 The Length of GIC ITS Structure is > wrong"). However, the madt's revision also changed from 3 to 4 Drat, I must have missed that. Thanks for pointing it out. > > 1. talk about to this change in this patch > 2. keep the original structure and add a new structure for revision 4, ex. > { /* for ACPI 6.0 */ > .major_version = 6, > .minor_version = 0, > .madt_version = 3, > .num_types = 16, > .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, > 16, 16, 12, 80, 24, 24, 16, 16 } > }, > { /* for ACPI 6.1 */ > .major_version = 6, > .minor_version = 0, > .madt_version = 4, > .num_types = 16, > .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, > 16, 16, 12, 80, 24, 24, 16, 20 } Off the top of my head, this appears to make sense, but I'll play around with it while we wait for Al to commment. > > Hi Al, > > Will you be able to give some suggestions? > > >> { /* terminator */ >> .major_version = 0, >> > >
On 07/14/2016 09:29 AM, Jeffrey Hugo wrote: > On 7/14/2016 12:50 AM, Alex Hung wrote: >> On 2016-07-14 02:41 AM, Jeffrey Hugo wrote: >>> The ACPI spec defines the MADT GIC ITS subtable length as 20 bytes. The >>> test is checking to see if the subtable is 16 bytes in length which >>> causes >>> an invalid test failure for MADT tables with GIC ITS subtables that >>> follow >>> the spec. >>> >>> Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org> >>> --- >>> src/acpi/madt/madt.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c >>> index b65b89e..65d3962 100644 >>> --- a/src/acpi/madt/madt.c >>> +++ b/src/acpi/madt/madt.c >>> @@ -183,7 +183,7 @@ static struct acpi_madt_subtable_lengths >>> spec_info[] = { >>> .madt_version = 3, >>> .num_types = 16, >>> .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, >>> - 16, 16, 12, 80, 24, 24, 16, 16 } >>> + 16, 16, 12, 80, 24, 24, 16, 20 } >>> }, >> >> Hi Jeffrey, >> >> Thanks a lot for pointing this out. >> >> I noticed the ITS's length is 16 in ACPI 6.0 and it is updated to 20 in >> ACPI 6.1 (See revision history "1487 The Length of GIC ITS Structure is >> wrong"). However, the madt's revision also changed from 3 to 4 > > Drat, I must have missed that. Thanks for pointing it out. > >> >> 1. talk about to this change in this patch >> 2. keep the original structure and add a new structure for revision 4, ex. >> { /* for ACPI 6.0 */ >> .major_version = 6, >> .minor_version = 0, >> .madt_version = 3, >> .num_types = 16, >> .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, >> 16, 16, 12, 80, 24, 24, 16, 16 } >> }, >> { /* for ACPI 6.1 */ >> .major_version = 6, >> .minor_version = 0, >> .madt_version = 4, >> .num_types = 16, >> .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, >> 16, 16, 12, 80, 24, 24, 16, 20 } > > Off the top of my head, this appears to make sense, but I'll play around with it > while we wait for Al to commment. > >> >> Hi Al, >> >> Will you be able to give some suggestions? >> >> >>> { /* terminator */ >>> .major_version = 0, >>> >> >> > > Oh, ick. So, if we look at the ACPI 6.0 spec, it does very clearly say the ITS subtable has a length of 16. But, if you add up the lengths of all the fields, the length should be 20. I would recommend we just accept that 6.0 has a typo (which is part of why 6.0a came out, btw) and incorporate this patch as is. If we leave it at 16, it's going to really mess up any traversals of the subtables.
diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c index b65b89e..65d3962 100644 --- a/src/acpi/madt/madt.c +++ b/src/acpi/madt/madt.c @@ -183,7 +183,7 @@ static struct acpi_madt_subtable_lengths spec_info[] = { .madt_version = 3, .num_types = 16, .lengths = { 8, 12, 10, 8, 6, 12, 16, SUBTABLE_VARIABLE, - 16, 16, 12, 80, 24, 24, 16, 16 } + 16, 16, 12, 80, 24, 24, 16, 20 } }, { /* terminator */ .major_version = 0,
The ACPI spec defines the MADT GIC ITS subtable length as 20 bytes. The test is checking to see if the subtable is 16 bytes in length which causes an invalid test failure for MADT tables with GIC ITS subtables that follow the spec. Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org> --- src/acpi/madt/madt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)