Re: [PATCH] arm_mpam: resctrl: Leave CDP off when there are too few PARTIDs

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

Hi Jinjie,

On 06/11/2026 08:35, Jinjie Ruan wrote:
> Hi Ben,
> 
> Is this modification okay?

Yes it's ok but there is more we can do here. Sashiko has raised a bug that points out we don't take
into account whether or not a resource is cdp_capable or not [1].

This leads to an additional check along the lines of:

if (enable && (!the_res.alloc_capable || !the_res.cdp_capable))
    return -EINVAL;

On a process note, it would helpful to me for reviewing and later on when collecting the patches
together if you could put related patches together in a series. In the case of this patch it it is
solving a cdp related problem, as is your patch "arm_mpam: resctrl: Limit num_rmids to the CLOSIDs
in use". fs/resctrl patches though should be kept separately unless there is a particular reason why
they should be in a series with MPAM patches, e.g. new resctrl_arch_ helpers and such like.

[1] https://sashiko.dev/#/bug/linux-05c19551-0bba-4a06-aa2c-3c90fab5853d

Thanks,

Ben

> 
> 在 2026/10/31 10:52, Jinjie Ruan 写道:
>> Enabling CDP requires at least 2 PARTIDs. If there are too few PARTIDs,
>> resctrl_arch_set_cdp_enabled() returns -EINVAL, but only after it has
>> recorded CDP as enabled, halved the monitor count and turned off the
>> mbw_max MBA control. resctrl then believes CDP is off, which leaves the
>> two sides out of step and the halved monitor count and the missing MBA
>> control are not undone.
>>
>> Check the PARTID limit before anything is changed so a failed enable has
>> no effect.
>>
>> Fixes: 6789fb99282c ("arm_mpam: resctrl: Add CDP emulation")
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>> RFC v2 -> v1:
>> - Check mpam_partid_max ahead and The partid_d/partid_i determination
>>   remains in place.
>> - Update the commit message as Ben suggested.
>> ---
>>  drivers/resctrl/mpam_resctrl.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
>> index 5468367776e2..aa3b3a093857 100644
>> --- a/drivers/resctrl/mpam_resctrl.c
>> +++ b/drivers/resctrl/mpam_resctrl.c
>> @@ -210,6 +210,9 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level rid, bool enable)
>>  	if (enable)
>>  		pr_warn("CDP is an expert feature and may cause MPAM to malfunction.\n");
>>  
>> +	if (enable && mpam_partid_max < 1)
>> +		return -EINVAL;
>> +
>>  	/*
>>  	 * resctrl_arch_set_cdp_enabled() is only called with enable set to
>>  	 * false on error and unmount.
>> @@ -239,9 +242,6 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level rid, bool enable)
>>  		mpam_resctrl_controls[RDT_RESOURCE_MBA].resctrl_res.alloc_capable = true;
>>  
>>  	if (enable) {
>> -		if (mpam_partid_max < 1)
>> -			return -EINVAL;
>> -
>>  		partid_d = resctrl_get_config_index(RESCTRL_RESERVED_CLOSID, CDP_DATA);
>>  		partid_i = resctrl_get_config_index(RESCTRL_RESERVED_CLOSID, CDP_CODE);
>>  	}
>