| Message ID | 1511830554-6602-1-git-send-email-ricardo.neri-calderon@linux.intel.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | acpi: madt: only inspect Local APIC/x2APIC/SAPIC tables if enabled | expand |
On 2017-11-28 08:55 AM, Ricardo Neri wrote: > The Advanced Configuration and Power Interface (ACPI) specification > version 6.2 Errata A in sections 5.2.12.2, 5.2.12.10 and 5.2.12.12 states > that "when a processor is not present, the Processor Local APIC/x2APIC/ > SAPIC information is either not reported or flagged as disabled." > > This implies that it is possible for firmware implementations to include > Local APIC/SAPIC/x2APIC tables for non-existent processors provided that > they are disabled. > > Thus, only test these tables if they have the Enable flag set. Otherwise, > skip the test. Also, print a message to record the fact that these tables > exist but are not tested. > > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> > Reported-by: Bin Song <binx.song@intel.com> > --- > src/acpi/madt/madt.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c > index 3bbaf2f..f997a5f 100644 > --- a/src/acpi/madt/madt.c > +++ b/src/acpi/madt/madt.c > @@ -546,6 +546,13 @@ static int madt_local_apic(fwts_framework *fw, > fwts_acpi_madt_processor_local_apic *lapic = > (fwts_acpi_madt_processor_local_apic *)data; > > + /* only test table if enabled */ > + if (!(lapic->flags & 0x1)) { > + fwts_skipped(fw, "MADT %s is disabled. Skipping test.", > + madt_sub_names[hdr->type]); > + goto out; > + } > + > madt_find_processor_uid(fw, lapic->acpi_processor_id, "LAPIC"); > > if (lapic->flags & 0xfffffffe) > @@ -561,6 +568,7 @@ static int madt_local_apic(fwts_framework *fw, > "reserved and properly set to zero.", > madt_sub_names[hdr->type]); > > +out: > return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); > } > > @@ -780,6 +788,7 @@ static int madt_local_sapic(fwts_framework *fw, > * initial boot state. > */ > > + > if (hdr->length != (strlen(lsapic->uid_string) + 16)) > fwts_failed(fw, LOG_LEVEL_MEDIUM, > "MADTLSAPICLength", > @@ -792,6 +801,13 @@ static int madt_local_sapic(fwts_framework *fw, > "MADT %s table length (%d) is correct.", > madt_sub_names[hdr->type], hdr->length); > > + /* only continue testing table if enabled */ > + if (!(lsapic->flags & 0x1)) { > + fwts_skipped(fw, "MADT %s is disabled. Skipping test.", > + madt_sub_names[hdr->type]); > + goto out; > + } > + > /* > * There are three values that need to be checked for a valid > * processor UID: the ACPI processor ID, the UID value, and the UID > @@ -878,6 +894,7 @@ static int madt_local_sapic(fwts_framework *fw, > "MADT %s UID string is an ASCII string.", > madt_sub_names[hdr->type]); > > +out: > return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); > } > > @@ -952,6 +969,13 @@ static int madt_local_x2apic(fwts_framework *fw, > fwts_acpi_madt_local_x2apic *lx2apic = > (fwts_acpi_madt_local_x2apic *)data; > > + /* only test table if enabled */ > + if (!(lx2apic->flags & 0x1)) { > + fwts_skipped(fw, "MADT %s is disabled. Skipping test.", > + madt_sub_names[hdr->type]); > + goto out; > + } > + > if (lx2apic->reserved) > fwts_failed(fw, LOG_LEVEL_MEDIUM, > "MADTLX2APICReservedNonZero", > @@ -980,6 +1004,7 @@ static int madt_local_x2apic(fwts_framework *fw, > > madt_find_processor_uid(fw, lx2apic->processor_uid, "X2APIC"); > > +out: > return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); > } > > Acked-by: Alex Hung <alex.hung@canonical.com>
On 11/28/2017 08:55 AM, Ricardo Neri wrote: > The Advanced Configuration and Power Interface (ACPI) specification > version 6.2 Errata A in sections 5.2.12.2, 5.2.12.10 and 5.2.12.12 states > that "when a processor is not present, the Processor Local APIC/x2APIC/ > SAPIC information is either not reported or flagged as disabled." > > This implies that it is possible for firmware implementations to include > Local APIC/SAPIC/x2APIC tables for non-existent processors provided that > they are disabled. > > Thus, only test these tables if they have the Enable flag set. Otherwise, > skip the test. Also, print a message to record the fact that these tables > exist but are not tested. > > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> > Reported-by: Bin Song <binx.song@intel.com> > --- > src/acpi/madt/madt.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c > index 3bbaf2f..f997a5f 100644 > --- a/src/acpi/madt/madt.c > +++ b/src/acpi/madt/madt.c > @@ -546,6 +546,13 @@ static int madt_local_apic(fwts_framework *fw, > fwts_acpi_madt_processor_local_apic *lapic = > (fwts_acpi_madt_processor_local_apic *)data; > > + /* only test table if enabled */ > + if (!(lapic->flags & 0x1)) { > + fwts_skipped(fw, "MADT %s is disabled. Skipping test.", > + madt_sub_names[hdr->type]); > + goto out; > + } > + > madt_find_processor_uid(fw, lapic->acpi_processor_id, "LAPIC"); > > if (lapic->flags & 0xfffffffe) > @@ -561,6 +568,7 @@ static int madt_local_apic(fwts_framework *fw, > "reserved and properly set to zero.", > madt_sub_names[hdr->type]); > > +out: > return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); > } > > @@ -780,6 +788,7 @@ static int madt_local_sapic(fwts_framework *fw, > * initial boot state. > */ > > + > if (hdr->length != (strlen(lsapic->uid_string) + 16)) > fwts_failed(fw, LOG_LEVEL_MEDIUM, > "MADTLSAPICLength", > @@ -792,6 +801,13 @@ static int madt_local_sapic(fwts_framework *fw, > "MADT %s table length (%d) is correct.", > madt_sub_names[hdr->type], hdr->length); > > + /* only continue testing table if enabled */ > + if (!(lsapic->flags & 0x1)) { > + fwts_skipped(fw, "MADT %s is disabled. Skipping test.", > + madt_sub_names[hdr->type]); > + goto out; > + } > + > /* > * There are three values that need to be checked for a valid > * processor UID: the ACPI processor ID, the UID value, and the UID > @@ -878,6 +894,7 @@ static int madt_local_sapic(fwts_framework *fw, > "MADT %s UID string is an ASCII string.", > madt_sub_names[hdr->type]); > > +out: > return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); > } > > @@ -952,6 +969,13 @@ static int madt_local_x2apic(fwts_framework *fw, > fwts_acpi_madt_local_x2apic *lx2apic = > (fwts_acpi_madt_local_x2apic *)data; > > + /* only test table if enabled */ > + if (!(lx2apic->flags & 0x1)) { > + fwts_skipped(fw, "MADT %s is disabled. Skipping test.", > + madt_sub_names[hdr->type]); > + goto out; > + } > + > if (lx2apic->reserved) > fwts_failed(fw, LOG_LEVEL_MEDIUM, > "MADTLX2APICReservedNonZero", > @@ -980,6 +1004,7 @@ static int madt_local_x2apic(fwts_framework *fw, > > madt_find_processor_uid(fw, lx2apic->processor_uid, "X2APIC"); > > +out: > return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); > } > > Acked-by: Ivan Hu <ivan.hu@canonical.com>
On 28/11/17 00:55, Ricardo Neri wrote: > The Advanced Configuration and Power Interface (ACPI) specification > version 6.2 Errata A in sections 5.2.12.2, 5.2.12.10 and 5.2.12.12 states > that "when a processor is not present, the Processor Local APIC/x2APIC/ > SAPIC information is either not reported or flagged as disabled." > > This implies that it is possible for firmware implementations to include > Local APIC/SAPIC/x2APIC tables for non-existent processors provided that > they are disabled. > > Thus, only test these tables if they have the Enable flag set. Otherwise, > skip the test. Also, print a message to record the fact that these tables > exist but are not tested. > > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> > Reported-by: Bin Song <binx.song@intel.com> > --- > src/acpi/madt/madt.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c > index 3bbaf2f..f997a5f 100644 > --- a/src/acpi/madt/madt.c > +++ b/src/acpi/madt/madt.c > @@ -546,6 +546,13 @@ static int madt_local_apic(fwts_framework *fw, > fwts_acpi_madt_processor_local_apic *lapic = > (fwts_acpi_madt_processor_local_apic *)data; > > + /* only test table if enabled */ > + if (!(lapic->flags & 0x1)) { > + fwts_skipped(fw, "MADT %s is disabled. Skipping test.", > + madt_sub_names[hdr->type]); > + goto out; > + } > + > madt_find_processor_uid(fw, lapic->acpi_processor_id, "LAPIC"); > > if (lapic->flags & 0xfffffffe) > @@ -561,6 +568,7 @@ static int madt_local_apic(fwts_framework *fw, > "reserved and properly set to zero.", > madt_sub_names[hdr->type]); > > +out: > return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); > } > > @@ -780,6 +788,7 @@ static int madt_local_sapic(fwts_framework *fw, > * initial boot state. > */ > > + > if (hdr->length != (strlen(lsapic->uid_string) + 16)) > fwts_failed(fw, LOG_LEVEL_MEDIUM, > "MADTLSAPICLength", > @@ -792,6 +801,13 @@ static int madt_local_sapic(fwts_framework *fw, > "MADT %s table length (%d) is correct.", > madt_sub_names[hdr->type], hdr->length); > > + /* only continue testing table if enabled */ > + if (!(lsapic->flags & 0x1)) { > + fwts_skipped(fw, "MADT %s is disabled. Skipping test.", > + madt_sub_names[hdr->type]); > + goto out; > + } > + > /* > * There are three values that need to be checked for a valid > * processor UID: the ACPI processor ID, the UID value, and the UID > @@ -878,6 +894,7 @@ static int madt_local_sapic(fwts_framework *fw, > "MADT %s UID string is an ASCII string.", > madt_sub_names[hdr->type]); > > +out: > return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); > } > > @@ -952,6 +969,13 @@ static int madt_local_x2apic(fwts_framework *fw, > fwts_acpi_madt_local_x2apic *lx2apic = > (fwts_acpi_madt_local_x2apic *)data; > > + /* only test table if enabled */ > + if (!(lx2apic->flags & 0x1)) { > + fwts_skipped(fw, "MADT %s is disabled. Skipping test.", > + madt_sub_names[hdr->type]); > + goto out; > + } > + > if (lx2apic->reserved) > fwts_failed(fw, LOG_LEVEL_MEDIUM, > "MADTLX2APICReservedNonZero", > @@ -980,6 +1004,7 @@ static int madt_local_x2apic(fwts_framework *fw, > > madt_find_processor_uid(fw, lx2apic->processor_uid, "X2APIC"); > > +out: > return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); > } > > Thanks Ricardo, Acked-by: Colin Ian King <colin.king@canonical.com>
diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c index 3bbaf2f..f997a5f 100644 --- a/src/acpi/madt/madt.c +++ b/src/acpi/madt/madt.c @@ -546,6 +546,13 @@ static int madt_local_apic(fwts_framework *fw, fwts_acpi_madt_processor_local_apic *lapic = (fwts_acpi_madt_processor_local_apic *)data; + /* only test table if enabled */ + if (!(lapic->flags & 0x1)) { + fwts_skipped(fw, "MADT %s is disabled. Skipping test.", + madt_sub_names[hdr->type]); + goto out; + } + madt_find_processor_uid(fw, lapic->acpi_processor_id, "LAPIC"); if (lapic->flags & 0xfffffffe) @@ -561,6 +568,7 @@ static int madt_local_apic(fwts_framework *fw, "reserved and properly set to zero.", madt_sub_names[hdr->type]); +out: return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); } @@ -780,6 +788,7 @@ static int madt_local_sapic(fwts_framework *fw, * initial boot state. */ + if (hdr->length != (strlen(lsapic->uid_string) + 16)) fwts_failed(fw, LOG_LEVEL_MEDIUM, "MADTLSAPICLength", @@ -792,6 +801,13 @@ static int madt_local_sapic(fwts_framework *fw, "MADT %s table length (%d) is correct.", madt_sub_names[hdr->type], hdr->length); + /* only continue testing table if enabled */ + if (!(lsapic->flags & 0x1)) { + fwts_skipped(fw, "MADT %s is disabled. Skipping test.", + madt_sub_names[hdr->type]); + goto out; + } + /* * There are three values that need to be checked for a valid * processor UID: the ACPI processor ID, the UID value, and the UID @@ -878,6 +894,7 @@ static int madt_local_sapic(fwts_framework *fw, "MADT %s UID string is an ASCII string.", madt_sub_names[hdr->type]); +out: return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); } @@ -952,6 +969,13 @@ static int madt_local_x2apic(fwts_framework *fw, fwts_acpi_madt_local_x2apic *lx2apic = (fwts_acpi_madt_local_x2apic *)data; + /* only test table if enabled */ + if (!(lx2apic->flags & 0x1)) { + fwts_skipped(fw, "MADT %s is disabled. Skipping test.", + madt_sub_names[hdr->type]); + goto out; + } + if (lx2apic->reserved) fwts_failed(fw, LOG_LEVEL_MEDIUM, "MADTLX2APICReservedNonZero", @@ -980,6 +1004,7 @@ static int madt_local_x2apic(fwts_framework *fw, madt_find_processor_uid(fw, lx2apic->processor_uid, "X2APIC"); +out: return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header)); }
The Advanced Configuration and Power Interface (ACPI) specification version 6.2 Errata A in sections 5.2.12.2, 5.2.12.10 and 5.2.12.12 states that "when a processor is not present, the Processor Local APIC/x2APIC/ SAPIC information is either not reported or flagged as disabled." This implies that it is possible for firmware implementations to include Local APIC/SAPIC/x2APIC tables for non-existent processors provided that they are disabled. Thus, only test these tables if they have the Enable flag set. Otherwise, skip the test. Also, print a message to record the fact that these tables exist but are not tested. Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> Reported-by: Bin Song <binx.song@intel.com> --- src/acpi/madt/madt.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)