ARM CCA Device Assignment support

[RFC PATCH v1 07/38] iommufd/viommu: Add support to associate viommu with kvm instance

Posted by Aneesh Kumar K.V (Arm) 2 months, 1 week ago

The associated kvm instance will be used in later patch by iommufd to
bind a tdi to kvm.

Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
---
 drivers/iommu/iommufd/viommu.c | 45 +++++++++++++++++++++++++++++++++-
 include/linux/iommufd.h        |  3 +++
 include/uapi/linux/iommufd.h   | 12 +++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommufd/viommu.c b/drivers/iommu/iommufd/viommu.c
index 2ca5809b238b..59f1e1176f7f 100644
--- a/drivers/iommu/iommufd/viommu.c
+++ b/drivers/iommu/iommufd/viommu.c
@@ -2,6 +2,36 @@
 /* Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES
  */
 #include "iommufd_private.h"
+#include "linux/tsm.h"
+
+#if IS_ENABLED(CONFIG_KVM)
+#include <linux/kvm_host.h>
+
+static int viommu_get_kvm(struct iommufd_viommu *viommu, int kvm_vm_fd)
+{
+	int rc = -EBADF;
+	struct file *filp;
+
+	filp = fget(kvm_vm_fd);
+
+	if (!file_is_kvm(filp))
+		goto err_out;
+
+	/* hold the kvm reference via file descriptor */
+	viommu->kvm_filp = filp;
+	return 0;
+err_out:
+	viommu->kvm_filp = NULL;
+	fput(filp);
+	return rc;
+}
+
+static void viommu_put_kvm(struct iommufd_viommu *viommu)
+{
+	fput(viommu->kvm_filp);
+	viommu->kvm_filp = NULL;
+}
+#endif
 
 void iommufd_viommu_destroy(struct iommufd_object *obj)
 {
@@ -12,6 +42,8 @@ void iommufd_viommu_destroy(struct iommufd_object *obj)
 		viommu->ops->destroy(viommu);
 	refcount_dec(&viommu->hwpt->common.obj.users);
 	xa_destroy(&viommu->vdevs);
+
+	viommu_put_kvm(viommu);
 }
 
 int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd)
@@ -29,7 +61,9 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd)
 	size_t viommu_size;
 	int rc;
 
-	if (cmd->flags || cmd->type == IOMMU_VIOMMU_TYPE_DEFAULT)
+	if (cmd->flags & ~IOMMU_VIOMMU_KVM_FD)
+		return -EOPNOTSUPP;
+	if (cmd->type == IOMMU_VIOMMU_TYPE_DEFAULT)
 		return -EOPNOTSUPP;
 
 	idev = iommufd_get_device(ucmd, cmd->dev_id);
@@ -100,8 +134,17 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd)
 		goto out_put_hwpt;
 	}
 
+	/* get the kvm details if specified. */
+	if (cmd->flags & IOMMU_VIOMMU_KVM_FD) {
+		rc = viommu_get_kvm(viommu, cmd->kvm_vm_fd);
+		if (rc)
+			goto out_put_hwpt;
+	}
+
 	cmd->out_viommu_id = viommu->obj.id;
 	rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd));
+	if (rc)
+		viommu_put_kvm(viommu);
 
 out_put_hwpt:
 	iommufd_put_object(ucmd->ictx, &hwpt_paging->common.obj);
diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h
index 55ae02581f9b..b7617ba7a536 100644
--- a/include/linux/iommufd.h
+++ b/include/linux/iommufd.h
@@ -12,6 +12,7 @@
 #include <linux/refcount.h>
 #include <linux/types.h>
 #include <linux/xarray.h>
+#include <linux/file.h>
 #include <uapi/linux/iommufd.h>
 
 struct device;
@@ -58,6 +59,7 @@ struct iommufd_object {
 	unsigned int id;
 };
 
+struct kvm;
 struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
 					   struct device *dev, u32 *id);
 void iommufd_device_unbind(struct iommufd_device *idev);
@@ -102,6 +104,7 @@ struct iommufd_viommu {
 	struct iommufd_ctx *ictx;
 	struct iommu_device *iommu_dev;
 	struct iommufd_hwpt_paging *hwpt;
+	struct file *kvm_filp;
 
 	const struct iommufd_viommu_ops *ops;
 
diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
index 548d4b5afcd4..9014c61a97d4 100644
--- a/include/uapi/linux/iommufd.h
+++ b/include/uapi/linux/iommufd.h
@@ -1023,6 +1023,17 @@ struct iommu_viommu_tegra241_cmdqv {
 	__aligned_u64 out_vintf_mmap_length;
 };
 
+/**
+ * define IOMMU_VIOMMU_KVM_FD - Flag indicating a valid KVM VM file descriptor
+ *
+ * This flag must be set when allocating a viommu instance that will be
+ * associated with a specific KVM VM. When allocating a viommu instance for a
+ * KVM VM, this flag must be set to inform the initialization logic that
+ * @iommu_viommu_alloc::kvm_vm_fd is properly initialized. If this flag is not
+ * provided but @iommu_viommu_alloc::kvm_vm_fd field will be ignored.
+ */
+#define IOMMU_VIOMMU_KVM_FD	BIT(0)
+
 /**
  * struct iommu_viommu_alloc - ioctl(IOMMU_VIOMMU_ALLOC)
  * @size: sizeof(struct iommu_viommu_alloc)
@@ -1057,6 +1068,7 @@ struct iommu_viommu_alloc {
 	__u32 data_len;
 	__u32 __reserved;
 	__aligned_u64 data_uptr;
+	__u32 kvm_vm_fd;
 };
 #define IOMMU_VIOMMU_ALLOC _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VIOMMU_ALLOC)
 
-- 
2.43.0

Re: [RFC PATCH v1 07/38] iommufd/viommu: Add support to associate viommu with kvm instance

Posted by Jonathan Cameron 2 months, 1 week ago

On Mon, 28 Jul 2025 19:21:44 +0530
"Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> wrote:

> The associated kvm instance will be used in later patch by iommufd to
> bind a tdi to kvm.
> 
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> ---
>  drivers/iommu/iommufd/viommu.c | 45 +++++++++++++++++++++++++++++++++-
>  include/linux/iommufd.h        |  3 +++
>  include/uapi/linux/iommufd.h   | 12 +++++++++
>  3 files changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/iommufd/viommu.c b/drivers/iommu/iommufd/viommu.c
> index 2ca5809b238b..59f1e1176f7f 100644
> --- a/drivers/iommu/iommufd/viommu.c
> +++ b/drivers/iommu/iommufd/viommu.c
> @@ -2,6 +2,36 @@
>  /* Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES
>   */
>  #include "iommufd_private.h"
> +#include "linux/tsm.h"
> +
> +#if IS_ENABLED(CONFIG_KVM)
> +#include <linux/kvm_host.h>
> +
> +static int viommu_get_kvm(struct iommufd_viommu *viommu, int kvm_vm_fd)
> +{
> +	int rc = -EBADF;
> +	struct file *filp;
> +
> +	filp = fget(kvm_vm_fd);
> +
> +	if (!file_is_kvm(filp))
> +		goto err_out;
> +
> +	/* hold the kvm reference via file descriptor */
> +	viommu->kvm_filp = filp;
> +	return 0;
> +err_out:
> +	viommu->kvm_filp = NULL;

Is this to undo side effects from this function on error?

kvm_filp is only set after all error paths so maybe this isn't
needed?

If this isn't needed then use __free(fput) and no_free_ptr() to
deal with filp more simply and in teh erorr path can just return -EBADF
directly rather than the goto.

Or are we avoiding that stuff in iommufd?

> +	fput(filp);
> +	return rc;
> +}
> +
> +static void viommu_put_kvm(struct iommufd_viommu *viommu)
> +{
> +	fput(viommu->kvm_filp);
> +	viommu->kvm_filp = NULL;
> +}
> +#endif

Re: [RFC PATCH v1 07/38] iommufd/viommu: Add support to associate viommu with kvm instance

Posted by Jason Gunthorpe 2 months, 1 week ago

On Tue, Jul 29, 2025 at 05:26:21PM +0100, Jonathan Cameron wrote:
> On Mon, 28 Jul 2025 19:21:44 +0530
> "Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> wrote:
> 
> > The associated kvm instance will be used in later patch by iommufd to
> > bind a tdi to kvm.
> > 
> > Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> > ---
> >  drivers/iommu/iommufd/viommu.c | 45 +++++++++++++++++++++++++++++++++-
> >  include/linux/iommufd.h        |  3 +++
> >  include/uapi/linux/iommufd.h   | 12 +++++++++
> >  3 files changed, 59 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iommu/iommufd/viommu.c b/drivers/iommu/iommufd/viommu.c
> > index 2ca5809b238b..59f1e1176f7f 100644
> > --- a/drivers/iommu/iommufd/viommu.c
> > +++ b/drivers/iommu/iommufd/viommu.c
> > @@ -2,6 +2,36 @@
> >  /* Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES
> >   */
> >  #include "iommufd_private.h"
> > +#include "linux/tsm.h"
> > +
> > +#if IS_ENABLED(CONFIG_KVM)
> > +#include <linux/kvm_host.h>
> > +
> > +static int viommu_get_kvm(struct iommufd_viommu *viommu, int kvm_vm_fd)
> > +{
> > +	int rc = -EBADF;
> > +	struct file *filp;
> > +
> > +	filp = fget(kvm_vm_fd);
> > +
> > +	if (!file_is_kvm(filp))
> > +		goto err_out;
> > +
> > +	/* hold the kvm reference via file descriptor */
> > +	viommu->kvm_filp = filp;
> > +	return 0;
> > +err_out:
> > +	viommu->kvm_filp = NULL;
> 
> Is this to undo side effects from this function on error?
> 
> kvm_filp is only set after all error paths so maybe this isn't
> needed?

Looks like you are right to me

> If this isn't needed then use __free(fput) and no_free_ptr() to
> deal with filp more simply and in teh erorr path can just return -EBADF
> directly rather than the goto.
> 
> Or are we avoiding that stuff in iommufd?

Nope, gentle obvious use is fine :)

Jason

Re: [RFC PATCH v1 07/38] iommufd/viommu: Add support to associate viommu with kvm instance

Posted by Jason Gunthorpe 2 months, 1 week ago

On Mon, Jul 28, 2025 at 07:21:44PM +0530, Aneesh Kumar K.V (Arm) wrote:

> +#if IS_ENABLED(CONFIG_KVM)
> +#include <linux/kvm_host.h>
> +
> +static int viommu_get_kvm(struct iommufd_viommu *viommu, int kvm_vm_fd)
> +{
> +	int rc = -EBADF;
> +	struct file *filp;
> +
> +	filp = fget(kvm_vm_fd);
> +
> +	if (!file_is_kvm(filp))
> +		goto err_out;
> +
> +	/* hold the kvm reference via file descriptor */
> +	viommu->kvm_filp = filp;
> +	return 0;
> +err_out:
> +	viommu->kvm_filp = NULL;
> +	fput(filp);
> +	return rc;
> +}
> +
> +static void viommu_put_kvm(struct iommufd_viommu *viommu)
> +{
> +	fput(viommu->kvm_filp);
> +	viommu->kvm_filp = NULL;
> +}
> +#endif

Missing stub functions for !CONFIG_KVM?

Looks like an OK design otherwise

> @@ -1057,6 +1068,7 @@ struct iommu_viommu_alloc {
>  	__u32 data_len;
>  	__u32 __reserved;
>  	__aligned_u64 data_uptr;
> +	__u32 kvm_vm_fd;

fds are __s32, they are signed numbers.

Jason

Re: [RFC PATCH v1 07/38] iommufd/viommu: Add support to associate viommu with kvm instance

Posted by Aneesh Kumar K.V 2 months, 1 week ago

Jason Gunthorpe <jgg@ziepe.ca> writes:

> On Mon, Jul 28, 2025 at 07:21:44PM +0530, Aneesh Kumar K.V (Arm) wrote:
>
>> +#if IS_ENABLED(CONFIG_KVM)
>> +#include <linux/kvm_host.h>
>> +
>> +static int viommu_get_kvm(struct iommufd_viommu *viommu, int kvm_vm_fd)
>> +{
>> +	int rc = -EBADF;
>> +	struct file *filp;
>> +
>> +	filp = fget(kvm_vm_fd);
>> +
>> +	if (!file_is_kvm(filp))
>> +		goto err_out;
>> +
>> +	/* hold the kvm reference via file descriptor */
>> +	viommu->kvm_filp = filp;
>> +	return 0;
>> +err_out:
>> +	viommu->kvm_filp = NULL;
>> +	fput(filp);
>> +	return rc;
>> +}
>> +
>> +static void viommu_put_kvm(struct iommufd_viommu *viommu)
>> +{
>> +	fput(viommu->kvm_filp);
>> +	viommu->kvm_filp = NULL;
>> +}
>> +#endif
>
> Missing stub functions for !CONFIG_KVM?
>
> Looks like an OK design otherwise
>
>> @@ -1057,6 +1068,7 @@ struct iommu_viommu_alloc {
>>  	__u32 data_len;
>>  	__u32 __reserved;
>>  	__aligned_u64 data_uptr;
>> +	__u32 kvm_vm_fd;
>
> fds are __s32, they are signed numbers.
>

Thanks for the review comments. I’ll update the patch with the suggested changes.

-aneesh