[prev in list] [next in list] [prev in thread] [next in thread]
List: fwts-devel
Subject: ACK: [PATCH 1/2] acpi: pmtt: update PMTT to revision 2 (mantis 1975)
From: ivanhu <ivan.hu () canonical ! com>
Date: 2021-04-26 6:23:36
Message-ID: 89ef32de-c060-d3e1-0ab9-74b676826e38 () canonical ! com
[Download RAW message or body]
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>
--
fwts-devel mailing list
fwts-devel@lists.ubuntu.com
Modify settings or unsubscribe at: \
https://lists.ubuntu.com/mailman/listinfo/fwts-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic