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

[David Hildenbrand]

> Am 28.02.2020 um 21:49 schrieb Peter Xu <address@hidden>:
> 
> 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

Yes, will double check, should have taken care of both.

> mind comment above the unmap+map with more information?  E.g.:
> 

Sure, will add - thanks!

>  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! Have a nice weekend!