| Message ID | 20230512135122.70403-3-viktor@daynix.com |
|---|---|
| State | New |
| Headers | show |
| Series | vhost: register and change IOMMU flag depending on ATS state | expand |
On Fri, May 12, 2023 at 9:51 PM Viktor Prutyanov <viktor@daynix.com> wrote: > > 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> > --- > hw/virtio/vhost-backend.c | 6 ++++++ > hw/virtio/vhost.c | 30 ++++++++++++++++++------------ > include/hw/virtio/vhost-backend.h | 3 +++ > include/hw/virtio/vhost.h | 1 + > 4 files changed, 28 insertions(+), 12 deletions(-) > > diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c > index 8e581575c9..d39bfefd2d 100644 > --- a/hw/virtio/vhost-backend.c > +++ b/hw/virtio/vhost-backend.c > @@ -297,6 +297,11 @@ static void vhost_kernel_set_iotlb_callback(struct vhost_dev *dev, > qemu_set_fd_handler((uintptr_t)dev->opaque, NULL, NULL, NULL); > } > > +static void vhost_kernel_toggle_device_iotlb(struct vhost_dev *dev) > +{ > + vhost_toggle_device_iotlb(dev); > +} > + > const VhostOps kernel_ops = { > .backend_type = VHOST_BACKEND_TYPE_KERNEL, > .vhost_backend_init = vhost_kernel_init, > @@ -328,6 +333,7 @@ const VhostOps kernel_ops = { > .vhost_vsock_set_running = vhost_kernel_vsock_set_running, > .vhost_set_iotlb_callback = vhost_kernel_set_iotlb_callback, > .vhost_send_device_iotlb_msg = vhost_kernel_send_device_iotlb_msg, > + .vhost_toggle_device_iotlb = vhost_kernel_toggle_device_iotlb, > }; > #endif > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > index 746d130c74..41c9fbf286 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); > - } So we lose this fallback. Is this really intended? E.g does it work if you are using virtio-iommu? Thanks > + 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,19 @@ static void vhost_iommu_region_del(MemoryListener *listener, > } > } > > +void vhost_toggle_device_iotlb(struct vhost_dev *dev) > +{ > + struct vhost_iommu *iommu; > + > + QLIST_FOREACH(iommu, &dev->iommu_list, iommu_next) { > + memory_region_unregister_iommu_notifier(iommu->mr, &iommu->n); > + iommu->n.notifier_flags = dev->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-backend.h b/include/hw/virtio/vhost-backend.h > index ec3fbae58d..10a3c36b4b 100644 > --- a/include/hw/virtio/vhost-backend.h > +++ b/include/hw/virtio/vhost-backend.h > @@ -133,6 +133,8 @@ typedef int (*vhost_set_config_call_op)(struct vhost_dev *dev, > > typedef void (*vhost_reset_status_op)(struct vhost_dev *dev); > > +typedef void (*vhost_toggle_device_iotlb_op)(struct vhost_dev *dev); > + > typedef struct VhostOps { > VhostBackendType backend_type; > vhost_backend_init vhost_backend_init; > @@ -181,6 +183,7 @@ typedef struct VhostOps { > vhost_force_iommu_op vhost_force_iommu; > vhost_set_config_call_op vhost_set_config_call; > vhost_reset_status_op vhost_reset_status; > + vhost_toggle_device_iotlb_op vhost_toggle_device_iotlb; > } VhostOps; > > int vhost_backend_update_device_iotlb(struct vhost_dev *dev, > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h > index a52f273347..785832ed46 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(struct vhost_dev *dev); > 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.35.1 >
On Thu, May 18, 2023 at 9:14 AM Jason Wang <jasowang@redhat.com> wrote: > > On Fri, May 12, 2023 at 9:51 PM Viktor Prutyanov <viktor@daynix.com> wrote: > > > > 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> > > --- > > hw/virtio/vhost-backend.c | 6 ++++++ > > hw/virtio/vhost.c | 30 ++++++++++++++++++------------ > > include/hw/virtio/vhost-backend.h | 3 +++ > > include/hw/virtio/vhost.h | 1 + > > 4 files changed, 28 insertions(+), 12 deletions(-) > > > > diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c > > index 8e581575c9..d39bfefd2d 100644 > > --- a/hw/virtio/vhost-backend.c > > +++ b/hw/virtio/vhost-backend.c > > @@ -297,6 +297,11 @@ static void vhost_kernel_set_iotlb_callback(struct vhost_dev *dev, > > qemu_set_fd_handler((uintptr_t)dev->opaque, NULL, NULL, NULL); > > } > > > > +static void vhost_kernel_toggle_device_iotlb(struct vhost_dev *dev) > > +{ > > + vhost_toggle_device_iotlb(dev); > > +} > > + > > const VhostOps kernel_ops = { > > .backend_type = VHOST_BACKEND_TYPE_KERNEL, > > .vhost_backend_init = vhost_kernel_init, > > @@ -328,6 +333,7 @@ const VhostOps kernel_ops = { > > .vhost_vsock_set_running = vhost_kernel_vsock_set_running, > > .vhost_set_iotlb_callback = vhost_kernel_set_iotlb_callback, > > .vhost_send_device_iotlb_msg = vhost_kernel_send_device_iotlb_msg, > > + .vhost_toggle_device_iotlb = vhost_kernel_toggle_device_iotlb, > > }; > > #endif > > > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > > index 746d130c74..41c9fbf286 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); > > - } > > So we lose this fallback. Is this really intended? > > E.g does it work if you are using virtio-iommu? It works for virtio-iommu because Linux guest doesn't enable PCI ATS in this situation. From my point of view, this fallback is not needed anymore, because it triggers only if Device-TLB isn't available on the host side but the guest misbehaves and tries to enable it. Also, I would like to discuss two more points: 1. The patch series in its current form will fix the issue for vhost+iommu setup for any VirtIO device with any vhost backend when ATS is enabled at the beginning. But if ATS is enabled/disabled in the runtime it will only work for virtio-net with vhost-kernel. All other devices and backends are out of scope and will need to add almost the same device_iotlb_toggle and vhost_device_iotlb_toggle handlers. Since the issue is general for any device and any backend, is it normal from architectural point of view? 2. When the series will be applied, we should enable DMA remapping for new Windows guest drivers, such as NetKVM. But if the user with enabled vhost+iommu updated the driver with old QEMU, the bug would reappear, because the guest doesn't know that the fix isn't present. May be we should discuss some mechanism to report that host is aware of guest's accept/reject of ATS/Device-TLB? Thanks, Viktor Prutyanov > > Thanks > > > + 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,19 @@ static void vhost_iommu_region_del(MemoryListener *listener, > > } > > } > > > > +void vhost_toggle_device_iotlb(struct vhost_dev *dev) > > +{ > > + struct vhost_iommu *iommu; > > + > > + QLIST_FOREACH(iommu, &dev->iommu_list, iommu_next) { > > + memory_region_unregister_iommu_notifier(iommu->mr, &iommu->n); > > + iommu->n.notifier_flags = dev->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-backend.h b/include/hw/virtio/vhost-backend.h > > index ec3fbae58d..10a3c36b4b 100644 > > --- a/include/hw/virtio/vhost-backend.h > > +++ b/include/hw/virtio/vhost-backend.h > > @@ -133,6 +133,8 @@ typedef int (*vhost_set_config_call_op)(struct vhost_dev *dev, > > > > typedef void (*vhost_reset_status_op)(struct vhost_dev *dev); > > > > +typedef void (*vhost_toggle_device_iotlb_op)(struct vhost_dev *dev); > > + > > typedef struct VhostOps { > > VhostBackendType backend_type; > > vhost_backend_init vhost_backend_init; > > @@ -181,6 +183,7 @@ typedef struct VhostOps { > > vhost_force_iommu_op vhost_force_iommu; > > vhost_set_config_call_op vhost_set_config_call; > > vhost_reset_status_op vhost_reset_status; > > + vhost_toggle_device_iotlb_op vhost_toggle_device_iotlb; > > } VhostOps; > > > > int vhost_backend_update_device_iotlb(struct vhost_dev *dev, > > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h > > index a52f273347..785832ed46 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(struct vhost_dev *dev); > > 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.35.1 > > >
On Sat, May 20, 2023 at 1:50 AM Viktor Prutyanov <viktor@daynix.com> wrote: > > On Thu, May 18, 2023 at 9:14 AM Jason Wang <jasowang@redhat.com> wrote: > > > > On Fri, May 12, 2023 at 9:51 PM Viktor Prutyanov <viktor@daynix.com> wrote: > > > > > > 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> > > > --- > > > hw/virtio/vhost-backend.c | 6 ++++++ > > > hw/virtio/vhost.c | 30 ++++++++++++++++++------------ > > > include/hw/virtio/vhost-backend.h | 3 +++ > > > include/hw/virtio/vhost.h | 1 + > > > 4 files changed, 28 insertions(+), 12 deletions(-) > > > > > > diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c > > > index 8e581575c9..d39bfefd2d 100644 > > > --- a/hw/virtio/vhost-backend.c > > > +++ b/hw/virtio/vhost-backend.c > > > @@ -297,6 +297,11 @@ static void vhost_kernel_set_iotlb_callback(struct vhost_dev *dev, > > > qemu_set_fd_handler((uintptr_t)dev->opaque, NULL, NULL, NULL); > > > } > > > > > > +static void vhost_kernel_toggle_device_iotlb(struct vhost_dev *dev) > > > +{ > > > + vhost_toggle_device_iotlb(dev); > > > +} > > > + > > > const VhostOps kernel_ops = { > > > .backend_type = VHOST_BACKEND_TYPE_KERNEL, > > > .vhost_backend_init = vhost_kernel_init, > > > @@ -328,6 +333,7 @@ const VhostOps kernel_ops = { > > > .vhost_vsock_set_running = vhost_kernel_vsock_set_running, > > > .vhost_set_iotlb_callback = vhost_kernel_set_iotlb_callback, > > > .vhost_send_device_iotlb_msg = vhost_kernel_send_device_iotlb_msg, > > > + .vhost_toggle_device_iotlb = vhost_kernel_toggle_device_iotlb, > > > }; > > > #endif > > > > > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > > > index 746d130c74..41c9fbf286 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); > > > - } > > > > So we lose this fallback. Is this really intended? > > > > E.g does it work if you are using virtio-iommu? > > It works for virtio-iommu because Linux guest doesn't enable PCI ATS in > this situation. From my point of view, this fallback is not needed > anymore, because it triggers only if Device-TLB isn't available on the > host side but the guest misbehaves and tries to enable it. Ok. > > Also, I would like to discuss two more points: > > 1. The patch series in its current form will fix the issue for > vhost+iommu setup for any VirtIO device with any vhost backend when > ATS is enabled at the beginning. But if ATS is enabled/disabled in the > runtime it will only work for virtio-net with vhost-kernel. All other > devices and backends are out of scope and will need to add almost the > same device_iotlb_toggle and vhost_device_iotlb_toggle handlers. Since > the issue is general for any device and any backend, is it normal from > architectural point of view? Yes, so I think it's better to fix others vhost backends. Actually I wonder if we can have simply reuse vhost_toggle_device_iotlb() since it's nothing specific to the vhost backend. It's just about the way to receive IOTLB invalidation from vIOMMU. > > 2. When the series will be applied, we should enable DMA remapping for > new Windows guest drivers, such as NetKVM. But if the user with enabled > vhost+iommu updated the driver with old QEMU, the bug would reappear, > because the guest doesn't know that the fix isn't present. May be we > should discuss some mechanism to report that host is aware of guest's > accept/reject of ATS/Device-TLB? I'm not sure how this can help? Or anything makes this fix different from other fixes? One thing I can think is to backport the fixes to -stable. Thanks > > Thanks, > Viktor Prutyanov > > > > > Thanks > > > > > + 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,19 @@ static void vhost_iommu_region_del(MemoryListener *listener, > > > } > > > } > > > > > > +void vhost_toggle_device_iotlb(struct vhost_dev *dev) > > > +{ > > > + struct vhost_iommu *iommu; > > > + > > > + QLIST_FOREACH(iommu, &dev->iommu_list, iommu_next) { > > > + memory_region_unregister_iommu_notifier(iommu->mr, &iommu->n); > > > + iommu->n.notifier_flags = dev->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-backend.h b/include/hw/virtio/vhost-backend.h > > > index ec3fbae58d..10a3c36b4b 100644 > > > --- a/include/hw/virtio/vhost-backend.h > > > +++ b/include/hw/virtio/vhost-backend.h > > > @@ -133,6 +133,8 @@ typedef int (*vhost_set_config_call_op)(struct vhost_dev *dev, > > > > > > typedef void (*vhost_reset_status_op)(struct vhost_dev *dev); > > > > > > +typedef void (*vhost_toggle_device_iotlb_op)(struct vhost_dev *dev); > > > + > > > typedef struct VhostOps { > > > VhostBackendType backend_type; > > > vhost_backend_init vhost_backend_init; > > > @@ -181,6 +183,7 @@ typedef struct VhostOps { > > > vhost_force_iommu_op vhost_force_iommu; > > > vhost_set_config_call_op vhost_set_config_call; > > > vhost_reset_status_op vhost_reset_status; > > > + vhost_toggle_device_iotlb_op vhost_toggle_device_iotlb; > > > } VhostOps; > > > > > > int vhost_backend_update_device_iotlb(struct vhost_dev *dev, > > > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h > > > index a52f273347..785832ed46 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(struct vhost_dev *dev); > > > 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.35.1 > > > > > >
On Wed, May 24, 2023 at 11:25 AM Jason Wang <jasowang@redhat.com> wrote: > > On Sat, May 20, 2023 at 1:50 AM Viktor Prutyanov <viktor@daynix.com> wrote: > > > > On Thu, May 18, 2023 at 9:14 AM Jason Wang <jasowang@redhat.com> wrote: > > > > > > On Fri, May 12, 2023 at 9:51 PM Viktor Prutyanov <viktor@daynix.com> wrote: > > > > > > > > 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> > > > > --- > > > > hw/virtio/vhost-backend.c | 6 ++++++ > > > > hw/virtio/vhost.c | 30 ++++++++++++++++++------------ > > > > include/hw/virtio/vhost-backend.h | 3 +++ > > > > include/hw/virtio/vhost.h | 1 + > > > > 4 files changed, 28 insertions(+), 12 deletions(-) > > > > > > > > diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c > > > > index 8e581575c9..d39bfefd2d 100644 > > > > --- a/hw/virtio/vhost-backend.c > > > > +++ b/hw/virtio/vhost-backend.c > > > > @@ -297,6 +297,11 @@ static void vhost_kernel_set_iotlb_callback(struct vhost_dev *dev, > > > > qemu_set_fd_handler((uintptr_t)dev->opaque, NULL, NULL, NULL); > > > > } > > > > > > > > +static void vhost_kernel_toggle_device_iotlb(struct vhost_dev *dev) > > > > +{ > > > > + vhost_toggle_device_iotlb(dev); > > > > +} > > > > + > > > > const VhostOps kernel_ops = { > > > > .backend_type = VHOST_BACKEND_TYPE_KERNEL, > > > > .vhost_backend_init = vhost_kernel_init, > > > > @@ -328,6 +333,7 @@ const VhostOps kernel_ops = { > > > > .vhost_vsock_set_running = vhost_kernel_vsock_set_running, > > > > .vhost_set_iotlb_callback = vhost_kernel_set_iotlb_callback, > > > > .vhost_send_device_iotlb_msg = vhost_kernel_send_device_iotlb_msg, > > > > + .vhost_toggle_device_iotlb = vhost_kernel_toggle_device_iotlb, > > > > }; > > > > #endif > > > > > > > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > > > > index 746d130c74..41c9fbf286 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); > > > > - } > > > > > > So we lose this fallback. Is this really intended? > > > > > > E.g does it work if you are using virtio-iommu? > > > > It works for virtio-iommu because Linux guest doesn't enable PCI ATS in > > this situation. From my point of view, this fallback is not needed > > anymore, because it triggers only if Device-TLB isn't available on the > > host side but the guest misbehaves and tries to enable it. > > Ok. > > > > > Also, I would like to discuss two more points: > > > > 1. The patch series in its current form will fix the issue for > > vhost+iommu setup for any VirtIO device with any vhost backend when > > ATS is enabled at the beginning. But if ATS is enabled/disabled in the > > runtime it will only work for virtio-net with vhost-kernel. All other > > devices and backends are out of scope and will need to add almost the > > same device_iotlb_toggle and vhost_device_iotlb_toggle handlers. Since > > the issue is general for any device and any backend, is it normal from > > architectural point of view? > > Yes, so I think it's better to fix others vhost backends. Actually I > wonder if we can have simply reuse vhost_toggle_device_iotlb() since > it's nothing specific to the vhost backend. It's just about the way to > receive IOTLB invalidation from vIOMMU. > > > > > 2. When the series will be applied, we should enable DMA remapping for > > new Windows guest drivers, such as NetKVM. But if the user with enabled > > vhost+iommu updated the driver with old QEMU, the bug would reappear, > > because the guest doesn't know that the fix isn't present. May be we > > should discuss some mechanism to report that host is aware of guest's > > accept/reject of ATS/Device-TLB? > > I'm not sure how this can help? Or anything makes this fix different > from other fixes? Let's imagine a user who runs Windows on QEMU with virtio-net-pci and intel-iommu with enabled vhost, ATS and Device-TLB. At the moment, DMA remapping is disabled through INF in NetKVM driver, and IOMMU is not working actually, and such a user doesn't observe the packet corruption issue at all. After DMA remapping in INF will be enabled, the issue will be observed with old QEMU. So, if such a user will not update QEMU but update the driver, he will encounter a problem he has never had before. > > One thing I can think is to backport the fixes to -stable. I think, it would be nice. It doesn't solve the problem 100%, though. Thanks > > Thanks > > > > > Thanks, > > Viktor Prutyanov > > > > > > > > Thanks > > > > > > > + 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,19 @@ static void vhost_iommu_region_del(MemoryListener *listener, > > > > } > > > > } > > > > > > > > +void vhost_toggle_device_iotlb(struct vhost_dev *dev) > > > > +{ > > > > + struct vhost_iommu *iommu; > > > > + > > > > + QLIST_FOREACH(iommu, &dev->iommu_list, iommu_next) { > > > > + memory_region_unregister_iommu_notifier(iommu->mr, &iommu->n); > > > > + iommu->n.notifier_flags = dev->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-backend.h b/include/hw/virtio/vhost-backend.h > > > > index ec3fbae58d..10a3c36b4b 100644 > > > > --- a/include/hw/virtio/vhost-backend.h > > > > +++ b/include/hw/virtio/vhost-backend.h > > > > @@ -133,6 +133,8 @@ typedef int (*vhost_set_config_call_op)(struct vhost_dev *dev, > > > > > > > > typedef void (*vhost_reset_status_op)(struct vhost_dev *dev); > > > > > > > > +typedef void (*vhost_toggle_device_iotlb_op)(struct vhost_dev *dev); > > > > + > > > > typedef struct VhostOps { > > > > VhostBackendType backend_type; > > > > vhost_backend_init vhost_backend_init; > > > > @@ -181,6 +183,7 @@ typedef struct VhostOps { > > > > vhost_force_iommu_op vhost_force_iommu; > > > > vhost_set_config_call_op vhost_set_config_call; > > > > vhost_reset_status_op vhost_reset_status; > > > > + vhost_toggle_device_iotlb_op vhost_toggle_device_iotlb; > > > > } VhostOps; > > > > > > > > int vhost_backend_update_device_iotlb(struct vhost_dev *dev, > > > > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h > > > > index a52f273347..785832ed46 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(struct vhost_dev *dev); > > > > 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.35.1 > > > > > > > > > >
On Thu, May 25, 2023 at 8:55 PM Viktor Prutyanov <viktor@daynix.com> wrote: > > On Wed, May 24, 2023 at 11:25 AM Jason Wang <jasowang@redhat.com> wrote: > > > > On Sat, May 20, 2023 at 1:50 AM Viktor Prutyanov <viktor@daynix.com> wrote: > > > > > > On Thu, May 18, 2023 at 9:14 AM Jason Wang <jasowang@redhat.com> wrote: > > > > > > > > On Fri, May 12, 2023 at 9:51 PM Viktor Prutyanov <viktor@daynix.com> wrote: > > > > > > > > > > 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> > > > > > --- > > > > > hw/virtio/vhost-backend.c | 6 ++++++ > > > > > hw/virtio/vhost.c | 30 ++++++++++++++++++------------ > > > > > include/hw/virtio/vhost-backend.h | 3 +++ > > > > > include/hw/virtio/vhost.h | 1 + > > > > > 4 files changed, 28 insertions(+), 12 deletions(-) > > > > > > > > > > diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c > > > > > index 8e581575c9..d39bfefd2d 100644 > > > > > --- a/hw/virtio/vhost-backend.c > > > > > +++ b/hw/virtio/vhost-backend.c > > > > > @@ -297,6 +297,11 @@ static void vhost_kernel_set_iotlb_callback(struct vhost_dev *dev, > > > > > qemu_set_fd_handler((uintptr_t)dev->opaque, NULL, NULL, NULL); > > > > > } > > > > > > > > > > +static void vhost_kernel_toggle_device_iotlb(struct vhost_dev *dev) > > > > > +{ > > > > > + vhost_toggle_device_iotlb(dev); > > > > > +} > > > > > + > > > > > const VhostOps kernel_ops = { > > > > > .backend_type = VHOST_BACKEND_TYPE_KERNEL, > > > > > .vhost_backend_init = vhost_kernel_init, > > > > > @@ -328,6 +333,7 @@ const VhostOps kernel_ops = { > > > > > .vhost_vsock_set_running = vhost_kernel_vsock_set_running, > > > > > .vhost_set_iotlb_callback = vhost_kernel_set_iotlb_callback, > > > > > .vhost_send_device_iotlb_msg = vhost_kernel_send_device_iotlb_msg, > > > > > + .vhost_toggle_device_iotlb = vhost_kernel_toggle_device_iotlb, > > > > > }; > > > > > #endif > > > > > > > > > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > > > > > index 746d130c74..41c9fbf286 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); > > > > > - } > > > > > > > > So we lose this fallback. Is this really intended? > > > > > > > > E.g does it work if you are using virtio-iommu? > > > > > > It works for virtio-iommu because Linux guest doesn't enable PCI ATS in > > > this situation. From my point of view, this fallback is not needed > > > anymore, because it triggers only if Device-TLB isn't available on the > > > host side but the guest misbehaves and tries to enable it. > > > > Ok. > > > > > > > > Also, I would like to discuss two more points: > > > > > > 1. The patch series in its current form will fix the issue for > > > vhost+iommu setup for any VirtIO device with any vhost backend when > > > ATS is enabled at the beginning. But if ATS is enabled/disabled in the > > > runtime it will only work for virtio-net with vhost-kernel. All other > > > devices and backends are out of scope and will need to add almost the > > > same device_iotlb_toggle and vhost_device_iotlb_toggle handlers. Since > > > the issue is general for any device and any backend, is it normal from > > > architectural point of view? > > > > Yes, so I think it's better to fix others vhost backends. Actually I > > wonder if we can have simply reuse vhost_toggle_device_iotlb() since > > it's nothing specific to the vhost backend. It's just about the way to > > receive IOTLB invalidation from vIOMMU. > > > > > > > > 2. When the series will be applied, we should enable DMA remapping for > > > new Windows guest drivers, such as NetKVM. But if the user with enabled > > > vhost+iommu updated the driver with old QEMU, the bug would reappear, > > > because the guest doesn't know that the fix isn't present. May be we > > > should discuss some mechanism to report that host is aware of guest's > > > accept/reject of ATS/Device-TLB? > > > > I'm not sure how this can help? Or anything makes this fix different > > from other fixes? > > Let's imagine a user who runs Windows on QEMU with virtio-net-pci and > intel-iommu with enabled vhost, ATS and Device-TLB. > > At the moment, DMA remapping is disabled through INF in NetKVM driver, > and IOMMU is not working actually, and such a user doesn't observe the > packet corruption issue at all. > > After DMA remapping in INF will be enabled, the issue will be observed > with old QEMU. So, if such a user will not update QEMU but update the > driver, he will encounter a problem he has never had before. Exactly, but this is not specific to this bug. If we don't backport other fixes to -stable, the old Qemu will suffer from other issues for sure. Thanks > > > > > One thing I can think is to backport the fixes to -stable. > > I think, it would be nice. It doesn't solve the problem 100%, though. > > Thanks > > > > > Thanks > > > > > > > > Thanks, > > > Viktor Prutyanov > > > > > > > > > > > Thanks > > > > > > > > > + 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,19 @@ static void vhost_iommu_region_del(MemoryListener *listener, > > > > > } > > > > > } > > > > > > > > > > +void vhost_toggle_device_iotlb(struct vhost_dev *dev) > > > > > +{ > > > > > + struct vhost_iommu *iommu; > > > > > + > > > > > + QLIST_FOREACH(iommu, &dev->iommu_list, iommu_next) { > > > > > + memory_region_unregister_iommu_notifier(iommu->mr, &iommu->n); > > > > > + iommu->n.notifier_flags = dev->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-backend.h b/include/hw/virtio/vhost-backend.h > > > > > index ec3fbae58d..10a3c36b4b 100644 > > > > > --- a/include/hw/virtio/vhost-backend.h > > > > > +++ b/include/hw/virtio/vhost-backend.h > > > > > @@ -133,6 +133,8 @@ typedef int (*vhost_set_config_call_op)(struct vhost_dev *dev, > > > > > > > > > > typedef void (*vhost_reset_status_op)(struct vhost_dev *dev); > > > > > > > > > > +typedef void (*vhost_toggle_device_iotlb_op)(struct vhost_dev *dev); > > > > > + > > > > > typedef struct VhostOps { > > > > > VhostBackendType backend_type; > > > > > vhost_backend_init vhost_backend_init; > > > > > @@ -181,6 +183,7 @@ typedef struct VhostOps { > > > > > vhost_force_iommu_op vhost_force_iommu; > > > > > vhost_set_config_call_op vhost_set_config_call; > > > > > vhost_reset_status_op vhost_reset_status; > > > > > + vhost_toggle_device_iotlb_op vhost_toggle_device_iotlb; > > > > > } VhostOps; > > > > > > > > > > int vhost_backend_update_device_iotlb(struct vhost_dev *dev, > > > > > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h > > > > > index a52f273347..785832ed46 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(struct vhost_dev *dev); > > > > > 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.35.1 > > > > > > > > > > > > > > >
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c index 8e581575c9..d39bfefd2d 100644 --- a/hw/virtio/vhost-backend.c +++ b/hw/virtio/vhost-backend.c @@ -297,6 +297,11 @@ static void vhost_kernel_set_iotlb_callback(struct vhost_dev *dev, qemu_set_fd_handler((uintptr_t)dev->opaque, NULL, NULL, NULL); } +static void vhost_kernel_toggle_device_iotlb(struct vhost_dev *dev) +{ + vhost_toggle_device_iotlb(dev); +} + const VhostOps kernel_ops = { .backend_type = VHOST_BACKEND_TYPE_KERNEL, .vhost_backend_init = vhost_kernel_init, @@ -328,6 +333,7 @@ const VhostOps kernel_ops = { .vhost_vsock_set_running = vhost_kernel_vsock_set_running, .vhost_set_iotlb_callback = vhost_kernel_set_iotlb_callback, .vhost_send_device_iotlb_msg = vhost_kernel_send_device_iotlb_msg, + .vhost_toggle_device_iotlb = vhost_kernel_toggle_device_iotlb, }; #endif diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 746d130c74..41c9fbf286 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,19 @@ static void vhost_iommu_region_del(MemoryListener *listener, } } +void vhost_toggle_device_iotlb(struct vhost_dev *dev) +{ + struct vhost_iommu *iommu; + + QLIST_FOREACH(iommu, &dev->iommu_list, iommu_next) { + memory_region_unregister_iommu_notifier(iommu->mr, &iommu->n); + iommu->n.notifier_flags = dev->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-backend.h b/include/hw/virtio/vhost-backend.h index ec3fbae58d..10a3c36b4b 100644 --- a/include/hw/virtio/vhost-backend.h +++ b/include/hw/virtio/vhost-backend.h @@ -133,6 +133,8 @@ typedef int (*vhost_set_config_call_op)(struct vhost_dev *dev, typedef void (*vhost_reset_status_op)(struct vhost_dev *dev); +typedef void (*vhost_toggle_device_iotlb_op)(struct vhost_dev *dev); + typedef struct VhostOps { VhostBackendType backend_type; vhost_backend_init vhost_backend_init; @@ -181,6 +183,7 @@ typedef struct VhostOps { vhost_force_iommu_op vhost_force_iommu; vhost_set_config_call_op vhost_set_config_call; vhost_reset_status_op vhost_reset_status; + vhost_toggle_device_iotlb_op vhost_toggle_device_iotlb; } VhostOps; int vhost_backend_update_device_iotlb(struct vhost_dev *dev, diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h index a52f273347..785832ed46 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(struct vhost_dev *dev); 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,
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> --- hw/virtio/vhost-backend.c | 6 ++++++ hw/virtio/vhost.c | 30 ++++++++++++++++++------------ include/hw/virtio/vhost-backend.h | 3 +++ include/hw/virtio/vhost.h | 1 + 4 files changed, 28 insertions(+), 12 deletions(-)