From: Jason Gunthorpe <jgg@nvidia.com>
To: "Tian, Kevin" <kevin.tian@intel.com>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
Vineeth Vijayan <vneethv@linux.ibm.com>,
Diana Craciun <diana.craciun@oss.nxp.com>,
Alexander Gordeev <agordeev@linux.ibm.com>,
Longfang Liu <liulongfang@huawei.com>,
"linux-s390@vger.kernel.org" <linux-s390@vger.kernel.org>,
"Liu, Yi L" <yi.l.liu@intel.com>,
Matthew Rosato <mjrosato@linux.ibm.com>,
Will Deacon <will@kernel.org>, Joerg Roedel <joro@8bytes.org>,
Halil Pasic <pasic@linux.ibm.com>,
"iommu@lists.linux.dev" <iommu@lists.linux.dev>,
Nicolin Chen <nicolinc@nvidia.com>,
Christian Borntraeger <borntraeger@linux.ibm.com>,
"intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>,
"Wang, Zhi A" <zhi.a.wang@intel.com>,
Jason Herne <jjherne@linux.ibm.com>,
Eric Farman <farman@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Heiko Carstens <hca@linux.ibm.com>,
Eric Auger <eric.auger@redhat.com>,
Alex Williamson <alex.williamson@redhat.com>,
Harald Freudenberger <freude@linux.ibm.com>,
"Vivi, Rodrigo" <rodrigo.vivi@intel.com>,
"intel-gvt-dev@lists.freedesktop.org"
<intel-gvt-dev@lists.freedesktop.org>,
Tony Krowiak <akrowiak@linux.ibm.com>,
Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
Yishai Hadas <yishaih@nvidia.com>,
Cornelia Huck <cohuck@redhat.com>,
Peter Oberparleiter <oberpar@linux.ibm.com>,
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>,
Sven Schnelle <svens@linux.ibm.com>,
Robin Murphy <robin.murphy@arm.com>,
Lu Baolu <baolu.lu@linux.intel.com>
Subject: Re: [PATCH v2 10/11] vfio: Make vfio_container optionally compiled
Date: Thu, 10 Nov 2022 13:52:39 -0400 [thread overview]
Message-ID: <Y206Z5vMwcyQK7d7@nvidia.com> (raw)
In-Reply-To: <BN9PR11MB5276494548F01A42694E366A8C019@BN9PR11MB5276.namprd11.prod.outlook.com>
On Thu, Nov 10, 2022 at 06:57:57AM +0000, Tian, Kevin wrote:
> > + /*
> > + * Emulation for NOIMMU is imperfect in that VFIO blocks almost all
> > + * other ioctls. We let them keep working but they mostly fail since no
> > + * IOAS should exist.
> > + */
> > + if (IS_ENABLED(CONFIG_VFIO_NOIOMMU) && type ==
> > VFIO_NOIOMMU_IOMMU)
> > + return 0;
> > +
> > if (type != VFIO_TYPE1_IOMMU && type != VFIO_TYPE1v2_IOMMU)
> > return -EINVAL;
>
> also need a check in iommufd_vfio_check_extension() so only
> VFIO_NOIOMMU_IOMMU is supported in no-iommu mode.
Mm, and some permission checks too
> > + if (!IS_ENABLED(CONFIG_VFIO_NO_IOMMU) ||
> > + group->type != VFIO_NO_IOMMU) {
> > + ret = iommufd_vfio_compat_ioas_id(iommufd,
> > &ioas_id);
> > + if (ret) {
> > + iommufd_ctx_put(group->iommufd);
> > + goto out_unlock;
> > + }
> > }
>
> with above I suppose other ioctls (map/unmap/etc.) are implicitly blocked
> given get_compat_ioas() will fail in those paths. this is good.
>
> btw vfio container requires exact match between group->type and
> container->noiommu, i.e. noiommu group can be only attached to noiommu
> container. this is another thing to be paired up.
Sure, as below
So, the missing ingredient here is somone who has the necessary device
to test dpdk? I wonder if qemu e1000 is able to do this path?
diff --git a/drivers/iommu/iommufd/vfio_compat.c b/drivers/iommu/iommufd/vfio_compat.c
index dbef3274803336..c20e55ddc9aa81 100644
--- a/drivers/iommu/iommufd/vfio_compat.c
+++ b/drivers/iommu/iommufd/vfio_compat.c
@@ -26,16 +26,35 @@ static struct iommufd_ioas *get_compat_ioas(struct iommufd_ctx *ictx)
}
/**
- * iommufd_vfio_compat_ioas_id - Return the IOAS ID that vfio should use
+ * iommufd_vfio_compat_ioas_get_id - Ensure a comat IOAS exists
+ * @ictx: Context to operate on
+ *
+ * Return the ID of the current compatability ioas. The ID can be passed into
+ * other functions that take an ioas_id.
+ */
+int iommufd_vfio_compat_ioas_get_id(struct iommufd_ctx *ictx, u32 *out_ioas_id)
+{
+ struct iommufd_ioas *ioas;
+
+ ioas = get_compat_ioas(ictx);
+ if (IS_ERR(ioas))
+ return PTR_ERR(ioas);
+ *out_ioas_id = ioas->obj.id;
+ iommufd_put_object(&ioas->obj);
+ return 0;
+}
+EXPORT_SYMBOL_NS_GPL(iommufd_vfio_compat_ioas_get_id, IOMMUFD_VFIO);
+
+/**
+ * iommufd_vfio_compat_ioas_create_id - Return the IOAS ID that vfio should use
* @ictx: Context to operate on
- * @out_ioas_id: The ioas_id the caller should use
*
* The compatibility IOAS is the IOAS that the vfio compatibility ioctls operate
* on since they do not have an IOAS ID input in their ABI. Only attaching a
- * group should cause a default creation of the internal ioas, this returns the
- * existing ioas if it has already been assigned somehow.
+ * group should cause a default creation of the internal ioas, this does nothing
+ * if an existing ioas has already been assigned somehow.
*/
-int iommufd_vfio_compat_ioas_id(struct iommufd_ctx *ictx, u32 *out_ioas_id)
+int iommufd_vfio_compat_ioas_create_id(struct iommufd_ctx *ictx)
{
struct iommufd_ioas *ioas = NULL;
struct iommufd_ioas *out_ioas;
@@ -53,7 +72,6 @@ int iommufd_vfio_compat_ioas_id(struct iommufd_ctx *ictx, u32 *out_ioas_id)
}
xa_unlock(&ictx->objects);
- *out_ioas_id = out_ioas->obj.id;
if (out_ioas != ioas) {
iommufd_put_object(&out_ioas->obj);
iommufd_object_abort(ictx, &ioas->obj);
@@ -68,7 +86,7 @@ int iommufd_vfio_compat_ioas_id(struct iommufd_ctx *ictx, u32 *out_ioas_id)
iommufd_object_finalize(ictx, &ioas->obj);
return 0;
}
-EXPORT_SYMBOL_NS_GPL(iommufd_vfio_compat_ioas_id, IOMMUFD_VFIO);
+EXPORT_SYMBOL_NS_GPL(iommufd_vfio_compat_ioas_create_id, IOMMUFD_VFIO);
int iommufd_vfio_ioas(struct iommufd_ucmd *ucmd)
{
@@ -230,6 +248,9 @@ static int iommufd_vfio_check_extension(struct iommufd_ctx *ictx,
case VFIO_UNMAP_ALL:
return 1;
+ case VFIO_NOIOMMU_IOMMU:
+ return IS_ENABLED(CONFIG_VFIO_NOIOMMU);
+
case VFIO_DMA_CC_IOMMU:
return iommufd_vfio_cc_iommu(ictx);
@@ -259,6 +280,17 @@ static int iommufd_vfio_set_iommu(struct iommufd_ctx *ictx, unsigned long type)
struct iommufd_ioas *ioas = NULL;
int rc = 0;
+ /*
+ * Emulation for NOIMMU is imperfect in that VFIO blocks almost all
+ * other ioctls. We let them keep working but they mostly fail since no
+ * IOAS should exist.
+ */
+ if (IS_ENABLED(CONFIG_VFIO_NOIOMMU) && type == VFIO_NOIOMMU_IOMMU) {
+ if (!capable(CAP_SYS_RAWIO))
+ return -EPERM;
+ return 0;
+ }
+
if (type != VFIO_TYPE1_IOMMU && type != VFIO_TYPE1v2_IOMMU)
return -EINVAL;
diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c
index 595c7b2146f88c..daa8039da7a8fa 100644
--- a/drivers/vfio/iommufd.c
+++ b/drivers/vfio/iommufd.c
@@ -18,6 +18,21 @@ int vfio_iommufd_bind(struct vfio_device *vdev, struct iommufd_ctx *ictx)
lockdep_assert_held(&vdev->dev_set->lock);
+ if (IS_ENABLED(CONFIG_VFIO_NO_IOMMU) &&
+ vdev->group->type == VFIO_NO_IOMMU) {
+ if (!capable(CAP_SYS_RAWIO))
+ return -EPERM;
+
+ /*
+ * Require no compat ioas to be assigned to proceed. The basic
+ * statement is that the user cannot have done something that
+ * implies they expected translation to exist
+ */
+ if (!iommufd_vfio_compat_ioas_get_id(ictx, &ioas_id))
+ return -EPERM;
+ return 0;
+ }
+
/*
* If the driver doesn't provide this op then it means the device does
* not do DMA at all. So nothing to do.
@@ -29,7 +44,7 @@ int vfio_iommufd_bind(struct vfio_device *vdev, struct iommufd_ctx *ictx)
if (ret)
return ret;
- ret = iommufd_vfio_compat_ioas_id(ictx, &ioas_id);
+ ret = iommufd_vfio_compat_ioas_get_id(ictx, &ioas_id);
if (ret)
goto err_unbind;
ret = vdev->ops->attach_ioas(vdev, &ioas_id);
@@ -53,6 +68,10 @@ void vfio_iommufd_unbind(struct vfio_device *vdev)
{
lockdep_assert_held(&vdev->dev_set->lock);
+ if (IS_ENABLED(CONFIG_VFIO_NO_IOMMU) &&
+ vdev->group->type == VFIO_NO_IOMMU)
+ return;
+
if (vdev->ops->unbind_iommufd)
vdev->ops->unbind_iommufd(vdev);
}
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index f3c48b8c45627d..b59eff30968a1e 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -747,12 +747,13 @@ static int vfio_group_ioctl_set_container(struct vfio_group *group,
iommufd = iommufd_ctx_from_file(f.file);
if (!IS_ERR(iommufd)) {
- u32 ioas_id;
-
- ret = iommufd_vfio_compat_ioas_id(iommufd, &ioas_id);
- if (ret) {
- iommufd_ctx_put(group->iommufd);
- goto out_unlock;
+ if (!IS_ENABLED(CONFIG_VFIO_NO_IOMMU) ||
+ group->type != VFIO_NO_IOMMU) {
+ ret = iommufd_vfio_compat_ioas_create_id(iommufd);
+ if (ret) {
+ iommufd_ctx_put(group->iommufd);
+ goto out_unlock;
+ }
}
group->iommufd = iommufd;
diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h
index 7a5d64a1dae482..bf2b3ea5f90fd2 100644
--- a/include/linux/iommufd.h
+++ b/include/linux/iommufd.h
@@ -61,7 +61,8 @@ void iommufd_access_unpin_pages(struct iommufd_access *access,
unsigned long iova, unsigned long length);
int iommufd_access_rw(struct iommufd_access *access, unsigned long iova,
void *data, size_t len, unsigned int flags);
-int iommufd_vfio_compat_ioas_id(struct iommufd_ctx *ictx, u32 *out_ioas_id);
+int iommufd_vfio_compat_ioas_get_id(struct iommufd_ctx *ictx, u32 *out_ioas_id);
+int iommufd_vfio_compat_ioas_create_id(struct iommufd_ctx *ictx);
#else /* !CONFIG_IOMMUFD */
static inline struct iommufd_ctx *iommufd_ctx_from_file(struct file *file)
{
@@ -93,8 +94,7 @@ static inline int iommufd_access_rw(struct iommufd_access *access, unsigned long
return -EOPNOTSUPP;
}
-static inline int iommufd_vfio_compat_ioas_id(struct iommufd_ctx *ictx,
- u32 *out_ioas_id)
+static inline int iommufd_vfio_compat_ioas_create_id(struct iommufd_ctx *ictx)
{
return -EOPNOTSUPP;
}
next prev parent reply other threads:[~2022-11-10 17:52 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-08 0:52 [PATCH v2 00/11] Connect VFIO to IOMMUFD Jason Gunthorpe
2022-11-08 0:52 ` [PATCH v2 01/11] vfio: Move vfio_device driver open/close code to a function Jason Gunthorpe
2022-11-08 0:52 ` [PATCH v2 02/11] vfio: Move vfio_device_assign_container() into vfio_device_first_open() Jason Gunthorpe
2022-11-08 0:52 ` [PATCH v2 03/11] vfio: Rename vfio_device_assign/unassign_container() Jason Gunthorpe
2022-11-08 0:52 ` [PATCH v2 04/11] vfio: Move storage of allow_unsafe_interrupts to vfio_main.c Jason Gunthorpe
2022-11-08 0:52 ` [PATCH v2 05/11] vfio: Use IOMMU_CAP_ENFORCE_CACHE_COHERENCY for vfio_file_enforced_coherent() Jason Gunthorpe
2022-11-10 2:48 ` Tian, Kevin
2022-11-08 0:52 ` [PATCH v2 06/11] vfio-iommufd: Allow iommufd to be used in place of a container fd Jason Gunthorpe
2022-11-10 2:51 ` Tian, Kevin
2022-11-08 0:52 ` [PATCH v2 07/11] vfio-iommufd: Support iommufd for physical VFIO devices Jason Gunthorpe
2022-11-08 6:10 ` Nicolin Chen
2022-11-08 7:41 ` Yi Liu
2022-11-08 17:51 ` Jason Gunthorpe
2022-11-10 3:12 ` Tian, Kevin
2022-11-08 17:48 ` Jason Gunthorpe
2022-11-10 3:11 ` Tian, Kevin
2022-11-10 17:20 ` Jason Gunthorpe
2022-11-10 23:58 ` Tian, Kevin
2022-11-11 4:12 ` Yi Liu
2022-11-14 14:47 ` Jason Gunthorpe
2022-11-08 0:52 ` [PATCH v2 08/11] vfio-iommufd: Support iommufd for emulated " Jason Gunthorpe
2022-11-10 5:33 ` Tian, Kevin
2022-11-08 0:52 ` [PATCH v2 09/11] vfio: Move container related MODULE_ALIAS statements into container.c Jason Gunthorpe
2022-11-10 5:34 ` Tian, Kevin
2022-11-11 4:13 ` Yi Liu
2022-11-08 0:52 ` [PATCH v2 10/11] vfio: Make vfio_container optionally compiled Jason Gunthorpe
2022-11-08 22:28 ` Alex Williamson
2022-11-09 0:54 ` Jason Gunthorpe
2022-11-09 17:18 ` Alex Williamson
2022-11-09 19:52 ` Jason Gunthorpe
2022-11-10 6:57 ` Tian, Kevin
2022-11-10 17:10 ` Alex Williamson
2022-11-10 17:52 ` Jason Gunthorpe [this message]
2022-11-08 0:52 ` [PATCH v2 11/11] iommufd: Allow iommufd to supply /dev/vfio/vfio Jason Gunthorpe
2022-11-10 7:01 ` Tian, Kevin
2022-11-11 4:16 ` Yi Liu
2022-11-11 6:38 ` Yi Liu
2022-11-14 14:50 ` Jason Gunthorpe
2022-11-08 9:19 ` [PATCH v2 00/11] Connect VFIO to IOMMUFD Nicolin Chen
2022-11-08 15:18 ` Yi Liu
2022-11-09 16:57 ` Jason Gunthorpe
2022-11-14 12:51 ` Yi Liu
2022-11-14 14:37 ` Yang, Lixiao
2022-11-15 5:41 ` He, Yu
2022-11-14 14:38 ` Jason Gunthorpe
2022-11-14 14:42 ` Yi Liu
2022-11-15 1:16 ` Matthew Rosato
2022-11-09 9:03 ` Tian, Kevin
2022-11-09 12:48 ` Jason Gunthorpe
2022-11-10 2:16 ` Tian, Kevin
2022-11-11 3:01 ` Matthew Rosato
2022-11-14 14:23 ` Jason Gunthorpe
2022-11-14 14:55 ` Matthew Rosato
2022-11-14 14:59 ` Jason Gunthorpe
2022-11-14 15:21 ` Matthew Rosato
2022-11-14 19:27 ` 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=Y206Z5vMwcyQK7d7@nvidia.com \
--to=jgg@nvidia.com \
--cc=agordeev@linux.ibm.com \
--cc=akrowiak@linux.ibm.com \
--cc=alex.williamson@redhat.com \
--cc=baolu.lu@linux.intel.com \
--cc=borntraeger@linux.ibm.com \
--cc=cohuck@redhat.com \
--cc=diana.craciun@oss.nxp.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=eric.auger@redhat.com \
--cc=farman@linux.ibm.com \
--cc=freude@linux.ibm.com \
--cc=gor@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-gvt-dev@lists.freedesktop.org \
--cc=iommu@lists.linux.dev \
--cc=jjherne@linux.ibm.com \
--cc=joro@8bytes.org \
--cc=kevin.tian@intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=liulongfang@huawei.com \
--cc=mjrosato@linux.ibm.com \
--cc=nicolinc@nvidia.com \
--cc=oberpar@linux.ibm.com \
--cc=pasic@linux.ibm.com \
--cc=robin.murphy@arm.com \
--cc=rodrigo.vivi@intel.com \
--cc=shameerali.kolothum.thodi@huawei.com \
--cc=svens@linux.ibm.com \
--cc=tvrtko.ursulin@linux.intel.com \
--cc=vneethv@linux.ibm.com \
--cc=will@kernel.org \
--cc=yi.l.liu@intel.com \
--cc=yishaih@nvidia.com \
--cc=zhi.a.wang@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).