On Wed, Oct 28, 2026 at 10:19 AM <[email protected]> wrote: .... > [Severity: High] > Can a late or error interrupt race on dsi->reg_dma_mem in vc4_dsi_transfer() > before disable_irq(dsi->irq) is reached in vc4_dsi_host_transfer(), > corrupting DSI1_CTRL before vc4_dsi_reset_and_restore() saves and restores > it? > > When wait_for_completion_timeout() times out in vc4_dsi_transfer(), > dsi->irq and INT_EN are still enabled while vc4_dsi_transfer() performs > five register writes before returning -ETIMEDOUT: > > drivers/gpu/drm/vc4/vc4_dsi.c:vc4_dsi_transfer() { > ... > DSI_PORT_WRITE(INT_EN, DSI_PORT_BIT(INTERRUPTS_ALWAYS_ENABLED)); > > if (ret) > goto reset_fifo_and_return; > ... > reset_fifo_and_return: > drm_err(drm, "DSI transfer failed, resetting: %d\n", ret); > > DSI_PORT_WRITE(TXPKT1C, DSI_PORT_READ(TXPKT1C) & ~DSI_TXPKT1C_CMD_EN); > udelay(1); > DSI_PORT_WRITE(CTRL, > DSI_PORT_READ(CTRL) | > DSI_PORT_BIT(CTRL_RESET_FIFOS)); > > DSI_PORT_WRITE(TXPKT1C, 0); > DSI_PORT_WRITE(INT_EN, DSI_PORT_BIT(INTERRUPTS_ALWAYS_ENABLED)); > return ret; > } > > On BCM2835 DSI1 (where broken_axi_workaround is enabled), DSI_PORT_WRITE() > calls dsi_dma_workaround_write(), which writes the value into the single > shared buffer *dsi->reg_dma_mem and submits a DMA transfer from > dsi->reg_dma_paddr: > > drivers/gpu/drm/vc4/vc4_dsi.c:dsi_dma_workaround_write() { > ... > *dsi->reg_dma_mem = val; > > tx = chan->device->device_prep_dma_memcpy(chan, > dsi->reg_paddr + offset, > dsi->reg_dma_paddr, > 4, 0); > ... > } > > If the threaded handler vc4_dsi_irq_handler() runs concurrently (for > example on an LPRX/TA timeout error interrupt or a late completion > interrupt) and calls DSI_PORT_WRITE(INT_STAT, stat), it overwrites > *dsi->reg_dma_mem = stat without any lock. > > If that happens during the read-modify-write of CTRL in > reset_fifo_and_return, the DMA transfer can write stat into DSI1_CTRL. > When vc4_dsi_reset_and_restore() then runs under disable_irq(dsi->irq), it > snapshots the corrupted register via u32 ctrl = DSI_PORT_READ(CTRL) and > writes it back at the end of the reset, leaving DSI1_CTRL corrupted (for > example with DSI1_CTRL_EN cleared so the block remains gated). > > Should disable_irq(dsi->irq) be called in vc4_dsi_transfer() before the > post-timeout register writes (or before reset_fifo_and_return)?
The race is real, but it is not introduced by this patch and not limited to the timeout path: on BCM2835/6/7 DSI1 every DSI_PORT_WRITE() goes through the single reg_dma_mem buffer without a lock, so the threaded handler can corrupt any register write made from process context (transfer setup, the bridge callbacks, the existing reset_fifo_and_return path). Moving disable_irq() would only cover this one path. I'm sending a separate fix that serializes dsi_dma_workaround_write() with a mutex, so this series stays as it is. Henry
