Re: [PATCH v6 15/18] iommufd: Persist iommu hardware pagetables for live update

[Baolu Lu <baolu.lu@linux.intel.com>]

On 10/25/2026 10:19 AM, Samiullah Khawaja wrote:
> From: YiFei Zhu <zhuyifei@google.com>
> 
> Register iommufd with the LUO framework and implement the preserve and
> unpreserve ops to save marked HWPTs.
> 
> To make sure mappings do not change during preserved state, add a
> liveupdate_immutable flag to IOAS. When an HWPT is preserved, its IOAS
> is marked immutable and any map/unmap attempts will fail with -EBUSY.
> This is synchronized using the domains_rwsem to prevent races with
> concurrent mapping operations.
> 
> The preserve callback iterates over the marked HWPTs, verifies that the
> backing memory pages are preserved, and calls iommu_preserve_domain() to
> preserve the associated IOMMU domain.
> 
> Signed-off-by: YiFei Zhu <zhuyifei@google.com>
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
> ---
>   MAINTAINERS                             |   1 +
>   drivers/iommu/iommufd/io_pagetable.c    |  24 ++
>   drivers/iommu/iommufd/iommufd_private.h |  16 ++
>   drivers/iommu/iommufd/liveupdate.c      | 327 ++++++++++++++++++++++++
>   drivers/iommu/iommufd/main.c            |  10 +-
>   include/linux/kho/abi/iommufd.h         |  56 ++++
>   6 files changed, 433 insertions(+), 1 deletion(-)
>   create mode 100644 include/linux/kho/abi/iommufd.h
> 

[...]
> diff --git a/drivers/iommu/iommufd/liveupdate.c b/drivers/iommu/iommufd/liveupdate.c
> index 5fb491ff3a3d..828203d7254c 100644
> --- a/drivers/iommu/iommufd/liveupdate.c
> +++ b/drivers/iommu/iommufd/liveupdate.c
> @@ -9,9 +9,28 @@
>   
>   #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-liveupdate.h>
> +#include <linux/mm.h>
> +#include <linux/pci.h>
>   
>   #include "iommufd_private.h"
> +#include "io_pagetable.h"
> +
> +static u32 iopt_inc_num_preserved_domains(struct io_pagetable *iopt)
> +{
> +	guard(rwsem_write)(&iopt->domains_rwsem);
> +	return ++iopt->num_preserved_domains;
> +}
> +
> +static void iopt_dec_num_preserved_domains(struct io_pagetable *iopt)
> +{
> +	guard(rwsem_write)(&iopt->domains_rwsem);
> +	if (!WARN_ON(!iopt->num_preserved_domains))
> +		iopt->num_preserved_domains--;
> +}
>   
>   int iommufd_hwpt_liveupdate_mark_preserve(struct iommufd_ucmd *ucmd)
>   {
> @@ -70,3 +89,311 @@ int iommufd_hwpt_liveupdate_mark_preserve(struct iommufd_ucmd *ucmd)
>   	iommufd_put_object(ictx, &hwpt->common.obj);
>   	return rc;
>   }
> +
> +static int iopt_area_validate_file_pages(struct iopt_area *area)
> +{
> +	unsigned long last_index = iopt_area_last_index(area);
> +	unsigned long index = iopt_area_index(area);
> +	unsigned int page_offset = area->page_offset;
> +	struct iopt_pages *pages = area->pages;
> +	unsigned long iova = iopt_area_iova(area);
> +
> +	guard(mutex)(&pages->mutex);
> +	if (WARN_ON(!area->storage_domain))
> +		return -EINVAL;
> +
> +	while (index <= last_index) {
> +		phys_addr_t phys = iommu_iova_to_phys(area->storage_domain,
> +						      iova) - page_offset;

iommu_iova_to_phys() returns 0 when there is no underlying DMA
translation. It would be better to check for !phys and return -EINVAL
before passing it to page_folio():

		if (!phys)
			return -EINVAL;

> +		struct folio *folio = page_folio(pfn_to_page(PHYS_PFN(phys)));
> +
> +		if (folio->mapping != pages->file->f_mapping)
> +			return -EINVAL;
> +
> +		iova += PAGE_SIZE - page_offset;

This walks a full page table per 4K page size, right? Isn't it slow for
large mappings? Why not jump ahead by folio_size()?

> +		page_offset = 0;
> +		index++;
> +	}
> +
> +	return 0;
> +}
> +
> +static int iopt_validate_preserved_memory(struct io_pagetable *iopt,
> +					  struct liveupdate_session *s)
> +{
> +	struct iopt_area *area;
> +	int ret = 0;
> +
> +	down_read(&iopt->iova_rwsem);
> +	for (area = iopt_area_iter_first(iopt, 0, ULONG_MAX); area;
> +	     area = iopt_area_iter_next(area, 0, ULONG_MAX)) {
> +		struct iopt_pages *pages = area->pages;
> +
> +		if (!pages)
> +			continue;
> +
> +		/* Only allow file based mapping */
> +		if (pages->type != IOPT_ADDRESS_FILE) {
> +			ret = -EINVAL;
> +			break;
> +		}
> +
> +		/* Make sure that the file was preserved. */
> +		ret = liveupdate_get_token_outgoing(s, pages->file, NULL);
> +		if (ret)
> +			break;
> +
> +		/*
> +		 * Make sure the mapped pages are still associated with the
> +		 * preserved memfd.
> +		 */
> +		ret = iopt_area_validate_file_pages(area);
> +		if (ret)
> +			break;
> +	}
> +	up_read(&iopt->iova_rwsem);
> +
> +	return ret;
> +}
> +
> +static int iommufd_preserve_hwpt(struct iommufd_hwpt_paging *hwpt,
> +				 struct iommufd_hwpt_ser *hwpt_ser,
> +				 struct liveupdate_session *session)
> +{
> +	struct iommu_domain_ser *domain_ser;
> +	int rc;
> +
> +	/*
> +	 * Make IOAS immutable so the DMA mappings do not change while
> +	 * the HWPT is preserved. Since one IOAS can have multiple
> +	 * HWPTs, if an error occurs this call needs to make the IOAS
> +	 * mutable again if it was the one that made it immutable.
> +	 */
> +	if (iopt_inc_num_preserved_domains(&hwpt->ioas->iopt) == 1) {
> +		rc = iopt_validate_preserved_memory(&hwpt->ioas->iopt, session);
> +		if (rc)
> +			goto err;
> +	}
> +
> +	hwpt_ser->size = sizeof(*hwpt_ser);
> +	hwpt_ser->token = hwpt->liveupdate_token;
> +
> +	rc = iommu_preserve_domain(hwpt->common.domain, &domain_ser);
> +	if (rc < 0)
> +		goto err;
> +
> +	hwpt_ser->domain_ser_phys = virt_to_phys(domain_ser);
> +	return 0;
> +
> +err:
> +	iopt_dec_num_preserved_domains(&hwpt->ioas->iopt);
> +	return rc;
> +}
> +
> +static void _iommufd_unpreserve(struct iommufd_ctx *ictx,
> +				struct iommufd_ser *ser)
> +{
> +	struct iommufd_hwpt_paging *hwpt;
> +	struct iommufd_object *obj;
> +	unsigned long index;
> +
> +	xa_lock(&ictx->objects);
> +	xa_for_each_marked(&ictx->objects, index, obj, IOMMUFD_OBJ_LIVEUPDATE_MARK) {
> +		if (obj->type != IOMMUFD_OBJ_HWPT_PAGING)
> +			continue;
> +
> +		hwpt = to_hwpt_paging(container_of(obj, struct iommufd_hw_pagetable, obj));
> +		if (!hwpt->liveupdate_preserved)
> +			continue;
> +
> +		xa_unlock(&ictx->objects);
> +
> +		iommu_unpreserve_domain(hwpt->common.domain);
> +		iopt_dec_num_preserved_domains(&hwpt->ioas->iopt);
> +
> +		hwpt->liveupdate_preserved = false;
> +		iommufd_put_object(ictx, obj);
> +
> +		xa_lock(&ictx->objects);
> +	}
> +	xa_unlock(&ictx->objects);
> +
> +	kho_unpreserve_free(ser);
> +}
> +
> +static int iommufd_liveupdate_preserve(struct liveupdate_file_op_args *args)
> +{
> +	struct iommufd_ctx *ictx;
> +	struct iommufd_hwpt_paging *hwpt;
> +	struct iommufd_ser *iommufd_ser;
> +	struct iommufd_object *obj;
> +	unsigned int num_hwpts;
> +	unsigned long index;
> +	unsigned int i;
> +	void *mem;
> +	int rc;
> +
> +	ictx = iommufd_ctx_from_file(args->file);
> +	if (IS_ERR(ictx))
> +		return PTR_ERR(ictx);
> +
> +	mutex_lock(&ictx->liveupdate_mutex);
> +
> +	/* Count the number of HWPTs to preserve */
> +	num_hwpts = 0;
> +	xa_lock(&ictx->objects);
> +	xa_for_each_marked(&ictx->objects, index, obj, IOMMUFD_OBJ_LIVEUPDATE_MARK) {
> +		if (obj->type != IOMMUFD_OBJ_HWPT_PAGING)
> +			continue;
> +
> +		hwpt = to_hwpt_paging(container_of(obj, struct iommufd_hw_pagetable, obj));
> +		if (!hwpt->common.domain) {
> +			rc = -EINVAL;
> +			xa_unlock(&ictx->objects);
> +			goto out_unlock;
> +		}
> +		num_hwpts++;
> +	}
> +	xa_unlock(&ictx->objects);
> +
> +	mem = kho_alloc_preserve(struct_size(iommufd_ser,
> +					     hwpt_array, num_hwpts));
> +	if (IS_ERR(mem)) {
> +		rc = PTR_ERR(mem);
> +		goto out_unlock;
> +	}
> +
> +	iommufd_ser = mem;
> +	iommufd_ser->size = sizeof(*iommufd_ser);
> +
> +	/* Preserve HWPTs */
> +	i = 0;
> +	xa_lock(&ictx->objects);
> +	xa_for_each_marked(&ictx->objects, index, obj, IOMMUFD_OBJ_LIVEUPDATE_MARK) {
> +		if (obj->type != IOMMUFD_OBJ_HWPT_PAGING)
> +			continue;
> +
> +		if (!iommufd_lock_obj(obj)) {
> +			rc = -ENOENT;
> +			xa_unlock(&ictx->objects);
> +			goto out_unpreserve;
> +		}
> +
> +		/*
> +		 * HWPT is locked so it will not be destroyed. The xarray lock
> +		 * can be released here before preserving the HWPT.
> +		 */
> +		xa_unlock(&ictx->objects);
> +		hwpt = to_hwpt_paging(container_of(obj, struct iommufd_hw_pagetable, obj));
> +		rc = iommufd_preserve_hwpt(hwpt, &iommufd_ser->hwpt_array[i++], args->session);
> +		if (rc) {
> +			iommufd_put_object(ictx, obj);
> +			goto out_unpreserve;
> +		}
> +
> +		/*
> +		 * Mark the HWPT as successfully preserved. This is distinct
> +		 * from IOMMUFD_OBJ_LIVEUPDATE_MARK, which only indicates the
> +		 * userspace intent to preserve.
> +		 */
> +		hwpt->liveupdate_preserved = true;

Can userspace re-mark an already-preserved HWPT with a new token? It
seems that mark_preserve() doesn't check hwpt->liveupdate_preserved.
Would it help to simply return -EBUSY from mark_preserve() once
hwpt->liveupdate_preserved is set?

> +		xa_lock(&ictx->objects);
> +	}
> +	xa_unlock(&ictx->objects);
> +
> +	/* Store the actual number of HWPTs that are preserved */
> +	iommufd_ser->num_hwpts = i;
> +
> +	args->serialized_data = virt_to_phys(iommufd_ser);
> +	mutex_unlock(&ictx->liveupdate_mutex);
> +	iommufd_ctx_put(ictx);
> +	return 0;
> +
> +out_unpreserve:
> +	_iommufd_unpreserve(ictx, iommufd_ser);
> +out_unlock:
> +	mutex_unlock(&ictx->liveupdate_mutex);
> +	iommufd_ctx_put(ictx);
> +	return rc;
> +}
> +
> +static void iommufd_liveupdate_unpreserve(struct liveupdate_file_op_args *args)
> +{
> +	struct iommufd_ctx *ictx;
> +
> +	ictx = iommufd_ctx_from_file(args->file);
> +	if (WARN_ON(IS_ERR(ictx)))
> +		return;
> +
> +	mutex_lock(&ictx->liveupdate_mutex);
> +	_iommufd_unpreserve(ictx, phys_to_virt(args->serialized_data));
> +	mutex_unlock(&ictx->liveupdate_mutex);
> +
> +	iommufd_ctx_put(ictx);
> +}
[...]

Thanks,
baolu