Re: [PATCH 3/4] hw/dma: xilinx_axidma: Reset qemu_irq when DMA/Stream is reset

[Peter Maydell]

On Fri, 26 Jul 2024 at 06:59, Jim Shu <jim.shu@sifive.com> wrote:
>
> Current DMA/Stream reset will clear interrupt pending bit of DMA device.
> The qemu_irq of device should be updated at the same time.
>
> Signed-off-by: Jim Shu <jim.shu@sifive.com>
> ---
>  hw/dma/xilinx_axidma.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
> index 728246f925..beb3fbf1d5 100644
> --- a/hw/dma/xilinx_axidma.c
> +++ b/hw/dma/xilinx_axidma.c
> @@ -177,11 +177,24 @@ static inline int stream_halted(struct Stream *s)
>      return !!(s->regs[R_DMASR] & DMASR_HALTED);
>  }
>
> +static void stream_update_irq(struct Stream *s)
> +{
> +    unsigned int pending, mask, irq;
> +
> +    pending = s->regs[R_DMASR] & DMASR_IRQ_MASK;
> +    mask = s->regs[R_DMACR] & DMASR_IRQ_MASK;
> +
> +    irq = pending & mask;
> +
> +    qemu_set_irq(s->irq, !!irq);
> +}
> +
>  static void stream_reset(struct Stream *s)
>  {
>      s->regs[R_DMASR] = DMASR_HALTED;  /* starts up halted.  */
>      s->regs[R_DMACR] = 1 << 16; /* Starts with one in compl threshold.  */
>      s->sof = true;
> +    stream_update_irq(s); /* Clear interrupt */
>  }

The general rule of thumb in QEMU is not to call
qemu_set_irq() from a DeviceState::reset function,
and xilinx_axidma_reset calls stream_reset. I think
probably the best thing is to separate out whether
this is a DeviceState::reset or a software-triggered
reset, and only call qemu_set_irq() in the latter case.

thanks
-- PMM