Re: [PATCH v3 09/18] iommu: Add APIs to get iommu and device preserved state

[Baolu Lu <baolu.lu@linux.intel.com>]

On 6/15/26 07:37, 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.
> 
> Reviewed-by: Pranjal Shrivastava <praan@google.com>
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
> ---
>   drivers/iommu/liveupdate.c       | 75 ++++++++++++++++++++++++++++++++
>   include/linux/iommu-liveupdate.h | 31 +++++++++++++
>   2 files changed, 106 insertions(+)
> 
> diff --git a/drivers/iommu/liveupdate.c b/drivers/iommu/liveupdate.c
> index 2d782023407d..90750e78cd2a 100644
> --- a/drivers/iommu/liveupdate.c
> +++ b/drivers/iommu/liveupdate.c
> @@ -18,6 +18,17 @@
>   #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 ((_idx) = 0, (_obj) = (_arr)->objects;			\
> +	     (_idx) < (_arr)->hdr.nr_objects; (_idx)++, (_obj)++)	\
> +		if (((_obj)->hdr.flags & IOMMU_SER_FLAG_DELETED))	\
> +			continue;					\
> +		else
> +
> +#define iommu_liveupdate_for_each_arr(_arr)				\
> +	for (; (_arr); (_arr) = (_arr)->hdr.next_array_phys ?		\
> +	     phys_to_virt((_arr)->hdr.next_array_phys) : NULL)
> +
>   struct iommu_flb_obj {
>   	struct mutex lock;
>   	struct iommu_flb_ser *ser;
> @@ -216,6 +227,70 @@ 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;
> +
> +	if (!flb_obj->ser->device_array_phys)
> +		return -ENOENT;
> +
> +	array = phys_to_virt(flb_obj->ser->device_array_phys);
> +	iommu_liveupdate_for_each_arr(array) {
> +		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);
> +
> +struct iommu_hw_ser *iommu_get_preserved_data(u64 token, enum iommu_type_ser type)
> +{
> +	struct iommu_hw_ser *iommu_ser = NULL;
> +	struct iommu_hw_array_ser *array;
> +	struct iommu_flb_obj *flb_obj;
> +	int ret, idx;
> +
> +	ret = liveupdate_flb_get_incoming(&iommu_flb, (void **)&flb_obj);
> +	if (ret == -ENODATA || ret == -ENOENT)
> +		return NULL;
> +
> +	if (ret)
> +		return ERR_PTR(ret);

Why does this helper return both NULL and an ERR_PTR()? Is NULL
considered an error case here? Also, why are -ENODATA and -ENOENT
treated differently?

> +
> +	if (!flb_obj->ser->iommu_array_phys) {
> +		iommu_ser = ERR_PTR(-EINVAL);
> +		goto out;
> +	}
> +
> +	array = phys_to_virt(flb_obj->ser->iommu_array_phys);
> +	iommu_liveupdate_for_each_arr(array) {
> +		iommu_liveupdate_for_each_obj(array, iommu_ser, idx) {
> +			if (iommu_ser->token == token && iommu_ser->type == type)
> +				goto out;
> +		}
> +	}
> +
> +	iommu_ser = NULL;
> +out:
> +	liveupdate_flb_put_incoming(&iommu_flb);
> +	return iommu_ser;
> +}
> +EXPORT_SYMBOL(iommu_get_preserved_data);

These two helpers are intended for use by IOMMU drivers, correct? If so,
please add kernel-doc style comments to document their usage and guide
how drivers should use them.

> +
>   static int alloc_object_ser(void **curr_array_ptr, u64 max_objs)
>   {
>   	struct iommu_array_hdr_ser *curr_array = *curr_array_ptr;
> diff --git a/include/linux/iommu-liveupdate.h b/include/linux/iommu-liveupdate.h
> index f9528c94979d..5ad006892cbd 100644
> --- a/include/linux/iommu-liveupdate.h
> +++ b/include/linux/iommu-liveupdate.h
> @@ -13,6 +13,8 @@
>   #include <linux/liveupdate.h>
>   #include <linux/kho/abi/iommu.h>
>   
> +typedef int (*iommu_preserved_device_iter_fn)(struct iommu_device_ser *ser,
> +					      void *arg);
>   #ifdef CONFIG_IOMMU_LIVEUPDATE
>   static inline void *dev_iommu_preserved_state(struct device *dev)
>   {
> @@ -28,6 +30,20 @@ static inline void *dev_iommu_preserved_state(struct device *dev)
>   	return NULL;
>   }
>   
> +static inline void *iommu_domain_restored_state(struct iommu_domain *domain)
> +{
> +	struct iommu_domain_ser *ser;
> +
> +	ser = domain->preserved_state;
> +	if (ser && (ser->hdr.flags & IOMMU_SER_FLAG_INCOMING))
> +		return ser;
> +
> +	return NULL;
> +}
> +
> +int iommu_for_each_preserved_device(iommu_preserved_device_iter_fn fn,
> +				    void *arg);
> +struct iommu_hw_ser *iommu_get_preserved_data(u64 token, enum iommu_type_ser type);
>   int iommu_preserve_domain(struct iommu_domain *domain, struct iommu_domain_ser **ser);
>   void iommu_unpreserve_domain(struct iommu_domain *domain);
>   int iommu_preserve_device(struct iommu_domain *domain,
> @@ -44,6 +60,21 @@ static inline void *dev_iommu_preserved_state(struct device *dev)
>   	return NULL;
>   }
>   
> +static inline void *iommu_domain_restored_state(struct iommu_domain *domain)
> +{
> +	return NULL;
> +}
> +
> +static inline int iommu_for_each_preserved_device(iommu_preserved_device_iter_fn fn, void *arg)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline struct iommu_hw_ser *iommu_get_preserved_data(u64 token, enum iommu_type_ser type)
> +{
> +	return NULL;
> +}
> +
>   static inline int iommu_preserve_domain(struct iommu_domain *domain, struct iommu_domain_ser **ser)
>   {
>   	return -EOPNOTSUPP;

Thanks,
baolu