Module Name: src
Committed By: martin
Date: Sat Jun 3 14:40:25 UTC 2023
Modified Files:
src/sys/dev/pci [netbsd-10]: virtio.c virtio_pci.c virtiovar.h
src/sys/dev/virtio [netbsd-10]: virtio_mmio.c
Log Message:
Pull up following revision(s) (requested by yamaguchi in ticket #186):
sys/dev/pci/virtio_pci.c: revision 1.41
sys/dev/pci/virtio_pci.c: revision 1.42
sys/dev/virtio/virtio_mmio.c: revision 1.10
sys/dev/pci/virtiovar.h: revision 1.29
sys/dev/pci/virtio.c: revision 1.75
sys/dev/pci/virtio.c: revision 1.76
sys/dev/pci/virtio.c: revision 1.77
sys/dev/pci/virtio.c: revision 1.78
virtio@pci: Fix assertion on detach.
If the child never attached in the first place, it's OK for it to not
have detached.
XXX This should not be a set of flags; this should be a state
enumeration, because some flags make no sense, like FINISHED|FAILED.
XXX This should not be asserted separately in each bus; there should
be a single place in virtio.c to assert this, uniformly in all buses.
PR kern/57357
Use enumeration for state of a child driver instead of flags
and check its detaching by using sc->sc_child in virtio_softc
pointed out by riastradh, thanks.
fixes PR/57357
Fix not to allocate unnecessary descriptor
fixes PR/57358
virtio(4): change variable name, nfc
virtio(4): change members of struct vring_desc_extra before free a slot
This prevents the following race condition.
1. Thread-A: calls virtio_dequeue_commit() and
puts a slot into free descriptor chain in vq_free_slot()
2. Thread-B: calls virtio_enqueue_prep() and get the slot stored by Thread-A
3. Thread-B: calls virtio_enqueue_reserve() and
changes desc_base and desc_free_idx for the slot
4. Thread-A: changes the same members updated by Thread-B
reported by hannken, thanks.
To generate a diff of this commit:
cvs rdiff -u -r1.63.2.4 -r1.63.2.5 src/sys/dev/pci/virtio.c
cvs rdiff -u -r1.38.4.1 -r1.38.4.2 src/sys/dev/pci/virtio_pci.c
cvs rdiff -u -r1.24.4.1 -r1.24.4.2 src/sys/dev/pci/virtiovar.h
cvs rdiff -u -r1.7.4.1 -r1.7.4.2 src/sys/dev/virtio/virtio_mmio.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/pci/virtio.c
diff -u src/sys/dev/pci/virtio.c:1.63.2.4 src/sys/dev/pci/virtio.c:1.63.2.5
--- src/sys/dev/pci/virtio.c:1.63.2.4 Sat May 13 10:56:10 2023
+++ src/sys/dev/pci/virtio.c Sat Jun 3 14:40:25 2023
@@ -1,4 +1,4 @@
-/* $NetBSD: virtio.c,v 1.63.2.4 2023/05/13 10:56:10 martin Exp $ */
+/* $NetBSD: virtio.c,v 1.63.2.5 2023/06/03 14:40:25 martin Exp $ */
/*
* Copyright (c) 2020 The NetBSD Foundation, Inc.
@@ -28,7 +28,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: virtio.c,v 1.63.2.4 2023/05/13 10:56:10 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: virtio.c,v 1.63.2.5 2023/06/03 14:40:25 martin Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -944,12 +944,12 @@ vq_alloc_slot_locked(struct virtio_softc
size_t nslots)
{
struct vring_desc *vd;
- uint16_t rv, tail;
+ uint16_t head, tail;
size_t i;
KASSERT(mutex_owned(&vq->vq_freedesc_lock));
- tail = virtio_rw16(sc, vq->vq_free_idx);
+ head = tail = virtio_rw16(sc, vq->vq_free_idx);
for (i = 0; i < nslots - 1; i++) {
if (tail == VRING_DESC_CHAIN_END)
return VRING_DESC_CHAIN_END;
@@ -962,13 +962,11 @@ vq_alloc_slot_locked(struct virtio_softc
if (tail == VRING_DESC_CHAIN_END)
return VRING_DESC_CHAIN_END;
- rv = virtio_rw16(sc, vq->vq_free_idx);
-
vd = &vq->vq_desc[tail];
vd->flags = virtio_rw16(sc, 0);
vq->vq_free_idx = vd->next;
- return rv;
+ return head;
}
static uint16_t
vq_alloc_slot(struct virtio_softc *sc, struct virtqueue *vq, size_t nslots)
@@ -1096,17 +1094,18 @@ virtio_enqueue_reserve(struct virtio_sof
}
vd[i].flags = virtio_rw16(sc, 0);
} else {
- uint16_t s;
+ if (nsegs > 1) {
+ uint16_t s;
- s = vq_alloc_slot(sc, vq, nsegs - 1);
- if (s == VRING_DESC_CHAIN_END) {
- vq_free_slot(sc, vq, slot);
- return EAGAIN;
+ s = vq_alloc_slot(sc, vq, nsegs - 1);
+ if (s == VRING_DESC_CHAIN_END) {
+ vq_free_slot(sc, vq, slot);
+ return EAGAIN;
+ }
+ vd->next = virtio_rw16(sc, s);
+ vd->flags = virtio_rw16(sc, VRING_DESC_F_NEXT);
}
- vd->next = virtio_rw16(sc, s);
- vd->flags = virtio_rw16(sc, VRING_DESC_F_NEXT);
-
vdx->desc_base = &vq->vq_desc[0];
vdx->desc_free_idx = slot;
}
@@ -1259,12 +1258,12 @@ virtio_enqueue_abort(struct virtio_softc
{
struct vring_desc_extra *vdx;
- vq_free_slot(sc, vq, slot);
-
vdx = &vq->vq_descx[slot];
vdx->desc_free_idx = VRING_DESC_CHAIN_END;
vdx->desc_base = NULL;
+ vq_free_slot(sc, vq, slot);
+
return 0;
}
@@ -1309,12 +1308,12 @@ virtio_dequeue_commit(struct virtio_soft
{
struct vring_desc_extra *vdx;
- vq_free_slot(sc, vq, slot);
-
vdx = &vq->vq_descx[slot];
vdx->desc_base = NULL;
vdx->desc_free_idx = VRING_DESC_CHAIN_END;
+ vq_free_slot(sc, vq, slot);
+
return 0;
}
@@ -1328,7 +1327,7 @@ virtio_child_attach_start(struct virtio_
char buf[1024];
KASSERT(sc->sc_child == NULL);
- KASSERT(!ISSET(sc->sc_child_flags, VIRTIO_CHILD_DETACHED));
+ KASSERT(sc->sc_child_state == VIRTIO_NO_CHILD);
sc->sc_child = child;
sc->sc_ipl = ipl;
@@ -1404,7 +1403,7 @@ virtio_child_attach_finish(struct virtio
}
}
- SET(sc->sc_child_flags, VIRTIO_CHILD_ATTACH_FINISHED);
+ sc->sc_child_state = VIRTIO_CHILD_ATTACH_FINISHED;
virtio_set_status(sc, VIRTIO_CONFIG_DEVICE_STATUS_DRIVER_OK);
return 0;
@@ -1425,10 +1424,9 @@ virtio_child_detach(struct virtio_softc
{
/* already detached */
- if (ISSET(sc->sc_child_flags, VIRTIO_CHILD_DETACHED))
+ if (sc->sc_child == NULL)
return;
- sc->sc_vqs = NULL;
virtio_device_reset(sc);
@@ -1439,7 +1437,8 @@ virtio_child_detach(struct virtio_softc
sc->sc_soft_ih = NULL;
}
- SET(sc->sc_child_flags, VIRTIO_CHILD_DETACHED);
+ sc->sc_vqs = NULL;
+ sc->sc_child = NULL;
}
void
@@ -1449,7 +1448,7 @@ virtio_child_attach_failed(struct virtio
virtio_set_status(sc, VIRTIO_CONFIG_DEVICE_STATUS_FAILED);
- SET(sc->sc_child_flags, VIRTIO_CHILD_ATTACH_FAILED);
+ sc->sc_child_state = VIRTIO_CHILD_ATTACH_FAILED;
}
bus_dma_tag_t
@@ -1485,19 +1484,29 @@ virtio_attach_failed(struct virtio_softc
if (sc->sc_childdevid == 0)
return 1;
- if (ISSET(sc->sc_child_flags, VIRTIO_CHILD_ATTACH_FAILED)) {
- aprint_error_dev(self, "virtio configuration failed\n");
- return 1;
- }
-
if (sc->sc_child == NULL) {
- aprint_error_dev(self,
- "no matching child driver; not configured\n");
+ switch (sc->sc_child_state) {
+ case VIRTIO_CHILD_ATTACH_FAILED:
+ aprint_error_dev(self,
+ "virtio configuration failed\n");
+ break;
+ case VIRTIO_NO_CHILD:
+ aprint_error_dev(self,
+ "no matching child driver; not configured\n");
+ break;
+ default:
+ /* sanity check */
+ aprint_error_dev(self,
+ "virtio internal error, "
+ "child driver is not configured\n");
+ break;
+ }
+
return 1;
}
/* sanity check */
- if (!ISSET(sc->sc_child_flags, VIRTIO_CHILD_ATTACH_FINISHED)) {
+ if (sc->sc_child_state != VIRTIO_CHILD_ATTACH_FINISHED) {
aprint_error_dev(self, "virtio internal error, child driver "
"signaled OK but didn't initialize interrupts\n");
return 1;
Index: src/sys/dev/pci/virtio_pci.c
diff -u src/sys/dev/pci/virtio_pci.c:1.38.4.1 src/sys/dev/pci/virtio_pci.c:1.38.4.2
--- src/sys/dev/pci/virtio_pci.c:1.38.4.1 Sat May 13 10:56:10 2023
+++ src/sys/dev/pci/virtio_pci.c Sat Jun 3 14:40:25 2023
@@ -1,4 +1,4 @@
-/* $NetBSD: virtio_pci.c,v 1.38.4.1 2023/05/13 10:56:10 martin Exp $ */
+/* $NetBSD: virtio_pci.c,v 1.38.4.2 2023/06/03 14:40:25 martin Exp $ */
/*
* Copyright (c) 2020 The NetBSD Foundation, Inc.
@@ -28,7 +28,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: virtio_pci.c,v 1.38.4.1 2023/05/13 10:56:10 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: virtio_pci.c,v 1.38.4.2 2023/06/03 14:40:25 martin Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -333,8 +333,8 @@ virtio_pci_detach(device_t self, int fla
if (r != 0)
return r;
- /* Check that child detached properly */
- KASSERT(ISSET(sc->sc_child_flags, VIRTIO_CHILD_DETACHED));
+ /* Check that child never attached, or detached properly */
+ KASSERT(sc->sc_child == NULL);
KASSERT(sc->sc_vqs == NULL);
KASSERT(psc->sc_ihs_num == 0);
Index: src/sys/dev/pci/virtiovar.h
diff -u src/sys/dev/pci/virtiovar.h:1.24.4.1 src/sys/dev/pci/virtiovar.h:1.24.4.2
--- src/sys/dev/pci/virtiovar.h:1.24.4.1 Sat May 13 10:56:10 2023
+++ src/sys/dev/pci/virtiovar.h Sat Jun 3 14:40:25 2023
@@ -1,4 +1,4 @@
-/* $NetBSD: virtiovar.h,v 1.24.4.1 2023/05/13 10:56:10 martin Exp $ */
+/* $NetBSD: virtiovar.h,v 1.24.4.2 2023/06/03 14:40:25 martin Exp $ */
/*
* Copyright (c) 2010 Minoura Makoto.
@@ -164,10 +164,11 @@ struct virtio_softc {
int sc_childdevid;
device_t sc_child; /* set by child */
- uint32_t sc_child_flags;
-#define VIRTIO_CHILD_ATTACH_FINISHED __BIT(0)
-#define VIRTIO_CHILD_ATTACH_FAILED __BIT(1)
-#define VIRTIO_CHILD_DETACHED __BIT(2)
+ enum {
+ VIRTIO_NO_CHILD = 0,
+ VIRTIO_CHILD_ATTACH_FINISHED,
+ VIRTIO_CHILD_ATTACH_FAILED
+ } sc_child_state;
virtio_callback sc_config_change; /* set by child */
virtio_callback sc_intrhand;
Index: src/sys/dev/virtio/virtio_mmio.c
diff -u src/sys/dev/virtio/virtio_mmio.c:1.7.4.1 src/sys/dev/virtio/virtio_mmio.c:1.7.4.2
--- src/sys/dev/virtio/virtio_mmio.c:1.7.4.1 Sat May 13 10:56:10 2023
+++ src/sys/dev/virtio/virtio_mmio.c Sat Jun 3 14:40:25 2023
@@ -1,4 +1,4 @@
-/* $NetBSD: virtio_mmio.c,v 1.7.4.1 2023/05/13 10:56:10 martin Exp $ */
+/* $NetBSD: virtio_mmio.c,v 1.7.4.2 2023/06/03 14:40:25 martin Exp $ */
/* $OpenBSD: virtio_mmio.c,v 1.2 2017/02/24 17:12:31 patrick Exp $ */
/*
@@ -29,7 +29,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: virtio_mmio.c,v 1.7.4.1 2023/05/13 10:56:10 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: virtio_mmio.c,v 1.7.4.2 2023/06/03 14:40:25 martin Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -217,7 +217,7 @@ virtio_mmio_common_detach(struct virtio_
if (r != 0)
return r;
- KASSERT(ISSET(vsc->sc_child_flags, VIRTIO_CHILD_DETACHED));
+ KASSERT(vsc->sc_child == NULL);
KASSERT(vsc->sc_vqs == NULL);
KASSERT(sc->sc_ih == NULL);
