Re: [PATCH] vhost: Don't special case vq->used_phys in vhost_get_log_size()

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Oct 23, 2020 at 01:10:06PM +0200, Greg Kurz wrote:
> On Sat, 10 Oct 2020 11:30:28 +0800
> Jason Wang <jasowang@redhat.com> wrote:
> 
> > 
> > On 2020/10/8 上午12:30, Greg Kurz wrote:
> > > The first loop in vhost_get_log_size() computes the size of the dirty log
> > > bitmap so that it allows to track changes in the entire guest memory, in
> > > terms of GPA.
> > >
> > > When not using a vIOMMU, the address of the vring's used structure,
> > > vq->used_phys, is a GPA. It is thus already covered by the first loop.
> > >
> > > When using a vIOMMU, vq->used_phys is a GIOVA that will be translated
> > > to an HVA when the vhost backend needs to update the used structure. It
> > > will log the corresponding GPAs into the bitmap but it certainly won't
> > > log the GIOVA.
> > >
> > > So in any case, vq->used_phys shouldn't be explicitly used to size the
> > > bitmap. Drop the second loop.
> > >
> > > This fixes a crash of the source when migrating a guest using in-kernel
> > > vhost-net and iommu_platform=on on POWER, because DMA regions are put
> > > over 0x800000000000000ULL. The resulting insanely huge log size causes
> > > g_malloc0() to abort.
> > >
> > > BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1879349
> > > Signed-off-by: Greg Kurz <groug@kaod.org>
> > > ---
> > >
> > > This supersedes "vhost: Ignore vrings in dirty log when using a vIOMMU"
> > >
> > > http://patchwork.ozlabs.org/project/qemu-devel/patch/160105498386.68108.2145229309875282336.stgit@bahia.lan/
> > > ---
> > >   hw/virtio/vhost.c |   10 ----------
> > >   1 file changed, 10 deletions(-)
> > >
> > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > > index 011951625442..c02b658b597f 100644
> > > --- a/hw/virtio/vhost.c
> > > +++ b/hw/virtio/vhost.c
> > > @@ -172,16 +172,6 @@ static uint64_t vhost_get_log_size(struct vhost_dev *dev)
> > >                                          reg->memory_size);
> > >           log_size = MAX(log_size, last / VHOST_LOG_CHUNK + 1);
> > >       }
> > > -    for (i = 0; i < dev->nvqs; ++i) {
> > > -        struct vhost_virtqueue *vq = dev->vqs + i;
> > > -
> > > -        if (!vq->used_phys && !vq->used_size) {
> > > -            continue;
> > > -        }
> > > -
> > > -        uint64_t last = vq->used_phys + vq->used_size - 1;
> > > -        log_size = MAX(log_size, last / VHOST_LOG_CHUNK + 1);
> > > -    }
> > >       return log_size;
> > >   }
> > >   
> > 
> > 
> > Acked-by: Jason Wang <jasowang@redhat.com>
> > 
> 
> Ping ?

tagged, thanks!

-- 
MST