Re: [PATCH v4 2/3] arm_mpam: resctrl: Only enable CDP on a resource that supports it

[Gavin Shan <gshan@redhat.com>]

On 11/11/26 6:13 PM, Jinjie Ruan wrote:
> resctrl_arch_set_cdp_enabled() enables CDP for any cache level that is
> requested, even one that has no MPAM cache controls. Mounting resctrl
> with -o cdpl2 when only L3 can allocate still turns CDP emulation on for
> the whole system, so every CLOSID takes two PARTIDs while resctrl keeps
> its CLOSID count at the number of PARTIDs. Control groups in the upper
> half then map to PARTIDs beyond mpam_partid_max, which index past the
> configuration and monitor arrays and program out of range PARTIDs into
> the MSCs. On QEMU with 5 PARTIDs the first monitor read after creating
> 2 control groups raises MSMONCFG_ID_RANGE for PARTID 5, and MPAM is
> disabled.
> 
> Do not enable CDP when a resource cannot allocate or is not CDP capable,
> as x86 does, so the mount fails instead. Only the enable path is checked,
> since unmount disables CDP on both cache levels to restore the MBA control.
> 
> A system where only MBA can allocate, or that only has monitors, now
> fails a mount with CDP as well: CDP hides the MBA control because
> mbw_max is a per-PARTID maximum, leaving nothing for CDP to apply to.
> 
> Fixes: 6789fb99282c ("arm_mpam: resctrl: Add CDP emulation")
> Link: https://sashiko.dev/#/bug/linux-05c19551-0bba-4a06-aa2c-3c90fab5853d
> Suggested-by: Ben Horgan <ben.horgan@arm.com>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> Cc: <stable@vger.kernel.org> # 7.1+
> ---
>   drivers/resctrl/mpam_resctrl.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
> index 573617ba09df..afa3c084c475 100644
> --- a/drivers/resctrl/mpam_resctrl.c
> +++ b/drivers/resctrl/mpam_resctrl.c
> @@ -222,6 +222,10 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level rid, bool enable)
>   	if (enable && mpam_partid_max < 1)
>   		return -EINVAL;
>   
> +	if (enable && (!mpam_resctrl_controls[rid].resctrl_res.alloc_capable ||
> +		       !mpam_resctrl_controls[rid].resctrl_res.cdp_capable))
> +		return -EINVAL;
> +

x86 only checks 'cdp_capable', so this check isn't exactly same as to what's
done on x86, as claimed in the commit log?

int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable)
{
         struct rdt_hw_resource *hw_res = &rdt_resources_all[l];

         if (!hw_res->r_resctrl.cdp_capable)
                 return -EINVAL;

         :
}


>   	/*
>   	 * resctrl_arch_set_cdp_enabled() is only called with enable set to
>   	 * false on error and unmount.

Thanks,
Gavin