Skip to content

Commit

Permalink
kvm/vfio: Remove vfio_group from kvm
Browse files Browse the repository at this point in the history
None of the VFIO APIs take in the vfio_group anymore, so we can remove it
completely.

This has a subtle side effect on the enforced coherency tracking. The
vfio_group_get_external_user() was holding on to the container_users which
would prevent the iommu_domain and thus the enforced coherency value from
changing while the group is registered with kvm.

It changes the security proof slightly into 'user must hold a group FD
that has a device that cannot enforce DMA coherence'. As opening the group
FD, not attaching the container, is the privileged operation this doesn't
change the security properties much.

On the flip side it paves the way to changing the iommu_domain/container
attached to a group at runtime which is something that will be required to
support nested translation.

Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>i
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Link: https://lore.kernel.org/r/7-v3-f7729924a7ea+25e33-vfio_kvm_no_group_jgg@nvidia.com
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
  • Loading branch information
Jason Gunthorpe authored and Alex Williamson committed May 13, 2022
1 parent ba70a89 commit 3e5449d
Showing 1 changed file with 8 additions and 43 deletions.
51 changes: 8 additions & 43 deletions virt/kvm/vfio.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
struct kvm_vfio_group {
struct list_head node;
struct file *file;
struct vfio_group *vfio_group;
};

struct kvm_vfio {
Expand All @@ -33,35 +32,6 @@ struct kvm_vfio {
bool noncoherent;
};

static struct vfio_group *kvm_vfio_group_get_external_user(struct file *filep)
{
struct vfio_group *vfio_group;
struct vfio_group *(*fn)(struct file *);

fn = symbol_get(vfio_group_get_external_user);
if (!fn)
return ERR_PTR(-EINVAL);

vfio_group = fn(filep);

symbol_put(vfio_group_get_external_user);

return vfio_group;
}

static void kvm_vfio_group_put_external_user(struct vfio_group *vfio_group)
{
void (*fn)(struct vfio_group *);

fn = symbol_get(vfio_group_put_external_user);
if (!fn)
return;

fn(vfio_group);

symbol_put(vfio_group_put_external_user);
}

static void kvm_vfio_file_set_kvm(struct file *file, struct kvm *kvm)
{
void (*fn)(struct file *file, struct kvm *kvm);
Expand Down Expand Up @@ -91,7 +61,6 @@ static bool kvm_vfio_file_enforced_coherent(struct file *file)
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);
Expand All @@ -108,6 +77,7 @@ 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)
{
Expand Down Expand Up @@ -157,7 +127,6 @@ static void kvm_vfio_update_coherency(struct kvm_device *dev)
static int kvm_vfio_group_add(struct kvm_device *dev, unsigned int fd)
{
struct kvm_vfio *kv = dev->private;
struct vfio_group *vfio_group;
struct kvm_vfio_group *kvg;
struct file *filp;
int ret;
Expand All @@ -166,6 +135,12 @@ static int kvm_vfio_group_add(struct kvm_device *dev, unsigned int fd)
if (!filp)
return -EBADF;

/* Ensure the FD is a vfio group FD.*/
if (!kvm_vfio_file_iommu_group(filp)) {
ret = -EINVAL;
goto err_fput;
}

mutex_lock(&kv->lock);

list_for_each_entry(kvg, &kv->group_list, node) {
Expand All @@ -181,15 +156,8 @@ static int kvm_vfio_group_add(struct kvm_device *dev, unsigned int fd)
goto err_unlock;
}

vfio_group = kvm_vfio_group_get_external_user(filp);
if (IS_ERR(vfio_group)) {
ret = PTR_ERR(vfio_group);
goto err_free;
}

kvg->file = filp;
list_add_tail(&kvg->node, &kv->group_list);
kvg->vfio_group = vfio_group;

kvm_arch_start_assignment(dev->kvm);

Expand All @@ -199,10 +167,9 @@ static int kvm_vfio_group_add(struct kvm_device *dev, unsigned int fd)
kvm_vfio_update_coherency(dev);

return 0;
err_free:
kfree(kvg);
err_unlock:
mutex_unlock(&kv->lock);
err_fput:
fput(filp);
return ret;
}
Expand Down Expand Up @@ -232,7 +199,6 @@ static int kvm_vfio_group_del(struct kvm_device *dev, unsigned int fd)
kvm_spapr_tce_release_vfio_group(dev->kvm, kvg);
#endif
kvm_vfio_file_set_kvm(kvg->file, NULL);
kvm_vfio_group_put_external_user(kvg->vfio_group);
fput(kvg->file);
kfree(kvg);
ret = 0;
Expand Down Expand Up @@ -361,7 +327,6 @@ static void kvm_vfio_destroy(struct kvm_device *dev)
kvm_spapr_tce_release_vfio_group(dev->kvm, kvg);
#endif
kvm_vfio_file_set_kvm(kvg->file, NULL);
kvm_vfio_group_put_external_user(kvg->vfio_group);
fput(kvg->file);
list_del(&kvg->node);
kfree(kvg);
Expand Down

0 comments on commit 3e5449d

Please sign in to comment.