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

[Baolu Lu]

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