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

Eric Auger posted 2 patches 6 years, 4 months ago
Test checkpatch passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test docker-clang@ubuntu passed
Test asan passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190924082517.13724-1-eric.auger@redhat.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Eric Auger <eric.auger@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Richard Henderson <rth@twiddle.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Alex Williamson <alex.williamson@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>
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(-)
[PATCH v4 0/2] Allow memory_region_register_iommu_notifier() to fail
Posted by Eric Auger 6 years, 4 months ago
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/20190829090141.21821-1-eric.auger@redhat.com/


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(-)

-- 
2.20.1


Re: [PATCH v4 0/2] Allow memory_region_register_iommu_notifier() to fail
Posted by David Gibson 6 years, 4 months ago
On Tue, Sep 24, 2019 at 10:25:15AM +0200, Eric Auger 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

For the series

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

and ppc parts

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> 
> 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/20190829090141.21821-1-eric.auger@redhat.com/
> 
> 
> 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(-)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [PATCH v4 0/2] Allow memory_region_register_iommu_notifier() to fail
Posted by Alex Williamson 6 years, 4 months ago
On Tue, 24 Sep 2019 10:25:15 +0200
Eric Auger <eric.auger@redhat.com> 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/20190829090141.21821-1-eric.auger@redhat.com/
> 
> 
> 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 <alex.williamson@redhat.com>

Paolo, would this go in through you given the memory API changes and
greater girth in patch 2/2?  Thanks,

Alex

Re: [PATCH v4 0/2] Allow memory_region_register_iommu_notifier() to fail
Posted by Paolo Bonzini 6 years, 4 months ago
On 24/09/19 23:31, Alex Williamson wrote:
> On Tue, 24 Sep 2019 10:25:15 +0200
> Eric Auger <eric.auger@redhat.com> 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/20190829090141.21821-1-eric.auger@redhat.com/
>>
>>
>> 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 <alex.williamson@redhat.com>
> 
> Paolo, would this go in through you given the memory API changes and
> greater girth in patch 2/2?  Thanks,

Fine, queued.

Paolo