Re: [PATCH v4 0/2] Allow memory_region_register_iommu_notifier() to fail

[Paolo Bonzini]

On 24/09/19 23:31, Alex Williamson wrote:
> On Tue, 24 Sep 2019 10:25:15 +0200
> Eric Auger <address@hidden> wrote:
> 
>> This series allows the memory_region_register_iommu_notifier()
>> to fail. As of now, when a MAP notifier is attempted to be
>> registered along with SMMUv3 or AMD IOMMU, we exit in the IOMMU
>> MR notify_flag_changed() callback.
>>
>> In case of VFIO assigned device hotplug, this could be handled
>> more nicely directly within the VFIO code, simply rejecting
>> the hotplug without exiting. This is what the series achieves
>> by handling the memory_region_register_iommu_notifier() returned
>> value and Error object.
>>
>> To propagate errors collected during vfio_listener_region_add()
>> we now store the error handle inside the VFIO container instead
>> of a returned value.
>>
>> The message now is:
>> (QEMU) device_add id=hot0 driver=vfio-pci host=0000:89:00.0 bus=pcie.1
>> {"error": {"class": "GenericError", "desc": "vfio 0000:89:00.0: failed
>> to setup container for group 2: memory listener initialization failed:
>> Region smmuv3-iommu-memory-region-0-6: device 01.00.0 requires iommu MAP
>> notifier which is not currently supported"}}
>>
>> Best Regards
>>
>> Eric
>>
>> This series can be found at:
>> https://github.com/eauger/qemu/tree/v4.1.0_register_iommu_notifier_fail_v4
>>
>> History:
>>
>> v3 -> v4:
>> - added Peter's R-b on 2d patch
>> - 1st patch: restore hw_error, remove useless ret assignment, improve
>>   DMA host window error message, remove local mr variable
>>
>> v2 -> v3:
>> - also pass an Error handle (suggested by Peter)
>>
>> v1 -> v2:
>> - Intel IOMMU now handles the problem differently with machine init done
>>   notifier and machine hotplug allowed hook.
>> - use assert(!ret)
>> - message rewording in SMMUv3
>>
>> Follow-up of "VFIO/SMMUv3: Fail on VFIO/HW nested paging detection"
>> https://patchew.org/QEMU/address@hidden/
>>
>>
>> Eric Auger (2):
>>   vfio: Turn the container error into an Error handle
>>   memory: allow memory_region_register_iommu_notifier() to fail
>>
>>  exec.c                        | 10 +++++--
>>  hw/arm/smmuv3.c               | 18 ++++++------
>>  hw/i386/amd_iommu.c           | 17 +++++++-----
>>  hw/i386/intel_iommu.c         |  8 ++++--
>>  hw/ppc/spapr_iommu.c          |  8 ++++--
>>  hw/vfio/common.c              | 52 +++++++++++++++++++++++------------
>>  hw/vfio/spapr.c               |  4 ++-
>>  hw/virtio/vhost.c             |  9 ++++--
>>  include/exec/memory.h         | 21 ++++++++++----
>>  include/hw/vfio/vfio-common.h |  2 +-
>>  memory.c                      | 31 +++++++++++++--------
>>  11 files changed, 120 insertions(+), 60 deletions(-)
> 
> For series,
> 
> Acked-by: Alex Williamson <address@hidden>
> 
> Paolo, would this go in through you given the memory API changes and
> greater girth in patch 2/2?  Thanks,

Fine, queued.

Paolo