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

[Sudeep Holla]

On Fri, Jul 24, 2026 at 06:08:52PM +0100, Ben Horgan wrote:
> Hi Andre,
> 
> On 7/23/26 16:54, Andre Przywara 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@xxxxxxx>
> > ---
> >  drivers/resctrl/Makefile        |   2 +-
> >  drivers/resctrl/mpam_devices.c  |  27 ++++-
> >  drivers/resctrl/mpam_fb.c       | 197 ++++++++++++++++++++++++++++++++
> >  drivers/resctrl/mpam_internal.h |  20 ++++
> >  include/linux/arm_mpam.h        |   2 +-
> >  5 files changed, 240 insertions(+), 8 deletions(-)
> >  create mode 100644 drivers/resctrl/mpam_fb.c
> > 

[...]

> > +struct mpam_fb_read_payload {
> > +	u32 msc_id;
> > +	u32 flags;
> > +	u32 reg_offset;
> > +} __packed;
> > +
> > +struct mpam_fb_write_payload {
> > +	u32 msc_id;
> > +	u32 flags;
> > +	u32 reg_offset;
> > +	u32 value;
> > +} __packed;
> > +
> > +#define PCC_CHAN_FLAGS_IRQ	BIT(0)
>
> Isn't this the same as PCC_CMD_COMPLETION_NOTIFY in include/acpi/pcc.h?
>

Yes, where ever possible, one must use that. I have TODO to unify the
loads of duplication we have amongst the users of PCC channels. That's
the reason I ignored more duplications in this driver in absence of any
alternatives.

> > +
> > +static atomic_t mpam_fb_token = ATOMIC_INIT(0);
> > +
> > +static void mpam_fb_build_version_message(unsigned int token,
> > +					  void __iomem *msg_buf)
> > +{
> > +	struct acpi_pcct_ext_pcc_shared_memory __iomem *pcc_shmem = msg_buf;
>
> What's the deal with .signature?
>

No, it's written by the platform firmware when the channel is ready to be used
by OSPM and OSPM just needs to verify. It will be soon part of the PCC core
driver and it will shout if that's not the case.

> > +
> > +	writel_relaxed(PCC_CHAN_FLAGS_IRQ, &pcc_shmem->flags);
>
> Are you sure it's ok to unconditionally set this irq bit? I know we found out that we do need it set
> sometimes. What does setting it actually based on what the platform does look like?
>

You need to set it if you are not polling the PCC channels for completions.

--
Regards,
Sudeep