Re: [PATCH v2 2/2] vhost-vdpa: introduce NO-IOMMU backend feature bit

Jason Wang
2024-09-24T07:43:48Z

On Fri, Sep 20, 2024 at 10:05 PM Srujana Challa <schalla@marvell.com> wrote:
>
> This patch introduces the VHOST_BACKEND_F_NOIOMMU feature flag.
> This flag allows userspace to identify if the driver can operate
> without an IOMMU, providing more flexibility in environments where
> IOMMU is not available or desired.
>
> Key changes include:
> - Addition of the VHOST_BACKEND_F_NOIOMMU feature flag.
> - Updates to vhost_vdpa_unlocked_ioctl to handle the NO-IOMMU
> feature.
> The NO-IOMMU mode is enabled if:
> - The vdpa device lacks an IOMMU domain.
> - The system has the required RAWIO permissions.
> - The vdpa device explicitly supports NO-IOMMU mode.
>
> This feature flag indicates to userspace that the driver can safely
> operate in NO-IOMMU mode. If the flag is absent, userspace should
> assume NO-IOMMU mode is unsupported and take appropriate actions.

This seems contradictory to what you said in patch 1

"""
When enabled, this mode provides no
device isolation, no DMA translation, no host kernel protection, and
cannot be used for device assignment to virtual machines.
"""

And I wonder what "appropriate actions" could the userspace take?
Generally, the IOMMU concept should be hidden from the userspace.

Thanks

>
> Signed-off-by: Srujana Challa <schalla@marvell.com>
> ---
> drivers/vhost/vdpa.c | 11 ++++++++++-
> include/uapi/linux/vhost_types.h | 2 ++
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index b3085189ea4a..de47349eceff 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -797,7 +797,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
> BIT_ULL(VHOST_BACKEND_F_IOTLB_PERSIST) |
> BIT_ULL(VHOST_BACKEND_F_SUSPEND) |
> BIT_ULL(VHOST_BACKEND_F_RESUME) |
> - BIT_ULL(VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK)))
> + BIT_ULL(VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK) |
> + BIT_ULL(VHOST_BACKEND_F_NOIOMMU)))
> return -EOPNOTSUPP;
> if ((features & BIT_ULL(VHOST_BACKEND_F_SUSPEND)) &&
> !vhost_vdpa_can_suspend(v))
> @@ -814,6 +815,12 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
> if ((features & BIT_ULL(VHOST_BACKEND_F_IOTLB_PERSIST)) &&
> !vhost_vdpa_has_persistent_map(v))
> return -EOPNOTSUPP;
> + if ((features & BIT_ULL(VHOST_BACKEND_F_NOIOMMU)) &&
> + !v->noiommu_en)
> + return -EOPNOTSUPP;
> + if (!(features & BIT_ULL(VHOST_BACKEND_F_NOIOMMU)) &&
> + v->noiommu_en)
> + return -EOPNOTSUPP;
> vhost_set_backend_features(&v->vdev, features);
> return 0;
> }
> @@ -871,6 +878,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
> features |= BIT_ULL(VHOST_BACKEND_F_DESC_ASID);
> if (vhost_vdpa_has_persistent_map(v))
> features |= BIT_ULL(VHOST_BACKEND_F_IOTLB_PERSIST);
> + if (v->noiommu_en)
> + features |= BIT_ULL(VHOST_BACKEND_F_NOIOMMU);
> features |= vhost_vdpa_get_backend_features(v);
> 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 d7656908f730..dda673c3456a 100644
> --- a/include/uapi/linux/vhost_types.h
> +++ b/include/uapi/linux/vhost_types.h
> @@ -192,5 +192,7 @@ struct vhost_vdpa_iova_range {
> #define VHOST_BACKEND_F_DESC_ASID 0x7
> /* IOTLB don't flush memory mapping across device reset */
> #define VHOST_BACKEND_F_IOTLB_PERSIST 0x8
> +/* Enables the device to operate in NO-IOMMU mode as well */
> +#define VHOST_BACKEND_F_NOIOMMU 0x9
>
> #endif
> --
> 2.25.1
>