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

[Vipin Sharma <vipinsh@google.com>]

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 ...

> 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.

> 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.

> +
> +int setup_cdev(const char *vfio_cdev_path)

Nit: s/setup_cdev/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.

> +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()?