[PATCH] fwts/madt: Implement I/O APIC and I/O SAPIC value comparison test
Alex Hung
alex.hung at canonical.com
Wed Jul 13 03:22:57 UTC 2016
On 2016-07-13 01:10 AM, Prarit Bhargava wrote:
> If both I/O APIC and I/O SAPIC subtables exist, there are two checks that
> should be run. The first is to confirm that there are at least as many
> I/O SAPIC tables as there are I/O APIC tables. The second is to check that
> every I/O APIC ID has a corresponding I/O SAPIC ID table.
>
> This patchset implements the tests.
>
> Additionally note a fixed typo for "MADIOSPAICAddrZero".
>
> Signed-off-by: Prarit Bhargava <prarit at redhat.com>
> ---
> src/acpi/madt/madt.c | 76 +++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 69 insertions(+), 7 deletions(-)
>
> diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c
> index 8ee231ee16be..d4fcc195b134 100644
> --- a/src/acpi/madt/madt.c
> +++ b/src/acpi/madt/madt.c
> @@ -121,6 +121,7 @@
> #define SUBTABLE_UNDEFINED 0x00
> #define SUBTABLE_VARIABLE 0xff
> #define NUM_SUBTABLE_TYPES 16
> +#define MAX_IO_APIC_ID 256 /* IO APIC ID field is 1 byte */
>
> struct acpi_madt_subtable_lengths {
> unsigned short major_version; /* from revision in FADT header */
> @@ -434,6 +435,7 @@ static int madt_local_apic(fwts_framework *fw,
> return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header));
> }
>
> +static unsigned char madt_io_apics[MAX_IO_APIC_ID];
> static int madt_io_apic(fwts_framework *fw,
> fwts_acpi_madt_sub_table_header *hdr,
> const uint8_t *data)
> @@ -461,6 +463,14 @@ static int madt_io_apic(fwts_framework *fw,
> fwts_passed(fw, "MADT %s address in non-zero.",
> madt_sub_names[hdr->type]);
>
> + if (!madt_io_apics[ioapic->io_apic_id])
> + madt_io_apics[ioapic->io_apic_id] = 1;
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "MADIOSAPICNotUnique",
> + "There are multiple entries for id %d.",
> + ioapic->io_apic_id);
> +
> return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header));
> }
>
> @@ -588,6 +598,7 @@ static int madt_lapic_addr_override(fwts_framework *fw,
> return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header));
> }
>
> +static unsigned char madt_io_sapics[MAX_IO_APIC_ID];
> static int madt_io_sapic(fwts_framework *fw,
> fwts_acpi_madt_sub_table_header *hdr,
> const uint8_t *data)
> @@ -608,7 +619,7 @@ static int madt_io_sapic(fwts_framework *fw,
>
> if (sapic->address == 0)
> fwts_failed(fw, LOG_LEVEL_MEDIUM,
> - "MADIOSPAICAddrZero",
> + "MADIOSAPICAddrZero",
> "MADT %s address is zero, needs to be defined.",
> madt_sub_names[hdr->type]);
> else
> @@ -616,12 +627,13 @@ static int madt_io_sapic(fwts_framework *fw,
> "MADT %s address set to non-zero value.",
> madt_sub_names[hdr->type]);
>
> - /*
> - * TODO: if both I/O APIC and I/O SAPIC subtables exist, there
> - * must be at least as many I/O SAPIC subtables as I/O APIC subtables,
> - * and that every I/O APIC has a corresponding I/O SAPIC with the
> - * same APIC ID.
> - */
> + if (!madt_io_sapics[sapic->io_sapic_id])
> + madt_io_sapics[sapic->io_sapic_id] = 1;
> + else
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "MADIOSAPICNotUnique",
> + "There are multiple entries for id %d.",
> + sapic->io_sapic_id);
>
> return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header));
> }
> @@ -1178,6 +1190,49 @@ static int madt_gic_its(fwts_framework *fw,
> return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header));
> }
>
> +static void madt_ioapic_sapic_compare(fwts_framework *fw,
> + int num_ioapics,
> + int num_iosapics)
> +{
> + bool failed;
> + int ioapic;
> +
> + if (!num_iosapics)
> + return; /* Nothing to do */
> +
> + /*
> + * If both I/O APIC and I/O SAPIC subtables exist, there must be at
> + * least as many I/O SAPIC subtables as I/O APIC subtables.
> + */
> + if (num_iosapics < num_ioapics)
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "MADTIOAPICSAPICCOMPARELessThan ",
> + "The number of IO SAPICS (%d) is less than "
> + "the number of IO APICS (%d).",
> + num_iosapics, num_ioapics);
> + else
> + fwts_passed(fw, "The number of IO SAPICS (%d) is at least"
> + "equal to the number of IO APICS (%d)",
> + num_iosapics, num_ioapics);
> +
> + /* Every I/O APIC must have a corresponding I/O SAPIC. */
> + failed = false;
> + for (ioapic = 0; ioapic <= MAX_IO_APIC_ID; ioapic++)
> + if (madt_io_apics[ioapic] &&
> + (madt_io_apics[ioapic] != madt_io_sapics[ioapic])) {
> + failed = true;
> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
> + "MADTIOAPICSAPICCOMPARENoSAPIC ",
> + "IO APIC %d does not have a IO "
> + "SAPIC entry",
> + ioapic);
> + }
When I compile, gcc complains
acpi/madt/madt.c:1221:20: error: iteration 256u invokes undefined
behavior [-Werror=aggressive-loop-optimizations]
if (madt_io_apics[ioapic] &&
^
acpi/madt/madt.c:1220:2: note: containing loop
for (ioapic = 0; ioapic <= MAX_IO_APIC_ID; ioapic++)
^
cc1: all warnings being treated as errors
This can be fixed as below (1. fix out-of-bound array, and 2) add "{}}
to for statement)
- for (ioapic = 0; ioapic <= MAX_IO_APIC_ID; ioapic++)
+ for (ioapic = 0; ioapic < MAX_IO_APIC_ID; ioapic++) {
if (madt_io_apics[ioapic] &&
(madt_io_apics[ioapic] != madt_io_sapics[ioapic])) {
failed = true;
....
}
+ }
> +
> + if (!failed)
> + fwts_passed(fw, "Each IO APIC entry has a corresponding"
> + "SAPIC entry.");
> +}
> +
> static int madt_subtables(fwts_framework *fw)
> {
> fwts_acpi_table_madt *madt = (fwts_acpi_table_madt *)mtable->data;
> @@ -1188,6 +1243,8 @@ static int madt_subtables(fwts_framework *fw)
> ssize_t length = mtable->length;
> int ii = 0;
> int proper_len;
> + int num_ioapics = 0;
> + int num_iosapics = 0;
>
> /*
> * check the correctness of each subtable type, and whether or
> @@ -1287,6 +1344,7 @@ static int madt_subtables(fwts_framework *fw)
>
> case FWTS_ACPI_MADT_IO_APIC:
> skip = madt_io_apic(fw, hdr, data);
> + num_ioapics++;
> break;
>
> case FWTS_ACPI_MADT_INTERRUPT_OVERRIDE:
> @@ -1307,6 +1365,7 @@ static int madt_subtables(fwts_framework *fw)
>
> case FWTS_ACPI_MADT_IO_SAPIC:
> skip = madt_io_sapic(fw, hdr, data);
> + num_iosapics++;
> break;
>
> case FWTS_ACPI_MADT_LOCAL_SAPIC:
> @@ -1385,6 +1444,9 @@ static int madt_subtables(fwts_framework *fw)
> length -= skip;
> }
>
> + /* run comparison tests */
> + madt_ioapic_sapic_compare(fw, num_ioapics, num_iosapics);
> +
> return FWTS_OK;
> }
>
>
--
Cheers,
Alex Hung
More information about the fwts-devel
mailing list