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

[Samiullah Khawaja <skhawaja@google.com>]

On Mon, May 18, 2026 at 01:13:49PM +0000, Pranjal Shrivastava wrote:
>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));
>

Agreed. I will update this (including other places).
>> +			       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.

Agreed. I think it is being discussed that bitfields are fine in the
other thread, but I will switch it to flags for readibility anyway.
>
>> +		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?

The ops are implemented in iommupt (separate scope) and it is done after
the iommu pages patch as that is also required for it.
>
>>  };
>
>With the above nits,
>
>Reviewed-by: Pranjal Shrivastava <praan@google.com>

Thanks Pranjal.
>
>Thanks,
>Praan

Sami