Re: [RFC PATCH] tests/qtest: properly initialise the vring used idx

[Eric Auger]

Hi Alex,

On 4/6/22 7:33 PM, Alex Bennée wrote:
> Eric noticed while attempting to enable the vhost-user-blk-test for
> Aarch64 that that things didn't work unless he put in a dummy
> guest_malloc() at the start of the test. Without it
> qvirtio_wait_used_elem() would assert when it reads a junk value for
> idx resulting in:
>
>   qvirtqueue_get_buf: idx:2401 last_idx:0
>   qvirtqueue_get_buf: 0x7ffcb6d3fe74, (nil)
>   qvirtio_wait_used_elem: 3000000/0
>   ERROR:../../tests/qtest/libqos/virtio.c:226:qvirtio_wait_used_elem: 
> assertion failed (got_desc_idx == desc_idx): (50331648 == 0)
>   Bail out! 
> ERROR:../../tests/qtest/libqos/virtio.c:226:qvirtio_wait_used_elem: assertion 
> failed (got_desc_idx == desc_idx): (50331648 == 0)
>
> What was actually happening is the guest_malloc() effectively pushed
> the allocation of the vring into the next page which just happened to
> have clear memory. After much tedious tracing of the code I could see
Many thanks for the tedious investigation!
> that qvring_init() does attempt initialise a bunch of the vring
> structures but skips the vring->used.idx value. It is probably not
> wise to assume guest memory is zeroed anyway. Once the ring is
> properly initialised the hack is no longer needed to get things
> working.
>
> Thanks-to: John Snow <jsnow@redhat.com> for helping debug
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Raphael Norwitz <raphael.norwitz@nutanix.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  tests/qtest/libqos/virtio.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tests/qtest/libqos/virtio.c b/tests/qtest/libqos/virtio.c
> index 6fe7bf9555..fba9186659 100644
> --- a/tests/qtest/libqos/virtio.c
> +++ b/tests/qtest/libqos/virtio.c
> @@ -260,6 +260,8 @@ void qvring_init(QTestState *qts, const QGuestAllocator 
> *alloc, QVirtQueue *vq,
>  
>      /* vq->used->flags */
>      qvirtio_writew(vq->vdev, qts, vq->used, 0);
> +    /* vq->used->idx */
> +    qvirtio_writew(vq->vdev, qts, vq->used + 2, 0);
>      /* vq->used->avail_event */
>      qvirtio_writew(vq->vdev, qts, vq->used + 2 +
>                     sizeof(struct vring_used_elem) * vq->size, 0);
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>

Eric