[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d80b0dd5-1db2-4796-bb88-37212f6a4d24@amazon.com>
Date: Wed, 28 Jan 2026 11:38:48 +0100
From: Alexander Graf <graf@...zon.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
CC: Johannes Thumshirn <Johannes.Thumshirn@....com>,
"virtualization@...ts.linux.dev" <virtualization@...ts.linux.dev>, Jason Wang
<jasowang@...hat.com>, Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
Eugenio Pérez <eperezma@...hat.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] virtio: silence KCSAN warning in
virtqueue_get_buf_ctx_split
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. 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.
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
Powered by blists - more mailing lists