Hi Peter,
Thanks for the suggestion.
axienet_eth_rx_notify() is also called by axidma_write() as a notify()
callback, so we need to check RCW1_RX in the function.
I think I could remove RCW1_RX checking in the enet_write() to avoid
double checking.
I will fix it in the v2 patchset.
Thanks,
JIm Shu.
On Mon, Jul 29, 2024 at 11:31 PM Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> On Fri, 26 Jul 2024 at 06:59, Jim Shu <jim....@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....@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
