Re: [PATCH v2 1/3] iommufd/viommu: Allow associating a KVM VM fd with a vIOMMU

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Mon,  9 Mar 2026 16:47:02 +0530
"Aneesh Kumar K.V (Arm)" <aneesh.kumar@kernel.org> wrote:

> Add optional KVM association to IOMMU_VIOMMU_ALLOC by introducing
> IOMMU_VIOMMU_KVM_FD and iommu_viommu_alloc::kvm_vm_fd.
> 
> When the flag is set, iommufd validates that kvm_vm_fd refers to a KVM
> VM file and stores a referenced struct file in the vIOMMU object, so
> later iommufd operations can safely resolve the owning VM.
> 
> This is preparatory plumbing for subsequent patches that bind TDI state
> to the associated KVM VM.
> 
> The patch also switch file_is_kvm from EXPORT_SYMBOL_FOR_KVM_INTERNAL to
> EXPORT_SYMBOL_GPL so that iommu module can use that.
> 
> Cc: Kevin Tian <kevin.tian@intel.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Will Deacon <will@kernel.org>
> Cc: Bjorn Helgaas <helgaas@kernel.org>
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Cc: Samuel Ortiz <sameo@rivosinc.com>
> Cc: Xu Yilun <yilun.xu@linux.intel.com>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Suzuki K Poulose <Suzuki.Poulose@arm.com>
> Cc: Steven Price <steven.price@arm.com>

Hi Aneesh,

Given we don't generally want a massive cc list in the git log,
move them below ---

If you are tracking them in your git tree, just add a --- after your sign off
and put them under that.

Also good to +CC everyone in the CC for patches on the cover letter.
I happened not be subscribed to any of the lists that went to so have to
pull it down by hand from lore.

Fairly minor stuff inline.

Thanks,

Jonathan

> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> ---
>  drivers/iommu/iommufd/viommu.c | 54 +++++++++++++++++++++++++++++++++-
>  include/linux/iommufd.h        |  3 ++
>  include/uapi/linux/iommufd.h   | 13 +++++++-
>  virt/kvm/kvm_main.c            |  2 +-
>  4 files changed, 69 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/iommufd/viommu.c b/drivers/iommu/iommufd/viommu.c
> index 4081deda9b33..08f8930c86da 100644
> --- a/drivers/iommu/iommufd/viommu.c
> +++ b/drivers/iommu/iommufd/viommu.c
> @@ -2,6 +2,45 @@
>  /* Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES
>   */
>  #include "iommufd_private.h"
> +#include <linux/cleanup.h>

Usually put linux includes before the local ones (though I haven't
checked local conventions in iommufd).

> +
> +#if IS_ENABLED(CONFIG_KVM)

Hmm. Really a question for Jason / Kevin, but so far this file is ifdef free.
Should we keep it that way by spinning this stuff off to a separate .c file,
or maybe use if (IS_ENABLED()) the implementation which I think means
we need a stub for file_is_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 __free(fput) = fget(kvm_vm_fd);
> +
> +	if (!file_is_kvm(filp))
> +		return rc;

		return -EBADF;

and drop the local variable.

> +
> +	/* hold the kvm reference via file descriptor */
> +	viommu->kvm_filp = no_free_ptr(filp);
> +	return 0;
> +}
> +
> +static void viommu_put_kvm(struct iommufd_viommu *viommu)
> +{
> +	if (!viommu->kvm_filp)
> +		return;
> +
> +	fput(viommu->kvm_filp);
> +	viommu->kvm_filp = NULL;
> +}
> +
> +#else
> +
> +static inline int viommu_get_kvm(struct iommufd_viommu *viommu, int kvm_vm_fd)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline void viommu_put_kvm(struct iommufd_viommu *viommu)
> +{
> +}
> +
> +#endif

> diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h
> index 6e7efe83bc5d..7c515d3c52db 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>

Just based on context in this patch, this smells alphabetical.
I checked it is, so this belongs between errno.h and iommu.h


>  #include <uapi/linux/iommufd.h>
>