| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAAywjhQSPveBybmq32CtRMnmz_kyzzqRgimqZ3euXB5yZq5-sg@mail.gmail.com>
Date: Fri, 9 Jan 2026 13:39:14 -0800
From: Samiullah Khawaja <skhawaja@...gle.com>
To: David Matlack <dmatlack@...gle.com>
Cc: David Woodhouse <dwmw2@...radead.org>, Lu Baolu <baolu.lu@...ux.intel.com>,
Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
Pasha Tatashin <pasha.tatashin@...een.com>, Jason Gunthorpe <jgg@...pe.ca>,
Robin Murphy <robin.murphy@....com>, Pratyush Yadav <pratyush@...nel.org>,
Kevin Tian <kevin.tian@...el.com>, Alex Williamson <alex@...zbot.org>, Shuah Khan <shuah@...nel.org>,
iommu@...ts.linux.dev, linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
Saeed Mahameed <saeedm@...dia.com>, Adithya Jayachandran <ajayachandra@...dia.com>,
Parav Pandit <parav@...dia.com>, Leon Romanovsky <leonro@...dia.com>, William Tu <witu@...dia.com>
Subject: Re: [PATCH 2/3] vfio: selftests: Add support of creating iommus from iommufd
On Wed, Jan 7, 2026 at 4:29 PM David Matlack <dmatlack@...gle.com> wrote:
>
> On 2026-01-07 08:17 PM, Samiullah Khawaja wrote:
> > Add API to init a struct iommu using an already opened iommufd instance
> > and attach devices to it.
> >
> > Signed-off-by: Samiullah Khawaja <skhawaja@...gle.com>
> > ---
> > .../vfio/lib/include/libvfio/iommu.h | 2 +
> > .../lib/include/libvfio/vfio_pci_device.h | 2 +
> > tools/testing/selftests/vfio/lib/iommu.c | 60 +++++++++++++++++--
> > .../selftests/vfio/lib/vfio_pci_device.c | 16 ++++-
> > 4 files changed, 74 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h b/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h
> > index 5c9b9dc6d993..9e96da1e6fd3 100644
> > --- a/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h
> > +++ b/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h
> > @@ -29,10 +29,12 @@ struct iommu {
> > int container_fd;
> > int iommufd;
> > u32 ioas_id;
> > + u32 hwpt_id;
> > struct list_head dma_regions;
> > };
> >
> > struct iommu *iommu_init(const char *iommu_mode);
> > +struct iommu *iommufd_iommu_init(int iommufd, u32 dev_id);
> > void iommu_cleanup(struct iommu *iommu);
> >
> > int __iommu_map(struct iommu *iommu, struct dma_region *region);
> > diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
> > index 2858885a89bb..1143ceb6a9b8 100644
> > --- a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
> > +++ b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
> > @@ -19,6 +19,7 @@ struct vfio_pci_device {
> > const char *bdf;
> > int fd;
> > int group_fd;
> > + u32 dev_id;
> >
> > struct iommu *iommu;
> >
> > @@ -65,6 +66,7 @@ void vfio_pci_config_access(struct vfio_pci_device *device, bool write,
> > #define vfio_pci_config_writew(_d, _o, _v) vfio_pci_config_write(_d, _o, _v, u16)
> > #define vfio_pci_config_writel(_d, _o, _v) vfio_pci_config_write(_d, _o, _v, u32)
> >
> > +void vfio_pci_device_attach_iommu(struct vfio_pci_device *device, struct iommu *iommu);
> > void vfio_pci_irq_enable(struct vfio_pci_device *device, u32 index,
> > u32 vector, int count);
> > void vfio_pci_irq_disable(struct vfio_pci_device *device, u32 index);
> > diff --git a/tools/testing/selftests/vfio/lib/iommu.c b/tools/testing/selftests/vfio/lib/iommu.c
> > index 58b7fb7430d4..2c67d7e24d0c 100644
> > --- a/tools/testing/selftests/vfio/lib/iommu.c
> > +++ b/tools/testing/selftests/vfio/lib/iommu.c
> > @@ -408,6 +408,18 @@ struct iommu_iova_range *iommu_iova_ranges(struct iommu *iommu, u32 *nranges)
> > return ranges;
> > }
> >
> > +static u32 iommufd_hwpt_alloc(struct iommu *iommu, u32 dev_id)
> > +{
> > + struct iommu_hwpt_alloc args = {
> > + .size = sizeof(args),
> > + .pt_id = iommu->ioas_id,
> > + .dev_id = dev_id,
> > + };
> > +
> > + ioctl_assert(iommu->iommufd, IOMMU_HWPT_ALLOC, &args);
> > + return args.out_hwpt_id;
> > +}
> > +
> > static u32 iommufd_ioas_alloc(int iommufd)
> > {
> > struct iommu_ioas_alloc args = {
> > @@ -418,11 +430,9 @@ static u32 iommufd_ioas_alloc(int iommufd)
> > return args.out_ioas_id;
> > }
> >
> > -struct iommu *iommu_init(const char *iommu_mode)
> > +static struct iommu *iommu_alloc(const char *iommu_mode)
> > {
> > - const char *container_path;
> > struct iommu *iommu;
> > - int version;
> >
> > iommu = calloc(1, sizeof(*iommu));
> > VFIO_ASSERT_NOT_NULL(iommu);
> > @@ -430,6 +440,16 @@ struct iommu *iommu_init(const char *iommu_mode)
> > INIT_LIST_HEAD(&iommu->dma_regions);
> >
> > iommu->mode = lookup_iommu_mode(iommu_mode);
> > + return iommu;
> > +}
> > +
> > +struct iommu *iommu_init(const char *iommu_mode)
> > +{
> > + const char *container_path;
> > + struct iommu *iommu;
> > + int version;
> > +
> > + iommu = iommu_alloc(iommu_mode);
> >
> > container_path = iommu->mode->container_path;
> > if (container_path) {
> > @@ -453,10 +473,42 @@ struct iommu *iommu_init(const char *iommu_mode)
> > return iommu;
> > }
> >
> > +struct iommu *iommufd_iommu_init(int iommufd, u32 dev_id)
>
> I don't think the name really captures what this routine is doing. How
> about iommufd_dup()?
The reason I used _iommu_init because it creates a new hwpt and ioas
also, and it represents a separate "struct iommu". dup might indicate
that it is pointing to the same IOAS? Do you think maybe dup is an
implementation detail that doesn't need to be conveyed?
Do you think maybe I should rename it to iommufd_device_iommu_init as
it is creating an hwpt compatible with the device "dev_id"? Or
iommufd_iommu_init_for_device?
>
> > +{
> > + struct iommu *iommu;
> > +
> > + iommu = iommu_alloc("iommufd");
> > +
> > + iommu->iommufd = dup(iommufd);
> > + VFIO_ASSERT_GT(iommu->iommufd, 0);
> > +
> > + iommu->ioas_id = iommufd_ioas_alloc(iommu->iommufd);
> > + iommu->hwpt_id = iommufd_hwpt_alloc(iommu, dev_id);
> > +
> > + return iommu;
> > +}
> > +
> > +static void iommufd_iommu_cleanup(struct iommu *iommu)
>
> nit: iommufd_cleanup()
Agreed.
>
> > +{
> > + struct iommu_destroy args = {
> > + .size = sizeof(args),
> > + };
> > +
> > + if (iommu->hwpt_id) {
> > + args.id = iommu->hwpt_id;
> > + ioctl_assert(iommu->iommufd, IOMMU_DESTROY, &args);
> > + }
> > +
> > + args.id = iommu->ioas_id;
> > + ioctl_assert(iommu->iommufd, IOMMU_DESTROY, &args);
> > +
> > + VFIO_ASSERT_EQ(close(iommu->iommufd), 0);
> > +}
> > +
> > void iommu_cleanup(struct iommu *iommu)
> > {
> > if (iommu->iommufd)
> > - VFIO_ASSERT_EQ(close(iommu->iommufd), 0);
> > + iommufd_iommu_cleanup(iommu);
> > else
> > VFIO_ASSERT_EQ(close(iommu->container_fd), 0);
> >
> > diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> > index fac4c0ecadef..9bc1f5ade5c4 100644
> > --- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> > +++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
> > @@ -298,7 +298,7 @@ const char *vfio_pci_get_cdev_path(const char *bdf)
> > return cdev_path;
> > }
> >
> > -static void vfio_device_bind_iommufd(int device_fd, int iommufd)
> > +static int vfio_device_bind_iommufd(int device_fd, int iommufd)
> > {
> > struct vfio_device_bind_iommufd args = {
> > .argsz = sizeof(args),
> > @@ -306,6 +306,7 @@ static void vfio_device_bind_iommufd(int device_fd, int iommufd)
> > };
> >
> > ioctl_assert(device_fd, VFIO_DEVICE_BIND_IOMMUFD, &args);
> > + return args.out_devid;
> > }
> >
> > static void vfio_device_attach_iommufd_pt(int device_fd, u32 pt_id)
> > @@ -326,10 +327,21 @@ static void vfio_pci_iommufd_setup(struct vfio_pci_device *device, const char *b
> > VFIO_ASSERT_GE(device->fd, 0);
> > free((void *)cdev_path);
> >
> > - vfio_device_bind_iommufd(device->fd, device->iommu->iommufd);
> > + device->dev_id = vfio_device_bind_iommufd(device->fd, device->iommu->iommufd);
> > vfio_device_attach_iommufd_pt(device->fd, device->iommu->ioas_id);
> > }
> >
> > +void vfio_pci_device_attach_iommu(struct vfio_pci_device *device, struct iommu *iommu)
> > +{
> > + u32 pt_id = iommu->ioas_id;
>
> /* Only iommufd supports changing struct iommu attachments */
> VFIO_ASSERT_TRUE(iommu->iommufd);
Agreed
>
> > +
> > + if (iommu->hwpt_id)
> > + pt_id = iommu->hwpt_id;
> > +
> > + VFIO_ASSERT_NE(pt_id, 0);
> > + vfio_device_attach_iommufd_pt(device->fd, pt_id);
>
> device->iommu = iommu;
>
> > +}
> > +
> > struct vfio_pci_device *vfio_pci_device_init(const char *bdf, struct iommu *iommu)
> > {
> > struct vfio_pci_device *device;
> > --
> > 2.52.0.351.gbe84eed79e-goog
> >
Thank you for the feedback.
Powered by blists - more mailing lists