Re: [PATCH v2 1/1] iommu/vt-d: Fix RID2PASID setup failure

[Baolu Lu]

On 2022/6/22 17:09, Ethan Zhao wrote:
> 在 2022/6/22 12:41, Lu Baolu 写道:
> > The IOMMU driver shares the pasid table for PCI alias devices. When the
> > RID2PASID entry of the shared pasid table has been filled by the first
> > device, the subsequent devices will encounter the "DMAR: Setup RID2PASID
> > failed" failure as the pasid entry has already been marked as present. As
> > the result, the IOMMU probing process will be aborted.
> >
> > This fixes it by skipping RID2PASID setting if the pasid entry has been
> > populated. This works because the IOMMU core ensures that only the same
> > IOMMU domain can be attached to all PCI alias devices at the same time.
> > Therefore the subsequent devices just try to setup the RID2PASID entry
> > with the same domain, which is negligible. This also adds domain validity
> > checks for more confidence anyway.
> >
> > Fixes: ef848b7e5a6a0 ("iommu/vt-d: Setup pasid entry for RID2PASID 
> > support")
> > Reported-by: Chenyi Qiang <chenyi.qiang@xxxxxxxxx>
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> > ---
> >   drivers/iommu/intel/pasid.c | 22 ++++++++++++++++------
> >   1 file changed, 16 insertions(+), 6 deletions(-)
> >
> > Change log:
> > v2:
> >   - Add domain validity check in RID2PASID entry setup.
> >
> > diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> > index cb4c1d0cf25c..4f3525f3346f 100644
> > --- a/drivers/iommu/intel/pasid.c
> > +++ b/drivers/iommu/intel/pasid.c
> > @@ -575,6 +575,19 @@ static inline int pasid_enable_wpe(struct 
> > pasid_entry *pte)
> >       return 0;
> >   };
> > +/*
> > + * Return true if @pasid is RID2PASID and the domain @did has already
> > + * been setup to the @pte. Otherwise, return false. PCI alias devices
> > + * probably share the single RID2PASID pasid entry in the shared pasid
> > + * table. It's reasonable that those devices try to set a share domain
> > + * in their probe paths.
> > + */
>
> I am thinking about the counter-part, the intel_pasid_tear_down_entry(),
>
> Multi devices share the same PASID entry, then one was detached from the 
> domain,
>
> so the entry doesn't exist anymore, while another devices don't know 
> about the change,
>
> and they are using the mapping, is it possible case ？shared thing, no 
> refer-counter,
>
> am I missing something ?

No. You are right. When any alias device is hot-removed from the system,
the shared RID2PASID will be cleared without any notification to other
devices. Hence any DMAs from those devices are blocked.

We still have a lot to do for sharing pasid table among alias devices.
Before we arrive there, let's remove it for now.

Best regards,
baolu