Messages in this thread | | | From | Camelia Alexandra Groza <> | Subject | RE: net: fman: IRQ races | Date | Tue, 7 Jun 2022 14:52:47 +0000 |
| |
> -----Original Message----- > From: Sean Anderson <sean.anderson@seco.com> > Sent: Tuesday, May 31, 2022 22:09 > To: Madalin Bucur <madalin.bucur@nxp.com>; netdev > <netdev@vger.kernel.org> > Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>; Florinel > Iordache <florinel.iordache@nxp.com> > Subject: net: fman: IRQ races > > Hi all, > > I'm doing some refactoring of the dpaa/fman drivers, and I'm a bit > confused by the way IRQs are handled. To contrast, in GAM/MACB, one of > the first things IRQ handler does is grab a spinlock guarding register > access. This lets it do read/modify/writes all it wants. However, I > don't see anything like that in the FMan code. I'd like to use two > examples to illustrate. > > First, consider call_mac_isr. It will race with both fman_register_intr: > > CPU0 (call_mac_isr) CPU1 (fman_register_intr) > isr_cb = foo > isr_cb(src_handle) > src_handle = bar > > and with fman_unregister_intr > > CPU0 (call_mac_isr) CPU1 (fman_unregister_intr) > if (isr_cb) > isr_cb = NULL > src_handle = NULL > isr_cb(src_handle) > > This is probably not too critical (since hopefully there are no > interrupts before/after the handler is registered), but it certainly > looks very strange. > > Second, consider dtsec_isr. It will race with (for example) dtsec_set_allmulti: > > CPU0 (dtsec_isr) CPU1 (dtsec_set_allmulti) > <XFUNEN interrupt> > ioread32be(rctrl) ioread32be(rctrl) > iowrite32be(rctrl | MPROM) > iowrite32be(rctrl | GRS) > > and suddenly the MPROM write is dropped. (Actually, the whole > FM_TX_LOCKUP_ERRATA_DTSEC6 errata code seems funky, since after > calling > fman_reset_mac it seems like everything would need to be reinitialized). > > So what's going on here? Is there actually no locking, or am I missing > something?
Hi
You are right, there is no locking. The original FMan driver design didn't intend on supporting runtime register changes. Clearly this was a mistake as you pointed out.
Camelia
| |