[PATCH v5 0/2] vhost: register and change IOMMU flag depending on ATS state

[PATCH v5 0/2] vhost: register and change IOMMU flag depending on ATS state

[Viktor Prutyanov]

When IOMMU and vhost are enabled together, QEMU tracks IOTLB or
Device-TLB unmap events depending on whether Device-TLB is enabled. But
even if Device-TLB and PCI ATS is enabled, the guest can reject to use
it. For example, this situation appears when Windows Server 2022 is
running with intel-iommu with device-iotlb=on and virtio-net-pci with
vhost=on. The guest implies that no address translation info cached in
device IOTLB and doesn't send device IOTLB invalidation commands. So,
it leads to irrelevant address translations in vhost-net in the host
kernel. Therefore network frames from the guest in host tap interface
contains wrong payload data.

This series adds checking of ATS state for proper unmap flag register
(IOMMU_NOTIFIER_UNMAP or IOMMU_NOTIFIER_DEVIOTLB_UNMAP).

Tested on Windows Server 2022, Windows 11 and Fedora guests with
 -device virtio-net-pci,bus=pci.3,netdev=nd0,iommu_platform=on,ats=on
 -netdev tap,id=nd0,ifname=tap1,script=no,downscript=no,vhost=on
 -device intel-iommu,intremap=on,eim=on,device-iotlb=on/off
Tested on Fedora guest with
 -device virtio-iommu

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2001312

v5: add vhost_toggle_device_iotlb to vhost-stub
v4: call vhost_toggle_device_iotlb regardless of vhost backend,
    move vhost_started check to generic part
v3: call virtio_pci_ats_ctrl_trigger directly, remove
    IOMMU_NOTIFIER_UNMAP fallbacks
v2: remove memory_region_iommu_notify_flags_changed, move trigger to
    VirtioDeviceClass, use vhost_ops, use device_iotlb name

Viktor Prutyanov (2):
  vhost: register and change IOMMU flag depending on Device-TLB state
  virtio-net: pass Device-TLB enable/disable events to vhost

 hw/net/virtio-net.c       |  1 +
 hw/virtio/vhost-stub.c    |  4 ++++
 hw/virtio/vhost.c         | 38 ++++++++++++++++++++++++++------------
 include/hw/virtio/vhost.h |  1 +
 4 files changed, 32 insertions(+), 12 deletions(-)

-- 
2.21.0

[PATCH v5 1/2] vhost: register and change IOMMU flag depending on Device-TLB state

[Viktor Prutyanov]

The guest can disable or never enable Device-TLB. In these cases,
it can't be used even if enabled in QEMU. So, check Device-TLB state
before registering IOMMU notifier and select unmap flag depending on
that. Also, implement a way to change IOMMU notifier flag if Device-TLB
state is changed.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2001312
Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio/vhost-stub.c    |  4 ++++
 hw/virtio/vhost.c         | 38 ++++++++++++++++++++++++++------------
 include/hw/virtio/vhost.h |  1 +
 3 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/hw/virtio/vhost-stub.c b/hw/virtio/vhost-stub.c
index c175148fce..aa858ef3fb 100644
--- a/hw/virtio/vhost-stub.c
+++ b/hw/virtio/vhost-stub.c
@@ -15,3 +15,7 @@ bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp)
 void vhost_user_cleanup(VhostUserState *user)
 {
 }
+
+void vhost_toggle_device_iotlb(VirtIODevice *vdev)
+{
+}
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 23da579ce2..e48507b5a1 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -781,7 +781,6 @@ static void vhost_iommu_region_add(MemoryListener *listener,
     Int128 end;
     int iommu_idx;
     IOMMUMemoryRegion *iommu_mr;
-    int ret;
 
     if (!memory_region_is_iommu(section->mr)) {
         return;
@@ -796,7 +795,9 @@ static void vhost_iommu_region_add(MemoryListener *listener,
     iommu_idx = memory_region_iommu_attrs_to_index(iommu_mr,
                                                    MEMTXATTRS_UNSPECIFIED);
     iommu_notifier_init(&iommu->n, vhost_iommu_unmap_notify,
-                        IOMMU_NOTIFIER_DEVIOTLB_UNMAP,
+                        dev->vdev->device_iotlb_enabled ?
+                            IOMMU_NOTIFIER_DEVIOTLB_UNMAP :
+                            IOMMU_NOTIFIER_UNMAP,
                         section->offset_within_region,
                         int128_get64(end),
                         iommu_idx);
@@ -804,16 +805,8 @@ static void vhost_iommu_region_add(MemoryListener *listener,
     iommu->iommu_offset = section->offset_within_address_space -
                           section->offset_within_region;
     iommu->hdev = dev;
-    ret = memory_region_register_iommu_notifier(section->mr, &iommu->n, NULL);
-    if (ret) {
-        /*
-         * Some vIOMMUs do not support dev-iotlb yet.  If so, try to use the
-         * UNMAP legacy message
-         */
-        iommu->n.notifier_flags = IOMMU_NOTIFIER_UNMAP;
-        memory_region_register_iommu_notifier(section->mr, &iommu->n,
-                                              &error_fatal);
-    }
+    memory_region_register_iommu_notifier(section->mr, &iommu->n,
+                                          &error_fatal);
     QLIST_INSERT_HEAD(&dev->iommu_list, iommu, iommu_next);
     /* TODO: can replay help performance here? */
 }
@@ -841,6 +834,27 @@ static void vhost_iommu_region_del(MemoryListener *listener,
     }
 }
 
+void vhost_toggle_device_iotlb(VirtIODevice *vdev)
+{
+    VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+    struct vhost_dev *dev;
+    struct vhost_iommu *iommu;
+
+    if (vdev->vhost_started) {
+        dev = vdc->get_vhost(vdev);
+    } else {
+        return;
+    }
+
+    QLIST_FOREACH(iommu, &dev->iommu_list, iommu_next) {
+        memory_region_unregister_iommu_notifier(iommu->mr, &iommu->n);
+        iommu->n.notifier_flags = vdev->device_iotlb_enabled ?
+                IOMMU_NOTIFIER_DEVIOTLB_UNMAP : IOMMU_NOTIFIER_UNMAP;
+        memory_region_register_iommu_notifier(iommu->mr, &iommu->n,
+                                              &error_fatal);
+    }
+}
+
 static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
                                     struct vhost_virtqueue *vq,
                                     unsigned idx, bool enable_log)
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index f7f10c8fb7..6a173cb9fa 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -320,6 +320,7 @@ bool vhost_has_free_slot(void);
 int vhost_net_set_backend(struct vhost_dev *hdev,
                           struct vhost_vring_file *file);
 
+void vhost_toggle_device_iotlb(VirtIODevice *vdev);
 int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write);
 
 int vhost_virtqueue_start(struct vhost_dev *dev, struct VirtIODevice *vdev,
-- 
2.21.0

[PATCH v5 2/2] virtio-net: pass Device-TLB enable/disable events to vhost

[Viktor Prutyanov]

If vhost is enabled for virtio-net, Device-TLB enable/disable events
must be passed to vhost for proper IOMMU unmap flag selection.

Signed-off-by: Viktor Prutyanov <viktor@daynix.com>
Acked-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/virtio-net.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 6df6b7329d..30497c00b5 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3951,6 +3951,7 @@ static void virtio_net_class_init(ObjectClass *klass, void *data)
     vdc->vmsd = &vmstate_virtio_net_device;
     vdc->primary_unplug_pending = primary_unplug_pending;
     vdc->get_vhost = virtio_net_get_vhost;
+    vdc->toggle_device_iotlb = vhost_toggle_device_iotlb;
 }
 
 static const TypeInfo virtio_net_info = {
-- 
2.21.0

Re: [PATCH v5 0/2] vhost: register and change IOMMU flag depending on ATS state

[Viktor Prutyanov]

On Mon, Jun 26, 2023 at 12:13 PM Viktor Prutyanov <viktor@daynix.com> wrote:
>
> When IOMMU and vhost are enabled together, QEMU tracks IOTLB or
> Device-TLB unmap events depending on whether Device-TLB is enabled. But
> even if Device-TLB and PCI ATS is enabled, the guest can reject to use
> it. For example, this situation appears when Windows Server 2022 is
> running with intel-iommu with device-iotlb=on and virtio-net-pci with
> vhost=on. The guest implies that no address translation info cached in
> device IOTLB and doesn't send device IOTLB invalidation commands. So,
> it leads to irrelevant address translations in vhost-net in the host
> kernel. Therefore network frames from the guest in host tap interface
> contains wrong payload data.
>
> This series adds checking of ATS state for proper unmap flag register
> (IOMMU_NOTIFIER_UNMAP or IOMMU_NOTIFIER_DEVIOTLB_UNMAP).
>
> Tested on Windows Server 2022, Windows 11 and Fedora guests with
>  -device virtio-net-pci,bus=pci.3,netdev=nd0,iommu_platform=on,ats=on
>  -netdev tap,id=nd0,ifname=tap1,script=no,downscript=no,vhost=on
>  -device intel-iommu,intremap=on,eim=on,device-iotlb=on/off
> Tested on Fedora guest with
>  -device virtio-iommu
>
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2001312
>
> v5: add vhost_toggle_device_iotlb to vhost-stub
> v4: call vhost_toggle_device_iotlb regardless of vhost backend,
>     move vhost_started check to generic part
> v3: call virtio_pci_ats_ctrl_trigger directly, remove
>     IOMMU_NOTIFIER_UNMAP fallbacks
> v2: remove memory_region_iommu_notify_flags_changed, move trigger to
>     VirtioDeviceClass, use vhost_ops, use device_iotlb name
>
> Viktor Prutyanov (2):
>   vhost: register and change IOMMU flag depending on Device-TLB state
>   virtio-net: pass Device-TLB enable/disable events to vhost
>
>  hw/net/virtio-net.c       |  1 +
>  hw/virtio/vhost-stub.c    |  4 ++++
>  hw/virtio/vhost.c         | 38 ++++++++++++++++++++++++++------------
>  include/hw/virtio/vhost.h |  1 +
>  4 files changed, 32 insertions(+), 12 deletions(-)
>
> --
> 2.21.0
>

ping

Re: [PATCH v5 0/2] vhost: register and change IOMMU flag depending on ATS state

[Viktor Prutyanov]

ping

On Mon, Jul 3, 2023 at 11:25 AM Viktor Prutyanov <viktor@daynix.com> wrote:
>
> On Mon, Jun 26, 2023 at 12:13 PM Viktor Prutyanov <viktor@daynix.com> wrote:
> >
> > When IOMMU and vhost are enabled together, QEMU tracks IOTLB or
> > Device-TLB unmap events depending on whether Device-TLB is enabled. But
> > even if Device-TLB and PCI ATS is enabled, the guest can reject to use
> > it. For example, this situation appears when Windows Server 2022 is
> > running with intel-iommu with device-iotlb=on and virtio-net-pci with
> > vhost=on. The guest implies that no address translation info cached in
> > device IOTLB and doesn't send device IOTLB invalidation commands. So,
> > it leads to irrelevant address translations in vhost-net in the host
> > kernel. Therefore network frames from the guest in host tap interface
> > contains wrong payload data.
> >
> > This series adds checking of ATS state for proper unmap flag register
> > (IOMMU_NOTIFIER_UNMAP or IOMMU_NOTIFIER_DEVIOTLB_UNMAP).
> >
> > Tested on Windows Server 2022, Windows 11 and Fedora guests with
> >  -device virtio-net-pci,bus=pci.3,netdev=nd0,iommu_platform=on,ats=on
> >  -netdev tap,id=nd0,ifname=tap1,script=no,downscript=no,vhost=on
> >  -device intel-iommu,intremap=on,eim=on,device-iotlb=on/off
> > Tested on Fedora guest with
> >  -device virtio-iommu
> >
> > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2001312
> >
> > v5: add vhost_toggle_device_iotlb to vhost-stub
> > v4: call vhost_toggle_device_iotlb regardless of vhost backend,
> >     move vhost_started check to generic part
> > v3: call virtio_pci_ats_ctrl_trigger directly, remove
> >     IOMMU_NOTIFIER_UNMAP fallbacks
> > v2: remove memory_region_iommu_notify_flags_changed, move trigger to
> >     VirtioDeviceClass, use vhost_ops, use device_iotlb name
> >
> > Viktor Prutyanov (2):
> >   vhost: register and change IOMMU flag depending on Device-TLB state
> >   virtio-net: pass Device-TLB enable/disable events to vhost
> >
> >  hw/net/virtio-net.c       |  1 +
> >  hw/virtio/vhost-stub.c    |  4 ++++
> >  hw/virtio/vhost.c         | 38 ++++++++++++++++++++++++++------------
> >  include/hw/virtio/vhost.h |  1 +
> >  4 files changed, 32 insertions(+), 12 deletions(-)
> >
> > --
> > 2.21.0
> >
>
> ping

Re: [PATCH v5 0/2] vhost: register and change IOMMU flag depending on ATS state

[Viktor Prutyanov]

On Mon, Jun 26, 2023 at 12:13 PM Viktor Prutyanov <viktor@daynix.com> wrote:
>
> When IOMMU and vhost are enabled together, QEMU tracks IOTLB or
> Device-TLB unmap events depending on whether Device-TLB is enabled. But
> even if Device-TLB and PCI ATS is enabled, the guest can reject to use
> it. For example, this situation appears when Windows Server 2022 is
> running with intel-iommu with device-iotlb=on and virtio-net-pci with
> vhost=on. The guest implies that no address translation info cached in
> device IOTLB and doesn't send device IOTLB invalidation commands. So,
> it leads to irrelevant address translations in vhost-net in the host
> kernel. Therefore network frames from the guest in host tap interface
> contains wrong payload data.
>
> This series adds checking of ATS state for proper unmap flag register
> (IOMMU_NOTIFIER_UNMAP or IOMMU_NOTIFIER_DEVIOTLB_UNMAP).
>
> Tested on Windows Server 2022, Windows 11 and Fedora guests with
>  -device virtio-net-pci,bus=pci.3,netdev=nd0,iommu_platform=on,ats=on
>  -netdev tap,id=nd0,ifname=tap1,script=no,downscript=no,vhost=on
>  -device intel-iommu,intremap=on,eim=on,device-iotlb=on/off
> Tested on Fedora guest with
>  -device virtio-iommu
>
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2001312
>
> v5: add vhost_toggle_device_iotlb to vhost-stub
> v4: call vhost_toggle_device_iotlb regardless of vhost backend,
>     move vhost_started check to generic part
> v3: call virtio_pci_ats_ctrl_trigger directly, remove
>     IOMMU_NOTIFIER_UNMAP fallbacks
> v2: remove memory_region_iommu_notify_flags_changed, move trigger to
>     VirtioDeviceClass, use vhost_ops, use device_iotlb name
>
> Viktor Prutyanov (2):
>   vhost: register and change IOMMU flag depending on Device-TLB state
>   virtio-net: pass Device-TLB enable/disable events to vhost
>
>  hw/net/virtio-net.c       |  1 +
>  hw/virtio/vhost-stub.c    |  4 ++++
>  hw/virtio/vhost.c         | 38 ++++++++++++++++++++++++++------------
>  include/hw/virtio/vhost.h |  1 +
>  4 files changed, 32 insertions(+), 12 deletions(-)
>
> --
> 2.21.0
>

As we discussed with Jason, this series should be also applied to
qemu-stable, as well as "virtio-pci: add handling of PCI ATS and
Device-TLB enable/disable"
https://patchwork.kernel.org/project/qemu-devel/patch/20230512135122.70403-2-viktor@daynix.com

Best regards
Viktor Prutyanov