Re: [PATCH RFC 09/15] arm_mpam: Fix MSC MMIO window size to use resource_size() instead of end - start

[Yin Li]

On 9/4/2026 11:05 AM, Yin Li wrote:
> On 9/3/2026 9:23 PM, Ben Horgan wrote:
> > Hi Yin,
> >
> > On 03/09/2026 11:20, Ben Horgan wrote:
> > > Hi Yin,
> > >
> > > On 11/08/2026 14:30, Yin Li wrote:
> > > > struct resource uses an inclusive end address, so the correct size is
> > > > end - start + 1. The previous calculation of end - start was off by 
> > > > one,
> > > > resulting in a mapped window one byte smaller than the actual resource.
> > > > Use resource_size() which correctly computes end - start + 1.
> > > >
> > > > Signed-off-by: Yin Li <yin.li@xxxxxxxxxxxxxxxx>
> > >
> > > I just got a kernel ci report for this one which asks for tags:
> > >
> > > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > > Closes: https://lore.kernel.org/oe-kbuild-all/202609030809.ObirDhR3- 
> > > lkp@xxxxxxxxx/
> > >
> > > It doesn't look to give a useful fixes tag though. I'd go with this 
> > > as that's where the error was
> > > introduced.
> > >
> > > Fixes: f04046f2577a ("arm_mpam: Add probe/remove for mpam msc driver 
> > > and kbuild boiler plate")
> > >
> > > Looks good to me.
> > >
> > > Reviewed-by: Ben Horgan <ben.horgan@xxxxxxx>
> >
> > Scratch that. As Ilpo points out,[1], there is no functional bug but 
> > just some misleading naming
> > which never the less would be good to fix. This does require > in the 
> > warnings becoming >= though
> > and there would be no need for fixes tag. Do you agree with this 
> > analysis?
> >
> > Thanks,
> >
> > Ben
> >
> > [1]
> > https://lore.kernel.org/ 
> > lkml/03055fbc-281f-4ed9-9282-4853d560e17f@xxxxxxx/T/ 
> > #mdb57d40c888ff4ce656a9d9a00ecf5d99466530c
>
> Hi Ben,
>
> Thanks, and thanks to Ilpo for the detailed analysis.
>
> Agreed — the current code is functionally correct because the off-by-one
> in "end - start" is cancelled out by the ">" checks, since
> mapped_hwpage_sz effectively holds the last mapped byte rather than the
> size. So there's no functional bug and no Fixes tag is needed.
>
> I'll update the patch to switch to resource_size() and change the
> corresponding ">" checks to ">=" together, so the naming becomes
> accurate while keeping the behaviour unchanged. I'll also reword the
> commit message to describe this as a naming/readability cleanup rather
> than a bugfix.

Hi Ben,

I looked into this more closely, and I think the situation is a bit
different from previous analysis — could you and Ilpo double-check?

All three bounds checks have the access width included on the left-hand
side, e.g.:

      WARN_ON_ONCE(reg + sizeof(u32) > msc->mapped_hwpage_sz);

Here "reg + sizeof(u32)" is the one-past-the-end offset of the write, so
a legal access needs "reg + 4 <= size", i.e. the out-of-bounds condition
is correctly "> size".

Take a 0x1000-sized window (valid offsets 0x000..0xFFF):

    - With the old value, mapped_hwpage_sz = end - start = 0xFFF (size 
- 1).
    A write at reg = 0xFFC touches bytes 0xFFC..0xFFF — exactly the last
    4 bytes, which is legal. But the check computes 0xFFC + 4 = 0x1000 >
    0xFFF → true, so it falsely warns on a valid access. That's a real
    off-by-one.

    - With resource_size() = 0x1000 (true size), the same access gives
    0x1000 > 0x1000 → false, so it correctly passes, while an access at
    reg = 0xFFD (0x1001 > 0x1000 → true) is correctly rejected.

So resource_size() fixes a genuine off-by-one here, and the ">" checks
should stay as ">". Changing them to ">=" would break the reg = 0xFFC
case (0x1000 >= 0x1000 → true) and wrongly reject a legal access.

So for the next version I plan to keep the resource_size() change, keep
the ">" checks unchanged, and describe it as an off-by-one fix rather
than a naming cleanup. Does this match what you and Ilpo see?

Best regards,
Yin Li




> > > Thanks,
> > >
> > > Ben
> > >
> > > > ---
> > > >   drivers/resctrl/mpam_devices.c | 2 +-
> > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/ 
> > > > mpam_devices.c
> > > > index 1e082fb60e30..5d1854d97371 100644
> > > > --- a/drivers/resctrl/mpam_devices.c
> > > > +++ b/drivers/resctrl/mpam_devices.c
> > > > @@ -2296,7 +2296,7 @@ static struct mpam_msc 
> > > > *do_mpam_msc_drv_probe(struct platform_device *pdev)
> > > >               dev_err_once(dev, "Failed to map MSC base address\n");
> > > >               return ERR_CAST(io);
> > > >           }
> > > > -        msc->mapped_hwpage_sz = msc_res->end - msc_res->start;
> > > > +        msc->mapped_hwpage_sz = resource_size(msc_res);
> > > >           msc->mapped_hwpage = io;
> > > >       } else {
> > > >           return ERR_PTR(-EINVAL);

--
Thx and BRs,
Yin