[PATCH 1/2] acpi: pmtt: update PMTT to revision 2 (mantis 1975)
Alex Hung
alex.hung at canonical.com
Fri Apr 16 07:40:25 UTC 2021
On Fri, Apr 16, 2021 at 1:35 AM Colin Ian King <colin.king at 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 at 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
>
>
--
Cheers,
Alex Hung
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/fwts-devel/attachments/20210416/3ef505df/attachment-0001.html>
More information about the fwts-devel
mailing list