diff mbox series

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

Message ID 20240920140530.775307-3-schalla@marvell.com (mailing list archive)
State New, archived
Headers show
Series vhost-vdpa: Add support for NO-IOMMU mode | expand

Commit Message

Srujana Challa Sept. 20, 2024, 2:05 p.m. UTC
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.

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(-)

Comments

Jason Wang Sept. 24, 2024, 7:43 a.m. UTC | #1
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
>
Srujana Challa Sept. 24, 2024, 10:01 a.m. UTC | #2
> 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?
I apologize, I should have stated that this feature flag indicates to userspace that the driver
can operate in NO-IOMMU mode. If the flag is absent, userspace should treat NO-IOMMU mode
as unsupported and refrain from proceeding when IOMMU is not present.

> Generally, the IOMMU concept should be hidden from the userspace.
The userspace framework, like DPDK, can determine the presence of IOMMU, enabling it to
choose between IOVA and PA as needed.  Since this is aimed at embedded use cases,
to be more secure, both Kernel VDPA driver and userspace app should require to establish
support for NO-IOMMU like vfio-noiommu.
Thanks.
> 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
> >
diff mbox series

Patch

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