Re: [PATCH v2 04/16] iommu: Implement device and IOMMU HW preservation

[David Matlack <dmatlack@google.com>]

On 2026-04-27 05:56 PM, Samiullah Khawaja wrote:
> Add IOMMU ops to preserve/unpreserve a device. These can be implemented

Can you make this comment more specific about what is being preserved?
Saying it preserves a device is vague and maybe even misleading. It's
more about  about preserving a device's attachment to a specific domain
correct?

> 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>

> diff --git a/drivers/iommu/liveupdate.c b/drivers/iommu/liveupdate.c
> index f71f14518248..765d042e22e3 100644
> --- a/drivers/iommu/liveupdate.c
> +++ b/drivers/iommu/liveupdate.c

> +static struct iommu_device_ser *alloc_iommu_device_ser(struct iommu_flb_obj *flb)

It is unforunate that struct iommu_device_ser has nothing to do with
struct iommu_device. The former represents an a PCI device, while the
latter represents an IOMMU.

> diff --git a/include/linux/iommu-liveupdate.h b/include/linux/iommu-liveupdate.h
> index 6019cfc27428..279c7ab04f09 100644
> --- a/include/linux/iommu-liveupdate.h
> +++ b/include/linux/iommu-liveupdate.h

>  int iommu_domain_preserve(struct iommu_domain *domain, struct iommu_domain_ser **ser);
>  void iommu_domain_unpreserve(struct iommu_domain *domain);
> +int iommu_preserve_device(struct iommu_domain *domain,
> +			  struct device *dev, u64 *preserved_state);
> +void iommu_unpreserve_device(struct iommu_domain *domain, struct device *dev);

The naming scheme is inconsistent... Maybe it can be:

  iommu_preserve_domain()
  iommu_unpreserve_domain()
  iommu_preserve_device() or iommu_preserve_device_attachment()
  iommu_unpreserve_device() or iommu_unpreserve_device_attachment()

> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 3853a3946733..1c424b32c5fc 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h

> +#ifdef CONFIG_IOMMU_LIVEUPDATE
> +	int (*preserve_device)(struct device *dev, struct iommu_device_ser *device_ser);
> +	void (*unpreserve_device)(struct device *dev, struct iommu_device_ser *device_ser);
> +	int (*preserve)(struct iommu_device *iommu, struct iommu_hw_ser *iommu_ser);
> +	void (*unpreserve)(struct iommu_device *iommu, struct iommu_hw_ser *iommu_ser);
> +#endif

Maybe we can make these names a little more specific:

  preserve_device_attachment()
  unpreserve_device_attachment()
  preserve_iommu()
  unpreserve_iommu()

?