On Thu, 6 Oct 2022 09:40:36 -0300
Jason Gunthorpe <jgg@xxxxxxxxxx> wrote:
> This replaces uses of vfio_file_iommu_group() which were only detecting if
> the file is a VFIO file with no interest in the actual group.
>
> The only remaning user of vfio_file_iommu_group() is in KVM for the SPAPR
> stuff. It passes the iommu_group into the arch code through kvm for some
> reason.
>
> Tested-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>
> Tested-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
> Tested-by: Eric Farman <farman@xxxxxxxxxxxxx>
> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> ---
> drivers/vfio/pci/vfio_pci_core.c | 2 +-
> drivers/vfio/vfio_main.c | 14 ++++++++++++++
> include/linux/vfio.h | 1 +
> virt/kvm/vfio.c | 20 ++++++++++++++++++--
> 4 files changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 59a28251bb0b97..badc9d828cac20 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -1313,7 +1313,7 @@ static int vfio_pci_ioctl_pci_hot_reset(struct vfio_pci_core_device *vdev,
> }
>
> /* Ensure the FD is a vfio group FD.*/
> - if (!vfio_file_iommu_group(file)) {
> + if (!vfio_file_is_group(file)) {
> fput(file);
> ret = -EINVAL;
> break;
> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index 9207e6c0e3cb26..7866849be56ef6 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
> @@ -1553,17 +1553,31 @@ static const struct file_operations vfio_device_fops = {
> * @file: VFIO group file
> *
> * The returned iommu_group is valid as long as a ref is held on the file.
> + * This function is deprecated, only the SPAPR path in kvm should call it.
> */
> struct iommu_group *vfio_file_iommu_group(struct file *file)
> {
> struct vfio_group *group = file->private_data;
>
> + if (!IS_ENABLED(CONFIG_SPAPR_TCE_IOMMU))
> + return NULL;
> +
> if (file->f_op != &vfio_group_fops)
Nit, with the function below, shouldn't the line above become:
if (!vfio_file_is_group(file))
Thanks,
Alex
> return NULL;
> return group->iommu_group;
> }
> EXPORT_SYMBOL_GPL(vfio_file_iommu_group);
>
> +/**
> + * vfio_file_is_group - True if the file is usable with VFIO aPIS
> + * @file: VFIO group file
> + */
> +bool vfio_file_is_group(struct file *file)
> +{
> + return file->f_op == &vfio_group_fops;
> +}
> +EXPORT_SYMBOL_GPL(vfio_file_is_group);
> +
> /**
> * vfio_file_enforced_coherent - True if the DMA associated with the VFIO file
> * is always CPU cache coherent
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index ee399a768070d0..e7cebeb875dd1a 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -199,6 +199,7 @@ int vfio_mig_get_next_state(struct vfio_device *device,
> * External user API
> */
> struct iommu_group *vfio_file_iommu_group(struct file *file);
> +bool vfio_file_is_group(struct file *file);
> bool vfio_file_enforced_coherent(struct file *file);
> void vfio_file_set_kvm(struct file *file, struct kvm *kvm);
> bool vfio_file_has_dev(struct file *file, struct vfio_device *device);
> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
> index ce1b01d02c5197..54aec3b0559c70 100644
> --- a/virt/kvm/vfio.c
> +++ b/virt/kvm/vfio.c
> @@ -61,6 +61,23 @@ static bool kvm_vfio_file_enforced_coherent(struct file *file)
> return ret;
> }
>
> +static bool kvm_vfio_file_is_group(struct file *file)
> +{
> + bool (*fn)(struct file *file);
> + bool ret;
> +
> + fn = symbol_get(vfio_file_is_group);
> + if (!fn)
> + return false;
> +
> + ret = fn(file);
> +
> + symbol_put(vfio_file_is_group);
> +
> + return ret;
> +}
> +
> +#ifdef CONFIG_SPAPR_TCE_IOMMU
> static struct iommu_group *kvm_vfio_file_iommu_group(struct file *file)
> {
> struct iommu_group *(*fn)(struct file *file);
> @@ -77,7 +94,6 @@ static struct iommu_group *kvm_vfio_file_iommu_group(struct file *file)
> return ret;
> }
>
> -#ifdef CONFIG_SPAPR_TCE_IOMMU
> static void kvm_spapr_tce_release_vfio_group(struct kvm *kvm,
> struct kvm_vfio_group *kvg)
> {
> @@ -136,7 +152,7 @@ static int kvm_vfio_group_add(struct kvm_device *dev, unsigned int fd)
> return -EBADF;
>
> /* Ensure the FD is a vfio group FD.*/
> - if (!kvm_vfio_file_iommu_group(filp)) {
> + if (!kvm_vfio_file_is_group(filp)) {
> ret = -EINVAL;
> goto err_fput;
> }
