* [PATCH v5 1/2] vhost: register and change IOMMU flag depending on Device-TLB state
2023-06-26 9:12 [PATCH v5 0/2] vhost: register and change IOMMU flag depending on ATS state Viktor Prutyanov
@ 2023-06-26 9:12 ` Viktor Prutyanov
2023-06-26 9:12 ` [PATCH v5 2/2] virtio-net: pass Device-TLB enable/disable events to vhost Viktor Prutyanov
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Viktor Prutyanov @ 2023-06-26 9:12 UTC (permalink / raw)
To: mst, jasowang; +Cc: qemu-devel, yan, yuri.benditovich, 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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v5 2/2] virtio-net: pass Device-TLB enable/disable events to vhost
2023-06-26 9:12 [PATCH v5 0/2] vhost: register and change IOMMU flag depending on ATS state Viktor Prutyanov
2023-06-26 9:12 ` [PATCH v5 1/2] vhost: register and change IOMMU flag depending on Device-TLB state Viktor Prutyanov
@ 2023-06-26 9:12 ` Viktor Prutyanov
2023-07-03 8:25 ` [PATCH v5 0/2] vhost: register and change IOMMU flag depending on ATS state Viktor Prutyanov
2023-07-18 11:15 ` Viktor Prutyanov
3 siblings, 0 replies; 6+ messages in thread
From: Viktor Prutyanov @ 2023-06-26 9:12 UTC (permalink / raw)
To: mst, jasowang; +Cc: qemu-devel, yan, yuri.benditovich, 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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v5 0/2] vhost: register and change IOMMU flag depending on ATS state
2023-06-26 9:12 [PATCH v5 0/2] vhost: register and change IOMMU flag depending on ATS state Viktor Prutyanov
2023-06-26 9:12 ` [PATCH v5 1/2] vhost: register and change IOMMU flag depending on Device-TLB state Viktor Prutyanov
2023-06-26 9:12 ` [PATCH v5 2/2] virtio-net: pass Device-TLB enable/disable events to vhost Viktor Prutyanov
@ 2023-07-03 8:25 ` Viktor Prutyanov
2023-07-10 14:31 ` Viktor Prutyanov
2023-07-18 11:15 ` Viktor Prutyanov
3 siblings, 1 reply; 6+ messages in thread
From: Viktor Prutyanov @ 2023-07-03 8:25 UTC (permalink / raw)
To: mst, jasowang; +Cc: qemu-devel, yan, yuri.benditovich
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 0/2] vhost: register and change IOMMU flag depending on ATS state
2023-07-03 8:25 ` [PATCH v5 0/2] vhost: register and change IOMMU flag depending on ATS state Viktor Prutyanov
@ 2023-07-10 14:31 ` Viktor Prutyanov
0 siblings, 0 replies; 6+ messages in thread
From: Viktor Prutyanov @ 2023-07-10 14:31 UTC (permalink / raw)
To: mst, jasowang; +Cc: qemu-devel, yan, yuri.benditovich
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 0/2] vhost: register and change IOMMU flag depending on ATS state
2023-06-26 9:12 [PATCH v5 0/2] vhost: register and change IOMMU flag depending on ATS state Viktor Prutyanov
` (2 preceding siblings ...)
2023-07-03 8:25 ` [PATCH v5 0/2] vhost: register and change IOMMU flag depending on ATS state Viktor Prutyanov
@ 2023-07-18 11:15 ` Viktor Prutyanov
3 siblings, 0 replies; 6+ messages in thread
From: Viktor Prutyanov @ 2023-07-18 11:15 UTC (permalink / raw)
To: mst, jasowang; +Cc: qemu-devel, yan, yuri.benditovich, qemu-stable
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
^ permalink raw reply [flat|nested] 6+ messages in thread