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

[Samiullah Khawaja <skhawaja@google.com>]

On Sat, Oct 31, 2026 at 10:46:56AM +0800, Baolu Lu wrote:
>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():

Agreed. I will also move the page_offset subtraction after the check.
>
>		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()?

Yes. When filling a domain, iommufd reuses pages that are already pinned
(by an access or another domain). If the memfd was punched in between,
an old page from a different folio can sit in the middle of a folio
range, and jumping by folio_size() would skip it. I will add a comment.
Also see batch_from_domain() implementation, it also does a similar page
walk.

In parallel, I sent a proposal for memfd to block or track hole punch,
so this walk can be skipped. It can be a follow up.
>
>>+		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?

Yes, I will handle this case.
>
>>+		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

Thanks,
Sami