> -----Original Message-----
> From: Philippe Mathieu-Daudé <phi...@linaro.org>
> Sent: 2024年11月22日 15:02
> To: Wafer <wa...@jaguarmicro.com>; m...@redhat.com;
> jasow...@redhat.com; gr...@kaod.org
> Cc: epere...@redhat.com; qemu-devel@nongnu.org; Angus Chen
> <angus.c...@jaguarmicro.com>
> Subject: Re: [PATCH] hw/virtio: Fix getting the correct ring number on loading
>
> External Mail: This email originated from OUTSIDE of the organization!
> Do not click links, open attachments or provide ANY information unless you
> recognize the sender and know the content is safe.
>
>
> Hi Wafer,
>
> On 22/11/24 03:00, Wafer wrote:
> > From: Wafer Xie <wa...@jaguarmicro.com>
> >
> > The virtio-1.2 specification writes:
> >
> > 2.7.6 The Virtqueue Available Ring:
> > "idx field indicates where the driver would put the next descriptor
> > entry in the ring (modulo the queue size). This starts at 0, and increases"
>
> "modulo" ...
>
> >
> > The idx will increase from 0 to 0xFFFF and repeat, So idx may be less
> > than last_avail_idx.
> >
> > Fixes: 616a6552 (virtio: add endian-ambivalent support to
> > VirtIODevice)
>
> This commit is only about endianness... Do you mean 1abeb5a65d
> ("virtio: fix up VQ checks") or 258dc7c96b ("virtio: sanity-check available
> index")?
Thanks, I mean 258dc7c96b ("virtio: sanity-check available index")
I will make changes in the next version.
>
> >
> > Signed-off-by: Wafer Xie <wa...@jaguarmicro.com>
> > ---
> > hw/virtio/virtio.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index
> > a26f18908e..ae7d407113 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -3362,7 +3362,13 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int
> version_id)
> > continue;
> > }
> >
> > - nheads = vring_avail_idx(&vdev->vq[i]) -
> > vdev->vq[i].last_avail_idx;
> > + if (vring_avail_idx(&vdev->vq[i]) >=
> > vdev->vq[i].last_avail_idx) {
> > + nheads = vring_avail_idx(&vdev->vq[i]) -
> > + vdev->vq[i].last_avail_idx;
> > + } else {
> > + nheads = UINT16_MAX - vdev->vq[i].last_avail_idx +
> > + vring_avail_idx(&vdev->vq[i]) + 1;
> > + }
>
> ... nheads %= UINT16_MAX; ?
nheads cannot exceed UINT16_MAX, but is invalid if it exceed vring.num
> > /* Check it isn't doing strange things with descriptor
> > numbers. */
> > if (nheads > vdev->vq[i].vring.num) {
> > virtio_error(vdev, "VQ %d size 0x%x Guest index 0x%x "
