Re: [PATCH net] net: emac: mal: fix W1C write race in ICINTSTAT clearing

[Rosen Penev]

On Fri, Jul 3, 2026 at 9:37 AM Andrew Lunn <andrew@xxxxxxx> wrote:
>
> On Fri, Jul 03, 2026 at 01:06:40PM +1000, David Gibson wrote:
> > On Thu, Jul 02, 2026 at 04:49:23PM -0700, Rosen Penev wrote:
> > > The ICINTSTAT register is write-1-to-clear (W1C).  The read-modify-write
> > > pattern in both mal_txeob() and mal_rxeob() can lose interrupts: if a bit
> > > that should not be cleared is already asserted when mfdcri() reads the
> > > register, it is included in the read value, retained by the bitwise OR, and
> > > then written back as 1 - inadvertently clearing a pending but unhandled
> > > interrupt.
> > >
> > > Fix by writing only the specific bit to clear (ICINTSTAT_ICTX for TXEOB,
> > > ICINTSTAT_ICRX for RXEOB).  W1C semantics guarantee that writing 0 to the
> > > other bits has no effect.
> >
> > Wow, it's a long time since I thought about the MAL.
> >
> > > Fixes: 1d3bb996481e ("Device tree aware EMAC driver")
> >
> > This doesn't appear correct.  The lines in question were added by
> > fbcc4bacee30c ("ibm_newemac: MAL support for PowerPC 405EZ")
> >
> > > Assisted-by: opencode:big-pickle
> > > Signed-off-by: Rosen Penev <rosenp@xxxxxxxxx>
> >
> > Assuming ICINTSTAT is indeed a W1C register (or "read/clear" as I
> > believe they were termed in the 405 documentation) the change looks
> > correct.  However, I no longer have access to the documentation that
> > would let me verify that.  I would absolutely not trust an LLM to know
> > if that's the case, since it's a fairly arbitrary and specific detail
> > of an obscure CPU.
>
> I agree. If this is pure LLM, we need some form of verification.
Only verification I have is the hardware and whether or not something breaks.

Sashiko reports:

This isn't a bug introduced by this patch, but does this read-modify-write
pattern on a write-1-to-clear interrupt status register cause lost
interrupts?
If another interrupt occurs and its bit is set before the mfdcri() read, the
read value will include that bit as a 1. The bitwise OR retains this 1, and
the subsequent mtdcri() will write it back, inadvertently clearing the newly
pending interrupt without processing it.

This is a pre-existing issue, but does this suffer from the same
write-1-to-clear read-modify-write problem as mal_txeob() where concurrent
interrupts might be accidentally cleared?
>
>   Andrew