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

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

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.

Thanks,
baolu