Re: [PATCH v4 10/10] arm_mpam: detect and enable MPAM-Fb PCC support
From: Srivathsa L Rao <hidden>
Date: 2026-07-27 16:27:26
Also in:
linux-acpi, lkml
On 7/23/2026 9:24 PM, Andre Przywara wrote:
quoted hunk ↗ jump to hunk
The Arm MPAM-Fb specification [1] describes a protocol to access MSC registers through a firmware interface. This requires a shared memory region to hold the message, and a mailbox to trigger the access. For ACPI this is wrapped as a PCC channel, described using existing ACPI abstractions. Add code to parse those PCC table descriptions associated with an MSC, and store the parsed information in the MSC struct. There can be multiple PCC channels, and each channel can serve multiple MSCs, so we need to keep track of the channel usage, using a list and a refcount. This will be used by the MPAM-Fb access wrapper code. [1] https://developer.arm.com/documentation/den0144/latest Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- drivers/acpi/arm64/mpam.c | 6 +- drivers/resctrl/mpam_devices.c | 127 ++++++++++++++++++++++++++++++++- 2 files changed, 129 insertions(+), 4 deletions(-)diff --git a/drivers/acpi/arm64/mpam.c b/drivers/acpi/arm64/mpam.c index 84963a20c3e7..ca9b8754ae5f 100644 --- a/drivers/acpi/arm64/mpam.c +++ b/drivers/acpi/arm64/mpam.c@@ -220,8 +220,8 @@ static struct platform_device * __init acpi_mpam_parse_msc(struct acpi_mpam_msc_ struct platform_device *pdev __free(platform_device_put) = platform_device_alloc("mpam_msc", tbl_msc->identifier); int next_res = 0, next_prop = 0, err; - /* pcc, nrdy, affinity and a sentinel */ - struct property_entry props[4] = { 0 }; + /* pcc, msc-id, nrdy, affinity and a sentinel */ + struct property_entry props[5] = { 0 }; /* mmio, 2xirq, no sentinel. */ struct resource res[3] = { 0 }; struct acpi_device *companion;@@ -256,6 +256,8 @@ static struct platform_device * __init acpi_mpam_parse_msc(struct acpi_mpam_msc_ } else if (iface == MPAM_IFACE_PCC) { props[next_prop++] = PROPERTY_ENTRY_U32("pcc-channel", tbl_msc->base_address); + props[next_prop++] = PROPERTY_ENTRY_U32("msc-id", + tbl_msc->identifier); } acpi_mpam_parse_irqs(pdev, tbl_msc, res, &next_res);diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c index fea3161ffbe9..fc4b5f87fbe9 100644 --- a/drivers/resctrl/mpam_devices.c +++ b/drivers/resctrl/mpam_devices.c@@ -19,14 +19,19 @@ #include <linux/irqdesc.h> #include <linux/list.h> #include <linux/lockdep.h> +#include <linux/mailbox_client.h> #include <linux/mutex.h> #include <linux/platform_device.h> #include <linux/printk.h> +#include <linux/property.h> #include <linux/srcu.h> #include <linux/spinlock.h> #include <linux/types.h> #include <linux/workqueue.h> +#include <acpi/pcc.h> +#include <acpi/acpi_io.h> + #include "mpam_internal.h" /* Values for the T241 errata workaround */@@ -49,6 +54,86 @@ static LIST_HEAD(mpam_all_msc); struct srcu_struct mpam_srcu; +/* PCC channels might be serving multiple MSCs, so keep a refcounted list. */ +static DEFINE_MUTEX(pcc_chan_list_lock); +static LIST_HEAD(pcc_chan_list); + +static void mpam_pcc_chan_release(struct kref *ref) +{ + struct mpam_pcc_chan *cur = container_of(ref, struct mpam_pcc_chan, + refcount); + + pcc_mbox_free_channel(cur->pcc_chan); + list_del(&cur->pcc_chans); + kfree(cur); +} + +static struct mpam_pcc_chan *mpam_pcc_chan_get(struct device *dev, + int subspace_id) +{ + struct mpam_pcc_chan *cur; + int ret; + + guard(mutex)(&pcc_chan_list_lock); + + list_for_each_entry(cur, &pcc_chan_list, pcc_chans) { + if (cur->subspace_id == subspace_id) { + kref_get(&cur->refcount); + + return cur; + } + } + + cur = kzalloc_obj(*cur); + if (!cur) + return ERR_PTR(-ENOMEM); + + cur->pcc_cl.dev = dev; + cur->pcc_cl.tx_block = true; + + cur->pcc_chan = pcc_mbox_request_channel(&cur->pcc_cl, subspace_id); + if (IS_ERR(cur->pcc_chan)) { + long err = PTR_ERR(cur->pcc_chan); + + kfree(cur); + return ERR_PTR(err); + } + + /* Timeout based on the "nominal latency" from the PCC ACPI table. */ + cur->pcc_cl.tx_tout = cur->pcc_chan->latency * 5; + + ret = devm_mutex_init(dev, &cur->pcc_chan_lock); + if (ret) + return ERR_PTR(ret); +
Here, devm_mutex_init() ties the lifetime of pcc_chan_lock to dev, which
is &pdev->dev of the first MSC to call mpam_pcc_chan_get(). But
mpam_pcc_chan is shared across multiple MSCs via kref, so it can
outlive that first device.
If a second MSC has incremented the refcount and is still active when
the first MSC's device is removed, the devm cleanup on the first device
would call mutex_destroy() on pcc_chan_lock while the second MSC may
still be inside:
guard(mutex)(&pcc_chan->pcc_chan_lock);
I am not 100% sure this is reachable in practice — it may depend on
the order in which platform devices are unbound, and whether that can
happen with MSCs sharing a channel. I also note that devm_mutex_init()
is a no-op on non-debug kernels, so this would only be observable with
CONFIG_DEBUG_MUTEXES=y. But it felt worth raising in case
it is a real path.
If my understanding is correct, would it make sense to use a plain
mutex_init() here instead, and move mutex_destroy() into
mpam_pcc_chan_release() where the kref guarantees no concurrent users
remain?
quoted hunk ↗ jump to hunk
+ cur->subspace_id = subspace_id; + kref_init(&cur->refcount); + + list_add_tail(&cur->pcc_chans, &pcc_chan_list); + + return cur; +} + +static int mpam_pcc_chan_put(struct mpam_pcc_chan *pcc_chan) +{ + struct mpam_pcc_chan *cur, *tmp; + + if (!pcc_chan) + return 0; + + guard(mutex)(&pcc_chan_list_lock); + + list_for_each_entry_safe(cur, tmp, &pcc_chan_list, pcc_chans) { + if (cur == pcc_chan) { + kref_put(&cur->refcount, mpam_pcc_chan_release); + + return 0; + } + } + + return -ENOENT; +} + /* * Number of MSCs that have been probed. Once all MSCs have been probed MPAM * can be enabled.@@ -2201,6 +2286,8 @@ static void mpam_msc_drv_remove(struct platform_device *pdev) { struct mpam_msc *msc = platform_get_drvdata(pdev); + mpam_pcc_chan_put(msc->pcc_chan); + mutex_lock(&mpam_list_lock); mpam_msc_destroy(msc); mutex_unlock(&mpam_list_lock);@@ -2211,7 +2298,7 @@ static void mpam_msc_drv_remove(struct platform_device *pdev) static struct mpam_msc *do_mpam_msc_drv_probe(struct platform_device *pdev) { int err; - u32 tmp; + u32 pcc_subspace_id; struct mpam_msc *msc; struct resource *msc_res; struct device *dev = &pdev->dev;@@ -2259,7 +2346,8 @@ static struct mpam_msc *do_mpam_msc_drv_probe(struct platform_device *pdev) if (err) return ERR_PTR(err); - if (device_property_read_u32(&pdev->dev, "pcc-channel", &tmp)) + if (device_property_read_u32(&pdev->dev, "pcc-channel", + &pcc_subspace_id)) msc->iface = MPAM_IFACE_MMIO; else msc->iface = MPAM_IFACE_PCC;@@ -2275,6 +2363,41 @@ static struct mpam_msc *do_mpam_msc_drv_probe(struct platform_device *pdev) } msc->mapped_hwpage_sz = msc_res->end - msc_res->start; msc->mapped_hwpage = io; + } else if (msc->iface == MPAM_IFACE_PCC) { + u32 msc_id; + int ret; + + if (device_property_read_u32(&pdev->dev, "msc-id", &msc_id)) { + pr_err("missing MPAM-Fb MSC identifier\n"); + return ERR_PTR(-EINVAL); + } + msc->mpam_fb_msc_id = msc_id; + + msc->pcc_chan = mpam_pcc_chan_get(&pdev->dev, pcc_subspace_id); + if (IS_ERR(msc->pcc_chan)) { + pr_err("Failed to request MSC PCC channel\n"); + return ERR_CAST(msc->pcc_chan); + } + + if (msc->pcc_chan->pcc_chan->shmem_size < MPAM_FB_MAX_MSG_SIZE) { + pr_err("MPAM-Fb PCC channel size too small.\n"); + mpam_pcc_chan_put(msc->pcc_chan); + return ERR_PTR(-ENOMEM); + } + ret = mpam_fb_get_protocol_version(msc); + if (ret < 0) { + pr_err("Cannot query MPAM-Fb protocol version.\n"); + mpam_pcc_chan_put(msc->pcc_chan); + + /* ret is an MPAM-Fb error code, return a Linux one. */ + return ERR_PTR(-EIO); + } + if ((ret >> 16) != 1) { + pr_err("Incompatible MPAM-Fb protocol version %d.%d\n", + ret >> 16, ret & 0xffff); + mpam_pcc_chan_put(msc->pcc_chan); + return ERR_PTR(-EINVAL); + } } else { return ERR_PTR(-EINVAL); }
Best Regards, Srivathsa