Re: [PATCH v2 08/16] iommu: Add APIs to get iommu and device preserved state

[Pranjal Shrivastava]

On Mon, Apr 27, 2026 at 05:56:25PM +0000, Samiullah Khawaja wrote:
> The preserved state of the device and IOMMU needs to be fetched during
> shutdown and boot in the next kernel. Add APIs that can be used to fetch
> the preserved state of a device and IOMMU. The APIs will only be used
> during shutdown and after liveupdate so no locking needed.
> 
> Signed-off-by: Samiullah Khawaja <skhawaja@xxxxxxxxxx>
> ---
>  drivers/iommu/liveupdate.c       | 57 ++++++++++++++++++++++++++++++++
>  include/linux/iommu-liveupdate.h | 31 +++++++++++++++++
>  2 files changed, 88 insertions(+)
> 
> diff --git a/drivers/iommu/liveupdate.c b/drivers/iommu/liveupdate.c
> index 765d042e22e3..60ee29b0c6bd 100644
> --- a/drivers/iommu/liveupdate.c
> +++ b/drivers/iommu/liveupdate.c
> @@ -17,6 +17,14 @@
>  #define iommu_max_objs_per_page(_array) \
>  	((PAGE_SIZE - sizeof(struct iommu_array_hdr_ser)) / sizeof((_array)->objects[0]))
>  
> +#define iommu_liveupdate_for_each_obj(_arr, _obj, _idx)				\
> +	for (; (_arr);								\
> +	     (_arr) = (_arr)->hdr.next_array_phys ?				\
> +	     phys_to_virt((_arr)->hdr.next_array_phys) : NULL)			\
> +		for ((_idx) = 0, (_obj) = (_arr)->objects;			\
> +		     (_idx) < (_arr)->hdr.nr_objects; (_idx)++, (_obj)++)	\
> +			if (!(_obj)->hdr.deleted)
> +

Nit: While the usage of this macro is safe right now because we use 
goto out; to short-circuit the scan. Such naked nested loops inside an
iterator macro can be a bit of a trap for the future. 

If anyone tries to reuse this macro down the road and uses a break; 
statement, it will only break the innermost object loop and continue 
scanning subsequent array blocks.

We could nest two macros or it as a helper fn to prevent such bugs.


>  static void *iommu_liveupdate_restore_array(u64 array_phys)
>  {
>  	struct iommu_array_hdr_ser *array_hdr;
> @@ -201,6 +209,55 @@ void iommu_liveupdate_unregister_flb(struct liveupdate_file_handler *handler)
>  }
>  EXPORT_SYMBOL(iommu_liveupdate_unregister_flb);
>  
> +int iommu_for_each_preserved_device(iommu_preserved_device_iter_fn fn,
> +				    void *arg)
> +{
> +	struct iommu_flb_obj *flb_obj;
> +	struct iommu_device_array_ser *array;
> +	struct iommu_device_ser *device_ser;
> +	int ret, idx;
> +
> +	ret = liveupdate_flb_get_incoming(&iommu_flb, (void **)&flb_obj);
> +	if (ret)
> +		return -ENOENT;
> +
> +	array = phys_to_virt(flb_obj->ser->device_array_phys);

Nit: phys_to_virt doesn't return an error for NULL. While we shouldn't
be in a case where serialized ptrs are NULL. I'm wondering how painful
catching such bugs would be.. should we check for NULL phys_addr here?
(and at other places)

> +	iommu_liveupdate_for_each_obj(array, device_ser, idx) {
> +		ret = fn(device_ser, arg);
> +		if (ret)
> +			goto out;
> +	}
> +
> +out:
> +	liveupdate_flb_put_incoming(&iommu_flb);
> +	return ret;
> +}
> +EXPORT_SYMBOL(iommu_for_each_preserved_device);
> +
 
With those two nits, 

Reviewed-by: Pranjal Shrivastava <praan@xxxxxxxxxx>