[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] virtio: fix descriptor counting in virtqueue_po
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH] virtio: fix descriptor counting in virtqueue_pop |
Date: |
Fri, 10 Nov 2017 22:34:25 +0200 |
On Fri, Nov 10, 2017 at 03:41:37PM +0000, Stefan Hajnoczi wrote:
> On Thu, Oct 05, 2017 at 08:03:35PM +0200, Alexandre DERUMIER wrote:
> > Hi,
> >
> > has somebody reviewed this patch ?
> >
> > I'm also able de reproduce the vm crash like the proxmox user.
> > This patch is fixing it for me too.
>
> This patch should go through Michael Tsirkin's tree. I have pinged him
> separately in case this email thread got buried in his inbox.
>
> Stefan
Isn't this upstream?
I see it as commit 37ef70be6af7e9f2a6f852c68f74bd98dac2664b there.
> >
> > Regards,
> >
> > Alexandre
> >
> >
> > ----- Mail original -----
> > De: "Wolfgang Bumiller" <address@hidden>
> > À: "qemu-devel" <address@hidden>
> > Cc: "pbonzini" <address@hidden>, "Michael S. Tsirkin" <address@hidden>
> > Envoyé: Mercredi 20 Septembre 2017 08:09:33
> > Objet: [Qemu-devel] [PATCH] virtio: fix descriptor counting in virtqueue_pop
> >
> > While changing the s/g list allocation, commit 3b3b0628
> > also changed the descriptor counting to count iovec entries
> > as split by cpu_physical_memory_map(). Previously only the
> > actual descriptor entries were counted and the split into
> > the iovec happened afterwards in virtqueue_map().
> > Count the entries again instead to avoid erroneous
> > "Looped descriptor" errors.
> >
> > Reported-by: Hans Middelhoek <address@hidden>
> > Link: https://forum.proxmox.com/threads/vm-crash-with-memory-hotplug.35904/
> > Fixes: 3b3b0628217e ("virtio: slim down allocation of VirtQueueElements")
> > Signed-off-by: Wolfgang Bumiller <address@hidden>
> > ---
> > hw/virtio/virtio.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index 890b4d7eb7..33bb770177 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -834,7 +834,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
> > int64_t len;
> > VirtIODevice *vdev = vq->vdev;
> > VirtQueueElement *elem = NULL;
> > - unsigned out_num, in_num;
> > + unsigned out_num, in_num, elem_entries;
> > hwaddr addr[VIRTQUEUE_MAX_SIZE];
> > struct iovec iov[VIRTQUEUE_MAX_SIZE];
> > VRingDesc desc;
> > @@ -852,7 +852,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
> > smp_rmb();
> >
> > /* When we start there are none of either input nor output. */
> > - out_num = in_num = 0;
> > + out_num = in_num = elem_entries = 0;
> >
> > max = vq->vring.num;
> >
> > @@ -922,7 +922,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
> > }
> >
> > /* If we've got too many, that implies a descriptor loop. */
> > - if ((in_num + out_num) > max) {
> > + if (++elem_entries > max) {
> > virtio_error(vdev, "Looped descriptor");
> > goto err_undo_map;
> > }
> > --
> > 2.11.0
> >
> >