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

From: Nicolin Chen

Date: Wed Oct 07 2026 - 17:32:40 EST


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