| Message ID | 20210416030954.700665-1-alex.hung@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] acpi: pmtt: update PMTT to revision 2 (mantis 1975) | expand |
On 16/04/2021 04:09, Alex Hung wrote: > There are signifcant changes in revision 2 which is not compatible with > revision 1. > > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- > src/acpi/pmtt/pmtt.c | 146 ++++++++++++++++++------------------ > src/lib/include/fwts_acpi.h | 21 ++---- > 2 files changed, 78 insertions(+), 89 deletions(-) > > diff --git a/src/acpi/pmtt/pmtt.c b/src/acpi/pmtt/pmtt.c > index 27ec58b8..7cd0952b 100644 > --- a/src/acpi/pmtt/pmtt.c > +++ b/src/acpi/pmtt/pmtt.c > @@ -23,10 +23,15 @@ > #include <inttypes.h> > #include <stdbool.h> > > +static void pmtt_memory_device(fwts_framework *fw, fwts_acpi_table_pmtt_header *entry, uint32_t offset, bool *passed); > + > static fwts_acpi_table_info *table; > acpi_table_init(PMTT, &table) > > -static void pmtt_subtable_header_test(fwts_framework *fw, fwts_acpi_table_pmtt_header *entry, bool *passed) > +static void pmtt_subtable_header_test( > + fwts_framework *fw, > + fwts_acpi_table_pmtt_header *entry, > + bool *passed) > { > fwts_log_info_verbatim(fw, " PMTT Subtable:"); > fwts_log_info_simp_int(fw, " Type: ", entry->type); > @@ -48,16 +53,14 @@ static void pmtt_subtable_header_test(fwts_framework *fw, fwts_acpi_table_pmtt_h > fwts_acpi_reserved_zero_check("PMTT", "Reserved2", entry->reserved2, passed); > } > > -static void pmtt_physical_component_test(fwts_framework *fw, fwts_acpi_table_pmtt_physical_component *entry, bool *passed) > +static void pmtt_physical_component_test( > + fwts_framework *fw, > + fwts_acpi_table_pmtt_physical_component *entry, > + bool *passed) > { > pmtt_subtable_header_test(fw, &entry->header, passed); > - fwts_log_info_simp_int(fw, " Physical Component Identifier: ", entry->component_id); > - fwts_log_info_simp_int(fw, " Reserved: ", entry->reserved); > - fwts_log_info_simp_int(fw, " Size of DIMM: ", entry->memory_size); > fwts_log_info_simp_int(fw, " SMBIOS Handle: ", entry->bios_handle); > > - fwts_acpi_reserved_zero_check("PMTT", "Reserved", entry->reserved, passed); > - > if ((entry->bios_handle & 0xFFFF0000) != 0 && entry->bios_handle != 0xFFFFFFFF) { > *passed = false; > fwts_failed(fw, LOG_LEVEL_HIGH, > @@ -67,43 +70,30 @@ static void pmtt_physical_component_test(fwts_framework *fw, fwts_acpi_table_pmt > } > } > > -static void pmtt_controller_test(fwts_framework *fw, fwts_acpi_table_pmtt_controller *entry, bool *passed) > +static void pmtt_controller_test( > + fwts_framework *fw, > + fwts_acpi_table_pmtt_controller *entry, > + uint32_t entry_offset, > + bool *passed) > { > fwts_acpi_table_pmtt_header *header; > uint32_t offset = 0; > - size_t i; > > pmtt_subtable_header_test(fw, &entry->header, passed); > - fwts_log_info_simp_int(fw, " Read Latency: ", entry->read_latency); > - fwts_log_info_simp_int(fw, " Write latency: ", entry->write_latency); > - fwts_log_info_simp_int(fw, " Read Bandwidth: ", entry->read_bandwidth); > - fwts_log_info_simp_int(fw, " Write Bandwidth: ", entry->write_bandwidth); > - fwts_log_info_simp_int(fw, " Optimal Access Unit: ", entry->access_width); > - fwts_log_info_simp_int(fw, " Optimal Access Alignment: ", entry->alignment); > + fwts_log_info_simp_int(fw, " Memory Controller ID ", entry->memory_controller_id); > fwts_log_info_simp_int(fw, " Reserved: ", entry->reserved); > - fwts_log_info_simp_int(fw, " Number of Proximity Domains: ", entry->domain_count); > > fwts_acpi_reserved_zero_check("PMTT", "Reserved", entry->reserved, passed); > > offset = sizeof(fwts_acpi_table_pmtt_controller); > - if (entry->header.length < offset + sizeof(fwts_acpi_table_pmtt_domain) * entry->domain_count) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "PMTTOutOfBound", > - "PMTT's length is too small to contain all fields"); > - return; > - } > - > - fwts_acpi_table_pmtt_domain *domain = (fwts_acpi_table_pmtt_domain *)(((char *) entry) + offset); > - for (i = 0; i < entry->domain_count; i++) { > - fwts_log_info_simp_int(fw, " Proximity Domain: ", domain->proximity_domain); > - domain++; > - /* TODO cross check proximity domain with SRAT table*/ > - } > - > - offset += sizeof(fwts_acpi_table_pmtt_domain) * entry->domain_count; > header = (fwts_acpi_table_pmtt_header *) (((char *) entry) + offset); > while (offset < entry->header.length) { > + /* stop if sub-structure is outside the table */ > + if (fwts_acpi_structure_range_check(fw, "PMTT", table->length, entry_offset + offset)) { > + *passed = false; > + break; > + } > + > if (header->length == 0) { > fwts_failed(fw, LOG_LEVEL_CRITICAL, > "PMTTBadSubtableLength", > @@ -111,22 +101,18 @@ static void pmtt_controller_test(fwts_framework *fw, fwts_acpi_table_pmtt_contro > break; > } > > - if (header->type == FWTS_ACPI_PMTT_TYPE_DIMM) { > - pmtt_physical_component_test(fw, (fwts_acpi_table_pmtt_physical_component *) header, passed); > - } else { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "PMTTBadSubtableType", > - "PMTT Controller must have subtable with Type 2, got " > - "0x%4.4" PRIx16 " instead", header->type); > - } > + pmtt_memory_device(fw, header, entry_offset + offset, passed); > > offset += header->length; > header = (fwts_acpi_table_pmtt_header *)(((char *) entry) + offset); > } > } > > -static void pmtt_socket_test(fwts_framework *fw, fwts_acpi_table_pmtt_socket *entry, bool *passed) > +static void pmtt_socket_test( > + fwts_framework *fw, > + fwts_acpi_table_pmtt_socket *entry, > + uint32_t entry_offset, > + bool *passed) > { > fwts_acpi_table_pmtt_header *header; > uint32_t offset; > @@ -140,6 +126,12 @@ static void pmtt_socket_test(fwts_framework *fw, fwts_acpi_table_pmtt_socket *en > offset = sizeof(fwts_acpi_table_pmtt_socket); > header = (fwts_acpi_table_pmtt_header *) (((char *) entry) + offset); > while (offset < entry->header.length) { > + /* stop if sub-structure is outside the table */ > + if (fwts_acpi_structure_range_check(fw, "PMTT", table->length, entry_offset + offset)) { > + *passed = false; > + break; > + } > + > if (header->length == 0) { > fwts_failed(fw, LOG_LEVEL_CRITICAL, > "PMTTBadSubtableLength", > @@ -147,21 +139,42 @@ static void pmtt_socket_test(fwts_framework *fw, fwts_acpi_table_pmtt_socket *en > break; > } > > - if (header->type == FWTS_ACPI_PMTT_TYPE_CONTROLLER) { > - pmtt_controller_test(fw, (fwts_acpi_table_pmtt_controller *) header, passed); > - } else { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "PMTTBadSubtableType", > - "PMTT Socket must have subtable with Type 1, got " > - "0x%4.4" PRIx16 " instead", header->type); > - } > + pmtt_memory_device(fw, header, entry_offset + offset, passed); > > offset += header->length; > header = (fwts_acpi_table_pmtt_header *)(((char *) entry) + offset); > } > } > > +static void pmtt_memory_device( > + fwts_framework *fw, > + fwts_acpi_table_pmtt_header *entry, > + uint32_t offset, > + bool *passed) > +{ > + switch(entry->type) { > + case FWTS_ACPI_PMTT_TYPE_SOCKET: > + pmtt_socket_test(fw, (fwts_acpi_table_pmtt_socket *) entry, offset, passed); > + break; > + case FWTS_ACPI_PMTT_TYPE_CONTROLLER: > + pmtt_controller_test(fw, (fwts_acpi_table_pmtt_controller *) entry, offset, passed); > + break; > + case FWTS_ACPI_PMTT_TYPE_DIMM: > + pmtt_physical_component_test(fw, (fwts_acpi_table_pmtt_physical_component *) entry, passed); > + break; > + case FWTS_ACPI_PMTT_TYPE_VENDOR_SPECIFIC: > + /* no tests for vendor-specific type */ > + break; > + default: > + *passed = false; > + fwts_failed(fw, LOG_LEVEL_HIGH, > + "PMTTBadSubtableType", > + "PMTT must have subtable with Type 1..2 or 0xFF, got " > + "0x%4.4" PRIx16 " instead", entry->type); > + break; > + } > +} > + > static int pmtt_test1(fwts_framework *fw) > { > fwts_acpi_table_pmtt *pmtt = (fwts_acpi_table_pmtt*) table->data; > @@ -169,10 +182,14 @@ static int pmtt_test1(fwts_framework *fw) > uint32_t offset; > bool passed = true; > > - fwts_log_info_verbatim(fw, "PMTT Table:"); > - fwts_log_info_simp_int(fw, " Reserved: ", pmtt->reserved); > + if (pmtt->header.revision < 2) { > + fwts_failed(fw, LOG_LEVEL_CRITICAL, "PMTTDeprecatedRevision", > + "PMTT Revision 1 has been deprecated in ACPI 6.4"); > + return FWTS_OK; > + } > > - fwts_acpi_reserved_zero_check("PMTT", "Reserved", pmtt->reserved, &passed); > + fwts_log_info_verbatim(fw, "PMTT Table:"); > + fwts_log_info_simp_int(fw, " Number of Memory Devices: ", pmtt->num_devices); > > entry = (fwts_acpi_table_pmtt_header *) (table->data + sizeof(fwts_acpi_table_pmtt)); > offset = sizeof(fwts_acpi_table_pmtt); > @@ -186,29 +203,12 @@ static int pmtt_test1(fwts_framework *fw) > break; > } > > - switch(entry->type) { > - case FWTS_ACPI_PMTT_TYPE_SOCKET: > - pmtt_socket_test(fw, (fwts_acpi_table_pmtt_socket *) entry, &passed); > - break; > - case FWTS_ACPI_PMTT_TYPE_CONTROLLER: > - pmtt_controller_test(fw, (fwts_acpi_table_pmtt_controller *) entry, &passed); > - break; > - case FWTS_ACPI_PMTT_TYPE_DIMM: > - pmtt_physical_component_test(fw, (fwts_acpi_table_pmtt_physical_component *) entry, &passed); > - break; > - default: > - passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "PMTTBadSubtableType", > - "PMTT must have subtable with Type 1..2, got " > - "0x%4.4" PRIx16 " instead", entry->type); > - break; > - } > + pmtt_memory_device(fw, entry, offset, &passed); > > offset += entry->length; > entry = (fwts_acpi_table_pmtt_header *) (table->data + offset); > + fwts_log_nl(fw); > } > - fwts_log_nl(fw); > > if (passed) > fwts_passed(fw, "No issues found in PMTT table."); > diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h > index 2a6c76ea..c1345803 100644 > --- a/src/lib/include/fwts_acpi.h > +++ b/src/lib/include/fwts_acpi.h > @@ -1134,7 +1134,7 @@ typedef struct { > */ > typedef struct { > fwts_acpi_table_header header; > - uint32_t reserved; > + uint32_t num_devices; > } __attribute__ ((packed)) fwts_acpi_table_pmtt; > > typedef struct { > @@ -1143,13 +1143,15 @@ typedef struct { > uint16_t length; > uint16_t flags; > uint16_t reserved2; > + uint32_t num_devices; > } __attribute__ ((packed)) fwts_acpi_table_pmtt_header; > > typedef enum { > FWTS_ACPI_PMTT_TYPE_SOCKET = 0, > FWTS_ACPI_PMTT_TYPE_CONTROLLER = 1, > FWTS_ACPI_PMTT_TYPE_DIMM = 2, > - FWTS_ACPI_PMTT_TYPE_RESERVED = 3 /* 0x03-0xFF are reserved */ > + FWTS_ACPI_PMTT_TYPE_RESERVED = 3, /* 0x03-0xFE are reserved */ > + FWTS_ACPI_PMTT_TYPE_VENDOR_SPECIFIC = 0xFF > } fwts_acpi_pmtt_type; > > typedef struct { > @@ -1160,25 +1162,12 @@ typedef struct { > > typedef struct { > fwts_acpi_table_pmtt_header header; > - uint32_t read_latency; > - uint32_t write_latency; > - uint32_t read_bandwidth; > - uint32_t write_bandwidth; > - uint16_t access_width; > - uint16_t alignment; > + uint16_t memory_controller_id; > uint16_t reserved; > - uint16_t domain_count; > } __attribute__ ((packed)) fwts_acpi_table_pmtt_controller; > > -typedef struct { > - uint32_t proximity_domain; > -} __attribute__ ((packed)) fwts_acpi_table_pmtt_domain; > - > typedef struct { > fwts_acpi_table_pmtt_header header; > - uint16_t component_id; > - uint16_t reserved; > - uint32_t memory_size; > uint32_t bios_handle; > } __attribute__ ((packed)) fwts_acpi_table_pmtt_physical_component; > > I'm getting a build issue based on today's tip: acpi/pmtt/pmtt.c:92:7: error: implicit declaration of function ‘fwts_acpi_structure_range_check’; did you mean ‘fwts_acpi_structure_length_check’? [-Werror=implicit-function-declaration] 92 | if (fwts_acpi_structure_range_check(fw, "PMTT", table->length, entry_offset + offset)) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | fwts_acpi_structure_length_check I guess there are some pending changes that are not in the repo yet. Colin
On Fri, Apr 16, 2021 at 1:35 AM Colin Ian King <colin.king@canonical.com> wrote: > On 16/04/2021 04:09, Alex Hung wrote: > > There are signifcant changes in revision 2 which is not compatible with > > revision 1. > > > > Signed-off-by: Alex Hung <alex.hung@canonical.com> > > --- > > src/acpi/pmtt/pmtt.c | 146 ++++++++++++++++++------------------ > > src/lib/include/fwts_acpi.h | 21 ++---- > > 2 files changed, 78 insertions(+), 89 deletions(-) > > > > diff --git a/src/acpi/pmtt/pmtt.c b/src/acpi/pmtt/pmtt.c > > index 27ec58b8..7cd0952b 100644 > > --- a/src/acpi/pmtt/pmtt.c > > +++ b/src/acpi/pmtt/pmtt.c > > @@ -23,10 +23,15 @@ > > #include <inttypes.h> > > #include <stdbool.h> > > > > +static void pmtt_memory_device(fwts_framework *fw, > fwts_acpi_table_pmtt_header *entry, uint32_t offset, bool *passed); > > + > > static fwts_acpi_table_info *table; > > acpi_table_init(PMTT, &table) > > > > -static void pmtt_subtable_header_test(fwts_framework *fw, > fwts_acpi_table_pmtt_header *entry, bool *passed) > > +static void pmtt_subtable_header_test( > > + fwts_framework *fw, > > + fwts_acpi_table_pmtt_header *entry, > > + bool *passed) > > { > > fwts_log_info_verbatim(fw, " PMTT Subtable:"); > > fwts_log_info_simp_int(fw, " Type: ", > entry->type); > > @@ -48,16 +53,14 @@ static void pmtt_subtable_header_test(fwts_framework > *fw, fwts_acpi_table_pmtt_h > > fwts_acpi_reserved_zero_check("PMTT", "Reserved2", > entry->reserved2, passed); > > } > > > > -static void pmtt_physical_component_test(fwts_framework *fw, > fwts_acpi_table_pmtt_physical_component *entry, bool *passed) > > +static void pmtt_physical_component_test( > > + fwts_framework *fw, > > + fwts_acpi_table_pmtt_physical_component *entry, > > + bool *passed) > > { > > pmtt_subtable_header_test(fw, &entry->header, passed); > > - fwts_log_info_simp_int(fw, " Physical Component Identifier: ", > entry->component_id); > > - fwts_log_info_simp_int(fw, " Reserved: ", > entry->reserved); > > - fwts_log_info_simp_int(fw, " Size of DIMM: ", > entry->memory_size); > > fwts_log_info_simp_int(fw, " SMBIOS Handle: ", > entry->bios_handle); > > > > - fwts_acpi_reserved_zero_check("PMTT", "Reserved", entry->reserved, > passed); > > - > > if ((entry->bios_handle & 0xFFFF0000) != 0 && entry->bios_handle > != 0xFFFFFFFF) { > > *passed = false; > > fwts_failed(fw, LOG_LEVEL_HIGH, > > @@ -67,43 +70,30 @@ static void > pmtt_physical_component_test(fwts_framework *fw, fwts_acpi_table_pmt > > } > > } > > > > -static void pmtt_controller_test(fwts_framework *fw, > fwts_acpi_table_pmtt_controller *entry, bool *passed) > > +static void pmtt_controller_test( > > + fwts_framework *fw, > > + fwts_acpi_table_pmtt_controller *entry, > > + uint32_t entry_offset, > > + bool *passed) > > { > > fwts_acpi_table_pmtt_header *header; > > uint32_t offset = 0; > > - size_t i; > > > > pmtt_subtable_header_test(fw, &entry->header, passed); > > - fwts_log_info_simp_int(fw, " Read Latency: ", > entry->read_latency); > > - fwts_log_info_simp_int(fw, " Write latency: ", > entry->write_latency); > > - fwts_log_info_simp_int(fw, " Read Bandwidth: ", > entry->read_bandwidth); > > - fwts_log_info_simp_int(fw, " Write Bandwidth: ", > entry->write_bandwidth); > > - fwts_log_info_simp_int(fw, " Optimal Access Unit: ", > entry->access_width); > > - fwts_log_info_simp_int(fw, " Optimal Access Alignment: ", > entry->alignment); > > + fwts_log_info_simp_int(fw, " Memory Controller ID ", > entry->memory_controller_id); > > fwts_log_info_simp_int(fw, " Reserved: ", > entry->reserved); > > - fwts_log_info_simp_int(fw, " Number of Proximity Domains: ", > entry->domain_count); > > > > fwts_acpi_reserved_zero_check("PMTT", "Reserved", entry->reserved, > passed); > > > > offset = sizeof(fwts_acpi_table_pmtt_controller); > > - if (entry->header.length < offset + > sizeof(fwts_acpi_table_pmtt_domain) * entry->domain_count) { > > - *passed = false; > > - fwts_failed(fw, LOG_LEVEL_HIGH, > > - "PMTTOutOfBound", > > - "PMTT's length is too small to contain all > fields"); > > - return; > > - } > > - > > - fwts_acpi_table_pmtt_domain *domain = (fwts_acpi_table_pmtt_domain > *)(((char *) entry) + offset); > > - for (i = 0; i < entry->domain_count; i++) { > > - fwts_log_info_simp_int(fw, " Proximity Domain: > ", domain->proximity_domain); > > - domain++; > > - /* TODO cross check proximity domain with SRAT table*/ > > - } > > - > > - offset += sizeof(fwts_acpi_table_pmtt_domain) * > entry->domain_count; > > header = (fwts_acpi_table_pmtt_header *) (((char *) entry) + > offset); > > while (offset < entry->header.length) { > > + /* stop if sub-structure is outside the table */ > > + if (fwts_acpi_structure_range_check(fw, "PMTT", > table->length, entry_offset + offset)) { > > + *passed = false; > > + break; > > + } > > + > > if (header->length == 0) { > > fwts_failed(fw, LOG_LEVEL_CRITICAL, > > "PMTTBadSubtableLength", > > @@ -111,22 +101,18 @@ static void pmtt_controller_test(fwts_framework > *fw, fwts_acpi_table_pmtt_contro > > break; > > } > > > > - if (header->type == FWTS_ACPI_PMTT_TYPE_DIMM) { > > - pmtt_physical_component_test(fw, > (fwts_acpi_table_pmtt_physical_component *) header, passed); > > - } else { > > - *passed = false; > > - fwts_failed(fw, LOG_LEVEL_HIGH, > > - "PMTTBadSubtableType", > > - "PMTT Controller must have subtable with > Type 2, got " > > - "0x%4.4" PRIx16 " instead", header->type); > > - } > > + pmtt_memory_device(fw, header, entry_offset + offset, > passed); > > > > offset += header->length; > > header = (fwts_acpi_table_pmtt_header *)(((char *) entry) > + offset); > > } > > } > > > > -static void pmtt_socket_test(fwts_framework *fw, > fwts_acpi_table_pmtt_socket *entry, bool *passed) > > +static void pmtt_socket_test( > > + fwts_framework *fw, > > + fwts_acpi_table_pmtt_socket *entry, > > + uint32_t entry_offset, > > + bool *passed) > > { > > fwts_acpi_table_pmtt_header *header; > > uint32_t offset; > > @@ -140,6 +126,12 @@ static void pmtt_socket_test(fwts_framework *fw, > fwts_acpi_table_pmtt_socket *en > > offset = sizeof(fwts_acpi_table_pmtt_socket); > > header = (fwts_acpi_table_pmtt_header *) (((char *) entry) + > offset); > > while (offset < entry->header.length) { > > + /* stop if sub-structure is outside the table */ > > + if (fwts_acpi_structure_range_check(fw, "PMTT", > table->length, entry_offset + offset)) { > > + *passed = false; > > + break; > > + } > > + > > if (header->length == 0) { > > fwts_failed(fw, LOG_LEVEL_CRITICAL, > > "PMTTBadSubtableLength", > > @@ -147,21 +139,42 @@ static void pmtt_socket_test(fwts_framework *fw, > fwts_acpi_table_pmtt_socket *en > > break; > > } > > > > - if (header->type == FWTS_ACPI_PMTT_TYPE_CONTROLLER) { > > - pmtt_controller_test(fw, > (fwts_acpi_table_pmtt_controller *) header, passed); > > - } else { > > - *passed = false; > > - fwts_failed(fw, LOG_LEVEL_HIGH, > > - "PMTTBadSubtableType", > > - "PMTT Socket must have subtable with Type > 1, got " > > - "0x%4.4" PRIx16 " instead", header->type); > > - } > > + pmtt_memory_device(fw, header, entry_offset + offset, > passed); > > > > offset += header->length; > > header = (fwts_acpi_table_pmtt_header *)(((char *) entry) > + offset); > > } > > } > > > > +static void pmtt_memory_device( > > + fwts_framework *fw, > > + fwts_acpi_table_pmtt_header *entry, > > + uint32_t offset, > > + bool *passed) > > +{ > > + switch(entry->type) { > > + case FWTS_ACPI_PMTT_TYPE_SOCKET: > > + pmtt_socket_test(fw, (fwts_acpi_table_pmtt_socket > *) entry, offset, passed); > > + break; > > + case FWTS_ACPI_PMTT_TYPE_CONTROLLER: > > + pmtt_controller_test(fw, > (fwts_acpi_table_pmtt_controller *) entry, offset, passed); > > + break; > > + case FWTS_ACPI_PMTT_TYPE_DIMM: > > + pmtt_physical_component_test(fw, > (fwts_acpi_table_pmtt_physical_component *) entry, passed); > > + break; > > + case FWTS_ACPI_PMTT_TYPE_VENDOR_SPECIFIC: > > + /* no tests for vendor-specific type */ > > + break; > > + default: > > + *passed = false; > > + fwts_failed(fw, LOG_LEVEL_HIGH, > > + "PMTTBadSubtableType", > > + "PMTT must have subtable with Type 1..2 or > 0xFF, got " > > + "0x%4.4" PRIx16 " instead", entry->type); > > + break; > > + } > > +} > > + > > static int pmtt_test1(fwts_framework *fw) > > { > > fwts_acpi_table_pmtt *pmtt = (fwts_acpi_table_pmtt*) table->data; > > @@ -169,10 +182,14 @@ static int pmtt_test1(fwts_framework *fw) > > uint32_t offset; > > bool passed = true; > > > > - fwts_log_info_verbatim(fw, "PMTT Table:"); > > - fwts_log_info_simp_int(fw, " Reserved: ", > pmtt->reserved); > > + if (pmtt->header.revision < 2) { > > + fwts_failed(fw, LOG_LEVEL_CRITICAL, > "PMTTDeprecatedRevision", > > + "PMTT Revision 1 has been deprecated in ACPI 6.4"); > > + return FWTS_OK; > > + } > > > > - fwts_acpi_reserved_zero_check("PMTT", "Reserved", pmtt->reserved, > &passed); > > + fwts_log_info_verbatim(fw, "PMTT Table:"); > > + fwts_log_info_simp_int(fw, " Number of Memory Devices: ", > pmtt->num_devices); > > > > entry = (fwts_acpi_table_pmtt_header *) (table->data + > sizeof(fwts_acpi_table_pmtt)); > > offset = sizeof(fwts_acpi_table_pmtt); > > @@ -186,29 +203,12 @@ static int pmtt_test1(fwts_framework *fw) > > break; > > } > > > > - switch(entry->type) { > > - case FWTS_ACPI_PMTT_TYPE_SOCKET: > > - pmtt_socket_test(fw, > (fwts_acpi_table_pmtt_socket *) entry, &passed); > > - break; > > - case FWTS_ACPI_PMTT_TYPE_CONTROLLER: > > - pmtt_controller_test(fw, > (fwts_acpi_table_pmtt_controller *) entry, &passed); > > - break; > > - case FWTS_ACPI_PMTT_TYPE_DIMM: > > - pmtt_physical_component_test(fw, > (fwts_acpi_table_pmtt_physical_component *) entry, &passed); > > - break; > > - default: > > - passed = false; > > - fwts_failed(fw, LOG_LEVEL_HIGH, > > - "PMTTBadSubtableType", > > - "PMTT must have subtable with Type > 1..2, got " > > - "0x%4.4" PRIx16 " instead", > entry->type); > > - break; > > - } > > + pmtt_memory_device(fw, entry, offset, &passed); > > > > offset += entry->length; > > entry = (fwts_acpi_table_pmtt_header *) (table->data + > offset); > > + fwts_log_nl(fw); > > } > > - fwts_log_nl(fw); > > > > if (passed) > > fwts_passed(fw, "No issues found in PMTT table."); > > diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h > > index 2a6c76ea..c1345803 100644 > > --- a/src/lib/include/fwts_acpi.h > > +++ b/src/lib/include/fwts_acpi.h > > @@ -1134,7 +1134,7 @@ typedef struct { > > */ > > typedef struct { > > fwts_acpi_table_header header; > > - uint32_t reserved; > > + uint32_t num_devices; > > } __attribute__ ((packed)) fwts_acpi_table_pmtt; > > > > typedef struct { > > @@ -1143,13 +1143,15 @@ typedef struct { > > uint16_t length; > > uint16_t flags; > > uint16_t reserved2; > > + uint32_t num_devices; > > } __attribute__ ((packed)) fwts_acpi_table_pmtt_header; > > > > typedef enum { > > FWTS_ACPI_PMTT_TYPE_SOCKET = 0, > > FWTS_ACPI_PMTT_TYPE_CONTROLLER = 1, > > FWTS_ACPI_PMTT_TYPE_DIMM = 2, > > - FWTS_ACPI_PMTT_TYPE_RESERVED = 3 /* 0x03-0xFF are > reserved */ > > + FWTS_ACPI_PMTT_TYPE_RESERVED = 3, /* 0x03-0xFE are > reserved */ > > + FWTS_ACPI_PMTT_TYPE_VENDOR_SPECIFIC = 0xFF > > } fwts_acpi_pmtt_type; > > > > typedef struct { > > @@ -1160,25 +1162,12 @@ typedef struct { > > > > typedef struct { > > fwts_acpi_table_pmtt_header header; > > - uint32_t read_latency; > > - uint32_t write_latency; > > - uint32_t read_bandwidth; > > - uint32_t write_bandwidth; > > - uint16_t access_width; > > - uint16_t alignment; > > + uint16_t memory_controller_id; > > uint16_t reserved; > > - uint16_t domain_count; > > } __attribute__ ((packed)) fwts_acpi_table_pmtt_controller; > > > > -typedef struct { > > - uint32_t proximity_domain; > > -} __attribute__ ((packed)) fwts_acpi_table_pmtt_domain; > > - > > typedef struct { > > fwts_acpi_table_pmtt_header header; > > - uint16_t component_id; > > - uint16_t reserved; > > - uint32_t memory_size; > > uint32_t bios_handle; > > } __attribute__ ((packed)) fwts_acpi_table_pmtt_physical_component; > > > > > > > I'm getting a build issue based on today's tip: > > acpi/pmtt/pmtt.c:92:7: error: implicit declaration of function > ‘fwts_acpi_structure_range_check’; did you mean > ‘fwts_acpi_structure_length_check’? [-Werror=implicit-function-declaration] > 92 | if (fwts_acpi_structure_range_check(fw, "PMTT", table->length, > entry_offset + offset)) { > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > | fwts_acpi_structure_length_check > > > I guess there are some pending changes that are not in the repo yet. > Yes the function is in another V2 patch ( http://patchwork.ozlabs.org/project/fwts/patch/20210411205621.116069-1-alex.hung@canonical.com/ ) > > Colin > >
On 4/16/21 11:09 AM, Alex Hung wrote: > There are signifcant changes in revision 2 which is not compatible with > revision 1. > > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- > src/acpi/pmtt/pmtt.c | 146 ++++++++++++++++++------------------ > src/lib/include/fwts_acpi.h | 21 ++---- > 2 files changed, 78 insertions(+), 89 deletions(-) > > diff --git a/src/acpi/pmtt/pmtt.c b/src/acpi/pmtt/pmtt.c > index 27ec58b8..7cd0952b 100644 > --- a/src/acpi/pmtt/pmtt.c > +++ b/src/acpi/pmtt/pmtt.c > @@ -23,10 +23,15 @@ > #include <inttypes.h> > #include <stdbool.h> > > +static void pmtt_memory_device(fwts_framework *fw, fwts_acpi_table_pmtt_header *entry, uint32_t offset, bool *passed); > + > static fwts_acpi_table_info *table; > acpi_table_init(PMTT, &table) > > -static void pmtt_subtable_header_test(fwts_framework *fw, fwts_acpi_table_pmtt_header *entry, bool *passed) > +static void pmtt_subtable_header_test( > + fwts_framework *fw, > + fwts_acpi_table_pmtt_header *entry, > + bool *passed) > { > fwts_log_info_verbatim(fw, " PMTT Subtable:"); > fwts_log_info_simp_int(fw, " Type: ", entry->type); > @@ -48,16 +53,14 @@ static void pmtt_subtable_header_test(fwts_framework *fw, fwts_acpi_table_pmtt_h > fwts_acpi_reserved_zero_check("PMTT", "Reserved2", entry->reserved2, passed); > } > > -static void pmtt_physical_component_test(fwts_framework *fw, fwts_acpi_table_pmtt_physical_component *entry, bool *passed) > +static void pmtt_physical_component_test( > + fwts_framework *fw, > + fwts_acpi_table_pmtt_physical_component *entry, > + bool *passed) > { > pmtt_subtable_header_test(fw, &entry->header, passed); > - fwts_log_info_simp_int(fw, " Physical Component Identifier: ", entry->component_id); > - fwts_log_info_simp_int(fw, " Reserved: ", entry->reserved); > - fwts_log_info_simp_int(fw, " Size of DIMM: ", entry->memory_size); > fwts_log_info_simp_int(fw, " SMBIOS Handle: ", entry->bios_handle); > > - fwts_acpi_reserved_zero_check("PMTT", "Reserved", entry->reserved, passed); > - > if ((entry->bios_handle & 0xFFFF0000) != 0 && entry->bios_handle != 0xFFFFFFFF) { > *passed = false; > fwts_failed(fw, LOG_LEVEL_HIGH, > @@ -67,43 +70,30 @@ static void pmtt_physical_component_test(fwts_framework *fw, fwts_acpi_table_pmt > } > } > > -static void pmtt_controller_test(fwts_framework *fw, fwts_acpi_table_pmtt_controller *entry, bool *passed) > +static void pmtt_controller_test( > + fwts_framework *fw, > + fwts_acpi_table_pmtt_controller *entry, > + uint32_t entry_offset, > + bool *passed) > { > fwts_acpi_table_pmtt_header *header; > uint32_t offset = 0; > - size_t i; > > pmtt_subtable_header_test(fw, &entry->header, passed); > - fwts_log_info_simp_int(fw, " Read Latency: ", entry->read_latency); > - fwts_log_info_simp_int(fw, " Write latency: ", entry->write_latency); > - fwts_log_info_simp_int(fw, " Read Bandwidth: ", entry->read_bandwidth); > - fwts_log_info_simp_int(fw, " Write Bandwidth: ", entry->write_bandwidth); > - fwts_log_info_simp_int(fw, " Optimal Access Unit: ", entry->access_width); > - fwts_log_info_simp_int(fw, " Optimal Access Alignment: ", entry->alignment); > + fwts_log_info_simp_int(fw, " Memory Controller ID ", entry->memory_controller_id); > fwts_log_info_simp_int(fw, " Reserved: ", entry->reserved); > - fwts_log_info_simp_int(fw, " Number of Proximity Domains: ", entry->domain_count); > > fwts_acpi_reserved_zero_check("PMTT", "Reserved", entry->reserved, passed); > > offset = sizeof(fwts_acpi_table_pmtt_controller); > - if (entry->header.length < offset + sizeof(fwts_acpi_table_pmtt_domain) * entry->domain_count) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "PMTTOutOfBound", > - "PMTT's length is too small to contain all fields"); > - return; > - } > - > - fwts_acpi_table_pmtt_domain *domain = (fwts_acpi_table_pmtt_domain *)(((char *) entry) + offset); > - for (i = 0; i < entry->domain_count; i++) { > - fwts_log_info_simp_int(fw, " Proximity Domain: ", domain->proximity_domain); > - domain++; > - /* TODO cross check proximity domain with SRAT table*/ > - } > - > - offset += sizeof(fwts_acpi_table_pmtt_domain) * entry->domain_count; > header = (fwts_acpi_table_pmtt_header *) (((char *) entry) + offset); > while (offset < entry->header.length) { > + /* stop if sub-structure is outside the table */ > + if (fwts_acpi_structure_range_check(fw, "PMTT", table->length, entry_offset + offset)) { > + *passed = false; > + break; > + } > + > if (header->length == 0) { > fwts_failed(fw, LOG_LEVEL_CRITICAL, > "PMTTBadSubtableLength", > @@ -111,22 +101,18 @@ static void pmtt_controller_test(fwts_framework *fw, fwts_acpi_table_pmtt_contro > break; > } > > - if (header->type == FWTS_ACPI_PMTT_TYPE_DIMM) { > - pmtt_physical_component_test(fw, (fwts_acpi_table_pmtt_physical_component *) header, passed); > - } else { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "PMTTBadSubtableType", > - "PMTT Controller must have subtable with Type 2, got " > - "0x%4.4" PRIx16 " instead", header->type); > - } > + pmtt_memory_device(fw, header, entry_offset + offset, passed); > > offset += header->length; > header = (fwts_acpi_table_pmtt_header *)(((char *) entry) + offset); > } > } > > -static void pmtt_socket_test(fwts_framework *fw, fwts_acpi_table_pmtt_socket *entry, bool *passed) > +static void pmtt_socket_test( > + fwts_framework *fw, > + fwts_acpi_table_pmtt_socket *entry, > + uint32_t entry_offset, > + bool *passed) > { > fwts_acpi_table_pmtt_header *header; > uint32_t offset; > @@ -140,6 +126,12 @@ static void pmtt_socket_test(fwts_framework *fw, fwts_acpi_table_pmtt_socket *en > offset = sizeof(fwts_acpi_table_pmtt_socket); > header = (fwts_acpi_table_pmtt_header *) (((char *) entry) + offset); > while (offset < entry->header.length) { > + /* stop if sub-structure is outside the table */ > + if (fwts_acpi_structure_range_check(fw, "PMTT", table->length, entry_offset + offset)) { > + *passed = false; > + break; > + } > + > if (header->length == 0) { > fwts_failed(fw, LOG_LEVEL_CRITICAL, > "PMTTBadSubtableLength", > @@ -147,21 +139,42 @@ static void pmtt_socket_test(fwts_framework *fw, fwts_acpi_table_pmtt_socket *en > break; > } > > - if (header->type == FWTS_ACPI_PMTT_TYPE_CONTROLLER) { > - pmtt_controller_test(fw, (fwts_acpi_table_pmtt_controller *) header, passed); > - } else { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "PMTTBadSubtableType", > - "PMTT Socket must have subtable with Type 1, got " > - "0x%4.4" PRIx16 " instead", header->type); > - } > + pmtt_memory_device(fw, header, entry_offset + offset, passed); > > offset += header->length; > header = (fwts_acpi_table_pmtt_header *)(((char *) entry) + offset); > } > } > > +static void pmtt_memory_device( > + fwts_framework *fw, > + fwts_acpi_table_pmtt_header *entry, > + uint32_t offset, > + bool *passed) > +{ > + switch(entry->type) { > + case FWTS_ACPI_PMTT_TYPE_SOCKET: > + pmtt_socket_test(fw, (fwts_acpi_table_pmtt_socket *) entry, offset, passed); > + break; > + case FWTS_ACPI_PMTT_TYPE_CONTROLLER: > + pmtt_controller_test(fw, (fwts_acpi_table_pmtt_controller *) entry, offset, passed); > + break; > + case FWTS_ACPI_PMTT_TYPE_DIMM: > + pmtt_physical_component_test(fw, (fwts_acpi_table_pmtt_physical_component *) entry, passed); > + break; > + case FWTS_ACPI_PMTT_TYPE_VENDOR_SPECIFIC: > + /* no tests for vendor-specific type */ > + break; > + default: > + *passed = false; > + fwts_failed(fw, LOG_LEVEL_HIGH, > + "PMTTBadSubtableType", > + "PMTT must have subtable with Type 1..2 or 0xFF, got " > + "0x%4.4" PRIx16 " instead", entry->type); > + break; > + } > +} > + > static int pmtt_test1(fwts_framework *fw) > { > fwts_acpi_table_pmtt *pmtt = (fwts_acpi_table_pmtt*) table->data; > @@ -169,10 +182,14 @@ static int pmtt_test1(fwts_framework *fw) > uint32_t offset; > bool passed = true; > > - fwts_log_info_verbatim(fw, "PMTT Table:"); > - fwts_log_info_simp_int(fw, " Reserved: ", pmtt->reserved); > + if (pmtt->header.revision < 2) { > + fwts_failed(fw, LOG_LEVEL_CRITICAL, "PMTTDeprecatedRevision", > + "PMTT Revision 1 has been deprecated in ACPI 6.4"); > + return FWTS_OK; > + } > > - fwts_acpi_reserved_zero_check("PMTT", "Reserved", pmtt->reserved, &passed); > + fwts_log_info_verbatim(fw, "PMTT Table:"); > + fwts_log_info_simp_int(fw, " Number of Memory Devices: ", pmtt->num_devices); > > entry = (fwts_acpi_table_pmtt_header *) (table->data + sizeof(fwts_acpi_table_pmtt)); > offset = sizeof(fwts_acpi_table_pmtt); > @@ -186,29 +203,12 @@ static int pmtt_test1(fwts_framework *fw) > break; > } > > - switch(entry->type) { > - case FWTS_ACPI_PMTT_TYPE_SOCKET: > - pmtt_socket_test(fw, (fwts_acpi_table_pmtt_socket *) entry, &passed); > - break; > - case FWTS_ACPI_PMTT_TYPE_CONTROLLER: > - pmtt_controller_test(fw, (fwts_acpi_table_pmtt_controller *) entry, &passed); > - break; > - case FWTS_ACPI_PMTT_TYPE_DIMM: > - pmtt_physical_component_test(fw, (fwts_acpi_table_pmtt_physical_component *) entry, &passed); > - break; > - default: > - passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "PMTTBadSubtableType", > - "PMTT must have subtable with Type 1..2, got " > - "0x%4.4" PRIx16 " instead", entry->type); > - break; > - } > + pmtt_memory_device(fw, entry, offset, &passed); > > offset += entry->length; > entry = (fwts_acpi_table_pmtt_header *) (table->data + offset); > + fwts_log_nl(fw); > } > - fwts_log_nl(fw); > > if (passed) > fwts_passed(fw, "No issues found in PMTT table."); > diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h > index 2a6c76ea..c1345803 100644 > --- a/src/lib/include/fwts_acpi.h > +++ b/src/lib/include/fwts_acpi.h > @@ -1134,7 +1134,7 @@ typedef struct { > */ > typedef struct { > fwts_acpi_table_header header; > - uint32_t reserved; > + uint32_t num_devices; > } __attribute__ ((packed)) fwts_acpi_table_pmtt; > > typedef struct { > @@ -1143,13 +1143,15 @@ typedef struct { > uint16_t length; > uint16_t flags; > uint16_t reserved2; > + uint32_t num_devices; > } __attribute__ ((packed)) fwts_acpi_table_pmtt_header; > > typedef enum { > FWTS_ACPI_PMTT_TYPE_SOCKET = 0, > FWTS_ACPI_PMTT_TYPE_CONTROLLER = 1, > FWTS_ACPI_PMTT_TYPE_DIMM = 2, > - FWTS_ACPI_PMTT_TYPE_RESERVED = 3 /* 0x03-0xFF are reserved */ > + FWTS_ACPI_PMTT_TYPE_RESERVED = 3, /* 0x03-0xFE are reserved */ > + FWTS_ACPI_PMTT_TYPE_VENDOR_SPECIFIC = 0xFF > } fwts_acpi_pmtt_type; > > typedef struct { > @@ -1160,25 +1162,12 @@ typedef struct { > > typedef struct { > fwts_acpi_table_pmtt_header header; > - uint32_t read_latency; > - uint32_t write_latency; > - uint32_t read_bandwidth; > - uint32_t write_bandwidth; > - uint16_t access_width; > - uint16_t alignment; > + uint16_t memory_controller_id; > uint16_t reserved; > - uint16_t domain_count; > } __attribute__ ((packed)) fwts_acpi_table_pmtt_controller; > > -typedef struct { > - uint32_t proximity_domain; > -} __attribute__ ((packed)) fwts_acpi_table_pmtt_domain; > - > typedef struct { > fwts_acpi_table_pmtt_header header; > - uint16_t component_id; > - uint16_t reserved; > - uint32_t memory_size; > uint32_t bios_handle; > } __attribute__ ((packed)) fwts_acpi_table_pmtt_physical_component; > > Acked-by: Ivan Hu <ivan.hu@canonical.com>
On 16/04/2021 04:09, Alex Hung wrote: > There are signifcant changes in revision 2 which is not compatible with > revision 1. > > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- > src/acpi/pmtt/pmtt.c | 146 ++++++++++++++++++------------------ > src/lib/include/fwts_acpi.h | 21 ++---- > 2 files changed, 78 insertions(+), 89 deletions(-) > > diff --git a/src/acpi/pmtt/pmtt.c b/src/acpi/pmtt/pmtt.c > index 27ec58b8..7cd0952b 100644 > --- a/src/acpi/pmtt/pmtt.c > +++ b/src/acpi/pmtt/pmtt.c > @@ -23,10 +23,15 @@ > #include <inttypes.h> > #include <stdbool.h> > > +static void pmtt_memory_device(fwts_framework *fw, fwts_acpi_table_pmtt_header *entry, uint32_t offset, bool *passed); > + > static fwts_acpi_table_info *table; > acpi_table_init(PMTT, &table) > > -static void pmtt_subtable_header_test(fwts_framework *fw, fwts_acpi_table_pmtt_header *entry, bool *passed) > +static void pmtt_subtable_header_test( > + fwts_framework *fw, > + fwts_acpi_table_pmtt_header *entry, > + bool *passed) > { > fwts_log_info_verbatim(fw, " PMTT Subtable:"); > fwts_log_info_simp_int(fw, " Type: ", entry->type); > @@ -48,16 +53,14 @@ static void pmtt_subtable_header_test(fwts_framework *fw, fwts_acpi_table_pmtt_h > fwts_acpi_reserved_zero_check("PMTT", "Reserved2", entry->reserved2, passed); > } > > -static void pmtt_physical_component_test(fwts_framework *fw, fwts_acpi_table_pmtt_physical_component *entry, bool *passed) > +static void pmtt_physical_component_test( > + fwts_framework *fw, > + fwts_acpi_table_pmtt_physical_component *entry, > + bool *passed) > { > pmtt_subtable_header_test(fw, &entry->header, passed); > - fwts_log_info_simp_int(fw, " Physical Component Identifier: ", entry->component_id); > - fwts_log_info_simp_int(fw, " Reserved: ", entry->reserved); > - fwts_log_info_simp_int(fw, " Size of DIMM: ", entry->memory_size); > fwts_log_info_simp_int(fw, " SMBIOS Handle: ", entry->bios_handle); > > - fwts_acpi_reserved_zero_check("PMTT", "Reserved", entry->reserved, passed); > - > if ((entry->bios_handle & 0xFFFF0000) != 0 && entry->bios_handle != 0xFFFFFFFF) { > *passed = false; > fwts_failed(fw, LOG_LEVEL_HIGH, > @@ -67,43 +70,30 @@ static void pmtt_physical_component_test(fwts_framework *fw, fwts_acpi_table_pmt > } > } > > -static void pmtt_controller_test(fwts_framework *fw, fwts_acpi_table_pmtt_controller *entry, bool *passed) > +static void pmtt_controller_test( > + fwts_framework *fw, > + fwts_acpi_table_pmtt_controller *entry, > + uint32_t entry_offset, > + bool *passed) > { > fwts_acpi_table_pmtt_header *header; > uint32_t offset = 0; > - size_t i; > > pmtt_subtable_header_test(fw, &entry->header, passed); > - fwts_log_info_simp_int(fw, " Read Latency: ", entry->read_latency); > - fwts_log_info_simp_int(fw, " Write latency: ", entry->write_latency); > - fwts_log_info_simp_int(fw, " Read Bandwidth: ", entry->read_bandwidth); > - fwts_log_info_simp_int(fw, " Write Bandwidth: ", entry->write_bandwidth); > - fwts_log_info_simp_int(fw, " Optimal Access Unit: ", entry->access_width); > - fwts_log_info_simp_int(fw, " Optimal Access Alignment: ", entry->alignment); > + fwts_log_info_simp_int(fw, " Memory Controller ID ", entry->memory_controller_id); > fwts_log_info_simp_int(fw, " Reserved: ", entry->reserved); > - fwts_log_info_simp_int(fw, " Number of Proximity Domains: ", entry->domain_count); > > fwts_acpi_reserved_zero_check("PMTT", "Reserved", entry->reserved, passed); > > offset = sizeof(fwts_acpi_table_pmtt_controller); > - if (entry->header.length < offset + sizeof(fwts_acpi_table_pmtt_domain) * entry->domain_count) { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "PMTTOutOfBound", > - "PMTT's length is too small to contain all fields"); > - return; > - } > - > - fwts_acpi_table_pmtt_domain *domain = (fwts_acpi_table_pmtt_domain *)(((char *) entry) + offset); > - for (i = 0; i < entry->domain_count; i++) { > - fwts_log_info_simp_int(fw, " Proximity Domain: ", domain->proximity_domain); > - domain++; > - /* TODO cross check proximity domain with SRAT table*/ > - } > - > - offset += sizeof(fwts_acpi_table_pmtt_domain) * entry->domain_count; > header = (fwts_acpi_table_pmtt_header *) (((char *) entry) + offset); > while (offset < entry->header.length) { > + /* stop if sub-structure is outside the table */ > + if (fwts_acpi_structure_range_check(fw, "PMTT", table->length, entry_offset + offset)) { > + *passed = false; > + break; > + } > + > if (header->length == 0) { > fwts_failed(fw, LOG_LEVEL_CRITICAL, > "PMTTBadSubtableLength", > @@ -111,22 +101,18 @@ static void pmtt_controller_test(fwts_framework *fw, fwts_acpi_table_pmtt_contro > break; > } > > - if (header->type == FWTS_ACPI_PMTT_TYPE_DIMM) { > - pmtt_physical_component_test(fw, (fwts_acpi_table_pmtt_physical_component *) header, passed); > - } else { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "PMTTBadSubtableType", > - "PMTT Controller must have subtable with Type 2, got " > - "0x%4.4" PRIx16 " instead", header->type); > - } > + pmtt_memory_device(fw, header, entry_offset + offset, passed); > > offset += header->length; > header = (fwts_acpi_table_pmtt_header *)(((char *) entry) + offset); > } > } > > -static void pmtt_socket_test(fwts_framework *fw, fwts_acpi_table_pmtt_socket *entry, bool *passed) > +static void pmtt_socket_test( > + fwts_framework *fw, > + fwts_acpi_table_pmtt_socket *entry, > + uint32_t entry_offset, > + bool *passed) > { > fwts_acpi_table_pmtt_header *header; > uint32_t offset; > @@ -140,6 +126,12 @@ static void pmtt_socket_test(fwts_framework *fw, fwts_acpi_table_pmtt_socket *en > offset = sizeof(fwts_acpi_table_pmtt_socket); > header = (fwts_acpi_table_pmtt_header *) (((char *) entry) + offset); > while (offset < entry->header.length) { > + /* stop if sub-structure is outside the table */ > + if (fwts_acpi_structure_range_check(fw, "PMTT", table->length, entry_offset + offset)) { > + *passed = false; > + break; > + } > + > if (header->length == 0) { > fwts_failed(fw, LOG_LEVEL_CRITICAL, > "PMTTBadSubtableLength", > @@ -147,21 +139,42 @@ static void pmtt_socket_test(fwts_framework *fw, fwts_acpi_table_pmtt_socket *en > break; > } > > - if (header->type == FWTS_ACPI_PMTT_TYPE_CONTROLLER) { > - pmtt_controller_test(fw, (fwts_acpi_table_pmtt_controller *) header, passed); > - } else { > - *passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "PMTTBadSubtableType", > - "PMTT Socket must have subtable with Type 1, got " > - "0x%4.4" PRIx16 " instead", header->type); > - } > + pmtt_memory_device(fw, header, entry_offset + offset, passed); > > offset += header->length; > header = (fwts_acpi_table_pmtt_header *)(((char *) entry) + offset); > } > } > > +static void pmtt_memory_device( > + fwts_framework *fw, > + fwts_acpi_table_pmtt_header *entry, > + uint32_t offset, > + bool *passed) > +{ > + switch(entry->type) { > + case FWTS_ACPI_PMTT_TYPE_SOCKET: > + pmtt_socket_test(fw, (fwts_acpi_table_pmtt_socket *) entry, offset, passed); > + break; > + case FWTS_ACPI_PMTT_TYPE_CONTROLLER: > + pmtt_controller_test(fw, (fwts_acpi_table_pmtt_controller *) entry, offset, passed); > + break; > + case FWTS_ACPI_PMTT_TYPE_DIMM: > + pmtt_physical_component_test(fw, (fwts_acpi_table_pmtt_physical_component *) entry, passed); > + break; > + case FWTS_ACPI_PMTT_TYPE_VENDOR_SPECIFIC: > + /* no tests for vendor-specific type */ > + break; > + default: > + *passed = false; > + fwts_failed(fw, LOG_LEVEL_HIGH, > + "PMTTBadSubtableType", > + "PMTT must have subtable with Type 1..2 or 0xFF, got " > + "0x%4.4" PRIx16 " instead", entry->type); > + break; > + } > +} > + > static int pmtt_test1(fwts_framework *fw) > { > fwts_acpi_table_pmtt *pmtt = (fwts_acpi_table_pmtt*) table->data; > @@ -169,10 +182,14 @@ static int pmtt_test1(fwts_framework *fw) > uint32_t offset; > bool passed = true; > > - fwts_log_info_verbatim(fw, "PMTT Table:"); > - fwts_log_info_simp_int(fw, " Reserved: ", pmtt->reserved); > + if (pmtt->header.revision < 2) { > + fwts_failed(fw, LOG_LEVEL_CRITICAL, "PMTTDeprecatedRevision", > + "PMTT Revision 1 has been deprecated in ACPI 6.4"); > + return FWTS_OK; > + } > > - fwts_acpi_reserved_zero_check("PMTT", "Reserved", pmtt->reserved, &passed); > + fwts_log_info_verbatim(fw, "PMTT Table:"); > + fwts_log_info_simp_int(fw, " Number of Memory Devices: ", pmtt->num_devices); > > entry = (fwts_acpi_table_pmtt_header *) (table->data + sizeof(fwts_acpi_table_pmtt)); > offset = sizeof(fwts_acpi_table_pmtt); > @@ -186,29 +203,12 @@ static int pmtt_test1(fwts_framework *fw) > break; > } > > - switch(entry->type) { > - case FWTS_ACPI_PMTT_TYPE_SOCKET: > - pmtt_socket_test(fw, (fwts_acpi_table_pmtt_socket *) entry, &passed); > - break; > - case FWTS_ACPI_PMTT_TYPE_CONTROLLER: > - pmtt_controller_test(fw, (fwts_acpi_table_pmtt_controller *) entry, &passed); > - break; > - case FWTS_ACPI_PMTT_TYPE_DIMM: > - pmtt_physical_component_test(fw, (fwts_acpi_table_pmtt_physical_component *) entry, &passed); > - break; > - default: > - passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "PMTTBadSubtableType", > - "PMTT must have subtable with Type 1..2, got " > - "0x%4.4" PRIx16 " instead", entry->type); > - break; > - } > + pmtt_memory_device(fw, entry, offset, &passed); > > offset += entry->length; > entry = (fwts_acpi_table_pmtt_header *) (table->data + offset); > + fwts_log_nl(fw); > } > - fwts_log_nl(fw); > > if (passed) > fwts_passed(fw, "No issues found in PMTT table."); > diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h > index 2a6c76ea..c1345803 100644 > --- a/src/lib/include/fwts_acpi.h > +++ b/src/lib/include/fwts_acpi.h > @@ -1134,7 +1134,7 @@ typedef struct { > */ > typedef struct { > fwts_acpi_table_header header; > - uint32_t reserved; > + uint32_t num_devices; > } __attribute__ ((packed)) fwts_acpi_table_pmtt; > > typedef struct { > @@ -1143,13 +1143,15 @@ typedef struct { > uint16_t length; > uint16_t flags; > uint16_t reserved2; > + uint32_t num_devices; > } __attribute__ ((packed)) fwts_acpi_table_pmtt_header; > > typedef enum { > FWTS_ACPI_PMTT_TYPE_SOCKET = 0, > FWTS_ACPI_PMTT_TYPE_CONTROLLER = 1, > FWTS_ACPI_PMTT_TYPE_DIMM = 2, > - FWTS_ACPI_PMTT_TYPE_RESERVED = 3 /* 0x03-0xFF are reserved */ > + FWTS_ACPI_PMTT_TYPE_RESERVED = 3, /* 0x03-0xFE are reserved */ > + FWTS_ACPI_PMTT_TYPE_VENDOR_SPECIFIC = 0xFF > } fwts_acpi_pmtt_type; > > typedef struct { > @@ -1160,25 +1162,12 @@ typedef struct { > > typedef struct { > fwts_acpi_table_pmtt_header header; > - uint32_t read_latency; > - uint32_t write_latency; > - uint32_t read_bandwidth; > - uint32_t write_bandwidth; > - uint16_t access_width; > - uint16_t alignment; > + uint16_t memory_controller_id; > uint16_t reserved; > - uint16_t domain_count; > } __attribute__ ((packed)) fwts_acpi_table_pmtt_controller; > > -typedef struct { > - uint32_t proximity_domain; > -} __attribute__ ((packed)) fwts_acpi_table_pmtt_domain; > - > typedef struct { > fwts_acpi_table_pmtt_header header; > - uint16_t component_id; > - uint16_t reserved; > - uint32_t memory_size; > uint32_t bios_handle; > } __attribute__ ((packed)) fwts_acpi_table_pmtt_physical_component; > > Acked-by: Colin Ian King <colin.king@canonical.com>
diff --git a/src/acpi/pmtt/pmtt.c b/src/acpi/pmtt/pmtt.c index 27ec58b8..7cd0952b 100644 --- a/src/acpi/pmtt/pmtt.c +++ b/src/acpi/pmtt/pmtt.c @@ -23,10 +23,15 @@ #include <inttypes.h> #include <stdbool.h> +static void pmtt_memory_device(fwts_framework *fw, fwts_acpi_table_pmtt_header *entry, uint32_t offset, bool *passed); + static fwts_acpi_table_info *table; acpi_table_init(PMTT, &table) -static void pmtt_subtable_header_test(fwts_framework *fw, fwts_acpi_table_pmtt_header *entry, bool *passed) +static void pmtt_subtable_header_test( + fwts_framework *fw, + fwts_acpi_table_pmtt_header *entry, + bool *passed) { fwts_log_info_verbatim(fw, " PMTT Subtable:"); fwts_log_info_simp_int(fw, " Type: ", entry->type); @@ -48,16 +53,14 @@ static void pmtt_subtable_header_test(fwts_framework *fw, fwts_acpi_table_pmtt_h fwts_acpi_reserved_zero_check("PMTT", "Reserved2", entry->reserved2, passed); } -static void pmtt_physical_component_test(fwts_framework *fw, fwts_acpi_table_pmtt_physical_component *entry, bool *passed) +static void pmtt_physical_component_test( + fwts_framework *fw, + fwts_acpi_table_pmtt_physical_component *entry, + bool *passed) { pmtt_subtable_header_test(fw, &entry->header, passed); - fwts_log_info_simp_int(fw, " Physical Component Identifier: ", entry->component_id); - fwts_log_info_simp_int(fw, " Reserved: ", entry->reserved); - fwts_log_info_simp_int(fw, " Size of DIMM: ", entry->memory_size); fwts_log_info_simp_int(fw, " SMBIOS Handle: ", entry->bios_handle); - fwts_acpi_reserved_zero_check("PMTT", "Reserved", entry->reserved, passed); - if ((entry->bios_handle & 0xFFFF0000) != 0 && entry->bios_handle != 0xFFFFFFFF) { *passed = false; fwts_failed(fw, LOG_LEVEL_HIGH, @@ -67,43 +70,30 @@ static void pmtt_physical_component_test(fwts_framework *fw, fwts_acpi_table_pmt } } -static void pmtt_controller_test(fwts_framework *fw, fwts_acpi_table_pmtt_controller *entry, bool *passed) +static void pmtt_controller_test( + fwts_framework *fw, + fwts_acpi_table_pmtt_controller *entry, + uint32_t entry_offset, + bool *passed) { fwts_acpi_table_pmtt_header *header; uint32_t offset = 0; - size_t i; pmtt_subtable_header_test(fw, &entry->header, passed); - fwts_log_info_simp_int(fw, " Read Latency: ", entry->read_latency); - fwts_log_info_simp_int(fw, " Write latency: ", entry->write_latency); - fwts_log_info_simp_int(fw, " Read Bandwidth: ", entry->read_bandwidth); - fwts_log_info_simp_int(fw, " Write Bandwidth: ", entry->write_bandwidth); - fwts_log_info_simp_int(fw, " Optimal Access Unit: ", entry->access_width); - fwts_log_info_simp_int(fw, " Optimal Access Alignment: ", entry->alignment); + fwts_log_info_simp_int(fw, " Memory Controller ID ", entry->memory_controller_id); fwts_log_info_simp_int(fw, " Reserved: ", entry->reserved); - fwts_log_info_simp_int(fw, " Number of Proximity Domains: ", entry->domain_count); fwts_acpi_reserved_zero_check("PMTT", "Reserved", entry->reserved, passed); offset = sizeof(fwts_acpi_table_pmtt_controller); - if (entry->header.length < offset + sizeof(fwts_acpi_table_pmtt_domain) * entry->domain_count) { - *passed = false; - fwts_failed(fw, LOG_LEVEL_HIGH, - "PMTTOutOfBound", - "PMTT's length is too small to contain all fields"); - return; - } - - fwts_acpi_table_pmtt_domain *domain = (fwts_acpi_table_pmtt_domain *)(((char *) entry) + offset); - for (i = 0; i < entry->domain_count; i++) { - fwts_log_info_simp_int(fw, " Proximity Domain: ", domain->proximity_domain); - domain++; - /* TODO cross check proximity domain with SRAT table*/ - } - - offset += sizeof(fwts_acpi_table_pmtt_domain) * entry->domain_count; header = (fwts_acpi_table_pmtt_header *) (((char *) entry) + offset); while (offset < entry->header.length) { + /* stop if sub-structure is outside the table */ + if (fwts_acpi_structure_range_check(fw, "PMTT", table->length, entry_offset + offset)) { + *passed = false; + break; + } + if (header->length == 0) { fwts_failed(fw, LOG_LEVEL_CRITICAL, "PMTTBadSubtableLength", @@ -111,22 +101,18 @@ static void pmtt_controller_test(fwts_framework *fw, fwts_acpi_table_pmtt_contro break; } - if (header->type == FWTS_ACPI_PMTT_TYPE_DIMM) { - pmtt_physical_component_test(fw, (fwts_acpi_table_pmtt_physical_component *) header, passed); - } else { - *passed = false; - fwts_failed(fw, LOG_LEVEL_HIGH, - "PMTTBadSubtableType", - "PMTT Controller must have subtable with Type 2, got " - "0x%4.4" PRIx16 " instead", header->type); - } + pmtt_memory_device(fw, header, entry_offset + offset, passed); offset += header->length; header = (fwts_acpi_table_pmtt_header *)(((char *) entry) + offset); } } -static void pmtt_socket_test(fwts_framework *fw, fwts_acpi_table_pmtt_socket *entry, bool *passed) +static void pmtt_socket_test( + fwts_framework *fw, + fwts_acpi_table_pmtt_socket *entry, + uint32_t entry_offset, + bool *passed) { fwts_acpi_table_pmtt_header *header; uint32_t offset; @@ -140,6 +126,12 @@ static void pmtt_socket_test(fwts_framework *fw, fwts_acpi_table_pmtt_socket *en offset = sizeof(fwts_acpi_table_pmtt_socket); header = (fwts_acpi_table_pmtt_header *) (((char *) entry) + offset); while (offset < entry->header.length) { + /* stop if sub-structure is outside the table */ + if (fwts_acpi_structure_range_check(fw, "PMTT", table->length, entry_offset + offset)) { + *passed = false; + break; + } + if (header->length == 0) { fwts_failed(fw, LOG_LEVEL_CRITICAL, "PMTTBadSubtableLength", @@ -147,21 +139,42 @@ static void pmtt_socket_test(fwts_framework *fw, fwts_acpi_table_pmtt_socket *en break; } - if (header->type == FWTS_ACPI_PMTT_TYPE_CONTROLLER) { - pmtt_controller_test(fw, (fwts_acpi_table_pmtt_controller *) header, passed); - } else { - *passed = false; - fwts_failed(fw, LOG_LEVEL_HIGH, - "PMTTBadSubtableType", - "PMTT Socket must have subtable with Type 1, got " - "0x%4.4" PRIx16 " instead", header->type); - } + pmtt_memory_device(fw, header, entry_offset + offset, passed); offset += header->length; header = (fwts_acpi_table_pmtt_header *)(((char *) entry) + offset); } } +static void pmtt_memory_device( + fwts_framework *fw, + fwts_acpi_table_pmtt_header *entry, + uint32_t offset, + bool *passed) +{ + switch(entry->type) { + case FWTS_ACPI_PMTT_TYPE_SOCKET: + pmtt_socket_test(fw, (fwts_acpi_table_pmtt_socket *) entry, offset, passed); + break; + case FWTS_ACPI_PMTT_TYPE_CONTROLLER: + pmtt_controller_test(fw, (fwts_acpi_table_pmtt_controller *) entry, offset, passed); + break; + case FWTS_ACPI_PMTT_TYPE_DIMM: + pmtt_physical_component_test(fw, (fwts_acpi_table_pmtt_physical_component *) entry, passed); + break; + case FWTS_ACPI_PMTT_TYPE_VENDOR_SPECIFIC: + /* no tests for vendor-specific type */ + break; + default: + *passed = false; + fwts_failed(fw, LOG_LEVEL_HIGH, + "PMTTBadSubtableType", + "PMTT must have subtable with Type 1..2 or 0xFF, got " + "0x%4.4" PRIx16 " instead", entry->type); + break; + } +} + static int pmtt_test1(fwts_framework *fw) { fwts_acpi_table_pmtt *pmtt = (fwts_acpi_table_pmtt*) table->data; @@ -169,10 +182,14 @@ static int pmtt_test1(fwts_framework *fw) uint32_t offset; bool passed = true; - fwts_log_info_verbatim(fw, "PMTT Table:"); - fwts_log_info_simp_int(fw, " Reserved: ", pmtt->reserved); + if (pmtt->header.revision < 2) { + fwts_failed(fw, LOG_LEVEL_CRITICAL, "PMTTDeprecatedRevision", + "PMTT Revision 1 has been deprecated in ACPI 6.4"); + return FWTS_OK; + } - fwts_acpi_reserved_zero_check("PMTT", "Reserved", pmtt->reserved, &passed); + fwts_log_info_verbatim(fw, "PMTT Table:"); + fwts_log_info_simp_int(fw, " Number of Memory Devices: ", pmtt->num_devices); entry = (fwts_acpi_table_pmtt_header *) (table->data + sizeof(fwts_acpi_table_pmtt)); offset = sizeof(fwts_acpi_table_pmtt); @@ -186,29 +203,12 @@ static int pmtt_test1(fwts_framework *fw) break; } - switch(entry->type) { - case FWTS_ACPI_PMTT_TYPE_SOCKET: - pmtt_socket_test(fw, (fwts_acpi_table_pmtt_socket *) entry, &passed); - break; - case FWTS_ACPI_PMTT_TYPE_CONTROLLER: - pmtt_controller_test(fw, (fwts_acpi_table_pmtt_controller *) entry, &passed); - break; - case FWTS_ACPI_PMTT_TYPE_DIMM: - pmtt_physical_component_test(fw, (fwts_acpi_table_pmtt_physical_component *) entry, &passed); - break; - default: - passed = false; - fwts_failed(fw, LOG_LEVEL_HIGH, - "PMTTBadSubtableType", - "PMTT must have subtable with Type 1..2, got " - "0x%4.4" PRIx16 " instead", entry->type); - break; - } + pmtt_memory_device(fw, entry, offset, &passed); offset += entry->length; entry = (fwts_acpi_table_pmtt_header *) (table->data + offset); + fwts_log_nl(fw); } - fwts_log_nl(fw); if (passed) fwts_passed(fw, "No issues found in PMTT table."); diff --git a/src/lib/include/fwts_acpi.h b/src/lib/include/fwts_acpi.h index 2a6c76ea..c1345803 100644 --- a/src/lib/include/fwts_acpi.h +++ b/src/lib/include/fwts_acpi.h @@ -1134,7 +1134,7 @@ typedef struct { */ typedef struct { fwts_acpi_table_header header; - uint32_t reserved; + uint32_t num_devices; } __attribute__ ((packed)) fwts_acpi_table_pmtt; typedef struct { @@ -1143,13 +1143,15 @@ typedef struct { uint16_t length; uint16_t flags; uint16_t reserved2; + uint32_t num_devices; } __attribute__ ((packed)) fwts_acpi_table_pmtt_header; typedef enum { FWTS_ACPI_PMTT_TYPE_SOCKET = 0, FWTS_ACPI_PMTT_TYPE_CONTROLLER = 1, FWTS_ACPI_PMTT_TYPE_DIMM = 2, - FWTS_ACPI_PMTT_TYPE_RESERVED = 3 /* 0x03-0xFF are reserved */ + FWTS_ACPI_PMTT_TYPE_RESERVED = 3, /* 0x03-0xFE are reserved */ + FWTS_ACPI_PMTT_TYPE_VENDOR_SPECIFIC = 0xFF } fwts_acpi_pmtt_type; typedef struct { @@ -1160,25 +1162,12 @@ typedef struct { typedef struct { fwts_acpi_table_pmtt_header header; - uint32_t read_latency; - uint32_t write_latency; - uint32_t read_bandwidth; - uint32_t write_bandwidth; - uint16_t access_width; - uint16_t alignment; + uint16_t memory_controller_id; uint16_t reserved; - uint16_t domain_count; } __attribute__ ((packed)) fwts_acpi_table_pmtt_controller; -typedef struct { - uint32_t proximity_domain; -} __attribute__ ((packed)) fwts_acpi_table_pmtt_domain; - typedef struct { fwts_acpi_table_pmtt_header header; - uint16_t component_id; - uint16_t reserved; - uint32_t memory_size; uint32_t bios_handle; } __attribute__ ((packed)) fwts_acpi_table_pmtt_physical_component;
There are signifcant changes in revision 2 which is not compatible with revision 1. Signed-off-by: Alex Hung <alex.hung@canonical.com> --- src/acpi/pmtt/pmtt.c | 146 ++++++++++++++++++------------------ src/lib/include/fwts_acpi.h | 21 ++---- 2 files changed, 78 insertions(+), 89 deletions(-)