Download raw body.
vio: Reduce code duplication in control queue handling
Hi,
the next reviewable chunk of the V5 vio multi-queue diff is to pull the
common parts of all the control queue operations into separate functions.
While there, avoid setting sc_ctrl_inuse FREE if it was RESET, except in
vio_stop. Doing so could lead to race conditions.
ok?
Cheers,
Stefan
diff --git a/sys/dev/pv/if_vio.c b/sys/dev/pv/if_vio.c
index 3444bba35ef..8f38feb4dd3 100644
--- a/sys/dev/pv/if_vio.c
+++ b/sys/dev/pv/if_vio.c
@@ -317,8 +317,9 @@ void vio_iff(struct vio_softc *);
int vio_media_change(struct ifnet *);
void vio_media_status(struct ifnet *, struct ifmediareq *);
int vio_ctrleof(struct virtqueue *);
-int vio_wait_ctrl(struct vio_softc *sc);
-int vio_wait_ctrl_done(struct vio_softc *sc);
+int vio_ctrl_start(struct vio_softc *, uint8_t, uint8_t, int, int *);
+int vio_ctrl_submit(struct vio_softc *, int);
+void vio_ctrl_finish(struct vio_softc *);
void vio_ctrl_wakeup(struct vio_softc *, enum vio_ctrl_state);
int vio_alloc_mem(struct vio_softc *);
int vio_alloc_dmamem(struct vio_softc *);
@@ -1483,59 +1484,109 @@ vio_tx_drain(struct vio_softc *sc)
/*
* Control vq
*/
-/* issue a VIRTIO_NET_CTRL_RX class command and wait for completion */
int
-vio_ctrl_rx(struct vio_softc *sc, int cmd, int onoff)
+vio_ctrl_start(struct vio_softc *sc, uint8_t class, uint8_t cmd, int nslots,
+ int *slotp)
{
struct virtio_softc *vsc = sc->sc_virtio;
struct virtqueue *vq = sc->sc_ctl_vq;
- int r, slot;
+ int r;
splassert(IPL_NET);
- if ((r = vio_wait_ctrl(sc)) != 0)
- return r;
+ while (sc->sc_ctrl_inuse != FREE) {
+ if (sc->sc_ctrl_inuse == RESET || vio_needs_reset(sc))
+ return ENXIO;
+ r = tsleep_nsec(&sc->sc_ctrl_inuse, PRIBIO, "viowait", INFSLP);
+ }
+ sc->sc_ctrl_inuse = INUSE;
- sc->sc_ctrl_cmd->class = VIRTIO_NET_CTRL_RX;
+ sc->sc_ctrl_cmd->class = class;
sc->sc_ctrl_cmd->command = cmd;
- sc->sc_ctrl_rx->onoff = onoff;
- r = virtio_enqueue_prep(vq, &slot);
+ r = virtio_enqueue_prep(vq, slotp);
if (r != 0)
panic("%s: %s virtio_enqueue_prep: control vq busy",
sc->sc_dev.dv_xname, __func__);
- r = virtio_enqueue_reserve(vq, slot, 3);
+ r = virtio_enqueue_reserve(vq, *slotp, nslots + 2);
if (r != 0)
panic("%s: %s virtio_enqueue_reserve: control vq busy",
sc->sc_dev.dv_xname, __func__);
- vio_dmamem_enqueue(vsc, sc, vq, slot, sc->sc_ctrl_cmd,
+
+ vio_dmamem_enqueue(vsc, sc, vq, *slotp, sc->sc_ctrl_cmd,
sizeof(*sc->sc_ctrl_cmd), 1);
- vio_dmamem_enqueue(vsc, sc, vq, slot, sc->sc_ctrl_rx,
- sizeof(*sc->sc_ctrl_rx), 1);
+
+ return 0;
+}
+
+int
+vio_ctrl_submit(struct vio_softc *sc, int slot)
+{
+ struct virtio_softc *vsc = sc->sc_virtio;
+ struct virtqueue *vq = sc->sc_ctl_vq;
+ int r;
+
vio_dmamem_enqueue(vsc, sc, vq, slot, sc->sc_ctrl_status,
sizeof(*sc->sc_ctrl_status), 0);
+
virtio_enqueue_commit(vsc, vq, slot, 1);
- if ((r = vio_wait_ctrl_done(sc)) != 0)
- goto out;
+ while (sc->sc_ctrl_inuse != DONE) {
+ if (sc->sc_ctrl_inuse == RESET || vio_needs_reset(sc))
+ return ENXIO;
+ r = tsleep_nsec(&sc->sc_ctrl_inuse, PRIBIO, "viodone",
+ VIRTIO_NET_CTRL_TIMEOUT);
+ if (r == EWOULDBLOCK) {
+ printf("%s: ctrl queue timeout\n",
+ sc->sc_dev.dv_xname);
+ vio_ctrl_wakeup(sc, RESET);
+ return ENXIO;
+ }
+ }
VIO_DMAMEM_SYNC(vsc, sc, sc->sc_ctrl_cmd,
sizeof(*sc->sc_ctrl_cmd), BUS_DMASYNC_POSTWRITE);
- VIO_DMAMEM_SYNC(vsc, sc, sc->sc_ctrl_rx,
- sizeof(*sc->sc_ctrl_rx), BUS_DMASYNC_POSTWRITE);
VIO_DMAMEM_SYNC(vsc, sc, sc->sc_ctrl_status,
sizeof(*sc->sc_ctrl_status), BUS_DMASYNC_POSTREAD);
- if (sc->sc_ctrl_status->ack == VIRTIO_NET_OK) {
- r = 0;
- } else {
+ r = sc->sc_ctrl_status->ack == VIRTIO_NET_OK ? 0 : EIO;
+
+ return r;
+}
+
+void
+vio_ctrl_finish(struct vio_softc *sc)
+{
+ if (sc->sc_ctrl_inuse != RESET)
+ vio_ctrl_wakeup(sc, FREE);
+}
+
+/* issue a VIRTIO_NET_CTRL_RX class command and wait for completion */
+int
+vio_ctrl_rx(struct vio_softc *sc, int cmd, int onoff)
+{
+ struct virtio_softc *vsc = sc->sc_virtio;
+ struct virtqueue *vq = sc->sc_ctl_vq;
+ int r, slot;
+
+ r = vio_ctrl_start(sc, VIRTIO_NET_CTRL_RX, cmd, 1, &slot);
+ if (r != 0)
+ return r;
+
+ sc->sc_ctrl_rx->onoff = onoff;
+
+ vio_dmamem_enqueue(vsc, sc, vq, slot, sc->sc_ctrl_rx,
+ sizeof(*sc->sc_ctrl_rx), 1);
+
+ r = vio_ctrl_submit(sc, slot);
+ VIO_DMAMEM_SYNC(vsc, sc, sc->sc_ctrl_rx,
+ sizeof(*sc->sc_ctrl_rx), BUS_DMASYNC_POSTWRITE);
+ if (r != 0)
printf("%s: ctrl cmd %d failed\n", sc->sc_dev.dv_xname, cmd);
- r = EIO;
- }
DPRINTF("%s: cmd %d %d: %d\n", __func__, cmd, onoff, r);
-out:
- vio_ctrl_wakeup(sc, FREE);
+
+ vio_ctrl_finish(sc);
return r;
}
@@ -1546,87 +1597,29 @@ vio_ctrl_guest_offloads(struct vio_softc *sc, uint64_t features)
struct virtqueue *vq = sc->sc_ctl_vq;
int r, slot;
- splassert(IPL_NET);
-
- if ((r = vio_wait_ctrl(sc)) != 0)
+ r = vio_ctrl_start(sc, VIRTIO_NET_CTRL_GUEST_OFFLOADS,
+ VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET, 1, &slot);
+ if (r != 0)
return r;
- sc->sc_ctrl_cmd->class = VIRTIO_NET_CTRL_GUEST_OFFLOADS;
- sc->sc_ctrl_cmd->command = VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET;
sc->sc_ctrl_guest_offloads->offloads = features;
- r = virtio_enqueue_prep(vq, &slot);
- if (r != 0)
- panic("%s: %s virtio_enqueue_prep: control vq busy",
- sc->sc_dev.dv_xname, __func__);
- r = virtio_enqueue_reserve(vq, slot, 3);
- if (r != 0)
- panic("%s: %s virtio_enqueue_reserve: control vq busy",
- sc->sc_dev.dv_xname, __func__);
- vio_dmamem_enqueue(vsc, sc, vq, slot, sc->sc_ctrl_cmd,
- sizeof(*sc->sc_ctrl_cmd), 1);
vio_dmamem_enqueue(vsc, sc, vq, slot, sc->sc_ctrl_guest_offloads,
sizeof(*sc->sc_ctrl_guest_offloads), 1);
- vio_dmamem_enqueue(vsc, sc, vq, slot, sc->sc_ctrl_status,
- sizeof(*sc->sc_ctrl_status), 0);
- virtio_enqueue_commit(vsc, vq, slot, 1);
- if ((r = vio_wait_ctrl_done(sc)) != 0)
- goto out;
+ r = vio_ctrl_submit(sc, slot);
- VIO_DMAMEM_SYNC(vsc, sc, sc->sc_ctrl_cmd,
- sizeof(*sc->sc_ctrl_cmd), BUS_DMASYNC_POSTWRITE);
VIO_DMAMEM_SYNC(vsc, sc, sc->sc_ctrl_guest_offloads,
sizeof(*sc->sc_ctrl_guest_offloads), BUS_DMASYNC_POSTWRITE);
- VIO_DMAMEM_SYNC(vsc, sc, sc->sc_ctrl_status,
- sizeof(*sc->sc_ctrl_status), BUS_DMASYNC_POSTREAD);
- if (sc->sc_ctrl_status->ack == VIRTIO_NET_OK) {
- r = 0;
- } else {
+ if (r != 0) {
printf("%s: offload features 0x%llx failed\n",
sc->sc_dev.dv_xname, features);
- r = EIO;
}
- DPRINTF("%s: features 0x%llx: %d\n", __func__, features, r);
- out:
- vio_ctrl_wakeup(sc, FREE);
- return r;
-}
-
-int
-vio_wait_ctrl(struct vio_softc *sc)
-{
- int r = 0;
-
- while (sc->sc_ctrl_inuse != FREE) {
- if (sc->sc_ctrl_inuse == RESET || vio_needs_reset(sc))
- return ENXIO;
- r = tsleep_nsec(&sc->sc_ctrl_inuse, PRIBIO, "viowait", INFSLP);
- }
- sc->sc_ctrl_inuse = INUSE;
-
- return r;
-}
-
-int
-vio_wait_ctrl_done(struct vio_softc *sc)
-{
- int r = 0;
+ DPRINTF("%s: offload features 0x%llx: %d\n", __func__, features, r);
- while (sc->sc_ctrl_inuse != DONE) {
- if (sc->sc_ctrl_inuse == RESET || vio_needs_reset(sc))
- return ENXIO;
- r = tsleep_nsec(&sc->sc_ctrl_inuse, PRIBIO, "viodone",
- VIRTIO_NET_CTRL_TIMEOUT);
- if (r == EWOULDBLOCK) {
- printf("%s: ctrl queue timeout\n",
- sc->sc_dev.dv_xname);
- vio_ctrl_wakeup(sc, RESET);
- return ENXIO;
- }
- }
+ vio_ctrl_finish(sc);
return r;
}
@@ -1665,55 +1658,35 @@ vio_set_rx_filter(struct vio_softc *sc)
struct virtio_softc *vsc = sc->sc_virtio;
struct virtqueue *vq = sc->sc_ctl_vq;
int r, slot;
+ size_t len_uc, len_mc;
- splassert(IPL_NET);
-
- if ((r = vio_wait_ctrl(sc)) != 0)
- return r;
-
- sc->sc_ctrl_cmd->class = VIRTIO_NET_CTRL_MAC;
- sc->sc_ctrl_cmd->command = VIRTIO_NET_CTRL_MAC_TABLE_SET;
- r = virtio_enqueue_prep(vq, &slot);
+ r = vio_ctrl_start(sc, VIRTIO_NET_CTRL_MAC,
+ VIRTIO_NET_CTRL_MAC_TABLE_SET, 2, &slot);
if (r != 0)
- panic("%s: %s virtio_enqueue_prep: control vq busy",
- sc->sc_dev.dv_xname, __func__);
- r = virtio_enqueue_reserve(vq, slot, 4);
- if (r != 0)
- panic("%s: %s virtio_enqueue_reserve: control vq busy",
- sc->sc_dev.dv_xname, __func__);
- vio_dmamem_enqueue(vsc, sc, vq, slot, sc->sc_ctrl_cmd,
- sizeof(*sc->sc_ctrl_cmd), 1);
- vio_dmamem_enqueue(vsc, sc, vq, slot, sc->sc_ctrl_mac_tbl_uc,
- sizeof(*sc->sc_ctrl_mac_tbl_uc) +
- sc->sc_ctrl_mac_tbl_uc->nentries * ETHER_ADDR_LEN, 1);
- vio_dmamem_enqueue(vsc, sc, vq, slot, sc->sc_ctrl_mac_tbl_mc,
- sizeof(*sc->sc_ctrl_mac_tbl_mc) +
- sc->sc_ctrl_mac_tbl_mc->nentries * ETHER_ADDR_LEN, 1);
- vio_dmamem_enqueue(vsc, sc, vq, slot, sc->sc_ctrl_status,
- sizeof(*sc->sc_ctrl_status), 0);
- virtio_enqueue_commit(vsc, vq, slot, 1);
-
- if ((r = vio_wait_ctrl_done(sc)) != 0)
- goto out;
-
- VIO_DMAMEM_SYNC(vsc, sc, sc->sc_ctrl_cmd,
- sizeof(*sc->sc_ctrl_cmd), BUS_DMASYNC_POSTWRITE);
- VIO_DMAMEM_SYNC(vsc, sc, sc->sc_ctrl_mac_info,
- VIO_CTRL_MAC_INFO_SIZE, BUS_DMASYNC_POSTWRITE);
- VIO_DMAMEM_SYNC(vsc, sc, sc->sc_ctrl_status,
- sizeof(*sc->sc_ctrl_status), BUS_DMASYNC_POSTREAD);
+ return r;
- if (sc->sc_ctrl_status->ack == VIRTIO_NET_OK) {
- r = 0;
- } else {
+ len_uc = sizeof(*sc->sc_ctrl_mac_tbl_uc) +
+ sc->sc_ctrl_mac_tbl_uc->nentries * ETHER_ADDR_LEN;
+ len_mc = sizeof(*sc->sc_ctrl_mac_tbl_mc) +
+ sc->sc_ctrl_mac_tbl_mc->nentries * ETHER_ADDR_LEN;
+ vio_dmamem_enqueue(vsc, sc, vq, slot, sc->sc_ctrl_mac_tbl_uc, len_uc,
+ 1);
+ vio_dmamem_enqueue(vsc, sc, vq, slot, sc->sc_ctrl_mac_tbl_mc, len_mc,
+ 1);
+
+ r = vio_ctrl_submit(sc, slot);
+ VIO_DMAMEM_SYNC(vsc, sc, sc->sc_ctrl_mac_tbl_uc, len_uc,
+ BUS_DMASYNC_POSTWRITE);
+ VIO_DMAMEM_SYNC(vsc, sc, sc->sc_ctrl_mac_tbl_mc, len_mc,
+ BUS_DMASYNC_POSTWRITE);
+
+ if (r != 0) {
/* The host's filter table is not large enough */
printf("%s: failed setting rx filter\n", sc->sc_dev.dv_xname);
- r = EIO;
}
-out:
- vio_ctrl_wakeup(sc, FREE);
+ vio_ctrl_finish(sc);
return r;
}
--
2.39.2
vio: Reduce code duplication in control queue handling