Re: [PATCH] arm_mpam: resctrl: Catch and propagate error from get_cpu_cacheinfo_id()

[Gavin Shan]

On 9/16/26 7:49 PM, Ben Horgan wrote:
> Hi Andre,
>
> On 07/09/2026 13:59, Ben Horgan wrote:
> > Hi Andre,
> >
> > On 02/09/2026 15:37, Andre Przywara wrote:
> > > get_cpu_cacheinfo_id() can fail, in which case it returns a negative
> > > error value.
> > >
> > > Check the returned value for this error condition, before passing the
> > > value on to other code, which would hide the negative number in some high
> > > value in the unsigned type.
> > >
> > > Fixes: 36528c7681b8 ("arm_mpam: resctrl: Add support for 'MB' resource")
> > > Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> >
> > This looks good to me. Out of interest what led you to find this?
> >
> > Reviewed-by: Ben Horgan <ben.horgan@xxxxxxx>
>
> I seem to have been a bit hasty here.
>
> Sashiko points out at [1] that 0xFFFFFFFF is the only value we were previously considering invalid
> and that the value coming from dt or acpi can provid other valid values that would after this patch
> be considered invalid.
>
> [1] https://sashiko.dev/#/patchset/20260902143757.3469690-1-andre.przywara%40arm.com

I think Sashiko is correct that the check 'if (cache_id < 0) return -ENOENT' is incorrect
because only -1 is a invalid ID. The correct check would be:

	u32 cache_id = get_cpu_cacheinfo_id(cpu, 3);

	if (cache_id = UINT_MAX)
		return -ENOENT;

Or

	int cache_id = get_cpu_cacheinfo_id(cpu, 3);

	if (cache_id == -1)
		return -ENOENT;

Thanks,
Gavin

> Thanks,
>
> Ben>
> > Thanks,
> >
> > Ben
> >
> > > ---
> > >   drivers/resctrl/mpam_resctrl.c | 5 ++++-
> > >   1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
> > > index 9d223057953ab..a5e661eff86d7 100644
> > > --- a/drivers/resctrl/mpam_resctrl.c
> > > +++ b/drivers/resctrl/mpam_resctrl.c
> > > @@ -786,7 +786,10 @@ static u32 get_mba_min(struct mpam_props *cprops)
> > >   /* Find the L3 cache that has affinity with this CPU */
> > >   static int find_l3_equivalent_bitmask(int cpu, cpumask_var_t tmp_cpumask)
> > >   {
> > > -	u32 cache_id = get_cpu_cacheinfo_id(cpu, 3);
> > > +	int cache_id = get_cpu_cacheinfo_id(cpu, 3);
> > > +
> > > +	if (cache_id < 0)
> > > +		return -ENOENT;
> > >   
> > >   	lockdep_assert_cpus_held();
> > >