Re: [PATCH v2 15/45] arm_mpam: resctrl: Add boilerplate cpuhp and domain allocation

Jonathan Cameron
2026-01-05T17:40:12Z

On Fri, 19 Dec 2025 18:11:17 +0000
Ben Horgan <ben.horgan@arm.com> wrote:

> From: James Morse <james.morse@arm.com>
>
> resctrl has its own data structures to describe its resources. We can't use
> these directly as we play tricks with the 'MBA' resource, picking the MPAM
> controls or monitors that best apply. We may export the same component as
> both L3 and MBA.
>
> Add mpam_resctrl_exports[] as the array of class->resctrl mappings we are
> exporting, and add the cpuhp hooks that allocated and free the resctrl
> domain structures.
>
> While we're here, plumb in a few other obvious things.
>
> CONFIG_ARM_CPU_RESCTRL is used to allow this code to be built even though
> it can't yet be linked against resctrl.
>
> Signed-off-by: James Morse <james.morse@arm.com>
> Signed-off-by: Ben Horgan <ben.horgan@arm.com>
> ---
> Domain list is an rcu list
> Add synchronize_rcu() to free the deleted element
> Code flow simplification (Jonathan)

Just trivial stuff and that one what do we loop over thing that
we continued discussing in the RFC thread (I think you already
tidied that up) Nothing here to stop:
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
(which is another way of saying I'm not planning to read it again :)

> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
> new file mode 100644
> index 000000000000..4beeeded00ff
> --- /dev/null
> +++ b/drivers/resctrl/mpam_resctrl.c

> +
> +static int mpam_resctrl_pick_domain_id(int cpu, struct mpam_component *comp)
> +{
> + struct mpam_class *class = comp->class;
> +
> + if (class->type == MPAM_CLASS_CACHE)
> + return comp->comp_id;
> +
> + /* TODO: repaint domain ids to match the L3 domain ids */
> + /*
> + * Otherwise, expose the ID used by the firmware table code.
Maybe more turns up in here. Otherwise, easily fits in single line comment.
> + */
> + return comp->comp_id;
> +}

> +static struct mpam_resctrl_dom *
> +mpam_resctrl_get_domain_from_cpu(int cpu, struct mpam_resctrl_res *res)
> +{
> + struct mpam_resctrl_dom *dom;
> + struct rdt_ctrl_domain *ctrl_d;
> + struct rdt_resource *r = &res->resctrl_res;
> +
> + lockdep_assert_cpus_held();
> +
> + list_for_each_entry_rcu(ctrl_d, &r->ctrl_domains, hdr.list) {

As a reminder, though I think you already changed this, discussion carried
on wrt to the RFC version of this loop.

> + dom = container_of(ctrl_d, struct mpam_resctrl_dom,
> + resctrl_ctrl_dom);
> +
> + if (cpumask_test_cpu(cpu, &dom->ctrl_comp->affinity))
> + return dom;
> + }
> +
> + return NULL;
> +}

> +
> +void mpam_resctrl_offline_cpu(unsigned int cpu)
> +{
> + resctrl_offline_cpu(cpu);
> +
> + guard(mutex)(&domain_list_lock);
> + for (int i = 0; i < RDT_NUM_RESOURCES; i++) {
> + struct mpam_resctrl_res *res;
> + struct mpam_resctrl_dom *dom;
> + struct rdt_mon_domain *mon_d;
> + struct rdt_ctrl_domain *ctrl_d;
> + bool ctrl_dom_empty, mon_dom_empty;
> +
> + res = &mpam_resctrl_controls[i];
> + if (!res->class)
> + continue; // dummy resource
> +
> + dom = mpam_resctrl_get_domain_from_cpu(cpu, res);
> + if (WARN_ON_ONCE(!dom))
> + continue;
> +
> + ctrl_dom_empty = true;
> + if (exposed_alloc_capable) {
> + ctrl_d = &dom->resctrl_ctrl_dom;
> + ctrl_dom_empty = mpam_resctrl_offline_domain_hdr(cpu, &ctrl_d->hdr);
> + if (ctrl_dom_empty)
> + resctrl_offline_ctrl_domain(&res->resctrl_res, ctrl_d);
> + }
really small thing but I'd do
} else {
ctrl_dom_empty = true;
}
here to make it visually obvious why that is set to true. The initialize and override
if we have more info pattern is to me less readable.

> +
> + mon_dom_empty = true;
> + if (exposed_mon_capable) {
> + mon_d = &dom->resctrl_mon_dom;
> + mon_dom_empty = mpam_resctrl_offline_domain_hdr(cpu, &mon_d->hdr);
> + if (mon_dom_empty)
> + resctrl_offline_mon_domain(&res->resctrl_res, mon_d);
> + }
Similar for this one.
> +
> + if (ctrl_dom_empty && mon_dom_empty)
> + kfree(dom);
> + }
> +}