Re: [RFC PATCH 2/3] vfio: selftests: Add iommufd hwpt replace test

[David Matlack <dmatlack@google.com>]

On 2026-09-25 01:13 AM, Samiullah Khawaja wrote:
> Add a test that does iommufd hwpt replace while a DMA is ongoing. This
> verifies the hitless replace of IOMMU domain without disrupting the DMA.
> 
> Note that the new domain is attached after mapping the required DMA
> memory at the same IOVA in the new domain.
> 
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>

Aside from the MODE_IOMMUFD nit,

Reviewed-by: David Matlack <dmatlack@google.com>

> ---
>  tools/testing/selftests/vfio/Makefile         |   1 +
>  .../vfio/vfio_iommufd_hwpt_replace_test.c     | 171 ++++++++++++++++++
>  2 files changed, 172 insertions(+)
>  create mode 100644 tools/testing/selftests/vfio/vfio_iommufd_hwpt_replace_test.c
> 
> diff --git a/tools/testing/selftests/vfio/Makefile b/tools/testing/selftests/vfio/Makefile
> index 2c32c48db509..01d0e808871b 100644
> --- a/tools/testing/selftests/vfio/Makefile
> +++ b/tools/testing/selftests/vfio/Makefile
> @@ -8,6 +8,7 @@ else
>  CFLAGS = $(KHDR_INCLUDES)
>  TEST_GEN_PROGS += vfio_dma_mapping_test
>  TEST_GEN_PROGS += vfio_dma_mapping_mmio_test
> +TEST_GEN_PROGS += vfio_iommufd_hwpt_replace_test
>  TEST_GEN_PROGS += vfio_iommufd_setup_test
>  TEST_GEN_PROGS += vfio_pci_device_test
>  TEST_GEN_PROGS += vfio_pci_device_init_perf_test
> diff --git a/tools/testing/selftests/vfio/vfio_iommufd_hwpt_replace_test.c b/tools/testing/selftests/vfio/vfio_iommufd_hwpt_replace_test.c
> new file mode 100644
> index 000000000000..3ec243aae626
> --- /dev/null
> +++ b/tools/testing/selftests/vfio/vfio_iommufd_hwpt_replace_test.c
> @@ -0,0 +1,171 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <string.h>
> +#include <sys/ioctl.h>
> +#include <sys/mman.h>
> +
> +#include <linux/sizes.h>
> +#include <linux/vfio.h>
> +
> +#include <libvfio.h>
> +
> +#include "kselftest_harness.h"
> +
> +static const char *device_bdf;
> +
> +static void region_setup(struct iommu *iommu,
> +			 struct iova_allocator *iova_allocator,
> +			 struct dma_region *region, u64 size)
> +{
> +	const int flags = MAP_SHARED | MAP_ANONYMOUS;
> +	const int prot = PROT_READ | PROT_WRITE;
> +	void *vaddr;
> +
> +	vaddr = mmap(NULL, size, prot, flags, -1, 0);
> +	VFIO_ASSERT_NE(vaddr, MAP_FAILED);
> +
> +	region->vaddr = vaddr;
> +	region->iova = iova_allocator_alloc(iova_allocator, size);
> +	region->size = size;
> +
> +	iommu_map(iommu, region);
> +}
> +
> +static void region_teardown(struct iommu *iommu, struct dma_region *region)
> +{
> +	iommu_unmap(iommu, region);
> +	VFIO_ASSERT_EQ(munmap(region->vaddr, region->size), 0);
> +}
> +
> +FIXTURE(vfio_iommufd_replace_hwpt_test) {
> +	struct iommu *iommu;
> +	struct vfio_pci_device *device;
> +	struct iova_allocator *iova_allocator;
> +	struct dma_region memcpy_region;
> +
> +	u64 size;
> +	void *src;
> +	void *dst;
> +	iova_t src_iova;
> +	iova_t dst_iova;
> +};
> +
> +FIXTURE_SETUP(vfio_iommufd_replace_hwpt_test)
> +{
> +	struct vfio_pci_driver *driver;
> +
> +	self->iommu = iommu_init("iommufd");

nit: iommu_init(MODE_IOMMUFD)

This needs to be fixed in iommufd_iommu_init() in the previous patch as
well.

> +	self->device = vfio_pci_device_init(device_bdf, self->iommu);
> +	self->iova_allocator = iova_allocator_init(self->iommu);
> +
> +	driver = &self->device->driver;
> +
> +	region_setup(self->iommu, self->iova_allocator, &self->memcpy_region, SZ_2M);
> +	region_setup(self->iommu, self->iova_allocator, &driver->region, SZ_2M);

The driver->region.size is now populated automatically based on what the
driver needs. So you can just pass in driver->region.size here instead
of hard-coding SZ_2M.

> +
> +	if (driver->ops)
> +		vfio_pci_driver_init(self->device);
> +
> +	self->size = self->memcpy_region.size / 2;
> +	self->src = self->memcpy_region.vaddr;
> +	self->dst = self->src + self->size;
> +
> +	self->src_iova = to_iova(self->device, self->src);
> +	self->dst_iova = to_iova(self->device, self->dst);
> +}
> +
> +FIXTURE_TEARDOWN(vfio_iommufd_replace_hwpt_test)
> +{
> +	struct vfio_pci_driver *driver = &self->device->driver;
> +
> +	if (driver->ops)
> +		vfio_pci_driver_remove(self->device);
> +
> +	region_teardown(self->iommu, &self->memcpy_region);
> +	region_teardown(self->iommu, &driver->region);
> +
> +	iova_allocator_cleanup(self->iova_allocator);
> +	vfio_pci_device_cleanup(self->device);
> +	iommu_cleanup(self->iommu);
> +}
> +
> +FIXTURE_VARIANT(vfio_iommufd_replace_hwpt_test) {
> +	u32 attaches;
> +};
> +
> +FIXTURE_VARIANT_ADD(vfio_iommufd_replace_hwpt_test, domain_replace) {
> +	.attaches = 1000,

Maybe we should replace memcpy_wait() with memcpy_poll() and move the
wait loop out into the generic driver layer (vfio_pci_driver.c)
repeatedly calling memcpy_poll(). The would let you do replaces until
the memcpy is done instead of hard-coding a random number, and also lift
more code out of the the drivers and into the generic core.

It should be easy enough to swap the drivers from wait() to poll() model
if you are feeling up for it. Not a blocker for this series though.

> +};
> +
> +FIXTURE_VARIANT_ADD(vfio_iommufd_replace_hwpt_test, noreplace) {
> +	.attaches = 0,
> +};
> +
> +TEST_F_TIMEOUT(vfio_iommufd_replace_hwpt_test, memcpy, 120)
> +{
> +	struct dma_region memcpy_region = {}, driver_region = {};
> +	struct vfio_pci_driver *driver = &self->device->driver;
> +	struct iommu *iommu2 = NULL;
> +	u64 size = 0;
> +	int ret;
> +	u32 i;
> +
> +	if (variant->attaches) {
> +		iommu2 = iommufd_iommu_init(self->iommu->iommufd,
> +					    self->device->dev_id);
> +
> +		memcpy_region = self->memcpy_region;
> +		driver_region = self->device->driver.region;
> +
> +		iommu_map(iommu2, &memcpy_region);
> +		iommu_map(iommu2, &driver_region);
> +	}
> +
> +	if (driver->ops) {
> +		memset(self->src, 'x', self->size);
> +		memset(self->dst, 'y', self->size);
> +
> +		size = min_t(u64, driver->max_memcpy_size, self->size);
> +
> +		/*
> +		 * Fill the device's queue. memcpy_start() returns with
> +		 * max_memcpy_count transfers still outstanding.
> +		 */
> +		vfio_pci_driver_memcpy_start(self->device,
> +					     self->src_iova,
> +					     self->dst_iova,
> +					     size, driver->max_memcpy_count);
> +	}
> +
> +	/*
> +	 * Replace the domain repeatedly while that transfer is in flight. Since
> +	 * domain replacement is a race with the translation requests, it needs
> +	 * to be done multiple times to capture the small window where iommu
> +	 * faults can occur.
> +	 *
> +	 * The loop ends on the original domain, so the teardown below is safe.
> +	 */
> +	for (i = 0; i < variant->attaches; i++) {
> +		vfio_pci_device_attach_iommu(self->device, iommu2);
> +		vfio_pci_device_attach_iommu(self->device, self->iommu);
> +	}
> +
> +	if (driver->ops) {
> +		ret = vfio_pci_driver_memcpy_wait(self->device);
> +
> +		ASSERT_EQ(0, ret);
> +		ASSERT_EQ(0, memcmp(self->src, self->dst, size));
> +	}
> +
> +	if (iommu2) {
> +		iommu_unmap(iommu2, &memcpy_region);
> +		iommu_unmap(iommu2, &driver_region);
> +		iommu_cleanup(iommu2);
> +	}
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	device_bdf = vfio_selftests_get_bdf(&argc, argv);
> +
> +	return test_harness_run(argc, argv);
> +}
> -- 
> 2.56.0.rc1.315.gc6ed9934b7-goog
>