Re: [PATCH v4 08/10] arm_mpam: add MPAM-Fb MSC firmware access support

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 23 Jul 2026 17:54:52 +0200
Andre Przywara <andre.przywara@arm.com> wrote:

> The Arm MPAM Firmware-backed (Fb) Profile document[1] describes an
> alternative way of accessing the "Memory System Components" (MSC) in an
> MPAM enabled system.
> 
> Normally the MSCs are MMIO mapped, but in some implementations this
> might not be possible (MSC located outside of the local socket, MSC
> mapped secure-only) or desirable (direct MMIO access too slow or needs
> to be mediated through a control processor). MPAM-fb standardises a
> protocol to abstract MSC accesses, building on the SCMI protocol.
> 
> Add functions that do an MSC read or write access by redirecting the
> request through a firmware interface. For now this done via an ACPI
> PCC shared memory and mailbox combination.
> 
> Since the protocol used is only a small subset of the full SCMI spec,
> and the SCMI protocol has no full ACPI support anyway, open-code the
> (simple) SCMI message generation, for just the fields we need.
> 
> [1] https://developer.arm.com/documentation/den0144/latest
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Just a couple of suggestions that you can ignore if you like

Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

> diff --git a/drivers/resctrl/mpam_fb.c b/drivers/resctrl/mpam_fb.c
> new file mode 100644
> index 000000000000..ec4bf0dd9182
> --- /dev/null
> +++ b/drivers/resctrl/mpam_fb.c
> @@ -0,0 +1,197 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2024 Arm Ltd.

Arm has either been working on this for rather a long time or that's a cut
and paste.  Even if 2024 is a good start given changes during revisions
seems reasonable to include 2026 as well.

> +struct mpam_fb_read_payload {
> +	u32 msc_id;
> +	u32 flags;
> +	u32 reg_offset;
> +} __packed;
> +
> +struct mpam_fb_write_payload {

Given these are only used in the specific functions that build the
messages you could move the structure definitions to be local to those
functions.

Alternatively you could move all these to the header and get the advantage
of using a union to get the maximum message size rather than a number
currently picked out of the air.

> +	u32 msc_id;
> +	u32 flags;
> +	u32 reg_offset;
> +	u32 value;
> +} __packed;



> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
> index b3a6ed9ed175..e4686ff0ba2e 100644
> --- a/drivers/resctrl/mpam_internal.h
> +++ b/drivers/resctrl/mpam_internal.h

...

> +#define MPAM_FB_PROT_HEADER_LEN	sizeof(u32)
> +#define MPAM_FB_MAX_MSG_SIZE	(MPAM_FB_PROT_HEADER_LEN + 4 * sizeof(u32))

Doesn't seem to be used until patch 10.  Might be more sensible to bring it
in there. A spec reference might also be good for this.  I think longest
is current the MPAM_MSC_WRITE.  A note above that one option would be
to make this explicit by having the message structure definitions in this
header.


> +