[PATCH 09/19] iommu/arm-smmu-v3: Hold arm_smmu_asid_lock during all of attach_dev

[Michael Shavit]

On Tue, Oct 24, 2023 at 10:44 AM Michael Shavit <mshavit at google.com> wrote:
>
> On Wed, Oct 11, 2023 at 8:33 AM Jason Gunthorpe <jgg at nvidia.com> wrote:
> >
> > The BTM support wants to be able to change the ASID of any smmu_domain.
> > When it goes to do this it holds the arm_smmu_asid_lock and iterates over
> > the target domain's devices list.
> >
> > During attach of a S1 domain we must ensure that the devices list and
> > CD are in sync, otherwise we could miss CD updates or a parallel CD update
> > could push an out of date CD.
> >
> > This is pretty complicated, and works today because arm_smmu_detach_dev()
> > remove the CD table from the STE before working on the CD entries.
> >
> > The next patch will allow the CD table to remain in the STE so solve this
> > racy by holding the lock for a longer period. The lock covers both of the
> > changes to the device list and the CD table entries.
> >
> > Move arm_smmu_detach_dev() till after we have initialized the domain so
> > the lock can be held for less time.
> >
> > Signed-off-by: Jason Gunthorpe <jgg at nvidia.com>
> > ---
> >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 24 ++++++++++++---------
> >  1 file changed, 14 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > index 2c06d3e3abe2b1..a29421f133a3c0 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -2535,8 +2535,6 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
> >                 return -EBUSY;
> >         }
> >
> > -       arm_smmu_detach_dev(master);
> > -
> >         mutex_lock(&smmu_domain->init_mutex);
> >
> >         if (!smmu_domain->smmu) {
> > @@ -2549,7 +2547,17 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
> >
> >         mutex_unlock(&smmu_domain->init_mutex);
> >         if (ret)
> > -               return ret;
> > +               goto out_unlock;
>
> Oh, missed this earlier but on a second look the asid_lock isn't
> grabbed here yet so this should stay as return ret.
>
Guess you must have noticed it too since it's fixed in patch 09 of the
second series :) .