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

[Baolu Lu]

On 8/28/26 01:52, Samiullah Khawaja wrote:
> > > +
> > > +static int domain_reattach_iommu(struct dmar_domain *domain,
> > > +                 struct intel_iommu *iommu,
> > > +                 struct iommu_device_ser *device_ser)
> > > +{
> > > +    struct iommu_domain_info *info, *curr;
> > > +    int restored_did;
> > > +    int ret;
> > > +
> > > +    if (!iommu_domain_restored_state(&domain->domain))
> > > +        return -EINVAL;
> > > +
> > > +    restored_did = device_ser->domain_iommu_ser.attachment_id;
> > > +    if (!ida_exists(&iommu->domain_ida, restored_did))
> > > +        return -EINVAL;
> >
> > It seems that checking only whether the domain ID is reserved on this
> > IOMMU may not be sufficient. It would be safer to also verify that:
> >
> > - device_ser->domain_iommu_ser.iommu_phys matches iommu->reg_phys, and
> > - device_ser->domain_iommu_ser.domain_phys matches @domain.
> >
> > ?
>
> Interesting.. The caller of this attach from core fetches the correct
> domain based on the checks you mentioned. But you are right, adding a
> check here makes sense to prevent anyone else calling attach with a
> mismatch.

If the iommu core has already verified the domain-device relationship,
then we probably don’t need to duplicate that check in the iommu driver.
Maybe just add a short comment above this function stating that
validation is done by the iommu core?

Thanks,
baolu