[PATCH 0/5] arm_mpam: Add MPAM-Fb firmware support

[PATCH 0/5] arm_mpam: Add MPAM-Fb firmware support

[Andre Przywara]

The Arm MPAM specification defines Memory System Components (MSCs),
which are devices that are programmed through an MMIO register frame. In
some occasions this turned out to be too limiting: the MSC might be
located behind a separate bus system (for instance inside an on-board
controller), it might be mapped secure-only, or in a different processor
socket without direct MMIO mapping. Also the MMIO access might be too slow
or it would need to be filtered or otherwise access controlled. Finally
there might be bugs in the MSC integration, which require a mediating
firmware to be accessible.

To accommodate all those different use cases, the MPAM-Fb specification
[1] describes an alternative way to access MSCs. Accesses to an MSC
would be wrapped in a message and communicated to the system using a
shared-memory/mailbox system mostly mimicking the Arm SCMI spec.
For ACPI systems, this would be abstracted through an ACPI PCC channel,
which provides the shared-memory region and the mailbox trigger. We can
lean on existing ACPI parsing code to register with these two
subsystems, but cannot rely on the existing SCMI code in the kernel.
This means we somewhat need to open code a very simplified SCMI handler,
which just provides enough functionality for the very basic subset of
SCMI that the MPAM-Fb spec requires.

Patch 1/5 adds more ACPI parsing code, to be able to learn all the
information we need.
Patch 2/5 solves a nasty problem: At the moment we protect stateful MSC
register accesses (mon_sel) through a spinlock. Unfortunately the mailbox
subsystem and the slow nature of the communication through this channel
forbid MPAM-Fb access in atomic context. So this patch splits the lock
in two: the "outer" one is a mutex, and only the inner one is a
spinlock, which would need to be taken when programming MSCs inside an
interrupt handler, for instance. We just deny the latter when using
MPAM-Fb, ideally we wouldn't need that (no need to IPI another core when
the MSC access does not need to be local to one particular core), or we
simply deny that part of the functionality (access through perf).
Patch 3/5 adds the code to redirect MSC accesses through the
shmem/mailbox system.
Patch 4/5 avoids the error IRQ handler to do an MSC access when using
MPAM_Fb, since those accesses cannot run in atomic context.
The final patch 5/5 then adds the code to detect and store the PCC
channel information from the ACPI tables, and eventually enables
MPAM-Fb accesses.

This would enable systems where some MSCs are not accessible via MMIO to
use those components anyway.

Please have a look and test!

Cheers,
Andre

[1] https://developer.arm.com/documentation/den0144/latest

Andre Przywara (3):
  arm_mpam: add MPAM-Fb MSC firmware access support
  arm_mpam: prevent MPAM-Fb accesses inside IRQ handler
  arm_mpam: detect and enable MPAM-Fb PCC support

James Morse (2):
  arm_mpam: Parse the rest of the ACPI table
  arm_mpam: Split the locking around the mon_sel registers

 drivers/acpi/arm64/mpam.c       |  93 ++++++++++++++++++-
 drivers/resctrl/Makefile        |   2 +-
 drivers/resctrl/mpam_devices.c  | 148 ++++++++++++++++++++++++------
 drivers/resctrl/mpam_fb.c       | 158 ++++++++++++++++++++++++++++++++
 drivers/resctrl/mpam_fb.h       |  17 ++++
 drivers/resctrl/mpam_internal.h |  72 +++++++++++----
 include/linux/arm_mpam.h        |   2 +-
 7 files changed, 439 insertions(+), 53 deletions(-)
 create mode 100644 drivers/resctrl/mpam_fb.c
 create mode 100644 drivers/resctrl/mpam_fb.h

-- 
2.43.0

[PATCH 1/5] arm_mpam: Parse the rest of the ACPI table

[Andre Przywara]

From: James Morse <james.morse@arm.com>

The MPAM ACPI table lists the MPAM MSCs and indicates which resources
in the system they control. Not everything this table can describe is
supported by resctrl, e.g. memory-side-caches.

Add the additional table parsing to avoid reporting these as 'unknown'
to the MPAM driver. This allows class+component hierarchys to be built.

Until resctrl has support for any of these resources, users would be
in-kernel managers of a resource/PARTID or perf to query bandwidth
counters on a resource resctrl is unaware of.

Signed-off-by: James Morse <james.morse@arm.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/acpi/arm64/mpam.c | 91 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 88 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/arm64/mpam.c b/drivers/acpi/arm64/mpam.c
index 84963a20c3e7..99c2bdbb3314 100644
--- a/drivers/acpi/arm64/mpam.c
+++ b/drivers/acpi/arm64/mpam.c
@@ -95,17 +95,51 @@ static void acpi_mpam_parse_irqs(struct platform_device *pdev,
 		res[(*res_idx)++] = DEFINE_RES_IRQ_NAMED(irq, "error");
 }
 
-static int acpi_mpam_parse_resource(struct mpam_msc *msc,
+#define UUID_MPAM_INTERCONNECT_TABLE		"fe2bd645-033b-49e6-9479-2e0b8b21d1cd"
+
+struct acpi_mpam_interconnect_descriptor_table {
+	u8	type_uuid[16];
+	u32	num_descriptors;
+};
+
+struct acpi_mpam_interconnect_descriptor {
+	u32	source_id;
+	u32	destination_id;
+	u8	link_type;
+	u8	reserved[3];
+};
+
+static int acpi_mpam_parse_resource(struct acpi_mpam_msc_node *tbl_msc,
+				    struct mpam_msc *msc,
 				    struct acpi_mpam_resource_node *res)
 {
+	struct acpi_mpam_interconnect_descriptor_table *tbl_int_tbl;
+	struct acpi_mpam_interconnect_descriptor *tbl_int;
+	guid_t int_tbl_uuid, spec_uuid;
 	int level, nid;
 	u32 cache_id;
+	off_t offset;
 
+	/*
+	 * Class IDs are somewhat arbitrary, but need to be co-ordinated.
+	 * 0-N are caches,
+	 * 64, 65: Interconnect, but ideally these would appear between the
+	 *     classes the controls are adjacent to.
+	 * 128: SMMU,
+	 * 192-192+level: Memory Side Caches, nothing checks that N is a
+	 *                small number.
+	 * 255: Memory Controllers
+	 *
+	 * ACPI devices would need a class id allocated based on the _HID.
+	 *
+	 * Classes that the mpam driver can't currently plumb into resctrl
+	 * are registered as UNKNOWN.
+	 */
 	switch (res->locator_type) {
 	case ACPI_MPAM_LOCATION_TYPE_PROCESSOR_CACHE:
 		cache_id = res->locator.cache_locator.cache_reference;
 		level = find_acpi_cache_level_from_id(cache_id);
-		if (level <= 0) {
+		if (level <= 0 || level >= 64) {
 			pr_err_once("Bad level (%d) for cache with id %u\n", level, cache_id);
 			return -EINVAL;
 		}
@@ -120,6 +154,57 @@ static int acpi_mpam_parse_resource(struct mpam_msc *msc,
 		}
 		return mpam_ris_create(msc, res->ris_index, MPAM_CLASS_MEMORY,
 				       MPAM_CLASS_ID_DEFAULT, nid);
+	case ACPI_MPAM_LOCATION_TYPE_SMMU:
+		return mpam_ris_create(msc, res->ris_index, MPAM_CLASS_UNKNOWN,
+				       128, res->locator.smmu_locator.smmu_interface);
+	case ACPI_MPAM_LOCATION_TYPE_MEMORY_CACHE:
+		cache_id = res->locator.mem_cache_locator.reference;
+		level = res->locator.mem_cache_locator.level;
+		if (192 + level >= 255) {
+			pr_err_once("Bad level for memory side cache with reference %u\n",
+				    cache_id);
+			return -EINVAL;
+		}
+
+		return mpam_ris_create(msc, res->ris_index, MPAM_CLASS_CACHE,
+				       192 + level, cache_id);
+
+	case ACPI_MPAM_LOCATION_TYPE_INTERCONNECT:
+		/* Find the descriptor table, and check it lands in the parent msc */
+		offset = res->locator.interconnect_ifc_locator.inter_connect_desc_tbl_off;
+		if (offset >= tbl_msc->length) {
+			pr_err_once("Bad offset for interconnect descriptor on msc %u\n",
+				    tbl_msc->identifier);
+			return -EINVAL;
+		}
+		tbl_int_tbl = ACPI_ADD_PTR(struct acpi_mpam_interconnect_descriptor_table,
+					   tbl_msc, offset);
+		guid_parse(UUID_MPAM_INTERCONNECT_TABLE, &spec_uuid);
+		import_guid(&int_tbl_uuid, tbl_int_tbl->type_uuid);
+		if (guid_equal(&spec_uuid, &int_tbl_uuid)) {
+			pr_err_once("Bad UUID for interconnect descriptor on msc %u\n",
+				    tbl_msc->identifier);
+			return -EINVAL;
+		}
+
+		offset += sizeof(*tbl_int_tbl);
+		offset += tbl_int_tbl->num_descriptors * sizeof(*tbl_int);
+		if (offset >= tbl_msc->length) {
+			pr_err_once("Bad num_descriptors for interconnect descriptor on msc %u\n",
+				    tbl_msc->identifier);
+			return -EINVAL;
+		}
+
+		tbl_int = ACPI_ADD_PTR(struct acpi_mpam_interconnect_descriptor,
+				       tbl_int_tbl, sizeof(*tbl_int_tbl));
+		cache_id = tbl_int->source_id;
+
+		/* Unknown link type? */
+		if (tbl_int->link_type != 0 && tbl_int->link_type == 1)
+			return 0;
+
+		return mpam_ris_create(msc, res->ris_index, MPAM_CLASS_UNKNOWN,
+				       64 + tbl_int->link_type, cache_id);
 	default:
 		/* These get discovered later and are treated as unknown */
 		return 0;
@@ -150,7 +235,7 @@ int acpi_mpam_parse_resources(struct mpam_msc *msc,
 			return -EINVAL;
 		}
 
-		err = acpi_mpam_parse_resource(msc, resource);
+		err = acpi_mpam_parse_resource(tbl_msc, msc, resource);
 		if (err)
 			return err;
 
-- 
2.43.0

[PATCH 2/5] arm_mpam: Split the locking around the mon_sel registers

[Andre Przywara]

From: James Morse <james.morse@arm.com>

The MSC MON_SEL register needs to be accessed from hardirq for the overflow
interrupt, and when taking an IPI to access these registers on platforms
where MSC are not accesible from every CPU. This makes an irqsave
spinlock the obvious lock to protect these registers. On systems with SCMI
mailboxes it must be able to sleep, meaning a mutex must be used. The
SCMI platforms can't support an overflow interrupt.

Clearly these two can't exist for one MSC at the same time.

Split the existing helper into a raw spinlock and a mutex, named inner
and outer. The outer lock must be taken in an a pre-emptible context
befroe the inner lock can be taken. On systems with SCMI mailboxes
where the MON_SEL accesses must sleep - the inner lock will fail tobe
taken if the caller is unable to sleep.
This will allow callers to fail withuot having to explicitly check
the interface type of each MSC.

Signed-off-by: James Morse <james.morse@arm.com>
---
 drivers/resctrl/mpam_devices.c  | 56 ++++++++++++++++++++-------
 drivers/resctrl/mpam_internal.h | 67 ++++++++++++++++++++++++---------
 2 files changed, 91 insertions(+), 32 deletions(-)

diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index 41b14344b16f..3be09b0c46ae 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -735,7 +735,7 @@ static bool _mpam_ris_hw_probe_hw_nrdy(struct mpam_msc_ris *ris, u32 mon_reg)
 	bool can_set, can_clear;
 	struct mpam_msc *msc = ris->vmsc->msc;
 
-	if (WARN_ON_ONCE(!mpam_mon_sel_lock(msc)))
+	if (WARN_ON_ONCE(!mpam_mon_sel_inner_lock(msc)))
 		return false;
 
 	mon_sel = FIELD_PREP(MSMON_CFG_MON_SEL_MON_SEL, 0) |
@@ -749,7 +749,7 @@ static bool _mpam_ris_hw_probe_hw_nrdy(struct mpam_msc_ris *ris, u32 mon_reg)
 	_mpam_write_monsel_reg(msc, mon_reg, 0);
 	now = _mpam_read_monsel_reg(msc, mon_reg);
 	can_clear = !(now & MSMON___NRDY);
-	mpam_mon_sel_unlock(msc);
+	mpam_mon_sel_inner_unlock(msc);
 
 	return (!can_set || !can_clear);
 }
@@ -873,7 +873,9 @@ static void mpam_ris_hw_probe(struct mpam_msc_ris *ris)
 					mpam_set_feature(mpam_feat_msmon_csu_xcl, props);
 
 				/* Is NRDY hardware managed? */
+				mpam_mon_sel_outer_lock(msc);
 				hw_managed = mpam_ris_hw_probe_hw_nrdy(ris, CSU);
+				mpam_mon_sel_outer_unlock(msc);
 				if (hw_managed)
 					mpam_set_feature(mpam_feat_msmon_csu_hw_nrdy, props);
 			}
@@ -907,7 +909,9 @@ static void mpam_ris_hw_probe(struct mpam_msc_ris *ris)
 				}
 
 				/* Is NRDY hardware managed? */
+				mpam_mon_sel_outer_lock(msc);
 				hw_managed = mpam_ris_hw_probe_hw_nrdy(ris, MBWU);
+				mpam_mon_sel_outer_unlock(msc);
 				if (hw_managed)
 					mpam_set_feature(mpam_feat_msmon_mbwu_hw_nrdy, props);
 
@@ -1201,7 +1205,7 @@ static void __ris_msmon_read(void *arg)
 	struct mpam_msc *msc = m->ris->vmsc->msc;
 	u32 mon_sel, ctl_val, flt_val, cur_ctl, cur_flt;
 
-	if (!mpam_mon_sel_lock(msc)) {
+	if (!mpam_mon_sel_inner_lock(msc)) {
 		m->err = -EIO;
 		return;
 	}
@@ -1301,7 +1305,7 @@ static void __ris_msmon_read(void *arg)
 	default:
 		m->err = -EINVAL;
 	}
-	mpam_mon_sel_unlock(msc);
+	mpam_mon_sel_inner_unlock(msc);
 
 	if (nrdy)
 		m->err = -EBUSY;
@@ -1323,6 +1327,7 @@ static int _msmon_read(struct mpam_component *comp, struct mon_read *arg)
 		struct mpam_msc *msc = vmsc->msc;
 		struct mpam_msc_ris *ris;
 
+		mpam_mon_sel_outer_lock(msc);
 		list_for_each_entry_srcu(ris, &vmsc->ris, vmsc_list,
 					 srcu_read_lock_held(&mpam_srcu)) {
 			arg->ris = ris;
@@ -1341,6 +1346,7 @@ static int _msmon_read(struct mpam_component *comp, struct mon_read *arg)
 			if (err)
 				any_err = err;
 		}
+		mpam_mon_sel_outer_unlock(msc);
 	}
 
 	return any_err;
@@ -1423,18 +1429,20 @@ void mpam_msmon_reset_mbwu(struct mpam_component *comp, struct mon_cfg *ctx)
 			continue;
 
 		msc = vmsc->msc;
+		mpam_mon_sel_outer_lock(msc);
 		list_for_each_entry_srcu(ris, &vmsc->ris, vmsc_list,
 					 srcu_read_lock_held(&mpam_srcu)) {
 			if (!mpam_has_feature(mpam_feat_msmon_mbwu, &ris->props))
 				continue;
 
-			if (WARN_ON_ONCE(!mpam_mon_sel_lock(msc)))
+			if (WARN_ON_ONCE(!mpam_mon_sel_inner_lock(msc)))
 				continue;
 
 			ris->mbwu_state[ctx->mon].correction = 0;
 			ris->mbwu_state[ctx->mon].reset_on_next_read = true;
-			mpam_mon_sel_unlock(msc);
+			mpam_mon_sel_inner_unlock(msc);
 		}
+		mpam_mon_sel_outer_unlock(msc);
 	}
 }
 
@@ -1635,8 +1643,11 @@ static int mpam_restore_mbwu_state(void *_ris)
 	u64 val;
 	struct mon_read mwbu_arg;
 	struct mpam_msc_ris *ris = _ris;
+	struct mpam_msc *msc = ris->vmsc->msc;
 	struct mpam_class *class = ris->vmsc->comp->class;
 
+	mpam_mon_sel_outer_lock(msc);
+
 	for (i = 0; i < ris->props.num_mbwu_mon; i++) {
 		if (ris->mbwu_state[i].enabled) {
 			mwbu_arg.ris = ris;
@@ -1648,10 +1659,12 @@ static int mpam_restore_mbwu_state(void *_ris)
 		}
 	}
 
+	mpam_mon_sel_outer_unlock(msc);
+
 	return 0;
 }
 
-/* Call with MSC cfg_lock held */
+/* Call with MSC cfg_lock and outer mon_sel lock held */
 static int mpam_save_mbwu_state(void *arg)
 {
 	int i;
@@ -1666,7 +1679,7 @@ static int mpam_save_mbwu_state(void *arg)
 		mbwu_state = &ris->mbwu_state[i];
 		cfg = &mbwu_state->cfg;
 
-		if (WARN_ON_ONCE(!mpam_mon_sel_lock(msc)))
+		if (WARN_ON_ONCE(!mpam_mon_sel_inner_lock(msc)))
 			return -EIO;
 
 		mon_sel = FIELD_PREP(MSMON_CFG_MON_SEL_MON_SEL, i) |
@@ -1691,7 +1704,7 @@ static int mpam_save_mbwu_state(void *arg)
 		cfg->partid = FIELD_GET(MSMON_CFG_x_FLT_PARTID, cur_flt);
 		mbwu_state->correction += val;
 		mbwu_state->enabled = FIELD_GET(MSMON_CFG_x_CTL_EN, cur_ctl);
-		mpam_mon_sel_unlock(msc);
+		mpam_mon_sel_inner_unlock(msc);
 	}
 
 	return 0;
@@ -1773,6 +1786,7 @@ static void mpam_reprogram_msc(struct mpam_msc *msc)
 	mpam_assert_partid_sizes_fixed();
 
 	mutex_lock(&msc->cfg_lock);
+	mpam_mon_sel_outer_lock(msc);
 	list_for_each_entry_srcu(ris, &msc->ris, msc_list,
 				 srcu_read_lock_held(&mpam_srcu)) {
 		if (!mpam_is_enabled() && !ris->in_reset_state) {
@@ -1797,6 +1811,7 @@ static void mpam_reprogram_msc(struct mpam_msc *msc)
 		if (mpam_has_feature(mpam_feat_msmon_mbwu, &ris->props))
 			mpam_touch_msc(msc, &mpam_restore_mbwu_state, ris);
 	}
+	mpam_mon_sel_outer_unlock(msc);
 	mutex_unlock(&msc->cfg_lock);
 }
 
@@ -1886,6 +1901,7 @@ static int mpam_cpu_offline(unsigned int cpu)
 			struct mpam_msc_ris *ris;
 
 			mutex_lock(&msc->cfg_lock);
+			mpam_mon_sel_outer_lock(msc);
 			list_for_each_entry_srcu(ris, &msc->ris, msc_list,
 						 srcu_read_lock_held(&mpam_srcu)) {
 				mpam_touch_msc(msc, &mpam_reset_ris, ris);
@@ -1899,6 +1915,7 @@ static int mpam_cpu_offline(unsigned int cpu)
 				if (mpam_is_enabled())
 					mpam_touch_msc(msc, &mpam_save_mbwu_state, ris);
 			}
+			mpam_mon_sel_outer_unlock(msc);
 			mutex_unlock(&msc->cfg_lock);
 		}
 	}
@@ -2589,11 +2606,13 @@ static void __destroy_component_cfg(struct mpam_component *comp)
 	list_for_each_entry(vmsc, &comp->vmsc, comp_list) {
 		msc = vmsc->msc;
 
-		if (mpam_mon_sel_lock(msc)) {
+		mpam_mon_sel_outer_lock(msc);
+		if (mpam_mon_sel_inner_lock(msc)) {
 			list_for_each_entry(ris, &vmsc->ris, vmsc_list)
 				add_to_garbage(ris->mbwu_state);
-			mpam_mon_sel_unlock(msc);
+			mpam_mon_sel_inner_unlock(msc);
 		}
+		mpam_mon_sel_outer_unlock(msc);
 	}
 }
 
@@ -2640,6 +2659,7 @@ static int __allocate_component_cfg(struct mpam_component *comp)
 	mpam_reset_component_cfg(comp);
 
 	list_for_each_entry(vmsc, &comp->vmsc, comp_list) {
+		int err = 0;
 		struct mpam_msc *msc;
 		struct mpam_msc_ris *ris;
 		struct msmon_mbwu_state *mbwu_state;
@@ -2648,6 +2668,7 @@ static int __allocate_component_cfg(struct mpam_component *comp)
 			continue;
 
 		msc = vmsc->msc;
+		mpam_mon_sel_outer_lock(msc);
 		list_for_each_entry(ris, &vmsc->ris, vmsc_list) {
 			if (!ris->props.num_mbwu_mon)
 				continue;
@@ -2656,16 +2677,21 @@ static int __allocate_component_cfg(struct mpam_component *comp)
 						  ris->props.num_mbwu_mon);
 			if (!mbwu_state) {
 				__destroy_component_cfg(comp);
-				return -ENOMEM;
+				err = -ENOMEM;
+				break;
 			}
 
 			init_garbage(&mbwu_state[0].garbage);
 
-			if (mpam_mon_sel_lock(msc)) {
+			if (mpam_mon_sel_inner_lock(msc)) {
 				ris->mbwu_state = mbwu_state;
-				mpam_mon_sel_unlock(msc);
+				mpam_mon_sel_inner_unlock(msc);
 			}
 		}
+		mpam_mon_sel_outer_unlock(msc);
+
+		if (err)
+			return err;
 	}
 
 	return 0;
@@ -2918,12 +2944,14 @@ int mpam_apply_config(struct mpam_component *comp, u16 partid,
 		msc = vmsc->msc;
 
 		mutex_lock(&msc->cfg_lock);
+		mpam_mon_sel_outer_lock(msc);
 		list_for_each_entry_srcu(ris, &vmsc->ris, vmsc_list,
 					 srcu_read_lock_held(&mpam_srcu)) {
 			arg.ris = ris;
 			mpam_touch_msc(msc, __write_config, &arg);
 			ris->in_reset_state = false;
 		}
+		mpam_mon_sel_outer_unlock(msc);
 		mutex_unlock(&msc->cfg_lock);
 	}
 
diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
index 1914aefdcba9..80213af10a64 100644
--- a/drivers/resctrl/mpam_internal.h
+++ b/drivers/resctrl/mpam_internal.h
@@ -116,16 +116,20 @@ struct mpam_msc {
 	/*
 	 * mon_sel_lock protects access to the MSC hardware registers that are
 	 * affected by MPAMCFG_MON_SEL, and the mbwu_state.
-	 * Access to mon_sel is needed from both process and interrupt contexts,
-	 * but is complicated by firmware-backed platforms that can't make any
-	 * access unless they can sleep.
-	 * Always use the mpam_mon_sel_lock() helpers.
-	 * Accesses to mon_sel need to be able to fail if they occur in the wrong
-	 * context.
+	 * Both the 'inner' and 'outer' must be taken.
+	 * For real MMIO MSC, the outer lock is unnecessary - but keeps the
+	 * code common with:
+	 * Firmware backed MSC need to sleep when accessing the MSC, which
+	 * means some code-paths will always fail. For these MSC the outer
+	 * lock is providing the protection, and the inner lock fails to
+	 * be taken if the task is unable to sleep.
+	 *
 	 * If needed, take msc->probe_lock first.
 	 */
-	raw_spinlock_t		_mon_sel_lock;
-	unsigned long		_mon_sel_flags;
+	struct mutex		outer_mon_sel_lock;
+	bool			outer_lock_held;
+	raw_spinlock_t		inner_mon_sel_lock;
+	unsigned long		inner_mon_sel_flags;
 
 	void __iomem		*mapped_hwpage;
 	size_t			mapped_hwpage_sz;
@@ -137,29 +141,56 @@ struct mpam_msc {
 };
 
 /* Returning false here means accesses to mon_sel must fail and report an error. */
-static inline bool __must_check mpam_mon_sel_lock(struct mpam_msc *msc)
+static inline bool __must_check mpam_mon_sel_inner_lock(struct mpam_msc *msc)
 {
-	/* Locking will require updating to support a firmware backed interface */
-	if (WARN_ON_ONCE(msc->iface != MPAM_IFACE_MMIO))
-		return false;
+	/*
+	 * The outer lock may be taken by a CPU that then issues an IPI to run
+	 * a helper that takes the inner lock. lockdep can't help us here.
+	 */
+	WARN_ON_ONCE(!READ_ONCE(msc->outer_lock_held));
+
+	if (msc->iface == MPAM_IFACE_MMIO) {
+		raw_spin_lock_irqsave(&msc->inner_mon_sel_lock, msc->inner_mon_sel_flags);
+		return true;
+	}
+
+	/* Accesses must fail if we are not pre-emptible */
+	return !!preemptible();
+}
 
-	raw_spin_lock_irqsave(&msc->_mon_sel_lock, msc->_mon_sel_flags);
-	return true;
+static inline void mpam_mon_sel_inner_unlock(struct mpam_msc *msc)
+{
+	WARN_ON_ONCE(!READ_ONCE(msc->outer_lock_held));
+
+	if (msc->iface == MPAM_IFACE_MMIO)
+		raw_spin_unlock_irqrestore(&msc->inner_mon_sel_lock, msc->inner_mon_sel_flags);
+}
+
+static inline void mpam_mon_sel_outer_lock(struct mpam_msc *msc)
+{
+	mutex_lock(&msc->outer_mon_sel_lock);
+	msc->outer_lock_held = true;
 }
 
-static inline void mpam_mon_sel_unlock(struct mpam_msc *msc)
+static inline void mpam_mon_sel_outer_unlock(struct mpam_msc *msc)
 {
-	raw_spin_unlock_irqrestore(&msc->_mon_sel_lock, msc->_mon_sel_flags);
+	msc->outer_lock_held = false;
+	mutex_unlock(&msc->outer_mon_sel_lock);
 }
 
 static inline void mpam_mon_sel_lock_held(struct mpam_msc *msc)
 {
-	lockdep_assert_held_once(&msc->_mon_sel_lock);
+	WARN_ON_ONCE(!READ_ONCE(msc->outer_lock_held));
+	if (msc->iface == MPAM_IFACE_MMIO)
+		lockdep_assert_held_once(&msc->inner_mon_sel_lock);
+	else
+		lockdep_assert_preemption_enabled();
 }
 
 static inline void mpam_mon_sel_lock_init(struct mpam_msc *msc)
 {
-	raw_spin_lock_init(&msc->_mon_sel_lock);
+	raw_spin_lock_init(&msc->inner_mon_sel_lock);
+	mutex_init(&msc->outer_mon_sel_lock);
 }
 
 /* Bits for mpam features bitmaps */
-- 
2.43.0

[PATCH 3/5] arm_mpam: add MPAM-Fb MSC firmware access support

[Andre Przywara]

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
SCMI message generation and handshake, for just the fields we need.

[1] https://developer.arm.com/documentation/den0144/latest

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/resctrl/Makefile        |   2 +-
 drivers/resctrl/mpam_devices.c  |  16 +++-
 drivers/resctrl/mpam_fb.c       | 158 ++++++++++++++++++++++++++++++++
 drivers/resctrl/mpam_fb.h       |  17 ++++
 drivers/resctrl/mpam_internal.h |   5 +
 include/linux/arm_mpam.h        |   2 +-
 6 files changed, 196 insertions(+), 4 deletions(-)
 create mode 100644 drivers/resctrl/mpam_fb.c
 create mode 100644 drivers/resctrl/mpam_fb.h

diff --git a/drivers/resctrl/Makefile b/drivers/resctrl/Makefile
index 4f6d0e81f9b8..097c036724e9 100644
--- a/drivers/resctrl/Makefile
+++ b/drivers/resctrl/Makefile
@@ -1,5 +1,5 @@
 obj-$(CONFIG_ARM64_MPAM_DRIVER)			+= mpam.o
-mpam-y						+= mpam_devices.o
+mpam-y						+= mpam_devices.o mpam_fb.o
 mpam-$(CONFIG_ARM64_MPAM_RESCTRL_FS)		+= mpam_resctrl.o
 
 ccflags-$(CONFIG_ARM64_MPAM_DRIVER_DEBUG)	+= -DDEBUG
diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index 3be09b0c46ae..90b49bfd5caa 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -28,6 +28,7 @@
 #include <linux/workqueue.h>
 
 #include "mpam_internal.h"
+#include "mpam_fb.h"
 
 /* Values for the T241 errata workaround */
 #define T241_CHIPS_MAX			4
@@ -175,6 +176,13 @@ static u32 __mpam_read_reg(struct mpam_msc *msc, u16 reg)
 {
 	WARN_ON_ONCE(!cpumask_test_cpu(smp_processor_id(), &msc->accessibility));
 
+	if (msc->iface == MPAM_IFACE_PCC) {
+		u32 ret;
+
+		mpam_fb_send_read_request(msc, reg, &ret);
+		return ret;
+	}
+
 	return readl_relaxed(msc->mapped_hwpage + reg);
 }
 
@@ -188,10 +196,14 @@ static inline u32 _mpam_read_partsel_reg(struct mpam_msc *msc, u16 reg)
 
 static void __mpam_write_reg(struct mpam_msc *msc, u16 reg, u32 val)
 {
-	WARN_ON_ONCE(reg + sizeof(u32) > msc->mapped_hwpage_sz);
 	WARN_ON_ONCE(!cpumask_test_cpu(smp_processor_id(), &msc->accessibility));
 
-	writel_relaxed(val, msc->mapped_hwpage + reg);
+	if (msc->iface == MPAM_IFACE_PCC) {
+		mpam_fb_send_write_request(msc, reg, val);
+	} else {
+		WARN_ON_ONCE(reg + sizeof(u32) > msc->mapped_hwpage_sz);
+		writel_relaxed(val, msc->mapped_hwpage + reg);
+	}
 }
 
 static inline void _mpam_write_partsel_reg(struct mpam_msc *msc, u16 reg, u32 val)
diff --git a/drivers/resctrl/mpam_fb.c b/drivers/resctrl/mpam_fb.c
new file mode 100644
index 000000000000..bfb5798c74b0
--- /dev/null
+++ b/drivers/resctrl/mpam_fb.c
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2024 Arm Ltd.
+
+#include <linux/arm_mpam.h>
+#include <linux/cleanup.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/gfp.h>
+#include <linux/iopoll.h>
+#include <linux/list.h>
+#include <linux/mailbox_client.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/printk.h>
+#include <linux/processor.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+#include <acpi/pcc.h>
+
+#include <asm/mpam.h>
+
+#include "mpam_fb.h"
+
+#define MPAM_FB_PROTOCOL_ID	0x1a
+#define MPAM_MSC_ATTRIBUTES_CMD	0x3
+#define MPAM_MSC_READ_CMD	0x4
+#define MPAM_MSC_WRITE_CMD	0x5
+
+#define MPAM_MSC_PROT_ID_MASK	GENMASK(17, 10)
+#define MPAM_MSC_TOKEN_MASK	GENMASK(27, 18)
+
+#define SCMI_CHAN_RSVD_OFS	0x00
+#define SCMI_CHAN_STATUS_OFS	0x04
+#define SCMI_CHAN_STATUS_FREE_BIT	BIT(0)
+#define SCMI_CHAN_FLAGS_OFS	0x10
+#define SCMI_CHAN_FLAGS_IRQ		BIT(0)
+#define SCMI_MSG_LENGTH_OFS	0x14
+#define SCMI_MSG_HEADER_OFS	0x18
+#define SCMI_MSG_PAYLOAD_OFS	0x1c
+
+#define MPAM_READ_MSG_SIZE	(SCMI_MSG_PAYLOAD_OFS + 3 * sizeof(u32))
+#define MPAM_WRITE_MSG_SIZE	(SCMI_MSG_PAYLOAD_OFS + 4 * sizeof(u32))
+
+static atomic_t mpam_fb_token = ATOMIC_INIT(0);
+
+static int mpam_fb_build_read_message(int msc_id, int reg, unsigned int token,
+				      void __iomem *msg_buf)
+{
+	writel_relaxed(SCMI_CHAN_FLAGS_IRQ, msg_buf + SCMI_CHAN_FLAGS_OFS);
+	writel_relaxed(MPAM_READ_MSG_SIZE, msg_buf + SCMI_MSG_LENGTH_OFS);
+	writel_relaxed(MPAM_MSC_READ_CMD |
+		       FIELD_PREP(MPAM_MSC_TOKEN_MASK, token) |
+		       FIELD_PREP(MPAM_MSC_PROT_ID_MASK, MPAM_FB_PROTOCOL_ID),
+		       msg_buf + SCMI_MSG_HEADER_OFS);
+
+	writel_relaxed(cpu_to_le32(msc_id), msg_buf + SCMI_MSG_PAYLOAD_OFS);
+	writel_relaxed(0, msg_buf + SCMI_MSG_PAYLOAD_OFS + 0x4);
+	writel_relaxed(cpu_to_le32(reg), msg_buf + SCMI_MSG_PAYLOAD_OFS + 0x8);
+
+	return MPAM_READ_MSG_SIZE;
+}
+
+static int mpam_fb_build_write_message(int msc_id, int reg, u32 val,
+				       unsigned int token,
+				       void __iomem *msg_buf)
+{
+	writel_relaxed(MPAM_WRITE_MSG_SIZE, msg_buf + SCMI_MSG_LENGTH_OFS);
+	writel_relaxed(MPAM_MSC_WRITE_CMD |
+		       FIELD_PREP(MPAM_MSC_TOKEN_MASK, token) |
+		       FIELD_PREP(MPAM_MSC_PROT_ID_MASK, MPAM_FB_PROTOCOL_ID),
+		       msg_buf + SCMI_MSG_HEADER_OFS);
+
+	writel_relaxed(cpu_to_le32(msc_id), msg_buf + SCMI_MSG_PAYLOAD_OFS);
+	writel_relaxed(0, msg_buf + SCMI_MSG_PAYLOAD_OFS + 0x4);
+	writel_relaxed(cpu_to_le32(reg), msg_buf + SCMI_MSG_PAYLOAD_OFS + 0x8);
+	writel_relaxed(cpu_to_le32(val), msg_buf + SCMI_MSG_PAYLOAD_OFS + 0xc);
+
+	return MPAM_WRITE_MSG_SIZE;
+}
+
+#define SCMI_CHANNEL_FREE	true
+#define SCMI_CHANNEL_BUSY	false
+static int mpam_fb_wait_for_channel(struct pcc_mbox_chan *chan,
+				    bool free)
+{
+	u32 status = free ? SCMI_CHAN_STATUS_FREE_BIT : 0;
+	u32 val;
+
+	/*
+	 * The channel should really be free always at this point, as we take
+	 * a lock for every read or write request. Check the free bit anyway,
+	 * for good measure and to catch corner cases.
+	 */
+	return readl_poll_timeout(chan->shmem + SCMI_CHAN_STATUS_OFS, val,
+				  (val & SCMI_CHAN_STATUS_FREE_BIT) == status,
+				  1, 10000);
+}
+
+static int mpam_fb_send_request(struct mpam_msc *msc, u16 reg, u32 *result,
+				bool is_write)
+{
+	unsigned int token = atomic_inc_return(&mpam_fb_token);
+	struct pcc_mbox_chan *chan = msc->pcc_chan;
+	u32 status;
+	int ret;
+
+	guard(mutex)(&msc->pcc_chan_lock);
+	ret = mpam_fb_wait_for_channel(chan, SCMI_CHANNEL_FREE);
+	if (ret < 0)
+		return ret;
+
+	/* Clear error bit and mark the channel as belonging to the callee */
+	writel(0, chan->shmem + SCMI_CHAN_STATUS_OFS);
+
+	if (is_write)
+		ret = mpam_fb_build_write_message(msc->mpam_fb_msc_id, reg,
+						  *result, token, chan->shmem);
+	else
+		ret = mpam_fb_build_read_message(msc->mpam_fb_msc_id, reg,
+						 token, chan->shmem);
+	if (ret < 0)
+		return ret;
+
+	ret = mbox_send_message(chan->mchan, NULL);
+	if (ret < 0)
+		return ret;
+
+	ret = mpam_fb_wait_for_channel(chan, SCMI_CHANNEL_FREE);
+	if (ret)
+		return ret;
+
+	status = readl(chan->shmem + SCMI_MSG_HEADER_OFS);
+	if (FIELD_GET(MPAM_MSC_TOKEN_MASK, status) != token)
+		return -ETIMEDOUT;
+
+	ret = readl(chan->shmem + SCMI_MSG_PAYLOAD_OFS + 0x0);
+	if (ret < 0)
+		return ret;
+
+	if (!is_write)
+		*result = readl(chan->shmem + SCMI_MSG_PAYLOAD_OFS + 0x4);
+
+	return 0;
+}
+
+int mpam_fb_send_read_request(struct mpam_msc *msc, u16 reg, u32 *result)
+{
+	return mpam_fb_send_request(msc, reg, result, false);
+}
+
+int mpam_fb_send_write_request(struct mpam_msc *msc, u16 reg, u32 value)
+{
+	return mpam_fb_send_request(msc, reg, &value, true);
+}
diff --git a/drivers/resctrl/mpam_fb.h b/drivers/resctrl/mpam_fb.h
new file mode 100644
index 000000000000..c3d1a239e16f
--- /dev/null
+++ b/drivers/resctrl/mpam_fb.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+// Copyright (C) 2024-2025 Arm Ltd.
+
+#ifndef MPAM_FB_H_
+#define MPAM_FB_H_
+
+#include <linux/types.h>
+#include "mpam_internal.h"
+
+#define SCMI_MSG_PAYLOAD_OFS	0x1c
+#define MPAM_WRITE_MSG_SIZE	(SCMI_MSG_PAYLOAD_OFS + 4 * sizeof(u32))
+#define MPAM_FB_MAX_MSG_SIZE	MPAM_WRITE_MSG_SIZE
+
+int mpam_fb_send_read_request(struct mpam_msc *msc, u16 reg, u32 *result);
+int mpam_fb_send_write_request(struct mpam_msc *msc, u16 reg, u32 value);
+
+#endif
diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
index 80213af10a64..3f524cd4fc81 100644
--- a/drivers/resctrl/mpam_internal.h
+++ b/drivers/resctrl/mpam_internal.h
@@ -11,6 +11,7 @@
 #include <linux/io.h>
 #include <linux/jump_label.h>
 #include <linux/llist.h>
+#include <linux/mailbox_client.h>
 #include <linux/mutex.h>
 #include <linux/resctrl.h>
 #include <linux/spinlock.h>
@@ -66,6 +67,10 @@ struct mpam_msc {
 
 	/* Not modified after mpam_is_enabled() becomes true */
 	enum mpam_msc_iface	iface;
+	struct mbox_client	pcc_cl;
+	struct pcc_mbox_chan	*pcc_chan;
+	struct mutex		pcc_chan_lock; /* only one message at a time */
+	int			mpam_fb_msc_id;	/* in its own name space */
 	u32			nrdy_usec;
 	cpumask_t		accessibility;
 	bool			has_extd_esr;
diff --git a/include/linux/arm_mpam.h b/include/linux/arm_mpam.h
index f92a36187a52..002f56e15362 100644
--- a/include/linux/arm_mpam.h
+++ b/include/linux/arm_mpam.h
@@ -12,7 +12,7 @@ struct mpam_msc;
 
 enum mpam_msc_iface {
 	MPAM_IFACE_MMIO,	/* a real MPAM MSC */
-	MPAM_IFACE_PCC,		/* a fake MPAM MSC */
+	MPAM_IFACE_PCC,		/* using the MPAM-Fb firmware redirection */
 };
 
 enum mpam_class_types {
-- 
2.43.0

[PATCH 4/5] arm_mpam: prevent MPAM-Fb accesses inside IRQ handler

[Andre Przywara]

When an MPAM MSC gets into an error condition, it can trigger an error
IRQ. We cannot really do much about those errors, but we at least query
and log the error, then disable MPAM functionality.

This error report relies on reading the MSC's error status register
(ESR) in the IRQ handler, which is not possible for MPAM-Fb based
MSC accesses, since they involve mailbox routines that might sleep.

For simplicity just skip the ESR read when the MSC is not using direct
MMIO accesses. We will wrap up MPAM functionality regardless, knowing
the exact error value will not change that.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/resctrl/mpam_devices.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index 90b49bfd5caa..62aa04cb6905 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -2472,7 +2472,7 @@ static int mpam_disable_msc_ecr(void *_msc)
 
 static irqreturn_t __mpam_irq_handler(int irq, struct mpam_msc *msc)
 {
-	u64 reg;
+	u64 reg = 0;
 	u16 partid;
 	u8 errcode, pmg, ris;
 
@@ -2481,22 +2481,30 @@ static irqreturn_t __mpam_irq_handler(int irq, struct mpam_msc *msc)
 					   &msc->accessibility)))
 		return IRQ_NONE;
 
-	reg = mpam_msc_read_esr(msc);
+	/* MPAM-Fb MSC accesses cannot be done in atomic context. */
+	if (msc->iface == MPAM_IFACE_MMIO) {
+		reg = mpam_msc_read_esr(msc);
 
-	errcode = FIELD_GET(MPAMF_ESR_ERRCODE, reg);
-	if (!errcode)
-		return IRQ_NONE;
+		errcode = FIELD_GET(MPAMF_ESR_ERRCODE, reg);
+		if (!errcode)
+			return IRQ_NONE;
+	}
 
 	/* Clear level triggered irq */
 	mpam_msc_clear_esr(msc);
 
-	partid = FIELD_GET(MPAMF_ESR_PARTID_MON, reg);
-	pmg = FIELD_GET(MPAMF_ESR_PMG, reg);
-	ris = FIELD_GET(MPAMF_ESR_RIS, reg);
+	if (reg) {
+		partid = FIELD_GET(MPAMF_ESR_PARTID_MON, reg);
+		pmg = FIELD_GET(MPAMF_ESR_PMG, reg);
+		ris = FIELD_GET(MPAMF_ESR_RIS, reg);
+
+		pr_err_ratelimited("error irq from msc:%u '%s', partid:%u, pmg: %u, ris: %u\n",
+				   msc->id, mpam_errcode_names[errcode], partid,
+				   pmg, ris);
+	} else {
+		pr_err_ratelimited("unknown error irq from msc:%u\n", msc->id);
+	}
 
-	pr_err_ratelimited("error irq from msc:%u '%s', partid:%u, pmg: %u, ris: %u\n",
-			   msc->id, mpam_errcode_names[errcode], partid, pmg,
-			   ris);
 
 	/* Disable this interrupt. */
 	mpam_disable_msc_ecr(msc);
-- 
2.43.0

[PATCH 5/5] arm_mpam: detect and enable MPAM-Fb PCC support

[Andre Przywara]

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.
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      |  2 ++
 drivers/resctrl/mpam_devices.c | 46 +++++++++++++++++++++++++++++++---
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/arm64/mpam.c b/drivers/acpi/arm64/mpam.c
index 99c2bdbb3314..edb4d10e8dc3 100644
--- a/drivers/acpi/arm64/mpam.c
+++ b/drivers/acpi/arm64/mpam.c
@@ -341,6 +341,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 62aa04cb6905..6f0d0959d3a4 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -19,6 +19,7 @@
 #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>
@@ -27,6 +28,9 @@
 #include <linux/types.h>
 #include <linux/workqueue.h>
 
+#include <acpi/pcc.h>
+#include <acpi/acpi_io.h>
+
 #include "mpam_internal.h"
 #include "mpam_fb.h"
 
@@ -1042,7 +1046,8 @@ static u64 mpam_msc_read_mbwu_l(struct mpam_msc *msc)
 
 	mpam_mon_sel_lock_held(msc);
 
-	WARN_ON_ONCE((MSMON_MBWU_L + sizeof(u64)) > msc->mapped_hwpage_sz);
+	if (msc->iface == MPAM_IFACE_MMIO)
+		WARN_ON_ONCE((MSMON_MBWU_L + sizeof(u64)) > msc->mapped_hwpage_sz);
 	WARN_ON_ONCE(!cpumask_test_cpu(smp_processor_id(), &msc->accessibility));
 
 	mbwu_l_high2 = __mpam_read_reg(msc, MSMON_MBWU_L + 4);
@@ -2042,10 +2047,15 @@ static void mpam_msc_drv_remove(struct platform_device *pdev)
 	mpam_free_garbage();
 }
 
+static void mpam_pcc_rx_callback(struct mbox_client *cl, void *msg)
+{
+	/* TODO: wake up tasks blocked on this MSC's PCC channel */
+}
+
 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;
@@ -2090,7 +2100,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;
@@ -2106,6 +2117,35 @@ 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;
+
+		msc->pcc_cl.dev = &pdev->dev;
+		msc->pcc_cl.rx_callback = mpam_pcc_rx_callback;
+		msc->pcc_cl.tx_block = false;
+		msc->pcc_cl.tx_tout = 1000; /* 1s */
+		msc->pcc_cl.knows_txdone = false;
+
+		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 = pcc_mbox_request_channel(&msc->pcc_cl,
+							 pcc_subspace_id);
+		if (IS_ERR(msc->pcc_chan)) {
+			pr_err("Failed to request MSC PCC channel\n");
+			return (void *)msc->pcc_chan;
+		}
+
+		if (msc->pcc_chan->shmem_size < MPAM_FB_MAX_MSG_SIZE) {
+			pr_err("MPAM-Fb PCC channel size too small.\n");
+			pcc_mbox_free_channel(msc->pcc_chan);
+			return ERR_PTR(-ENOMEM);
+		}
+
+		mutex_init(&msc->pcc_chan_lock);
 	} else {
 		return ERR_PTR(-EINVAL);
 	}
-- 
2.43.0

Re: [PATCH 5/5] arm_mpam: detect and enable MPAM-Fb PCC support

[Sudeep Holla]

On Wed, Apr 29, 2026 at 04:13:39PM +0200, Andre Przywara wrote:
> 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.
> 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      |  2 ++
>  drivers/resctrl/mpam_devices.c | 46 +++++++++++++++++++++++++++++++---
>  2 files changed, 45 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/arm64/mpam.c b/drivers/acpi/arm64/mpam.c
> index 99c2bdbb3314..edb4d10e8dc3 100644
> --- a/drivers/acpi/arm64/mpam.c
> +++ b/drivers/acpi/arm64/mpam.c
> @@ -341,6 +341,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);

I may be looking at the wrong documents, but neither DEN0065 nor DEN0144 carry
any definitions of pcc-channel and msc-id for the device with HID
"“ARMHAA5C". Since "pcc-channel" is already merged, I think I am looking at
wrong documents, please point me to the right one.

-- 
Regards,
Sudeep

Re: [PATCH 5/5] arm_mpam: detect and enable MPAM-Fb PCC support

[Andre Przywara]

Hi Sudeep,

thanks for having a look!

On 4/30/26 10:35, Sudeep Holla wrote:
> On Wed, Apr 29, 2026 at 04:13:39PM +0200, Andre Przywara wrote:
>> 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.
>> 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      |  2 ++
>>   drivers/resctrl/mpam_devices.c | 46 +++++++++++++++++++++++++++++++---
>>   2 files changed, 45 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/arm64/mpam.c b/drivers/acpi/arm64/mpam.c
>> index 99c2bdbb3314..edb4d10e8dc3 100644
>> --- a/drivers/acpi/arm64/mpam.c
>> +++ b/drivers/acpi/arm64/mpam.c
>> @@ -341,6 +341,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);
> 
> I may be looking at the wrong documents, but neither DEN0065 nor DEN0144 carry
> any definitions of pcc-channel and msc-id for the device with HID
> "“ARMHAA5C". Since "pcc-channel" is already merged, I think I am looking at
> wrong documents, please point me to the right one.

Please excuse my ignorance, but I was under the assumption that the 
strings used here are just unique identifiers that need to match the 
property_get calls in the MPAM code. Is there any requirement to match 
those property_entry.name fields with the names given in some spec? And 
those strings are kernel-internal only, right? But for DT would match 
exactly the property names?

Those properties correspond to fields in table 4 in DEN0065, as also 
described in struct acpi_mpam_msc_node in include/acpi/actbl2.h:
- There is "Identifier", that uses distinct IDs for PCC and native MSCs. 
I named it msc-id, because I'd assume that to be a good name for any 
(yet to be defined) DT property.
- Then there is "Base address", which doubles as the "subspace ID of the 
PCC channel", when MPAM-Fb is used. As you mentioned, this is already 
in, and the original spec name would be very misleading, I think.

So shall those strings be renamed to match the struct names? Or shall I 
keep the more readable names as of now, and add comments linking them to 
the spec/struct?

Cheers,
Andre

Re: [PATCH 5/5] arm_mpam: detect and enable MPAM-Fb PCC support

[Sudeep Holla]

On Thu, Apr 30, 2026 at 11:20:42AM +0200, Andre Przywara wrote:
> Hi Sudeep,
> 
> thanks for having a look!
> 
> On 4/30/26 10:35, Sudeep Holla wrote:
> > On Wed, Apr 29, 2026 at 04:13:39PM +0200, Andre Przywara wrote:
> > > 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.
> > > 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      |  2 ++
> > >   drivers/resctrl/mpam_devices.c | 46 +++++++++++++++++++++++++++++++---
> > >   2 files changed, 45 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/acpi/arm64/mpam.c b/drivers/acpi/arm64/mpam.c
> > > index 99c2bdbb3314..edb4d10e8dc3 100644
> > > --- a/drivers/acpi/arm64/mpam.c
> > > +++ b/drivers/acpi/arm64/mpam.c
> > > @@ -341,6 +341,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);
> > 
> > I may be looking at the wrong documents, but neither DEN0065 nor DEN0144 carry
> > any definitions of pcc-channel and msc-id for the device with HID
> > "“ARMHAA5C". Since "pcc-channel" is already merged, I think I am looking at
> > wrong documents, please point me to the right one.
> 
> Please excuse my ignorance, but I was under the assumption that the strings
> used here are just unique identifiers that need to match the property_get
> calls in the MPAM code. Is there any requirement to match those
> property_entry.name fields with the names given in some spec? And those
> strings are kernel-internal only, right? But for DT would match exactly the
> property names?
> 

My bad, I missed to see that these are properties added to the software node
created for ACPI. I assumed the ACPI code is expecting this property in the
namespace device with HID 'ARMHAA5C'. Ignore my comment.

-- 
Regards,
Sudeep