Skip to content

Commit 4398776

Browse files
siwliu-kernelmstsirkin
authored andcommitted
vhost-vdpa: introduce IOTLB_PERSIST backend feature bit
Userspace needs this feature flag to distinguish if vhost-vdpa iotlb in the kernel can be trusted to persist IOTLB mapping across vDPA reset. Without it, userspace has no way to tell apart if it's running on an older kernel, which could silently drop all iotlb mapping across vDPA reset, especially with broken parent driver implementation for the .reset driver op. The broken driver may incorrectly drop all mappings of its own as part of .reset, which inadvertently ends up with corrupted mapping state between vhost-vdpa userspace and the kernel. As a workaround, to make the mapping behaviour predictable across reset, userspace has to pro-actively remove all mappings before vDPA reset, and then restore all the mappings afterwards. This workaround is done unconditionally on top of all parent drivers today, due to the parent driver implementation issue and no means to differentiate. This workaround had been utilized in QEMU since day one when the corresponding vhost-vdpa userspace backend came to the world. There are 3 cases that backend may claim this feature bit on for: - parent device that has to work with platform IOMMU - parent device with on-chip IOMMU that has the expected .reset_map support in driver - parent device with vendor specific IOMMU implementation with persistent IOTLB mapping already that has to specifically declare this backend feature The reason why .reset_map is being one of the pre-condition for persistent iotlb is because without it, vhost-vdpa can't switch back iotlb to the initial state later on, especially for the on-chip IOMMU case which starts with identity mapping at device creation. virtio-vdpa requires on-chip IOMMU to perform 1:1 passthrough translation from PA to IOVA as-is to begin with, and .reset_map is the only means to turn back iotlb to the identity mapping mode after vhost-vdpa is gone. The difference in behavior did not matter as QEMU unmaps all the memory unregistering the memory listener at vhost_vdpa_dev_start( started = false), but the backend acknowledging this feature flag allows QEMU to make sure it is safe to skip this unmap & map in the case of vhost stop & start cycle. In that sense, this feature flag is actually a signal for userspace to know that the driver bug has been solved. Not offering it indicates that userspace cannot trust the kernel will retain the maps. Signed-off-by: Si-Wei Liu <[email protected]> Acked-by: Eugenio Pérez <[email protected]> Message-Id: <[email protected]> Signed-off-by: Michael S. Tsirkin <[email protected]> Tested-by: Lei Yang <[email protected]>
1 parent 1d0f874 commit 4398776

File tree

2 files changed

+17
-0
lines changed

2 files changed

+17
-0
lines changed

drivers/vhost/vdpa.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,15 @@ static u64 vhost_vdpa_get_backend_features(const struct vhost_vdpa *v)
439439
return ops->get_backend_features(vdpa);
440440
}
441441

442+
static bool vhost_vdpa_has_persistent_map(const struct vhost_vdpa *v)
443+
{
444+
struct vdpa_device *vdpa = v->vdpa;
445+
const struct vdpa_config_ops *ops = vdpa->config;
446+
447+
return (!ops->set_map && !ops->dma_map) || ops->reset_map ||
448+
vhost_vdpa_get_backend_features(v) & BIT_ULL(VHOST_BACKEND_F_IOTLB_PERSIST);
449+
}
450+
442451
static long vhost_vdpa_set_features(struct vhost_vdpa *v, u64 __user *featurep)
443452
{
444453
struct vdpa_device *vdpa = v->vdpa;
@@ -726,6 +735,7 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
726735
return -EFAULT;
727736
if (features & ~(VHOST_VDPA_BACKEND_FEATURES |
728737
BIT_ULL(VHOST_BACKEND_F_DESC_ASID) |
738+
BIT_ULL(VHOST_BACKEND_F_IOTLB_PERSIST) |
729739
BIT_ULL(VHOST_BACKEND_F_SUSPEND) |
730740
BIT_ULL(VHOST_BACKEND_F_RESUME) |
731741
BIT_ULL(VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK)))
@@ -742,6 +752,9 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
742752
if ((features & BIT_ULL(VHOST_BACKEND_F_DESC_ASID)) &&
743753
!vhost_vdpa_has_desc_group(v))
744754
return -EOPNOTSUPP;
755+
if ((features & BIT_ULL(VHOST_BACKEND_F_IOTLB_PERSIST)) &&
756+
!vhost_vdpa_has_persistent_map(v))
757+
return -EOPNOTSUPP;
745758
vhost_set_backend_features(&v->vdev, features);
746759
return 0;
747760
}
@@ -797,6 +810,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
797810
features |= BIT_ULL(VHOST_BACKEND_F_RESUME);
798811
if (vhost_vdpa_has_desc_group(v))
799812
features |= BIT_ULL(VHOST_BACKEND_F_DESC_ASID);
813+
if (vhost_vdpa_has_persistent_map(v))
814+
features |= BIT_ULL(VHOST_BACKEND_F_IOTLB_PERSIST);
800815
features |= vhost_vdpa_get_backend_features(v);
801816
if (copy_to_user(featurep, &features, sizeof(features)))
802817
r = -EFAULT;

include/uapi/linux/vhost_types.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,5 +190,7 @@ struct vhost_vdpa_iova_range {
190190
* buffers may reside. Requires VHOST_BACKEND_F_IOTLB_ASID.
191191
*/
192192
#define VHOST_BACKEND_F_DESC_ASID 0x7
193+
/* IOTLB don't flush memory mapping across device reset */
194+
#define VHOST_BACKEND_F_IOTLB_PERSIST 0x8
193195

194196
#endif

0 commit comments

Comments
 (0)