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

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

[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);
 	}
-- 
2.34.1

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

[Jinjie Ruan]

Hi Ben,

Is this modification okay?

在 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);
>  	}

-- 
Best regards,
Jinjie

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

[Ben Horgan]

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);
>>  	}
>