| Message ID | 20201126211231.417829-1-alex.hung@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [V2] acpi: fadt: X_GPE0/1_BLK can be zero when not used | expand |
On 26/11/2020 21:12, Alex Hung wrote: > ACPI spec states "if this register block is not supported, this field > contains zero". > > This patch also replaces spaces by a tab for fwts_acpi_gas struct. > > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- > src/acpi/fadt/fadt.c | 22 ++++++++++++---------- > src/lib/include/fwts_acpi.h | 7 ++++--- > src/lib/src/fwts_acpi.c | 11 +++++++++++ > 3 files changed, 27 insertions(+), 13 deletions(-) > > diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c > index ab5a3b2b..5159ec81 100644 > --- a/src/acpi/fadt/fadt.c > +++ b/src/acpi/fadt/fadt.c > @@ -1527,21 +1527,23 @@ static void acpi_table_check_fadt_x_gpex_blk(fwts_framework *fw) { > if (fadt->x_gpe0_blk.access_width == 1) > fwts_passed(fw, "FADT X_GPE0_BLK has correct byte access width."); > else { > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "X_GPE0_BLKBadAccessWidth", > - "FADT X_GPE0_BLK Access width 0x%2.2" PRIx8 > - " but it should be 1 (byte access).", > - fadt->x_gpe0_blk.access_width); > + if (!fwts_acpi_data_zero((const void *) &fadt->x_gpe0_blk, sizeof(fwts_acpi_gas))) > + fwts_failed(fw, LOG_LEVEL_HIGH, > + "X_GPE0_BLKBadAccessWidth", > + "FADT X_GPE0_BLK Access width 0x%2.2" PRIx8 > + " but it should be 1 (byte access).", > + fadt->x_gpe0_blk.access_width); > } > > if (fadt->x_gpe1_blk.access_width == 1) > fwts_passed(fw, "FADT X_GPE1_BLK has correct byte access width."); > else { > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "X_GPE1_BLKBadAccessWidth", > - "FADT X_GPE1_BLK Access width 0x%2.2" PRIx8 > - " but it should be 1 (byte access).", > - fadt->x_gpe1_blk.access_width); > + if (!fwts_acpi_data_zero((const void *) &fadt->x_gpe1_blk, sizeof(fwts_acpi_gas))) > + fwts_failed(fw, LOG_LEVEL_HIGH, > + "X_GPE1_BLKBadAccessWidth", > + "FADT X_GPE1_BLK Access width 0x%2.2" PRIx8 > + " but it should be 1 (byte access).", > + fadt->x_gpe1_blk.access_width); > } > } > > diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h > index efa988f2..d4dfa46f 100644 > --- a/src/lib/include/fwts_acpi.h > +++ b/src/lib/include/fwts_acpi.h > @@ -117,9 +117,9 @@ extern const char *fwts_acpi_fadt_preferred_pm_profile[]; > typedef struct { > uint8_t address_space_id; > uint8_t register_bit_width; > - uint8_t register_bit_offset; > - uint8_t access_width; > - uint64_t address; > + uint8_t register_bit_offset; > + uint8_t access_width; > + uint64_t address; > } __attribute__ ((packed)) fwts_acpi_gas; > > /* > @@ -1830,6 +1830,7 @@ typedef struct { > } __attribute__ ((packed)) fwts_acpi_table_hest_generic_hardware_error_source_v2; > > void fwts_acpi_table_get_header(fwts_acpi_table_header *hdr, uint8_t *data); > +bool fwts_acpi_data_zero(const void *data, const size_t len); > > /* > * ACPI CSTR (Core System Resources Table) > diff --git a/src/lib/src/fwts_acpi.c b/src/lib/src/fwts_acpi.c > index 55ac50bc..172e630c 100644 > --- a/src/lib/src/fwts_acpi.c > +++ b/src/lib/src/fwts_acpi.c > @@ -51,4 +51,15 @@ void fwts_acpi_table_get_header(fwts_acpi_table_header *hdr, uint8_t *data) > memcpy(hdr, data, sizeof(fwts_acpi_table_header)); > } > > +bool fwts_acpi_data_zero(const void *data, const size_t len) { > + uint8_t *ptr = (uint8_t *) data; > + uint8_t i; > + > + for (i = 0; i < len; i++) > + if (*ptr++) > + return false; > + > + return true; > +} > + > #endif > Thanks Alex, looks good to me. Acked-by: Colin Ian King <colin.king@canonical.com>
On 11/27/20 5:12 AM, Alex Hung wrote: > ACPI spec states "if this register block is not supported, this field > contains zero". > > This patch also replaces spaces by a tab for fwts_acpi_gas struct. > > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- > src/acpi/fadt/fadt.c | 22 ++++++++++++---------- > src/lib/include/fwts_acpi.h | 7 ++++--- > src/lib/src/fwts_acpi.c | 11 +++++++++++ > 3 files changed, 27 insertions(+), 13 deletions(-) > > diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c > index ab5a3b2b..5159ec81 100644 > --- a/src/acpi/fadt/fadt.c > +++ b/src/acpi/fadt/fadt.c > @@ -1527,21 +1527,23 @@ static void acpi_table_check_fadt_x_gpex_blk(fwts_framework *fw) { > if (fadt->x_gpe0_blk.access_width == 1) > fwts_passed(fw, "FADT X_GPE0_BLK has correct byte access width."); > else { > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "X_GPE0_BLKBadAccessWidth", > - "FADT X_GPE0_BLK Access width 0x%2.2" PRIx8 > - " but it should be 1 (byte access).", > - fadt->x_gpe0_blk.access_width); > + if (!fwts_acpi_data_zero((const void *) &fadt->x_gpe0_blk, sizeof(fwts_acpi_gas))) > + fwts_failed(fw, LOG_LEVEL_HIGH, > + "X_GPE0_BLKBadAccessWidth", > + "FADT X_GPE0_BLK Access width 0x%2.2" PRIx8 > + " but it should be 1 (byte access).", > + fadt->x_gpe0_blk.access_width); > } > > if (fadt->x_gpe1_blk.access_width == 1) > fwts_passed(fw, "FADT X_GPE1_BLK has correct byte access width."); > else { > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "X_GPE1_BLKBadAccessWidth", > - "FADT X_GPE1_BLK Access width 0x%2.2" PRIx8 > - " but it should be 1 (byte access).", > - fadt->x_gpe1_blk.access_width); > + if (!fwts_acpi_data_zero((const void *) &fadt->x_gpe1_blk, sizeof(fwts_acpi_gas))) > + fwts_failed(fw, LOG_LEVEL_HIGH, > + "X_GPE1_BLKBadAccessWidth", > + "FADT X_GPE1_BLK Access width 0x%2.2" PRIx8 > + " but it should be 1 (byte access).", > + fadt->x_gpe1_blk.access_width); > } > } > > diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h > index efa988f2..d4dfa46f 100644 > --- a/src/lib/include/fwts_acpi.h > +++ b/src/lib/include/fwts_acpi.h > @@ -117,9 +117,9 @@ extern const char *fwts_acpi_fadt_preferred_pm_profile[]; > typedef struct { > uint8_t address_space_id; > uint8_t register_bit_width; > - uint8_t register_bit_offset; > - uint8_t access_width; > - uint64_t address; > + uint8_t register_bit_offset; > + uint8_t access_width; > + uint64_t address; > } __attribute__ ((packed)) fwts_acpi_gas; > > /* > @@ -1830,6 +1830,7 @@ typedef struct { > } __attribute__ ((packed)) fwts_acpi_table_hest_generic_hardware_error_source_v2; > > void fwts_acpi_table_get_header(fwts_acpi_table_header *hdr, uint8_t *data); > +bool fwts_acpi_data_zero(const void *data, const size_t len); > > /* > * ACPI CSTR (Core System Resources Table) > diff --git a/src/lib/src/fwts_acpi.c b/src/lib/src/fwts_acpi.c > index 55ac50bc..172e630c 100644 > --- a/src/lib/src/fwts_acpi.c > +++ b/src/lib/src/fwts_acpi.c > @@ -51,4 +51,15 @@ void fwts_acpi_table_get_header(fwts_acpi_table_header *hdr, uint8_t *data) > memcpy(hdr, data, sizeof(fwts_acpi_table_header)); > } > > +bool fwts_acpi_data_zero(const void *data, const size_t len) { > + uint8_t *ptr = (uint8_t *) data; > + uint8_t i; > + > + for (i = 0; i < len; i++) > + if (*ptr++) > + return false; > + > + return true; > +} > + > #endif > Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c index ab5a3b2b..5159ec81 100644 --- a/src/acpi/fadt/fadt.c +++ b/src/acpi/fadt/fadt.c @@ -1527,21 +1527,23 @@ static void acpi_table_check_fadt_x_gpex_blk(fwts_framework *fw) { if (fadt->x_gpe0_blk.access_width == 1) fwts_passed(fw, "FADT X_GPE0_BLK has correct byte access width."); else { - fwts_failed(fw, LOG_LEVEL_HIGH, - "X_GPE0_BLKBadAccessWidth", - "FADT X_GPE0_BLK Access width 0x%2.2" PRIx8 - " but it should be 1 (byte access).", - fadt->x_gpe0_blk.access_width); + if (!fwts_acpi_data_zero((const void *) &fadt->x_gpe0_blk, sizeof(fwts_acpi_gas))) + fwts_failed(fw, LOG_LEVEL_HIGH, + "X_GPE0_BLKBadAccessWidth", + "FADT X_GPE0_BLK Access width 0x%2.2" PRIx8 + " but it should be 1 (byte access).", + fadt->x_gpe0_blk.access_width); } if (fadt->x_gpe1_blk.access_width == 1) fwts_passed(fw, "FADT X_GPE1_BLK has correct byte access width."); else { - fwts_failed(fw, LOG_LEVEL_HIGH, - "X_GPE1_BLKBadAccessWidth", - "FADT X_GPE1_BLK Access width 0x%2.2" PRIx8 - " but it should be 1 (byte access).", - fadt->x_gpe1_blk.access_width); + if (!fwts_acpi_data_zero((const void *) &fadt->x_gpe1_blk, sizeof(fwts_acpi_gas))) + fwts_failed(fw, LOG_LEVEL_HIGH, + "X_GPE1_BLKBadAccessWidth", + "FADT X_GPE1_BLK Access width 0x%2.2" PRIx8 + " but it should be 1 (byte access).", + fadt->x_gpe1_blk.access_width); } } diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h index efa988f2..d4dfa46f 100644 --- a/src/lib/include/fwts_acpi.h +++ b/src/lib/include/fwts_acpi.h @@ -117,9 +117,9 @@ extern const char *fwts_acpi_fadt_preferred_pm_profile[]; typedef struct { uint8_t address_space_id; uint8_t register_bit_width; - uint8_t register_bit_offset; - uint8_t access_width; - uint64_t address; + uint8_t register_bit_offset; + uint8_t access_width; + uint64_t address; } __attribute__ ((packed)) fwts_acpi_gas; /* @@ -1830,6 +1830,7 @@ typedef struct { } __attribute__ ((packed)) fwts_acpi_table_hest_generic_hardware_error_source_v2; void fwts_acpi_table_get_header(fwts_acpi_table_header *hdr, uint8_t *data); +bool fwts_acpi_data_zero(const void *data, const size_t len); /* * ACPI CSTR (Core System Resources Table) diff --git a/src/lib/src/fwts_acpi.c b/src/lib/src/fwts_acpi.c index 55ac50bc..172e630c 100644 --- a/src/lib/src/fwts_acpi.c +++ b/src/lib/src/fwts_acpi.c @@ -51,4 +51,15 @@ void fwts_acpi_table_get_header(fwts_acpi_table_header *hdr, uint8_t *data) memcpy(hdr, data, sizeof(fwts_acpi_table_header)); } +bool fwts_acpi_data_zero(const void *data, const size_t len) { + uint8_t *ptr = (uint8_t *) data; + uint8_t i; + + for (i = 0; i < len; i++) + if (*ptr++) + return false; + + return true; +} + #endif
ACPI spec states "if this register block is not supported, this field contains zero". This patch also replaces spaces by a tab for fwts_acpi_gas struct. Signed-off-by: Alex Hung <alex.hung@canonical.com> --- src/acpi/fadt/fadt.c | 22 ++++++++++++---------- src/lib/include/fwts_acpi.h | 7 ++++--- src/lib/src/fwts_acpi.c | 11 +++++++++++ 3 files changed, 27 insertions(+), 13 deletions(-)