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