Re: [PATCH v3 12/18] iommu/vt-d: Handle reattach of the restored domain

[Samiullah Khawaja <skhawaja@google.com>]

On Mon, Jun 22, 2026 at 01:44:06PM +0800, Baolu Lu wrote:
>On 6/15/26 07:37, Samiullah Khawaja wrote:
>>Reattach the restored domain to the preserved device using restored
>>domain ID. While reattaching do not setup the context and PASID entries
>>as those are preserved during liveupdate.
>>
>>Signed-off-by: Samiullah Khawaja<skhawaja@google.com>
>>---
>>  drivers/iommu/intel/iommu.c      |  46 ++++++++++---
>>  drivers/iommu/intel/iommu.h      |  17 +++++
>>  drivers/iommu/intel/liveupdate.c | 111 +++++++++++++++++++++++++++++++
>>  3 files changed, 163 insertions(+), 11 deletions(-)
>>
>>diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>>index cd40e274482b..91b67ccba011 100644
>>--- a/drivers/iommu/intel/iommu.c
>>+++ b/drivers/iommu/intel/iommu.c
>>@@ -1311,10 +1311,16 @@ static int dmar_domain_attach_device(struct dmar_domain *domain,
>>  {
>>  	struct device_domain_info *info = dev_iommu_priv_get(dev);
>>  	struct intel_iommu *iommu = info->iommu;
>>+	struct iommu_device_ser *device_ser;
>>  	unsigned long flags;
>>  	int ret;
>>-	ret = domain_attach_iommu(domain, iommu);
>>+	device_ser = dev_iommu_restored_state(dev);
>>+	if (!device_ser)
>>+		ret = domain_attach_iommu(domain, iommu);
>>+	else
>>+		ret = intel_iommu_domain_reattach_iommu(domain,
>>+							iommu, device_ser);
>>  	if (ret)
>>  		return ret;
>>@@ -1327,16 +1333,20 @@ static int dmar_domain_attach_device(struct dmar_domain *domain,
>>  	if (dev_is_real_dma_subdevice(dev))
>>  		return 0;
>>-	if (!sm_supported(iommu))
>>-		ret = domain_context_mapping(domain, dev);
>>-	else if (intel_domain_is_fs_paging(domain))
>>-		ret = domain_setup_first_level(iommu, domain, dev,
>>-					       IOMMU_NO_PASID, NULL);
>>-	else if (intel_domain_is_ss_paging(domain))
>>-		ret = domain_setup_second_level(iommu, domain, dev,
>>-						IOMMU_NO_PASID, NULL);
>>-	else if (WARN_ON(true))
>>-		ret = -EINVAL;
>>+	if (!device_ser) {
>>+		if (!sm_supported(iommu))
>>+			ret = domain_context_mapping(domain, dev);
>>+		else if (intel_domain_is_fs_paging(domain))
>>+			ret = domain_setup_first_level(iommu, domain, dev,
>>+						       IOMMU_NO_PASID, NULL);
>>+		else if (intel_domain_is_ss_paging(domain))
>>+			ret = domain_setup_second_level(iommu, domain, dev,
>>+							IOMMU_NO_PASID, NULL);
>>+		else if (WARN_ON(true))
>>+			ret = -EINVAL;
>>+	} else if (!sm_supported(iommu)) {
>>+		iommu_enable_pci_ats(info);
>>+	}
>
>Instead of merging domain restoration into the attach_dev path, how
>about adding a new callback to restore a preserved domain for a device?

Even with a new callback, the driver still just fetches the restored
state and takes a different path. I am guessing we can just do the
following inside the driver to keep it clean:

static int intel_iommu_attach_device(struct iommu_domain *domain,
                                 	struct device *dev,
                                         struct iommu_domain *old)
{
	struct iommu_device_ser *device_ser = dev_iommu_restored_state(dev);

	if (device_ser)
		return _intel_iommu_restore_dev(domain, dev, device_ser);

	return _intel_iommu_attach_device(domain, dev, old);
}

This keeps the separation you want without touching the generic ops.
WDYT?

With the new callback, this check is just moved into the core inside
__iommu_attach_device(). I am concerned that later down the road when we
add PASID support, we will add restore_dev_pasid().

>Something like:
>
>diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>index b2f614367074..e61409f2d9fc 100644
>--- a/include/linux/iommu.h
>+++ b/include/linux/iommu.h
>@@ -748,6 +748,8 @@ struct iommu_ops {
>  * * <others>  - treated as ENODEV by the caller. Use is discouraged
>  * @set_dev_pasid: set or replace an iommu domain to a pasid of 
>device. The pasid of
>  *                 the device should be left in the old config in 
>error case.
>+ * @restore_dev: Set a domain that is restored from the previous 
>live-updated
>+ *               kernel to a device.
>  * @map_pages: map a physically contiguous set of pages of the same 
>size to
>  *             an iommu domain.
>  * @unmap_pages: unmap a number of pages of the same size from an 
>iommu domain
>@@ -772,6 +774,9 @@ struct iommu_ops {
> struct iommu_domain_ops {
>        int (*attach_dev)(struct iommu_domain *domain, struct device *dev,
>                          struct iommu_domain *old);
>+#ifdef CONFIG_IOMMU_LIVEUPDATE
>+       int (*restore_dev)(struct iommu_domain *domain, struct device *dev);
>+#endif
>        int (*set_dev_pasid)(struct iommu_domain *domain, struct 
>device *dev,
>                             ioasid_t pasid, struct iommu_domain *old);
>
>Thanks,
>baolu

Thanks,
Sami