[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/2] virtio: check VirtQueue Vring object is
From: |
Cornelia Huck |
Subject: |
Re: [Qemu-devel] [PATCH v3 1/2] virtio: check VirtQueue Vring object is set |
Date: |
Tue, 28 Nov 2017 10:11:54 +0100 |
On Mon, 27 Nov 2017 23:25:28 +0530 (IST)
P J P <address@hidden> wrote:
> +-- On Mon, 27 Nov 2017, Cornelia Huck wrote --+
> |The check for align is not really needed, as virtio-1 disallows setting
> align
> |anyway.
>
> disallows...?
See the check in virtio_queue_set_align(). Moreover, the calculation
that breaks virtqueue setup for align == 0 is only called for the
legacy setup, IOW not for this virtio-1 only function.
>
> | Checking for !desc is wrong (why shouldn't a driver be able to unset a
> | descriptor table?)
>
> +-- On Mon, 27 Nov 2017, Stefan Hajnoczi wrote --+
> | > + if (!vdev->vq[n].vring.num || !desc || !vdev->vq[n].vring.align) {
> | ...
> | vdev->vq[n].vring.desc = desc;
> |
> | Why !desc?
>
> virtio_queue_set_rings
> virtio_init_region_cache
> VirtQueue *vq = &vdev->vq[n];
> ...
> addr = vq->vring.desc;
> if (!addr) {
> return;
> }
>
> These checks seem to be repeating all over. As mentioned earlier, could these
> be collated in one place, maybe virtio_queue_get_num()?
>
> int virtio_queue_get_num(VirtIODevice *vdev, int n)
> {
> VirtQueue *vq = &vdev->vq[n];
>
> if (!vq->.vring.num
> || !vq->vring.desc
> || !vq->vring.align) {
> return 0; /* vq not set */
> }
>
> return vdev->vq[n].vring.num;
> }
This is conflating different things:
- vq does not exist (num == 0)
- vq is not setup by the guest (desc == 0)
- vq has no valid alignment (which is only relevant for legacy)
- [Qemu-devel] [PATCH v3 0/2] check VirtiQueue Vring objects, P J P, 2017/11/24
- [Qemu-devel] [PATCH v3 1/2] virtio: check VirtQueue Vring object is set, P J P, 2017/11/24
- Re: [Qemu-devel] [PATCH v3 1/2] virtio: check VirtQueue Vring object is set, Cornelia Huck, 2017/11/27
- Re: [Qemu-devel] [PATCH v3 1/2] virtio: check VirtQueue Vring object is set, Stefan Hajnoczi, 2017/11/27
- Re: [Qemu-devel] [PATCH v3 1/2] virtio: check VirtQueue Vring object is set, P J P, 2017/11/27
- Re: [Qemu-devel] [PATCH v3 1/2] virtio: check VirtQueue Vring object is set,
Cornelia Huck <=
- Re: [Qemu-devel] [PATCH v3 1/2] virtio: check VirtQueue Vring object is set, Stefan Hajnoczi, 2017/11/28
- Re: [Qemu-devel] [PATCH v3 1/2] virtio: check VirtQueue Vring object is set, P J P, 2017/11/28
- Re: [Qemu-devel] [PATCH v3 1/2] virtio: check VirtQueue Vring object is set, Cornelia Huck, 2017/11/28
- Re: [Qemu-devel] [PATCH v3 1/2] virtio: check VirtQueue Vring object is set, P J P, 2017/11/29
- Re: [Qemu-devel] [PATCH v3 1/2] virtio: check VirtQueue Vring object is set, Cornelia Huck, 2017/11/29
- Re: [Qemu-devel] [PATCH v3 1/2] virtio: check VirtQueue Vring object is set, P J P, 2017/11/30
[Qemu-devel] [PATCH v3 2/2] tests: add test to check VirtQueue object, P J P, 2017/11/24