Commits
f0b002d01d5 "Release the netlock when sleeping for control
messages in in vioioctl()" and
126b881f71 "Insert a workaround for
per-ifp ioctl being called w/o NET_LOCK()." in vio(4) fixed a deadlock
but may cause a crash with a protection fault trap if addresses are
added/removed concurrently.
The actual issue is that signals are not handled correctly while
sleeping. After a signal, there is a race condition where sc_ctrl_inuse
is first set to FREE and then the interrupt handler sets it to DONE,
causing a hang in the next vio_wait_ctrl() call.
To fix it:
* Revert the NET_LOCK unlocking work-around.
* Remove PCATCH from the sleep call when we wait for control queue,
avoiding the race with vio_ctrleof(). To ensure that we don't hang
forever, use a 5 second timeout.
* If the timeout is hit, or if the hypervisor has set the
DEVICE_NEEDS_RESET status bit, do not try to use the control queue
until the next ifconfig down/up which resets the device.
* In order to allow reading the device status from device drivers, add a
new interface to the virtio transport drivers.
* Avoid a crash if there is outgoing traffic while doing ifconfig down.
OK bluhm@
-/* $OpenBSD: virtio_mmio.c,v 1.12 2024/01/15 02:35:23 dv Exp $ */
+/* $OpenBSD: virtio_mmio.c,v 1.13 2024/05/17 16:37:10 sf Exp $ */
/* $NetBSD: virtio.c,v 1.3 2011/11/02 23:05:52 njoly Exp $ */
/*
void virtio_mmio_write_device_config_8(struct virtio_softc *, int, uint64_t);
uint16_t virtio_mmio_read_queue_size(struct virtio_softc *, uint16_t);
void virtio_mmio_setup_queue(struct virtio_softc *, struct virtqueue *, uint64_t);
+int virtio_mmio_get_status(struct virtio_softc *);
void virtio_mmio_set_status(struct virtio_softc *, int);
int virtio_mmio_negotiate_features(struct virtio_softc *,
const struct virtio_feature_name *);
virtio_mmio_write_device_config_8,
virtio_mmio_read_queue_size,
virtio_mmio_setup_queue,
+ virtio_mmio_get_status,
virtio_mmio_set_status,
virtio_mmio_negotiate_features,
virtio_mmio_intr,
}
}
+int
+virtio_mmio_get_status(struct virtio_softc *vsc)
+{
+ struct virtio_mmio_softc *sc = (struct virtio_mmio_softc *)vsc;
+
+ return bus_space_read_4(sc->sc_iot, sc->sc_ioh,
+ VIRTIO_MMIO_STATUS);
+}
+
void
virtio_mmio_set_status(struct virtio_softc *vsc, int status)
{
-/* $OpenBSD: virtio_pci.c,v 1.36 2024/01/15 02:35:23 dv Exp $ */
+/* $OpenBSD: virtio_pci.c,v 1.37 2024/05/17 16:37:10 sf Exp $ */
/* $NetBSD: virtio.c,v 1.3 2011/11/02 23:05:52 njoly Exp $ */
/*
void virtio_pci_write_device_config_8(struct virtio_softc *, int, uint64_t);
uint16_t virtio_pci_read_queue_size(struct virtio_softc *, uint16_t);
void virtio_pci_setup_queue(struct virtio_softc *, struct virtqueue *, uint64_t);
+int virtio_pci_get_status(struct virtio_softc *);
void virtio_pci_set_status(struct virtio_softc *, int);
int virtio_pci_negotiate_features(struct virtio_softc *, const struct virtio_feature_name *);
int virtio_pci_negotiate_features_10(struct virtio_softc *, const struct virtio_feature_name *);
virtio_pci_write_device_config_8,
virtio_pci_read_queue_size,
virtio_pci_setup_queue,
+ virtio_pci_get_status,
virtio_pci_set_status,
virtio_pci_negotiate_features,
virtio_pci_poll_intr,
}
}
+int
+virtio_pci_get_status(struct virtio_softc *vsc)
+{
+ struct virtio_pci_softc *sc = (struct virtio_pci_softc *)vsc;
+
+ if (sc->sc_sc.sc_version_1)
+ return CREAD(sc, device_status);
+ else
+ return bus_space_read_1(sc->sc_iot, sc->sc_ioh,
+ VIRTIO_CONFIG_DEVICE_STATUS);
+}
+
void
virtio_pci_set_status(struct virtio_softc *vsc, int status)
{
-/* $OpenBSD: if_vio.c,v 1.33 2024/05/07 18:35:23 jan Exp $ */
+/* $OpenBSD: if_vio.c,v 1.34 2024/05/17 16:37:10 sf Exp $ */
/*
* Copyright (c) 2012 Stefan Fritsch, Alexander Fiveg.
#define VIRTIO_NET_TX_MAXNSEGS 16 /* for larger chains, defrag */
#define VIRTIO_NET_CTRL_MAC_MC_ENTRIES 64 /* for more entries, use ALLMULTI */
#define VIRTIO_NET_CTRL_MAC_UC_ENTRIES 1 /* one entry for own unicast addr */
+#define VIRTIO_NET_CTRL_TIMEOUT (5*1000*1000*1000ULL) /* 5 seconds */
#define VIO_CTRL_MAC_INFO_SIZE \
(2*sizeof(struct virtio_net_ctrl_mac_tbl) + \
}
}
+static int vio_needs_reset(struct vio_softc *sc)
+{
+ if (virtio_get_status(sc->sc_virtio) &
+ VIRTIO_CONFIG_DEVICE_STATUS_DEVICE_NEEDS_RESET) {
+ printf("%s: device needs reset", sc->sc_dev.dv_xname);
+ vio_ctrl_wakeup(sc, RESET);
+ return 1;
+ }
+ return 0;
+}
+
void
vio_attach(struct device *parent, struct device *self, void *aux)
{
{
struct vio_softc *sc = (struct vio_softc *)vsc->sc_child;
vio_link_state(&sc->sc_ac.ac_if);
+ vio_needs_reset(sc);
return 1;
}
virtio_reset(vsc);
vio_rxeof(sc);
if (vsc->sc_nvqs >= 3)
- vio_ctrleof(&sc->sc_vq[VQCTL]);
+ vio_ctrl_wakeup(sc, RESET);
vio_tx_drain(sc);
if (disable)
vio_rx_drain(sc);
if (vsc->sc_nvqs >= 3)
virtio_start_vq_intr(vsc, &sc->sc_vq[VQCTL]);
virtio_reinit_end(vsc);
- if (vsc->sc_nvqs >= 3) {
- if (sc->sc_ctrl_inuse != FREE)
- sc->sc_ctrl_inuse = RESET;
- wakeup(&sc->sc_ctrl_inuse);
- }
+ if (vsc->sc_nvqs >= 3)
+ vio_ctrl_wakeup(sc, FREE);
}
static inline uint16_t
int r = 0;
int slot, len;
+ if (!ISSET(ifp->if_flags, IFF_RUNNING))
+ return 0;
+
while (virtio_dequeue(vsc, vq, &slot, &len) == 0) {
struct virtio_net_hdr *hdr = &sc->sc_tx_hdrs[slot];
r++;
return r;
}
-/*
- * XXXSMP As long as some per-ifp ioctl(2)s are executed with the
- * NET_LOCK() deadlocks are possible. So release it here.
- */
-static inline int
-vio_sleep(struct vio_softc *sc, const char *wmesg)
-{
- int status = rw_status(&netlock);
-
- if (status != RW_WRITE && status != RW_READ)
- return tsleep_nsec(&sc->sc_ctrl_inuse, PRIBIO|PCATCH, wmesg,
- INFSLP);
-
- return rwsleep_nsec(&sc->sc_ctrl_inuse, &netlock, PRIBIO|PCATCH, wmesg,
- INFSLP);
-}
-
int
vio_wait_ctrl(struct vio_softc *sc)
{
int r = 0;
while (sc->sc_ctrl_inuse != FREE) {
- r = vio_sleep(sc, "viowait");
- if (r == EINTR)
- return r;
+ 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;
{
int r = 0;
- while (sc->sc_ctrl_inuse != DONE && sc->sc_ctrl_inuse != RESET) {
- if (sc->sc_ctrl_inuse == RESET) {
- r = 1;
- break;
+ 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", sc->sc_dev.dv_xname);
+ vio_ctrl_wakeup(sc, RESET);
+ return ENXIO;
}
- r = vio_sleep(sc, "viodone");
- if (r == EINTR)
- break;
}
return r;
}
-/* $OpenBSD: virtiovar.h,v 1.17 2024/05/13 01:15:51 jsg Exp $ */
+/* $OpenBSD: virtiovar.h,v 1.18 2024/05/17 16:37:10 sf Exp $ */
/* $NetBSD: virtiovar.h,v 1.1 2011/10/30 12:12:21 hannken Exp $ */
/*
void (*write_dev_cfg_8)(struct virtio_softc *, int, uint64_t);
uint16_t (*read_queue_size)(struct virtio_softc *, uint16_t);
void (*setup_queue)(struct virtio_softc *, struct virtqueue *, uint64_t);
+ int (*get_status)(struct virtio_softc *);
void (*set_status)(struct virtio_softc *, int);
int (*neg_features)(struct virtio_softc *, const struct virtio_feature_name *);
int (*poll_intr)(void *);
#define virtio_setup_queue(sc, i, v) (sc)->sc_ops->setup_queue(sc, i, v)
#define virtio_negotiate_features(sc, n) (sc)->sc_ops->neg_features(sc, n)
#define virtio_poll_intr(sc) (sc)->sc_ops->poll_intr(sc)
+#define virtio_get_status(sc) (sc)->sc_ops->get_status(sc)
+#define virtio_set_status(sc, i) (sc)->sc_ops->set_status(sc, i)
/* only for transport drivers */
-#define virtio_set_status(sc, i) (sc)->sc_ops->set_status(sc, i)
#define virtio_device_reset(sc) virtio_set_status((sc), 0)
static inline int
projects / openbsd / commitdiff