Re: [PATCH v5 06/18] iommu: Implement device and IOMMU HW preservation

[Samiullah Khawaja <skhawaja@google.com>]

On Tue, Oct 06, 2026 at 08:21:15PM -0700, Nicolin Chen wrote:
>On Mon, Sep 21, 2026 at 12:48:22AM +0000, Samiullah Khawaja wrote:
>> @@ -827,6 +838,8 @@ struct iommu_domain_ops {
>>   * @singleton_group: Used internally for drivers that have only one group
>>   * @max_pasids: number of supported PASIDs
>>   * @ready: set once iommu_device_register() has completed successfully
>> + * @outgoing_preserved_state: preserved iommu state of outgoing kernel for
>> + * liveupdate.
>>   */
>>  struct iommu_device {
>>  	struct list_head list;
>> @@ -836,6 +849,10 @@ struct iommu_device {
>>  	struct iommu_group *singleton_group;
>>  	u32 max_pasids;
>>  	bool ready;
>> +
>> +#ifdef CONFIG_IOMMU_LIVEUPDATE
>> +	struct iommu_hw_ser *outgoing_preserved_state;
>> +#endif
>
>It'd be nicer to align the naming.
>
>There are:
>	struct iommu_hw_ser *outgoing_preserved_state;
>	..
>	struct iommu_device_ser *device_ser;
>	..
>	struct iommu_domain_ser *preserved_state
>
>Maybe use "_ser" for all?

That is a fair point. I will update this.
>
>>
>>  /**
>> @@ -890,6 +907,9 @@ struct dev_iommu {
>>  	u32				pci_32bit_workaround:1;
>>  	u32				require_direct:1;
>>  	u32				shadow_on_flush:1;
>> +#ifdef CONFIG_IOMMU_LIVEUPDATE
>> +	struct iommu_device_ser		*device_ser;
>> +#endif
>
>Missing its kdoc update.

Will update.
>
>
>> @@ -1206,6 +1226,28 @@ static inline void *dev_iommu_priv_get(struct device *dev)
>>
>>  void dev_iommu_priv_set(struct device *dev, void *priv);
>>
>> +/**
>> + * typedef iommu_dev_iter_fn - Callback for iterating IOMMU attached devices
>> + * @dev: Attached device
>> + * @iommu: IOMMU instance
>> + * @arg: Private argument passed to iterator
>> + *
>> + * Return: 0 on success, or negative error code.
>> + */
>> +typedef int (*iommu_dev_iter_fn)(struct device *dev,
>> +				 struct iommu_device *iommu, void *arg);
>> +
>> +/**
>> + * struct iommu_dev_iter - Iterator for devices attached to an IOMMU
>> + */
>> +struct iommu_dev_iter {
>> +	struct iommu_device *iommu;
>> +	iommu_dev_iter_fn fn;
>> +	void *arg;
>> +};
>> +
>> +int iommu_for_each_dev(struct iommu_dev_iter *iter);
>
>It isn't very necessarily to expose iommu_dev_iter.
>
>iommu_for_each_dev could be simpler:
>
>int iommu_for_each_dev(struct iommu_device *iommu, iommu_dev_iter_fn fn,
>		       void *arg);
>
>Also, arg seems unused?

Agreed. I will update it to this:

int iommu_for_each_dev(struct iommu_device *iommu, iommu_dev_iter_fn fn);
>
>Nicolin

Thanks,
Sami