Hi Ben,

On 23/07/2025 17:39, Ben Horgan wrote:

On 7/16/25 18:07, Jonathan Cameron wrote:

quoted

On Fri, 11 Jul 2025 18:36:22 +0000
James Morse [off-list ref] wrote:

quoted

Add code to parse the arm64 specific MPAM table, looking up the cache
level from the PPTT and feeding the end result into the MPAM driver.

Throw in a link to the spec perhaps?  Particularly useful to know which
version this was written against when reviewing it.

As I comment below this code checks the table revision is 1 and so we can assume it was
written against version 2 of the spec. As of Monday, there is a new version hot off the
press,
https://developer.arm.com/documentation/den0065/3-0bet/?lang=en which introduces an "MMIO
size" field to allow for disabled nodes. This should be considered here to avoid
advertising msc that aren't present.

Sure. Bit of an unfortunate race with the spec people there!

Added as:
--------------------%<--------------------

diff --git a/drivers/acpi/arm64/mpam.c b/drivers/acpi/arm64/mpam.c
index 9ff5a6df9f1b..d8c6224a76f8 100644
--- a/drivers/acpi/arm64/mpam.c
+++ b/drivers/acpi/arm64/mpam.c

@@ -202,6 +202,9 @@ static int __init _parse_table(struct acpi_table_header *table)
                if (tbl_msc->reserved || tbl_msc->reserved1 || tbl_msc->reserved2)
                        continue;

+               if (!tbl_msc->mmio_size)
+                       continue;
+
                if (decode_interface_type(tbl_msc, &iface))
                        continue;

@@ -290,7 +293,7 @@ static struct acpi_table_header *get_table(void)
        if (ACPI_FAILURE(status))
                return NULL;

-       if (table->revision != 1)
+       if (table->revision < 1)
                return NULL;

        return table;

@@ -321,6 +324,9 @@ static int _count_msc(struct acpi_table_header *table)
        table_end = (char *)table + table->length;

        while (table_offset < table_end) {
+               if (!tbl_msc->mmio_size)
+                       continue;
+
                if (tbl_msc->length < sizeof(*tbl_msc))
                        return -EINVAL;
--------------------%<--------------------

Amusingly, PCC also defines mmio_size==0 as disabled, so _count_msc() doesn't need to know
what kind of thing this is. In principle they could change this as its beta, but a zero
sized MSC should probably be treated as an error anyway.


Thanks,

James