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

[Nicolin Chen <nicolinc@nvidia.com>]

On Mon, Sep 21, 2026 at 12:48:31AM +0000, Samiullah Khawaja wrote:
> diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
> index a33b32708afa..a4ddc29ec5ae 100644
> --- a/drivers/iommu/iommufd/iommufd_private.h
> +++ b/drivers/iommu/iommufd/iommufd_private.h
> @@ -98,6 +98,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
> +	u32 nr_preserved_domains;
> +#endif

In iommufd, "num_" is used more often than "nr_".

>  	unsigned long iova_alignment;
>  };
>  
> @@ -398,6 +401,7 @@ struct iommufd_hwpt_paging {
>  	bool enforce_cache_coherency : 1;
>  	bool nest_parent : 1;
>  #ifdef CONFIG_IOMMU_LIVEUPDATE
> +	bool liveupdate_preserved;
>  	u64 liveupdate_token;
>  #endif
>  	/* Head at iommufd_ioas::hwpt_list */

> +static inline bool iopt_liveupdate_immutable(const struct io_pagetable *iopt)
> +{
> +	return iopt->nr_preserved_domains > 0;
> +}

It's used in io_pagetable file only. I'd move it out of the header:

static inline bool iopt_liveupdate_immutable(const struct io_pagetable *iopt)
{
	bool immutable = false;

	lockdep_assert_held(&iopt->domains_rwsem);

#ifdef CONFIG_IOMMU_LIVEUPDATE
	/* iopt becomes immutable once liveupdate preservation starts */
	immutable = iopt->num_preserved_domains;
#endif
	return immutable;
}

> diff --git a/drivers/iommu/iommufd/liveupdate.c b/drivers/iommu/iommufd/liveupdate.c
> +static bool ioas_set_immutable(struct iommufd_ioas *ioas, bool set)
> +{
> +	bool was_immutable;
> +
> +	down_write(&ioas->iopt.domains_rwsem);
> +	was_immutable = ioas->iopt.nr_preserved_domains > 0;
> +	if (set)
> +		ioas->iopt.nr_preserved_domains++;
> +	else if (!WARN_ON(!was_immutable))
> +		ioas->iopt.nr_preserved_domains--;
> +
> +	up_write(&ioas->iopt.domains_rwsem);
> +
> +	return was_immutable;
> +}

Since it has to handle refcount, an unset doesn't really unset the
flag, which is confusing.

Maybe just:
  iopt_inc_num_preserved_domains
  iopt_dec_num_preserved_domains

> +static int check_iopt_pages_preserved(struct liveupdate_session *s,
> +				      struct iommufd_hwpt_paging *hwpt)

static int iopt_validate_preserved_memory(struct io_pagetable *iopt,
					  struct liveupdate_session *s)

> +{
> +	u32 req_seals = F_SEAL_SEAL | F_SEAL_GROW | F_SEAL_SHRINK;
> +	struct iopt_area *area;
> +	int ret = 0;
> +
> +	down_read(&hwpt->ioas->iopt.iova_rwsem);
> +	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;

Move req_seals inside the for loop.

> +static int iommufd_liveupdate_preserve(struct liveupdate_file_op_args *args)
[...]
> +	iommufd_ser = mem;
> +	iommufd_ser->nr_hwpts = nr_hwpts;
> +
> +	/* Preserve HWPTs */
> +	i = 0;
> +	xa_lock(&ictx->objects);
> +	xa_for_each_marked(&ictx->objects, index, obj, IOMMUFD_OBJ_LIVEUPDATE_MARK) {
[...]
> +	}
> +	xa_unlock(&ictx->objects);
> +
> +	/* Store the actual number of HWPTs that are preserved */
> +	iommufd_ser->nr_hwpts = i;

iommufd_ser->nr_hwpts is set twice. First one seems redundant.

> +++ b/include/linux/kho/abi/iommufd.h
> +/**
> + * struct iommu_hwpt_ser - IOMMUFD HWPT serialized state
> + * @domain_data: Physical address of the serialized state of associated domain

Then, it could be domain_ser_phys?

> + * @token: User provided token
> + * @reclaimed: Whether the HWPT is reclaimed

Once the hwpt is reclaimed, could we clear domain_data to indicate
"it is reclaimed" instead of a separate field?

Nicolin