| Message ID | 1506384924-24711-1-git-send-email-alex.hung@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] acpi: replace checks for reserved bits by fwts_acpi_reserved_bits_check | expand |
On 26/09/17 01:15, Alex Hung wrote: > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- > src/acpi/bgrt/bgrt.c | 11 +++------ > src/acpi/gtdt/gtdt.c | 54 +++++++++++++++---------------------------- > src/acpi/hest/hest.c | 22 +++++++----------- > src/acpi/iort/iort.c | 65 +++++++++++++++------------------------------------- > src/acpi/lpit/lpit.c | 9 +------- > src/acpi/mchi/mchi.c | 22 +++++------------- > src/acpi/nfit/nfit.c | 9 +------- > src/acpi/pptt/pptt.c | 9 +------- > src/acpi/spmi/spmi.c | 19 +++------------ > 9 files changed, 59 insertions(+), 161 deletions(-) > > diff --git a/src/acpi/bgrt/bgrt.c b/src/acpi/bgrt/bgrt.c > index 25030ad..5b9b413 100644 > --- a/src/acpi/bgrt/bgrt.c > +++ b/src/acpi/bgrt/bgrt.c > @@ -66,14 +66,9 @@ static int bgrt_test1(fwts_framework *fw) > " and not the expected value of 0x01", > bgrt->version); > } > - if (bgrt->status & ~0x7) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_MEDIUM, > - "BGRTStatusRersevedBits", > - "BGRT: Status field is 0x%" PRIx8 > - ", reserved bits [7:3] should be zero", > - bgrt->status); > - } > + > + fwts_acpi_reserved_bits_check(fw, "BGRT", "BGRT Status", bgrt->status, sizeof(bgrt->status), 3, 7, &passed); > + > if (bgrt->image_type > 0x00) { > passed = false; > fwts_failed(fw, LOG_LEVEL_MEDIUM, > diff --git a/src/acpi/gtdt/gtdt.c b/src/acpi/gtdt/gtdt.c > index 1559d31..f3a8044 100644 > --- a/src/acpi/gtdt/gtdt.c > +++ b/src/acpi/gtdt/gtdt.c > @@ -67,6 +67,7 @@ static int gtdt_test1(fwts_framework *fw) > fwts_acpi_table_gtdt_block *block; > fwts_acpi_table_gtdt_block_timer *block_timer; > fwts_acpi_table_gtdt_watchdog *watchdog; > + char field[80]; > > switch (*ptr) { > case 0x00: > @@ -145,33 +146,18 @@ static int gtdt_test1(fwts_framework *fw) > block_timer->reserved[1], > block_timer->reserved[2]); > } > - if (block_timer->phys_timer_flags & ~0x3) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_LOW, > - "GTDTFBlockPhysTimerFlagReservedNonZero", > - "GTDT block %" PRIu32 " physical timer " > - "flags reserved bits 2 to 31 are " > - "non-zero, instead got 0x%" PRIx32 ".", > - i, block_timer->phys_timer_flags); > - } > - if (block_timer->virt_timer_flags & ~0x3) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_LOW, > - "GTDTFBlockVirtTimerFlagReservedNonZero", > - "GTDT block %" PRIu32 " virtual timer " > - "flags reserved bits 2 to 31 are " > - "non-zero, instead got 0x%" PRIx32 ".", > - i, block_timer->virt_timer_flags); > - } > - if (block_timer->common_flags & ~0x3) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_LOW, > - "GTDTFBlockCommonFlagReservedNonZero", > - "GTDT block %" PRIu32 " common flags " > - "reserved bits 2 to 31 are " > - "non-zero, instead got 0x%" PRIx32 ".", > - i, block_timer->common_flags); > - } > + > + snprintf(field, sizeof(field), "block %" PRIu32 " physical timer flags", i); > + fwts_acpi_reserved_bits_check(fw, "GTDT", field, block_timer->phys_timer_flags, > + sizeof(block_timer->phys_timer_flags), 2, 31, &passed); > + > + snprintf(field, sizeof(field), "block %" PRIu32 " virtual timer flags", i); > + fwts_acpi_reserved_bits_check(fw, "GTDT", field, block_timer->virt_timer_flags, > + sizeof(block_timer->virt_timer_flags), 2, 31, &passed); > + > + snprintf(field, sizeof(field), "block %" PRIu32 " common flags", i); > + fwts_acpi_reserved_bits_check(fw, "GTDT", field, block_timer->common_flags, > + sizeof(block_timer->common_flags), 2, 31, &passed); > } > ptr += block->length; > break; > @@ -203,15 +189,11 @@ static int gtdt_test1(fwts_framework *fw) > " reserved is non-zero, got 0x%" PRIx8, > i, watchdog->reserved); > } > - if (watchdog->watchdog_timer_flags & ~0x7) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_LOW, > - "GTDTWatchDogTimerFlagReservedNonZero", > - "GTDT SBSA generic watchdog timer %" PRIu32 > - " flags reserved bits 3 to 31 are non-zero, " > - "instead got 0x%" PRIx8 ".", > - i, watchdog->watchdog_timer_flags); > - } > + > + snprintf(field, sizeof(field), "SBSA generic watchdog timer %" PRIu32 " flags", i); > + fwts_acpi_reserved_bits_check(fw, "GTDT", field, watchdog->watchdog_timer_flags, > + sizeof(watchdog->watchdog_timer_flags), 3, 31, &passed); > + > ptr += watchdog->length; > break; > default: > diff --git a/src/acpi/hest/hest.c b/src/acpi/hest/hest.c > index 86a5312..289bd18 100644 > --- a/src/acpi/hest/hest.c > +++ b/src/acpi/hest/hest.c > @@ -658,13 +658,10 @@ static void hest_check_generic_error_source( > "expecting value 0 to 11", > source->notification.type); > } > - if (source->notification.configuration_write_enable & ~0x3f) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_LOW, > - "HESTIA32ConfigWriteEnabledReservedNonZero", > - "HEST Configuration Write Enabled Reserved bits [6:31] " > - "are non-zero."); > - } > + > + fwts_acpi_reserved_bits_check(fw, "HEST", "HEST Configuration Write Enabled", > + source->notification.configuration_write_enable, > + sizeof(source->notification.configuration_write_enable), 6, 31, passed); > > *length -= sizeof(fwts_acpi_table_hest_generic_hardware_error_source); > *data += sizeof(fwts_acpi_table_hest_generic_hardware_error_source); > @@ -776,13 +773,10 @@ static void hest_check_generic_error_source_v2( > "expecting value 0 to 11", > source->notification.type); > } > - if (source->notification.configuration_write_enable & ~0x3f) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_LOW, > - "HESTIA32ConfigWriteEnabledReservedNonZero", > - "HEST Configuration Write Enabled Reserved bits [6:31] " > - "are non-zero."); > - } > + > + fwts_acpi_reserved_bits_check(fw, "HEST", "HEST Configuration Write Enabled", > + source->notification.configuration_write_enable, > + sizeof(source->notification.configuration_write_enable), 6, 31, passed); > > *length -= sizeof(fwts_acpi_table_hest_generic_hardware_error_source_v2); > *data += sizeof(fwts_acpi_table_hest_generic_hardware_error_source_v2); > diff --git a/src/acpi/iort/iort.c b/src/acpi/iort/iort.c > index b7046ea..9c20610 100644 > --- a/src/acpi/iort/iort.c > +++ b/src/acpi/iort/iort.c > @@ -148,28 +148,6 @@ static void iort_id_mappings_dump( > } > > /* > - * iort_id_mapping_check() > - * sanity check ID mapping > - */ > -static void iort_id_mapping_check( > - fwts_framework *fw, > - uint32_t i, > - fwts_acpi_table_iort_id_mapping *id_mapping, > - bool *passed) > -{ > - if (id_mapping->flags & ~1) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_MEDIUM, > - "IORTIdMappingReservedNonZero", > - "IORT ID Mapping %" PRIu32 " Reserved Flags field reserved " > - "bits [31:1] should be all zero, got 0x%" PRIx32 > - " instead", > - i, id_mapping->flags); > - } > - /* Should also check output_reference is out of range or not */ > -} > - > -/* > * iort_id_mappings_check() > * snaity check array of ID mappings > */ > @@ -182,6 +160,7 @@ static void iort_id_mappings_check( > uint32_t i; > fwts_acpi_table_iort_node *node = (fwts_acpi_table_iort_node *)data; > fwts_acpi_table_iort_id_mapping *id_mapping; > + char field[80]; > > id_mapping = (fwts_acpi_table_iort_id_mapping *)(data + node->id_array_offset); > > @@ -195,7 +174,9 @@ static void iort_id_mappings_check( > "or the IORT table size or the node is too small.", i); > break; > } > - iort_id_mapping_check(fw, i, id_mapping, passed); > + > + snprintf(field, sizeof(field), "ID Mapping %" PRIu32 " flags", i); > + fwts_acpi_reserved_bits_check(fw, "IORT", field, id_mapping->flags, sizeof(id_mapping->flags), 1, 31, passed); > } > } > > @@ -381,6 +362,7 @@ static void iort_memory_access_properties_check( > bool *passed) > { > uint8_t cca, cpm, dacs; > + char field[80]; > > if (properties->cache_coherent > 1) { > *passed = false; > @@ -392,30 +374,19 @@ static void iort_memory_access_properties_check( > "not coherent).", > name, properties->cache_coherent); > } > - if (properties->allocation_hints & ~0xf) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "IORTAllocHintsReservedNonZero", > - "IORT %s Allocation Hints reserved bits " > - "[7:4] are reserved and should be zero.", > - name); > - } > - if (properties->reserved) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_MEDIUM, > - "IORTNodeReservedNonZero", > - "IORT %s Node Reserved is 0x%" PRIx32 > - " and is reserved and should be zero.", > - name, properties->reserved); > - } > - if (properties->memory_access_flags & ~0x3) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "IORTMemoryAccessFlagsReservedNonZero", > - "IORT %s Memory Access Flags is 0x%" PRIx8 > - " and all the reserved bits [7:2] should be zero but some are not.", > - name, properties->memory_access_flags); > - } > + > + snprintf(field, sizeof(field), "%s Allocation Hints", name); > + fwts_acpi_reserved_bits_check(fw, "IORT", field, properties->allocation_hints, > + sizeof(properties->allocation_hints), 4, 7, passed); > + > + snprintf(field, sizeof(field), "%s Reserved", name); > + fwts_acpi_reserved_zero_check(fw, "IORT", field, properties->reserved, > + sizeof(properties->reserved), passed); > + > + snprintf(field, sizeof(field), "%s Memory Access Flags", name); > + fwts_acpi_reserved_bits_check(fw, "IORT", field, properties->memory_access_flags, > + sizeof(properties->memory_access_flags), 2, 7, passed); > + > cca = properties->cache_coherent & 1; > cpm = properties->memory_access_flags & 1; > dacs = (properties->memory_access_flags >> 1) & 1; > diff --git a/src/acpi/lpit/lpit.c b/src/acpi/lpit/lpit.c > index 2273ee4..eb4e3af 100644 > --- a/src/acpi/lpit/lpit.c > +++ b/src/acpi/lpit/lpit.c > @@ -95,14 +95,7 @@ static void lpit_check_type_0( > fwts_log_nl(fw); > > fwts_acpi_reserved_zero_check(fw, "LPIT", "Native C-state based LPI structure reserved", lpi->reserved, sizeof(lpi->reserved), passed); > - > - if (lpi->flags & ~3) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_LOW, > - "LPITNativeCStateLpitFlagsReserved", > - "Some of the Native C-state based LPI structure flags " > - "bits [31:2] are set, they are expected to be zero"); > - } > + fwts_acpi_reserved_bits_check(fw, "LPIT", "LPI structure flags", lpi->flags, sizeof(lpi->flags), 2, 31, passed); > > /* 2.2.1.2, if FFH, then it is a MSR, check GAS fields */ > if (((lpi->flags & 2) == 0) && > diff --git a/src/acpi/mchi/mchi.c b/src/acpi/mchi/mchi.c > index 6244b93..8ddff2c 100644 > --- a/src/acpi/mchi/mchi.c > +++ b/src/acpi/mchi/mchi.c > @@ -116,14 +116,9 @@ static int mchi_test1(fwts_framework *fw) > "allowed values are 0x00 (Unspecifier), 0x01 (MCTP), 0x02 (IPMI) or " > "255 (OEM defined)", mchi->protocol_identifier); > } > - if (mchi->interrupt_type & ~0x03) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "MCHIInterruptTypeReservedNonZero", > - "MCHI Interrupt Type 0x%2.2" PRIx8 " has some reserved " > - "bits [7:2] set, these should be all zero.", > - mchi->interrupt_type); > - } > + > + fwts_acpi_reserved_bits_check(fw, "MCHI", "Interrupt Type", mchi->interrupt_type, sizeof(mchi->interrupt_type), 2, 7, &passed); > + > if (((mchi->interrupt_type & 0x01) == 0) && > (mchi->gpe != 0)) { > passed = false; > @@ -134,14 +129,9 @@ static int mchi_test1(fwts_framework *fw) > "(SCI triggered through GPE non-supported)", > mchi->gpe); > } > - if (mchi->pci_device_flag & ~0x01) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "MCHIPciDeviceFlagReservedNonZero", > - "MCHI PCI Device Flag 0x%2.2" PRIx8 " has some reserved " > - "bits [7:1] set, these should be all zero.", > - mchi->pci_device_flag); > - } > + > + fwts_acpi_reserved_bits_check(fw, "MCHI", "PCI Device Flag", mchi->pci_device_flag, sizeof(mchi->pci_device_flag), 1, 7, &passed); > + > if (((mchi->interrupt_type & 0x02) == 0) && > (mchi->global_system_interrupt != 0)) { > passed = false; > diff --git a/src/acpi/nfit/nfit.c b/src/acpi/nfit/nfit.c > index fc69798..e1a4815 100644 > --- a/src/acpi/nfit/nfit.c > +++ b/src/acpi/nfit/nfit.c > @@ -284,14 +284,7 @@ static int nfit_test1(fwts_framework *fw) > if (nfit_struct->reserved != 0) > reserved_passed = nfit_struct->reserved; > > - if (nfit_struct->valid_fields & ~0x01) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "NFITBadValidField", > - "NFIT Valid Field's Bits[7..1] must be zero, got " > - "0x%2.2" PRIx8 " instead", nfit_struct->valid_fields); > - } > - > + fwts_acpi_reserved_bits_check(fw, "NFIT", "Valid", nfit_struct->valid_fields, sizeof(nfit_struct->valid_fields), 1, 7, &passed); > fwts_acpi_reserved_bits_check(fw, "NFIT", "NVDIMM Control Region Flags", nfit_struct->flags, sizeof(nfit_struct->flags), 1, 15, &passed); > > } else if (entry->type == FWTS_ACPI_NFIT_TYPE_DATA_REGION) { > diff --git a/src/acpi/pptt/pptt.c b/src/acpi/pptt/pptt.c > index 50b7491..25f5d9a 100644 > --- a/src/acpi/pptt/pptt.c > +++ b/src/acpi/pptt/pptt.c > @@ -85,14 +85,7 @@ static void pptt_cache_test(fwts_framework *fw, const fwts_acpi_table_pptt_cache > > fwts_acpi_reserved_zero_check(fw, "PPTT", "Reserved", entry->reserved, sizeof(entry->reserved), passed); > fwts_acpi_reserved_bits_check(fw, "PPTT", "Flags", entry->flags, sizeof(entry->flags), 7, 31, passed); > - > - if (entry->attributes & ~0x1f) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "PPTTBadAttributes", > - "PPTT attributes's Bits[7..5] must be zero, got " > - "0x%2.2" PRIx8 " instead", entry->attributes); > - } > + fwts_acpi_reserved_bits_check(fw, "PPTT", "Attributes", entry->attributes, sizeof(entry->attributes), 5, 7, passed); > } > > static void pptt_id_test(fwts_framework *fw, const fwts_acpi_table_pptt_id *entry, bool *passed) > diff --git a/src/acpi/spmi/spmi.c b/src/acpi/spmi/spmi.c > index 23b9b22..86acb11 100644 > --- a/src/acpi/spmi/spmi.c > +++ b/src/acpi/spmi/spmi.c > @@ -123,14 +123,7 @@ static int spmi_test1(fwts_framework *fw) > "0x%2.2" PRIx8 " instead", spmi->reserved1); > } > > - /* Only bottom 2 bits should be set */ > - if (spmi->interrupt_type & ~0x03) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_LOW, > - "SPMIInterruptTypeReservedNonZero", > - "SPMI Interrupt Type reserved bits [7:2] must be 0, got " > - "0x%2.2" PRIx8 " instead", spmi->interrupt_type); > - } > + fwts_acpi_reserved_bits_check(fw, "SPMI", "Interrupt type", spmi->interrupt_type, sizeof(spmi->interrupt_type), 2, 7, &passed); > > /* Check for zero GPE on specific condition of interrupt type */ > if (((spmi->interrupt_type & 1) == 0) && > @@ -149,14 +142,8 @@ static int spmi_test1(fwts_framework *fw) > "SPMI reserved field (offset 42) must be 0x00, got " > "0x%2.2" PRIx8 " instead", spmi->reserved2); > } > - /* Only bottom 1 bit should be set */ > - if (spmi->pci_device_flag & ~0x01) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_MEDIUM, > - "SPMIPciDeviceFlag", > - "SPMI PCI Device Flag reserved bits [7:1] must be 0, got " > - "0x%2.2" PRIx8 " instead", spmi->pci_device_flag); > - } > + > + fwts_acpi_reserved_bits_check(fw, "SPMI", "PCI device flag", spmi->pci_device_flag, sizeof(spmi->pci_device_flag), 1, 7, &passed); > > if (((spmi->interrupt_type & 2) == 0) && > (spmi->global_system_interrupt != 0)) { > Acked-by: Colin Ian King <colin.king@canonical.com>
On 09/25/2017 08:15 PM, Alex Hung wrote: > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- > src/acpi/bgrt/bgrt.c | 11 +++------ > src/acpi/gtdt/gtdt.c | 54 +++++++++++++++---------------------------- > src/acpi/hest/hest.c | 22 +++++++----------- > src/acpi/iort/iort.c | 65 +++++++++++++++------------------------------------- > src/acpi/lpit/lpit.c | 9 +------- > src/acpi/mchi/mchi.c | 22 +++++------------- > src/acpi/nfit/nfit.c | 9 +------- > src/acpi/pptt/pptt.c | 9 +------- > src/acpi/spmi/spmi.c | 19 +++------------ > 9 files changed, 59 insertions(+), 161 deletions(-) > > diff --git a/src/acpi/bgrt/bgrt.c b/src/acpi/bgrt/bgrt.c > index 25030ad..5b9b413 100644 > --- a/src/acpi/bgrt/bgrt.c > +++ b/src/acpi/bgrt/bgrt.c > @@ -66,14 +66,9 @@ static int bgrt_test1(fwts_framework *fw) > " and not the expected value of 0x01", > bgrt->version); > } > - if (bgrt->status & ~0x7) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_MEDIUM, > - "BGRTStatusRersevedBits", > - "BGRT: Status field is 0x%" PRIx8 > - ", reserved bits [7:3] should be zero", > - bgrt->status); > - } > + > + fwts_acpi_reserved_bits_check(fw, "BGRT", "BGRT Status", bgrt->status, sizeof(bgrt->status), 3, 7, &passed); > + > if (bgrt->image_type > 0x00) { > passed = false; > fwts_failed(fw, LOG_LEVEL_MEDIUM, > diff --git a/src/acpi/gtdt/gtdt.c b/src/acpi/gtdt/gtdt.c > index 1559d31..f3a8044 100644 > --- a/src/acpi/gtdt/gtdt.c > +++ b/src/acpi/gtdt/gtdt.c > @@ -67,6 +67,7 @@ static int gtdt_test1(fwts_framework *fw) > fwts_acpi_table_gtdt_block *block; > fwts_acpi_table_gtdt_block_timer *block_timer; > fwts_acpi_table_gtdt_watchdog *watchdog; > + char field[80]; > > switch (*ptr) { > case 0x00: > @@ -145,33 +146,18 @@ static int gtdt_test1(fwts_framework *fw) > block_timer->reserved[1], > block_timer->reserved[2]); > } > - if (block_timer->phys_timer_flags & ~0x3) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_LOW, > - "GTDTFBlockPhysTimerFlagReservedNonZero", > - "GTDT block %" PRIu32 " physical timer " > - "flags reserved bits 2 to 31 are " > - "non-zero, instead got 0x%" PRIx32 ".", > - i, block_timer->phys_timer_flags); > - } > - if (block_timer->virt_timer_flags & ~0x3) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_LOW, > - "GTDTFBlockVirtTimerFlagReservedNonZero", > - "GTDT block %" PRIu32 " virtual timer " > - "flags reserved bits 2 to 31 are " > - "non-zero, instead got 0x%" PRIx32 ".", > - i, block_timer->virt_timer_flags); > - } > - if (block_timer->common_flags & ~0x3) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_LOW, > - "GTDTFBlockCommonFlagReservedNonZero", > - "GTDT block %" PRIu32 " common flags " > - "reserved bits 2 to 31 are " > - "non-zero, instead got 0x%" PRIx32 ".", > - i, block_timer->common_flags); > - } > + > + snprintf(field, sizeof(field), "block %" PRIu32 " physical timer flags", i); > + fwts_acpi_reserved_bits_check(fw, "GTDT", field, block_timer->phys_timer_flags, > + sizeof(block_timer->phys_timer_flags), 2, 31, &passed); > + > + snprintf(field, sizeof(field), "block %" PRIu32 " virtual timer flags", i); > + fwts_acpi_reserved_bits_check(fw, "GTDT", field, block_timer->virt_timer_flags, > + sizeof(block_timer->virt_timer_flags), 2, 31, &passed); > + > + snprintf(field, sizeof(field), "block %" PRIu32 " common flags", i); > + fwts_acpi_reserved_bits_check(fw, "GTDT", field, block_timer->common_flags, > + sizeof(block_timer->common_flags), 2, 31, &passed); > } > ptr += block->length; > break; > @@ -203,15 +189,11 @@ static int gtdt_test1(fwts_framework *fw) > " reserved is non-zero, got 0x%" PRIx8, > i, watchdog->reserved); > } > - if (watchdog->watchdog_timer_flags & ~0x7) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_LOW, > - "GTDTWatchDogTimerFlagReservedNonZero", > - "GTDT SBSA generic watchdog timer %" PRIu32 > - " flags reserved bits 3 to 31 are non-zero, " > - "instead got 0x%" PRIx8 ".", > - i, watchdog->watchdog_timer_flags); > - } > + > + snprintf(field, sizeof(field), "SBSA generic watchdog timer %" PRIu32 " flags", i); > + fwts_acpi_reserved_bits_check(fw, "GTDT", field, watchdog->watchdog_timer_flags, > + sizeof(watchdog->watchdog_timer_flags), 3, 31, &passed); > + > ptr += watchdog->length; > break; > default: > diff --git a/src/acpi/hest/hest.c b/src/acpi/hest/hest.c > index 86a5312..289bd18 100644 > --- a/src/acpi/hest/hest.c > +++ b/src/acpi/hest/hest.c > @@ -658,13 +658,10 @@ static void hest_check_generic_error_source( > "expecting value 0 to 11", > source->notification.type); > } > - if (source->notification.configuration_write_enable & ~0x3f) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_LOW, > - "HESTIA32ConfigWriteEnabledReservedNonZero", > - "HEST Configuration Write Enabled Reserved bits [6:31] " > - "are non-zero."); > - } > + > + fwts_acpi_reserved_bits_check(fw, "HEST", "HEST Configuration Write Enabled", > + source->notification.configuration_write_enable, > + sizeof(source->notification.configuration_write_enable), 6, 31, passed); > > *length -= sizeof(fwts_acpi_table_hest_generic_hardware_error_source); > *data += sizeof(fwts_acpi_table_hest_generic_hardware_error_source); > @@ -776,13 +773,10 @@ static void hest_check_generic_error_source_v2( > "expecting value 0 to 11", > source->notification.type); > } > - if (source->notification.configuration_write_enable & ~0x3f) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_LOW, > - "HESTIA32ConfigWriteEnabledReservedNonZero", > - "HEST Configuration Write Enabled Reserved bits [6:31] " > - "are non-zero."); > - } > + > + fwts_acpi_reserved_bits_check(fw, "HEST", "HEST Configuration Write Enabled", > + source->notification.configuration_write_enable, > + sizeof(source->notification.configuration_write_enable), 6, 31, passed); > > *length -= sizeof(fwts_acpi_table_hest_generic_hardware_error_source_v2); > *data += sizeof(fwts_acpi_table_hest_generic_hardware_error_source_v2); > diff --git a/src/acpi/iort/iort.c b/src/acpi/iort/iort.c > index b7046ea..9c20610 100644 > --- a/src/acpi/iort/iort.c > +++ b/src/acpi/iort/iort.c > @@ -148,28 +148,6 @@ static void iort_id_mappings_dump( > } > > /* > - * iort_id_mapping_check() > - * sanity check ID mapping > - */ > -static void iort_id_mapping_check( > - fwts_framework *fw, > - uint32_t i, > - fwts_acpi_table_iort_id_mapping *id_mapping, > - bool *passed) > -{ > - if (id_mapping->flags & ~1) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_MEDIUM, > - "IORTIdMappingReservedNonZero", > - "IORT ID Mapping %" PRIu32 " Reserved Flags field reserved " > - "bits [31:1] should be all zero, got 0x%" PRIx32 > - " instead", > - i, id_mapping->flags); > - } > - /* Should also check output_reference is out of range or not */ > -} > - > -/* > * iort_id_mappings_check() > * snaity check array of ID mappings > */ > @@ -182,6 +160,7 @@ static void iort_id_mappings_check( > uint32_t i; > fwts_acpi_table_iort_node *node = (fwts_acpi_table_iort_node *)data; > fwts_acpi_table_iort_id_mapping *id_mapping; > + char field[80]; > > id_mapping = (fwts_acpi_table_iort_id_mapping *)(data + node->id_array_offset); > > @@ -195,7 +174,9 @@ static void iort_id_mappings_check( > "or the IORT table size or the node is too small.", i); > break; > } > - iort_id_mapping_check(fw, i, id_mapping, passed); > + > + snprintf(field, sizeof(field), "ID Mapping %" PRIu32 " flags", i); > + fwts_acpi_reserved_bits_check(fw, "IORT", field, id_mapping->flags, sizeof(id_mapping->flags), 1, 31, passed); > } > } > > @@ -381,6 +362,7 @@ static void iort_memory_access_properties_check( > bool *passed) > { > uint8_t cca, cpm, dacs; > + char field[80]; > > if (properties->cache_coherent > 1) { > *passed = false; > @@ -392,30 +374,19 @@ static void iort_memory_access_properties_check( > "not coherent).", > name, properties->cache_coherent); > } > - if (properties->allocation_hints & ~0xf) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "IORTAllocHintsReservedNonZero", > - "IORT %s Allocation Hints reserved bits " > - "[7:4] are reserved and should be zero.", > - name); > - } > - if (properties->reserved) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_MEDIUM, > - "IORTNodeReservedNonZero", > - "IORT %s Node Reserved is 0x%" PRIx32 > - " and is reserved and should be zero.", > - name, properties->reserved); > - } > - if (properties->memory_access_flags & ~0x3) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "IORTMemoryAccessFlagsReservedNonZero", > - "IORT %s Memory Access Flags is 0x%" PRIx8 > - " and all the reserved bits [7:2] should be zero but some are not.", > - name, properties->memory_access_flags); > - } > + > + snprintf(field, sizeof(field), "%s Allocation Hints", name); > + fwts_acpi_reserved_bits_check(fw, "IORT", field, properties->allocation_hints, > + sizeof(properties->allocation_hints), 4, 7, passed); > + > + snprintf(field, sizeof(field), "%s Reserved", name); > + fwts_acpi_reserved_zero_check(fw, "IORT", field, properties->reserved, > + sizeof(properties->reserved), passed); > + > + snprintf(field, sizeof(field), "%s Memory Access Flags", name); > + fwts_acpi_reserved_bits_check(fw, "IORT", field, properties->memory_access_flags, > + sizeof(properties->memory_access_flags), 2, 7, passed); > + > cca = properties->cache_coherent & 1; > cpm = properties->memory_access_flags & 1; > dacs = (properties->memory_access_flags >> 1) & 1; > diff --git a/src/acpi/lpit/lpit.c b/src/acpi/lpit/lpit.c > index 2273ee4..eb4e3af 100644 > --- a/src/acpi/lpit/lpit.c > +++ b/src/acpi/lpit/lpit.c > @@ -95,14 +95,7 @@ static void lpit_check_type_0( > fwts_log_nl(fw); > > fwts_acpi_reserved_zero_check(fw, "LPIT", "Native C-state based LPI structure reserved", lpi->reserved, sizeof(lpi->reserved), passed); > - > - if (lpi->flags & ~3) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_LOW, > - "LPITNativeCStateLpitFlagsReserved", > - "Some of the Native C-state based LPI structure flags " > - "bits [31:2] are set, they are expected to be zero"); > - } > + fwts_acpi_reserved_bits_check(fw, "LPIT", "LPI structure flags", lpi->flags, sizeof(lpi->flags), 2, 31, passed); > > /* 2.2.1.2, if FFH, then it is a MSR, check GAS fields */ > if (((lpi->flags & 2) == 0) && > diff --git a/src/acpi/mchi/mchi.c b/src/acpi/mchi/mchi.c > index 6244b93..8ddff2c 100644 > --- a/src/acpi/mchi/mchi.c > +++ b/src/acpi/mchi/mchi.c > @@ -116,14 +116,9 @@ static int mchi_test1(fwts_framework *fw) > "allowed values are 0x00 (Unspecifier), 0x01 (MCTP), 0x02 (IPMI) or " > "255 (OEM defined)", mchi->protocol_identifier); > } > - if (mchi->interrupt_type & ~0x03) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "MCHIInterruptTypeReservedNonZero", > - "MCHI Interrupt Type 0x%2.2" PRIx8 " has some reserved " > - "bits [7:2] set, these should be all zero.", > - mchi->interrupt_type); > - } > + > + fwts_acpi_reserved_bits_check(fw, "MCHI", "Interrupt Type", mchi->interrupt_type, sizeof(mchi->interrupt_type), 2, 7, &passed); > + > if (((mchi->interrupt_type & 0x01) == 0) && > (mchi->gpe != 0)) { > passed = false; > @@ -134,14 +129,9 @@ static int mchi_test1(fwts_framework *fw) > "(SCI triggered through GPE non-supported)", > mchi->gpe); > } > - if (mchi->pci_device_flag & ~0x01) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "MCHIPciDeviceFlagReservedNonZero", > - "MCHI PCI Device Flag 0x%2.2" PRIx8 " has some reserved " > - "bits [7:1] set, these should be all zero.", > - mchi->pci_device_flag); > - } > + > + fwts_acpi_reserved_bits_check(fw, "MCHI", "PCI Device Flag", mchi->pci_device_flag, sizeof(mchi->pci_device_flag), 1, 7, &passed); > + > if (((mchi->interrupt_type & 0x02) == 0) && > (mchi->global_system_interrupt != 0)) { > passed = false; > diff --git a/src/acpi/nfit/nfit.c b/src/acpi/nfit/nfit.c > index fc69798..e1a4815 100644 > --- a/src/acpi/nfit/nfit.c > +++ b/src/acpi/nfit/nfit.c > @@ -284,14 +284,7 @@ static int nfit_test1(fwts_framework *fw) > if (nfit_struct->reserved != 0) > reserved_passed = nfit_struct->reserved; > > - if (nfit_struct->valid_fields & ~0x01) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "NFITBadValidField", > - "NFIT Valid Field's Bits[7..1] must be zero, got " > - "0x%2.2" PRIx8 " instead", nfit_struct->valid_fields); > - } > - > + fwts_acpi_reserved_bits_check(fw, "NFIT", "Valid", nfit_struct->valid_fields, sizeof(nfit_struct->valid_fields), 1, 7, &passed); > fwts_acpi_reserved_bits_check(fw, "NFIT", "NVDIMM Control Region Flags", nfit_struct->flags, sizeof(nfit_struct->flags), 1, 15, &passed); > > } else if (entry->type == FWTS_ACPI_NFIT_TYPE_DATA_REGION) { > diff --git a/src/acpi/pptt/pptt.c b/src/acpi/pptt/pptt.c > index 50b7491..25f5d9a 100644 > --- a/src/acpi/pptt/pptt.c > +++ b/src/acpi/pptt/pptt.c > @@ -85,14 +85,7 @@ static void pptt_cache_test(fwts_framework *fw, const fwts_acpi_table_pptt_cache > > fwts_acpi_reserved_zero_check(fw, "PPTT", "Reserved", entry->reserved, sizeof(entry->reserved), passed); > fwts_acpi_reserved_bits_check(fw, "PPTT", "Flags", entry->flags, sizeof(entry->flags), 7, 31, passed); > - > - if (entry->attributes & ~0x1f) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "PPTTBadAttributes", > - "PPTT attributes's Bits[7..5] must be zero, got " > - "0x%2.2" PRIx8 " instead", entry->attributes); > - } > + fwts_acpi_reserved_bits_check(fw, "PPTT", "Attributes", entry->attributes, sizeof(entry->attributes), 5, 7, passed); > } > > static void pptt_id_test(fwts_framework *fw, const fwts_acpi_table_pptt_id *entry, bool *passed) > diff --git a/src/acpi/spmi/spmi.c b/src/acpi/spmi/spmi.c > index 23b9b22..86acb11 100644 > --- a/src/acpi/spmi/spmi.c > +++ b/src/acpi/spmi/spmi.c > @@ -123,14 +123,7 @@ static int spmi_test1(fwts_framework *fw) > "0x%2.2" PRIx8 " instead", spmi->reserved1); > } > > - /* Only bottom 2 bits should be set */ > - if (spmi->interrupt_type & ~0x03) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_LOW, > - "SPMIInterruptTypeReservedNonZero", > - "SPMI Interrupt Type reserved bits [7:2] must be 0, got " > - "0x%2.2" PRIx8 " instead", spmi->interrupt_type); > - } > + fwts_acpi_reserved_bits_check(fw, "SPMI", "Interrupt type", spmi->interrupt_type, sizeof(spmi->interrupt_type), 2, 7, &passed); > > /* Check for zero GPE on specific condition of interrupt type */ > if (((spmi->interrupt_type & 1) == 0) && > @@ -149,14 +142,8 @@ static int spmi_test1(fwts_framework *fw) > "SPMI reserved field (offset 42) must be 0x00, got " > "0x%2.2" PRIx8 " instead", spmi->reserved2); > } > - /* Only bottom 1 bit should be set */ > - if (spmi->pci_device_flag & ~0x01) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_MEDIUM, > - "SPMIPciDeviceFlag", > - "SPMI PCI Device Flag reserved bits [7:1] must be 0, got " > - "0x%2.2" PRIx8 " instead", spmi->pci_device_flag); > - } > + > + fwts_acpi_reserved_bits_check(fw, "SPMI", "PCI device flag", spmi->pci_device_flag, sizeof(spmi->pci_device_flag), 1, 7, &passed); > > if (((spmi->interrupt_type & 2) == 0) && > (spmi->global_system_interrupt != 0)) { > Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff --git a/src/acpi/bgrt/bgrt.c b/src/acpi/bgrt/bgrt.c index 25030ad..5b9b413 100644 --- a/src/acpi/bgrt/bgrt.c +++ b/src/acpi/bgrt/bgrt.c @@ -66,14 +66,9 @@ static int bgrt_test1(fwts_framework *fw) " and not the expected value of 0x01", bgrt->version); } - if (bgrt->status & ~0x7) { - passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "BGRTStatusRersevedBits", - "BGRT: Status field is 0x%" PRIx8 - ", reserved bits [7:3] should be zero", - bgrt->status); - } + + fwts_acpi_reserved_bits_check(fw, "BGRT", "BGRT Status", bgrt->status, sizeof(bgrt->status), 3, 7, &passed); + if (bgrt->image_type > 0x00) { passed = false; fwts_failed(fw, LOG_LEVEL_MEDIUM, diff --git a/src/acpi/gtdt/gtdt.c b/src/acpi/gtdt/gtdt.c index 1559d31..f3a8044 100644 --- a/src/acpi/gtdt/gtdt.c +++ b/src/acpi/gtdt/gtdt.c @@ -67,6 +67,7 @@ static int gtdt_test1(fwts_framework *fw) fwts_acpi_table_gtdt_block *block; fwts_acpi_table_gtdt_block_timer *block_timer; fwts_acpi_table_gtdt_watchdog *watchdog; + char field[80]; switch (*ptr) { case 0x00: @@ -145,33 +146,18 @@ static int gtdt_test1(fwts_framework *fw) block_timer->reserved[1], block_timer->reserved[2]); } - if (block_timer->phys_timer_flags & ~0x3) { - passed = false; - fwts_failed(fw, LOG_LEVEL_LOW, - "GTDTFBlockPhysTimerFlagReservedNonZero", - "GTDT block %" PRIu32 " physical timer " - "flags reserved bits 2 to 31 are " - "non-zero, instead got 0x%" PRIx32 ".", - i, block_timer->phys_timer_flags); - } - if (block_timer->virt_timer_flags & ~0x3) { - passed = false; - fwts_failed(fw, LOG_LEVEL_LOW, - "GTDTFBlockVirtTimerFlagReservedNonZero", - "GTDT block %" PRIu32 " virtual timer " - "flags reserved bits 2 to 31 are " - "non-zero, instead got 0x%" PRIx32 ".", - i, block_timer->virt_timer_flags); - } - if (block_timer->common_flags & ~0x3) { - passed = false; - fwts_failed(fw, LOG_LEVEL_LOW, - "GTDTFBlockCommonFlagReservedNonZero", - "GTDT block %" PRIu32 " common flags " - "reserved bits 2 to 31 are " - "non-zero, instead got 0x%" PRIx32 ".", - i, block_timer->common_flags); - } + + snprintf(field, sizeof(field), "block %" PRIu32 " physical timer flags", i); + fwts_acpi_reserved_bits_check(fw, "GTDT", field, block_timer->phys_timer_flags, + sizeof(block_timer->phys_timer_flags), 2, 31, &passed); + + snprintf(field, sizeof(field), "block %" PRIu32 " virtual timer flags", i); + fwts_acpi_reserved_bits_check(fw, "GTDT", field, block_timer->virt_timer_flags, + sizeof(block_timer->virt_timer_flags), 2, 31, &passed); + + snprintf(field, sizeof(field), "block %" PRIu32 " common flags", i); + fwts_acpi_reserved_bits_check(fw, "GTDT", field, block_timer->common_flags, + sizeof(block_timer->common_flags), 2, 31, &passed); } ptr += block->length; break; @@ -203,15 +189,11 @@ static int gtdt_test1(fwts_framework *fw) " reserved is non-zero, got 0x%" PRIx8, i, watchdog->reserved); } - if (watchdog->watchdog_timer_flags & ~0x7) { - passed = false; - fwts_failed(fw, LOG_LEVEL_LOW, - "GTDTWatchDogTimerFlagReservedNonZero", - "GTDT SBSA generic watchdog timer %" PRIu32 - " flags reserved bits 3 to 31 are non-zero, " - "instead got 0x%" PRIx8 ".", - i, watchdog->watchdog_timer_flags); - } + + snprintf(field, sizeof(field), "SBSA generic watchdog timer %" PRIu32 " flags", i); + fwts_acpi_reserved_bits_check(fw, "GTDT", field, watchdog->watchdog_timer_flags, + sizeof(watchdog->watchdog_timer_flags), 3, 31, &passed); + ptr += watchdog->length; break; default: diff --git a/src/acpi/hest/hest.c b/src/acpi/hest/hest.c index 86a5312..289bd18 100644 --- a/src/acpi/hest/hest.c +++ b/src/acpi/hest/hest.c @@ -658,13 +658,10 @@ static void hest_check_generic_error_source( "expecting value 0 to 11", source->notification.type); } - if (source->notification.configuration_write_enable & ~0x3f) { - *passed = false; - fwts_failed(fw, LOG_LEVEL_LOW, - "HESTIA32ConfigWriteEnabledReservedNonZero", - "HEST Configuration Write Enabled Reserved bits [6:31] " - "are non-zero."); - } + + fwts_acpi_reserved_bits_check(fw, "HEST", "HEST Configuration Write Enabled", + source->notification.configuration_write_enable, + sizeof(source->notification.configuration_write_enable), 6, 31, passed); *length -= sizeof(fwts_acpi_table_hest_generic_hardware_error_source); *data += sizeof(fwts_acpi_table_hest_generic_hardware_error_source); @@ -776,13 +773,10 @@ static void hest_check_generic_error_source_v2( "expecting value 0 to 11", source->notification.type); } - if (source->notification.configuration_write_enable & ~0x3f) { - *passed = false; - fwts_failed(fw, LOG_LEVEL_LOW, - "HESTIA32ConfigWriteEnabledReservedNonZero", - "HEST Configuration Write Enabled Reserved bits [6:31] " - "are non-zero."); - } + + fwts_acpi_reserved_bits_check(fw, "HEST", "HEST Configuration Write Enabled", + source->notification.configuration_write_enable, + sizeof(source->notification.configuration_write_enable), 6, 31, passed); *length -= sizeof(fwts_acpi_table_hest_generic_hardware_error_source_v2); *data += sizeof(fwts_acpi_table_hest_generic_hardware_error_source_v2); diff --git a/src/acpi/iort/iort.c b/src/acpi/iort/iort.c index b7046ea..9c20610 100644 --- a/src/acpi/iort/iort.c +++ b/src/acpi/iort/iort.c @@ -148,28 +148,6 @@ static void iort_id_mappings_dump( } /* - * iort_id_mapping_check() - * sanity check ID mapping - */ -static void iort_id_mapping_check( - fwts_framework *fw, - uint32_t i, - fwts_acpi_table_iort_id_mapping *id_mapping, - bool *passed) -{ - if (id_mapping->flags & ~1) { - *passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "IORTIdMappingReservedNonZero", - "IORT ID Mapping %" PRIu32 " Reserved Flags field reserved " - "bits [31:1] should be all zero, got 0x%" PRIx32 - " instead", - i, id_mapping->flags); - } - /* Should also check output_reference is out of range or not */ -} - -/* * iort_id_mappings_check() * snaity check array of ID mappings */ @@ -182,6 +160,7 @@ static void iort_id_mappings_check( uint32_t i; fwts_acpi_table_iort_node *node = (fwts_acpi_table_iort_node *)data; fwts_acpi_table_iort_id_mapping *id_mapping; + char field[80]; id_mapping = (fwts_acpi_table_iort_id_mapping *)(data + node->id_array_offset); @@ -195,7 +174,9 @@ static void iort_id_mappings_check( "or the IORT table size or the node is too small.", i); break; } - iort_id_mapping_check(fw, i, id_mapping, passed); + + snprintf(field, sizeof(field), "ID Mapping %" PRIu32 " flags", i); + fwts_acpi_reserved_bits_check(fw, "IORT", field, id_mapping->flags, sizeof(id_mapping->flags), 1, 31, passed); } } @@ -381,6 +362,7 @@ static void iort_memory_access_properties_check( bool *passed) { uint8_t cca, cpm, dacs; + char field[80]; if (properties->cache_coherent > 1) { *passed = false; @@ -392,30 +374,19 @@ static void iort_memory_access_properties_check( "not coherent).", name, properties->cache_coherent); } - if (properties->allocation_hints & ~0xf) { - *passed = false; - fwts_failed(fw, LOG_LEVEL_HIGH, - "IORTAllocHintsReservedNonZero", - "IORT %s Allocation Hints reserved bits " - "[7:4] are reserved and should be zero.", - name); - } - if (properties->reserved) { - *passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "IORTNodeReservedNonZero", - "IORT %s Node Reserved is 0x%" PRIx32 - " and is reserved and should be zero.", - name, properties->reserved); - } - if (properties->memory_access_flags & ~0x3) { - *passed = false; - fwts_failed(fw, LOG_LEVEL_HIGH, - "IORTMemoryAccessFlagsReservedNonZero", - "IORT %s Memory Access Flags is 0x%" PRIx8 - " and all the reserved bits [7:2] should be zero but some are not.", - name, properties->memory_access_flags); - } + + snprintf(field, sizeof(field), "%s Allocation Hints", name); + fwts_acpi_reserved_bits_check(fw, "IORT", field, properties->allocation_hints, + sizeof(properties->allocation_hints), 4, 7, passed); + + snprintf(field, sizeof(field), "%s Reserved", name); + fwts_acpi_reserved_zero_check(fw, "IORT", field, properties->reserved, + sizeof(properties->reserved), passed); + + snprintf(field, sizeof(field), "%s Memory Access Flags", name); + fwts_acpi_reserved_bits_check(fw, "IORT", field, properties->memory_access_flags, + sizeof(properties->memory_access_flags), 2, 7, passed); + cca = properties->cache_coherent & 1; cpm = properties->memory_access_flags & 1; dacs = (properties->memory_access_flags >> 1) & 1; diff --git a/src/acpi/lpit/lpit.c b/src/acpi/lpit/lpit.c index 2273ee4..eb4e3af 100644 --- a/src/acpi/lpit/lpit.c +++ b/src/acpi/lpit/lpit.c @@ -95,14 +95,7 @@ static void lpit_check_type_0( fwts_log_nl(fw); fwts_acpi_reserved_zero_check(fw, "LPIT", "Native C-state based LPI structure reserved", lpi->reserved, sizeof(lpi->reserved), passed); - - if (lpi->flags & ~3) { - *passed = false; - fwts_failed(fw, LOG_LEVEL_LOW, - "LPITNativeCStateLpitFlagsReserved", - "Some of the Native C-state based LPI structure flags " - "bits [31:2] are set, they are expected to be zero"); - } + fwts_acpi_reserved_bits_check(fw, "LPIT", "LPI structure flags", lpi->flags, sizeof(lpi->flags), 2, 31, passed); /* 2.2.1.2, if FFH, then it is a MSR, check GAS fields */ if (((lpi->flags & 2) == 0) && diff --git a/src/acpi/mchi/mchi.c b/src/acpi/mchi/mchi.c index 6244b93..8ddff2c 100644 --- a/src/acpi/mchi/mchi.c +++ b/src/acpi/mchi/mchi.c @@ -116,14 +116,9 @@ static int mchi_test1(fwts_framework *fw) "allowed values are 0x00 (Unspecifier), 0x01 (MCTP), 0x02 (IPMI) or " "255 (OEM defined)", mchi->protocol_identifier); } - if (mchi->interrupt_type & ~0x03) { - passed = false; - fwts_failed(fw, LOG_LEVEL_HIGH, - "MCHIInterruptTypeReservedNonZero", - "MCHI Interrupt Type 0x%2.2" PRIx8 " has some reserved " - "bits [7:2] set, these should be all zero.", - mchi->interrupt_type); - } + + fwts_acpi_reserved_bits_check(fw, "MCHI", "Interrupt Type", mchi->interrupt_type, sizeof(mchi->interrupt_type), 2, 7, &passed); + if (((mchi->interrupt_type & 0x01) == 0) && (mchi->gpe != 0)) { passed = false; @@ -134,14 +129,9 @@ static int mchi_test1(fwts_framework *fw) "(SCI triggered through GPE non-supported)", mchi->gpe); } - if (mchi->pci_device_flag & ~0x01) { - passed = false; - fwts_failed(fw, LOG_LEVEL_HIGH, - "MCHIPciDeviceFlagReservedNonZero", - "MCHI PCI Device Flag 0x%2.2" PRIx8 " has some reserved " - "bits [7:1] set, these should be all zero.", - mchi->pci_device_flag); - } + + fwts_acpi_reserved_bits_check(fw, "MCHI", "PCI Device Flag", mchi->pci_device_flag, sizeof(mchi->pci_device_flag), 1, 7, &passed); + if (((mchi->interrupt_type & 0x02) == 0) && (mchi->global_system_interrupt != 0)) { passed = false; diff --git a/src/acpi/nfit/nfit.c b/src/acpi/nfit/nfit.c index fc69798..e1a4815 100644 --- a/src/acpi/nfit/nfit.c +++ b/src/acpi/nfit/nfit.c @@ -284,14 +284,7 @@ static int nfit_test1(fwts_framework *fw) if (nfit_struct->reserved != 0) reserved_passed = nfit_struct->reserved; - if (nfit_struct->valid_fields & ~0x01) { - passed = false; - fwts_failed(fw, LOG_LEVEL_HIGH, - "NFITBadValidField", - "NFIT Valid Field's Bits[7..1] must be zero, got " - "0x%2.2" PRIx8 " instead", nfit_struct->valid_fields); - } - + fwts_acpi_reserved_bits_check(fw, "NFIT", "Valid", nfit_struct->valid_fields, sizeof(nfit_struct->valid_fields), 1, 7, &passed); fwts_acpi_reserved_bits_check(fw, "NFIT", "NVDIMM Control Region Flags", nfit_struct->flags, sizeof(nfit_struct->flags), 1, 15, &passed); } else if (entry->type == FWTS_ACPI_NFIT_TYPE_DATA_REGION) { diff --git a/src/acpi/pptt/pptt.c b/src/acpi/pptt/pptt.c index 50b7491..25f5d9a 100644 --- a/src/acpi/pptt/pptt.c +++ b/src/acpi/pptt/pptt.c @@ -85,14 +85,7 @@ static void pptt_cache_test(fwts_framework *fw, const fwts_acpi_table_pptt_cache fwts_acpi_reserved_zero_check(fw, "PPTT", "Reserved", entry->reserved, sizeof(entry->reserved), passed); fwts_acpi_reserved_bits_check(fw, "PPTT", "Flags", entry->flags, sizeof(entry->flags), 7, 31, passed); - - if (entry->attributes & ~0x1f) { - *passed = false; - fwts_failed(fw, LOG_LEVEL_HIGH, - "PPTTBadAttributes", - "PPTT attributes's Bits[7..5] must be zero, got " - "0x%2.2" PRIx8 " instead", entry->attributes); - } + fwts_acpi_reserved_bits_check(fw, "PPTT", "Attributes", entry->attributes, sizeof(entry->attributes), 5, 7, passed); } static void pptt_id_test(fwts_framework *fw, const fwts_acpi_table_pptt_id *entry, bool *passed) diff --git a/src/acpi/spmi/spmi.c b/src/acpi/spmi/spmi.c index 23b9b22..86acb11 100644 --- a/src/acpi/spmi/spmi.c +++ b/src/acpi/spmi/spmi.c @@ -123,14 +123,7 @@ static int spmi_test1(fwts_framework *fw) "0x%2.2" PRIx8 " instead", spmi->reserved1); } - /* Only bottom 2 bits should be set */ - if (spmi->interrupt_type & ~0x03) { - passed = false; - fwts_failed(fw, LOG_LEVEL_LOW, - "SPMIInterruptTypeReservedNonZero", - "SPMI Interrupt Type reserved bits [7:2] must be 0, got " - "0x%2.2" PRIx8 " instead", spmi->interrupt_type); - } + fwts_acpi_reserved_bits_check(fw, "SPMI", "Interrupt type", spmi->interrupt_type, sizeof(spmi->interrupt_type), 2, 7, &passed); /* Check for zero GPE on specific condition of interrupt type */ if (((spmi->interrupt_type & 1) == 0) && @@ -149,14 +142,8 @@ static int spmi_test1(fwts_framework *fw) "SPMI reserved field (offset 42) must be 0x00, got " "0x%2.2" PRIx8 " instead", spmi->reserved2); } - /* Only bottom 1 bit should be set */ - if (spmi->pci_device_flag & ~0x01) { - passed = false; - fwts_failed(fw, LOG_LEVEL_MEDIUM, - "SPMIPciDeviceFlag", - "SPMI PCI Device Flag reserved bits [7:1] must be 0, got " - "0x%2.2" PRIx8 " instead", spmi->pci_device_flag); - } + + fwts_acpi_reserved_bits_check(fw, "SPMI", "PCI device flag", spmi->pci_device_flag, sizeof(spmi->pci_device_flag), 1, 7, &passed); if (((spmi->interrupt_type & 2) == 0) && (spmi->global_system_interrupt != 0)) {
Signed-off-by: Alex Hung <alex.hung@canonical.com> --- src/acpi/bgrt/bgrt.c | 11 +++------ src/acpi/gtdt/gtdt.c | 54 +++++++++++++++---------------------------- src/acpi/hest/hest.c | 22 +++++++----------- src/acpi/iort/iort.c | 65 +++++++++++++++------------------------------------- src/acpi/lpit/lpit.c | 9 +------- src/acpi/mchi/mchi.c | 22 +++++------------- src/acpi/nfit/nfit.c | 9 +------- src/acpi/pptt/pptt.c | 9 +------- src/acpi/spmi/spmi.c | 19 +++------------ 9 files changed, 59 insertions(+), 161 deletions(-)