[PATCH 11/33] arm_mpam: Add the class and component structures for firmware described ris

Ben Horgan posted 33 patches 3 months ago
There is a newer version of this series
[PATCH 11/33] arm_mpam: Add the class and component structures for firmware described ris
Posted by Ben Horgan 3 months ago
From: James Morse <james.morse@arm.com>

An MSC is a container of resources, each identified by their RIS index.
Some RIS are described by firmware to provide their position in the system.
Others are discovered when the driver probes the hardware.

To configure a resource it needs to be found by its class, e.g. 'L2'.
There are two kinds of grouping, a class is a set of components, which
are visible to user-space as there are likely to be multiple instances
of the L2 cache. (e.g. one per cluster or package)

Add support for creating and destroying structures to allow a hierarchy
of resources to be created.

CC: Ben Horgan <ben.horgan@arm.com>
Tested-by: Fenghua Yu <fenghuay@nvidia.com>
Tested-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
Tested-by: Peter Newman <peternewman@google.com>
Signed-off-by: James Morse <james.morse@arm.com>
Signed-off-by: Ben Horgan <ben.horgan@arm.com>
---
Changes since v3:
Jonathan:
Code reordering.
Comments.
---
 drivers/resctrl/mpam_devices.c  | 393 +++++++++++++++++++++++++++++++-
 drivers/resctrl/mpam_internal.h |  94 ++++++++
 include/linux/arm_mpam.h        |   5 +
 3 files changed, 491 insertions(+), 1 deletion(-)

diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index 6c6be133d73a..48a344d5cb43 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -36,6 +36,384 @@ struct srcu_struct mpam_srcu;
  */
 static atomic_t mpam_num_msc;
 
+/*
+ * An MSC is a physical container for controls and monitors, each identified by
+ * their RIS index. These share a base-address, interrupts and some MMIO
+ * registers. A vMSC is a virtual container for RIS in an MSC that control or
+ * monitor the same thing. Members of a vMSC are all RIS in the same MSC, but
+ * not all RIS in an MSC share a vMSC.
+ * Components are a group of vMSC that control or monitor the same thing but
+ * are from different MSC, so have different base-address, interrupts etc.
+ * Classes are the set components of the same type.
+ *
+ * The features of a vMSC is the union of the RIS it contains.
+ * The features of a Class and Component are the common subset of the vMSC
+ * they contain.
+ *
+ * e.g. The system cache may have bandwidth controls on multiple interfaces,
+ * for regulating traffic from devices independently of traffic from CPUs.
+ * If these are two RIS in one MSC, they will be treated as controlling
+ * different things, and will not share a vMSC/component/class.
+ *
+ * e.g. The L2 may have one MSC and two RIS, one for cache-controls another
+ * for bandwidth. These two RIS are members of the same vMSC.
+ *
+ * e.g. The set of RIS that make up the L2 are grouped as a component. These
+ * are sometimes termed slices. They should be configured the same, as if there
+ * were only one.
+ *
+ * e.g. The SoC probably has more than one L2, each attached to a distinct set
+ * of CPUs. All the L2 components are grouped as a class.
+ *
+ * When creating an MSC, struct mpam_msc is added to the all mpam_all_msc list,
+ * then linked via struct mpam_ris to a vmsc, component and class.
+ * The same MSC may exist under different class->component->vmsc paths, but the
+ * RIS index will be unique.
+ */
+LIST_HEAD(mpam_classes);
+
+/* List of all objects that can be free()d after synchronise_srcu() */
+static LLIST_HEAD(mpam_garbage);
+
+static inline void init_garbage(struct mpam_garbage *garbage)
+{
+	init_llist_node(&garbage->llist);
+}
+
+#define add_to_garbage(x)				\
+do {							\
+	__typeof__(x) _x = (x);				\
+	_x->garbage.to_free = _x;			\
+	llist_add(&_x->garbage.llist, &mpam_garbage);	\
+} while (0)
+
+static void mpam_free_garbage(void)
+{
+	struct mpam_garbage *iter, *tmp;
+	struct llist_node *to_free = llist_del_all(&mpam_garbage);
+
+	if (!to_free)
+		return;
+
+	synchronize_srcu(&mpam_srcu);
+
+	llist_for_each_entry_safe(iter, tmp, to_free, llist) {
+		if (iter->pdev)
+			devm_kfree(&iter->pdev->dev, iter->to_free);
+		else
+			kfree(iter->to_free);
+	}
+}
+
+static struct mpam_vmsc *
+mpam_vmsc_alloc(struct mpam_component *comp, struct mpam_msc *msc)
+{
+	struct mpam_vmsc *vmsc;
+
+	lockdep_assert_held(&mpam_list_lock);
+
+	vmsc = kzalloc(sizeof(*vmsc), GFP_KERNEL);
+	if (!vmsc)
+		return ERR_PTR(-ENOMEM);
+	init_garbage(&vmsc->garbage);
+
+	INIT_LIST_HEAD_RCU(&vmsc->ris);
+	INIT_LIST_HEAD_RCU(&vmsc->comp_list);
+	vmsc->comp = comp;
+	vmsc->msc = msc;
+
+	list_add_rcu(&vmsc->comp_list, &comp->vmsc);
+
+	return vmsc;
+}
+
+static void mpam_component_destroy(struct mpam_component *comp);
+
+static void mpam_vmsc_destroy(struct mpam_vmsc *vmsc)
+{
+	struct mpam_component *comp = vmsc->comp;
+
+	lockdep_assert_held(&mpam_list_lock);
+
+	list_del_rcu(&vmsc->comp_list);
+	add_to_garbage(vmsc);
+
+	if (list_empty(&comp->vmsc))
+		mpam_component_destroy(comp);
+}
+
+static struct mpam_vmsc *
+mpam_vmsc_find(struct mpam_component *comp, struct mpam_msc *msc)
+{
+	struct mpam_vmsc *vmsc;
+
+	lockdep_assert_held(&mpam_list_lock);
+
+	list_for_each_entry(vmsc, &comp->vmsc, comp_list) {
+		if (vmsc->msc->id == msc->id)
+			return vmsc;
+	}
+
+	return mpam_vmsc_alloc(comp, msc);
+}
+
+static struct mpam_component *
+mpam_component_alloc(struct mpam_class *class, int id)
+{
+	struct mpam_component *comp;
+
+	lockdep_assert_held(&mpam_list_lock);
+
+	comp = kzalloc(sizeof(*comp), GFP_KERNEL);
+	if (!comp)
+		return ERR_PTR(-ENOMEM);
+	init_garbage(&comp->garbage);
+
+	comp->comp_id = id;
+	INIT_LIST_HEAD_RCU(&comp->vmsc);
+	/* affinity is updated when ris are added */
+	INIT_LIST_HEAD_RCU(&comp->class_list);
+	comp->class = class;
+
+	list_add_rcu(&comp->class_list, &class->components);
+
+	return comp;
+}
+
+static void mpam_class_destroy(struct mpam_class *class);
+
+static void mpam_component_destroy(struct mpam_component *comp)
+{
+	struct mpam_class *class = comp->class;
+
+	lockdep_assert_held(&mpam_list_lock);
+
+	list_del_rcu(&comp->class_list);
+	add_to_garbage(comp);
+
+	if (list_empty(&class->components))
+		mpam_class_destroy(class);
+}
+
+static struct mpam_component *
+mpam_component_find(struct mpam_class *class, int id)
+{
+	struct mpam_component *comp;
+
+	lockdep_assert_held(&mpam_list_lock);
+
+	list_for_each_entry(comp, &class->components, class_list) {
+		if (comp->comp_id == id)
+			return comp;
+	}
+
+	return mpam_component_alloc(class, id);
+}
+
+static struct mpam_class *
+mpam_class_alloc(u8 level_idx, enum mpam_class_types type)
+{
+	struct mpam_class *class;
+
+	lockdep_assert_held(&mpam_list_lock);
+
+	class = kzalloc(sizeof(*class), GFP_KERNEL);
+	if (!class)
+		return ERR_PTR(-ENOMEM);
+	init_garbage(&class->garbage);
+
+	INIT_LIST_HEAD_RCU(&class->components);
+	/* affinity is updated when ris are added */
+	class->level = level_idx;
+	class->type = type;
+	INIT_LIST_HEAD_RCU(&class->classes_list);
+
+	list_add_rcu(&class->classes_list, &mpam_classes);
+
+	return class;
+}
+
+static void mpam_class_destroy(struct mpam_class *class)
+{
+	lockdep_assert_held(&mpam_list_lock);
+
+	list_del_rcu(&class->classes_list);
+	add_to_garbage(class);
+}
+
+static struct mpam_class *
+mpam_class_find(u8 level_idx, enum mpam_class_types type)
+{
+	struct mpam_class *class;
+
+	lockdep_assert_held(&mpam_list_lock);
+
+	list_for_each_entry(class, &mpam_classes, classes_list) {
+		if (class->type == type && class->level == level_idx)
+			return class;
+	}
+
+	return mpam_class_alloc(level_idx, type);
+}
+
+/*
+ * The cacheinfo structures are only populated when CPUs are online.
+ * This helper walks the acpi tables to include offline CPUs too.
+ */
+int mpam_get_cpumask_from_cache_id(unsigned long cache_id, u32 cache_level,
+				   cpumask_t *affinity)
+{
+	return acpi_pptt_get_cpumask_from_cache_id(cache_id, affinity);
+}
+
+/*
+ * cpumask_of_node() only knows about online CPUs. This can't tell us whether
+ * a class is represented on all possible CPUs.
+ */
+static void get_cpumask_from_node_id(u32 node_id, cpumask_t *affinity)
+{
+	int cpu;
+
+	for_each_possible_cpu(cpu) {
+		if (node_id == cpu_to_node(cpu))
+			cpumask_set_cpu(cpu, affinity);
+	}
+}
+
+static int mpam_ris_get_affinity(struct mpam_msc *msc, cpumask_t *affinity,
+				 enum mpam_class_types type,
+				 struct mpam_class *class,
+				 struct mpam_component *comp)
+{
+	int err;
+
+	switch (type) {
+	case MPAM_CLASS_CACHE:
+		err = mpam_get_cpumask_from_cache_id(comp->comp_id, class->level,
+						     affinity);
+		if (err)
+			return err;
+
+		if (cpumask_empty(affinity))
+			dev_warn_once(&msc->pdev->dev,
+				      "no CPUs associated with cache node\n");
+
+		break;
+	case MPAM_CLASS_MEMORY:
+		get_cpumask_from_node_id(comp->comp_id, affinity);
+		/* affinity may be empty for CPU-less memory nodes */
+		break;
+	case MPAM_CLASS_UNKNOWN:
+		return 0;
+	}
+
+	cpumask_and(affinity, affinity, &msc->accessibility);
+
+	return 0;
+}
+
+static int mpam_ris_create_locked(struct mpam_msc *msc, u8 ris_idx,
+				  enum mpam_class_types type, u8 class_id,
+				  int component_id)
+{
+	int err;
+	struct mpam_vmsc *vmsc;
+	struct mpam_msc_ris *ris;
+	struct mpam_class *class;
+	struct mpam_component *comp;
+	struct platform_device *pdev = msc->pdev;
+
+	lockdep_assert_held(&mpam_list_lock);
+
+	if (ris_idx > MPAM_MSC_MAX_NUM_RIS)
+		return -EINVAL;
+
+	if (test_and_set_bit(ris_idx, &msc->ris_idxs))
+		return -EBUSY;
+
+	ris = devm_kzalloc(&msc->pdev->dev, sizeof(*ris), GFP_KERNEL);
+	if (!ris)
+		return -ENOMEM;
+	init_garbage(&ris->garbage);
+	ris->garbage.pdev = pdev;
+
+	class = mpam_class_find(class_id, type);
+	if (IS_ERR(class))
+		return PTR_ERR(class);
+
+	comp = mpam_component_find(class, component_id);
+	if (IS_ERR(comp)) {
+		if (list_empty(&class->components))
+			mpam_class_destroy(class);
+		return PTR_ERR(comp);
+	}
+
+	vmsc = mpam_vmsc_find(comp, msc);
+	if (IS_ERR(vmsc)) {
+		if (list_empty(&comp->vmsc))
+			mpam_component_destroy(comp);
+		return PTR_ERR(vmsc);
+	}
+
+	err = mpam_ris_get_affinity(msc, &ris->affinity, type, class, comp);
+	if (err) {
+		if (list_empty(&vmsc->ris))
+			mpam_vmsc_destroy(vmsc);
+		return err;
+	}
+
+	ris->ris_idx = ris_idx;
+	INIT_LIST_HEAD_RCU(&ris->msc_list);
+	INIT_LIST_HEAD_RCU(&ris->vmsc_list);
+	ris->vmsc = vmsc;
+
+	cpumask_or(&comp->affinity, &comp->affinity, &ris->affinity);
+	cpumask_or(&class->affinity, &class->affinity, &ris->affinity);
+	list_add_rcu(&ris->vmsc_list, &vmsc->ris);
+	list_add_rcu(&ris->msc_list, &msc->ris);
+
+	return 0;
+}
+
+static void mpam_ris_destroy(struct mpam_msc_ris *ris)
+{
+	struct mpam_vmsc *vmsc = ris->vmsc;
+	struct mpam_msc *msc = vmsc->msc;
+	struct mpam_component *comp = vmsc->comp;
+	struct mpam_class *class = comp->class;
+
+	lockdep_assert_held(&mpam_list_lock);
+
+	/*
+	 * It is assumed affinities don't overlap. If they do the class becomes
+	 * unusable immediately.
+	 */
+	cpumask_andnot(&class->affinity, &class->affinity, &ris->affinity);
+	cpumask_andnot(&comp->affinity, &comp->affinity, &ris->affinity);
+	clear_bit(ris->ris_idx, &msc->ris_idxs);
+	list_del_rcu(&ris->msc_list);
+	list_del_rcu(&ris->vmsc_list);
+	add_to_garbage(ris);
+
+	if (list_empty(&vmsc->ris))
+		mpam_vmsc_destroy(vmsc);
+}
+
+int mpam_ris_create(struct mpam_msc *msc, u8 ris_idx,
+		    enum mpam_class_types type, u8 class_id, int component_id)
+{
+	int err;
+
+	mutex_lock(&mpam_list_lock);
+	err = mpam_ris_create_locked(msc, ris_idx, type, class_id,
+				     component_id);
+	mutex_unlock(&mpam_list_lock);
+	if (err)
+		mpam_free_garbage();
+
+	return err;
+}
+
 /*
  * An MSC can control traffic from a set of CPUs, but may only be accessible
  * from a (hopefully wider) set of CPUs. The common reason for this is power
@@ -60,14 +438,25 @@ static int update_msc_accessibility(struct mpam_msc *msc)
 
 static int fw_num_msc;
 
+/*
+ * There are two ways of reaching a struct mpam_msc_ris. Via the
+ * class->component->vmsc->ris, or via the msc.
+ * When destroying the msc, the other side needs unlinking and cleaning up too.
+ */
 static void mpam_msc_destroy(struct mpam_msc *msc)
 {
 	struct platform_device *pdev = msc->pdev;
+	struct mpam_msc_ris *ris, *tmp;
 
 	lockdep_assert_held(&mpam_list_lock);
 
+	list_for_each_entry_safe(ris, tmp, &msc->ris, msc_list)
+		mpam_ris_destroy(ris);
+
 	list_del_rcu(&msc->all_msc_list);
 	platform_set_drvdata(pdev, NULL);
+
+	add_to_garbage(msc);
 }
 
 static void mpam_msc_drv_remove(struct platform_device *pdev)
@@ -81,7 +470,7 @@ static void mpam_msc_drv_remove(struct platform_device *pdev)
 	mpam_msc_destroy(msc);
 	mutex_unlock(&mpam_list_lock);
 
-	synchronize_srcu(&mpam_srcu);
+	mpam_free_garbage();
 }
 
 static struct mpam_msc *do_mpam_msc_drv_probe(struct platform_device *pdev)
@@ -97,6 +486,8 @@ static struct mpam_msc *do_mpam_msc_drv_probe(struct platform_device *pdev)
 	msc = devm_kzalloc(&pdev->dev, sizeof(*msc), GFP_KERNEL);
 	if (!msc)
 		return ERR_PTR(-ENOMEM);
+	init_garbage(&msc->garbage);
+	msc->garbage.pdev = pdev;
 
 	err = devm_mutex_init(dev, &msc->probe_lock);
 	if (err)
diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
index 540066903eca..8f7a28d2c021 100644
--- a/drivers/resctrl/mpam_internal.h
+++ b/drivers/resctrl/mpam_internal.h
@@ -7,11 +7,30 @@
 #include <linux/arm_mpam.h>
 #include <linux/cpumask.h>
 #include <linux/io.h>
+#include <linux/llist.h>
 #include <linux/mutex.h>
+#include <linux/srcu.h>
 #include <linux/types.h>
 
+#define MPAM_MSC_MAX_NUM_RIS	16
+
 struct platform_device;
 
+/*
+ * Structures protected by SRCU may not be freed for a surprising amount of
+ * time (especially if perf is running). To ensure the MPAM error interrupt can
+ * tear down all the structures, build a list of objects that can be garbage
+ * collected once synchronize_srcu() has returned.
+ * If pdev is non-NULL, use devm_kfree().
+ */
+struct mpam_garbage {
+	/* member of mpam_garbage */
+	struct llist_node	llist;
+
+	void			*to_free;
+	struct platform_device	*pdev;
+};
+
 struct mpam_msc {
 	/* member of mpam_all_msc */
 	struct list_head	all_msc_list;
@@ -45,5 +64,80 @@ struct mpam_msc {
 
 	void __iomem		*mapped_hwpage;
 	size_t			mapped_hwpage_sz;
+
+	struct mpam_garbage	garbage;
+};
+
+struct mpam_class {
+	/* mpam_components in this class */
+	struct list_head	components;
+
+	cpumask_t		affinity;
+
+	u8			level;
+	enum mpam_class_types	type;
+
+	/* member of mpam_classes */
+	struct list_head	classes_list;
+
+	struct mpam_garbage	garbage;
+};
+
+struct mpam_component {
+	u32			comp_id;
+
+	/* mpam_vmsc in this component */
+	struct list_head	vmsc;
+
+	cpumask_t		affinity;
+
+	/* member of mpam_class:components */
+	struct list_head	class_list;
+
+	/* parent: */
+	struct mpam_class	*class;
+
+	struct mpam_garbage	garbage;
+};
+
+struct mpam_vmsc {
+	/* member of mpam_component:vmsc_list */
+	struct list_head	comp_list;
+
+	/* mpam_msc_ris in this vmsc */
+	struct list_head	ris;
+
+	/* All RIS in this vMSC are members of this MSC */
+	struct mpam_msc		*msc;
+
+	/* parent: */
+	struct mpam_component	*comp;
+
+	struct mpam_garbage	garbage;
+};
+
+struct mpam_msc_ris {
+	u8			ris_idx;
+
+	cpumask_t		affinity;
+
+	/* member of mpam_vmsc:ris */
+	struct list_head	vmsc_list;
+
+	/* member of mpam_msc:ris */
+	struct list_head	msc_list;
+
+	/* parent: */
+	struct mpam_vmsc	*vmsc;
+
+	struct mpam_garbage	garbage;
 };
+
+/* List of all classes - protected by srcu*/
+extern struct srcu_struct mpam_srcu;
+extern struct list_head mpam_classes;
+
+int mpam_get_cpumask_from_cache_id(unsigned long cache_id, u32 cache_level,
+				   cpumask_t *affinity);
+
 #endif /* MPAM_INTERNAL_H */
diff --git a/include/linux/arm_mpam.h b/include/linux/arm_mpam.h
index a3828ef91aee..5a3aab6bb1d4 100644
--- a/include/linux/arm_mpam.h
+++ b/include/linux/arm_mpam.h
@@ -37,11 +37,16 @@ static inline int acpi_mpam_parse_resources(struct mpam_msc *msc,
 static inline int acpi_mpam_count_msc(void) { return -EINVAL; }
 #endif
 
+#ifdef CONFIG_ARM64_MPAM_DRIVER
+int mpam_ris_create(struct mpam_msc *msc, u8 ris_idx,
+		    enum mpam_class_types type, u8 class_id, int component_id);
+#else
 static inline int mpam_ris_create(struct mpam_msc *msc, u8 ris_idx,
 				  enum mpam_class_types type, u8 class_id,
 				  int component_id)
 {
 	return -EINVAL;
 }
+#endif
 
 #endif /* __LINUX_ARM_MPAM_H */
-- 
2.43.0
Re: [PATCH 11/33] arm_mpam: Add the class and component structures for firmware described ris
Posted by Fenghua Yu 2 months, 4 weeks ago
Hi, Ben and James,

On 11/7/25 04:34, Ben Horgan wrote:
> From: James Morse <james.morse@arm.com>

[SNIP]

> +static int mpam_ris_create_locked(struct mpam_msc *msc, u8 ris_idx,
> +				  enum mpam_class_types type, u8 class_id,
> +				  int component_id)
> +{
> +	int err;
> +	struct mpam_vmsc *vmsc;
> +	struct mpam_msc_ris *ris;
> +	struct mpam_class *class;
> +	struct mpam_component *comp;
> +	struct platform_device *pdev = msc->pdev;
> +
> +	lockdep_assert_held(&mpam_list_lock);
> +
> +	if (ris_idx > MPAM_MSC_MAX_NUM_RIS)
> +		return -EINVAL;
> +
> +	if (test_and_set_bit(ris_idx, &msc->ris_idxs))
> +		return -EBUSY;
> +
> +	ris = devm_kzalloc(&msc->pdev->dev, sizeof(*ris), GFP_KERNEL);
> +	if (!ris)
> +		return -ENOMEM;

The ris_idx bit in msc->ris_idxs is not cleared on error paths in this 
function. The bit cannot be set again.

Not sure if this is a real problem in any case. Clearing the bit on 
error paths may be clean code.

[SNIP]

Thanks.

-Fenghua
Re: [PATCH 11/33] arm_mpam: Add the class and component structures for firmware described ris
Posted by Ben Horgan 2 months, 4 weeks ago
Hi Fenghua,

On 11/13/25 03:23, Fenghua Yu wrote:
> Hi, Ben and James,
> 
> On 11/7/25 04:34, Ben Horgan wrote:
>> From: James Morse <james.morse@arm.com>
> 
> [SNIP]
> 
>> +static int mpam_ris_create_locked(struct mpam_msc *msc, u8 ris_idx,
>> +                  enum mpam_class_types type, u8 class_id,
>> +                  int component_id)
>> +{
>> +    int err;
>> +    struct mpam_vmsc *vmsc;
>> +    struct mpam_msc_ris *ris;
>> +    struct mpam_class *class;
>> +    struct mpam_component *comp;
>> +    struct platform_device *pdev = msc->pdev;
>> +
>> +    lockdep_assert_held(&mpam_list_lock);
>> +
>> +    if (ris_idx > MPAM_MSC_MAX_NUM_RIS)
>> +        return -EINVAL;
>> +
>> +    if (test_and_set_bit(ris_idx, &msc->ris_idxs))
>> +        return -EBUSY;
>> +
>> +    ris = devm_kzalloc(&msc->pdev->dev, sizeof(*ris), GFP_KERNEL);
>> +    if (!ris)
>> +        return -ENOMEM;
> 
> The ris_idx bit in msc->ris_idxs is not cleared on error paths in this
> function. The bit cannot be set again.
> 
> Not sure if this is a real problem in any case. Clearing the bit on
> error paths may be clean code.

I think this would just add noise. There is no anticipation of any
functionality after failing to create RIS and I expect there are more
places in the code where this assumption is relied on.

> 
> [SNIP]
> 
> Thanks.
> 
> -Fenghua

Thanks,

Ben

Re: [PATCH 11/33] arm_mpam: Add the class and component structures for firmware described ris
Posted by Jonathan Cameron 3 months ago
On Fri,  7 Nov 2025 12:34:28 +0000
Ben Horgan <ben.horgan@arm.com> wrote:

> From: James Morse <james.morse@arm.com>
> 
> An MSC is a container of resources, each identified by their RIS index.
> Some RIS are described by firmware to provide their position in the system.
> Others are discovered when the driver probes the hardware.
> 
> To configure a resource it needs to be found by its class, e.g. 'L2'.
> There are two kinds of grouping, a class is a set of components, which
> are visible to user-space as there are likely to be multiple instances
> of the L2 cache. (e.g. one per cluster or package)
> 
> Add support for creating and destroying structures to allow a hierarchy
> of resources to be created.
> 
> CC: Ben Horgan <ben.horgan@arm.com>
Hi Ben,

Remember to clear out CC'ing yourself.

> Tested-by: Fenghua Yu <fenghuay@nvidia.com>
> Tested-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
> Tested-by: Peter Newman <peternewman@google.com>
> Signed-off-by: James Morse <james.morse@arm.com>
> Signed-off-by: Ben Horgan <ben.horgan@arm.com>
> ---
> Changes since v3:
> Jonathan:
> Code reordering.

I'm guessing I may have sent things in a slightly less than ideal directly.

Why can't we have ordering as follows (with no forwards declarations)

mpam_class_alloc()
mpam_class_destroy()
//maybe other mpam_class stuff here
mpam_component_alloc()
mpam_component_destroy() - needs mpam_class_destroy()
//maybe other mpam_component stuff here
mpam_vmsc_alloc()
mpam_vmsc_destroy() - needs mpam_component_destroy()
//other mpam_vmsc here
mpam_ris_create_locked() - needs all the destroys.
mpam_ris_destroy() - needs mpam vmsc_destroy()

I may well have missed a more complex dependency chain.

Other than that, LGTM. Given any change in ordering can be trivially verified
by building it and Gavin's comments seem simple to resolve.

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


> Comments.
> ---
>  drivers/resctrl/mpam_devices.c  | 393 +++++++++++++++++++++++++++++++-
>  drivers/resctrl/mpam_internal.h |  94 ++++++++
>  include/linux/arm_mpam.h        |   5 +
>  3 files changed, 491 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 6c6be133d73a..48a344d5cb43 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -36,6 +36,384 @@ struct srcu_struct mpam_srcu;
>   */
>  static atomic_t mpam_num_msc;
>  
> +/*
> + * An MSC is a physical container for controls and monitors, each identified by
> + * their RIS index. These share a base-address, interrupts and some MMIO
> + * registers. A vMSC is a virtual container for RIS in an MSC that control or
> + * monitor the same thing. Members of a vMSC are all RIS in the same MSC, but
> + * not all RIS in an MSC share a vMSC.
> + * Components are a group of vMSC that control or monitor the same thing but
> + * are from different MSC, so have different base-address, interrupts etc.
> + * Classes are the set components of the same type.
> + *
> + * The features of a vMSC is the union of the RIS it contains.
> + * The features of a Class and Component are the common subset of the vMSC
> + * they contain.
> + *
> + * e.g. The system cache may have bandwidth controls on multiple interfaces,
> + * for regulating traffic from devices independently of traffic from CPUs.
> + * If these are two RIS in one MSC, they will be treated as controlling
> + * different things, and will not share a vMSC/component/class.
> + *
> + * e.g. The L2 may have one MSC and two RIS, one for cache-controls another
> + * for bandwidth. These two RIS are members of the same vMSC.
> + *
> + * e.g. The set of RIS that make up the L2 are grouped as a component. These
> + * are sometimes termed slices. They should be configured the same, as if there
> + * were only one.
> + *
> + * e.g. The SoC probably has more than one L2, each attached to a distinct set
> + * of CPUs. All the L2 components are grouped as a class.
> + *
> + * When creating an MSC, struct mpam_msc is added to the all mpam_all_msc list,
> + * then linked via struct mpam_ris to a vmsc, component and class.
> + * The same MSC may exist under different class->component->vmsc paths, but the
> + * RIS index will be unique.
> + */
> +LIST_HEAD(mpam_classes);
> +
> +/* List of all objects that can be free()d after synchronise_srcu() */
> +static LLIST_HEAD(mpam_garbage);
> +
> +static inline void init_garbage(struct mpam_garbage *garbage)
> +{
> +	init_llist_node(&garbage->llist);
> +}
> +
> +#define add_to_garbage(x)				\
> +do {							\
> +	__typeof__(x) _x = (x);				\
> +	_x->garbage.to_free = _x;			\
> +	llist_add(&_x->garbage.llist, &mpam_garbage);	\
> +} while (0)
> +
> +static void mpam_free_garbage(void)
> +{
> +	struct mpam_garbage *iter, *tmp;
> +	struct llist_node *to_free = llist_del_all(&mpam_garbage);
> +
> +	if (!to_free)
> +		return;
> +
> +	synchronize_srcu(&mpam_srcu);
> +
> +	llist_for_each_entry_safe(iter, tmp, to_free, llist) {
> +		if (iter->pdev)
> +			devm_kfree(&iter->pdev->dev, iter->to_free);
> +		else
> +			kfree(iter->to_free);
> +	}
> +}
> +
> +static struct mpam_vmsc *
> +mpam_vmsc_alloc(struct mpam_component *comp, struct mpam_msc *msc)
> +{
> +	struct mpam_vmsc *vmsc;
> +
> +	lockdep_assert_held(&mpam_list_lock);
> +
> +	vmsc = kzalloc(sizeof(*vmsc), GFP_KERNEL);
> +	if (!vmsc)
> +		return ERR_PTR(-ENOMEM);
> +	init_garbage(&vmsc->garbage);
> +
> +	INIT_LIST_HEAD_RCU(&vmsc->ris);
> +	INIT_LIST_HEAD_RCU(&vmsc->comp_list);
> +	vmsc->comp = comp;
> +	vmsc->msc = msc;
> +
> +	list_add_rcu(&vmsc->comp_list, &comp->vmsc);
> +
> +	return vmsc;
> +}
> +
> +static void mpam_component_destroy(struct mpam_component *comp);
> +
> +static void mpam_vmsc_destroy(struct mpam_vmsc *vmsc)
> +{
> +	struct mpam_component *comp = vmsc->comp;
> +
> +	lockdep_assert_held(&mpam_list_lock);
> +
> +	list_del_rcu(&vmsc->comp_list);
> +	add_to_garbage(vmsc);
> +
> +	if (list_empty(&comp->vmsc))
> +		mpam_component_destroy(comp);
> +}
> +
> +static struct mpam_vmsc *
> +mpam_vmsc_find(struct mpam_component *comp, struct mpam_msc *msc)
> +{
> +	struct mpam_vmsc *vmsc;
> +
> +	lockdep_assert_held(&mpam_list_lock);
> +
> +	list_for_each_entry(vmsc, &comp->vmsc, comp_list) {
> +		if (vmsc->msc->id == msc->id)
> +			return vmsc;
> +	}
> +
> +	return mpam_vmsc_alloc(comp, msc);
> +}
> +
> +static struct mpam_component *
> +mpam_component_alloc(struct mpam_class *class, int id)
> +{
> +	struct mpam_component *comp;
> +
> +	lockdep_assert_held(&mpam_list_lock);
> +
> +	comp = kzalloc(sizeof(*comp), GFP_KERNEL);
> +	if (!comp)
> +		return ERR_PTR(-ENOMEM);
> +	init_garbage(&comp->garbage);
> +
> +	comp->comp_id = id;
> +	INIT_LIST_HEAD_RCU(&comp->vmsc);
> +	/* affinity is updated when ris are added */
> +	INIT_LIST_HEAD_RCU(&comp->class_list);
> +	comp->class = class;
> +
> +	list_add_rcu(&comp->class_list, &class->components);
> +
> +	return comp;
> +}
Re: [PATCH 11/33] arm_mpam: Add the class and component structures for firmware described ris
Posted by Ben Horgan 2 months, 4 weeks ago
Hi Jonathan,

On 11/10/25 17:10, Jonathan Cameron wrote:
> On Fri,  7 Nov 2025 12:34:28 +0000
> Ben Horgan <ben.horgan@arm.com> wrote:
> 
>> From: James Morse <james.morse@arm.com>
>>
>> An MSC is a container of resources, each identified by their RIS index.
>> Some RIS are described by firmware to provide their position in the system.
>> Others are discovered when the driver probes the hardware.
>>
>> To configure a resource it needs to be found by its class, e.g. 'L2'.
>> There are two kinds of grouping, a class is a set of components, which
>> are visible to user-space as there are likely to be multiple instances
>> of the L2 cache. (e.g. one per cluster or package)
>>
>> Add support for creating and destroying structures to allow a hierarchy
>> of resources to be created.
>>
>> CC: Ben Horgan <ben.horgan@arm.com>
> Hi Ben,
> 
> Remember to clear out CC'ing yourself.
> 
>> Tested-by: Fenghua Yu <fenghuay@nvidia.com>
>> Tested-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
>> Tested-by: Peter Newman <peternewman@google.com>
>> Signed-off-by: James Morse <james.morse@arm.com>
>> Signed-off-by: Ben Horgan <ben.horgan@arm.com>
>> ---
>> Changes since v3:
>> Jonathan:
>> Code reordering.
> 
> I'm guessing I may have sent things in a slightly less than ideal directly.
> 
> Why can't we have ordering as follows (with no forwards declarations)
> 
> mpam_class_alloc()
> mpam_class_destroy()
> //maybe other mpam_class stuff here
> mpam_component_alloc()
> mpam_component_destroy() - needs mpam_class_destroy()
> //maybe other mpam_component stuff here
> mpam_vmsc_alloc()
> mpam_vmsc_destroy() - needs mpam_component_destroy()
> //other mpam_vmsc here

This works and then I need to add mpam_ris_get_affinity() as
mpam_ris_create_locked() depends on it.

I also add the helper functions it depends on
mpam_get_cpumask_from_cache_id() and get_cpumask_from_node_id().

> mpam_ris_create_locked() - needs all the destroys.
> mpam_ris_destroy() - needs mpam vmsc_destroy()

> 
> I may well have missed a more complex dependency chain.
> 
> Other than that, LGTM. Given any change in ordering can be trivially verified
> by building it and Gavin's comments seem simple to resolve.
> 
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

Thanks,

Ben
Re: [PATCH 11/33] arm_mpam: Add the class and component structures for firmware described ris
Posted by Gavin Shan 3 months ago
Hi Ben,

On 11/7/25 10:34 PM, Ben Horgan wrote:
> From: James Morse <james.morse@arm.com>
> 
> An MSC is a container of resources, each identified by their RIS index.
> Some RIS are described by firmware to provide their position in the system.
> Others are discovered when the driver probes the hardware.
> 
> To configure a resource it needs to be found by its class, e.g. 'L2'.
> There are two kinds of grouping, a class is a set of components, which
> are visible to user-space as there are likely to be multiple instances
> of the L2 cache. (e.g. one per cluster or package)
> 
> Add support for creating and destroying structures to allow a hierarchy
> of resources to be created.
> 
> CC: Ben Horgan <ben.horgan@arm.com>
> Tested-by: Fenghua Yu <fenghuay@nvidia.com>
> Tested-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
> Tested-by: Peter Newman <peternewman@google.com>
> Signed-off-by: James Morse <james.morse@arm.com>
> Signed-off-by: Ben Horgan <ben.horgan@arm.com>
> ---
> Changes since v3:
> Jonathan:
> Code reordering.
> Comments.
> ---
>   drivers/resctrl/mpam_devices.c  | 393 +++++++++++++++++++++++++++++++-
>   drivers/resctrl/mpam_internal.h |  94 ++++++++
>   include/linux/arm_mpam.h        |   5 +
>   3 files changed, 491 insertions(+), 1 deletion(-)
> 

Some minor comments below and some of them may be invalid. Nothing really
looks incorrect to me:

Reviewed-by: Gavin Shan <gshan@redhat.com>

> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 6c6be133d73a..48a344d5cb43 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -36,6 +36,384 @@ struct srcu_struct mpam_srcu;
>    */
>   static atomic_t mpam_num_msc;
>   
> +/*
> + * An MSC is a physical container for controls and monitors, each identified by
> + * their RIS index. These share a base-address, interrupts and some MMIO
> + * registers. A vMSC is a virtual container for RIS in an MSC that control or
> + * monitor the same thing. Members of a vMSC are all RIS in the same MSC, but
> + * not all RIS in an MSC share a vMSC.

s/a virtual container for RIS/a virtual container for RISes
s/all RIS/all RISes

An empty line may be needed here as paragraph separator.

> + * Components are a group of vMSC that control or monitor the same thing but
> + * are from different MSC, so have different base-address, interrupts etc.
> + * Classes are the set components of the same type.
> + *
> + * The features of a vMSC is the union of the RIS it contains.
> + * The features of a Class and Component are the common subset of the vMSC
> + * they contain.
> + *

s/the RIS/the RISes

> + * e.g. The system cache may have bandwidth controls on multiple interfaces,
> + * for regulating traffic from devices independently of traffic from CPUs.
> + * If these are two RIS in one MSC, they will be treated as controlling
> + * different things, and will not share a vMSC/component/class.
> + *
> + * e.g. The L2 may have one MSC and two RIS, one for cache-controls another
> + * for bandwidth. These two RIS are members of the same vMSC.
> + *
> + * e.g. The set of RIS that make up the L2 are grouped as a component. These
> + * are sometimes termed slices. They should be configured the same, as if there
> + * were only one.
> + *
> + * e.g. The SoC probably has more than one L2, each attached to a distinct set
> + * of CPUs. All the L2 components are grouped as a class.
> + *
> + * When creating an MSC, struct mpam_msc is added to the all mpam_all_msc list,
> + * then linked via struct mpam_ris to a vmsc, component and class.
> + * The same MSC may exist under different class->component->vmsc paths, but the
> + * RIS index will be unique.
> + */
> +LIST_HEAD(mpam_classes);
> +
> +/* List of all objects that can be free()d after synchronise_srcu() */
> +static LLIST_HEAD(mpam_garbage);
> +
> +static inline void init_garbage(struct mpam_garbage *garbage)
> +{
> +	init_llist_node(&garbage->llist);
> +}
> +
> +#define add_to_garbage(x)				\
> +do {							\
> +	__typeof__(x) _x = (x);				\
> +	_x->garbage.to_free = _x;			\
> +	llist_add(&_x->garbage.llist, &mpam_garbage);	\
> +} while (0)
> +
> +static void mpam_free_garbage(void)
> +{
> +	struct mpam_garbage *iter, *tmp;
> +	struct llist_node *to_free = llist_del_all(&mpam_garbage);
> +
> +	if (!to_free)
> +		return;
> +
> +	synchronize_srcu(&mpam_srcu);
> +
> +	llist_for_each_entry_safe(iter, tmp, to_free, llist) {
> +		if (iter->pdev)
> +			devm_kfree(&iter->pdev->dev, iter->to_free);
> +		else
> +			kfree(iter->to_free);
> +	}
> +}
> +
> +static struct mpam_vmsc *
> +mpam_vmsc_alloc(struct mpam_component *comp, struct mpam_msc *msc)
> +{
> +	struct mpam_vmsc *vmsc;
> +
> +	lockdep_assert_held(&mpam_list_lock);
> +
> +	vmsc = kzalloc(sizeof(*vmsc), GFP_KERNEL);
> +	if (!vmsc)
> +		return ERR_PTR(-ENOMEM);
> +	init_garbage(&vmsc->garbage);
> +
> +	INIT_LIST_HEAD_RCU(&vmsc->ris);
> +	INIT_LIST_HEAD_RCU(&vmsc->comp_list);
> +	vmsc->comp = comp;
> +	vmsc->msc = msc;
> +
> +	list_add_rcu(&vmsc->comp_list, &comp->vmsc);
> +
> +	return vmsc;
> +}
> +
> +static void mpam_component_destroy(struct mpam_component *comp);
> +
> +static void mpam_vmsc_destroy(struct mpam_vmsc *vmsc)
> +{
> +	struct mpam_component *comp = vmsc->comp;
> +
> +	lockdep_assert_held(&mpam_list_lock);
> +
> +	list_del_rcu(&vmsc->comp_list);
> +	add_to_garbage(vmsc);
> +
> +	if (list_empty(&comp->vmsc))
> +		mpam_component_destroy(comp);
> +}
> +
> +static struct mpam_vmsc *
> +mpam_vmsc_find(struct mpam_component *comp, struct mpam_msc *msc)
> +{
> +	struct mpam_vmsc *vmsc;
> +
> +	lockdep_assert_held(&mpam_list_lock);
> +
> +	list_for_each_entry(vmsc, &comp->vmsc, comp_list) {
> +		if (vmsc->msc->id == msc->id)
> +			return vmsc;
> +	}
> +
> +	return mpam_vmsc_alloc(comp, msc);
> +}
> +
> +static struct mpam_component *
> +mpam_component_alloc(struct mpam_class *class, int id)
> +{
> +	struct mpam_component *comp;
> +
> +	lockdep_assert_held(&mpam_list_lock);
> +
> +	comp = kzalloc(sizeof(*comp), GFP_KERNEL);
> +	if (!comp)
> +		return ERR_PTR(-ENOMEM);
> +	init_garbage(&comp->garbage);
> +
> +	comp->comp_id = id;
> +	INIT_LIST_HEAD_RCU(&comp->vmsc);
> +	/* affinity is updated when ris are added */

	/* Affinity is updated when RIS is added */

> +	INIT_LIST_HEAD_RCU(&comp->class_list);
> +	comp->class = class;
> +
> +	list_add_rcu(&comp->class_list, &class->components);
> +
> +	return comp;
> +}
> +
> +static void mpam_class_destroy(struct mpam_class *class);
> +
> +static void mpam_component_destroy(struct mpam_component *comp)
> +{
> +	struct mpam_class *class = comp->class;
> +
> +	lockdep_assert_held(&mpam_list_lock);
> +
> +	list_del_rcu(&comp->class_list);
> +	add_to_garbage(comp);
> +
> +	if (list_empty(&class->components))
> +		mpam_class_destroy(class);
> +}
> +
> +static struct mpam_component *
> +mpam_component_find(struct mpam_class *class, int id)
> +{
> +	struct mpam_component *comp;
> +
> +	lockdep_assert_held(&mpam_list_lock);
> +
> +	list_for_each_entry(comp, &class->components, class_list) {
> +		if (comp->comp_id == id)
> +			return comp;
> +	}
> +
> +	return mpam_component_alloc(class, id);
> +}
> +
> +static struct mpam_class *
> +mpam_class_alloc(u8 level_idx, enum mpam_class_types type)
> +{
> +	struct mpam_class *class;
> +
> +	lockdep_assert_held(&mpam_list_lock);
> +
> +	class = kzalloc(sizeof(*class), GFP_KERNEL);
> +	if (!class)
> +		return ERR_PTR(-ENOMEM);
> +	init_garbage(&class->garbage);
> +
> +	INIT_LIST_HEAD_RCU(&class->components);
> +	/* affinity is updated when ris are added */

	/* Affinity is updated when RIS is added */

> +	class->level = level_idx;
> +	class->type = type;
> +	INIT_LIST_HEAD_RCU(&class->classes_list);
> +
> +	list_add_rcu(&class->classes_list, &mpam_classes);
> +
> +	return class;
> +}
> +
> +static void mpam_class_destroy(struct mpam_class *class)
> +{
> +	lockdep_assert_held(&mpam_list_lock);
> +
> +	list_del_rcu(&class->classes_list);
> +	add_to_garbage(class);
> +}
> +
> +static struct mpam_class *
> +mpam_class_find(u8 level_idx, enum mpam_class_types type)
> +{
> +	struct mpam_class *class;
> +
> +	lockdep_assert_held(&mpam_list_lock);
> +
> +	list_for_each_entry(class, &mpam_classes, classes_list) {
> +		if (class->type == type && class->level == level_idx)
> +			return class;
> +	}
> +
> +	return mpam_class_alloc(level_idx, type);
> +}
> +
> +/*
> + * The cacheinfo structures are only populated when CPUs are online.
> + * This helper walks the acpi tables to include offline CPUs too.
> + */
> +int mpam_get_cpumask_from_cache_id(unsigned long cache_id, u32 cache_level,
> +				   cpumask_t *affinity)
> +{
> +	return acpi_pptt_get_cpumask_from_cache_id(cache_id, affinity);
> +}
> +

This function is only used in mpam_devices.c and won't be exposed in the
future, we can make it 'static' and 'inline'.

> +/*
> + * cpumask_of_node() only knows about online CPUs. This can't tell us whether
> + * a class is represented on all possible CPUs.
> + */
> +static void get_cpumask_from_node_id(u32 node_id, cpumask_t *affinity)
> +{
> +	int cpu;
> +
> +	for_each_possible_cpu(cpu) {
> +		if (node_id == cpu_to_node(cpu))
> +			cpumask_set_cpu(cpu, affinity);
> +	}
> +}
> +
> +static int mpam_ris_get_affinity(struct mpam_msc *msc, cpumask_t *affinity,
> +				 enum mpam_class_types type,
> +				 struct mpam_class *class,
> +				 struct mpam_component *comp)
> +{
> +	int err;
> +
> +	switch (type) {
> +	case MPAM_CLASS_CACHE:
> +		err = mpam_get_cpumask_from_cache_id(comp->comp_id, class->level,
> +						     affinity);
> +		if (err)
> +			return err;

It's worthy to add a warning message here.

> +
> +		if (cpumask_empty(affinity))
> +			dev_warn_once(&msc->pdev->dev,
> +				      "no CPUs associated with cache node\n");

{} is needed here.

> +
> +		break;
> +	case MPAM_CLASS_MEMORY:
> +		get_cpumask_from_node_id(comp->comp_id, affinity);
> +		/* affinity may be empty for CPU-less memory nodes */
> +		break;
> +	case MPAM_CLASS_UNKNOWN:
> +		return 0;
> +	}
> +
> +	cpumask_and(affinity, affinity, &msc->accessibility);
> +
> +	return 0;
> +}
> +
> +static int mpam_ris_create_locked(struct mpam_msc *msc, u8 ris_idx,
> +				  enum mpam_class_types type, u8 class_id,
> +				  int component_id)
> +{
> +	int err;
> +	struct mpam_vmsc *vmsc;
> +	struct mpam_msc_ris *ris;
> +	struct mpam_class *class;
> +	struct mpam_component *comp;
> +	struct platform_device *pdev = msc->pdev;
> +
> +	lockdep_assert_held(&mpam_list_lock);
> +
> +	if (ris_idx > MPAM_MSC_MAX_NUM_RIS)
> +		return -EINVAL;
> +
> +	if (test_and_set_bit(ris_idx, &msc->ris_idxs))
> +		return -EBUSY;
> +
> +	ris = devm_kzalloc(&msc->pdev->dev, sizeof(*ris), GFP_KERNEL);
> +	if (!ris)
> +		return -ENOMEM;
> +	init_garbage(&ris->garbage);
> +	ris->garbage.pdev = pdev;
> +
> +	class = mpam_class_find(class_id, type);
> +	if (IS_ERR(class))
> +		return PTR_ERR(class);
> +
> +	comp = mpam_component_find(class, component_id);
> +	if (IS_ERR(comp)) {
> +		if (list_empty(&class->components))
> +			mpam_class_destroy(class);
> +		return PTR_ERR(comp);
> +	}
> +
> +	vmsc = mpam_vmsc_find(comp, msc);
> +	if (IS_ERR(vmsc)) {
> +		if (list_empty(&comp->vmsc))
> +			mpam_component_destroy(comp);
> +		return PTR_ERR(vmsc);
> +	}
> +
> +	err = mpam_ris_get_affinity(msc, &ris->affinity, type, class, comp);
> +	if (err) {
> +		if (list_empty(&vmsc->ris))
> +			mpam_vmsc_destroy(vmsc);
> +		return err;
> +	}
> +
> +	ris->ris_idx = ris_idx;
> +	INIT_LIST_HEAD_RCU(&ris->msc_list);
> +	INIT_LIST_HEAD_RCU(&ris->vmsc_list);
> +	ris->vmsc = vmsc;
> +
> +	cpumask_or(&comp->affinity, &comp->affinity, &ris->affinity);
> +	cpumask_or(&class->affinity, &class->affinity, &ris->affinity);
> +	list_add_rcu(&ris->vmsc_list, &vmsc->ris);
> +	list_add_rcu(&ris->msc_list, &msc->ris);
> +
> +	return 0;
> +}
> +
> +static void mpam_ris_destroy(struct mpam_msc_ris *ris)
> +{
> +	struct mpam_vmsc *vmsc = ris->vmsc;
> +	struct mpam_msc *msc = vmsc->msc;
> +	struct mpam_component *comp = vmsc->comp;
> +	struct mpam_class *class = comp->class;
> +
> +	lockdep_assert_held(&mpam_list_lock);
> +
> +	/*
> +	 * It is assumed affinities don't overlap. If they do the class becomes
> +	 * unusable immediately.
> +	 */
> +	cpumask_andnot(&class->affinity, &class->affinity, &ris->affinity);
> +	cpumask_andnot(&comp->affinity, &comp->affinity, &ris->affinity);
> +	clear_bit(ris->ris_idx, &msc->ris_idxs);
> +	list_del_rcu(&ris->msc_list);
> +	list_del_rcu(&ris->vmsc_list);
> +	add_to_garbage(ris);
> +
> +	if (list_empty(&vmsc->ris))
> +		mpam_vmsc_destroy(vmsc);
> +}
> +
> +int mpam_ris_create(struct mpam_msc *msc, u8 ris_idx,
> +		    enum mpam_class_types type, u8 class_id, int component_id)
> +{
> +	int err;
> +
> +	mutex_lock(&mpam_list_lock);
> +	err = mpam_ris_create_locked(msc, ris_idx, type, class_id,
> +				     component_id);
> +	mutex_unlock(&mpam_list_lock);
> +	if (err)
> +		mpam_free_garbage();
> +
> +	return err;
> +}
> +
>   /*
>    * An MSC can control traffic from a set of CPUs, but may only be accessible
>    * from a (hopefully wider) set of CPUs. The common reason for this is power
> @@ -60,14 +438,25 @@ static int update_msc_accessibility(struct mpam_msc *msc)
>   
>   static int fw_num_msc;
>   
> +/*
> + * There are two ways of reaching a struct mpam_msc_ris. Via the
> + * class->component->vmsc->ris, or via the msc.
> + * When destroying the msc, the other side needs unlinking and cleaning up too.
> + */
>   static void mpam_msc_destroy(struct mpam_msc *msc)
>   {
>   	struct platform_device *pdev = msc->pdev;
> +	struct mpam_msc_ris *ris, *tmp;
>   
>   	lockdep_assert_held(&mpam_list_lock);
>   
> +	list_for_each_entry_safe(ris, tmp, &msc->ris, msc_list)
> +		mpam_ris_destroy(ris);
> +
>   	list_del_rcu(&msc->all_msc_list);
>   	platform_set_drvdata(pdev, NULL);
> +
> +	add_to_garbage(msc);
>   }
>   
>   static void mpam_msc_drv_remove(struct platform_device *pdev)
> @@ -81,7 +470,7 @@ static void mpam_msc_drv_remove(struct platform_device *pdev)
>   	mpam_msc_destroy(msc);
>   	mutex_unlock(&mpam_list_lock);
>   
> -	synchronize_srcu(&mpam_srcu);
> +	mpam_free_garbage();
>   }
>   
>   static struct mpam_msc *do_mpam_msc_drv_probe(struct platform_device *pdev)
> @@ -97,6 +486,8 @@ static struct mpam_msc *do_mpam_msc_drv_probe(struct platform_device *pdev)
>   	msc = devm_kzalloc(&pdev->dev, sizeof(*msc), GFP_KERNEL);
>   	if (!msc)
>   		return ERR_PTR(-ENOMEM);
> +	init_garbage(&msc->garbage);
> +	msc->garbage.pdev = pdev;
>   
>   	err = devm_mutex_init(dev, &msc->probe_lock);
>   	if (err)
> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
> index 540066903eca..8f7a28d2c021 100644
> --- a/drivers/resctrl/mpam_internal.h
> +++ b/drivers/resctrl/mpam_internal.h
> @@ -7,11 +7,30 @@
>   #include <linux/arm_mpam.h>
>   #include <linux/cpumask.h>
>   #include <linux/io.h>
> +#include <linux/llist.h>
>   #include <linux/mutex.h>
> +#include <linux/srcu.h>
>   #include <linux/types.h>
>   
> +#define MPAM_MSC_MAX_NUM_RIS	16
> +
>   struct platform_device;
>   
> +/*
> + * Structures protected by SRCU may not be freed for a surprising amount of
> + * time (especially if perf is running). To ensure the MPAM error interrupt can
> + * tear down all the structures, build a list of objects that can be garbage
> + * collected once synchronize_srcu() has returned.
> + * If pdev is non-NULL, use devm_kfree().
> + */
> +struct mpam_garbage {
> +	/* member of mpam_garbage */
> +	struct llist_node	llist;
> +
> +	void			*to_free;
> +	struct platform_device	*pdev;
> +};
> +
>   struct mpam_msc {
>   	/* member of mpam_all_msc */
>   	struct list_head	all_msc_list;
> @@ -45,5 +64,80 @@ struct mpam_msc {
>   
>   	void __iomem		*mapped_hwpage;
>   	size_t			mapped_hwpage_sz;
> +
> +	struct mpam_garbage	garbage;
> +};
> +
> +struct mpam_class {
> +	/* mpam_components in this class */
> +	struct list_head	components;
> +
> +	cpumask_t		affinity;
> +
> +	u8			level;
> +	enum mpam_class_types	type;
> +
> +	/* member of mpam_classes */
> +	struct list_head	classes_list;
> +
> +	struct mpam_garbage	garbage;
> +};
> +
> +struct mpam_component {
> +	u32			comp_id;
> +
> +	/* mpam_vmsc in this component */
> +	struct list_head	vmsc;
> +
> +	cpumask_t		affinity;
> +
> +	/* member of mpam_class:components */
> +	struct list_head	class_list;
> +
> +	/* parent: */
> +	struct mpam_class	*class;
> +
> +	struct mpam_garbage	garbage;
> +};
> +
> +struct mpam_vmsc {
> +	/* member of mpam_component:vmsc_list */
> +	struct list_head	comp_list;
> +
> +	/* mpam_msc_ris in this vmsc */
> +	struct list_head	ris;
> +
> +	/* All RIS in this vMSC are members of this MSC */
> +	struct mpam_msc		*msc;
> +
> +	/* parent: */
> +	struct mpam_component	*comp;
> +
> +	struct mpam_garbage	garbage;
> +};
> +
> +struct mpam_msc_ris {
> +	u8			ris_idx;
> +
> +	cpumask_t		affinity;
> +
> +	/* member of mpam_vmsc:ris */
> +	struct list_head	vmsc_list;
> +
> +	/* member of mpam_msc:ris */
> +	struct list_head	msc_list;
> +
> +	/* parent: */
> +	struct mpam_vmsc	*vmsc;
> +
> +	struct mpam_garbage	garbage;
>   };
> +
> +/* List of all classes - protected by srcu*/
> +extern struct srcu_struct mpam_srcu;
> +extern struct list_head mpam_classes;
> +
> +int mpam_get_cpumask_from_cache_id(unsigned long cache_id, u32 cache_level,
> +				   cpumask_t *affinity);
> +
>   #endif /* MPAM_INTERNAL_H */
> diff --git a/include/linux/arm_mpam.h b/include/linux/arm_mpam.h
> index a3828ef91aee..5a3aab6bb1d4 100644
> --- a/include/linux/arm_mpam.h
> +++ b/include/linux/arm_mpam.h
> @@ -37,11 +37,16 @@ static inline int acpi_mpam_parse_resources(struct mpam_msc *msc,
>   static inline int acpi_mpam_count_msc(void) { return -EINVAL; }
>   #endif
>   
> +#ifdef CONFIG_ARM64_MPAM_DRIVER
> +int mpam_ris_create(struct mpam_msc *msc, u8 ris_idx,
> +		    enum mpam_class_types type, u8 class_id, int component_id);
> +#else
>   static inline int mpam_ris_create(struct mpam_msc *msc, u8 ris_idx,
>   				  enum mpam_class_types type, u8 class_id,
>   				  int component_id)
>   {
>   	return -EINVAL;
>   }
> +#endif
>   
>   #endif /* __LINUX_ARM_MPAM_H */

Thanks,
Gavin
Re: [PATCH 11/33] arm_mpam: Add the class and component structures for firmware described ris
Posted by Ben Horgan 2 months, 4 weeks ago
Hi Gavin,

On 11/9/25 00:07, Gavin Shan wrote:
> Hi Ben,
> 
> On 11/7/25 10:34 PM, Ben Horgan wrote:
>> From: James Morse <james.morse@arm.com>
>>
>> An MSC is a container of resources, each identified by their RIS index.
>> Some RIS are described by firmware to provide their position in the
>> system.
>> Others are discovered when the driver probes the hardware.
>>
>> To configure a resource it needs to be found by its class, e.g. 'L2'.
>> There are two kinds of grouping, a class is a set of components, which
>> are visible to user-space as there are likely to be multiple instances
>> of the L2 cache. (e.g. one per cluster or package)
>>
>> Add support for creating and destroying structures to allow a hierarchy
>> of resources to be created.
>>
>> CC: Ben Horgan <ben.horgan@arm.com>
>> Tested-by: Fenghua Yu <fenghuay@nvidia.com>
>> Tested-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
>> Tested-by: Peter Newman <peternewman@google.com>
>> Signed-off-by: James Morse <james.morse@arm.com>
>> Signed-off-by: Ben Horgan <ben.horgan@arm.com>
>> ---
>> Changes since v3:
>> Jonathan:
>> Code reordering.
>> Comments.
>> ---
>>   drivers/resctrl/mpam_devices.c  | 393 +++++++++++++++++++++++++++++++-
>>   drivers/resctrl/mpam_internal.h |  94 ++++++++
>>   include/linux/arm_mpam.h        |   5 +
>>   3 files changed, 491 insertions(+), 1 deletion(-)
>>
> 
> Some minor comments below and some of them may be invalid. Nothing really
> looks incorrect to me:
> 
> Reviewed-by: Gavin Shan <gshan@redhat.com>
> 
>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/
>> mpam_devices.c
>> index 6c6be133d73a..48a344d5cb43 100644
>> --- a/drivers/resctrl/mpam_devices.c
>> +++ b/drivers/resctrl/mpam_devices.c
>> @@ -36,6 +36,384 @@ struct srcu_struct mpam_srcu;
>>    */
>>   static atomic_t mpam_num_msc;
>>   +/*
>> + * An MSC is a physical container for controls and monitors, each
>> identified by
>> + * their RIS index. These share a base-address, interrupts and some MMIO
>> + * registers. A vMSC is a virtual container for RIS in an MSC that
>> control or
>> + * monitor the same thing. Members of a vMSC are all RIS in the same
>> MSC, but
>> + * not all RIS in an MSC share a vMSC.
> 
> s/a virtual container for RIS/a virtual container for RISes
> s/all RIS/all RISes

The comments/message in the driver treat RIS is as an invariable noun;
that is the singular and plural are the same. This makes sense when read
out loud. I'll keep it as is unless you think making a singular/plural
disctinction makes it significantly easier to understand.

> 
> An empty line may be needed here as paragraph separator.

Done

> 
>> + * Components are a group of vMSC that control or monitor the same
>> thing but
>> + * are from different MSC, so have different base-address, interrupts
>> etc.
>> + * Classes are the set components of the same type.
>> + *
>> + * The features of a vMSC is the union of the RIS it contains.
>> + * The features of a Class and Component are the common subset of the
>> vMSC
>> + * they contain.
>> + *
> 
> s/the RIS/the RISes
> 
>> + * e.g. The system cache may have bandwidth controls on multiple
>> interfaces,
>> + * for regulating traffic from devices independently of traffic from
>> CPUs.
>> + * If these are two RIS in one MSC, they will be treated as controlling
>> + * different things, and will not share a vMSC/component/class.
>> + *
>> + * e.g. The L2 may have one MSC and two RIS, one for cache-controls
>> another
>> + * for bandwidth. These two RIS are members of the same vMSC.
>> + *
>> + * e.g. The set of RIS that make up the L2 are grouped as a
>> component. These
>> + * are sometimes termed slices. They should be configured the same,
>> as if there
>> + * were only one.
>> + *
>> + * e.g. The SoC probably has more than one L2, each attached to a
>> distinct set
>> + * of CPUs. All the L2 components are grouped as a class.
>> + *
>> + * When creating an MSC, struct mpam_msc is added to the all
>> mpam_all_msc list,
>> + * then linked via struct mpam_ris to a vmsc, component and class.
>> + * The same MSC may exist under different class->component->vmsc
>> paths, but the
>> + * RIS index will be unique.
>> + */
>> +LIST_HEAD(mpam_classes);
>> +
>> +/* List of all objects that can be free()d after synchronise_srcu() */
>> +static LLIST_HEAD(mpam_garbage);
>> +
>> +static inline void init_garbage(struct mpam_garbage *garbage)
>> +{
>> +    init_llist_node(&garbage->llist);
>> +}
>> +
>> +#define add_to_garbage(x)                \
>> +do {                            \
>> +    __typeof__(x) _x = (x);                \
>> +    _x->garbage.to_free = _x;            \
>> +    llist_add(&_x->garbage.llist, &mpam_garbage);    \
>> +} while (0)
>> +
>> +static void mpam_free_garbage(void)
>> +{
>> +    struct mpam_garbage *iter, *tmp;
>> +    struct llist_node *to_free = llist_del_all(&mpam_garbage);
>> +
>> +    if (!to_free)
>> +        return;
>> +
>> +    synchronize_srcu(&mpam_srcu);
>> +
>> +    llist_for_each_entry_safe(iter, tmp, to_free, llist) {
>> +        if (iter->pdev)
>> +            devm_kfree(&iter->pdev->dev, iter->to_free);
>> +        else
>> +            kfree(iter->to_free);
>> +    }
>> +}
>> +
>> +static struct mpam_vmsc *
>> +mpam_vmsc_alloc(struct mpam_component *comp, struct mpam_msc *msc)
>> +{
>> +    struct mpam_vmsc *vmsc;
>> +
>> +    lockdep_assert_held(&mpam_list_lock);
>> +
>> +    vmsc = kzalloc(sizeof(*vmsc), GFP_KERNEL);
>> +    if (!vmsc)
>> +        return ERR_PTR(-ENOMEM);
>> +    init_garbage(&vmsc->garbage);
>> +
>> +    INIT_LIST_HEAD_RCU(&vmsc->ris);
>> +    INIT_LIST_HEAD_RCU(&vmsc->comp_list);
>> +    vmsc->comp = comp;
>> +    vmsc->msc = msc;
>> +
>> +    list_add_rcu(&vmsc->comp_list, &comp->vmsc);
>> +
>> +    return vmsc;
>> +}
>> +
>> +static void mpam_component_destroy(struct mpam_component *comp);
>> +
>> +static void mpam_vmsc_destroy(struct mpam_vmsc *vmsc)
>> +{
>> +    struct mpam_component *comp = vmsc->comp;
>> +
>> +    lockdep_assert_held(&mpam_list_lock);
>> +
>> +    list_del_rcu(&vmsc->comp_list);
>> +    add_to_garbage(vmsc);
>> +
>> +    if (list_empty(&comp->vmsc))
>> +        mpam_component_destroy(comp);
>> +}
>> +
>> +static struct mpam_vmsc *
>> +mpam_vmsc_find(struct mpam_component *comp, struct mpam_msc *msc)
>> +{
>> +    struct mpam_vmsc *vmsc;
>> +
>> +    lockdep_assert_held(&mpam_list_lock);
>> +
>> +    list_for_each_entry(vmsc, &comp->vmsc, comp_list) {
>> +        if (vmsc->msc->id == msc->id)
>> +            return vmsc;
>> +    }
>> +
>> +    return mpam_vmsc_alloc(comp, msc);
>> +}
>> +
>> +static struct mpam_component *
>> +mpam_component_alloc(struct mpam_class *class, int id)
>> +{
>> +    struct mpam_component *comp;
>> +
>> +    lockdep_assert_held(&mpam_list_lock);
>> +
>> +    comp = kzalloc(sizeof(*comp), GFP_KERNEL);
>> +    if (!comp)
>> +        return ERR_PTR(-ENOMEM);
>> +    init_garbage(&comp->garbage);
>> +
>> +    comp->comp_id = id;
>> +    INIT_LIST_HEAD_RCU(&comp->vmsc);
>> +    /* affinity is updated when ris are added */
> 
>     /* Affinity is updated when RIS is added */
> 
>> +    INIT_LIST_HEAD_RCU(&comp->class_list);
>> +    comp->class = class;
>> +
>> +    list_add_rcu(&comp->class_list, &class->components);
>> +
>> +    return comp;
>> +}
>> +
>> +static void mpam_class_destroy(struct mpam_class *class);
>> +
>> +static void mpam_component_destroy(struct mpam_component *comp)
>> +{
>> +    struct mpam_class *class = comp->class;
>> +
>> +    lockdep_assert_held(&mpam_list_lock);
>> +
>> +    list_del_rcu(&comp->class_list);
>> +    add_to_garbage(comp);
>> +
>> +    if (list_empty(&class->components))
>> +        mpam_class_destroy(class);
>> +}
>> +
>> +static struct mpam_component *
>> +mpam_component_find(struct mpam_class *class, int id)
>> +{
>> +    struct mpam_component *comp;
>> +
>> +    lockdep_assert_held(&mpam_list_lock);
>> +
>> +    list_for_each_entry(comp, &class->components, class_list) {
>> +        if (comp->comp_id == id)
>> +            return comp;
>> +    }
>> +
>> +    return mpam_component_alloc(class, id);
>> +}
>> +
>> +static struct mpam_class *
>> +mpam_class_alloc(u8 level_idx, enum mpam_class_types type)
>> +{
>> +    struct mpam_class *class;
>> +
>> +    lockdep_assert_held(&mpam_list_lock);
>> +
>> +    class = kzalloc(sizeof(*class), GFP_KERNEL);
>> +    if (!class)
>> +        return ERR_PTR(-ENOMEM);
>> +    init_garbage(&class->garbage);
>> +
>> +    INIT_LIST_HEAD_RCU(&class->components);
>> +    /* affinity is updated when ris are added */
> 
>     /* Affinity is updated when RIS is added */
> 
>> +    class->level = level_idx;
>> +    class->type = type;
>> +    INIT_LIST_HEAD_RCU(&class->classes_list);
>> +
>> +    list_add_rcu(&class->classes_list, &mpam_classes);
>> +
>> +    return class;
>> +}
>> +
>> +static void mpam_class_destroy(struct mpam_class *class)
>> +{
>> +    lockdep_assert_held(&mpam_list_lock);
>> +
>> +    list_del_rcu(&class->classes_list);
>> +    add_to_garbage(class);
>> +}
>> +
>> +static struct mpam_class *
>> +mpam_class_find(u8 level_idx, enum mpam_class_types type)
>> +{
>> +    struct mpam_class *class;
>> +
>> +    lockdep_assert_held(&mpam_list_lock);
>> +
>> +    list_for_each_entry(class, &mpam_classes, classes_list) {
>> +        if (class->type == type && class->level == level_idx)
>> +            return class;
>> +    }
>> +
>> +    return mpam_class_alloc(level_idx, type);
>> +}
>> +
>> +/*
>> + * The cacheinfo structures are only populated when CPUs are online.
>> + * This helper walks the acpi tables to include offline CPUs too.
>> + */
>> +int mpam_get_cpumask_from_cache_id(unsigned long cache_id, u32
>> cache_level,
>> +                   cpumask_t *affinity)
>> +{
>> +    return acpi_pptt_get_cpumask_from_cache_id(cache_id, affinity);
>> +}
>> +
> 
> This function is only used in mpam_devices.c and won't be exposed in the
> future, we can make it 'static' and 'inline'.

Done

> 
>> +/*
>> + * cpumask_of_node() only knows about online CPUs. This can't tell us
>> whether
>> + * a class is represented on all possible CPUs.
>> + */
>> +static void get_cpumask_from_node_id(u32 node_id, cpumask_t *affinity)
>> +{
>> +    int cpu;
>> +
>> +    for_each_possible_cpu(cpu) {
>> +        if (node_id == cpu_to_node(cpu))
>> +            cpumask_set_cpu(cpu, affinity);
>> +    }
>> +}
>> +
>> +static int mpam_ris_get_affinity(struct mpam_msc *msc, cpumask_t
>> *affinity,
>> +                 enum mpam_class_types type,
>> +                 struct mpam_class *class,
>> +                 struct mpam_component *comp)
>> +{
>> +    int err;
>> +
>> +    switch (type) {
>> +    case MPAM_CLASS_CACHE:
>> +        err = mpam_get_cpumask_from_cache_id(comp->comp_id, class-
>> >level,
>> +                             affinity);
>> +        if (err)
>> +            return err;
> 
> It's worthy to add a warning message here.

Added: Failed to determine CPU affinity

> 
>> +
>> +        if (cpumask_empty(affinity))
>> +            dev_warn_once(&msc->pdev->dev,
>> +                      "no CPUs associated with cache node\n");
> 
> {} is needed here.

Made it one line.

> 
>> +
>> +        break;
>> +    case MPAM_CLASS_MEMORY:
>> +        get_cpumask_from_node_id(comp->comp_id, affinity);
>> +        /* affinity may be empty for CPU-less memory nodes */
>> +        break;
>> +    case MPAM_CLASS_UNKNOWN:
>> +        return 0;
>> +    }
>> +
>> +    cpumask_and(affinity, affinity, &msc->accessibility);
>> +
>> +    return 0;
>> +}
>> +
>> +static int mpam_ris_create_locked(struct mpam_msc *msc, u8 ris_idx,
>> +                  enum mpam_class_types type, u8 class_id,
>> +                  int component_id)
>> +{
>> +    int err;
>> +    struct mpam_vmsc *vmsc;
>> +    struct mpam_msc_ris *ris;
>> +    struct mpam_class *class;
>> +    struct mpam_component *comp;
>> +    struct platform_device *pdev = msc->pdev;
>> +
>> +    lockdep_assert_held(&mpam_list_lock);
>> +
>> +    if (ris_idx > MPAM_MSC_MAX_NUM_RIS)
>> +        return -EINVAL;
>> +
>> +    if (test_and_set_bit(ris_idx, &msc->ris_idxs))
>> +        return -EBUSY;
>> +
>> +    ris = devm_kzalloc(&msc->pdev->dev, sizeof(*ris), GFP_KERNEL);
>> +    if (!ris)
>> +        return -ENOMEM;
>> +    init_garbage(&ris->garbage);
>> +    ris->garbage.pdev = pdev;
>> +
>> +    class = mpam_class_find(class_id, type);
>> +    if (IS_ERR(class))
>> +        return PTR_ERR(class);
>> +
>> +    comp = mpam_component_find(class, component_id);
>> +    if (IS_ERR(comp)) {
>> +        if (list_empty(&class->components))
>> +            mpam_class_destroy(class);
>> +        return PTR_ERR(comp);
>> +    }
>> +
>> +    vmsc = mpam_vmsc_find(comp, msc);
>> +    if (IS_ERR(vmsc)) {
>> +        if (list_empty(&comp->vmsc))
>> +            mpam_component_destroy(comp);
>> +        return PTR_ERR(vmsc);
>> +    }
>> +
>> +    err = mpam_ris_get_affinity(msc, &ris->affinity, type, class, comp);
>> +    if (err) {
>> +        if (list_empty(&vmsc->ris))
>> +            mpam_vmsc_destroy(vmsc);
>> +        return err;
>> +    }
>> +
>> +    ris->ris_idx = ris_idx;
>> +    INIT_LIST_HEAD_RCU(&ris->msc_list);
>> +    INIT_LIST_HEAD_RCU(&ris->vmsc_list);
>> +    ris->vmsc = vmsc;
>> +
>> +    cpumask_or(&comp->affinity, &comp->affinity, &ris->affinity);
>> +    cpumask_or(&class->affinity, &class->affinity, &ris->affinity);
>> +    list_add_rcu(&ris->vmsc_list, &vmsc->ris);
>> +    list_add_rcu(&ris->msc_list, &msc->ris);
>> +
>> +    return 0;
>> +}
>> +
>> +static void mpam_ris_destroy(struct mpam_msc_ris *ris)
>> +{
>> +    struct mpam_vmsc *vmsc = ris->vmsc;
>> +    struct mpam_msc *msc = vmsc->msc;
>> +    struct mpam_component *comp = vmsc->comp;
>> +    struct mpam_class *class = comp->class;
>> +
>> +    lockdep_assert_held(&mpam_list_lock);
>> +
>> +    /*
>> +     * It is assumed affinities don't overlap. If they do the class
>> becomes
>> +     * unusable immediately.
>> +     */
>> +    cpumask_andnot(&class->affinity, &class->affinity, &ris->affinity);
>> +    cpumask_andnot(&comp->affinity, &comp->affinity, &ris->affinity);
>> +    clear_bit(ris->ris_idx, &msc->ris_idxs);
>> +    list_del_rcu(&ris->msc_list);
>> +    list_del_rcu(&ris->vmsc_list);
>> +    add_to_garbage(ris);
>> +
>> +    if (list_empty(&vmsc->ris))
>> +        mpam_vmsc_destroy(vmsc);
>> +}
>> +
>> +int mpam_ris_create(struct mpam_msc *msc, u8 ris_idx,
>> +            enum mpam_class_types type, u8 class_id, int component_id)
>> +{
>> +    int err;
>> +
>> +    mutex_lock(&mpam_list_lock);
>> +    err = mpam_ris_create_locked(msc, ris_idx, type, class_id,
>> +                     component_id);
>> +    mutex_unlock(&mpam_list_lock);
>> +    if (err)
>> +        mpam_free_garbage();
>> +
>> +    return err;
>> +}
>> +
>>   /*
>>    * An MSC can control traffic from a set of CPUs, but may only be
>> accessible
>>    * from a (hopefully wider) set of CPUs. The common reason for this
>> is power
>> @@ -60,14 +438,25 @@ static int update_msc_accessibility(struct
>> mpam_msc *msc)
>>     static int fw_num_msc;
>>   +/*
>> + * There are two ways of reaching a struct mpam_msc_ris. Via the
>> + * class->component->vmsc->ris, or via the msc.
>> + * When destroying the msc, the other side needs unlinking and
>> cleaning up too.
>> + */
>>   static void mpam_msc_destroy(struct mpam_msc *msc)
>>   {
>>       struct platform_device *pdev = msc->pdev;
>> +    struct mpam_msc_ris *ris, *tmp;
>>         lockdep_assert_held(&mpam_list_lock);
>>   +    list_for_each_entry_safe(ris, tmp, &msc->ris, msc_list)
>> +        mpam_ris_destroy(ris);
>> +
>>       list_del_rcu(&msc->all_msc_list);
>>       platform_set_drvdata(pdev, NULL);
>> +
>> +    add_to_garbage(msc);
>>   }
>>     static void mpam_msc_drv_remove(struct platform_device *pdev)
>> @@ -81,7 +470,7 @@ static void mpam_msc_drv_remove(struct
>> platform_device *pdev)
>>       mpam_msc_destroy(msc);
>>       mutex_unlock(&mpam_list_lock);
>>   -    synchronize_srcu(&mpam_srcu);
>> +    mpam_free_garbage();
>>   }
>>     static struct mpam_msc *do_mpam_msc_drv_probe(struct
>> platform_device *pdev)
>> @@ -97,6 +486,8 @@ static struct mpam_msc
>> *do_mpam_msc_drv_probe(struct platform_device *pdev)
>>       msc = devm_kzalloc(&pdev->dev, sizeof(*msc), GFP_KERNEL);
>>       if (!msc)
>>           return ERR_PTR(-ENOMEM);
>> +    init_garbage(&msc->garbage);
>> +    msc->garbage.pdev = pdev;
>>         err = devm_mutex_init(dev, &msc->probe_lock);
>>       if (err)
>> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/
>> mpam_internal.h
>> index 540066903eca..8f7a28d2c021 100644
>> --- a/drivers/resctrl/mpam_internal.h
>> +++ b/drivers/resctrl/mpam_internal.h
>> @@ -7,11 +7,30 @@
>>   #include <linux/arm_mpam.h>
>>   #include <linux/cpumask.h>
>>   #include <linux/io.h>
>> +#include <linux/llist.h>
>>   #include <linux/mutex.h>
>> +#include <linux/srcu.h>
>>   #include <linux/types.h>
>>   +#define MPAM_MSC_MAX_NUM_RIS    16
>> +
>>   struct platform_device;
>>   +/*
>> + * Structures protected by SRCU may not be freed for a surprising
>> amount of
>> + * time (especially if perf is running). To ensure the MPAM error
>> interrupt can
>> + * tear down all the structures, build a list of objects that can be
>> garbage
>> + * collected once synchronize_srcu() has returned.
>> + * If pdev is non-NULL, use devm_kfree().
>> + */
>> +struct mpam_garbage {
>> +    /* member of mpam_garbage */
>> +    struct llist_node    llist;
>> +
>> +    void            *to_free;
>> +    struct platform_device    *pdev;
>> +};
>> +
>>   struct mpam_msc {
>>       /* member of mpam_all_msc */
>>       struct list_head    all_msc_list;
>> @@ -45,5 +64,80 @@ struct mpam_msc {
>>         void __iomem        *mapped_hwpage;
>>       size_t            mapped_hwpage_sz;
>> +
>> +    struct mpam_garbage    garbage;
>> +};
>> +
>> +struct mpam_class {
>> +    /* mpam_components in this class */
>> +    struct list_head    components;
>> +
>> +    cpumask_t        affinity;
>> +
>> +    u8            level;
>> +    enum mpam_class_types    type;
>> +
>> +    /* member of mpam_classes */
>> +    struct list_head    classes_list;
>> +
>> +    struct mpam_garbage    garbage;
>> +};
>> +
>> +struct mpam_component {
>> +    u32            comp_id;
>> +
>> +    /* mpam_vmsc in this component */
>> +    struct list_head    vmsc;
>> +
>> +    cpumask_t        affinity;
>> +
>> +    /* member of mpam_class:components */
>> +    struct list_head    class_list;
>> +
>> +    /* parent: */
>> +    struct mpam_class    *class;
>> +
>> +    struct mpam_garbage    garbage;
>> +};
>> +
>> +struct mpam_vmsc {
>> +    /* member of mpam_component:vmsc_list */
>> +    struct list_head    comp_list;
>> +
>> +    /* mpam_msc_ris in this vmsc */
>> +    struct list_head    ris;
>> +
>> +    /* All RIS in this vMSC are members of this MSC */
>> +    struct mpam_msc        *msc;
>> +
>> +    /* parent: */
>> +    struct mpam_component    *comp;
>> +
>> +    struct mpam_garbage    garbage;
>> +};
>> +
>> +struct mpam_msc_ris {
>> +    u8            ris_idx;
>> +
>> +    cpumask_t        affinity;
>> +
>> +    /* member of mpam_vmsc:ris */
>> +    struct list_head    vmsc_list;
>> +
>> +    /* member of mpam_msc:ris */
>> +    struct list_head    msc_list;
>> +
>> +    /* parent: */
>> +    struct mpam_vmsc    *vmsc;
>> +
>> +    struct mpam_garbage    garbage;
>>   };
>> +
>> +/* List of all classes - protected by srcu*/
>> +extern struct srcu_struct mpam_srcu;
>> +extern struct list_head mpam_classes;
>> +
>> +int mpam_get_cpumask_from_cache_id(unsigned long cache_id, u32
>> cache_level,
>> +                   cpumask_t *affinity);
>> +
>>   #endif /* MPAM_INTERNAL_H */
>> diff --git a/include/linux/arm_mpam.h b/include/linux/arm_mpam.h
>> index a3828ef91aee..5a3aab6bb1d4 100644
>> --- a/include/linux/arm_mpam.h
>> +++ b/include/linux/arm_mpam.h
>> @@ -37,11 +37,16 @@ static inline int acpi_mpam_parse_resources(struct
>> mpam_msc *msc,
>>   static inline int acpi_mpam_count_msc(void) { return -EINVAL; }
>>   #endif
>>   +#ifdef CONFIG_ARM64_MPAM_DRIVER
>> +int mpam_ris_create(struct mpam_msc *msc, u8 ris_idx,
>> +            enum mpam_class_types type, u8 class_id, int component_id);
>> +#else
>>   static inline int mpam_ris_create(struct mpam_msc *msc, u8 ris_idx,
>>                     enum mpam_class_types type, u8 class_id,
>>                     int component_id)
>>   {
>>       return -EINVAL;
>>   }
>> +#endif
>>     #endif /* __LINUX_ARM_MPAM_H */
> 
> Thanks,
> Gavin
> 

-- 
Thanks,

Ben

Re: [PATCH 11/33] arm_mpam: Add the class and component structures for firmware described ris
Posted by Ben Horgan 2 months, 4 weeks ago
Hi Gavin,

I was a bit hasty on one of those changes.

On 11/12/25 16:39, Ben Horgan wrote:
> Hi Gavin,
> 
> On 11/9/25 00:07, Gavin Shan wrote:
>> Hi Ben,
>>
>> On 11/7/25 10:34 PM, Ben Horgan wrote:
>>> From: James Morse <james.morse@arm.com>
>>>
>>> An MSC is a container of resources, each identified by their RIS index.
>>> Some RIS are described by firmware to provide their position in the
>>> system.
>>> Others are discovered when the driver probes the hardware.
>>>
>>> To configure a resource it needs to be found by its class, e.g. 'L2'.
>>> There are two kinds of grouping, a class is a set of components, which
>>> are visible to user-space as there are likely to be multiple instances
>>> of the L2 cache. (e.g. one per cluster or package)
>>>
>>> Add support for creating and destroying structures to allow a hierarchy
>>> of resources to be created.
>>>
>>> CC: Ben Horgan <ben.horgan@arm.com>
>>> Tested-by: Fenghua Yu <fenghuay@nvidia.com>
>>> Tested-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
>>> Tested-by: Peter Newman <peternewman@google.com>
>>> Signed-off-by: James Morse <james.morse@arm.com>
>>> Signed-off-by: Ben Horgan <ben.horgan@arm.com>

>> This function is only used in mpam_devices.c and won't be exposed in the
>> future, we can make it 'static' and 'inline'.
> 
> Done

Gets used later in mpam_resctl.c so I'll keep as is.

Thanks,

Ben