[PATCH v2 12/19] iommu/arm-smmu-v3: Put writing the context descriptor in the right order

[Michael Shavit]

On Fri, Nov 17, 2023 at 12:47 AM Jason Gunthorpe <jgg at nvidia.com> wrote:
>
> On Wed, Nov 15, 2023 at 11:32:28PM +0800, Michael Shavit wrote:
>
> > > Lift this code out of arm_smmu_detach_dev() so it can all be sequenced
> > > properly. The only other caller is arm_smmu_release_device() and it is
> > > going to free the cdtable anyhow, so it doesn't matter what is in it.
> > >
> > > Signed-off-by: Jason Gunthorpe <jgg at nvidia.com>
> > Reviewed-by: Michael Shavit <mshavit at google.com>
> >
> > This patch might be a better fit before the previous one. When going
> > from S1 to S2 or bypass:
> > Pre-both patches, attach_dev() installs a NULL STE, then clears the
> > now unused CDE, then installs a new STE.
> > After the previous patch, attach_dev() clears the *still used* CDE,
> > and then replaces the STE.
> > After this patch, attach_dev() replaces the STE, and then clears the CDE
> >
> > Reordering the two patches removes the scenario where we could hit a
> > NULL-ed CDE.
>
> NULLed = non-valid
>
> I see what you mean, but I haven't thought carefully about a different
> order so I'd rather leave it..
Ack; it's probably too subtle to matter much anyhow.
>
> Regardless of order the two prior patches will have cases that hit
> non-valid/abort STE/CDEs, each step removes a few cases.
>
> Thanks,
> Jason

On Fri, Nov 17, 2023 at 12:47 AM Jason Gunthorpe <jgg at nvidia.com> wrote:
>
> On Wed, Nov 15, 2023 at 11:32:28PM +0800, Michael Shavit wrote:
>
> > > Lift this code out of arm_smmu_detach_dev() so it can all be sequenced
> > > properly. The only other caller is arm_smmu_release_device() and it is
> > > going to free the cdtable anyhow, so it doesn't matter what is in it.
> > >
> > > Signed-off-by: Jason Gunthorpe <jgg at nvidia.com>
> > Reviewed-by: Michael Shavit <mshavit at google.com>
> >
> > This patch might be a better fit before the previous one. When going
> > from S1 to S2 or bypass:
> > Pre-both patches, attach_dev() installs a NULL STE, then clears the
> > now unused CDE, then installs a new STE.
> > After the previous patch, attach_dev() clears the *still used* CDE,
> > and then replaces the STE.
> > After this patch, attach_dev() replaces the STE, and then clears the CDE
> >
> > Reordering the two patches removes the scenario where we could hit a
> > NULL-ed CDE.
>
> NULLed = non-valid
>
> I see what you mean, but I haven't thought carefully about a different
> order so I'd rather leave it..
>
> Regardless of order the two prior patches will have cases that hit
> non-valid/abort STE/CDEs, each step removes a few cases.
>
> Thanks,
> Jason