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

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

On 6/23/2026 8:26 AM, Samiullah Khawaja wrote:
> 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().

Okay, that also works for me.

Thanks,
baolu