Re: [RFC PATCH 10/36] ACPI / MPAM: Parse the MPAM table

[Ben Horgan <ben.horgan@arm.com>]

Hi James,

On 8/5/25 18:07, James Morse wrote:
> Hi Ben,
> 
> On 23/07/2025 17:39, Ben Horgan wrote:
>> On 7/16/25 18:07, Jonathan Cameron wrote:
>>> On Fri, 11 Jul 2025 18:36:22 +0000
>>> James Morse <james.morse@arm.com> wrote:
>>>
>>>> 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;
> --------------------%<--------------------
This seems fine as long as any later table revisions are guaranteed to 
be  backwards compatible.

> 
> 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

Thanks,

Ben