Re: kern/60144: virtio(4) cache coherence issue

["Tetsuya Isaki via gnats" <gnats-admin%NetBSD.org@localhost>]

The following reply was made to PR kern/60144; it has been noted by GNATS.

From: Tetsuya Isaki <isaki%pastel-flower.jp@localhost>
To: Jason Thorpe <thorpej%me.com@localhost>, gnats-bugs%netbsd.org@localhost,
	kern-bug-people%netbsd.org@localhost,
	netbsd-bugs%netbsd.org@localhost
Cc: 
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Fri, 10 Apr 2026 13:58:30 +0900

 At Thu, 2 Apr 2026 05:42:10 -0700,
 Jason Thorpe wrote:
 > If that=A2s the case, then I=A2d be concerned that there=A2s some other
 > issue in the virtio code that could cause a problem on 68040 as
 > well, because what it indicates is that, after the cache was
 > invalidated, something accessed the relevant memory (or memory
 > adjacent enough to the relevant memory) that caused a cache line
 > fill to take place, thus putting data that=A2s about to be stale back
 > into the cache.
 
 Thank you for your suggestion.
 I found it.
 
 sys/dev/pci/virtio.c:
  1221 int
  1222 virtio_enqueue_commit(struct virtio_softc *sc, struct virtqueue *vq, =
 int
  1222  slot,
  1223     bool notifynow)
  1224 {
   :
  1247         /*
  1248          * Prepare for `device->CPU' (host->guest) transfer
  1249          * into the buffer.  This must happen before we commit
  1250          * the vq->vq_avail->idx update to ensure we're not
  1251          * still using the buffer in case program-prior loads
  1252          * or stores in it get delayed past the store to
  1253          * vq->vq_avail->idx.
  1254          */
  1255         vq_sync_uring_all(sc, vq, BUS_DMASYNC_PREREAD);
 
    ^^-- Issues PREREAD.  On 68030/-current, it invalidates the data cache
         (containing vq->vq_used at least).
 
  1256
  1257         /* ensure payload is published, then avail idx */
  1258         vq_sync_aring_payload(sc, vq, BUS_DMASYNC_PREWRITE);
  1259         vq->vq_avail->idx =3D virtio_rw16(sc, vq->vq_avail_idx);
  1260         vq_sync_aring_header(sc, vq, BUS_DMASYNC_PREWRITE);
  1261         vq->vq_queued++;
  1262
  1263         /*
  1264          * Ensure we publish the avail idx _before_ we check whether
  1265          * the host needs to notified.
  1266          */
  1267         paravirt_membar_sync();
  1268
  1269         if (sc->sc_active_features & VIRTIO_F_RING_EVENT_IDX) {
  1270             vq_sync_uring_avail(sc, vq, BUS_DMASYNC_POSTREAD);
  1271             t =3D virtio_rw16(sc, *vq->vq_avail_event) + 1;
  1272             if ((uint16_t) (n - t) < (uint16_t) (n - o))
  1273                 sc->sc_ops->kick(sc, vq->vq_index);
  1274         } else {
  1275             vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
  1276             flags =3D virtio_rw16(sc, vq->vq_used->flags);
 
    ^^-- Reads vq->vq_used region again(!!)
         See text below.
 
  1277             if (!(flags & VRING_USED_F_NO_NOTIFY))
  1278                 sc->sc_ops->kick(sc, vq->vq_index);
 
    ^^-- And then starts the virtio HW process.
 
  1279         }
  1280     }
  1281     mutex_exit(&vq->vq_aring_lock);
  1282
  1283     return 0;
  1284 }
 =20
 
 vq->vq_used is shared region between the driver(CPU) and the device(HW),
 and is defined as:
 
   struct vring_used {
         uint16_t flags;
         uint16_t idx;
         struct vring_used_elem ring[];
 	/* trailed by uint16_t avail_event when VIRTIO_F_RING_EVENT_IDX */
   } __packed;
 
 Reading vq->vq_used->flags here will also load the adjacent
 vq->vq_used->idx into the cache (at least on the 68030 cache system).
 This cached idx may still remain in the cache in the next interrupt
 handler even though the virtio device has already updated idx in memory.
 ---
 Tetsuya Isaki <isaki%pastel-flower.jp@localhost / isaki%NetBSD.org@localhost>