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

["Izumi Tsutsui via gnats" <gnats-admin%NetBSD.org@localhost>]

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

From: Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>
To: riastradh%NetBSD.org@localhost
Cc: thorpej%me.com@localhost, isaki%pastel-flower.jp@localhost, nick.hudson%gmx.co.uk@localhost,
        gnats-bugs%netbsd.org@localhost, kern-bug-people%netbsd.org@localhost,
        netbsd-bugs%netbsd.org@localhost, tsutsui%ceres.dti.ne.jp@localhost
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Tue, 12 May 2026 01:38:08 +0900

 riastradh@ wrote:
 
 > What I'm still not sure about -- until further thought after a lunch
 > barrier at least -- is:
 > 
 > 1. Does a _polling loop_ require additional PREREADs?
 > 
 > 	PREREAD;
 > 	trigger DMA;
 > 	POSTREAD;
 > 	while (!buf->ready) {
 > 		PREREAD;	// <--- ???
 > 		POSTREAD;
 > 	}
 > 	POSTREAD;
 > 	use(buf->data);
 > 
 >    And, can the rule about this be phrased in terms of higher-level
 >    semantics of DMA operations than details of the 68030 cache
 >    implementation?
 
 I would like to first clarify the PREREAD/POSTREAD rule for a polling
 loop, because I think this is not specific to virtio.
 
 For a DMA READ buffer, my understanding is that POSTREAD allows the CPU
 to inspect the buffer.  If the device may still write to the same buffer
 after that, the driver should issue PREREAD again before the next device
 update may happen.
 
 This seems to match existing drivers.  For example, src/sys/dev/ici82557.c
 does this kind of sequence while polling command completion:
 
 ```
         for (i = 1000; i > 0; i--) {
                 FXP_CDCONFIGSYNC(sc,
                     BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
                 status = le16toh(cbp->cb_status);
                 FXP_CDCONFIGSYNC(sc, BUS_DMASYNC_PREREAD);
                 if ((status & FXP_CB_STATUS_C) != 0)
                         break;
                 DELAY(1);
         }
 ```
 
 and IIRC these PREREAD ops were necessary for sgimips O2 (which had
 non-COHERENT DMA).
 
 So my current understanding is that this is an existing bus_dma ownership
 handoff pattern, not a virtio-specific special case.
 
 > 2. Does the VIRTIO_F_RING_EVENT_IDX case require PREREAD too?
 > 
 > --- src/sys/dev/pci/virtio.c
 > +++ src/sys/dev/pci/virtio.c
 > @@ -1269,6 +1269,7 @@
 >  		if (sc->sc_active_features & VIRTIO_F_RING_EVENT_IDX) {
 >  			vq_sync_uring_avail(sc, vq, BUS_DMASYNC_POSTREAD);
 >  			t = virtio_rw16(sc, *vq->vq_avail_event) + 1;
 > +			vq_sync_uring_avail(sc, vq, BUS_DMASYNC_PREREAD);
 >  			if ((uint16_t) (n - t) < (uint16_t) (n - o))
 >  				sc->sc_ops->kick(sc, vq->vq_index);
 >  		} else {
 
 I think this is correct.
 
 ---
 Izumi Tsutsui