Re: [PATCH] dmaengine: mv_xor: protect MBUS window access with a spinlock

[Frank Li <Frank.li@oss.nxp.com>]

On Wed, Sep 16, 2026 at 12:34:31PM -0700, Rosen Penev wrote:
> On Wed, Sep 16, 2026 at 12:07 PM Frank Li <Frank.li@oss.nxp.com> wrote:
> >
> > On Wed, Sep 16, 2026 at 11:26:08AM -0700, Rosen Penev wrote:
> > > mv_xor_add_io_win() reads and writes shared MBUS window registers
> > > and updates the shared win_start/win_end arrays in mv_xor_device.
> > > Multiple DMA channels can call this function concurrently via
> > > mv_xor_prep_dma_xor(), leading to races where two threads can
> > > select the same free window slot, corrupt the registers, or produce
> > > inconsistent cached state.
> > >
> > > Add a spinlock to struct mv_xor_device and hold it across the
> > > register read-modify-write and cache update in mv_xor_add_io_win().
> > >
> > > Fixes: 77ff7a706f01 ("mv_xor: Add support for IO (PCIe) src/dst areas")
> > > Assisted-by: LLM
> > > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > > ---
> > >  drivers/dma/mv_xor.c | 15 +++++++++++++--
> > >  drivers/dma/mv_xor.h |  1 +
> > >  2 files changed, 14 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
> > > index 0de2b1ad5c30..e7c6ec54ce25 100644
> > > --- a/drivers/dma/mv_xor.c
> > > +++ b/drivers/dma/mv_xor.c
> > > @@ -497,11 +497,14 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)
> > >       u8 target, attr;
> > >       int ret;
> > >       int i;
> > > +     unsigned long flags;
> > >
> > >       /* Nothing needs to get done for the Armada 3700 */
> > >       if (xordev->xor_type == XOR_ARMADA_37XX)
> > >               return 0;
> > >
> > > +     spin_lock_irqsave(&xordev->win_lock, flags);
> > > +
> >
> > use auto clean up guard()
> Could that be backported though?

I think it should be fine now. guard() already use quick popular.

Frank

> >
> > Frank
> > >       /*
> > >        * Loop over the cached windows to check, if the requested area
> > >        * is already mapped. If this the case, nothing needs to be done
> > > @@ -511,6 +514,7 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)
> > >               if (addr >= xordev->win_start[i] &&
> > >                   addr <= xordev->win_end[i]) {
> > >                       /* Window is already mapped */
> > > +                     spin_unlock_irqrestore(&xordev->win_lock, flags);
> > >                       return 0;
> > >               }
> > >       }
> > > @@ -521,8 +525,10 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)
> > >
> > >       /* If no IO window is found that addr has to be located in SDRAM */
> > >       ret = mvebu_mbus_get_io_win_info(addr, &size, &target, &attr);
> > > -     if (ret < 0)
> > > +     if (ret < 0) {
> > > +             spin_unlock_irqrestore(&xordev->win_lock, flags);
> > >               return 0;
> > > +     }
> > >
> > >       /*
> > >        * Mask the base addr 'addr' according to 'size' read back from the
> > > @@ -540,8 +546,10 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)
> > >
> > >       /* Set 'i' to the first free window to write the new values to */
> > >       i = ffs(~win_enable) - 1;
> > > -     if (i >= WINDOW_COUNT)
> > > +     if (i >= WINDOW_COUNT) {
> > > +             spin_unlock_irqrestore(&xordev->win_lock, flags);
> > >               return -ENOMEM;
> > > +     }
> > >
> > >       writel((addr & 0xffff0000) | (attr << 8) | target,
> > >              base + WINDOW_BASE(i));
> > > @@ -556,6 +564,8 @@ static int mv_xor_add_io_win(struct mv_xor_chan *mv_chan, u32 addr)
> > >       writel(win_enable, base + WINDOW_BAR_ENABLE(0));
> > >       writel(win_enable, base + WINDOW_BAR_ENABLE(1));
> > >
> > > +     spin_unlock_irqrestore(&xordev->win_lock, flags);
> > > +
> > >       return 0;
> > >  }
> > >
> > > @@ -1353,6 +1363,7 @@ static int mv_xor_probe(struct platform_device *pdev)
> > >
> > >       platform_set_drvdata(pdev, xordev);
> > >
> > > +     spin_lock_init(&xordev->win_lock);
> > >
> > >       /*
> > >        * We need to know which type of XOR device we use before
> > > diff --git a/drivers/dma/mv_xor.h b/drivers/dma/mv_xor.h
> > > index c87cefd38a07..034db4d0bfb1 100644
> > > --- a/drivers/dma/mv_xor.h
> > > +++ b/drivers/dma/mv_xor.h
> > > @@ -80,6 +80,7 @@ struct mv_xor_device {
> > >       struct clk           *clk;
> > >       struct mv_xor_chan   *channels[MV_XOR_MAX_CHANNELS];
> > >       int                  xor_type;
> > > +     spinlock_t           win_lock;
> > >
> > >       u32                  win_start[WINDOW_COUNT];
> > >       u32                  win_end[WINDOW_COUNT];
> > > --
> > > 2.55.0
> > >