From: Jason Gunthorpe <jgg@nvidia.com>
To: unlisted-recipients:; (no To-header on input)
Cc: Tony Krowiak <akrowiak@linux.ibm.com>,
Eric Farman <farman@linux.ibm.com>,
Christoph Hellwig <hch@lst.de>,
intel-gfx@lists.freedesktop.org,
intel-gvt-dev@lists.freedesktop.org,
"Jason J. Herne" <jjherne@linux.ibm.com>,
"Tian, Kevin" <kevin.tian@intel.com>,
kvm@vger.kernel.org, "Liu, Yi L" <yi.l.liu@intel.com>
Subject: [PATCH v3 7/7] vfio: Remove calls to vfio_group_add_container_user()
Date: Mon, 2 May 2022 14:31:37 -0300 [thread overview]
Message-ID: <7-v3-e131a9b6b467+14b6-vfio_mdev_no_group_jgg@nvidia.com> (raw)
In-Reply-To: <0-v3-e131a9b6b467+14b6-vfio_mdev_no_group_jgg@nvidia.com>
When the open_device() op is called the container_users is incremented and
held incremented until close_device(). Thus, so long as drivers call
functions within their open_device()/close_device() region they do not
need to worry about the container_users.
These functions can all only be called between open_device() and
close_device():
vfio_pin_pages()
vfio_unpin_pages()
vfio_dma_rw()
vfio_register_notifier()
vfio_unregister_notifier()
Eliminate the calls to vfio_group_add_container_user() and add
vfio_assert_device_open() to detect driver mis-use. This causes the
close_device() op to check device->open_count so always leave it elevated
while calling the op.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
drivers/vfio/vfio.c | 78 +++++++++------------------------------------
1 file changed, 15 insertions(+), 63 deletions(-)
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 0184d760ec1e44..f7d1898129ad1c 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1115,6 +1115,12 @@ static int vfio_group_add_container_user(struct vfio_group *group)
static const struct file_operations vfio_device_fops;
+/* true if the vfio_device has open_device() called but not close_device() */
+static bool vfio_assert_device_open(struct vfio_device *device)
+{
+ return !WARN_ON_ONCE(!READ_ONCE(device->open_count));
+}
+
static int vfio_group_get_device_fd(struct vfio_group *group, char *buf)
{
struct vfio_device *device;
@@ -1329,8 +1335,10 @@ static int vfio_device_fops_release(struct inode *inode, struct file *filep)
struct vfio_device *device = filep->private_data;
mutex_lock(&device->dev_set->lock);
- if (!--device->open_count && device->ops->close_device)
+ vfio_assert_device_open(device);
+ if (device->open_count == 1 && device->ops->close_device)
device->ops->close_device(device);
+ device->open_count--;
mutex_unlock(&device->dev_set->lock);
module_put(device->dev->driver->owner);
@@ -1897,7 +1905,7 @@ int vfio_pin_pages(struct vfio_device *vdev, unsigned long *user_pfn, int npage,
struct vfio_iommu_driver *driver;
int ret;
- if (!user_pfn || !phys_pfn || !npage)
+ if (!user_pfn || !phys_pfn || !npage || !vfio_assert_device_open(vdev))
return -EINVAL;
if (npage > VFIO_PIN_PAGES_MAX_ENTRIES)
@@ -1906,10 +1914,6 @@ int vfio_pin_pages(struct vfio_device *vdev, unsigned long *user_pfn, int npage,
if (group->dev_counter > 1)
return -EINVAL;
- ret = vfio_group_add_container_user(group);
- if (ret)
- return ret;
-
container = group->container;
driver = container->iommu_driver;
if (likely(driver && driver->ops->pin_pages))
@@ -1919,8 +1923,6 @@ int vfio_pin_pages(struct vfio_device *vdev, unsigned long *user_pfn, int npage,
else
ret = -ENOTTY;
- vfio_group_try_dissolve_container(group);
-
return ret;
}
EXPORT_SYMBOL(vfio_pin_pages);
@@ -1941,16 +1943,12 @@ int vfio_unpin_pages(struct vfio_device *vdev, unsigned long *user_pfn,
struct vfio_iommu_driver *driver;
int ret;
- if (!user_pfn || !npage)
+ if (!user_pfn || !npage || !vfio_assert_device_open(vdev))
return -EINVAL;
if (npage > VFIO_PIN_PAGES_MAX_ENTRIES)
return -E2BIG;
- ret = vfio_group_add_container_user(vdev->group);
- if (ret)
- return ret;
-
container = vdev->group->container;
driver = container->iommu_driver;
if (likely(driver && driver->ops->unpin_pages))
@@ -1959,8 +1957,6 @@ int vfio_unpin_pages(struct vfio_device *vdev, unsigned long *user_pfn,
else
ret = -ENOTTY;
- vfio_group_try_dissolve_container(vdev->group);
-
return ret;
}
EXPORT_SYMBOL(vfio_unpin_pages);
@@ -1989,13 +1985,9 @@ int vfio_dma_rw(struct vfio_device *vdev, dma_addr_t user_iova, void *data,
struct vfio_iommu_driver *driver;
int ret = 0;
- if (!data || len <= 0)
+ if (!data || len <= 0 || !vfio_assert_device_open(vdev))
return -EINVAL;
- ret = vfio_group_add_container_user(vdev->group);
- if (ret)
- return ret;
-
container = vdev->group->container;
driver = container->iommu_driver;
@@ -2004,9 +1996,6 @@ int vfio_dma_rw(struct vfio_device *vdev, dma_addr_t user_iova, void *data,
user_iova, data, len, write);
else
ret = -ENOTTY;
-
- vfio_group_try_dissolve_container(vdev->group);
-
return ret;
}
EXPORT_SYMBOL(vfio_dma_rw);
@@ -2019,10 +2008,6 @@ static int vfio_register_iommu_notifier(struct vfio_group *group,
struct vfio_iommu_driver *driver;
int ret;
- ret = vfio_group_add_container_user(group);
- if (ret)
- return -EINVAL;
-
container = group->container;
driver = container->iommu_driver;
if (likely(driver && driver->ops->register_notifier))
@@ -2030,9 +2015,6 @@ static int vfio_register_iommu_notifier(struct vfio_group *group,
events, nb);
else
ret = -ENOTTY;
-
- vfio_group_try_dissolve_container(group);
-
return ret;
}
@@ -2043,10 +2025,6 @@ static int vfio_unregister_iommu_notifier(struct vfio_group *group,
struct vfio_iommu_driver *driver;
int ret;
- ret = vfio_group_add_container_user(group);
- if (ret)
- return -EINVAL;
-
container = group->container;
driver = container->iommu_driver;
if (likely(driver && driver->ops->unregister_notifier))
@@ -2054,9 +2032,6 @@ static int vfio_unregister_iommu_notifier(struct vfio_group *group,
nb);
else
ret = -ENOTTY;
-
- vfio_group_try_dissolve_container(group);
-
return ret;
}
@@ -2085,10 +2060,6 @@ static int vfio_register_group_notifier(struct vfio_group *group,
if (*events)
return -EINVAL;
- ret = vfio_group_add_container_user(group);
- if (ret)
- return -EINVAL;
-
ret = blocking_notifier_chain_register(&group->notifier, nb);
/*
@@ -2098,25 +2069,6 @@ static int vfio_register_group_notifier(struct vfio_group *group,
if (!ret && set_kvm && group->kvm)
blocking_notifier_call_chain(&group->notifier,
VFIO_GROUP_NOTIFY_SET_KVM, group->kvm);
-
- vfio_group_try_dissolve_container(group);
-
- return ret;
-}
-
-static int vfio_unregister_group_notifier(struct vfio_group *group,
- struct notifier_block *nb)
-{
- int ret;
-
- ret = vfio_group_add_container_user(group);
- if (ret)
- return -EINVAL;
-
- ret = blocking_notifier_chain_unregister(&group->notifier, nb);
-
- vfio_group_try_dissolve_container(group);
-
return ret;
}
@@ -2126,7 +2078,7 @@ int vfio_register_notifier(struct vfio_device *dev, enum vfio_notify_type type,
struct vfio_group *group = dev->group;
int ret;
- if (!nb || !events || (*events == 0))
+ if (!nb || !events || (*events == 0) || !vfio_assert_device_open(dev))
return -EINVAL;
switch (type) {
@@ -2150,7 +2102,7 @@ int vfio_unregister_notifier(struct vfio_device *dev,
struct vfio_group *group = dev->group;
int ret;
- if (!nb)
+ if (!nb || !vfio_assert_device_open(dev))
return -EINVAL;
switch (type) {
@@ -2158,7 +2110,7 @@ int vfio_unregister_notifier(struct vfio_device *dev,
ret = vfio_unregister_iommu_notifier(group, nb);
break;
case VFIO_GROUP_NOTIFY:
- ret = vfio_unregister_group_notifier(group, nb);
+ ret = blocking_notifier_chain_unregister(&group->notifier, nb);
break;
default:
ret = -EINVAL;
--
2.36.0
WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe <jgg@nvidia.com>
Cc: Tony Krowiak <akrowiak@linux.ibm.com>,
"Jason J. Herne" <jjherne@linux.ibm.com>,
Eric Farman <farman@linux.ibm.com>,
"Liu, Yi L" <yi.l.liu@intel.com>,
kvm@vger.kernel.org, intel-gfx@lists.freedesktop.org,
intel-gvt-dev@lists.freedesktop.org,
Christoph Hellwig <hch@lst.de>
Subject: [Intel-gfx] [PATCH v3 7/7] vfio: Remove calls to vfio_group_add_container_user()
Date: Mon, 2 May 2022 14:31:37 -0300 [thread overview]
Message-ID: <7-v3-e131a9b6b467+14b6-vfio_mdev_no_group_jgg@nvidia.com> (raw)
In-Reply-To: <0-v3-e131a9b6b467+14b6-vfio_mdev_no_group_jgg@nvidia.com>
When the open_device() op is called the container_users is incremented and
held incremented until close_device(). Thus, so long as drivers call
functions within their open_device()/close_device() region they do not
need to worry about the container_users.
These functions can all only be called between open_device() and
close_device():
vfio_pin_pages()
vfio_unpin_pages()
vfio_dma_rw()
vfio_register_notifier()
vfio_unregister_notifier()
Eliminate the calls to vfio_group_add_container_user() and add
vfio_assert_device_open() to detect driver mis-use. This causes the
close_device() op to check device->open_count so always leave it elevated
while calling the op.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
drivers/vfio/vfio.c | 78 +++++++++------------------------------------
1 file changed, 15 insertions(+), 63 deletions(-)
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 0184d760ec1e44..f7d1898129ad1c 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1115,6 +1115,12 @@ static int vfio_group_add_container_user(struct vfio_group *group)
static const struct file_operations vfio_device_fops;
+/* true if the vfio_device has open_device() called but not close_device() */
+static bool vfio_assert_device_open(struct vfio_device *device)
+{
+ return !WARN_ON_ONCE(!READ_ONCE(device->open_count));
+}
+
static int vfio_group_get_device_fd(struct vfio_group *group, char *buf)
{
struct vfio_device *device;
@@ -1329,8 +1335,10 @@ static int vfio_device_fops_release(struct inode *inode, struct file *filep)
struct vfio_device *device = filep->private_data;
mutex_lock(&device->dev_set->lock);
- if (!--device->open_count && device->ops->close_device)
+ vfio_assert_device_open(device);
+ if (device->open_count == 1 && device->ops->close_device)
device->ops->close_device(device);
+ device->open_count--;
mutex_unlock(&device->dev_set->lock);
module_put(device->dev->driver->owner);
@@ -1897,7 +1905,7 @@ int vfio_pin_pages(struct vfio_device *vdev, unsigned long *user_pfn, int npage,
struct vfio_iommu_driver *driver;
int ret;
- if (!user_pfn || !phys_pfn || !npage)
+ if (!user_pfn || !phys_pfn || !npage || !vfio_assert_device_open(vdev))
return -EINVAL;
if (npage > VFIO_PIN_PAGES_MAX_ENTRIES)
@@ -1906,10 +1914,6 @@ int vfio_pin_pages(struct vfio_device *vdev, unsigned long *user_pfn, int npage,
if (group->dev_counter > 1)
return -EINVAL;
- ret = vfio_group_add_container_user(group);
- if (ret)
- return ret;
-
container = group->container;
driver = container->iommu_driver;
if (likely(driver && driver->ops->pin_pages))
@@ -1919,8 +1923,6 @@ int vfio_pin_pages(struct vfio_device *vdev, unsigned long *user_pfn, int npage,
else
ret = -ENOTTY;
- vfio_group_try_dissolve_container(group);
-
return ret;
}
EXPORT_SYMBOL(vfio_pin_pages);
@@ -1941,16 +1943,12 @@ int vfio_unpin_pages(struct vfio_device *vdev, unsigned long *user_pfn,
struct vfio_iommu_driver *driver;
int ret;
- if (!user_pfn || !npage)
+ if (!user_pfn || !npage || !vfio_assert_device_open(vdev))
return -EINVAL;
if (npage > VFIO_PIN_PAGES_MAX_ENTRIES)
return -E2BIG;
- ret = vfio_group_add_container_user(vdev->group);
- if (ret)
- return ret;
-
container = vdev->group->container;
driver = container->iommu_driver;
if (likely(driver && driver->ops->unpin_pages))
@@ -1959,8 +1957,6 @@ int vfio_unpin_pages(struct vfio_device *vdev, unsigned long *user_pfn,
else
ret = -ENOTTY;
- vfio_group_try_dissolve_container(vdev->group);
-
return ret;
}
EXPORT_SYMBOL(vfio_unpin_pages);
@@ -1989,13 +1985,9 @@ int vfio_dma_rw(struct vfio_device *vdev, dma_addr_t user_iova, void *data,
struct vfio_iommu_driver *driver;
int ret = 0;
- if (!data || len <= 0)
+ if (!data || len <= 0 || !vfio_assert_device_open(vdev))
return -EINVAL;
- ret = vfio_group_add_container_user(vdev->group);
- if (ret)
- return ret;
-
container = vdev->group->container;
driver = container->iommu_driver;
@@ -2004,9 +1996,6 @@ int vfio_dma_rw(struct vfio_device *vdev, dma_addr_t user_iova, void *data,
user_iova, data, len, write);
else
ret = -ENOTTY;
-
- vfio_group_try_dissolve_container(vdev->group);
-
return ret;
}
EXPORT_SYMBOL(vfio_dma_rw);
@@ -2019,10 +2008,6 @@ static int vfio_register_iommu_notifier(struct vfio_group *group,
struct vfio_iommu_driver *driver;
int ret;
- ret = vfio_group_add_container_user(group);
- if (ret)
- return -EINVAL;
-
container = group->container;
driver = container->iommu_driver;
if (likely(driver && driver->ops->register_notifier))
@@ -2030,9 +2015,6 @@ static int vfio_register_iommu_notifier(struct vfio_group *group,
events, nb);
else
ret = -ENOTTY;
-
- vfio_group_try_dissolve_container(group);
-
return ret;
}
@@ -2043,10 +2025,6 @@ static int vfio_unregister_iommu_notifier(struct vfio_group *group,
struct vfio_iommu_driver *driver;
int ret;
- ret = vfio_group_add_container_user(group);
- if (ret)
- return -EINVAL;
-
container = group->container;
driver = container->iommu_driver;
if (likely(driver && driver->ops->unregister_notifier))
@@ -2054,9 +2032,6 @@ static int vfio_unregister_iommu_notifier(struct vfio_group *group,
nb);
else
ret = -ENOTTY;
-
- vfio_group_try_dissolve_container(group);
-
return ret;
}
@@ -2085,10 +2060,6 @@ static int vfio_register_group_notifier(struct vfio_group *group,
if (*events)
return -EINVAL;
- ret = vfio_group_add_container_user(group);
- if (ret)
- return -EINVAL;
-
ret = blocking_notifier_chain_register(&group->notifier, nb);
/*
@@ -2098,25 +2069,6 @@ static int vfio_register_group_notifier(struct vfio_group *group,
if (!ret && set_kvm && group->kvm)
blocking_notifier_call_chain(&group->notifier,
VFIO_GROUP_NOTIFY_SET_KVM, group->kvm);
-
- vfio_group_try_dissolve_container(group);
-
- return ret;
-}
-
-static int vfio_unregister_group_notifier(struct vfio_group *group,
- struct notifier_block *nb)
-{
- int ret;
-
- ret = vfio_group_add_container_user(group);
- if (ret)
- return -EINVAL;
-
- ret = blocking_notifier_chain_unregister(&group->notifier, nb);
-
- vfio_group_try_dissolve_container(group);
-
return ret;
}
@@ -2126,7 +2078,7 @@ int vfio_register_notifier(struct vfio_device *dev, enum vfio_notify_type type,
struct vfio_group *group = dev->group;
int ret;
- if (!nb || !events || (*events == 0))
+ if (!nb || !events || (*events == 0) || !vfio_assert_device_open(dev))
return -EINVAL;
switch (type) {
@@ -2150,7 +2102,7 @@ int vfio_unregister_notifier(struct vfio_device *dev,
struct vfio_group *group = dev->group;
int ret;
- if (!nb)
+ if (!nb || !vfio_assert_device_open(dev))
return -EINVAL;
switch (type) {
@@ -2158,7 +2110,7 @@ int vfio_unregister_notifier(struct vfio_device *dev,
ret = vfio_unregister_iommu_notifier(group, nb);
break;
case VFIO_GROUP_NOTIFY:
- ret = vfio_unregister_group_notifier(group, nb);
+ ret = blocking_notifier_chain_unregister(&group->notifier, nb);
break;
default:
ret = -EINVAL;
--
2.36.0
next prev parent reply other threads:[~2022-05-02 17:31 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-02 17:31 [PATCH v3 0/7] Make the rest of the VFIO driver interface use vfio_device Jason Gunthorpe
2022-05-02 17:31 ` [Intel-gfx] " Jason Gunthorpe
2022-05-02 17:31 ` [PATCH v3 1/7] vfio: Make vfio_(un)register_notifier accept a vfio_device Jason Gunthorpe
2022-05-02 17:31 ` [Intel-gfx] " Jason Gunthorpe
2022-05-02 17:31 ` [PATCH v3 2/7] vfio/ccw: Remove mdev from struct channel_program Jason Gunthorpe
2022-05-02 17:31 ` [Intel-gfx] " Jason Gunthorpe
2022-05-02 17:31 ` [PATCH v3 3/7] vfio/mdev: Pass in a struct vfio_device * to vfio_pin/unpin_pages() Jason Gunthorpe
2022-05-02 17:31 ` [Intel-gfx] " Jason Gunthorpe
2022-05-02 17:31 ` [PATCH v3 4/7] vfio/mdev: Pass in a struct vfio_device * to vfio_dma_rw() Jason Gunthorpe
2022-05-02 17:31 ` [Intel-gfx] " Jason Gunthorpe
2022-05-02 17:31 ` [PATCH v3 5/7] drm/i915/gvt: Change from vfio_group_(un)pin_pages to vfio_(un)pin_pages Jason Gunthorpe
2022-05-02 17:31 ` [Intel-gfx] " Jason Gunthorpe
2022-05-02 17:31 ` [PATCH v3 6/7] vfio: Remove dead code Jason Gunthorpe
2022-05-02 17:31 ` [Intel-gfx] " Jason Gunthorpe
2022-05-02 17:31 ` Jason Gunthorpe [this message]
2022-05-02 17:31 ` [Intel-gfx] [PATCH v3 7/7] vfio: Remove calls to vfio_group_add_container_user() Jason Gunthorpe
2022-05-02 18:01 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Make the rest of the VFIO driver interface use vfio_device (rev4) Patchwork
2022-05-02 18:01 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2022-05-02 18:25 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-05-03 1:27 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2022-05-04 17:49 ` [Intel-gfx] [PATCH v3 0/7] Make the rest of the VFIO driver interface use vfio_device Jason Gunthorpe
2022-05-04 17:49 ` Jason Gunthorpe
2022-05-05 18:56 ` Alex Williamson
2022-05-05 18:56 ` [Intel-gfx] " Alex Williamson
2022-05-05 19:44 ` Jason Gunthorpe
2022-05-05 19:44 ` [Intel-gfx] " Jason Gunthorpe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7-v3-e131a9b6b467+14b6-vfio_mdev_no_group_jgg@nvidia.com \
--to=jgg@nvidia.com \
--cc=akrowiak@linux.ibm.com \
--cc=farman@linux.ibm.com \
--cc=hch@lst.de \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-gvt-dev@lists.freedesktop.org \
--cc=jjherne@linux.ibm.com \
--cc=kevin.tian@intel.com \
--cc=kvm@vger.kernel.org \
--cc=yi.l.liu@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.