Virtio Bug

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Virtio Bug

Gary Zibrat
I've been looking at an issue on virtual hardware that only happens on
openbsd.
The problem is that the first access to a virtio scsi queue is reading out
an uninitialized value.

I can't claim to know a lot about openbsd but looking that the source here
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/pv/virtio.c?rev=1.16&content-type=text/x-cvsweb-markup
looks a bit suspicious.

In virtio_alloc_vq, virtio_setup_queue is called before the queue is filled
in completely by virtio_init_vq.

I see virtio_reinit_start has the ordering flipped:
virtio_init_vq(sc, vq, 1);
virtio_setup_queue(sc, vq, vq->vq_dmamap->dm_segs[0].ds_addr);

I saw that a change here that moved virtio_setup_queue later, but not as
late as I'd think it should be.
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/dev/pv/virtio.c.diff?r1=1.13&r2=1.14&f=h
We are currently trying to test a version that has this change, but in the
meantime it'd be nice if someone could confirm that the ordering of the
calls is correct.

The reasons I suspect it is an OS bug are due to the ways I was able to fix
the issue:
In the virtual hardware I simply put a sleep before the first access and it
worked fine.
The other way I was able to fix it was by having the firmware zero the
pages the OS ends up using for the virtio queue.
Reply | Threaded
Open this post in threaded view
|

Re: Virtio Bug

Stefan Fritsch
Hi Gary,

On Tue, 16 Apr 2019, Gary Zibrat wrote:

> In virtio_alloc_vq, virtio_setup_queue is called before the queue is filled
> in completely by virtio_init_vq.

Yes, you are right. That bug has been there from the very start. I have
committed a fix.

> The reasons I suspect it is an OS bug are due to the ways I was able to fix
> the issue:
> In the virtual hardware I simply put a sleep before the first access and it
> worked fine.
> The other way I was able to fix it was by having the firmware zero the
> pages the OS ends up using for the virtio queue.

I guess this bug is not visible with other virtio devices (in qemu, vmm,
...) because those do not look at the virtqueues before the driver sends
the first notify to the device. Maybe that would be a way you could be
compatible with older openbsd versions.

Cheers,
Stefan