[PATCH 0/2] Additional vIOMMU fixes related to UNMAP notifiers

[PATCH 0/2] Additional vIOMMU fixes related to UNMAP notifiers

[Eric Auger]

958ec334bca3 ("vhost: Unbreak SMMU and virtio-iommu on dev-iotlb
support") fixed part of the UNMAP related regressions introduced by
b68ba1ca5767 ("memory: Add IOMMU_NOTIFIER_DEVIOTLB_UNMAP
IOMMUTLBNotificationType").

However the case of the spapr_iommu was not addressed. It should be
identical to the others. Also 958ec334bca3 introduced a regresion
on the VIRTIO-IOMMU/VFIO integration.

spapr_iommu is not tested.

Best Regards

Eric

Eric Auger (2):
  vfio: Do not register any IOMMU_NOTIFIER_DEVIOTLB_UNMAP notifier
  spapr_iommu: Fix vhost integration regression

 hw/ppc/spapr_iommu.c | 5 +++++
 hw/vfio/common.c     | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

-- 
2.26.2

[PATCH 1/2] vfio: Do not register any IOMMU_NOTIFIER_DEVIOTLB_UNMAP notifier

[Eric Auger]

In an attempt to fix smmu/virtio-iommu - vhost regression, commit
958ec334bca3 ("vhost: Unbreak SMMU and virtio-iommu on dev-iotlb support")
broke virtio-iommu integration. This is due to the fact VFIO registers
IOMMU_NOTIFIER_ALL notifiers, which includes IOMMU_NOTIFIER_DEVIOTLB_UNMAP
and this latter now is rejected by the virtio-iommu. As a consequence,
the registration fails. VHOST behaves like a device with an ATC cache. The
VFIO device does not support this scheme yet.

Let's register only legacy MAP and UNMAP notifiers.

Fixes: 958ec334bca3 ("vhost: Unbreak SMMU and virtio-iommu on dev-iotlb support")
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/vfio/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 6ff1daa763..a50b10c801 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -787,7 +787,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
         iommu_idx = memory_region_iommu_attrs_to_index(iommu_mr,
                                                        MEMTXATTRS_UNSPECIFIED);
         iommu_notifier_init(&giommu->n, vfio_iommu_map_notify,
-                            IOMMU_NOTIFIER_ALL,
+                            IOMMU_NOTIFIER_IOTLB_EVENTS,
                             section->offset_within_region,
                             int128_get64(llend),
                             iommu_idx);
-- 
2.26.2

Re: [PATCH 1/2] vfio: Do not register any IOMMU_NOTIFIER_DEVIOTLB_UNMAP notifier

[Jason Wang]

On 2021/2/10 上午5:32, Eric Auger wrote:
> In an attempt to fix smmu/virtio-iommu - vhost regression, commit
> 958ec334bca3 ("vhost: Unbreak SMMU and virtio-iommu on dev-iotlb support")
> broke virtio-iommu integration. This is due to the fact VFIO registers
> IOMMU_NOTIFIER_ALL notifiers, which includes IOMMU_NOTIFIER_DEVIOTLB_UNMAP
> and this latter now is rejected by the virtio-iommu. As a consequence,
> the registration fails. VHOST behaves like a device with an ATC cache. The
> VFIO device does not support this scheme yet.
>
> Let's register only legacy MAP and UNMAP notifiers.
>
> Fixes: 958ec334bca3 ("vhost: Unbreak SMMU and virtio-iommu on dev-iotlb support")
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>   hw/vfio/common.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 6ff1daa763..a50b10c801 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -787,7 +787,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
>           iommu_idx = memory_region_iommu_attrs_to_index(iommu_mr,
>                                                          MEMTXATTRS_UNSPECIFIED);
>           iommu_notifier_init(&giommu->n, vfio_iommu_map_notify,
> -                            IOMMU_NOTIFIER_ALL,
> +                            IOMMU_NOTIFIER_IOTLB_EVENTS,
>                               section->offset_within_region,
>                               int128_get64(llend),
>                               iommu_idx);


Acked-by: Jason Wang <jasowang@redhat.com>

Re: [PATCH 1/2] vfio: Do not register any IOMMU_NOTIFIER_DEVIOTLB_UNMAP notifier

[Alex Williamson]

On Tue,  9 Feb 2021 22:32:32 +0100
Eric Auger <eric.auger@redhat.com> wrote:

> In an attempt to fix smmu/virtio-iommu - vhost regression, commit
> 958ec334bca3 ("vhost: Unbreak SMMU and virtio-iommu on dev-iotlb support")
> broke virtio-iommu integration. This is due to the fact VFIO registers
> IOMMU_NOTIFIER_ALL notifiers, which includes IOMMU_NOTIFIER_DEVIOTLB_UNMAP
> and this latter now is rejected by the virtio-iommu. As a consequence,
> the registration fails. VHOST behaves like a device with an ATC cache. The
> VFIO device does not support this scheme yet.
> 
> Let's register only legacy MAP and UNMAP notifiers.
> 
> Fixes: 958ec334bca3 ("vhost: Unbreak SMMU and virtio-iommu on dev-iotlb support")
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  hw/vfio/common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 6ff1daa763..a50b10c801 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -787,7 +787,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
>          iommu_idx = memory_region_iommu_attrs_to_index(iommu_mr,
>                                                         MEMTXATTRS_UNSPECIFIED);
>          iommu_notifier_init(&giommu->n, vfio_iommu_map_notify,
> -                            IOMMU_NOTIFIER_ALL,
> +                            IOMMU_NOTIFIER_IOTLB_EVENTS,
>                              section->offset_within_region,
>                              int128_get64(llend),
>                              iommu_idx);

Acked-by: Alex Williamson <alex.williamson@redhat.com>

[PATCH 2/2] spapr_iommu: Fix vhost integration regression

[Eric Auger]

Previous work on dev-iotlb message broke spapr_iommu/vhost integration
as it did for SMMU and virtio-iommu. The spapr_iommu currently
only sends IOMMU_NOTIFIER_UNMAP notifications. Since commit
958ec334bca3 ("vhost: Unbreak SMMU and virtio-iommu on dev-iotlb support"),
VHOST first tries to register IOMMU_NOTIFIER_DEVIOTLB_UNMAP notifier
and if it fails, falls back to legacy IOMMU_NOTIFIER_UNMAP. So
spapr_iommu must fail on the IOMMU_NOTIFIER_DEVIOTLB_UNMAP
registration.

Reported-by: Peter Xu <peterx@redhat.com>
Fixes: b68ba1ca57677acf870d5ab10579e6105c1f5338
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/ppc/spapr_iommu.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index 30352df00e..24537ffcbd 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -212,6 +212,11 @@ static int spapr_tce_notify_flag_changed(IOMMUMemoryRegion *iommu,
 {
     struct SpaprTceTable *tbl = container_of(iommu, SpaprTceTable, iommu);
 
+    if (new & IOMMU_NOTIFIER_DEVIOTLB_UNMAP) {
+        error_setg(errp, "spart_tce does not support dev-iotlb yet");
+        return -EINVAL;
+    }
+
     if (old == IOMMU_NOTIFIER_NONE && new != IOMMU_NOTIFIER_NONE) {
         spapr_tce_set_need_vfio(tbl, true);
     } else if (old != IOMMU_NOTIFIER_NONE && new == IOMMU_NOTIFIER_NONE) {
-- 
2.26.2

Re: [PATCH 2/2] spapr_iommu: Fix vhost integration regression

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1756 bytes --]

On Tue, Feb 09, 2021 at 10:32:33PM +0100, Eric Auger wrote:
> Previous work on dev-iotlb message broke spapr_iommu/vhost integration
> as it did for SMMU and virtio-iommu. The spapr_iommu currently
> only sends IOMMU_NOTIFIER_UNMAP notifications. Since commit
> 958ec334bca3 ("vhost: Unbreak SMMU and virtio-iommu on dev-iotlb support"),
> VHOST first tries to register IOMMU_NOTIFIER_DEVIOTLB_UNMAP notifier
> and if it fails, falls back to legacy IOMMU_NOTIFIER_UNMAP. So
> spapr_iommu must fail on the IOMMU_NOTIFIER_DEVIOTLB_UNMAP
> registration.
> 
> Reported-by: Peter Xu <peterx@redhat.com>
> Fixes: b68ba1ca57677acf870d5ab10579e6105c1f5338
> Signed-off-by: Eric Auger <eric.auger@redhat.com>

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

> ---
>  hw/ppc/spapr_iommu.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index 30352df00e..24537ffcbd 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -212,6 +212,11 @@ static int spapr_tce_notify_flag_changed(IOMMUMemoryRegion *iommu,
>  {
>      struct SpaprTceTable *tbl = container_of(iommu, SpaprTceTable, iommu);
>  
> +    if (new & IOMMU_NOTIFIER_DEVIOTLB_UNMAP) {
> +        error_setg(errp, "spart_tce does not support dev-iotlb yet");
> +        return -EINVAL;
> +    }
> +
>      if (old == IOMMU_NOTIFIER_NONE && new != IOMMU_NOTIFIER_NONE) {
>          spapr_tce_set_need_vfio(tbl, true);
>      } else if (old != IOMMU_NOTIFIER_NONE && new == IOMMU_NOTIFIER_NONE) {

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] spapr_iommu: Fix vhost integration regression

[Jason Wang]

On 2021/2/10 上午5:32, Eric Auger wrote:
> Previous work on dev-iotlb message broke spapr_iommu/vhost integration
> as it did for SMMU and virtio-iommu. The spapr_iommu currently
> only sends IOMMU_NOTIFIER_UNMAP notifications. Since commit
> 958ec334bca3 ("vhost: Unbreak SMMU and virtio-iommu on dev-iotlb support"),
> VHOST first tries to register IOMMU_NOTIFIER_DEVIOTLB_UNMAP notifier
> and if it fails, falls back to legacy IOMMU_NOTIFIER_UNMAP. So
> spapr_iommu must fail on the IOMMU_NOTIFIER_DEVIOTLB_UNMAP
> registration.
>
> Reported-by: Peter Xu <peterx@redhat.com>
> Fixes: b68ba1ca57677acf870d5ab10579e6105c1f5338
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---


Acked-by: Jason Wang <jasowang@redhat.com>


>   hw/ppc/spapr_iommu.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index 30352df00e..24537ffcbd 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -212,6 +212,11 @@ static int spapr_tce_notify_flag_changed(IOMMUMemoryRegion *iommu,
>   {
>       struct SpaprTceTable *tbl = container_of(iommu, SpaprTceTable, iommu);
>   
> +    if (new & IOMMU_NOTIFIER_DEVIOTLB_UNMAP) {
> +        error_setg(errp, "spart_tce does not support dev-iotlb yet");
> +        return -EINVAL;
> +    }
> +
>       if (old == IOMMU_NOTIFIER_NONE && new != IOMMU_NOTIFIER_NONE) {
>           spapr_tce_set_need_vfio(tbl, true);
>       } else if (old != IOMMU_NOTIFIER_NONE && new == IOMMU_NOTIFIER_NONE) {

Re: [PATCH 2/2] spapr_iommu: Fix vhost integration regression

[Michael S. Tsirkin]

On Tue, Feb 09, 2021 at 10:32:33PM +0100, Eric Auger wrote:
> Previous work on dev-iotlb message broke spapr_iommu/vhost integration
> as it did for SMMU and virtio-iommu. The spapr_iommu currently
> only sends IOMMU_NOTIFIER_UNMAP notifications. Since commit
> 958ec334bca3 ("vhost: Unbreak SMMU and virtio-iommu on dev-iotlb support"),
> VHOST first tries to register IOMMU_NOTIFIER_DEVIOTLB_UNMAP notifier
> and if it fails, falls back to legacy IOMMU_NOTIFIER_UNMAP. So
> spapr_iommu must fail on the IOMMU_NOTIFIER_DEVIOTLB_UNMAP
> registration.
> 
> Reported-by: Peter Xu <peterx@redhat.com>
> Fixes: b68ba1ca57677acf870d5ab10579e6105c1f5338
> Signed-off-by: Eric Auger <eric.auger@redhat.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/ppc/spapr_iommu.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index 30352df00e..24537ffcbd 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -212,6 +212,11 @@ static int spapr_tce_notify_flag_changed(IOMMUMemoryRegion *iommu,
>  {
>      struct SpaprTceTable *tbl = container_of(iommu, SpaprTceTable, iommu);
>  
> +    if (new & IOMMU_NOTIFIER_DEVIOTLB_UNMAP) {
> +        error_setg(errp, "spart_tce does not support dev-iotlb yet");
> +        return -EINVAL;
> +    }
> +
>      if (old == IOMMU_NOTIFIER_NONE && new != IOMMU_NOTIFIER_NONE) {
>          spapr_tce_set_need_vfio(tbl, true);
>      } else if (old != IOMMU_NOTIFIER_NONE && new == IOMMU_NOTIFIER_NONE) {
> -- 
> 2.26.2

Re: [PATCH 2/2] spapr_iommu: Fix vhost integration regression

[Greg Kurz]

On Tue,  9 Feb 2021 22:32:33 +0100
Eric Auger <eric.auger@redhat.com> wrote:

> Previous work on dev-iotlb message broke spapr_iommu/vhost integration
> as it did for SMMU and virtio-iommu. The spapr_iommu currently
> only sends IOMMU_NOTIFIER_UNMAP notifications. Since commit
> 958ec334bca3 ("vhost: Unbreak SMMU and virtio-iommu on dev-iotlb support"),
> VHOST first tries to register IOMMU_NOTIFIER_DEVIOTLB_UNMAP notifier
> and if it fails, falls back to legacy IOMMU_NOTIFIER_UNMAP. So
> spapr_iommu must fail on the IOMMU_NOTIFIER_DEVIOTLB_UNMAP
> registration.
> 
> Reported-by: Peter Xu <peterx@redhat.com>
> Fixes: b68ba1ca57677acf870d5ab10579e6105c1f5338
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr_iommu.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index 30352df00e..24537ffcbd 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -212,6 +212,11 @@ static int spapr_tce_notify_flag_changed(IOMMUMemoryRegion *iommu,
>  {
>      struct SpaprTceTable *tbl = container_of(iommu, SpaprTceTable, iommu);
>  
> +    if (new & IOMMU_NOTIFIER_DEVIOTLB_UNMAP) {
> +        error_setg(errp, "spart_tce does not support dev-iotlb yet");
> +        return -EINVAL;
> +    }
> +
>      if (old == IOMMU_NOTIFIER_NONE && new != IOMMU_NOTIFIER_NONE) {
>          spapr_tce_set_need_vfio(tbl, true);
>      } else if (old != IOMMU_NOTIFIER_NONE && new == IOMMU_NOTIFIER_NONE) {

Re: [PATCH 0/2] Additional vIOMMU fixes related to UNMAP notifiers

[Peter Xu]

On Tue, Feb 09, 2021 at 10:32:31PM +0100, Eric Auger wrote:
> 958ec334bca3 ("vhost: Unbreak SMMU and virtio-iommu on dev-iotlb
> support") fixed part of the UNMAP related regressions introduced by
> b68ba1ca5767 ("memory: Add IOMMU_NOTIFIER_DEVIOTLB_UNMAP
> IOMMUTLBNotificationType").
> 
> However the case of the spapr_iommu was not addressed. It should be
> identical to the others. Also 958ec334bca3 introduced a regresion
> on the VIRTIO-IOMMU/VFIO integration.
> 
> spapr_iommu is not tested.

Reviewed-by: Peter Xu <peterx@redhat.com>

Thanks!

-- 
Peter Xu