NetBSD Problem Report #60144
From www@netbsd.org Mon Mar 30 02:53:01 2026
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits)
client-signature RSA-PSS (2048 bits))
(Client CN "mail.netbsd.org", Issuer "R12" (not verified))
by mollari.NetBSD.org (Postfix) with ESMTPS id BBE0D1A9239
for <gnats-bugs@gnats.NetBSD.org>; Mon, 30 Mar 2026 02:53:01 +0000 (UTC)
Message-Id: <20260330025300.BD68D1A923E@mollari.NetBSD.org>
Date: Mon, 30 Mar 2026 02:53:00 +0000 (UTC)
From: isaki@pastel-flower.jp
Reply-To: isaki@pastel-flower.jp
To: gnats-bugs@NetBSD.org
Subject: virtio(4) cache coherence issue
X-Send-Pr-Version: www-1.0
X-From4GNATS: "isaki@pastel-flower.jp via gnats" <gnats-admin@NetBSD.org>
>Number: 60144
>Notify-List: riastradh@NetBSD.org
>Category: kern
>Synopsis: virtio(4) cache coherence issue
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: kern-bug-people
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Mon Mar 30 02:55:00 +0000 2026
>Closed-Date: Tue Jun 30 07:44:57 +0000 2026
>Last-Modified: Tue Jun 30 07:44:57 +0000 2026
>Originator: Tetsuya Isaki
>Release: NetBSD/virt68k 11.0_RC2
>Organization:
>Environment:
NetBSD 11.0_RC2 virt68k
>Description:
The kernel sometimes hangs up with "Spurious interrupt on CPU ipl 5",
when accessing ld@virtio on virt68k.
This message means "an interrupt occurred, but no handlers took it".
Here, ipl 5 is the interrupt that virtio is assigned (and may be
shared with other devices).
At least in virtio_is_enqueue(), I suspect that there may be some
insufficient cache line invalidation.
sys/dev/pci/virtio.c:
601 virtio_vq_is_enqueued(struct virtio_softc *sc, struct virtqueue *vq)
602 {
603
604 if (vq->vq_queued) {
605 vq->vq_queued = 0;
606 vq_sync_aring_all(sc, vq, BUS_DMASYNC_POSTWRITE);
607 }
608
609 vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
610 if (vq->vq_used_idx == virtio_rw16(sc, vq->vq_used->idx))
611 return 0;
612 vq_sync_uring_payload(sc, vq, BUS_DMASYNC_POSTREAD);
613 return 1;
614 }
The virtio device incremented its own index and wrote it to
vq->vq_used->idx. But when the interrupt was lost, (un)luckily
the data cache still held the previous vq->vq_used->idx, so that
CPU read it from the cache(!).
As you know, the previous vq->vq_used->idx is the same as
vq->vq_used_idx, therefore the function returned 0 (which means
vq is empty), even though the device notified as vq-is-enqueued.
I think that vq_sync_uring_header(sc, vq, BUS_DMASYNC_PREREAD) is
necessary to invalidate the cache line before reading fresh
vq->vq_used->idx (at line 610) ?
This is the only case that I was able to observe by tracing on
emulator. But many other places look similar.
And the following four results I observed also support this assumption.
- qemu (68040, without cache impl.) could not reproduce.
- nono (68030, with cache impl.) could reproduce.
- nono (68040, without cache impl. yet) could not reproduce.
- nono (68030, force disable data cache) could not reproduce.
>How-To-Repeat:
Boot NetBSD/virt68k on emulator which implements a data cache.
Access ld@virtio. But not 100% reproducible.
If someone else updates the same cache line, this problem will not
reproduce. When reproducible, I typically encountered it within
20 sets of the following command pair.
# mount /dev/ld1e /mnt; umount /mnt
:
>Fix:
See above.
>Release-Note:
>Audit-Trail:
From: Taylor R Campbell <riastradh@NetBSD.org>
To: Tetsuya Isaki <isaki@pastel-flower.jp>
Cc: Jason Thorpe <thorpej@NetBSD.org>,
netbsd-bugs@NetBSD.org, gnats-bugs@NetBSD.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Mon, 30 Mar 2026 03:47:22 +0000
> Date: Mon, 30 Mar 2026 02:53:00 +0000 (UTC)
> From: Tetsuya Isaki <isaki@pastel-flower.jp>
>=20
> At least in virtio_is_enqueue(), I suspect that there may be some
> insufficient cache line invalidation.
>=20
> sys/dev/pci/virtio.c:
> 601 virtio_vq_is_enqueued(struct virtio_softc *sc, struct virtqueue *=
vq)
> 602 {
> 603
> 604 if (vq->vq_queued) {
> 605 vq->vq_queued =3D 0;
> 606 vq_sync_aring_all(sc, vq, BUS_DMASYNC_POSTWRITE);
> 607 }
> 608
> 609 vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
> 610 if (vq->vq_used_idx =3D=3D virtio_rw16(sc, vq->vq_used->idx))
> 611 return 0;
> 612 vq_sync_uring_payload(sc, vq, BUS_DMASYNC_POSTREAD);
> 613 return 1;
> 614 }
>=20
> The virtio device incremented its own index and wrote it to
> vq->vq_used->idx. But when the interrupt was lost, (un)luckily
> the data cache still held the previous vq->vq_used->idx, so that
> CPU read it from the cache(!).
> As you know, the previous vq->vq_used->idx is the same as
> vq->vq_used_idx, therefore the function returned 0 (which means
> vq is empty), even though the device notified as vq-is-enqueued.
>=20
> I think that vq_sync_uring_header(sc, vq, BUS_DMASYNC_PREREAD) is
> necessary to invalidate the cache line before reading fresh
> vq->vq_used->idx (at line 610) ?
I believe you have got the usage model of the preread vs postread
wrong -- it is confusing, because `PREREAD' does not mean `before we
read from the buffer' but rather `before the device does a DMA read
operation to initialize the buffer'.
Suppose a typical driver wants to request a DMA operation to read
data, from network or disk or whatever, into a buffer in memory. The
idiom the driver should use is roughly:
0. (allocate buffer)
1. BUS_DMASYNC_PREREAD
2. trigger DMA read operation
3. wait for completion / interrupt handler
4. BUS_DMASYNC_POSTREAD
5. use data in buffer
/*
* 1. Prepare the buffer in memory -- usually, this is a
* matter of forcing any pending stores that the CPU issued
* so that they happen happen before the DMA operation
* begins; this way, they don't accidentally overwrite the
* result of the device's DMA operation.
*
* For bounced DMA operations, no memcpy is needed here.
*/
bus_dmamap_sync(BUS_DMASYNC_PREREAD);
/*
* 2. Trigger the DMA read operation.
*/
bus_space_write(DMA_CTRL_REG, TRIGGER_DMA);
/*
* 3. Wait for the DMA operation to complete.
*/
while (bus_space_read(DMA_STATUS_REG) !=3D DMA_DONE) {
wait for interrupt;
}
/*
* 4. Make sure what we next read out of the buffer is
* up-to-date, i.e., the result of the DMA operation,
* rather than anything the CPU had previously cached from
* the memory where the buffer lives -- usually, this is a
* matter of flushing any cached data so the next time the
* CPU loads from memory, it doesn't get anything stale
* from the cache but reads from main memory instead.
*
* For bounced DMA operations, this is where we memcpy from
* the bounce buffer to the real buffer.
*/
bus_dmamap_sync(BUS_DMASYNC_POSTREAD);
/*
* 5. It is now safe to use the result of the DMA read.
*/
printf("stuff 0x%x\n", buf[0]);
Here, the `DMA read operation' of virtio is to initialize
vq->vq_used->idx.
If some kind of cache invalidation must happen between the interrupt
delivery or DMA_STATUS_REG read at (3), and the use of the data in the
buffer at (5), then the m68k and virt68k bus_dmamap_sync logic might
be missing something for BUS_DMAMAP_POSTREAD -- it seems to have cases
only for BUS_DMAMAP_PREWRITE and BUS_DMAMAP_PREREAD:
https://nxr.NetBSD.org/xref/src/sys/arch/m68k/m68k/bus_dma.c?r=3D1.41#491
https://nxr.NetBSD.org/xref/src/sys/arch/virt68k/virt68k/bus_dma.c?r=3D1.4#=
476
Perhaps m68k needs new POSTREAD logic to be written, or perhaps there
was a mistake and the PREREAD logic should have been for POSTREAD?
PREREAD is generally only needed if the CPU can reorder stores; if the
CPU issues all stores in order (to regular memory and I/O memory
alike), then logic like the following likely doesn't need to do
anything at the PREREAD point:
buf[0] =3D 0xdeadbeef; /* last user of buf */
/* PREREAD */
*(memt + DMA_CTRL_REG) =3D TRIGGER_DMA; /* bus_space_write */
But, for a device that doesn't participate in the CPU's cache
coherence protocol, logic like the following almost always needs
something at the POSTREAD point so the CPU doesn't operate on stale
cached data:
x =3D buf[0]; /* caches buf[0] */
dostuff(x); /* last user of buf */
free(buf);
...
buf =3D alloc(); /* same buf reused */
/* PREREAD */
*(memt + DMA_CTRL_REG) =3D TRIGGER_DMA; /* bus_space_write */
...
/* bus_space_read */
while (*(memt + DMA_STATUS_REG) !=3D DMA_DONE) wait;
/* POSTREAD */
printf("stuff 0x%x\n", buf[0]); /* cached buf[0]? */
From: Jason Thorpe <thorpej@me.com>
To: gnats-bugs@netbsd.org
Cc: kern-bug-people@netbsd.org,
gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Sun, 29 Mar 2026 21:39:43 -0700
> On Mar 29, 2026, at 7:55=E2=80=AFPM, isaki@pastel-flower.jp via gnats =
<gnats-admin@netbsd.org> wrote:
>=20
> And the following four results I observed also support this =
assumption.
> - qemu (68040, without cache impl.) could not reproduce.
> - nono (68030, with cache impl.) could reproduce.
> - nono (68040, without cache impl. yet) could not reproduce.
> - nono (68030, force disable data cache) could not reproduce.
This actually brings up an interesting philosophical question: What does =
DMA coherency mean in the context of virtio?
If you think about whence virtio originated: virtualized devices, the =
same machine, a guest <-> hypervisor communication channel. It=E2=80=99s =
all the same machine, it=E2=80=99s all the same memory. If you think =
about it, it=E2=80=99s inherently coherent, and modeling the cache =
effects of a device doesn=E2=80=99t really seem aligned with what virtio =
**fundamentally is**.
Does that make sense?
-- thorpej
From: Robert Elz <kre@munnari.OZ.AU>
To: Jason Thorpe <thorpej@me.com>
Cc: gnats-bugs@netbsd.org, kern-bug-people@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Mon, 30 Mar 2026 23:41:28 +0700
Date: Sun, 29 Mar 2026 21:39:43 -0700
From: Jason Thorpe <thorpej@me.com>
Message-ID: <52E223DA-EF6B-4266-836F-29955CE7FAAC@me.com>
| This actually brings up an interesting philosophical question:
| What does DMA coherency mean in the context of virtio?
Well, as you suggest, but didn't explicitly say, caching in a virtual
device can't logically be to make anything run faster, so the obvious
interpretation would be that it is intended to assist in finding caching
bugs.
If I were implementing something like that (and I certainly am not), I'd
be modelling all the memory the process generally sees (RAM) as cache,
ie: 100% cache coverage -- all ram is always in "cache".
Then I'd model real ram as only ever used by operations that explicitly
don't use the cache - like device DMA, but possibly also interrupt vector
fetch - anything which the modelled hardware doesn't pass by the cache
(and for devices with multiple cache levels, that means all of them.)
For that I'd only allocate real memory for the non-cache pages which are
actually used - when one is first allocated, I'd simply copy the "cached"
memory into it (as if at that instant, for whatever reason, the cache was
completely consistent) - but after that the only operations which would
ever move data between the external memory (where it has been allocated) and
the cache memory would be those which explicitly affect the cache in the
hardware design - operations which flush a cache line to ram (in real hardware)
would copy that amount of data from the cache to the external ram, and
operations which invalidate the cache (without writeback) would simply
copy the external ram into the cache memory (as if the cache had been
invalidated, and then something magically referenced the relevant cache
lines, and copied the ram data into the cache). But both only when there
is an actual ram page allocated, when not, the cache ops would just be
no-ops.
When modelling a multi-cpu system with caching, I'd allocate entirely
separate memory for each cpu (its cache), but share memory for the external
ram - so any ram intended to be visible to multiple CPUs needs the cache
flush logic implemented properly - in this case I'd probably allocate the
ram backing pages for anything which might be shared between cpus, as well
as when used for dma etc.
So, I'm not sure that the general concept is quite as absurd as it seems
at first glance - it might help locate all kinds of caching issues such
as the one Taylor described.
kre
ps: everyone, when replying to gnats messages, it is a good idea, I
believe, to delete the gnats-admin@ address from the destination fields.
Having it there serves no useful purpose as best I understand things.
From: Jason Thorpe <thorpej@me.com>
To: Robert Elz <kre@munnari.OZ.AU>
Cc: gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Mon, 30 Mar 2026 22:43:11 -0700
> On Mar 30, 2026, at 9:41=E2=80=AFAM, Robert Elz <kre@munnari.OZ.AU> =
wrote:
>=20
> So, I'm not sure that the general concept is quite as absurd as it =
seems
> at first glance - it might help locate all kinds of caching issues =
such
> as the one Taylor described.
While what you say is true, it=E2=80=99s important to remember that one =
of the design goals of VirtIO was to break through performance =
bottlenecks associated with emulating actual hardware devices (which =
includes all of the cache interaction).
I=E2=80=99m not saying that this use if VirtIO couldn=E2=80=99t be =
useful.. but it=E2=80=99s also worth considering that the issues in this =
case may not lie with the bus_dma back-end but rather with the VirtIO =
code in the kernel itself (based on my preliminary conversion with =
Taylor about the problem). It=E2=80=99s been demonstrated by years of =
=E2=80=9Cworking at all=E2=80=9D that the m68k bus_dma back-end does in =
fact work (the virt68k copy was lifted straight from mvme68k and is one =
of my next targets for de-duplication).
Do we really want to add cache management overhead to the VirtIO layer =
for the benefit of a system emulator when VirtIO itself as a system was =
designed to be inherently coherent? One could argue that any system =
emulator which includes VirtIO has to accept the =E2=80=9Ccaches are =
fully coherent with main memory=E2=80=9D model that seems to be implied =
by VirtIO=E2=80=99s nature.
-- thorpej
From: Tetsuya Isaki <isaki@pastel-flower.jp>
To: gnats-bugs@netbsd.org
Cc: kern-bug-people@netbsd.org,
gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Tue, 31 Mar 2026 18:55:25 +0900
At Mon, 30 Mar 2026 16:45:02 +0000 (UTC),
Robert Elz via gnats wrote:
> | This actually brings up an interesting philosophical question:
> | What does DMA coherency mean in the context of virtio?
>
> Well, as you suggest, but didn't explicitly say, caching in a virtual
> device can't logically be to make anything run faster, so the obvious
> interpretation would be that it is intended to assist in finding caching
> bugs.
Mostly yes.
In my emulator, I tend to focus on accuracy (though performance is
also important).
If asked why you are implementing a cache (even though it could not
contribute to performance), my answer is "because it's there." :)
But seriously, though it's off topic,
speaking of 68030, it is VA cache. When a cache hit occurs,
it can skip not only memory access but also both ATC lookup and
table search. So, my cache implementation also contributed to
performance well on my 68030 emulation.
---
Tetsuya Isaki <isaki@pastel-flower.jp / isaki@NetBSD.org>
From: Tetsuya Isaki <isaki@pastel-flower.jp>
To: Taylor R Campbell <riastradh@NetBSD.org>
Cc: Jason Thorpe <thorpej@NetBSD.org>,
netbsd-bugs@NetBSD.org,
gnats-bugs@NetBSD.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Tue, 31 Mar 2026 19:34:03 +0900
At Mon, 30 Mar 2026 03:47:22 +0000,
Taylor R Campbell wrote:
> > I think that vq_sync_uring_header(sc, vq, BUS_DMASYNC_PREREAD) is
> > necessary to invalidate the cache line before reading fresh
> > vq->vq_used->idx (at line 610) ?
>
> I believe you have got the usage model of the preread vs postread
> wrong -- it is confusing, because `PREREAD' does not mean `before we
> read from the buffer' but rather `before the device does a DMA read
> operation to initialize the buffer'.
(snip)
Thank you for explanation!
It's confusing...
> If some kind of cache invalidation must happen between the interrupt
> delivery or DMA_STATUS_REG read at (3), and the use of the data in the
> buffer at (5), then the m68k and virt68k bus_dmamap_sync logic might
> be missing something for BUS_DMAMAP_POSTREAD -- it seems to have cases
> only for BUS_DMAMAP_PREWRITE and BUS_DMAMAP_PREREAD:
>
> https://nxr.NetBSD.org/xref/src/sys/arch/m68k/m68k/bus_dma.c?r=1.41#491
> https://nxr.NetBSD.org/xref/src/sys/arch/virt68k/virt68k/bus_dma.c?r=1.4#476
>
> Perhaps m68k needs new POSTREAD logic to be written, or perhaps there
> was a mistake and the PREREAD logic should have been for POSTREAD?
Hmm, that's it... Thanks!
sys/arch/virt68k/virt68k/bus_dma.c:
429 void
430 _bus_dmamap_sync_030(bus_dma_tag_t t, bus_dmamap_t map, bus_addr_t offset,
431 bus_size_t len, int ops)
432 {
433
434 /* Nothing yet */
435 }
I try to implement (and test) it.
Or would someone take this?
---
Tetsuya Isaki <isaki@pastel-flower.jp / isaki@NetBSD.org>
From: Jason Thorpe <thorpej@me.com>
To: Tetsuya Isaki <isaki@pastel-flower.jp>
Cc: Taylor Campbell <riastradh@NetBSD.org>,
Jason Thorpe <thorpej@NetBSD.org>,
"netbsd-bugs@netbsd.org" <netbsd-bugs@NetBSD.org>,
"gnats-bugs@netbsd.org" <gnats-bugs@NetBSD.org>
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Tue, 31 Mar 2026 06:24:36 -0700
> On Mar 31, 2026, at 3:34=E2=80=AFAM, Tetsuya Isaki =
<isaki@pastel-flower.jp> wrote:
>=20
> try to implement (and test) it.
> Or would someone take this?
I=E2=80=99ll be happy to implement it, and let you test it :-). It=E2=80=99=
s very trivial on 68030 since the cache is write-through.
-- thorpej
From: Jason Thorpe <thorpej@me.com>
To: Tetsuya Isaki <isaki@pastel-flower.jp>
Cc: gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org,
gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Tue, 31 Mar 2026 06:33:30 -0700
> On Mar 31, 2026, at 2:55=E2=80=AFAM, Tetsuya Isaki =
<isaki@pastel-flower.jp> wrote:
>=20
> Mostly yes.
> In my emulator, I tend to focus on accuracy (though performance is
> also important).
> If asked why you are implementing a cache (even though it could not
> contribute to performance), my answer is "because it's there." :)
>=20
> But seriously, though it's off topic,
> speaking of 68030, it is VA cache. When a cache hit occurs,
> it can skip not only memory access but also both ATC lookup and
> table search. So, my cache implementation also contributed to
> performance well on my 68030 emulation.
Oh, I don=E2=80=99t doubt that one bit! I was more speaking =
philosophically about VirtIO specifically.
-- thorpej
From: Tetsuya Isaki <isaki@pastel-flower.jp>
To: Jason Thorpe <thorpej@me.com>
Cc: Taylor Campbell <riastradh@NetBSD.org>,
"netbsd-bugs@netbsd.org" <netbsd-bugs@NetBSD.org>,
"gnats-bugs@netbsd.org" <gnats-bugs@NetBSD.org>
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Tue, 31 Mar 2026 22:36:19 +0900
At Tue, 31 Mar 2026 06:24:36 -0700,
Jason Thorpe wrote:
> > try to implement (and test) it.
> > Or would someone take this?
>=20
> I=A2ll be happy to implement it, and let you test it :-). It=A2s very tri=
vial on 68030 since the cache is write-through.
Okay :)
---
Tetsuya Isaki <isaki@pastel-flower.jp / isaki@NetBSD.org>
From: Jason Thorpe <thorpej@me.com>
To: gnats-bugs@netbsd.org
Cc: kern-bug-people@netbsd.org,
netbsd-bugs@netbsd.org,
isaki@pastel-flower.jp
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Tue, 31 Mar 2026 06:46:34 -0700
> On Mar 31, 2026, at 6:40=E2=80=AFAM, Tetsuya Isaki via gnats =
<gnats-admin@netbsd.org> wrote:
>=20
> Okay :)
Please try:
virt68k/virt68k/bus_dma.c:
$NetBSD: bus_dma.c,v 1.5 2026/03/31 13:45:00 thorpej Exp $
-- thorpej
From: "Jason R Thorpe" <thorpej@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/60144 CVS commit: src/sys/arch/virt68k/virt68k
Date: Tue, 31 Mar 2026 13:45:00 +0000
Module Name: src
Committed By: thorpej
Date: Tue Mar 31 13:45:00 UTC 2026
Modified Files:
src/sys/arch/virt68k/virt68k: bus_dma.c
Log Message:
Fill in bus_dmamap_sync() for 68030. Since 68030 caches are write-through,
this is pretty trivial: just invalidate the on-chip + any external D cache
in the PREREAD case. (See comment in code for why PREREAD is preferred
over POSTREAD.)
PR kern/60144
To generate a diff of this commit:
cvs rdiff -u -r1.4 -r1.5 src/sys/arch/virt68k/virt68k/bus_dma.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: Tetsuya Isaki <isaki@pastel-flower.jp>
To: Jason Thorpe <thorpej@me.com>, gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org,
netbsd-bugs@netbsd.org
Cc:
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Thu, 02 Apr 2026 21:00:50 +0900
At Tue, 31 Mar 2026 06:46:34 -0700,
Jason Thorpe wrote:
> Please try:
>
> virt68k/virt68k/bus_dma.c:
>
> $NetBSD: bus_dma.c,v 1.5 2026/03/31 13:45:00 thorpej Exp $
The spurious interrupt issue still occurs with this bus_dma.c,v 1.5
patch. When I rewrote it as below, the issue didn't seem to occur.
- if (ops & BUS_DMASYNC_PREREAD) {
+ if (ops & BUS_DMASYNC_POSTREAD) {
By the way I've tested it on -11 branch. (because virt68k-current has
another issue. I'll report once this issue is resolved)
---
Tetsuya Isaki <isaki@pastel-flower.jp / isaki@NetBSD.org>
From: Jason Thorpe <thorpej@me.com>
To: Tetsuya Isaki <isaki@pastel-flower.jp>
Cc: gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Thu, 2 Apr 2026 05:42:10 -0700
> On Apr 2, 2026, at 5:00=E2=80=AFAM, Tetsuya Isaki =
<isaki@pastel-flower.jp> wrote:
>=20
> At Tue, 31 Mar 2026 06:46:34 -0700,
> Jason Thorpe wrote:
>> Please try:
>>=20
>> virt68k/virt68k/bus_dma.c:
>>=20
>> $NetBSD: bus_dma.c,v 1.5 2026/03/31 13:45:00 thorpej Exp $
>=20
> The spurious interrupt issue still occurs with this bus_dma.c,v 1.5
> patch. When I rewrote it as below, the issue didn't seem to occur.
>=20
> - if (ops & BUS_DMASYNC_PREREAD) {
> + if (ops & BUS_DMASYNC_POSTREAD) {
If that=E2=80=99s the case, then I=E2=80=99d be concerned that there=E2=80=
=99s 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=E2=80=99s about to be stale back into the =
cache. The reason this would be problematic on 68040 is that the cache =
is configured to write-back mode and thus if any of the lines covering =
the relevant memory were to become dirty, the cache write-back would =
clobber the data just read into that memory as part of the DMA =
transaction.
Now, on real hardware there=E2=80=99s a catch: when DMA is taking place, =
the bus arbitration that the DMA controller engages in to acquire the =
bus for the DMA operation may, in fact, stall the cache fill, which =
could make this particular programming error harder to detect. Does =
your implementation of 68030 cache emulation model the bus arbitration =
behavior?
> By the way I've tested it on -11 branch. (because virt68k-current has
> another issue. I'll report once this issue is resolved)
I=E2=80=99m assuming on emulated-68030 only? In any case, thanks, =
looking forward to the report.
-- thorpej
From: Tetsuya Isaki <isaki@pastel-flower.jp>
To: Jason Thorpe <thorpej@me.com>
Cc: gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Wed, 08 Apr 2026 12:11:44 +0900
At Thu, 2 Apr 2026 05:42:10 -0700,
Jason Thorpe wrote:
> I=A2m assuming on emulated-68030 only? In any case, thanks, looking forw=
ard to the report.
I have just reported as pr 60182.
I think that one is virtio MI issue but may relate this one.
---
Tetsuya Isaki <isaki@pastel-flower.jp / isaki@NetBSD.org>
From: Tetsuya Isaki <isaki@pastel-flower.jp>
To: Jason Thorpe <thorpej@me.com>, gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org,
netbsd-bugs@netbsd.org
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 / isaki@NetBSD.org>
From: Jason Thorpe <thorpej@me.com>
To: Tetsuya Isaki <isaki@pastel-flower.jp>
Cc: Taylor Campbell <riastradh@netbsd.org>,
gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Fri, 10 Apr 2026 04:48:32 -0700
> On Apr 9, 2026, at 9:58=E2=80=AFPM, Tetsuya Isaki =
<isaki@pastel-flower.jp> wrote:
>=20
> At Thu, 2 Apr 2026 05:42:10 -0700,
> Jason Thorpe wrote:
>> If that=E2=80=99s the case, then I=E2=80=99d be concerned that =
there=E2=80=99s 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=E2=80=99s about to be =
stale back
>> into the cache.
>=20
> Thank you for your suggestion.
> I found it.
Good sleuthing!
<snip>
> vq->vq_used is shared region between the driver(CPU) and the =
device(HW),
> and is defined as:
>=20
> 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;
>=20
> 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.
Ok, I would definitely argue that this is (a) either a virtio bug, or =
(b) proof that virtio inherently assumes that the shared memory used by =
the messaging system is coherent (also note that the ring entries are =
tightly packed and no apparent effort is made in the spec to segregate =
them into their own cache lines). We=E2=80=99re basically lucky that =
it=E2=80=99s a read filling the cache, because if it were a write by the =
CPU, it would be possible to dirty an unrelated ring entry and clobber =
it during a PREWRITE operation.
Taylor, what=E2=80=99s your take on the situation?
-- thorpej
From: Taylor R Campbell <riastradh@NetBSD.org>
To: Jason Thorpe <thorpej@me.com>
Cc: Tetsuya Isaki <isaki@pastel-flower.jp>, gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Sat, 11 Apr 2026 02:35:39 +0000
> Date: Fri, 10 Apr 2026 04:48:32 -0700
> From: Jason Thorpe <thorpej@me.com>
>=20
> Taylor, what's your take on the situation?
I don't know any details about m68k caches (let alone 68030 vs 68040).
There has to be a simple usage rule for the bus_dmamap_sync API
contract, and the virtio author can't have to think about m68k cache
semantics to apply the rule.
Let's try to isolate the relevant logic in this case. There's one
virtqueue (struct ld_virtio_softc::sc_vq), and only the `used' ring is
relevant (struct virtqueue::vq_used), documented at
https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html=
#x1-540008
which gives the following procedure:
> If the VIRTIO_F_EVENT_IDX feature bit is not negotiated:
>=20
> - The driver MUST ignore the avail_event value.
> - After the driver writes a descriptor index into the available ring:
> o If flags is 1, the driver SHOULD NOT send a notification.
> o If flags is 0, the driver MUST send a notification.
The flags live at vq->vq_uring->flags, and 1 is spelled
VRING_USED_F_NO_NOTIFY in our code.
After an interrupt arrives, we check vq->vq_uring->idx to see if it
has completed, and the layout is:
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;
https://nxr.NetBSD.org/xref/src/sys/dev/pci/virtioreg.h?r=3D1.15#177
The vring_used structure is written only by the host (`device', in the
language of the virtio spec); the guest (`driver') only reads it.
We can stipulate for this analysis that the flags and idx members live
in the same cache line.
The logic for the guest to initiate a transfer is something like
(eliding all syncs _except_ for the vring_used ones):
1. prepare transfer
2. bus_dmamap_sync(vq_used, PREREAD)
3. start transfer
4. bus_dmamap_sync(vq_used, POSTREAD)
5. read vq_used->flags to see if kick needed
6. (left intentionally blank)
7. interrupt handler called
8. bus_dmamap_sync(vq_used, POSTREAD)
9. read vq_used->idx to see if completed
/*
* 1. prepare transfer
*/
bus_dmamap_load();
vq->vq_avail->ring[vq->vq_avail_idx++] =3D slot;
/*
* 2. host device will return result in vq->vq_used, so
* prepare to read
*/
bus_dmamap_sync(vq->vq_used, BUS_DMASYNC_PREREAD);
/*
* 3. start transfer -- host device may, at any time after
* this, start to write data into vq->vq_used (may require
* a kick, but may not); guest driver must not read
* vq->vq_used->flags before this point
*/
vq->vq_avail->idx =3D vq->vq_avail_idx;
/*
* 4. we're going to read vq->vq_used->flags, but since we
* started a DMA transfer into vq->vq_used, we can't read
* the result until after BUS_DMASYNC_POSTREAD
*/
bus_dmamap_sync(vq->vq_used, BUS_DMASYNC_POSTREAD);
/*
* 5. check vq->vq_used->flags to see if a kick is required
*/
flags =3D vq->vq_used->flags;
/* 6. (left intentionally blank) */
if (!(flags & VRING_USED_F_NO_NOTIFY))
kick();
/*
* 7. time passes and an interrupt handler is called
*/
/*
* 8. we're going to read vq->vq_used->idx, but since we
* started a DMA transfer into vq->vq_used, we can't read
* the result until after BUS_DMASYNC_POSTREAD
*/
bus_dmamap_sync(vq->vq_used, BUS_DMASYNC_POSTREAD);
/*
* 9. check vq->vq_used->idx to see if completed
*/
if (vq->vq_used->idx !=3D vq->vq_used_idx)
process_result();
I think, from the bus_dma(9) documentation, this logic is essentially
correct. It follows the ordering rules:
(a) Between preparation at (1) and start of transfer at (3), there is a
BUS_DMASYNC_PREREAD at (3).
(b) Between a potential change of state (start of transfer) at (3) and
use of vq->vq_used->flags in the transfer buffer vq->vq_used at
(5), there is a BUS_DMASYNC_POSTREAD at (4).
=3D> (Actually, this one is even stronger: there is a
paravirt_membar_sync between (3) and (5), which is a barrier
that doesn't even really exist in the m68k architecture -- it
is necessary for store-before-load ordering on a multiprocessor
host, even if the guest is running with a single vCPU, so
behind the scenes on an x86 host it has to be MFENCE or LOCK
ADD or similar. But none of that is relevant here.)
(c) Between a potential change of state (interrupt delivery) at (7)
and use of vq->vq_used->idx in the transfer buffer vq->vq_used at
(9), there is a BUS_DMASYNC_POSTREAD at (8).
But there's a snag: the results of the DMA operation might come
piecemeal over time, with the host device _first_ putting data for the
guest driver to read in vq->vq_used->flags, and _later_ putting data
for the guest driver to read in vq->vq_used->idx.
So that raises the key question:
If we POSTREAD and read _one part_ of the transfer buffer at
one time, and POSTREAD and read _another part_ of it at
another time, do we have to do a PREREAD in the interim at (6)
because between the two reads is essentially a new DMA
operation?
The question doesn't just apply here -- it also applies to the polling
scenario (like PR kern/60182: ld@virtio sometimes hangs up):
bus_dmamap_sync(buf, BUS_DMASYNC_PREREAD);
trigger_dma();
for (;;) {
bus_dmamap_sync(buf, BUS_DMASYNC_POSTREAD);
if (buf->ready)
break;
}
use(buf->data);
Does this need to be rewritten with another PREREAD for every
iteration?
bus_dmamap_sync(buf, BUS_DMASYNC_PREREAD);
trigger_dma();
for (;;) {
bus_dmamap_sync(buf, BUS_DMASYNC_POSTREAD);
if (buf->ready)
break;
bus_dmamap_sync(buf, BUS_DMASYNC_PREREAD);
}
use(buf->data);
If the answer is YES, then virtio(4) is broken and we need to sprinkle
some vq_sync_uring_header(sc, vq, BUS_DMASYNC_PREREAD) into both
virtio_enqueue_commit and virtio_vq_is_enqueued (partly because it is
used for polling, partly because it might share an interrupt line and
so might legitimately called several times owing to other devices'
interrupts before it returns true).
If the answer is NO, then the 68030 bus_dmamap_sync(POSTREAD) is
broken, and flushing the cache in PREREAD is not enough.
From: Taylor R Campbell <riastradh@NetBSD.org>
To: Jason Thorpe <thorpej@me.com>
Cc: Tetsuya Isaki <isaki@pastel-flower.jp>, gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Sat, 11 Apr 2026 02:54:31 +0000
> Date: Sat, 11 Apr 2026 02:35:39 +0000
> From: Taylor R Campbell <riastradh@NetBSD.org>
>
> So that raises the key question:
>
> If we POSTREAD and read _one part_ of the transfer buffer at
> one time, and POSTREAD and read _another part_ of it at
> another time, do we have to do a PREREAD in the interim at (6)
> because between the two reads is essentially a new DMA
> operation?
>
> The question doesn't just apply here -- it also applies to the polling
> scenario (like PR kern/60182: ld@virtio sometimes hangs up):
>
> bus_dmamap_sync(buf, BUS_DMASYNC_PREREAD);
> trigger_dma();
> for (;;) {
> bus_dmamap_sync(buf, BUS_DMASYNC_POSTREAD);
> if (buf->ready)
> break;
> }
> use(buf->data);
>
> Does this need to be rewritten with another PREREAD for every
> iteration?
>
> bus_dmamap_sync(buf, BUS_DMASYNC_PREREAD);
> trigger_dma();
> for (;;) {
> bus_dmamap_sync(buf, BUS_DMASYNC_POSTREAD);
> if (buf->ready)
> break;
> bus_dmamap_sync(buf, BUS_DMASYNC_PREREAD);
> }
> use(buf->data);
Oops -- I forgot add another POSTREAD between testing buf->ready and
using buf->data, in case the device's stores are reordered as observed
by loads in the driver. So the options are:
bus_dmamap_sync(buf, BUS_DMASYNC_PREREAD);
trigger_dma();
for (;;) {
bus_dmamap_sync(buf, BUS_DMASYNC_POSTREAD);
if (buf->ready)
break;
}
bus_dmamap_sync(buf, BUS_DMASYNC_POSTREAD);
use(buf->data);
versus
bus_dmamap_sync(buf, BUS_DMASYNC_PREREAD);
trigger_dma();
for (;;) {
bus_dmamap_sync(buf, BUS_DMASYNC_POSTREAD);
ready = buf->ready;
bus_dmamap_sync(buf, BUS_DMASYNC_PREREAD);
if (ready)
break;
}
bus_dmamap_sync(buf, BUS_DMASYNC_POSTREAD);
use(buf->data);
From: Nick Hudson <nick.hudson@gmx.co.uk>
To: Taylor R Campbell <riastradh@NetBSD.org>,
Jason Thorpe <thorpej@me.com>
Cc: Tetsuya Isaki <isaki@pastel-flower.jp>,
gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Sat, 11 Apr 2026 08:03:43 +0100
On 11/04/2026 03:35, Taylor R Campbell wrote:
>> Date: Fri, 10 Apr 2026 04:48:32 -0700
>> From: Jason Thorpe <thorpej@me.com>
>>=20
>> Taylor, what's your take on the situation?
> I don't know any details about m68k caches (let alone 68030 vs 68040).
=46rom my skimming of the conversation m68k caches (and the m68k bus_dma
implementation) behave in the same way as other architectures, e.g.
pre-armv7
> There has to be a simple usage rule for the bus_dmamap_sync API
> contract, and the virtio author can't have to think about m68k cache
> semantics to apply the rule.
> Let's try to isolate the relevant logic in this case. There's one
> virtqueue (struct ld_virtio_softc::sc_vq), and only the `used' ring is
> relevant (struct virtqueue::vq_used), documented at
> =
https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.htm=
l#x1-540008
> which gives the following procedure:
>> If the VIRTIO_F_EVENT_IDX feature bit is not negotiated:
>>=20
>> - The driver MUST ignore the avail_event value.
>> - After the driver writes a descriptor index into the available =
ring:
>> o If flags is 1, the driver SHOULD NOT send a notification.
>> o If flags is 0, the driver MUST send a notification.
> The flags live at vq->vq_uring->flags, and 1 is spelled
> VRING_USED_F_NO_NOTIFY in our code.
> After an interrupt arrives, we check vq->vq_uring->idx to see if it
> has completed, and the layout is:
> 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;
> https://nxr.NetBSD.org/xref/src/sys/dev/pci/virtioreg.h?r=3D1.15#177
> The vring_used structure is written only by the host (`device', in the
> language of the virtio spec); the guest (`driver') only reads it.
> We can stipulate for this analysis that the flags and idx members live
> in the same cache line.
> The logic for the guest to initiate a transfer is something like
> (eliding all syncs _except_ for the vring_used ones):
> 1. prepare transfer
> 2. bus_dmamap_sync(vq_used, PREREAD)
> 3. start transfer
> 4. bus_dmamap_sync(vq_used, POSTREAD)
> 5. read vq_used->flags to see if kick needed
> 6. (left intentionally blank)
> 7. interrupt handler called
> 8. bus_dmamap_sync(vq_used, POSTREAD)
> 9. read vq_used->idx to see if completed
> /*
> * 1. prepare transfer
> */
> bus_dmamap_load();
> vq->vq_avail->ring[vq->vq_avail_idx++] =3D slot;
> /*
> * 2. host device will return result in vq->vq_used, so
> * prepare to read
> */
> bus_dmamap_sync(vq->vq_used, BUS_DMASYNC_PREREAD);
> /*
> * 3. start transfer -- host device may, at any time after
> * this, start to write data into vq->vq_used (may require
> * a kick, but may not); guest driver must not read
> * vq->vq_used->flags before this point
> */
> vq->vq_avail->idx =3D vq->vq_avail_idx;
> /*
> * 4. we're going to read vq->vq_used->flags, but since we
> * started a DMA transfer into vq->vq_used, we can't read
> * the result until after BUS_DMASYNC_POSTREAD
> */
> bus_dmamap_sync(vq->vq_used, BUS_DMASYNC_POSTREAD);
> /*
> * 5. check vq->vq_used->flags to see if a kick is required
> */
> flags =3D vq->vq_used->flags;
> /* 6. (left intentionally blank) */
> if (!(flags & VRING_USED_F_NO_NOTIFY))
> kick();
> /*
> * 7. time passes and an interrupt handler is called
> */
> /*
> * 8. we're going to read vq->vq_used->idx, but since we
> * started a DMA transfer into vq->vq_used, we can't read
> * the result until after BUS_DMASYNC_POSTREAD
> */
> bus_dmamap_sync(vq->vq_used, BUS_DMASYNC_POSTREAD);
> /*
> * 9. check vq->vq_used->idx to see if completed
> */
> if (vq->vq_used->idx !=3D vq->vq_used_idx)
> process_result();
> I think, from the bus_dma(9) documentation, this logic is essentially
> correct. It follows the ordering rules:
> (a) Between preparation at (1) and start of transfer at (3), there is =
a
> BUS_DMASYNC_PREREAD at (3).
> (b) Between a potential change of state (start of transfer) at (3) and
> use of vq->vq_used->flags in the transfer buffer vq->vq_used at
> (5), there is a BUS_DMASYNC_POSTREAD at (4).
> =3D> (Actually, this one is even stronger: there is a
> paravirt_membar_sync between (3) and (5), which is a barrier
> that doesn't even really exist in the m68k architecture -- it
> is necessary for store-before-load ordering on a multiprocessor
> host, even if the guest is running with a single vCPU, so
> behind the scenes on an x86 host it has to be MFENCE or LOCK
> ADD or similar. But none of that is relevant here.)
> (c) Between a potential change of state (interrupt delivery) at (7)
> and use of vq->vq_used->idx in the transfer buffer vq->vq_used at
> (9), there is a BUS_DMASYNC_POSTREAD at (8).
> But there's a snag: the results of the DMA operation might come
> piecemeal over time, with the host device _first_ putting data for the
> guest driver to read in vq->vq_used->flags, and _later_ putting data
> for the guest driver to read in vq->vq_used->idx.
> So that raises the key question:
> If we POSTREAD and read _one part_ of the transfer buffer at
> one time, and POSTREAD and read _another part_ of it at
> another time, do we have to do a PREREAD in the interim at (6)
> because between the two reads is essentially a new DMA
> operation?
I think the answer is yes.
> The question doesn't just apply here -- it also applies to the polling
> scenario (like PR kern/60182: ld@virtio sometimes hangs up):
> bus_dmamap_sync(buf, BUS_DMASYNC_PREREAD);
> trigger_dma();
> for (;;) {
> bus_dmamap_sync(buf, BUS_DMASYNC_POSTREAD);
> if (buf->ready)
> break;
> }
> use(buf->data);
> Does this need to be rewritten with another PREREAD for every
> iteration?
> bus_dmamap_sync(buf, BUS_DMASYNC_PREREAD);
> trigger_dma();
> for (;;) {
> bus_dmamap_sync(buf, BUS_DMASYNC_POSTREAD);
> if (buf->ready)
> break;
> bus_dmamap_sync(buf, BUS_DMASYNC_PREREAD);
> }
> use(buf->data);
So we need this...
It=E2=80=99s also required in the coherent case so we get the =
appropriate memory barriers, right?
> If the answer is YES, then virtio(4) is broken and we need to sprinkle
> some vq_sync_uring_header(sc, vq, BUS_DMASYNC_PREREAD) into both
> virtio_enqueue_commit and virtio_vq_is_enqueued (partly because it is
> used for polling, partly because it might share an interrupt line and
> so might legitimately called several times owing to other devices'
> interrupts before it returns true).
> If the answer is NO, then the 68030 bus_dmamap_sync(POSTREAD) is
> broken, and flushing the cache in PREREAD is not enough.
From: Jason Thorpe <thorpej@me.com>
To: Taylor Campbell <riastradh@NetBSD.org>
Cc: Tetsuya Isaki <isaki@pastel-flower.jp>,
gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Sat, 11 Apr 2026 11:48:32 +0100
> On Apr 11, 2026, at 3:35=E2=80=AFAM, Taylor R Campbell =
<riastradh@NetBSD.org> wrote:
>=20
>> Date: Fri, 10 Apr 2026 04:48:32 -0700
>> From: Jason Thorpe <thorpej@me.com>
>>=20
>> Taylor, what's your take on the situation?
>=20
> I don't know any details about m68k caches (let alone 68030 vs 68040).
Let me give a quick primer on them just so we=E2=80=99re all on the same =
page regarding now they work.
The 68010 does not have any caches.
The 68020 has a small instruction cache, and no data cache. The =
instruction cache is a 256 byte direct-mapped cache arranged as 64 =
longword entries. The cache is VIVT by virtue of the MMU being external =
to the CPU. Some 68020 systems have external combined caches, which may =
be VIVT or PIPT, depending on how the system board is wired up. If an =
external cache exists, it is always write-through.
The 68030, being an incremental update to the 68020, adds a small =
on-chip data cache. That said, the 68030=E2=80=99s instruction cache is =
not the same as the 68020=E2=80=99s. Rather both the instruction and =
data cache of the 68030 are both arranged as 16 lines of 16 bytes each =
(256 bytes for each cache). Each longword in a cache line has a VALID =
bit. Cache lines may be filled one longword at a time, or a burst-mode =
fill operation may be used to fill an entire cache line (one-at-a-time =
mode always works, and burst mode requires the machine=E2=80=99s memory =
subsystem to support the burst-mode bus protocol). Despite the fact =
that the 68030 has a built-in MMU, the 68030 cache is VIVT. This =
arrangement allows for some internal optimizations. The ATC (address =
translation cache) on the 68030 is fairly small, so it is advantageous =
to perform cache lookups using virtual addresses; a cache hit isn=E2=80=99=
t very meaningful if the MMU has to do a table-walk in order to =
determine if you got a hit or not. Despite the 68030 data cache being =
VIVT, if the Write-Allocate mode is used, it does not suffer from cache =
alias problems due to the small size and internal cache arrangement. =
The 68030 data cache is always write-though. There are external caches =
on some 68030 systems, as well. These are also always write-though, and =
due to the MMU being integrated into the CPU, these external caches are =
always PIPT.
The 68040 is a complete departure from the 68030=E2=80=99s MMU and cache =
implementation. The 68040 has dual MMUs (one for instruction, one for =
data, although they share a single root table pointer). There is also a =
4KB instruction cache and a 4KB data cache on-chip. Both caches are =
4-way set-associative, arranged as 64 sets of 4 16-byte lines. The =
major differences that matter to software: the caches are PIPT, and the =
MMU controls on a page-by-page basis: write-through or write-back (in =
addition to the non-cacheable, which is also something you can do with =
the other CPUs; I will skip the details of how that works with an =
external MMU). I do not know of any 68040 systems that have external =
caches. The 68040 has a snooping protocol that an external cache would =
need to participate in.
The 68060 is largely the same as the 68040, except the caches are larger =
(8K + 8K), and there is an addition of a Branch Target Cache for the =
branch predictor.
Here endeth the lesson.
-- thorpej
From: Taylor R Campbell <riastradh@NetBSD.org>
To: Jason Thorpe <thorpej@me.com>
Cc: Tetsuya Isaki <isaki@pastel-flower.jp>,
Nick Hudson <nick.hudson@gmx.co.uk>,
gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Sat, 11 Apr 2026 15:50:47 +0000
> Date: Sat, 11 Apr 2026 11:48:32 +0100
> From: Jason Thorpe <thorpej@me.com>
>=20
> Let me give a quick primer on them just so we're all on the same
> page regarding now they work.
That's all very interesting, but this is really a question about the
API rules for bus_dma.
In
if (buf->ready)
use(buf->data);
what bus_dmamap_sync barriers are needed to separate the loads of
buf->ready and buf->data? (Similarly if we load buf->ready in a
polling loop: what barriers are necessary in consecutive loads of
buf->ready?)
I have long thought that only POSTREAD is necessary here, since we
already started a DMA transfer into buf a long time ago, with the
matching PREREAD already issued before it started:
buf =3D alloc();
bus_dmamap_sync(buf, PREREAD);
bus_space_write_4(bst, bsh, TRIGGER_DMA, bufaddr); // start DMA
...
/* interrupt handler -- buf->ready _may_ have been updated */
bus_dmamap_sync(&buf->ready, POSTREAD);
if (buf->ready) {
/* buf->data _has definitely_ been updated */
bus_dmamap_sync(&buf->data, POSTREAD);
use(buf->data);
}
And I'm pretty sure we have a lot of drivers written like this (most
of which I did not write!). For example, here's the wm(4) rx
interrupt handler (Intel ethernet driver):
10217 static bool
10218 wm_rxeof(struct wm_rxqueue *rxq, u_int limit)
10219 {
...
10232 for (i =3D rxq->rxq_ptr;; i =3D WM_NEXTRX(i)) {
10233 rxs =3D &rxq->rxq_soft[i];
...
10238 wm_cdrxsync(rxq, i,
10239 BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
...
10250 if (!wm_rxdesc_dd(rxq, i, status))
10251 break;
...
10277 bus_dmamap_sync(sc->sc_dmat, rxs->rxs_dmamap, 0,
10278 rxs->rxs_dmamap->dm_mapsize, BUS_DMASYNC_POSTREAD);
10279=20
10280 m =3D rxs->rxs_mbuf;
...
10385 if_percpuq_enqueue(sc->sc_ipq, m);
https://nxr.netbsd.org/xref/src/sys/dev/pci/if_wm.c?r=3D1.801#10212
The call wm_rxdesc_dd corresponds to testing buf->ready, and the
subsequent use of the content of rxs->rxs_mbuf corresponds to
use(buf->data).
Here's the mfi(4) interrupt routine (SCSI driver):
1362 int
1363 mfi_intr(void *arg)
1364 {
...
1378 bus_dmamap_sync(sc->sc_dmat, MFIMEM_MAP(sc->sc_pcq), 0,
1379 sizeof(uint32_t) * sc->sc_max_cmds + sizeof(struct mfi_prod_co=
ns),
1380 BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
1381=20
1382 producer =3D pcq->mpc_producer;
1383 consumer =3D pcq->mpc_consumer;
1384=20
1385 while (consumer !=3D producer) {
...
1389 ctx =3D pcq->mpc_reply_q[consumer];
...
1397 ccb =3D &sc->sc_ccb[ctx];
...
1400 bus_dmamap_sync(sc->sc_dmat, MFIMEM_MAP(sc->sc_frames),
1401 ccb->ccb_pframe - MFIMEM_DVA(sc->sc_frames),
1402 sc->sc_frames_size,
1403 BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
1404 ccb->ccb_done(ccb);
https://nxr.netbsd.org/xref/src/sys/dev/ic/mfi.c?r=3D1.80#1362
Testing pcq->mpc_producer/consumer and loading pcq->mpc_reply_q
roughly corresponds to testing buf->ready, and the subsequent use of
ccb corresponding to pcq->mpc_reply_q[consumer] corresponds to using
buf->data.
The hifn(4) driver (crypto accelerator) does issue another PREREAD,
but not between testing buf->ready and using buf->data -- only in the
polling loop of repeatedly testing buf->ready, essentially:
1931 static int
1932 hifn_intr(void *arg)
1933 {
...
2000 HIFN_RESR_SYNC(sc, i,
2001 BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
2002 if (dma->resr[i].l & htole32(HIFN_D_VALID)) {
2003 HIFN_RESR_SYNC(sc, i,
2004 BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE);
2005 break;
2006 }
...
2011 HIFN_RES_SYNC(sc, i, BUS_DMASYNC_POSTREAD);
2012 cmd =3D dma->hifn_commands[i];
...
2017 hifn_callback(sc, cmd, dma->result_bufs[i]);
https://nxr.netbsd.org/xref/src/sys/dev/pci/hifn7751.c#1931
Do we need to go through all these drivers and insert another PREREAD
between testing buf->ready and using buf->data? (And between repeated
tests of buf->ready?)
Note that all of these drivers _already_ issue POSTREAD barriers
between all the relevant loads. (There are some drivers that don't do
even that, and I'm willing to accept that those drivers are simply
buggy.)
From: Taylor R Campbell <riastradh@NetBSD.org>
To: Jason Thorpe <thorpej@me.com>
Cc: Tetsuya Isaki <isaki@pastel-flower.jp>,
Nick Hudson <nick.hudson@gmx.co.uk>,
gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Sat, 11 Apr 2026 16:53:28 +0000
I talked it over with Nick on ICB and he convinced me that this
question about
buf = alloc()
PREREAD
trigger DMA
interrupt
POSTREAD
if (buf->ready) {
???
use(buf->data)
}
is not quite analogous, though a polling loop around buf->ready may
be analogous. We had a similar issue with ehci on armv5:
https://mail-index.NetBSD.org/source-changes-d/2025/09/29/msg014539.html
Here's the virtio(4) logic in question:
/* virtio_enqueue_commit */
1255 vq_sync_uring_all(sc, vq, BUS_DMASYNC_PREREAD);
...
1259 vq->vq_avail->idx = virtio_rw16(sc, vq->vq_avail_idx);
...
1275 vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
1276 flags = virtio_rw16(sc, vq->vq_used->flags);
1277 if (!(flags & VRING_USED_F_NO_NOTIFY))
1278 sc->sc_ops->kick(sc, vq->vq_index);
...
https://nxr.netbsd.org/xref/src/sys/dev/pci/virtio.c?r=1.84#1247
/* virtio_vq_is_enqueued */
609 vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
610 if (vq->vq_used_idx == virtio_rw16(sc, vq->vq_used->idx))
611 return 0;
612 vq_sync_uring_payload(sc, vq, BUS_DMASYNC_POSTREAD);
https://nxr.netbsd.org/xref/src/sys/dev/pci/virtio.c?r=1.84#609
The trouble is that the CPU issues an extra load at line 1276 _before
the DMA operation may have actually been triggered_ at line 1278, and,
more subtly, the order of operations by the virtio host device isn't
quite the same in the two places where there's a (correct) POSTREAD
barrier.
For normal buf->ready / buf->data access, the protocol is more or
less:
host device model
-----------------
H1. store buf->data
H2. (barrier)
H3. set buf->ready := 1
guest driver
------------
G1. test buf->ready
G2. POSTREAD
G3. load buf->data
And that guest driver logic is what happens in virtio_vq_is_enqueued,
with a POSTREAD between testing buf->ready (vq->vq_used->idx) and
using the data (uring payload). So that part is fine.
What's different with flags/idx is that the host device _can't_ start
the DMA operation before setting flags. So the full host side looks
like:
virtio host device
------------------
H1'. set vq->vq_used->flags appropriately
H2'. store-before-load barrier
H3'. load vq->vq_avail->idx
H4'. DMA operation to fill uring payload
H5'. (barrier)
H6'. store vq->vq_used->idx
Note that the ordering here is reversed: while the simplified host
device model separates H1 (store buf->data) by some barrier from H3
(set buf->ready) _in that order_, the virtio host device sets
vq->vq_used->flags before it can possibly even start the DMA
operation. (Then H4'/H5'/H6' correspond to the simplified host device
model's H1/H2/H3.)
So I'm convinced that our guest virtio(4) driver should issue another
PREREAD barrier between loading vq->vq_used->flags and either
(a) kicking the DMA, or
(b) loading vq->vq_used->idx to check for DMA completion.
--- src/sys/dev/pci/virtio.c
+++ src/sys/dev/pci/virtio.c
@@ -1274,6 +1274,7 @@
} else {
vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
flags = virtio_rw16(sc, vq->vq_used->flags);
+ vq_sync_uring_header(sc, vq, BUS_DMASYNC_PREREAD);
if (!(flags & VRING_USED_F_NO_NOTIFY))
sc->sc_ops->kick(sc, vq->vq_index);
}
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?
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 {
From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
To: riastradh@NetBSD.org
Cc: thorpej@me.com, isaki@pastel-flower.jp, nick.hudson@gmx.co.uk,
gnats-bugs@netbsd.org, kern-bug-people@netbsd.org,
netbsd-bugs@netbsd.org, tsutsui@ceres.dti.ne.jp
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
From: Taylor R Campbell <riastradh@NetBSD.org>
To: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
Cc: thorpej@me.com, isaki@pastel-flower.jp, nick.hudson@gmx.co.uk,
gnats-bugs@netbsd.org, kern-bug-people@netbsd.org,
netbsd-bugs@netbsd.org, tsutsui@ceres.dti.ne.jp,
adrian@NetBSD.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Mon, 18 May 2026 23:29:58 +0000
This is a multi-part message in MIME format.
--=_iH22Mq8cEGadrlwrn55fhppMX7+AHtfW
OK, here is a candidate patch to address this issue. Most of what
needs review is the commit message, but it would be good to test
whether it empirically addresses the problem too. (Note that alone it
is probably _not_ enough to fix PR 60182, but it is probably needed
_in addition_ to the patch I already posted there.)
Isaki-san: Can you verify that this patch works?
--=_iH22Mq8cEGadrlwrn55fhppMX7+AHtfW
Content-Type: text/plain; charset="ISO-8859-1"; name="pr60144-virtiodmasync"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment; filename="pr60144-virtiodmasync.patch"
# HG changeset patch
# User Taylor R Campbell <riastradh@NetBSD.org>
# Date 1779137898 0
# Mon May 18 20:58:18 2026 +0000
# Branch trunk
# Node ID 0f2733c1e2e8f9e109ce9e1258023da90fe016f5
# Parent 7c298a30dee7608bf965a5082affaaafadcefa9b
# EXP-Topic riastradh-pr60144-virtiodmasync
virtio(4): Add missing BUS_DMASYNC_PREREAD operations.
And one missing BUS_DMASYNC_POSTREAD operation.
With this change, loads from vq->vq_used->flags (which occur
immediately after a transfer is submitted to test whether we need to
kick the host device) is separated by a PREREAD/POSTREAD cycle from
loads from vq->vq_used->idx (which occur when we think a transfer may
have completed, e.g. upon receiving an interrupt, to test whether it
has, in fact, completed).
Additionally, with this change, consecutive loads from
*vq->vq_avail_event are separated by a PREREAD/POSTREAD cycle.
Should fix virtio(4) issues on m68k and other related architectures
like mips and armv<7:
PR kern/60144: virtio(4) cache coherence issue
(And may make what I think is a mere potential performance
improvement on those and other architectures like x86 or aarch64,
because of the change to virtio_postpone_intr.)
Why?
This is tricky, and (despite a remark I made to the contrary in the
PR) I'm still not entirely convinced we have the rules articulated
sufficiently to prove the driver correct with or without the
changes, or to prove a bus_dma implementation correct.
But, based on the current mengarie of bus_dma implementations we
have, the crux of the issue is:
When we load twice from the same cache line affected by a DMA read
operation, if the DMA read operation may complete in the interim
between the two loads, it is necessary to have a PREREAD/POSTREAD
cycle between the loads -- not just POSTREAD.
For example, in a polling loop:
buf =3D alloc_buf()
bus_dmamap_load(map, buf)
bus_dmamap_sync(map, 0, offsetof(buf->ready), PREREAD)
bus_space_write(trigger_dma_at, map->dm_segs[0].ds_addr);
bus_dmamap_sync(map, 0, offsetof(buf->ready), POSTREAD)
while (!buf->ready) {
bus_dmamap_sync(map, 0, offsetof(buf->ready), PREREAD)
bus_dmamap_sync(map, 0, offsetof(buf->ready), POSTREAD)
}
...
The pair of PREREAD/POSTREAD syncs in the body of the loop must
_both_ appear in order for this to work; POSTREAD alone is not
enough.
On some architectures, such as x86, PREREAD/POSTREAD is stronger than
needed.
But on others -- such as m68k, mips, and armv<7 -- POSTREAD alone
isn't enough because it's a noop: these architectures assume that
after PREREAD, the driver doesn't load the memory in question until
the DMA read operation has completed, so flushing cached data in
PREREAD and doing nothing in POSTREAD is enough to ensure that the
driver's load isn't stale.
This scenario isn't really spelled out in the bus_dma(9) man page.
And bus_dma(9) doesn't even have a concept of cache lines. But, even
outside virtio(4), the issue has already bitten us with real
hardware:
1. USB ehci(4) TDs (transfer descriptors), back when they were packed
into shared cache lines. This required BUS_DMA_COHERENT, which is
-- by the letter of bus_dma(9) -- not supposed to affect
correctness.
Packing multiple TDs into a single shared cache line required
BUS_DMA_COHERENT because the host CPU and the device might
otherwise attempt to update the same cache line simultaneously,
leading one party to lose its updates; there was no sequence of
bus_dmamap_syncs that could make it work.
This was addressed by splitting the TDs across cache lines:
https://mail-index.NetBSD.org/source-changes/2024/09/23/msg153483.html
2. Polling loops, such as the following in fxp(4), where the driver
repeatedly loads from the same DMA buffer in memory, separated by
bus_dmamap_sync calls, until the DMA read operation is done and
the CPU loads a different value out of the buffer.
We don't need to define a notion of `the same cache line' in
bus_dma(9) for this case -- it's exactly the same location in
memory every time. The marked PREREAD was added to fix the driver
on sgimips O2.
1858 FXP_CDCONFIGSYNC(sc, BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE);
1859
1860 /*
1861 * Start the config command/DMA.
1862 */
1863 fxp_scb_wait(sc);
1864 CSR_WRITE_4(sc, FXP_CSR_SCB_GENERAL, sc->sc_cddma + FXP_CDCONFIGOF=
F);
1865 fxp_scb_cmd(sc, FXP_SCB_COMMAND_CU_START);
1866 /* ...and wait for it to complete. */
1867 for (i =3D 1000; i > 0; i--) {
1868 FXP_CDCONFIGSYNC(sc,
1869 BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
1870 status =3D le16toh(cbp->cb_status);
=3D> 1871 FXP_CDCONFIGSYNC(sc, BUS_DMASYNC_PREREAD);
1872 if ((status & FXP_CB_STATUS_C) !=3D 0)
1873 break;
1874 DELAY(1);
1875 }
https://nxr.NetBSD.org/xref/src/sys/dev/ic/i82557.c?r=3D1.162#1858
In the case of virtio(4), we have both of these issues:
1. Once the guest driver has published a transfer by a store to
vq->vq_avail->idx, it must load vq->vq_used->flags to decide
whether to kick the host:
1259 vq->vq_avail->idx =3D virtio_rw16(sc, vq->vq_avail_idx);
...
1275 vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
1276 flags =3D virtio_rw16(sc, vq->vq_used->flags);
1277 if (!(flags & VRING_USED_F_NO_NOTIFY))
1278 sc->sc_ops->kick(sc, vq->vq_index);
Later on, to determine whether the transfer has completed, it must
load vq->vq_used->idx:
https://nxr.NetBSD.org/xref/src/sys/dev/pci/virtio.c?r=3D1.84#1259
600 bool
601 virtio_vq_is_enqueued(struct virtio_softc *sc, struct virtqueue *vq)
602 {
...
609 vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
610 if (vq->vq_used_idx =3D=3D virtio_rw16(sc, vq->vq_used->idx))
611 return 0;
...
613 return 1;
614 }
https://nxr.NetBSD.org/xref/src/sys/dev/pci/virtio.c?r=3D1.84#600
But vq->vq_used->flags and vq->vq_used->idx are 16-bit integers
in a likely-aligned buffer, so they are almost certain to share a
cache line:
177 struct vring_used {
178 uint16_t flags;
179 uint16_t idx;
https://nxr.NetBSD.org/xref/src/sys/dev/pci/virtioreg.h?r=3D1.15#177
Fortunately, the guest driver and the host device are not competing
to update the cache line at the same time, so to fix virtio(4) on
the affected architectures, it is just a matter of issuing a
PREREAD/POSTREAD cycle between the load of vq->vq_used->flags and
vq->vq_used->idx, not just POSTREAD.
2. Virtio interrupt handlers use virtio_vq_is_enqueued to test
whether a transfer on a virtqueue has completed. If the interrupt
line is shared with other devices, a driver might test
virtio_vq_is_enqueued several times in a row from unrelated
interrupts before it returns true, and this boils down to:
*interrupt*
POSTREAD
load vq->vq_used->idx
*interrupt*
POSTREAD
load vq->vq_used->idx
*interrupt*
POSTREAD
load vq->vq_used->idx
...
On some architectures, since POSTREAD does nothing, the first
value of vq->vq_used->idx may remain cached through the subsequent
loads.
Note: this is related to, and probably necessary for, PR kern/60182:
ld@virtio sometimes hangs up. But there is also another issue there
with missing POSTREAD in a loop inside ld(4) -- not just PREREAD. To
be fixed in a separate commit.
diff -r 7c298a30dee7 -r 0f2733c1e2e8 sys/dev/pci/virtio.c
--- a/sys/dev/pci/virtio.c Sun May 17 01:31:04 2026 +0000
+++ b/sys/dev/pci/virtio.c Mon May 18 20:58:18 2026 +0000
@@ -607,8 +607,10 @@ virtio_vq_is_enqueued(struct virtio_soft
}
=20
vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
- if (vq->vq_used_idx =3D=3D virtio_rw16(sc, vq->vq_used->idx))
+ if (vq->vq_used_idx =3D=3D virtio_rw16(sc, vq->vq_used->idx)) {
+ vq_sync_uring_header(sc, vq, BUS_DMASYNC_PREREAD);
return 0;
+ }
vq_sync_uring_payload(sc, vq, BUS_DMASYNC_POSTREAD);
return 1;
}
@@ -629,8 +631,10 @@ virtio_postpone_intr(struct virtio_softc
vq_sync_aring_used(vq->vq_owner, vq, BUS_DMASYNC_PREWRITE);
vq->vq_queued++;
=20
+ vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
nused =3D (uint16_t)
(virtio_rw16(sc, vq->vq_used->idx) - vq->vq_used_idx);
+ vq_sync_uring_header(sc, vq, BUS_DMASYNC_PREREAD);
KASSERT(nused <=3D vq->vq_num);
=20
return nslots < nused;
@@ -717,8 +721,10 @@ virtio_start_vq_intr(struct virtio_softc
paravirt_membar_sync();
=20
vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
- if (vq->vq_used_idx =3D=3D virtio_rw16(sc, vq->vq_used->idx))
+ if (vq->vq_used_idx =3D=3D virtio_rw16(sc, vq->vq_used->idx)) {
+ vq_sync_uring_header(sc, vq, BUS_DMASYNC_PREREAD);
return 0;
+ }
vq_sync_uring_payload(sc, vq, BUS_DMASYNC_POSTREAD);
return 1;
}
@@ -1269,11 +1275,13 @@ notify:
if (sc->sc_active_features & VIRTIO_F_RING_EVENT_IDX) {
vq_sync_uring_avail(sc, vq, BUS_DMASYNC_POSTREAD);
t =3D 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 {
vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
flags =3D virtio_rw16(sc, vq->vq_used->flags);
+ vq_sync_uring_header(sc, vq, BUS_DMASYNC_PREREAD);
if (!(flags & VRING_USED_F_NO_NOTIFY))
sc->sc_ops->kick(sc, vq->vq_index);
}
--=_iH22Mq8cEGadrlwrn55fhppMX7+AHtfW--
State-Changed-From-To: open->feedback
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Mon, 18 May 2026 23:32:39 +0000
State-Changed-Why:
candidate fix posted
From: Tetsuya Isaki <isaki@pastel-flower.jp>
To: Taylor R Campbell <riastradh@NetBSD.org>
Cc: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>,
thorpej@me.com,
nick.hudson@gmx.co.uk,
gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org,
netbsd-bugs@netbsd.org,
adrian@NetBSD.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Fri, 22 May 2026 13:48:30 +0900
At Mon, 18 May 2026 23:29:58 +0000,
Taylor R Campbell wrote:
> OK, here is a candidate patch to address this issue. Most of what
> needs review is the commit message, but it would be good to test
> whether it empirically addresses the problem too. (Note that alone it
> is probably _not_ enough to fix PR 60182, but it is probably needed
> _in addition_ to the patch I already posted there.)
>
> Isaki-san: Can you verify that this patch works?
It worked with these three patches applied. Thank you!
- sys/arch/virt68k/virt68k/bus_dma.c,v 1.5 (I tested it on -11)
- sys/dev/pci/ld_virtio.c (from kern/60182)
- sys/dev/pci/virtio.c (this patch)
By the way, one spurious interrupt has still occurred.
...
virtio1: allocated 24576 byte for virtqueue 0 for I/O request, size 32
virtio1: using 16384 byte (1024 entries) indirect descriptors
virtio1: interrupting at gfpic4 irq 2 (IPL 5)
ld1: 8192 MB, 4161 cyl, 64 head, 63 sec, 512 bytes/sect x 16777216 sectors (0 bytes/physsect)
Spurious interrupt on CPU ipl 5
No statclock; using hardclock.
Initialzing hardclock: freq=100 Hz, interval=10000 usec
timecounter: Timecounter "clockinterrupt" frequency 100 Hz quality 0
swwdog0: software watchdog initialized
root on ld0a dumps on ld0b
root file system type: ffs
---
Tetsuya Isaki <isaki@pastel-flower.jp / isaki@NetBSD.org>
From: Jason Thorpe <thorpej@me.com>
To: gnats-bugs@netbsd.org
Cc: kern-bug-people@netbsd.org,
gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org,
isaki@pastel-flower.jp
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Fri, 22 May 2026 08:31:20 -0400
> On May 18, 2026, at 7:30=E2=80=AFPM, Taylor R Campbell via gnats =
<gnats-admin@NetBSD.org> wrote:
>=20
> But on others -- such as m68k, mips, and armv<7 -- POSTREAD alone
> isn't enough because it's a noop: these architectures assume that
> after PREREAD, the driver doesn't load the memory in question until
> the DMA read operation has completed, so flushing cached data in
> PREREAD and doing nothing in POSTREAD is enough to ensure that the
> driver's load isn't stale.
This isn=E2=80=99t why. The real reason is that the cache =
implementation on some of these platforms does not have an =
invalidate-without-writeback operation.
-- thorpej
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/60144 CVS commit: src/sys/dev/pci
Date: Fri, 22 May 2026 14:36:37 +0000
Module Name: src
Committed By: riastradh
Date: Fri May 22 14:36:37 UTC 2026
Modified Files:
src/sys/dev/pci: virtio.c
Log Message:
virtio(4): Add missing BUS_DMASYNC_PREREAD operations.
And one missing BUS_DMASYNC_POSTREAD operation.
With this change, loads from vq->vq_used->flags (which occur
immediately after a transfer is submitted to test whether we need to
kick the host device) is separated by a PREREAD/POSTREAD cycle from
loads from vq->vq_used->idx (which occur when we think a transfer may
have completed, e.g. upon receiving an interrupt, to test whether it
has, in fact, completed).
Additionally, with this change, consecutive loads from
*vq->vq_avail_event are separated by a PREREAD/POSTREAD cycle.
Should fix virtio(4) issues on m68k and other related architectures
like mips and armv<7:
PR kern/60144: virtio(4) cache coherence issue
(And may make what I think is a mere potential performance
improvement on those and other architectures like x86 or aarch64,
because of the change to virtio_postpone_intr.)
Why?
This is tricky, and (despite a remark I made to the contrary in the
PR) I'm still not entirely convinced we have the rules articulated
sufficiently to prove the driver correct with or without the
changes, or to prove a bus_dma implementation correct.
But, based on the current mengarie of bus_dma implementations we
have, the crux of the issue is:
When we load twice from the same cache line affected by a DMA read
operation, if the DMA read operation may complete in the interim
between the two loads, it is necessary to have a PREREAD/POSTREAD
cycle between the loads -- not just POSTREAD.
For example, in a polling loop:
buf = alloc_buf()
bus_dmamap_load(map, buf)
bus_dmamap_sync(map, 0, offsetof(buf->ready), PREREAD)
bus_space_write(trigger_dma_at, map->dm_segs[0].ds_addr);
bus_dmamap_sync(map, 0, offsetof(buf->ready), POSTREAD)
while (!buf->ready) {
bus_dmamap_sync(map, 0, offsetof(buf->ready), PREREAD)
bus_dmamap_sync(map, 0, offsetof(buf->ready), POSTREAD)
}
...
The pair of PREREAD/POSTREAD syncs in the body of the loop must
_both_ appear in order for this to work; POSTREAD alone is not
enough.
On some architectures, such as x86, PREREAD/POSTREAD is stronger than
needed.
But on others -- such as m68k, mips, and armv<7 -- POSTREAD alone
isn't enough because it's a noop: these architectures assume that
after PREREAD, the driver doesn't load the memory in question until
the DMA read operation has completed, so flushing cached data in
PREREAD and doing nothing in POSTREAD is enough to ensure that the
driver's load isn't stale.
This scenario isn't really spelled out in the bus_dma(9) man page.
And bus_dma(9) doesn't even have a concept of cache lines. But, even
outside virtio(4), the issue has already bitten us with real
hardware:
1. USB ehci(4) TDs (transfer descriptors), back when they were packed
into shared cache lines. This required BUS_DMA_COHERENT, which is
-- by the letter of bus_dma(9) -- not supposed to affect
correctness.
Packing multiple TDs into a single shared cache line required
BUS_DMA_COHERENT because the host CPU and the device might
otherwise attempt to update the same cache line simultaneously,
leading one party to lose its updates; there was no sequence of
bus_dmamap_syncs that could make it work.
This was addressed by splitting the TDs across cache lines:
https://mail-index.NetBSD.org/source-changes/2024/09/23/msg153483.html
2. Polling loops, such as the following in fxp(4), where the driver
repeatedly loads from the same DMA buffer in memory, separated by
bus_dmamap_sync calls, until the DMA read operation is done and
the CPU loads a different value out of the buffer.
We don't need to define a notion of `the same cache line' in
bus_dma(9) for this case -- it's exactly the same location in
memory every time. The marked PREREAD was added to fix the driver
on sgimips O2.
1858 FXP_CDCONFIGSYNC(sc, BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE);
1859
1860 /*
1861 * Start the config command/DMA.
1862 */
1863 fxp_scb_wait(sc);
1864 CSR_WRITE_4(sc, FXP_CSR_SCB_GENERAL, sc->sc_cddma + FXP_CDCONFIGOFF);
1865 fxp_scb_cmd(sc, FXP_SCB_COMMAND_CU_START);
1866 /* ...and wait for it to complete. */
1867 for (i = 1000; i > 0; i--) {
1868 FXP_CDCONFIGSYNC(sc,
1869 BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
1870 status = le16toh(cbp->cb_status);
=> 1871 FXP_CDCONFIGSYNC(sc, BUS_DMASYNC_PREREAD);
1872 if ((status & FXP_CB_STATUS_C) != 0)
1873 break;
1874 DELAY(1);
1875 }
https://nxr.NetBSD.org/xref/src/sys/dev/ic/i82557.c?r=1.162#1858
In the case of virtio(4), we have both of these issues:
1. Once the guest driver has published a transfer by a store to
vq->vq_avail->idx, it must load vq->vq_used->flags to decide
whether to kick the host:
1259 vq->vq_avail->idx = virtio_rw16(sc, vq->vq_avail_idx);
...
1275 vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
1276 flags = virtio_rw16(sc, vq->vq_used->flags);
1277 if (!(flags & VRING_USED_F_NO_NOTIFY))
1278 sc->sc_ops->kick(sc, vq->vq_index);
Later on, to determine whether the transfer has completed, it must
load vq->vq_used->idx:
https://nxr.NetBSD.org/xref/src/sys/dev/pci/virtio.c?r=1.84#1259
600 bool
601 virtio_vq_is_enqueued(struct virtio_softc *sc, struct virtqueue *vq)
602 {
...
609 vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
610 if (vq->vq_used_idx == virtio_rw16(sc, vq->vq_used->idx))
611 return 0;
...
613 return 1;
614 }
https://nxr.NetBSD.org/xref/src/sys/dev/pci/virtio.c?r=1.84#600
But vq->vq_used->flags and vq->vq_used->idx are 16-bit integers
in a likely-aligned buffer, so they are almost certain to share a
cache line:
177 struct vring_used {
178 uint16_t flags;
179 uint16_t idx;
https://nxr.NetBSD.org/xref/src/sys/dev/pci/virtioreg.h?r=1.15#177
Fortunately, the guest driver and the host device are not competing
to update the cache line at the same time, so to fix virtio(4) on
the affected architectures, it is just a matter of issuing a
PREREAD/POSTREAD cycle between the load of vq->vq_used->flags and
vq->vq_used->idx, not just POSTREAD.
2. Virtio interrupt handlers use virtio_vq_is_enqueued to test
whether a transfer on a virtqueue has completed. If the interrupt
line is shared with other devices, a driver might test
virtio_vq_is_enqueued several times in a row from unrelated
interrupts before it returns true, and this boils down to:
*interrupt*
POSTREAD
load vq->vq_used->idx
*interrupt*
POSTREAD
load vq->vq_used->idx
*interrupt*
POSTREAD
load vq->vq_used->idx
...
On some architectures, since POSTREAD does nothing, the first
value of vq->vq_used->idx may remain cached through the subsequent
loads.
Note: this is related to, and probably necessary for, PR kern/60182:
ld@virtio sometimes hangs up. But there is also another issue there
with missing POSTREAD in a loop inside ld(4) -- not just PREREAD. To
be fixed in a separate commit.
To generate a diff of this commit:
cvs rdiff -u -r1.84 -r1.85 src/sys/dev/pci/virtio.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: feedback->needs-pullups
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Fri, 22 May 2026 15:05:39 +0000
State-Changed-Why:
(partial?) fix committed to HEAD, needs pullup-11, pullup-10, pullup-9
From: Taylor R Campbell <riastradh@NetBSD.org>
To: Tetsuya Isaki <isaki@pastel-flower.jp>
Cc: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>, thorpej@me.com,
nick.hudson@gmx.co.uk, gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org, netbsd-bugs@netbsd.org, adrian@NetBSD.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Fri, 22 May 2026 15:08:59 +0000
> Date: Fri, 22 May 2026 13:48:30 +0900
> From: Tetsuya Isaki <isaki@pastel-flower.jp>
>=20
> At Mon, 18 May 2026 23:29:58 +0000,
> Taylor R Campbell wrote:
> > Isaki-san: Can you verify that this patch works?
>=20
> It worked with these three patches applied. Thank you!
> - sys/arch/virt68k/virt68k/bus_dma.c,v 1.5 (I tested it on -11)
> - sys/dev/pci/ld_virtio.c (from kern/60182)
> - sys/dev/pci/virtio.c (this patch)
Great, thanks!
> By the way, one spurious interrupt has still occurred.
>=20
> ...
> virtio1: allocated 24576 byte for virtqueue 0 for I/O request, size 32
> virtio1: using 16384 byte (1024 entries) indirect descriptors
> virtio1: interrupting at gfpic4 irq 2 (IPL 5)
> ld1: 8192 MB, 4161 cyl, 64 head, 63 sec, 512 bytes/sect x 16777216 sector=
s (0 bytes/physsect)
> Spurious interrupt on CPU ipl 5
Just to confirm, is there anything else at gfpic4 irq 2 ipl 5, or is
the interrupt definitely meant for, but not acknowledged by, virtio1?
From: Taylor R Campbell <riastradh@NetBSD.org>
To: Jason Thorpe <thorpej@me.com>
Cc: Tetsuya Isaki <isaki@pastel-flower.jp>,
Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
nick.hudson@gmx.co.uk, gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org, netbsd-bugs@netbsd.org, adrian@NetBSD.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Fri, 22 May 2026 16:44:53 +0000
> Date: Fri, 22 May 2026 08:31:20 -0400
> From: Jason Thorpe <thorpej@me.com>
>=20
> > On May 18, 2026, at 7:30=E2=80=AFPM, Taylor R Campbell via gnats <gnats=
-admin@NetBSD.org> wrote:
> >=20
> > But on others -- such as m68k, mips, and armv<7 -- POSTREAD alone
> > isn't enough because it's a noop: these architectures assume that
> > after PREREAD, the driver doesn't load the memory in question until
> > the DMA read operation has completed, so flushing cached data in
> > PREREAD and doing nothing in POSTREAD is enough to ensure that the
> > driver's load isn't stale.
>=20
> This isn=E2=80=99t why. The real reason is that the cache
> implementation on some of these platforms does not have an
> invalidate-without-writeback operation.
I don't understand, why would avoiding writeback matter in PREREAD or
POSTREAD?
For PREREAD: If there are buffered stores to the memory in question,
surely it shouldn't matter whether we have the CPU write them back or
discard them, because the hardware device is going to overwrite the
memory anyway?
For POSTREAD: Surely, from the moment PREREAD returns, there should be
no buffered stores pending from the CPU to the relevant part of
memory?
And if the driver issues any additional stores between PREREAD and
POSTREAD, causing stores to get buffered so writeback would make a
difference, isn't that a driver bug no matter what the cache
architecture is?
If there are cached _loads_, do _those_ get written back because the
architecture doesn't distinguish cached loads from buffered stores, so
invalidate-with-writeback on POSTREAD would clobber any updates made
by the hardware device?
From: Jason Thorpe <thorpej@me.com>
To: gnats-bugs@netbsd.org
Cc: kern-bug-people@netbsd.org,
gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org,
isaki@pastel-flower.jp,
"riastradh@netbsd.org" <riastradh@NetBSD.org>
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Fri, 22 May 2026 21:55:39 -0400
> On May 22, 2026, at 12:45=E2=80=AFPM, Taylor R Campbell via gnats =
<gnats-admin@NetBSD.org> wrote:
>=20
> I don't understand, why would avoiding writeback matter in PREREAD or
> POSTREAD?
It=E2=80=99s not a matter of avoiding write-back =E2=80=A6 you can=E2=80=99=
t avoid it, it=E2=80=99s the only way to invalidate a cache line on a =
68040 (it=E2=80=99s wb-inv type operation).
The issue is where you do the write-back. If you do it in POSTREAD, you =
could clobber the data you just got via DMA if a neighboring datum in =
the same cache line were modified by the CPU for some other reason. If =
you do the write-back in PREREAD, there=E2=80=99s no danger of =
clobbering the datum you want to receive via DMA.
Some MIPS CPUs have similar cache behavior.
-- thorpej
From: Tetsuya Isaki <isaki@pastel-flower.jp>
To: Taylor R Campbell <riastradh@NetBSD.org>
Cc: gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Sat, 23 May 2026 13:39:56 +0900
At Fri, 22 May 2026 15:08:59 +0000,
Taylor R Campbell wrote:
> > virtio1: interrupting at gfpic4 irq 2 (IPL 5)
> > ld1: 8192 MB, 4161 cyl, 64 head, 63 sec, 512 bytes/sect x 16777216 sectors (0 bytes/physsect)
> > Spurious interrupt on CPU ipl 5
>
> Just to confirm, is there anything else at gfpic4 irq 2 ipl 5, or is
> the interrupt definitely meant for, but not acknowledged by, virtio1?
Then I tested it under several conditions, and it might not be related
to ld@virtio1. I'll take a look but separate it from this PR for now.
Thank you for your help.
---
Tetsuya Isaki <isaki@pastel-flower.jp / isaki@NetBSD.org>
From: Tetsuya Isaki <isaki@pastel-flower.jp>
To: Taylor R Campbell <riastradh@NetBSD.org>
Cc: gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Thu, 28 May 2026 13:10:27 +0900
At Sat, 23 May 2026 13:39:56 +0900,
Tetsuya Isaki wrote:
> > > virtio1: interrupting at gfpic4 irq 2 (IPL 5)
> > > ld1: 8192 MB, 4161 cyl, 64 head, 63 sec, 512 bytes/sect x 16777216 sectors (0 bytes/physsect)
> > > Spurious interrupt on CPU ipl 5
> >
> > Just to confirm, is there anything else at gfpic4 irq 2 ipl 5, or is
> > the interrupt definitely meant for, but not acknowledged by, virtio1?
>
> Then I tested it under several conditions, and it might not be related
> to ld@virtio1. I'll take a look but separate it from this PR for now.
I filed this as a separate issue, PR kern/60297
"virtio(4) polling is not properly supported for MMIO".
Thanks,
---
Tetsuya Isaki <isaki@pastel-flower.jp / isaki@NetBSD.org>
From: Tetsuya Isaki <isaki@pastel-flower.jp>
To: Taylor R Campbell <riastradh@NetBSD.org>
Cc: thorpej@me.com,
gnats-bugs@netbsd.org,
kern-bug-people@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: kern/60144: virtio(4) cache coherence issue
Date: Fri, 19 Jun 2026 16:51:07 +0900
At Fri, 22 May 2026 15:08:59 +0000,
Taylor R Campbell wrote:
> > It worked with these three patches applied. Thank you!
> > - sys/arch/virt68k/virt68k/bus_dma.c,v 1.5 (I tested it on -11)
> > - sys/dev/pci/ld_virtio.c (from kern/60182)
> > - sys/dev/pci/virtio.c (this patch)
>
> Great, thanks!
Would you pullup these fixes to netbsd-11 ? or may I?
---
Tetsuya Isaki <isaki@pastel-flower.jp / isaki@NetBSD.org>
State-Changed-From-To: needs-pullups->pending-pullups
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Fri, 26 Jun 2026 02:24:06 +0000
State-Changed-Why:
pullup-11 #332 https://releng.netbsd.org/cgi-bin/req-11.cgi?show=332
pullup-10 #1280 https://releng.netbsd.org/cgi-bin/req-10.cgi?show=1280
needs a lot more work for netbsd-9, lotta other changes
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/60144 CVS commit: [netbsd-11] src/sys/dev/pci
Date: Sat, 27 Jun 2026 16:00:25 +0000
Module Name: src
Committed By: martin
Date: Sat Jun 27 16:00:25 UTC 2026
Modified Files:
src/sys/dev/pci [netbsd-11]: virtio.c
Log Message:
Pull up following revision(s) (requested by riastradh in ticket #332):
sys/dev/pci/virtio.c: revision 1.85
virtio(4): Add missing BUS_DMASYNC_PREREAD operations.
And one missing BUS_DMASYNC_POSTREAD operation.
With this change, loads from vq->vq_used->flags (which occur
immediately after a transfer is submitted to test whether we need to
kick the host device) is separated by a PREREAD/POSTREAD cycle from
loads from vq->vq_used->idx (which occur when we think a transfer may
have completed, e.g. upon receiving an interrupt, to test whether it
has, in fact, completed).
Additionally, with this change, consecutive loads from
*vq->vq_avail_event are separated by a PREREAD/POSTREAD cycle.
Should fix virtio(4) issues on m68k and other related architectures
like mips and armv<7:
PR kern/60144: virtio(4) cache coherence issue
(And may make what I think is a mere potential performance
improvement on those and other architectures like x86 or aarch64,
because of the change to virtio_postpone_intr.)
Why?
This is tricky, and (despite a remark I made to the contrary in the
PR) I'm still not entirely convinced we have the rules articulated
sufficiently to prove the driver correct with or without the
changes, or to prove a bus_dma implementation correct.
But, based on the current mengarie of bus_dma implementations we
have, the crux of the issue is:
When we load twice from the same cache line affected by a DMA read
operation, if the DMA read operation may complete in the interim
between the two loads, it is necessary to have a PREREAD/POSTREAD
cycle between the loads -- not just POSTREAD.
For example, in a polling loop:
buf = alloc_buf()
bus_dmamap_load(map, buf)
bus_dmamap_sync(map, 0, offsetof(buf->ready), PREREAD)
bus_space_write(trigger_dma_at, map->dm_segs[0].ds_addr);
bus_dmamap_sync(map, 0, offsetof(buf->ready), POSTREAD)
while (!buf->ready) {
bus_dmamap_sync(map, 0, offsetof(buf->ready), PREREAD)
bus_dmamap_sync(map, 0, offsetof(buf->ready), POSTREAD)
}
...
The pair of PREREAD/POSTREAD syncs in the body of the loop must
_both_ appear in order for this to work; POSTREAD alone is not
enough.
On some architectures, such as x86, PREREAD/POSTREAD is stronger than
needed.
But on others -- such as m68k, mips, and armv<7 -- POSTREAD alone
isn't enough because it's a noop: these architectures assume that
after PREREAD, the driver doesn't load the memory in question until
the DMA read operation has completed, so flushing cached data in
PREREAD and doing nothing in POSTREAD is enough to ensure that the
driver's load isn't stale.
This scenario isn't really spelled out in the bus_dma(9) man page.
And bus_dma(9) doesn't even have a concept of cache lines. But, even
outside virtio(4), the issue has already bitten us with real
hardware:
1. USB ehci(4) TDs (transfer descriptors), back when they were packed
into shared cache lines. This required BUS_DMA_COHERENT, which is
-- by the letter of bus_dma(9) -- not supposed to affect
correctness.
Packing multiple TDs into a single shared cache line required
BUS_DMA_COHERENT because the host CPU and the device might
otherwise attempt to update the same cache line simultaneously,
leading one party to lose its updates; there was no sequence of
bus_dmamap_syncs that could make it work.
This was addressed by splitting the TDs across cache lines:
<a rel="nofollow" href="https://mail-index.NetBSD.org/source-changes/2024/09/23/msg153483.html">https://mail-index.NetBSD.org/source-changes/2024/09/23/msg153483.html</a>
2. Polling loops, such as the following in fxp(4), where the driver
repeatedly loads from the same DMA buffer in memory, separated by
bus_dmamap_sync calls, until the DMA read operation is done and
the CPU loads a different value out of the buffer.
We don't need to define a notion of `the same cache line' in
bus_dma(9) for this case -- it's exactly the same location in
memory every time. The marked PREREAD was added to fix the driver
on sgimips O2.
1858 FXP_CDCONFIGSYNC(sc, BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE);
1859
1860 /*
1861 * Start the config command/DMA.
1862 */
1863 fxp_scb_wait(sc);
1864 CSR_WRITE_4(sc, FXP_CSR_SCB_GENERAL, sc->sc_cddma + FXP_CDCONFIGOFF);
1865 fxp_scb_cmd(sc, FXP_SCB_COMMAND_CU_START);
1866 /* ...and wait for it to complete. */
1867 for (i = 1000; i > 0; i--) {
1868 FXP_CDCONFIGSYNC(sc,
1869 BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
1870 status = le16toh(cbp->cb_status);
=> 1871 FXP_CDCONFIGSYNC(sc, BUS_DMASYNC_PREREAD);
1872 if ((status & FXP_CB_STATUS_C) != 0)
1873 break;
1874 DELAY(1);
1875 }
https://nxr.NetBSD.org/xref/src/sys/dev/ic/i82557.c?r=1.162#1858
In the case of virtio(4), we have both of these issues:
1. Once the guest driver has published a transfer by a store to
vq->vq_avail->idx, it must load vq->vq_used->flags to decide
whether to kick the host:
1259 vq->vq_avail->idx = virtio_rw16(sc, vq->vq_avail_idx);
...
1275 vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
1276 flags = virtio_rw16(sc, vq->vq_used->flags);
1277 if (!(flags & VRING_USED_F_NO_NOTIFY))
1278 sc->sc_ops->kick(sc, vq->vq_index);
Later on, to determine whether the transfer has completed, it must
load vq->vq_used->idx:
https://nxr.NetBSD.org/xref/src/sys/dev/pci/virtio.c?r=1.84#1259
600 bool
601 virtio_vq_is_enqueued(struct virtio_softc *sc, struct virtqueue *vq)
602 {
...
609 vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
610 if (vq->vq_used_idx == virtio_rw16(sc, vq->vq_used->idx))
611 return 0;
...
613 return 1;
614 }
https://nxr.NetBSD.org/xref/src/sys/dev/pci/virtio.c?r=1.84#600
But vq->vq_used->flags and vq->vq_used->idx are 16-bit integers
in a likely-aligned buffer, so they are almost certain to share a
cache line:
177 struct vring_used {
178 uint16_t flags;
179 uint16_t idx;
https://nxr.NetBSD.org/xref/src/sys/dev/pci/virtioreg.h?r=1.15#177
Fortunately, the guest driver and the host device are not competing
to update the cache line at the same time, so to fix virtio(4) on
the affected architectures, it is just a matter of issuing a
PREREAD/POSTREAD cycle between the load of vq->vq_used->flags and
vq->vq_used->idx, not just POSTREAD.
2. Virtio interrupt handlers use virtio_vq_is_enqueued to test
whether a transfer on a virtqueue has completed. If the interrupt
line is shared with other devices, a driver might test
virtio_vq_is_enqueued several times in a row from unrelated
interrupts before it returns true, and this boils down to:
*interrupt*
POSTREAD
load vq->vq_used->idx
*interrupt*
POSTREAD
load vq->vq_used->idx
*interrupt*
POSTREAD
load vq->vq_used->idx
...
On some architectures, since POSTREAD does nothing, the first
value of vq->vq_used->idx may remain cached through the subsequent
loads.
Note: this is related to, and probably necessary for, PR kern/60182:
ld@virtio sometimes hangs up. But there is also another issue there
with missing POSTREAD in a loop inside ld(4) -- not just PREREAD. To
be fixed in a separate commit.
To generate a diff of this commit:
cvs rdiff -u -r1.83.2.1 -r1.83.2.2 src/sys/dev/pci/virtio.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/60144 CVS commit: [netbsd-10] src/sys/dev/pci
Date: Sat, 27 Jun 2026 16:02:36 +0000
Module Name: src
Committed By: martin
Date: Sat Jun 27 16:02:36 UTC 2026
Modified Files:
src/sys/dev/pci [netbsd-10]: virtio.c
Log Message:
Pull up following revision(s) (requested by riastradh in ticket #1280):
sys/dev/pci/virtio.c: revision 1.85
virtio(4): Add missing BUS_DMASYNC_PREREAD operations.
And one missing BUS_DMASYNC_POSTREAD operation.
With this change, loads from vq->vq_used->flags (which occur
immediately after a transfer is submitted to test whether we need to
kick the host device) is separated by a PREREAD/POSTREAD cycle from
loads from vq->vq_used->idx (which occur when we think a transfer may
have completed, e.g. upon receiving an interrupt, to test whether it
has, in fact, completed).
Additionally, with this change, consecutive loads from
*vq->vq_avail_event are separated by a PREREAD/POSTREAD cycle.
Should fix virtio(4) issues on m68k and other related architectures
like mips and armv<7:
PR kern/60144: virtio(4) cache coherence issue
(And may make what I think is a mere potential performance
improvement on those and other architectures like x86 or aarch64,
because of the change to virtio_postpone_intr.)
Why?
This is tricky, and (despite a remark I made to the contrary in the
PR) I'm still not entirely convinced we have the rules articulated
sufficiently to prove the driver correct with or without the
changes, or to prove a bus_dma implementation correct.
But, based on the current mengarie of bus_dma implementations we
have, the crux of the issue is:
When we load twice from the same cache line affected by a DMA read
operation, if the DMA read operation may complete in the interim
between the two loads, it is necessary to have a PREREAD/POSTREAD
cycle between the loads -- not just POSTREAD.
For example, in a polling loop:
buf = alloc_buf()
bus_dmamap_load(map, buf)
bus_dmamap_sync(map, 0, offsetof(buf->ready), PREREAD)
bus_space_write(trigger_dma_at, map->dm_segs[0].ds_addr);
bus_dmamap_sync(map, 0, offsetof(buf->ready), POSTREAD)
while (!buf->ready) {
bus_dmamap_sync(map, 0, offsetof(buf->ready), PREREAD)
bus_dmamap_sync(map, 0, offsetof(buf->ready), POSTREAD)
}
...
The pair of PREREAD/POSTREAD syncs in the body of the loop must
_both_ appear in order for this to work; POSTREAD alone is not
enough.
On some architectures, such as x86, PREREAD/POSTREAD is stronger than
needed.
But on others -- such as m68k, mips, and armv<7 -- POSTREAD alone
isn't enough because it's a noop: these architectures assume that
after PREREAD, the driver doesn't load the memory in question until
the DMA read operation has completed, so flushing cached data in
PREREAD and doing nothing in POSTREAD is enough to ensure that the
driver's load isn't stale.
This scenario isn't really spelled out in the bus_dma(9) man page.
And bus_dma(9) doesn't even have a concept of cache lines. But, even
outside virtio(4), the issue has already bitten us with real
hardware:
1. USB ehci(4) TDs (transfer descriptors), back when they were packed
into shared cache lines. This required BUS_DMA_COHERENT, which is
-- by the letter of bus_dma(9) -- not supposed to affect
correctness.
Packing multiple TDs into a single shared cache line required
BUS_DMA_COHERENT because the host CPU and the device might
otherwise attempt to update the same cache line simultaneously,
leading one party to lose its updates; there was no sequence of
bus_dmamap_syncs that could make it work.
This was addressed by splitting the TDs across cache lines:
<a rel="nofollow" href="https://mail-index.NetBSD.org/source-changes/2024/09/23/msg153483.html">https://mail-index.NetBSD.org/source-changes/2024/09/23/msg153483.html</a>
2. Polling loops, such as the following in fxp(4), where the driver
repeatedly loads from the same DMA buffer in memory, separated by
bus_dmamap_sync calls, until the DMA read operation is done and
the CPU loads a different value out of the buffer.
We don't need to define a notion of `the same cache line' in
bus_dma(9) for this case -- it's exactly the same location in
memory every time. The marked PREREAD was added to fix the driver
on sgimips O2.
1858 FXP_CDCONFIGSYNC(sc, BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE);
1859
1860 /*
1861 * Start the config command/DMA.
1862 */
1863 fxp_scb_wait(sc);
1864 CSR_WRITE_4(sc, FXP_CSR_SCB_GENERAL, sc->sc_cddma + FXP_CDCONFIGOFF);
1865 fxp_scb_cmd(sc, FXP_SCB_COMMAND_CU_START);
1866 /* ...and wait for it to complete. */
1867 for (i = 1000; i > 0; i--) {
1868 FXP_CDCONFIGSYNC(sc,
1869 BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE);
1870 status = le16toh(cbp->cb_status);
=> 1871 FXP_CDCONFIGSYNC(sc, BUS_DMASYNC_PREREAD);
1872 if ((status & FXP_CB_STATUS_C) != 0)
1873 break;
1874 DELAY(1);
1875 }
https://nxr.NetBSD.org/xref/src/sys/dev/ic/i82557.c?r=1.162#1858
In the case of virtio(4), we have both of these issues:
1. Once the guest driver has published a transfer by a store to
vq->vq_avail->idx, it must load vq->vq_used->flags to decide
whether to kick the host:
1259 vq->vq_avail->idx = virtio_rw16(sc, vq->vq_avail_idx);
...
1275 vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
1276 flags = virtio_rw16(sc, vq->vq_used->flags);
1277 if (!(flags & VRING_USED_F_NO_NOTIFY))
1278 sc->sc_ops->kick(sc, vq->vq_index);
Later on, to determine whether the transfer has completed, it must
load vq->vq_used->idx:
https://nxr.NetBSD.org/xref/src/sys/dev/pci/virtio.c?r=1.84#1259
600 bool
601 virtio_vq_is_enqueued(struct virtio_softc *sc, struct virtqueue *vq)
602 {
...
609 vq_sync_uring_header(sc, vq, BUS_DMASYNC_POSTREAD);
610 if (vq->vq_used_idx == virtio_rw16(sc, vq->vq_used->idx))
611 return 0;
...
613 return 1;
614 }
https://nxr.NetBSD.org/xref/src/sys/dev/pci/virtio.c?r=1.84#600
But vq->vq_used->flags and vq->vq_used->idx are 16-bit integers
in a likely-aligned buffer, so they are almost certain to share a
cache line:
177 struct vring_used {
178 uint16_t flags;
179 uint16_t idx;
https://nxr.NetBSD.org/xref/src/sys/dev/pci/virtioreg.h?r=1.15#177
Fortunately, the guest driver and the host device are not competing
to update the cache line at the same time, so to fix virtio(4) on
the affected architectures, it is just a matter of issuing a
PREREAD/POSTREAD cycle between the load of vq->vq_used->flags and
vq->vq_used->idx, not just POSTREAD.
2. Virtio interrupt handlers use virtio_vq_is_enqueued to test
whether a transfer on a virtqueue has completed. If the interrupt
line is shared with other devices, a driver might test
virtio_vq_is_enqueued several times in a row from unrelated
interrupts before it returns true, and this boils down to:
*interrupt*
POSTREAD
load vq->vq_used->idx
*interrupt*
POSTREAD
load vq->vq_used->idx
*interrupt*
POSTREAD
load vq->vq_used->idx
...
On some architectures, since POSTREAD does nothing, the first
value of vq->vq_used->idx may remain cached through the subsequent
loads.
Note: this is related to, and probably necessary for, PR kern/60182:
ld@virtio sometimes hangs up. But there is also another issue there
with missing POSTREAD in a loop inside ld(4) -- not just PREREAD. To
be fixed in a separate commit.
To generate a diff of this commit:
cvs rdiff -u -r1.63.2.7 -r1.63.2.8 src/sys/dev/pci/virtio.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: pending-pullups->needs-pullups
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Sat, 27 Jun 2026 16:36:22 +0000
State-Changed-Why:
fixed in HEAD, pulled up to 11 and 10
leaving this in needs-pullups for 9 but realistically I doubt it'll
happen
State-Changed-From-To: needs-pullups->closed
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Sat, 27 Jun 2026 17:05:15 +0000
State-Changed-Why:
probably not worth the trouble for 9
State-Changed-From-To: closed->needs-pullups
State-Changed-By: isaki@NetBSD.org
State-Changed-When: Sun, 28 Jun 2026 04:39:18 +0000
State-Changed-Why:
I handle the MD part.
State-Changed-From-To: needs-pullups->pending-pullups
State-Changed-By: isaki@NetBSD.org
State-Changed-When: Sun, 28 Jun 2026 05:31:43 +0000
State-Changed-Why:
[pullup-11 #347] for -11.
No need to pullup -10 because virt68k didn't exist there yet.
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/60144 CVS commit: [netbsd-11] src/sys/arch/virt68k/virt68k
Date: Sun, 28 Jun 2026 09:36:55 +0000
Module Name: src
Committed By: martin
Date: Sun Jun 28 09:36:55 UTC 2026
Modified Files:
src/sys/arch/virt68k/virt68k [netbsd-11]: bus_dma.c
Log Message:
Pull up following revision(s) (requested by isaki in ticket #347):
sys/arch/virt68k/virt68k/bus_dma.c: revision 1.5
Fill in bus_dmamap_sync() for 68030. Since 68030 caches are write-through,
this is pretty trivial: just invalidate the on-chip + any external D cache
in the PREREAD case. (See comment in code for why PREREAD is preferred
over POSTREAD.)
PR kern/60144
To generate a diff of this commit:
cvs rdiff -u -r1.1 -r1.1.4.1 src/sys/arch/virt68k/virt68k/bus_dma.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: pending-pullups->closed
State-Changed-By: isaki@NetBSD.org
State-Changed-When: Tue, 30 Jun 2026 07:44:57 +0000
State-Changed-Why:
pulled up.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.51 2026/08/10 02:28:17 riastradh Exp $
$NetBSD: gnats_config.sh,v 1.10 2026/05/13 22:00:09 riastradh Exp $
Copyright © 1994-2026
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home