Re: [PATCH v6 05/18] iommu: Implement device and IOMMU HW preservation

[Samiullah Khawaja <skhawaja@google.com>]

On Sat, Oct 31, 2026 at 01:34:00PM +0800, Baolu Lu wrote:
>On 10/31/2026 9:45 AM, Samiullah Khawaja wrote:
>>On Fri, Oct 30, 2026 at 01:12:31PM +0800, Baolu Lu wrote:
>>>On 10/25/2026 10:19 AM, Samiullah Khawaja wrote:
>>>>Add IOMMU ops to preserve/unpreserve the device state handled by the
>>>>IOMMU driver. This is IOMMU specific state of the device that needs to
>>>>be preserved for liveupdate. These ops can be implemented by the IOMMU
>>>>drivers that support preservation of devices that have their IOMMU
>>>>domains preserved. During device preservation the state of the
>>>>associated IOMMU is also preserved as dependency.
>>>>
>>>>Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
>>>>---
>>>> drivers/iommu/iommu-priv.h       |   2 +
>>>> drivers/iommu/iommu.c            |  72 +++++++++++-
>>>> drivers/iommu/liveupdate.c       | 189 +++++++++++++++++++++++++++++++
>>>> include/linux/iommu-liveupdate.h |  76 +++++++++++++
>>>> include/linux/iommu.h            |  33 ++++++
>>>> 5 files changed, 371 insertions(+), 1 deletion(-)
>>>>
>>>>diff --git a/drivers/iommu/iommu-priv.h b/drivers/iommu/iommu-priv.h
>>>>index 1f9f886b2c10..6b0ef210b32f 100644
>>>>--- a/drivers/iommu/iommu-priv.h
>>>>+++ b/drivers/iommu/iommu-priv.h
>>>>@@ -21,6 +21,8 @@ static inline const struct iommu_ops 
>>>>*dev_iommu_ops(struct device *dev)
>>>> void dev_iommu_free(struct device *dev);
>>>>+bool iommu_group_is_singleton(struct iommu_group *group);
>>>>+
>>>> const struct iommu_ops *iommu_ops_from_fwnode(const struct 
>>>>fwnode_handle *fwnode);
>>>> static inline const struct iommu_ops *iommu_fwspec_ops(struct 
>>>>iommu_fwspec *fwspec)
>>>>diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>>>>index b486b8bbd1fc..a66363789ca9 100644
>>>>--- a/drivers/iommu/iommu.c
>>>>+++ b/drivers/iommu/iommu.c
>>>>@@ -18,6 +18,7 @@
>>>> #include <linux/errno.h>
>>>> #include <linux/host1x_context_bus.h>
>>>> #include <linux/iommu.h>
>>>>+#include <linux/iommu-liveupdate.h>
>>>> #include <linux/iommufd.h>
>>>> #include <linux/idr.h>
>>>> #include <linux/err.h>
>>>>@@ -328,6 +329,47 @@ void iommu_device_unregister(struct 
>>>>iommu_device *iommu)
>>>> }
>>>> EXPORT_SYMBOL_GPL(iommu_device_unregister);
>>>>+struct iommu_dev_iter {
>>>>+    struct iommu_device *iommu;
>>>>+    iommu_dev_iter_fn fn;
>>>>+};
>>>>+
>>>>+static int _iommu_for_each_dev_cb(struct device *dev, void *data)
>>>>+{
>>>>+    struct iommu_dev_iter *iter = data;
>>>>+
>>>>+    if (dev->iommu && dev->iommu->iommu_dev == iter->iommu)
>>>>+        return iter->fn(dev, iter->iommu);
>>>>+
>>>>+    return 0;
>>>>+}
>>>>+
>>>>+/**
>>>>+ * iommu_for_each_dev() - Iterate over all devices attached to an IOMMU
>>>>+ * @iommu: IOMMU instance
>>>>+ * @fn: Callback to call for each device attached to @iommu
>>>>+ *
>>>>+ * Return: 0 on success, or negative error code.
>>>>+ */
>>>>+int iommu_for_each_dev(struct iommu_device *iommu, 
>>>>iommu_dev_iter_fn fn)
>>>>+{
>>>>+    struct iommu_dev_iter iter = {
>>>>+        .iommu = iommu,
>>>>+        .fn = fn,
>>>>+    };
>>>>+    int ret;
>>>>+
>>>>+    for (int i = 0; i < ARRAY_SIZE(iommu_buses); i++) {
>>>>+        ret = bus_for_each_dev(iommu_buses[i], NULL, &iter,
>>>>+                       _iommu_for_each_dev_cb);
>>>>+        if (ret)
>>>>+            return ret;
>>>>+    }
>>>>+
>>>>+    return 0;
>>>>+}
>>>>+EXPORT_SYMBOL_GPL(iommu_for_each_dev);
>>>
>>>The function has no locking against iommu probe or release, so the
>>>callback can read dev->iommu while it is being freed or before it is
>>>fully set up.
>>
>>It is only called from intel_iommu_shutdown(), which runs after the
>>other CPUs are stopped and interrupts are disabled, so probe and release
>>can't run. I will rename it to iommu_for_each_dev_unsafe(), document
>>that it must only be called at shutdown and drop the export, since the
>>only user is built in.
>
>If it's only intended for the intel iommu driver, then it should be
>open-coded there. There is no need to add an immature interface like
>this to the core. It could be misused in the future.

Agreed. I will open code it in the VT-d driver in the next revision.

>
>Thanks,
>baolu

Thanks,
Sami