Re: [PATCH RFC v3 4/4] vhost-vdpa: introduce IOTLB_PERSIST backend feature bit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On 9/10/2023 8:52 PM, Jason Wang wrote:
On Sat, Sep 9, 2023 at 9:46 PM Si-Wei Liu <si-wei.liu@xxxxxxxxxx> wrote:
Userspace needs this feature flag to distinguish if vhost-vdpa
iotlb in the kernel supports persistent IOTLB mapping across
device reset.
As discussed, the IOTLB persists for devices with platform IOMMU at
least. You've mentioned that this behaviour is covered by Qemu since
it reset IOTLB on reset. I wonder what happens if we simply decouple
the IOTLB reset from vDPA reset in Qemu. Could we meet bugs there?
Not sure I understand. Simple decouple meaning to remove memory_listener registration/unregistration calls *unconditionally* from the vDPA dev start/stop path in QEMU, or make it conditional around the existence of PERSIST_IOTLB? If unconditional, it breaks older host kernel, as the older kernel still silently drops all mapping across vDPA reset (VM reboot), ending up with network loss afterwards; if make the QEMU change conditional on PERSIST_IOTLB without the .reset_map API, it can't cover the virtio-vdpa 1:1 identity mapping case.

Btw, is there a Qemu patch for reference for this new feature?
There's a WIP version, not ready yet for review:
branch: vdpa-svq-asid-poc

Will need to clean up code and split to smaller patches before I can post it, if the kernel part can be settled.



There are two cases that backend may claim
this feature bit on:

- parent device that has to work with platform IOMMU
- parent device with on-chip IOMMU that has the expected
   .reset_map support in driver

Signed-off-by: Si-Wei Liu <si-wei.liu@xxxxxxxxxx>
RFC v2 -> v3:
   - fix missing return due to merge error

  drivers/vhost/vdpa.c             | 16 +++++++++++++++-
  include/uapi/linux/vhost_types.h |  2 ++
  2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 71fbd559..b404504 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -414,6 +414,14 @@ static bool vhost_vdpa_has_desc_group(const struct vhost_vdpa *v)
         return ops->get_vq_desc_group;

+static bool vhost_vdpa_has_persistent_map(const struct vhost_vdpa *v)
+       struct vdpa_device *vdpa = v->vdpa;
+       const struct vdpa_config_ops *ops = vdpa->config;
+       return (!ops->set_map && !ops->dma_map) || ops->reset_map;
  static long vhost_vdpa_get_features(struct vhost_vdpa *v, u64 __user *featurep)
         struct vdpa_device *vdpa = v->vdpa;
@@ -716,7 +724,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
                 if (features & ~(VHOST_VDPA_BACKEND_FEATURES |
                                  BIT_ULL(VHOST_BACKEND_F_DESC_ASID) |
                                  BIT_ULL(VHOST_BACKEND_F_SUSPEND) |
-                                BIT_ULL(VHOST_BACKEND_F_RESUME)))
+                                BIT_ULL(VHOST_BACKEND_F_RESUME) |
+                                BIT_ULL(VHOST_BACKEND_F_IOTLB_PERSIST)))
                         return -EOPNOTSUPP;
                 if ((features & BIT_ULL(VHOST_BACKEND_F_SUSPEND)) &&
@@ -730,6 +739,9 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
                 if ((features & BIT_ULL(VHOST_BACKEND_F_DESC_ASID)) &&
                         return -EOPNOTSUPP;
+               if ((features & BIT_ULL(VHOST_BACKEND_F_IOTLB_PERSIST)) &&
+                    !vhost_vdpa_has_persistent_map(v))
+                       return -EOPNOTSUPP;
                 vhost_set_backend_features(&v->vdev, features);
                 return 0;
@@ -785,6 +797,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
                         features |= BIT_ULL(VHOST_BACKEND_F_RESUME);
                 if (vhost_vdpa_has_desc_group(v))
                         features |= BIT_ULL(VHOST_BACKEND_F_DESC_ASID);
+               if (vhost_vdpa_has_persistent_map(v))
+                       features |= BIT_ULL(VHOST_BACKEND_F_IOTLB_PERSIST);
                 if (copy_to_user(featurep, &features, sizeof(features)))
                         r = -EFAULT;
diff --git a/include/uapi/linux/vhost_types.h b/include/uapi/linux/vhost_types.h
index 6acc604..0fdb6f0 100644
--- a/include/uapi/linux/vhost_types.h
+++ b/include/uapi/linux/vhost_types.h
@@ -186,5 +186,7 @@ struct vhost_vdpa_iova_range {
   * buffers may reside. Requires VHOST_BACKEND_F_IOTLB_ASID.
  #define VHOST_BACKEND_F_DESC_ASID    0x6
+/* IOTLB don't flush memory mapping across device reset */


Virtualization mailing list

[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux