Re: [PATCH 11/14] iommufd-lu: Persist iommu hardware pagetables for live update

[Pranjal Shrivastava <praan@google.com>]

On Tue, Feb 03, 2026 at 10:09:45PM +0000, Samiullah Khawaja wrote:
> From: YiFei Zhu <zhuyifei@google.com>
> 
> The caller is expected to mark each HWPT to be preserved with an ioctl
> call, with a token that will be used in restore. At preserve time, each
> HWPT's domain is then called with iommu_domain_preserve to preserve the
> iommu domain.
> 
> The HWPTs containing dma mappings backed by unpreserved memory should
> not be preserved. During preservation check if the mappings contained in
> the HWPT being preserved are only file based and all the files are
> preserved.
> 
> The memfd file preservation check is not enough when preserving iommufd.
> The memfd might have shrunk between the mapping and memfd preservation.
> This means that if it shrunk some pages that are right now pinned due to
> iommu mappings are not preserved with the memfd. Only allow iommufd
> preservation when all the iopt_pages are file backed and the memory file
> was seal sealed during mapping. This guarantees that all the pages that
> were backing memfd when it was mapped are preserved.
> 
> Once HWPT is preserved the iopt associated with the HWPT is made
> immutable. Since the map and unmap ioctls operates directly on iopt,
> which contains an array of domains, while each hwpt contains only one
> domain. The logic then becomes that mapping and unmapping is prohibited
> if any of the domains in an iopt belongs to a preserved hwpt. However,
> tracing to the hwpt through the domain is a lot more tedious than
> tracing through the ioas, so if an hwpt is preserved, hwpt->ioas->iopt
> is made immutable.
> 
> When undoing this (making the iopts mutable again), there's never
> a need to make some iopts mutable and some kept immutable, since
> the undo only happen on unpreserve and error path of preserve.
> Simply iterate all the ioas and clear the immutability flag on all
> their iopts.
> 
> Signed-off-by: YiFei Zhu <zhuyifei@google.com>
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
> ---
>  drivers/iommu/iommufd/io_pagetable.c    |  17 ++
>  drivers/iommu/iommufd/io_pagetable.h    |   1 +
>  drivers/iommu/iommufd/iommufd_private.h |  25 ++
>  drivers/iommu/iommufd/liveupdate.c      | 300 ++++++++++++++++++++++++
>  drivers/iommu/iommufd/main.c            |  14 +-
>  drivers/iommu/iommufd/pages.c           |   8 +
>  include/linux/kho/abi/iommufd.h         |  39 +++
>  7 files changed, 403 insertions(+), 1 deletion(-)
>  create mode 100644 include/linux/kho/abi/iommufd.h
> 
> diff --git a/drivers/iommu/iommufd/io_pagetable.c b/drivers/iommu/iommufd/io_pagetable.c
> index 436992331111..43e8a2443793 100644
> --- a/drivers/iommu/iommufd/io_pagetable.c
> +++ b/drivers/iommu/iommufd/io_pagetable.c
> @@ -270,6 +270,11 @@ static int iopt_alloc_area_pages(struct io_pagetable *iopt,
>  	}
>  
>  	down_write(&iopt->iova_rwsem);
> +	if (iopt_lu_map_immutable(iopt)) {
> +		rc = -EBUSY;
> +		goto out_unlock;
> +	}
> +
>  	if ((length & (iopt->iova_alignment - 1)) || !length) {
>  		rc = -EINVAL;
>  		goto out_unlock;
> @@ -328,6 +333,7 @@ static void iopt_abort_area(struct iopt_area *area)
>  		WARN_ON(area->pages);
>  	if (area->iopt) {
>  		down_write(&area->iopt->iova_rwsem);
> +		WARN_ON(iopt_lu_map_immutable(area->iopt));
>  		interval_tree_remove(&area->node, &area->iopt->area_itree);
>  		up_write(&area->iopt->iova_rwsem);
>  	}
> @@ -755,6 +761,12 @@ static int iopt_unmap_iova_range(struct io_pagetable *iopt, unsigned long start,
>  again:
>  	down_read(&iopt->domains_rwsem);
>  	down_write(&iopt->iova_rwsem);
> +
> +	if (iopt_lu_map_immutable(iopt)) {
> +		rc = -EBUSY;
> +		goto out_unlock_iova;
> +	}
> +
>  	while ((area = iopt_area_iter_first(iopt, start, last))) {
>  		unsigned long area_last = iopt_area_last_iova(area);
>  		unsigned long area_first = iopt_area_iova(area);
> @@ -1398,6 +1410,11 @@ int iopt_cut_iova(struct io_pagetable *iopt, unsigned long *iovas,
>  	int i;
>  
>  	down_write(&iopt->iova_rwsem);
> +	if (iopt_lu_map_immutable(iopt)) {
> +		up_write(&iopt->iova_rwsem);
> +		return -EBUSY;
> +	}
> +
>  	for (i = 0; i < num_iovas; i++) {
>  		struct iopt_area *area;
>  
> diff --git a/drivers/iommu/iommufd/io_pagetable.h b/drivers/iommu/iommufd/io_pagetable.h
> index 14cd052fd320..b64cb4cf300c 100644
> --- a/drivers/iommu/iommufd/io_pagetable.h
> +++ b/drivers/iommu/iommufd/io_pagetable.h
> @@ -234,6 +234,7 @@ struct iopt_pages {
>  		struct {			/* IOPT_ADDRESS_FILE */
>  			struct file *file;
>  			unsigned long start;
> +			u32 seals;
>  		};
>  		/* IOPT_ADDRESS_DMABUF */
>  		struct iopt_pages_dmabuf dmabuf;
> diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
> index 6424e7cea5b2..f8366a23999f 100644
> --- a/drivers/iommu/iommufd/iommufd_private.h
> +++ b/drivers/iommu/iommufd/iommufd_private.h
> @@ -94,6 +94,9 @@ struct io_pagetable {
>  	/* IOVA that cannot be allocated, struct iopt_reserved */
>  	struct rb_root_cached reserved_itree;
>  	u8 disable_large_pages;
> +#ifdef CONFIG_IOMMU_LIVEUPDATE
> +	bool lu_map_immutable;
> +#endif
>  	unsigned long iova_alignment;
>  };
>  
> @@ -712,12 +715,34 @@ iommufd_get_vdevice(struct iommufd_ctx *ictx, u32 id)
>  }
>  
>  #ifdef CONFIG_IOMMU_LIVEUPDATE
> +int iommufd_liveupdate_register_lufs(void);
> +int iommufd_liveupdate_unregister_lufs(void);
> +
>  int iommufd_hwpt_lu_set_preserve(struct iommufd_ucmd *ucmd);
> +static inline bool iopt_lu_map_immutable(const struct io_pagetable *iopt)
> +{
> +	return iopt->lu_map_immutable;
> +}
>  #else
> +static inline int iommufd_liveupdate_register_lufs(void)
> +{
> +	return 0;
> +}
> +
> +static inline int iommufd_liveupdate_unregister_lufs(void)
> +{
> +	return 0;
> +}
> +
>  static inline int iommufd_hwpt_lu_set_preserve(struct iommufd_ucmd *ucmd)
>  {
>  	return -ENOTTY;
>  }
> +
> +static inline bool iopt_lu_map_immutable(const struct io_pagetable *iopt)
> +{
> +	return false;
> +}
>  #endif
>  
>  #ifdef CONFIG_IOMMUFD_TEST
> diff --git a/drivers/iommu/iommufd/liveupdate.c b/drivers/iommu/iommufd/liveupdate.c
> index ae74f5b54735..ec11ae345fe7 100644
> --- a/drivers/iommu/iommufd/liveupdate.c
> +++ b/drivers/iommu/iommufd/liveupdate.c
> @@ -4,9 +4,15 @@
>  
>  #include <linux/file.h>
>  #include <linux/iommufd.h>
> +#include <linux/kexec_handover.h>
> +#include <linux/kho/abi/iommufd.h>
>  #include <linux/liveupdate.h>
> +#include <linux/iommu-lu.h>
> +#include <linux/mm.h>
> +#include <linux/pci.h>
>  
>  #include "iommufd_private.h"
> +#include "io_pagetable.h"
>  
>  int iommufd_hwpt_lu_set_preserve(struct iommufd_ucmd *ucmd)
>  {
> @@ -47,3 +53,297 @@ int iommufd_hwpt_lu_set_preserve(struct iommufd_ucmd *ucmd)
>  	return rc;
>  }
>  
> +static void iommufd_set_ioas_mutable(struct iommufd_ctx *ictx)
> +{
> +	struct iommufd_object *obj;
> +	struct iommufd_ioas *ioas;
> +	unsigned long index;
> +
> +	xa_lock(&ictx->objects);
> +	xa_for_each(&ictx->objects, index, obj) {
> +		if (obj->type != IOMMUFD_OBJ_IOAS)
> +			continue;
> +
> +		ioas = container_of(obj, struct iommufd_ioas, obj);
> +
> +		/*
> +		 * Not taking any IOAS lock here. All writers take LUO
> +		 * session mutex, and this writer racing with readers is not
> +		 * really a problem.
> +		 */
> +		WRITE_ONCE(ioas->iopt.lu_map_immutable, false);
> +	}
> +	xa_unlock(&ictx->objects);
> +}
> +
> +static int check_iopt_pages_preserved(struct liveupdate_session *s,
> +				      struct iommufd_hwpt_paging *hwpt)
> +{
> +	u32 req_seals = F_SEAL_SEAL | F_SEAL_GROW | F_SEAL_SHRINK;
> +	struct iopt_area *area;
> +	int ret;
> +
> +	for (area = iopt_area_iter_first(&hwpt->ioas->iopt, 0, ULONG_MAX); area;
> +	     area = iopt_area_iter_next(area, 0, ULONG_MAX)) {
> +		struct iopt_pages *pages = area->pages;
> +
> +		/* Only allow file based mapping */
> +		if (pages->type != IOPT_ADDRESS_FILE)
> +			return -EINVAL;

That's an important check! should we document it somewhere for the user
to know about it too? Perhaps in the uAPI header for the preserve IOCTL,
so VMM authors are aware of the anonymous memory restriction upfront?

> +
> +		/*
> +		 * When this memory file was mapped it should be sealed and seal
> +		 * should be sealed. This means that since mapping was done the
> +		 * memory file was not grown or shrink and the pages being used
> +		 * until now remain pinnned and preserved.
> +		 */
> +		if ((pages->seals & req_seals) != req_seals)
> +			return -EINVAL;
> +
> +		/* Make sure that the file was preserved. */
> +		ret = liveupdate_get_token_outgoing(s, pages->file, NULL);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int iommufd_save_hwpts(struct iommufd_ctx *ictx,
> +			      struct iommufd_lu *iommufd_lu,
> +			      struct liveupdate_session *session)
> +{
> +	struct iommufd_hwpt_paging *hwpt, **hwpts = NULL;
> +	struct iommu_domain_ser *domain_ser;
> +	struct iommufd_hwpt_lu *hwpt_lu;
> +	struct iommufd_object *obj;
> +	unsigned int nr_hwpts = 0;
> +	unsigned long index;
> +	unsigned int i;
> +	int rc = 0;
> +
> +	if (iommufd_lu) {
> +		hwpts = kcalloc(iommufd_lu->nr_hwpts, sizeof(*hwpts),
> +				GFP_KERNEL);
> +		if (!hwpts)
> +			return -ENOMEM;
> +	}
> +
> +	xa_lock(&ictx->objects);
> +	xa_for_each(&ictx->objects, index, obj) {
> +		if (obj->type != IOMMUFD_OBJ_HWPT_PAGING)
> +			continue;
> +
> +		hwpt = container_of(obj, struct iommufd_hwpt_paging, common.obj);
> +		if (!hwpt->lu_preserve)
> +			continue;
> +
> +		if (hwpt->ioas) {
> +			/*
> +			 * Obtain exclusive access to the IOAS and IOPT while we
> +			 * set immutability
> +			 */
> +			mutex_lock(&hwpt->ioas->mutex);

Doubling down on Ankit's comment here, we should absolutely avoid taking
sleepable locks in atomic context. I can attempt testing the next series
with lockdep enabled.

> +			down_write(&hwpt->ioas->iopt.domains_rwsem);
> +			down_write(&hwpt->ioas->iopt.iova_rwsem);
> +
> +			hwpt->ioas->iopt.lu_map_immutable = true;
> +
> +			up_write(&hwpt->ioas->iopt.iova_rwsem);
> +			up_write(&hwpt->ioas->iopt.domains_rwsem);
> +			mutex_unlock(&hwpt->ioas->mutex);

Additionally, I'm wondering if this could be a helper function?
iommufd_ioas_set_immutable?

> +		}
> +
> +		if (!hwpt->common.domain) {
> +			rc = -EINVAL;
> +			xa_unlock(&ictx->objects);
> +			goto out;
> +		}
> +
> +		if (!iommufd_lu) {
> +			rc = check_iopt_pages_preserved(session, hwpt);

This seems slightly redundant, we are calling check_iopt_pages_preserved
for every single preserved HWPT. However, multiple HWPTs can share the 
same underlying IOAS. 

If a VMM has 5 devices sharing a single IOAS, this code will walk the
exact same iopt_area_itree and validate the exact same memfd seals 5
times. Should we perhaps track which iopts have already been validated
during the loop to avoid this redundant tree traversal?

> +			if (rc) {
> +				xa_unlock(&ictx->objects);
> +				goto out;
> +			}
> +		} else if (iommufd_lu) {
> +			hwpts[nr_hwpts] = hwpt;
> +			hwpt_lu = &iommufd_lu->hwpts[nr_hwpts];
> +
> +			hwpt_lu->token = hwpt->lu_token;
> +			hwpt_lu->reclaimed = false;
> +		}
> +
> +		nr_hwpts++;
> +	}
> +	xa_unlock(&ictx->objects);
> +
> +	if (WARN_ON(iommufd_lu && iommufd_lu->nr_hwpts != nr_hwpts)) {
> +		rc = -EFAULT;
> +		goto out;
> +	}
> +
> +	if (iommufd_lu) {
> +		/*
> +		 * iommu_domain_preserve may sleep and must be called
> +		 * outside of xa_lock
> +		 */
> +		for (i = 0; i < nr_hwpts; i++) {
> +			hwpt = hwpts[i];
> +			hwpt_lu = &iommufd_lu->hwpts[i];
> +
> +			rc = iommu_domain_preserve(hwpt->common.domain, &domain_ser);
> +			if (rc < 0)
> +				goto out;
> +
> +			hwpt_lu->domain_data = __pa(domain_ser);
> +		}
> +	}
> +
> +	rc = nr_hwpts;
> +
> +out:
> +	kfree(hwpts);
> +	return rc;
> +}
> +
> +static int iommufd_liveupdate_preserve(struct liveupdate_file_op_args *args)
> +{
> +	struct iommufd_ctx *ictx = iommufd_ctx_from_file(args->file);
> +	struct iommufd_lu *iommufd_lu;
> +	size_t serial_size;
> +	void *mem;
> +	int rc;
> +
> +	if (IS_ERR(ictx))
> +		return PTR_ERR(ictx);
> +
> +	rc = iommufd_save_hwpts(ictx, NULL, args->session);
> +	if (rc < 0)
> +		goto err_ioas_mutable;
> +
> +	serial_size = struct_size(iommufd_lu, hwpts, rc);
> +
> +	mem = kho_alloc_preserve(serial_size);
> +	if (!mem) {
> +		rc = -ENOMEM;
> +		goto err_ioas_mutable;
> +	}
> +
> +	iommufd_lu = mem;
> +	iommufd_lu->nr_hwpts = rc;
> +	rc = iommufd_save_hwpts(ictx, iommufd_lu, args->session);

We call iommufd_save_hwpts twice (first to count/validate & second to 
serialize). Because xa_lock is dropped between these two calls to 
allocate KHO memory, we have a TOCTOU race.

If userspace concurrently creates or destroys a preserved HWPT during
this window, nr_hwpts will change, and the second pass will hit:
WARN_ON(iommufd_lu && iommufd_lu->nr_hwpts != nr_hwpts)

I'm not sure if that's a situation to WARN? Also, what about kernels
which have panic_on_warn?

> +	if (rc < 0)
> +		goto err_free;
> +
> +	args->serialized_data = virt_to_phys(iommufd_lu);
> +	iommufd_ctx_put(ictx);
> +	return 0;
> +
> +err_free:
> +	kho_unpreserve_free(mem);
> +err_ioas_mutable:
> +	iommufd_set_ioas_mutable(ictx);
> +	iommufd_ctx_put(ictx);
> +	return rc;
> +}
> +
> +static int iommufd_liveupdate_freeze(struct liveupdate_file_op_args *args)
> +{
> +	/* No-Op; everything should be made read-only */

Is there a plan to populate this in a future series? If it's a permanent
No-Op because the lu_map_immutable flag already handles the read-only
enforcement during the preserve phase, we should probably update the
comment to explicitly state that, rather than implying there is missing
logic?

> +	return 0;
> +}
> +
 
[ ---- >8 ----- ]

> diff --git a/include/linux/kho/abi/iommufd.h b/include/linux/kho/abi/iommufd.h
> new file mode 100644
> index 000000000000..f7393ac78aa9
> --- /dev/null
> +++ b/include/linux/kho/abi/iommufd.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Copyright (C) 2025, Google LLC
> + * Author: Samiullah Khawaja <skhawaja@google.com>
> + */
> +
> +#ifndef _LINUX_KHO_ABI_IOMMUFD_H
> +#define _LINUX_KHO_ABI_IOMMUFD_H
> +
> +#include <linux/mutex_types.h>
> +#include <linux/compiler.h>
> +#include <linux/types.h>
> +
> +/**
> + * DOC: IOMMUFD Live Update ABI
> + *
> + * This header defines the ABI for preserving the state of an IOMMUFD file
> + * across a kexec reboot using LUO.
> + *
> + * This interface is a contract. Any modification to any of the serialization
> + * structs defined here constitutes a breaking change. Such changes require
> + * incrementing the version number in the IOMMUFD_LUO_COMPATIBLE string.
> + */
> +
> +#define IOMMUFD_LUO_COMPATIBLE "iommufd-v1"
> +
> +struct iommufd_hwpt_lu {
> +	u32 token;
> +	u64 domain_data;
> +	bool reclaimed;
> +} __packed;
> +

Nit: Because of __packed, putting the u64 after the u32 forces an
unaligned 8-byte access at offset 4. Also, it's generally safer to use
explicitly sized types like u8 instead of bool for cross-kernel ABI
structs to avoid any compiler-specific sizing ambiguities. (This is the
reason most uAPI defs avoid using bool)

We can achieve natural alignment without implicit padding by ordering it
from largest to smallest and explicitly padding it out:

struct iommufd_hwpt_lu {
	__u64 domain_data;
	__u32 token;
	__u8 reclaimed;
	__u8 padding[3];
} __packed;

This guarantees an exact 16-byte footprint with perfectly aligned 64-bit
and 32-bit accesses.

> +struct iommufd_lu {
> +	unsigned int nr_hwpts;
> +	struct iommufd_hwpt_lu hwpts[];
> +};

Same here regarding explicitly sized types and packing (as pointed by
Vipin too)

> +
> +#endif /* _LINUX_KHO_ABI_IOMMUFD_H */

Thanks,
Praan