Re: [PATCH 14/14] iommufd/selftest: Add test to verify iommufd preservation

[Samiullah Khawaja]

On Mon, Mar 23, 2026 at 03:18:39PM -0700, Vipin Sharma wrote:
> On Tue, Feb 03, 2026 at 10:09:48PM +0000, Samiullah Khawaja wrote:
> > Test iommufd preservation by setting up an iommufd and vfio cdev and
> > preserve it across live update. Test takes VFIO cdev path of a device
> > bound to vfio-pci driver and binds it to an iommufd being preserved. It
> > also preserves the vfio cdev so the iommufd state associated with it is
> > also preserved.
> >
> > The restore path is tested by restoring the preserved vfio cdev only.
> > Test tries to finish the session without restoring iommufd and confirms
> > that it fails.
>
> May be alos add more details that VFIO bind will also fail, it will return EPERM
> due to ...

Agreed will add more detail.
> > diff --git a/tools/testing/selftests/iommu/Makefile b/tools/testing/selftests/iommu/Makefile
> > +$(TEST_GEN_PROGS_EXTENDED): %: %.o $(LIBLIVEUPDATE_O)
> > +	$(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_ARCH) $< $(LIBLIVEUPDATE_O) $(LDLIBS) -static -o $@
>
> Why static? User can pass static flag through EXTRA_CFLAGS.

I will remove this.
> > diff --git a/tools/testing/selftests/iommu/iommufd_liveupdate.c b/tools/testing/selftests/iommu/iommufd_liveupdate.c
> > +
> > +#define ksft_assert(condition) \
> > +	do { if (!(condition)) \
> > +	ksft_exit_fail_msg("Failed: %s at %s %d: %s\n", \
> > +	#condition, __FILE__, __LINE__, strerror(errno)); } while (0)
>
> Please add indentation to the code.

Will do.
> > +
> > +int setup_cdev(const char *vfio_cdev_path)
>
> Nit: s/setup_cdev/open_cdev

Will change this to open_cdev.
> Since you are using open_iommufd() name below, this will make it
> consistent as there not setup here apart from opening.
>
> > +int setup_iommufd(int iommufd, int memfd, int cdev_fd, int hwpt_token)
> > +	bind.iommufd = iommufd;
> > +	ret = ioctl(cdev_fd, VFIO_DEVICE_BIND_IOMMUFD, &bind);
> > +	ksft_assert(!ret);
> > +
> > +	ret = ioctl(iommufd, IOMMU_IOAS_ALLOC, &alloc_data);
> > +	ksft_assert(!ret);
> > +
> > +	hwpt_alloc.dev_id = bind.out_devid;
> > +	hwpt_alloc.pt_id = alloc_data.out_ioas_id;
> > +	ret = ioctl(iommufd, IOMMU_HWPT_ALLOC, &hwpt_alloc);
> > +	ksft_assert(!ret);
> > +
> > +	attach_data.pt_id = hwpt_alloc.out_hwpt_id;
> > +	ret = ioctl(cdev_fd, VFIO_DEVICE_ATTACH_IOMMUFD_PT, &attach_data);
> > +	ksft_assert(!ret);
> > +
> > +	map_file.ioas_id = alloc_data.out_ioas_id;
> > +	ret = ioctl(iommufd, IOMMU_IOAS_MAP_FILE, &map_file);
> > +	ksft_assert(!ret);
> > +
> > +	set_preserve.hwpt_id = attach_data.pt_id;
> > +	ret = ioctl(iommufd, IOMMU_HWPT_LU_SET_PRESERVE, &set_preserve);
> > +	ksft_assert(!ret);
> > +
> > +	return ret;
> > +}
>
> iommufd_utils.h has functions defined for iommufd ioctls. I think we
> should use those functions here and if related to iommufd are not
> present we should add there.

iommufd_utils.h has utilities to use with mock testing, so those cannot
be used here. I will see if I can make separate helper functions for
these and use those here.
> > +int main(int argc, char *argv[])
> > +{
> > +	int iommufd, cdev_fd, memfd, luo, session, ret;
> > +	const int token = 0x123456;
> > +	const int cdev_token = 0x654321;
> > +	const int hwpt_token = 0x789012;
> > +	const int memfd_token = 0x890123;
> > +
> > +	if (argc < 2) {
> > +		printf("Usage: ./iommufd_liveupdate <vfio_cdev_path>\n");
> > +		return 1;
> > +	}
> > +
> > +	luo = luo_open_device();
> > +	ksft_assert(luo > 0);
> > +
> > +	session = luo_retrieve_session(luo, "iommufd-test");
> > +	if (session == -ENOENT) {
> > +		session = luo_create_session(luo, "iommufd-test");
> > +
> > +		iommufd = open_iommufd();
> > +		memfd = create_sealed_memfd(SZ_1M);
> > +		cdev_fd = setup_cdev(argv[1]);
> > +
> > +		ret = setup_iommufd(iommufd, memfd, cdev_fd, hwpt_token);
> > +		ksft_assert(!ret);
> > +
> > +		/* Cannot preserve cdev without iommufd */
> > +		ret = luo_session_preserve_fd(session, cdev_fd, cdev_token);
> > +		ksft_assert(ret);
> > +
> > +		/* Cannot preserve iommufd without preserving memfd. */
> > +		ret = luo_session_preserve_fd(session, iommufd, token);
> > +		ksft_assert(ret);
> > +
> > +		ret = luo_session_preserve_fd(session, memfd, memfd_token);
> > +		ksft_assert(!ret);
> > +
> > +		ret = luo_session_preserve_fd(session, iommufd, token);
> > +		ksft_assert(!ret);
> > +
> > +		ret = luo_session_preserve_fd(session, cdev_fd, cdev_token);
> > +		ksft_assert(!ret);
> > +
>
> All of these ksft_assert are hurting my eyes:) I like the approach in
> VFIO where library APIs does validation and test code only check actual
> things needed.
>
> Should we at least create a common function to combine both
> luo_session_preserve() and ksft_assert()?

Agreed. Will rework these.
>

Thanks,
Sami