Message ID | 20230506150111.2496-1-yin31149@gmail.com |
---|---|
State | New |
Headers | show |
Series | [RESEND] vhost: fix possible wrap in SVQ descriptor ring | expand |
On Sat, May 6, 2023 at 5:01 PM Hawkins Jiawei <yin31149@gmail.com> wrote: > > QEMU invokes vhost_svq_add() when adding a guest's element into SVQ. > In vhost_svq_add(), it uses vhost_svq_available_slots() to check > whether QEMU can add the element into the SVQ. If there is > enough space, then QEMU combines some out descriptors and > some in descriptors into one descriptor chain, and add it into > svq->vring.desc by vhost_svq_vring_write_descs(). > > Yet the problem is that, `svq->shadow_avail_idx - svq->shadow_used_idx` > in vhost_svq_available_slots() return the number of occupied elements, > or the number of descriptor chains, instead of the number of occupied > descriptors, which may cause wrapping in SVQ descriptor ring. > > Here is an example. In vhost_handle_guest_kick(), QEMU forwards > as many available buffers to device by virtqueue_pop() and > vhost_svq_add_element(). virtqueue_pop() return a guest's element, > and use vhost_svq_add_elemnt(), a wrapper to vhost_svq_add(), to > add this element into SVQ. If QEMU invokes virtqueue_pop() and > vhost_svq_add_element() `svq->vring.num` times, vhost_svq_available_slots() > thinks QEMU just ran out of slots and everything should work fine. > But in fact, virtqueue_pop() return `svq-vring.num` elements or > descriptor chains, more than `svq->vring.num` descriptors, due to > guest memory fragmentation, and this cause wrapping in SVQ descriptor ring. > The bug is valid even before marking the descriptors used. If the guest memory is fragmented, SVQ must add chains so it can try to add more descriptors than possible. > Therefore, this patch adds `num_free` field in VhostShadowVirtqueue > structure, updates this field in vhost_svq_add() and > vhost_svq_get_buf(), to record the number of free descriptors. > Then we can avoid wrap in SVQ descriptor ring by refactoring > vhost_svq_available_slots(). > > Fixes: 100890f7ca ("vhost: Shadow virtqueue buffers forwarding") > Signed-off-by: Hawkins Jiawei <yin31149@gmail.com> > --- > hw/virtio/vhost-shadow-virtqueue.c | 9 ++++++++- > hw/virtio/vhost-shadow-virtqueue.h | 3 +++ > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c > index 8361e70d1b..e1c6952b10 100644 > --- a/hw/virtio/vhost-shadow-virtqueue.c > +++ b/hw/virtio/vhost-shadow-virtqueue.c > @@ -68,7 +68,7 @@ bool vhost_svq_valid_features(uint64_t features, Error **errp) > */ > static uint16_t vhost_svq_available_slots(const VhostShadowVirtqueue *svq) > { > - return svq->vring.num - (svq->shadow_avail_idx - svq->shadow_used_idx); > + return svq->num_free; > } > > /** > @@ -263,6 +263,9 @@ int vhost_svq_add(VhostShadowVirtqueue *svq, const struct iovec *out_sg, > return -EINVAL; > } > > + /* Update the size of SVQ vring free descriptors */ > + svq->num_free -= ndescs; > + > svq->desc_state[qemu_head].elem = elem; > svq->desc_state[qemu_head].ndescs = ndescs; > vhost_svq_kick(svq); > @@ -450,6 +453,9 @@ static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq, > svq->desc_next[last_used_chain] = svq->free_head; > svq->free_head = used_elem.id; > > + /* Update the size of SVQ vring free descriptors */ No need for this comment. Apart from that, Acked-by: Eugenio Pérez <eperezma@redhat.com> > + svq->num_free += num; > + > *len = used_elem.len; > return g_steal_pointer(&svq->desc_state[used_elem.id].elem); > } > @@ -659,6 +665,7 @@ void vhost_svq_start(VhostShadowVirtqueue *svq, VirtIODevice *vdev, > svq->iova_tree = iova_tree; > > svq->vring.num = virtio_queue_get_num(vdev, virtio_get_queue_index(vq)); > + svq->num_free = svq->vring.num; > driver_size = vhost_svq_driver_area_size(svq); > device_size = vhost_svq_device_area_size(svq); > svq->vring.desc = qemu_memalign(qemu_real_host_page_size(), driver_size); > diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h > index 926a4897b1..6efe051a70 100644 > --- a/hw/virtio/vhost-shadow-virtqueue.h > +++ b/hw/virtio/vhost-shadow-virtqueue.h > @@ -107,6 +107,9 @@ typedef struct VhostShadowVirtqueue { > > /* Next head to consume from the device */ > uint16_t last_used_idx; > + > + /* Size of SVQ vring free descriptors */ > + uint16_t num_free; > } VhostShadowVirtqueue; > > bool vhost_svq_valid_features(uint64_t features, Error **errp); > -- > 2.25.1 >
Hi Eugenio, Thanks for reviewing. On 2023/5/9 1:26, Eugenio Perez Martin wrote: > On Sat, May 6, 2023 at 5:01 PM Hawkins Jiawei <yin31149@gmail.com> wrote: >> >> QEMU invokes vhost_svq_add() when adding a guest's element into SVQ. >> In vhost_svq_add(), it uses vhost_svq_available_slots() to check >> whether QEMU can add the element into the SVQ. If there is >> enough space, then QEMU combines some out descriptors and >> some in descriptors into one descriptor chain, and add it into >> svq->vring.desc by vhost_svq_vring_write_descs(). >> >> Yet the problem is that, `svq->shadow_avail_idx - svq->shadow_used_idx` >> in vhost_svq_available_slots() return the number of occupied elements, >> or the number of descriptor chains, instead of the number of occupied >> descriptors, which may cause wrapping in SVQ descriptor ring. >> >> Here is an example. In vhost_handle_guest_kick(), QEMU forwards >> as many available buffers to device by virtqueue_pop() and >> vhost_svq_add_element(). virtqueue_pop() return a guest's element, >> and use vhost_svq_add_elemnt(), a wrapper to vhost_svq_add(), to >> add this element into SVQ. If QEMU invokes virtqueue_pop() and >> vhost_svq_add_element() `svq->vring.num` times, vhost_svq_available_slots() >> thinks QEMU just ran out of slots and everything should work fine. >> But in fact, virtqueue_pop() return `svq-vring.num` elements or >> descriptor chains, more than `svq->vring.num` descriptors, due to >> guest memory fragmentation, and this cause wrapping in SVQ descriptor ring. >> > > The bug is valid even before marking the descriptors used. If the > guest memory is fragmented, SVQ must add chains so it can try to add > more descriptors than possible. I will add this in the commit message in v2 patch. > >> Therefore, this patch adds `num_free` field in VhostShadowVirtqueue >> structure, updates this field in vhost_svq_add() and >> vhost_svq_get_buf(), to record the number of free descriptors. >> Then we can avoid wrap in SVQ descriptor ring by refactoring >> vhost_svq_available_slots(). >> >> Fixes: 100890f7ca ("vhost: Shadow virtqueue buffers forwarding") >> Signed-off-by: Hawkins Jiawei <yin31149@gmail.com> >> --- >> hw/virtio/vhost-shadow-virtqueue.c | 9 ++++++++- >> hw/virtio/vhost-shadow-virtqueue.h | 3 +++ >> 2 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c >> index 8361e70d1b..e1c6952b10 100644 >> --- a/hw/virtio/vhost-shadow-virtqueue.c >> +++ b/hw/virtio/vhost-shadow-virtqueue.c >> @@ -68,7 +68,7 @@ bool vhost_svq_valid_features(uint64_t features, Error **errp) >> */ >> static uint16_t vhost_svq_available_slots(const VhostShadowVirtqueue *svq) >> { >> - return svq->vring.num - (svq->shadow_avail_idx - svq->shadow_used_idx); >> + return svq->num_free; >> } >> >> /** >> @@ -263,6 +263,9 @@ int vhost_svq_add(VhostShadowVirtqueue *svq, const struct iovec *out_sg, >> return -EINVAL; >> } >> >> + /* Update the size of SVQ vring free descriptors */ >> + svq->num_free -= ndescs; >> + >> svq->desc_state[qemu_head].elem = elem; >> svq->desc_state[qemu_head].ndescs = ndescs; >> vhost_svq_kick(svq); >> @@ -450,6 +453,9 @@ static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq, >> svq->desc_next[last_used_chain] = svq->free_head; >> svq->free_head = used_elem.id; >> >> + /* Update the size of SVQ vring free descriptors */ > > No need for this comment. > > Apart from that, > > Acked-by: Eugenio Pérez <eperezma@redhat.com> > Thanks for your suggestion. I will remove the comment in v2 patch, with this tag on. >> + svq->num_free += num; >> + >> *len = used_elem.len; >> return g_steal_pointer(&svq->desc_state[used_elem.id].elem); >> } >> @@ -659,6 +665,7 @@ void vhost_svq_start(VhostShadowVirtqueue *svq, VirtIODevice *vdev, >> svq->iova_tree = iova_tree; >> >> svq->vring.num = virtio_queue_get_num(vdev, virtio_get_queue_index(vq)); >> + svq->num_free = svq->vring.num; >> driver_size = vhost_svq_driver_area_size(svq); >> device_size = vhost_svq_device_area_size(svq); >> svq->vring.desc = qemu_memalign(qemu_real_host_page_size(), driver_size); >> diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h >> index 926a4897b1..6efe051a70 100644 >> --- a/hw/virtio/vhost-shadow-virtqueue.h >> +++ b/hw/virtio/vhost-shadow-virtqueue.h >> @@ -107,6 +107,9 @@ typedef struct VhostShadowVirtqueue { >> >> /* Next head to consume from the device */ >> uint16_t last_used_idx; >> + >> + /* Size of SVQ vring free descriptors */ >> + uint16_t num_free; >> } VhostShadowVirtqueue; >> >> bool vhost_svq_valid_features(uint64_t features, Error **errp); >> -- >> 2.25.1 >> >
QE applied this patch to do sanity testing on vhost-net, there is no any regression problem. Tested-by: Lei Yang <leiyang@redhat.com> On Tue, May 9, 2023 at 1:28 AM Eugenio Perez Martin <eperezma@redhat.com> wrote: > > On Sat, May 6, 2023 at 5:01 PM Hawkins Jiawei <yin31149@gmail.com> wrote: > > > > QEMU invokes vhost_svq_add() when adding a guest's element into SVQ. > > In vhost_svq_add(), it uses vhost_svq_available_slots() to check > > whether QEMU can add the element into the SVQ. If there is > > enough space, then QEMU combines some out descriptors and > > some in descriptors into one descriptor chain, and add it into > > svq->vring.desc by vhost_svq_vring_write_descs(). > > > > Yet the problem is that, `svq->shadow_avail_idx - svq->shadow_used_idx` > > in vhost_svq_available_slots() return the number of occupied elements, > > or the number of descriptor chains, instead of the number of occupied > > descriptors, which may cause wrapping in SVQ descriptor ring. > > > > Here is an example. In vhost_handle_guest_kick(), QEMU forwards > > as many available buffers to device by virtqueue_pop() and > > vhost_svq_add_element(). virtqueue_pop() return a guest's element, > > and use vhost_svq_add_elemnt(), a wrapper to vhost_svq_add(), to > > add this element into SVQ. If QEMU invokes virtqueue_pop() and > > vhost_svq_add_element() `svq->vring.num` times, vhost_svq_available_slots() > > thinks QEMU just ran out of slots and everything should work fine. > > But in fact, virtqueue_pop() return `svq-vring.num` elements or > > descriptor chains, more than `svq->vring.num` descriptors, due to > > guest memory fragmentation, and this cause wrapping in SVQ descriptor ring. > > > > The bug is valid even before marking the descriptors used. If the > guest memory is fragmented, SVQ must add chains so it can try to add > more descriptors than possible. > > > Therefore, this patch adds `num_free` field in VhostShadowVirtqueue > > structure, updates this field in vhost_svq_add() and > > vhost_svq_get_buf(), to record the number of free descriptors. > > Then we can avoid wrap in SVQ descriptor ring by refactoring > > vhost_svq_available_slots(). > > > > Fixes: 100890f7ca ("vhost: Shadow virtqueue buffers forwarding") > > Signed-off-by: Hawkins Jiawei <yin31149@gmail.com> > > --- > > hw/virtio/vhost-shadow-virtqueue.c | 9 ++++++++- > > hw/virtio/vhost-shadow-virtqueue.h | 3 +++ > > 2 files changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c > > index 8361e70d1b..e1c6952b10 100644 > > --- a/hw/virtio/vhost-shadow-virtqueue.c > > +++ b/hw/virtio/vhost-shadow-virtqueue.c > > @@ -68,7 +68,7 @@ bool vhost_svq_valid_features(uint64_t features, Error **errp) > > */ > > static uint16_t vhost_svq_available_slots(const VhostShadowVirtqueue *svq) > > { > > - return svq->vring.num - (svq->shadow_avail_idx - svq->shadow_used_idx); > > + return svq->num_free; > > } > > > > /** > > @@ -263,6 +263,9 @@ int vhost_svq_add(VhostShadowVirtqueue *svq, const struct iovec *out_sg, > > return -EINVAL; > > } > > > > + /* Update the size of SVQ vring free descriptors */ > > + svq->num_free -= ndescs; > > + > > svq->desc_state[qemu_head].elem = elem; > > svq->desc_state[qemu_head].ndescs = ndescs; > > vhost_svq_kick(svq); > > @@ -450,6 +453,9 @@ static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq, > > svq->desc_next[last_used_chain] = svq->free_head; > > svq->free_head = used_elem.id; > > > > + /* Update the size of SVQ vring free descriptors */ > > No need for this comment. > > Apart from that, > > Acked-by: Eugenio Pérez <eperezma@redhat.com> > > > + svq->num_free += num; > > + > > *len = used_elem.len; > > return g_steal_pointer(&svq->desc_state[used_elem.id].elem); > > } > > @@ -659,6 +665,7 @@ void vhost_svq_start(VhostShadowVirtqueue *svq, VirtIODevice *vdev, > > svq->iova_tree = iova_tree; > > > > svq->vring.num = virtio_queue_get_num(vdev, virtio_get_queue_index(vq)); > > + svq->num_free = svq->vring.num; > > driver_size = vhost_svq_driver_area_size(svq); > > device_size = vhost_svq_device_area_size(svq); > > svq->vring.desc = qemu_memalign(qemu_real_host_page_size(), driver_size); > > diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h > > index 926a4897b1..6efe051a70 100644 > > --- a/hw/virtio/vhost-shadow-virtqueue.h > > +++ b/hw/virtio/vhost-shadow-virtqueue.h > > @@ -107,6 +107,9 @@ typedef struct VhostShadowVirtqueue { > > > > /* Next head to consume from the device */ > > uint16_t last_used_idx; > > + > > + /* Size of SVQ vring free descriptors */ > > + uint16_t num_free; > > } VhostShadowVirtqueue; > > > > bool vhost_svq_valid_features(uint64_t features, Error **errp); > > -- > > 2.25.1 > > > >
QE applied this patch to do sanity testing on vhost-vdpa, there is no any regression problem. Tested-by: Lei Yang <leiyang@redhat.com> On Wed, May 10, 2023 at 9:32 AM Lei Yang <leiyang@redhat.com> wrote: > QE applied this patch to do sanity testing on vhost-net, there is no > any regression problem. > > Tested-by: Lei Yang <leiyang@redhat.com> > > > > On Tue, May 9, 2023 at 1:28 AM Eugenio Perez Martin <eperezma@redhat.com> > wrote: > > > > On Sat, May 6, 2023 at 5:01 PM Hawkins Jiawei <yin31149@gmail.com> > wrote: > > > > > > QEMU invokes vhost_svq_add() when adding a guest's element into SVQ. > > > In vhost_svq_add(), it uses vhost_svq_available_slots() to check > > > whether QEMU can add the element into the SVQ. If there is > > > enough space, then QEMU combines some out descriptors and > > > some in descriptors into one descriptor chain, and add it into > > > svq->vring.desc by vhost_svq_vring_write_descs(). > > > > > > Yet the problem is that, `svq->shadow_avail_idx - svq->shadow_used_idx` > > > in vhost_svq_available_slots() return the number of occupied elements, > > > or the number of descriptor chains, instead of the number of occupied > > > descriptors, which may cause wrapping in SVQ descriptor ring. > > > > > > Here is an example. In vhost_handle_guest_kick(), QEMU forwards > > > as many available buffers to device by virtqueue_pop() and > > > vhost_svq_add_element(). virtqueue_pop() return a guest's element, > > > and use vhost_svq_add_elemnt(), a wrapper to vhost_svq_add(), to > > > add this element into SVQ. If QEMU invokes virtqueue_pop() and > > > vhost_svq_add_element() `svq->vring.num` times, > vhost_svq_available_slots() > > > thinks QEMU just ran out of slots and everything should work fine. > > > But in fact, virtqueue_pop() return `svq-vring.num` elements or > > > descriptor chains, more than `svq->vring.num` descriptors, due to > > > guest memory fragmentation, and this cause wrapping in SVQ descriptor > ring. > > > > > > > The bug is valid even before marking the descriptors used. If the > > guest memory is fragmented, SVQ must add chains so it can try to add > > more descriptors than possible. > > > > > Therefore, this patch adds `num_free` field in VhostShadowVirtqueue > > > structure, updates this field in vhost_svq_add() and > > > vhost_svq_get_buf(), to record the number of free descriptors. > > > Then we can avoid wrap in SVQ descriptor ring by refactoring > > > vhost_svq_available_slots(). > > > > > > Fixes: 100890f7ca ("vhost: Shadow virtqueue buffers forwarding") > > > Signed-off-by: Hawkins Jiawei <yin31149@gmail.com> > > > --- > > > hw/virtio/vhost-shadow-virtqueue.c | 9 ++++++++- > > > hw/virtio/vhost-shadow-virtqueue.h | 3 +++ > > > 2 files changed, 11 insertions(+), 1 deletion(-) > > > > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c > b/hw/virtio/vhost-shadow-virtqueue.c > > > index 8361e70d1b..e1c6952b10 100644 > > > --- a/hw/virtio/vhost-shadow-virtqueue.c > > > +++ b/hw/virtio/vhost-shadow-virtqueue.c > > > @@ -68,7 +68,7 @@ bool vhost_svq_valid_features(uint64_t features, > Error **errp) > > > */ > > > static uint16_t vhost_svq_available_slots(const VhostShadowVirtqueue > *svq) > > > { > > > - return svq->vring.num - (svq->shadow_avail_idx - > svq->shadow_used_idx); > > > + return svq->num_free; > > > } > > > > > > /** > > > @@ -263,6 +263,9 @@ int vhost_svq_add(VhostShadowVirtqueue *svq, const > struct iovec *out_sg, > > > return -EINVAL; > > > } > > > > > > + /* Update the size of SVQ vring free descriptors */ > > > + svq->num_free -= ndescs; > > > + > > > svq->desc_state[qemu_head].elem = elem; > > > svq->desc_state[qemu_head].ndescs = ndescs; > > > vhost_svq_kick(svq); > > > @@ -450,6 +453,9 @@ static VirtQueueElement > *vhost_svq_get_buf(VhostShadowVirtqueue *svq, > > > svq->desc_next[last_used_chain] = svq->free_head; > > > svq->free_head = used_elem.id; > > > > > > + /* Update the size of SVQ vring free descriptors */ > > > > No need for this comment. > > > > Apart from that, > > > > Acked-by: Eugenio Pérez <eperezma@redhat.com> > > > > > + svq->num_free += num; > > > + > > > *len = used_elem.len; > > > return g_steal_pointer(&svq->desc_state[used_elem.id].elem); > > > } > > > @@ -659,6 +665,7 @@ void vhost_svq_start(VhostShadowVirtqueue *svq, > VirtIODevice *vdev, > > > svq->iova_tree = iova_tree; > > > > > > svq->vring.num = virtio_queue_get_num(vdev, > virtio_get_queue_index(vq)); > > > + svq->num_free = svq->vring.num; > > > driver_size = vhost_svq_driver_area_size(svq); > > > device_size = vhost_svq_device_area_size(svq); > > > svq->vring.desc = qemu_memalign(qemu_real_host_page_size(), > driver_size); > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.h > b/hw/virtio/vhost-shadow-virtqueue.h > > > index 926a4897b1..6efe051a70 100644 > > > --- a/hw/virtio/vhost-shadow-virtqueue.h > > > +++ b/hw/virtio/vhost-shadow-virtqueue.h > > > @@ -107,6 +107,9 @@ typedef struct VhostShadowVirtqueue { > > > > > > /* Next head to consume from the device */ > > > uint16_t last_used_idx; > > > + > > > + /* Size of SVQ vring free descriptors */ > > > + uint16_t num_free; > > > } VhostShadowVirtqueue; > > > > > > bool vhost_svq_valid_features(uint64_t features, Error **errp); > > > -- > > > 2.25.1 > > > > > > > >
diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c index 8361e70d1b..e1c6952b10 100644 --- a/hw/virtio/vhost-shadow-virtqueue.c +++ b/hw/virtio/vhost-shadow-virtqueue.c @@ -68,7 +68,7 @@ bool vhost_svq_valid_features(uint64_t features, Error **errp) */ static uint16_t vhost_svq_available_slots(const VhostShadowVirtqueue *svq) { - return svq->vring.num - (svq->shadow_avail_idx - svq->shadow_used_idx); + return svq->num_free; } /** @@ -263,6 +263,9 @@ int vhost_svq_add(VhostShadowVirtqueue *svq, const struct iovec *out_sg, return -EINVAL; } + /* Update the size of SVQ vring free descriptors */ + svq->num_free -= ndescs; + svq->desc_state[qemu_head].elem = elem; svq->desc_state[qemu_head].ndescs = ndescs; vhost_svq_kick(svq); @@ -450,6 +453,9 @@ static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq, svq->desc_next[last_used_chain] = svq->free_head; svq->free_head = used_elem.id; + /* Update the size of SVQ vring free descriptors */ + svq->num_free += num; + *len = used_elem.len; return g_steal_pointer(&svq->desc_state[used_elem.id].elem); } @@ -659,6 +665,7 @@ void vhost_svq_start(VhostShadowVirtqueue *svq, VirtIODevice *vdev, svq->iova_tree = iova_tree; svq->vring.num = virtio_queue_get_num(vdev, virtio_get_queue_index(vq)); + svq->num_free = svq->vring.num; driver_size = vhost_svq_driver_area_size(svq); device_size = vhost_svq_device_area_size(svq); svq->vring.desc = qemu_memalign(qemu_real_host_page_size(), driver_size); diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h index 926a4897b1..6efe051a70 100644 --- a/hw/virtio/vhost-shadow-virtqueue.h +++ b/hw/virtio/vhost-shadow-virtqueue.h @@ -107,6 +107,9 @@ typedef struct VhostShadowVirtqueue { /* Next head to consume from the device */ uint16_t last_used_idx; + + /* Size of SVQ vring free descriptors */ + uint16_t num_free; } VhostShadowVirtqueue; bool vhost_svq_valid_features(uint64_t features, Error **errp);
QEMU invokes vhost_svq_add() when adding a guest's element into SVQ. In vhost_svq_add(), it uses vhost_svq_available_slots() to check whether QEMU can add the element into the SVQ. If there is enough space, then QEMU combines some out descriptors and some in descriptors into one descriptor chain, and add it into svq->vring.desc by vhost_svq_vring_write_descs(). Yet the problem is that, `svq->shadow_avail_idx - svq->shadow_used_idx` in vhost_svq_available_slots() return the number of occupied elements, or the number of descriptor chains, instead of the number of occupied descriptors, which may cause wrapping in SVQ descriptor ring. Here is an example. In vhost_handle_guest_kick(), QEMU forwards as many available buffers to device by virtqueue_pop() and vhost_svq_add_element(). virtqueue_pop() return a guest's element, and use vhost_svq_add_elemnt(), a wrapper to vhost_svq_add(), to add this element into SVQ. If QEMU invokes virtqueue_pop() and vhost_svq_add_element() `svq->vring.num` times, vhost_svq_available_slots() thinks QEMU just ran out of slots and everything should work fine. But in fact, virtqueue_pop() return `svq-vring.num` elements or descriptor chains, more than `svq->vring.num` descriptors, due to guest memory fragmentation, and this cause wrapping in SVQ descriptor ring. Therefore, this patch adds `num_free` field in VhostShadowVirtqueue structure, updates this field in vhost_svq_add() and vhost_svq_get_buf(), to record the number of free descriptors. Then we can avoid wrap in SVQ descriptor ring by refactoring vhost_svq_available_slots(). Fixes: 100890f7ca ("vhost: Shadow virtqueue buffers forwarding") Signed-off-by: Hawkins Jiawei <yin31149@gmail.com> --- hw/virtio/vhost-shadow-virtqueue.c | 9 ++++++++- hw/virtio/vhost-shadow-virtqueue.h | 3 +++ 2 files changed, 11 insertions(+), 1 deletion(-)