Re: [PATCH v3 12/15] util: vfio-helpers: Implement ram_block_resized()

[Peter Xu]

On Fri, Feb 28, 2020 at 03:19:45PM -0500, David Hildenbrand wrote:
> 
> 
> > Am 28.02.2020 um 20:55 schrieb Peter Xu <address@hidden>:
> > 
> > On Thu, Feb 27, 2020 at 11:12:02AM +0100, David Hildenbrand wrote:
> >> +static void qemu_vfio_dma_map_resize(QEMUVFIOState *s, void *host,
> >> +                                     size_t old_size, size_t new_size)
> >> +{
> >> +    IOVAMapping *m;
> >> +    int index = 0;
> >> +
> >> +    qemu_mutex_lock(&s->lock);
> >> +    m = qemu_vfio_find_mapping(s, host, &index);
> >> +    if (!m) {
> >> +        return;
> >> +    }
> >> +    assert(m->size == old_size);
> >> +
> >> +    /* Note: Not atomic - we need a new ioctl for that. */
> >> +    qemu_vfio_undo_mapping(s, m->iova, m->size);
> >> +    qemu_vfio_do_mapping(s, host, m->iova, new_size);
> > 
> > Another way to ask my previous question 1 (in the other reply): Can we
> > simply map/unmap the extra, while keep the shared untouched?
> 
> As far as I understand the kernel implementation, unfortunately no. You might 
> be able to grow (by mapping the delta), but shrinking is not possible AFAIR. 
> And I *think* with many resizes, there could be an issue if I remember 
> correctly.

Ah right (and I think the same thing will happen to vfio-pci during
memory_region_set_size()).  Then please double confirm that virtio-mem
is disabled for both vfio-helper users and vfio-pci.  Also, would you
mind comment above the unmap+map with more information?  E.g.:

  For now, we must unmap the whole mapped range first and remap with
  the new size.  The reason is that VFIO_IOMMU_UNMAP_DMA might fail
  with partial unmap of previous mappings, so even we can add extra
  new mappings to extend the old range, however we still won't able to
  always success on shrinking memories.  The side effect is that it's
  never safe to do resizing during VM execution.

Or something better.  With an enriched comment, I think it's fine:

Reviewed-by: Peter Xu <address@hidden>

Thanks,

-- 
Peter Xu