Re: [PATCH 4/4] hw/net: xilinx_axienet: Fix DMA RX IRQ if ethernet disable RX

[Peter Maydell]

On Fri, 26 Jul 2024 at 06:59, Jim Shu <jim.shu@sifive.com> wrote:
>
> When AXI ethernet RX is disabled, it shouldn't send packets to AXI DMA,
> which may let AXI DMA to send RX full IRQs. It is aligned with real AXI
> DMA/ethernet IP behavior in the FPGA.
>
> Also, now ethernet RX blocks the RX packets when it is disabled. It
> should send and clear the remaining RX packets when enabling it.
>
> Signed-off-by: Jim Shu <jim.shu@sifive.com>
> ---
>  hw/net/xilinx_axienet.c | 71 ++++++++++++++++++++++++-----------------
>  1 file changed, 42 insertions(+), 29 deletions(-)
>
> diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
> index 05d41bd548..8428f10946 100644
> --- a/hw/net/xilinx_axienet.c
> +++ b/hw/net/xilinx_axienet.c
> @@ -530,6 +530,40 @@ static uint64_t enet_read(void *opaque, hwaddr addr, 
> unsigned size)
>      return r;
>  }
>
> +static void axienet_eth_rx_notify(void *opaque)
> +{
> +    XilinxAXIEnet *s = XILINX_AXI_ENET(opaque);
> +
> +    /* If RX is disabled, don't trigger DMA to update RX desc and send IRQ */
> +    if (!axienet_rx_enabled(s)) {
> +        return;
> +    }

This checks s->rcw[1] & RCW1_RX, and does nothing if it's not set...

>  static void enet_write(void *opaque, hwaddr addr,
>                         uint64_t value, unsigned size)
>  {
> @@ -546,6 +580,14 @@ static void enet_write(void *opaque, hwaddr addr,
>              } else {
>                  qemu_flush_queued_packets(qemu_get_queue(s->nic));
>              }
> +
> +            /*
> +             * When RX is enabled, check if any remaining data in rxmem
> +             * and send them.
> +             */
> +            if ((addr & 1) && s->rcw[addr & 1] & RCW1_RX) {
> +                axienet_eth_rx_notify(s);
> +            }

...but at this callsite we open-code a check on RCW1_RX and
skip the call if it's not set. We don't need to check twice.

>              break;
>
>          case R_TC:

thanks
-- PMM