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

[Ben Horgan <ben.horgan@arm.com>]

Hi Shaopeng,

On 1/8/26 10:36, Shaopeng Tan (Fujitsu) wrote:
> Hello Ben,
> 
>> 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)
>> ---
[...]
>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
>> index 860181266b15..303aacfcb9d6 100644
>> --- a/drivers/resctrl/mpam_devices.c
>> +++ b/drivers/resctrl/mpam_devices.c
>> @@ -1628,6 +1628,9 @@ static int mpam_cpu_online(unsigned int cpu)
>>                          mpam_reprogram_msc(msc);
>>          }
>>  
>> +       if (mpam_is_enabled())
>> +               mpam_resctrl_online_cpu(cpu);
> 
> return mpam_resctrl_online_cpu(cpu);

Ack

> 
> Considering the fixes added in [PATCH v2 27/45],
> How about changing the code as follows? 
> 
> +		err = mpam_resctrl_control_init(res, i);
> +		if (err) {
> +			pr_debug("Failed to initialise rid %u\n", i);
> +			pr_debug("Internal error %d - resctrl not supported\n", err);
> +			cpus_read_unlock();
> +			return err;
> +		}
> +	}
> +           /* Find some classes to use for monitors */
> +         mpam_resctrl_pick_counters();
> +         for (enum resctrl_event_id j = 0; j < QOS_NUM_EVENTS; j++) {
> +         …
> + }
> +	cpus_read_unlock();

Hmm, probably better to propagate the error from
mpam_resctrl_monitor_init_abmc() rather moving the early exit.

> 
> 
> Best regards,
> Shaopeng TAN
> 
>> +       if (!exposed_alloc_capable && !exposed_mon_capable) {
>> +               pr_debug("No alloc(%u) or monitor(%u) found - resctrl not supported\n",
>> +                        exposed_alloc_capable, exposed_mon_capable);
>> +               return -EOPNOTSUPP;
>> +       }
>> +
>> +       if (!is_power_of_2(mpam_pmg_max + 1)) {
>> +               /*
>> +                * If not all the partid*pmg values are valid indexes,
>> +                * resctrl may allocate pmg that don't exist. This
>> +                * should cause an error interrupt.
>> +                */
>> +               pr_warn("Number of PMG is not a power of 2! resctrl may misbehave");
>> +       }
>> +
>> +       /* TODO: call resctrl_init() */
>> +
>> +       return err;
>> +}
>> diff --git a/include/linux/arm_mpam.h b/include/linux/arm_mpam.h
>> index 7f00c5285a32..2c7d1413a401 100644
>> --- a/include/linux/arm_mpam.h
>> +++ b/include/linux/arm_mpam.h
>> @@ -49,6 +49,9 @@ static inline int mpam_ris_create(struct mpam_msc *msc, u8 ris_idx,
>>  }
>>  #endif
>>  
>> +bool resctrl_arch_alloc_capable(void);
>> +bool resctrl_arch_mon_capable(void);
>> +
>>  /**
>>   * mpam_register_requestor() - Register a requestor with the MPAM driver
>>   * @partid_max:                The maximum PARTID value the requestor can generate.
>> --
>> 2.43.0

Thanks,

Ben