[src/trunk]: src/sys/dev/pci sip(4): Tidy up DMA syncs.

[riastradh <riastradh%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/f80c606bca8e
branches:  trunk
changeset: 368806:f80c606bca8e
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sat Aug 06 15:38:42 2022 +0000

description:
sip(4): Tidy up DMA syncs.

- No membar_producer in sip_init_rxdesc -- use bus_dmamap_sync with
  BUS_DMASYNC_PREWRITE to order updates to the DMA descriptors.

- Omit needless membar_producer in sip_init_txdesc -- the hardware
  will not look at any of these descriptors until we set CMDSTS_OWN
  on the first one in the sequence, which is done later in the
  caller, sipcom_start.

- In gsip_rxintr, make sure to read cmdsts _before_ extsts, by
  separating them with BUS_DMASYNC_PREREAD.  Otherwise, the CPU might
  reorder the loads and read a stale extsts first before witnessing
  an updated cmdsts with the CMDSTS_OWN bit that transfers ownership
  of the rx packet to us.

diffstat:

 sys/dev/pci/if_sip.c |  16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diffs (58 lines):

diff -r 809dc022d404 -r f80c606bca8e sys/dev/pci/if_sip.c
--- a/sys/dev/pci/if_sip.c      Sat Aug 06 10:22:22 2022 +0000
+++ b/sys/dev/pci/if_sip.c      Sat Aug 06 15:38:42 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_sip.c,v 1.186 2022/05/24 20:50:19 andvar Exp $      */
+/*     $NetBSD: if_sip.c,v 1.187 2022/08/06 15:38:42 riastradh Exp $   */
 
 /*-
  * Copyright (c) 2001, 2002 The NetBSD Foundation, Inc.
@@ -73,7 +73,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_sip.c,v 1.186 2022/05/24 20:50:19 andvar Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_sip.c,v 1.187 2022/08/06 15:38:42 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -569,7 +569,6 @@
                sipd->sipd_words[sc->sc_bufptr_idx] = htole32(bufptr);
        }
        sipd->sipd_words[sc->sc_extsts_idx] = 0;
-       membar_producer();
        sipd->sipd_words[sc->sc_cmdsts_idx] = htole32(cmdsts);
        /* sip_cdtxsync() will be done later. */
 }
@@ -596,7 +595,7 @@
                    htole32(rxs->rxs_dmamap->dm_segs[0].ds_addr);
        }
        sipd->sipd_words[sc->sc_extsts_idx] = 0;
-       membar_producer();
+       sip_cdrxsync(sc, x, BUS_DMASYNC_PREWRITE);
        sipd->sipd_words[sc->sc_cmdsts_idx] =
            htole32(CMDSTS_INTR | (sc->sc_parm->p_rxbuf_len &
                                   sc->sc_bits.b_cmdsts_size_mask));
@@ -2126,9 +2125,6 @@
 
                cmdsts =
                    le32toh(sc->sc_rxdescs[i].sipd_words[sc->sc_cmdsts_idx]);
-               extsts =
-                   le32toh(sc->sc_rxdescs[i].sipd_words[sc->sc_extsts_idx]);
-               len = CMDSTS_SIZE(sc, cmdsts);
 
                /*
                 * NOTE: OWN is set if owned by _consumer_.  We're the
@@ -2142,6 +2138,12 @@
                        break;
                }
 
+               sip_cdrxsync(sc, i, BUS_DMASYNC_POSTREAD);
+
+               extsts =
+                   le32toh(sc->sc_rxdescs[i].sipd_words[sc->sc_extsts_idx]);
+               len = CMDSTS_SIZE(sc, cmdsts);
+
                if (__predict_false(sc->sc_rxdiscard)) {
                        sip_init_rxdesc(sc, i);
                        if ((cmdsts & CMDSTS_MORE) == 0) {