Hi Jonathan,
On 12/18/25 11:38, Jonathan Cameron wrote:
> On Fri, 5 Dec 2025 21:58:31 +0000
> James Morse <james.morse@arm.com> wrote:
>
>> Systems with MPAM support may have a variety of control types at any
>> point of their system layout. We can only expose certain types of
>> control, and only if they exist at particular locations.
>>
>> Start with the well-know caches. These have to be depth 2 or 3
>> and support MPAM's cache portion bitmap controls, with a number
>> of portions fewer than resctrl's limit.
>
> Another one that is a bit random on wrap point. Probably worth tidying
> up formatting of all the commit messages so fussy people like me stop
> moaning ;)
Will do.
>
> Otherwise trivial stuff inline.
>>
>> Signed-off-by: James Morse <james.morse@arm.com>
>> ---
>> drivers/resctrl/mpam_resctrl.c | 91 +++++++++++++++++++++++++++++++++-
>> 1 file changed, 89 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
>> index 320cebbd37ce..ceaf11af4fc1 100644
>> --- a/drivers/resctrl/mpam_resctrl.c
>> +++ b/drivers/resctrl/mpam_resctrl.c
>> @@ -60,10 +60,96 @@ struct rdt_resource *resctrl_arch_get_resource(enum resctrl_res_level l)
>> return &mpam_resctrl_controls[l].resctrl_res;
>> }
>>
>> +static bool cache_has_usable_cpor(struct mpam_class *class)
>> +{
>> + struct mpam_props *cprops = &class->props;
>> +
>> + if (!mpam_has_feature(mpam_feat_cpor_part, cprops))
>> + return false;
>> +
>> + /* resctrl uses u32 for all bitmap configurations */
>> + return (class->props.cpbm_wd <= 32);
>
> For me those brackets aren't adding anything. It's not like
> anyone forgets precedence wrt return vs operators.
>
>> +}
>> +
>> +/* Test whether we can export MPAM_CLASS_CACHE:{2,3}? */
>> +static void mpam_resctrl_pick_caches(void)
>> +{
>> + struct mpam_class *class;
>> + struct mpam_resctrl_res *res;
>> +
>> + lockdep_assert_cpus_held();
>> +
>> + guard(srcu)(&mpam_srcu);
>> + list_for_each_entry_srcu(class, &mpam_classes, classes_list,
>> + srcu_read_lock_held(&mpam_srcu)) {
>> + if (class->type != MPAM_CLASS_CACHE) {
>> + pr_debug("class %u is not a cache\n", class->level);
>
> Lots of things aren't caches and seems unlikely that is a case that will
> make people wonder why pick caches didn't happen. So maybe this debug
> print is excessive?
Only memory or cache so far. I'll leave it here for now.
>
>> + continue;
>> + }
>> +
>> + if (class->level != 2 && class->level != 3) {
>> + pr_debug("class %u is not L2 or L3\n", class->level);
>> + continue;
>> + }
>> +
>> + if (!cache_has_usable_cpor(class)) {
>> + pr_debug("class %u cache misses CPOR\n", class->level);
>> + continue;
>> + }
>> +
>> + if (!cpumask_equal(&class->affinity, cpu_possible_mask)) {
>> + pr_debug("class %u has missing CPUs\n", class->level);
>> + pr_debug("class %u mask %*pb != %*pb\n", class->level,
>> + cpumask_pr_args(&class->affinity),
>> + cpumask_pr_args(cpu_possible_mask));
>
> Unless this is getting more complex in later patches, doesn't seem like
> pr_debug("class %u has missing CPUs, mask %*pb != %*pb\n",
> is too much to put on one line.
Done.
>
>
>> + continue;
>> + }
>> +
>> + if (class->level == 2)
>> + res = &mpam_resctrl_controls[RDT_RESOURCE_L2];
>> + else
>> + res = &mpam_resctrl_controls[RDT_RESOURCE_L3];
>> + res->class = class;
>> + exposed_alloc_capable = true;
>> + }
>> +}
>> +
>> static int mpam_resctrl_control_init(struct mpam_resctrl_res *res,
>> enum resctrl_res_level type)
>> {
>> - /* TODO: initialise the resctrl resources */
>> + struct mpam_class *class = res->class;
>> + struct rdt_resource *r = &res->resctrl_res;
>> +
>> + switch (res->resctrl_res.rid) {
>
> switch (r->rid) { ?
>
>> + case RDT_RESOURCE_L2:
>> + case RDT_RESOURCE_L3:
>> + r->alloc_capable = true;
>> + r->schema_fmt = RESCTRL_SCHEMA_BITMAP;
>> + r->cache.arch_has_sparse_bitmasks = true;
>> +
>> + r->cache.cbm_len = class->props.cpbm_wd;
>> + /* mpam_devices will reject empty bitmaps */
>> + r->cache.min_cbm_bits = 1;
>> +
>> + if (r->rid == RDT_RESOURCE_L2) {
>> + r->name = "L2";
>> + r->ctrl_scope = RESCTRL_L2_CACHE;
>> + } else {
>> + r->name = "L3";
>> + r->ctrl_scope = RESCTRL_L3_CACHE;
>> + }
>> +
>> + /*
>> + * Which bits are shared with other ...things...
>> + * Unknown devices use partid-0 which uses all the bitmap
>> + * fields. Until we configured the SMMU and GIC not to do this
>> + * 'all the bits' is the correct answer here.
>> + */
>> + r->cache.shareable_bits = resctrl_get_default_ctrl(r);
>> + break;
>> + default:
>> + break;
>> + }
>>
>> return 0;
>> }
>> @@ -286,7 +372,8 @@ int mpam_resctrl_setup(void)
>> res->resctrl_res.rid = i;
>> }
>>
>> - /* TODO: pick MPAM classes to map to resctrl resources */
>> + /* Find some classes to use for controls */
>> + mpam_resctrl_pick_caches();
>>
>> /* Initialise the resctrl structures from the classes */
>> for (i = 0; i < RDT_NUM_RESOURCES; i++) {
>
Thanks,
Ben