Re: kern/57304: Recent updates to virtio trigger KASSERT with virtio_mmio on aarch64

[Shoichi Yamaguchi <yamaguchi%netbsd.org@localhost>]

On Tue, Mar 28, 2023 at 3:10 PM Nick Hudson <nick.hudson%gmx.co.uk@localhost> wrote:
>
>  Bisecting shows this change as the first bad commit
>
>  Author: yamaguchi <yamaguchi%NetBSD.org@localhost>
>  Date:   Thu Mar 23 03:55:11 2023 +0000
>
>       Added functions to set interrupt handler and index into virtqueue
>

Thank you for your bisecting. It was very helpful for me.
Could you apply the following patch?

----- patch -----
diff --git a/sys/dev/pci/virtio.c b/sys/dev/pci/virtio.c
index bfe3b28d6a92..a4938a701024 100644
--- a/sys/dev/pci/virtio.c
+++ b/sys/dev/pci/virtio.c
@@ -765,7 +765,7 @@ int
 virtio_alloc_vq(struct virtio_softc *sc, struct virtqueue *vq,
     int maxsegsize, int maxnsegs, const char *name)
 {
- bus_size_t size_desc, size_avail, size_used, size_indirect;
+ bus_size_t size_used, size_indirect;
  bus_size_t allocsize = 0, size_desc_avail;
  int rsegs, r, hdrlen;
  unsigned int vq_num;
@@ -782,16 +782,17 @@ virtio_alloc_vq(struct virtio_softc *sc, struct
virtqueue *vq,

  hdrlen = sc->sc_active_features & VIRTIO_F_RING_EVENT_IDX ? 3 : 2;

- size_desc = sizeof(vq->vq_desc[0]) * vq_num;
- size_avail = sizeof(uint16_t) * hdrlen
-    + sizeof(vq->vq_avail[0].ring) * vq_num;
- size_used = sizeof(uint16_t) *hdrlen
-    + sizeof(vq->vq_used[0].ring) * vq_num;
- size_indirect = (sc->sc_indirect && maxnsegs >= MINSEG_INDIRECT) ?
-    sizeof(struct vring_desc) * maxnsegs * vq_num : 0;
-
- size_desc_avail = VIRTQUEUE_ALIGN(size_desc + size_avail);
- size_used = VIRTQUEUE_ALIGN(size_used);
+ size_desc_avail = VIRTQUEUE_ALIGN(
+    sizeof(struct vring_desc) * vq_num
+    + sizeof(uint16_t) * (hdrlen + vq_num));
+ size_used = VIRTQUEUE_ALIGN(
+    sizeof(uint16_t) * hdrlen
+    + sizeof(struct vring_used_elem) * vq_num);
+ if (sc->sc_indirect && maxnsegs >= MINSEG_INDIRECT) {
+ size_indirect = sizeof(struct vring_desc) * maxnsegs * vq_num;
+ } else {
+ size_indirect = 0;
+ }

  allocsize = size_desc_avail + size_used + size_indirect;

@@ -836,24 +837,22 @@ virtio_alloc_vq(struct virtio_softc *sc, struct
virtqueue *vq,
  vq->vq_maxsegsize = maxsegsize;
  vq->vq_maxnsegs = maxnsegs;

-#define VIRTIO_PTR(base, offset) (void *)((intptr_t)(base) + (offset))
- /* initialize vring pointers */
- vq->vq_desc = VIRTIO_PTR(vq->vq_vaddr, 0);
- vq->vq_availoffset = size_desc;
- vq->vq_avail = VIRTIO_PTR(vq->vq_vaddr, vq->vq_availoffset);
- vq->vq_used_event = VIRTIO_PTR(vq->vq_avail,
-    offsetof(struct vring_avail, ring[vq_num]));
+ vq->vq_availoffset = sizeof(struct vring_desc) * vq_num;
  vq->vq_usedoffset = size_desc_avail;
- vq->vq_used = VIRTIO_PTR(vq->vq_vaddr, vq->vq_usedoffset);
- vq->vq_avail_event = VIRTIO_PTR(vq->vq_used,
-    offsetof(struct vring_used, ring[vq_num]));
+
+ vq->vq_desc = vq->vq_vaddr;
+ vq->vq_avail = (void *)(((char *)vq->vq_desc) + vq->vq_availoffset);
+ vq->vq_used_event = (uint16_t *)((char *)vq->vq_avail +
+    offsetof(struct vring_avail, ring[vq->vq_num]));
+ vq->vq_used = (void *)(((char *)vq->vq_desc) + vq->vq_usedoffset);
+ vq->vq_avail_event = (uint16_t *)((char *)vq->vq_used +
+    offsetof(struct vring_used, ring[vq->vq_num]));

  if (size_indirect > 0) {
  vq->vq_indirectoffset = size_desc_avail + size_used;
- vq->vq_indirect = VIRTIO_PTR(vq->vq_vaddr,
-    vq->vq_indirectoffset);
+ vq->vq_indirect = (void *)(((char *)vq->vq_desc)
+    + vq->vq_indirectoffset);
  }
-#undef VIRTIO_PTR

  /* free slot management */
  vq->vq_entries = kmem_zalloc(sizeof(struct vq_entry) * vq_num,