[RFC PATCH 1/3] vfio: selftests: Add support of creating iommus from iommufd
Samiullah Khawaja
skhawaja at google.com
Fri Oct 23 16:04:10 PDT 2026
On Wed, Oct 21, 2026 at 10:19:34PM +0000, David Matlack wrote:
>On 2026-09-25 01:13 AM, 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 at google.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 | 25 ++++++--
>> 4 files changed, 80 insertions(+), 9 deletions(-)
>>
>> diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h b/tools/testing/selftests/vfio/lib/include/libvfio/iommu.h
>> index e9a3386a4719..62f1e8affd2b 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 e19bd94b8dd2..4dcc23bc9b78 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;
>>
>> @@ -82,6 +83,7 @@ static inline void vfio_pci_cmd_clear(struct vfio_pci_device *device, u16 bits)
>> vfio_pci_config_writew(device, PCI_COMMAND, cmd & ~bits);
>> }
>>
>> +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 b6f3c5c84e01..6d2d5c82813c 100644
>> --- a/tools/testing/selftests/vfio/lib/iommu.c
>> +++ b/tools/testing/selftests/vfio/lib/iommu.c
>> @@ -405,6 +405,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 = {
>> @@ -415,17 +427,25 @@ 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_assert(1, sizeof(*iommu));
>>
>> 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) {
>> @@ -449,10 +469,42 @@ struct iommu *iommu_init(const char *iommu_mode)
>> return iommu;
>> }
>>
>> +struct iommu *iommufd_iommu_init(int iommufd, u32 dev_id)
>
>I think this is a good opportunity to start adding kernel-doc comments.
>It's not very obvious how and when to use both iommufd_iommu_init() and
>vfio_pci_device_attach_iommu(). A kernel-doc comment would help clarify
>that.
Agreed. I will add it.
>
>> +{
>> + 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_cleanup(struct iommu *iommu)
>> +{
>> + 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);
>
>Please put IOMMU_DESTROY into its own helper so this can be:
>
>if (iommu->hwpt_id)
> iommufd_iommu_destroy(iommu, iommu->hwpt_id);
>
>iommufd_iommu_destroy(iommu, iommu->ioas_id);
Will do
>
>> +
>> + VFIO_ASSERT_EQ(close(iommu->iommufd), 0);
>> +}
>> +
>> void iommu_cleanup(struct iommu *iommu)
>> {
>> if (iommu->iommufd)
>> - VFIO_ASSERT_EQ(close(iommu->iommufd), 0);
>> + iommufd_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 774d1e90ec0e..81d3879d064f 100644
>> --- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
>> +++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
>> @@ -469,15 +469,15 @@ int __vfio_device_bind_iommufd(int device_fd, int iommufd, const char *vf_token)
>> if (ioctl(device_fd, VFIO_DEVICE_BIND_IOMMUFD, &args))
>> return -errno;
>>
>> - return 0;
>> + return args.out_devid;
>
>This breaks vfio_pci_sriov_uapi_test for all iommufd variants (9/45
>fail). iommufd_setup() in that test returns the result of
>__vfio_device_bind_iommufd() directly, and the
>fixture/ASSERT_COND_VF_CREATION() treat any non-zero value as failure.
>
>Something like the following is needed to fix it:
>
>diff --git a/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c b/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c
>--- a/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c
>+++ b/tools/testing/selftests/vfio/vfio_pci_sriov_uapi_test.c
>@@ -32,9 +32,14 @@ static int container_setup(struct vfio_pci_device *device, const char *bdf,
> static int iommufd_setup(struct vfio_pci_device *device, const char *bdf,
> const char *vf_token)
> {
>+ int ret;
>+
> vfio_pci_cdev_open(device, bdf);
>- return __vfio_device_bind_iommufd(device->fd,
>- device->iommu->iommufd, vf_token);
>+ ret = __vfio_device_bind_iommufd(device->fd,
>+ device->iommu->iommufd, vf_token);
>+
>+ /* On success, __vfio_device_bind_iommufd() returns the device ID. */
>+ return ret < 0 ? ret : 0;
> }
>
> static int device_init(const char *bdf, struct iommu *iommu,
Will update.
>
>> }
>>
>> -static void vfio_device_bind_iommufd(int device_fd, int iommufd,
>> - const char *vf_token)
>> +int vfio_device_bind_iommufd(int device_fd, int iommufd, const char *vf_token)
>> {
>> int ret = __vfio_device_bind_iommufd(device_fd, iommufd, vf_token);
>>
>> - VFIO_ASSERT_EQ(ret, 0, "Failed VFIO_DEVICE_BIND_IOMMUFD ioctl\n");
>> + VFIO_ASSERT_GE(ret, 0, "Failed VFIO_DEVICE_BIND_IOMMUFD ioctl\n");
>> + return ret;
>> }
>>
>> static void vfio_device_attach_iommufd_pt(int device_fd, u32 pt_id)
>> @@ -503,10 +503,25 @@ static void vfio_pci_iommufd_setup(struct vfio_pci_device *device,
>> const char *bdf, const char *vf_token)
>> {
>> vfio_pci_cdev_open(device, bdf);
>> - vfio_device_bind_iommufd(device->fd, device->iommu->iommufd, vf_token);
>> + device->dev_id = vfio_device_bind_iommufd(device->fd, device->iommu->iommufd, vf_token);
>> 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);
>> +
>> + if (iommu->hwpt_id)
>> + pt_id = iommu->hwpt_id;
>> +
>> + VFIO_ASSERT_NE(pt_id, 0);
>
>Is this assert needed? (Will vfio_device_attach_iommufd_pt() not handle
>this appropriately?)
It should be covered by the assert for the ioctl. I will remove this.
>
>> + vfio_device_attach_iommufd_pt(device->fd, pt_id);
>> + device->iommu = iommu;
>> +}
>> +
>> struct vfio_pci_device *vfio_pci_device_alloc(const char *bdf, struct iommu *iommu)
>> {
>> struct vfio_pci_device *device;
>> --
>> 2.56.0.rc1.315.gc6ed9934b7-goog
>>
Thanks,
Sami
More information about the linux-arm-kernel
mailing list