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

[Alex Williamson]

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,

Alex