On Wed, Jan 28, 2026 at 11:38:48AM +0100, Alexander Graf wrote: > > On 28.01.26 11:34, Michael S. Tsirkin wrote: > > On Wed, Jan 28, 2026 at 11:30:20AM +0100, Alexander Graf wrote: > > > On 28.01.26 10:13, Johannes Thumshirn wrote: > > > > On 1/28/26 10:03 AM, Alexander Graf wrote: > > > > > On 28.01.26 09:47, Johannes Thumshirn wrote: > > > > > > On 1/27/26 5:30 PM, Alexander Graf wrote: > > > > > > > This patches the split vring format, but does not touch the > > > > > > > packed one. > > > > > > > What happens if you run the same test with the packed format? You > > > > > > > can do > > > > > > > so by passing "packed=on" as argument to your -device parameter. > > > > > > This opened up a whole new can of worms... :( > > > > > That's what I expected :). > > > > > > > > > > How do other DMA based devices handle this? Is the real problem that > > > > > virtio by default does not use the DMA API and so it confuses generic > > > > > KCSAN logic that would otherwise track DMA regions as "can be modified > > > > > by DMA at any time"? > > > > > > > > > > If that is the case, maybe what we really want is to force enable use > > > > > of > > > > > the DMA API when KCSAN is active. Does something like the (whitespace > > > > > broken) patch below work? > > > > > > > > > > Alex > > > > > > > > > > diff --git a/drivers/virtio/virtio_ring.c > > > > > b/drivers/virtio/virtio_ring.c > > > > > index ddab68959671..b1dd790ce622 100644 > > > > > --- a/drivers/virtio/virtio_ring.c > > > > > +++ b/drivers/virtio/virtio_ring.c > > > > > @@ -284,6 +284,13 @@ static bool vring_use_map_api(const struct > > > > > virtio_device *vdev) > > > > > if (xen_domain()) > > > > > return true; > > > > > > > > > > + /* > > > > > + * KCSAN needs to track who can modify memory. DMA API gets > > > > > + * us that, so always use it. > > > > > + */ > > > > > + if (IS_ENABLED(CONFIG_KCSAN)) > > > > > + return true; > > > > > + > > > > > return false; > > > > > } > > > > Unfortunately this doesn't get us any further (I'd love though, it looks > > > > way cleaner!) > > > > > > > > I still see the KCSAN messages even on boot. > > > > > > Ah, looks like the important bit for KCSAN is not the mapping mechanism, > > > it's the actual compiler annotation for the read. So these virtio ring > > > reads > > > should all be annotated as READ_ONCE() to make sure KCSAN knows the read > > > itself is atomic. > > > > > > Alex > > > > > so then: > > > > return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev, > > READ_ONCE(vq->split.vring.used->idx)); > > > Yes, which is on the verge of getting unreadable.
Well we can wrap virtio16_to_cpu and READ_ONCE in a single macro if you like. VIRTIO16_READ ? > I'll work with Johannes on > v3. We'll play with ways to make it a bit more maintainable. Please discard > the current patches for now. Sure. > > Alex > > > > > Amazon Web Services Development Center Germany GmbH > Tamara-Danz-Str. 13 > 10243 Berlin > Geschaeftsfuehrung: Christof Hellmis, Andreas Stieger > Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B > Sitz: Berlin > Ust-ID: DE 365 538 597
