Re: [PATCH v2 03/16] iommu: Implement IOMMU domain preservation

[Pranjal Shrivastava <praan@google.com>]

On Mon, Apr 27, 2026 at 05:56:20PM +0000, Samiullah Khawaja wrote:
> Add IOMMU domain ops that can be implemented by the IOMMU drivers if
> they support IOMMU domain preservation across liveupdate. The new IOMMU
> domain preserve, unpreserve and restore APIs call these ops to perform
> respective live update operations.
> 
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
> ---
>  drivers/iommu/liveupdate.c       | 97 ++++++++++++++++++++++++++++++++
>  include/linux/iommu-liveupdate.h | 14 +++++
>  include/linux/iommu.h            | 13 +++++
>  3 files changed, 124 insertions(+)
> 
> diff --git a/drivers/iommu/liveupdate.c b/drivers/iommu/liveupdate.c
> index a26099b145c3..f71f14518248 100644
> --- a/drivers/iommu/liveupdate.c
> +++ b/drivers/iommu/liveupdate.c
> @@ -13,6 +13,9 @@
>  #include <linux/iommu.h>
>  #include <linux/errno.h>
>  
> +#define iommu_max_objs_per_page(_array) \
> +	((PAGE_SIZE - sizeof(struct iommu_array_hdr_ser)) / sizeof((_array)->objects[0]))
> +
>  static void *iommu_liveupdate_restore_array(u64 array_phys)
>  {
>  	struct iommu_array_hdr_ser *array_hdr;
> @@ -196,3 +199,97 @@ void iommu_liveupdate_unregister_flb(struct liveupdate_file_handler *handler)
>  	liveupdate_unregister_flb(handler, &iommu_flb);
>  }
>  EXPORT_SYMBOL(iommu_liveupdate_unregister_flb);
> +
> +static int alloc_object_ser(struct iommu_array_hdr_ser **curr_array_ptr, u64 max_objs)
> +{
> +	struct iommu_array_hdr_ser *curr_array = *curr_array_ptr;
> +	struct iommu_array_hdr_ser *next_array;
> +
> +	if (curr_array->nr_objects >= max_objs) {
> +		next_array = kho_alloc_preserve(PAGE_SIZE);
> +		if (IS_ERR(next_array))
> +			return PTR_ERR(next_array);
> +
> +		curr_array->next_array_phys = virt_to_phys(next_array);
> +		*curr_array_ptr = next_array;
> +		curr_array = next_array;
> +	}
> +
> +	return curr_array->nr_objects++;
> +}
> +
> +static struct iommu_domain_ser *alloc_iommu_domain_ser(struct iommu_flb_obj *flb)
> +{
> +	int idx;
> +
> +	idx = alloc_object_ser((struct iommu_array_hdr_ser **)&flb->curr_domain_array,

Nit: Such type-casts could be brittle and risk strict-aliasing warnings
based on the compiler. 

Since alloc_object_ser only updates the top-level tracking pointer, we 
can make this much cleaner by changing alloc_object_ser to take a 
void curr_array_ptr. That way, we can pass (void )&flb->curr_domain_array
cleanly without the double-pointer type-punning. Something like:

static int alloc_object_ser(void **curr_array_ptr, u64 max_objs)
{
	struct iommu_array_hdr_ser *curr_array = *curr_array_ptr;
	struct iommu_array_hdr_ser *next_array;

	...
}

and we can pass it as:

idx = alloc_object_ser((void **)&flb->curr_domain_array,
		       iommu_max_objs_per_page(flb->curr_domain_array));

> +			       iommu_max_objs_per_page(flb->curr_domain_array));
> +	if (idx < 0)
> +		return ERR_PTR(idx);
> +
> +	flb->curr_domain_array->objects[idx].hdr.ref_count = 1;
> +	return &flb->curr_domain_array->objects[idx];
> +}
> +
> +int iommu_domain_preserve(struct iommu_domain *domain, struct iommu_domain_ser **ser)
> +{
> +	struct iommu_domain_ser *domain_ser;
> +	struct iommu_flb_obj *flb_obj;
> +	int ret;
> +
> +	if (!domain->ops->preserve)
> +		return -EOPNOTSUPP;
> +
> +	ret = liveupdate_flb_get_outgoing(&iommu_flb, (void **)&flb_obj);
> +	if (ret)
> +		return ret;
> +
> +	guard(mutex)(&flb_obj->lock);
> +	domain_ser = alloc_iommu_domain_ser(flb_obj);
> +	if (IS_ERR(domain_ser))
> +		return PTR_ERR(domain_ser);
> +
> +	ret = domain->ops->preserve(domain, domain_ser);
> +	if (ret) {
> +		domain_ser->hdr.deleted = true;

Nit: This will become domain_ser->hdr.flags |= IOMMU_SER_FLAG_DELETED;
once we convert the bitfields in PATCH 2 to flags.

> +		return ret;
> +	}
> +
> +	domain->preserved_state = domain_ser;
> +	*ser = domain_ser;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(iommu_domain_preserve);
> +

[...]

>  static inline bool iommu_is_dma_domain(struct iommu_domain *domain)
> @@ -752,6 +757,11 @@ struct iommu_ops {
>   *                           specific mechanisms.
>   * @set_pgtable_quirks: Set io page table quirks (IO_PGTABLE_QUIRK_*)
>   * @free: Release the domain after use.
> + * @preserve: Preserve the iommu domain for liveupdate.
> + *            Returns 0 on success, a negative errno on failure.
> + * @unpreserve: Unpreserve the iommu domain that was preserved earlier.
> + * @restore: Restore the iommu domain after liveupdate.
> + *           Returns 0 on success, a negative errno on failure.
>   */
>  struct iommu_domain_ops {
>  	int (*attach_dev)(struct iommu_domain *domain, struct device *dev,
> @@ -782,6 +792,9 @@ struct iommu_domain_ops {
>  				  unsigned long quirks);
>  
>  	void (*free)(struct iommu_domain *domain);
> +	int (*preserve)(struct iommu_domain *domain, struct iommu_domain_ser *ser);
> +	void (*unpreserve)(struct iommu_domain *domain, struct iommu_domain_ser *ser);
> +	int (*restore)(struct iommu_domain *domain, struct iommu_domain_ser *ser);

Nit: Should we add restore in the patch we implement it?

>  };

With the above nits,

Reviewed-by: Pranjal Shrivastava <praan@google.com>

Thanks,
Praan