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

[Samiullah Khawaja <skhawaja@google.com>]

On Mon, Jun 22, 2026 at 11:10:55AM +0800, Baolu Lu wrote:
>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?

NULL indicates that no preserved state exists for this iommu HW, so
setup normally as cold boot. ERR_PTR() indicates a fatal parsing or
corruption error in the KHO tree that the driver needs to explicitly
handle or abort on.

liveupdate_flb_get_incoming() returns two different errors for two
different missing-data scenarios:

-ENODATA: The system did not perform a live update.
-ENOENT: The system did perform a live update, but no data was preserved
for this specific FLB.

 From the IOMMU driver's perspective, both scenarios simply mean "proceed
with a normal cold boot," which is why we intercept them and return
NULL.

But as you suggested later, I will add kdocs for these to document all
these things.
>
>>+
>>+	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.

Agreed. Will do.
>
>>+
>>  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
>

Thanks,
Sami