On Mon, 4 May 2020 18:43:51 +0200
Jean-Philippe Brucker <jean-philippe@xxxxxxxxxx> wrote:
> On Thu, Apr 30, 2020 at 02:16:17PM -0700, Jacob Pan wrote:
> > > +static void arm_smmu_mm_invalidate_range(struct mmu_notifier *mn,
> > > + struct mm_struct *mm,
> > > + unsigned long start,
> > > unsigned long end) +{
> > > + /* TODO: invalidate ATS */
> > > +}
> > > +
> > > +static void arm_smmu_mm_release(struct mmu_notifier *mn, struct
> > > mm_struct *mm) +{
> > > + struct arm_smmu_mmu_notifier *smmu_mn = mn_to_smmu(mn);
> > > + struct arm_smmu_domain *smmu_domain;
> > > +
> > > + mutex_lock(&arm_smmu_sva_lock);
> > > + if (smmu_mn->cleared) {
> > > + mutex_unlock(&arm_smmu_sva_lock);
> > > + return;
> > > + }
> > > +
> > > + smmu_domain = smmu_mn->domain;
> > > +
> > > + /*
> > > + * DMA may still be running. Keep the cd valid but
> > > disable
> > > + * translation, so that new events will still result in
> > > stall.
> > > + */
> > Does "disable translation" also disable translated requests?
>
> No it doesn't disable translated requests, it only prevents the SMMU
> from accessing the pgd.
>
OK. same as VT-d.
> > I guess
> > release is called after tlb invalidate range, so assuming no more
> > devTLB left to generate translated request?
>
> I'm counting on the invalidate below (here a TODO, implemented in next
> patch) to drop all devTLB entries. After that invalidate, the device:
> * issues a Translation Request, returns with R=W=0 because we disabled
> translation (and it isn't present in the SMMU TLB).
> * issues a Page Request, returns with InvalidRequest because
> mmget_not_zero() fails.
>
Same flow. Thanks for the explanation.
> >
> > > + arm_smmu_write_ctx_desc(smmu_domain, mm->pasid,
> > > &invalid_cd); +
> > > + arm_smmu_tlb_inv_asid(smmu_domain->smmu,
> > > smmu_mn->cd->asid);
> > > + /* TODO: invalidate ATS */
> > > +
> > If mm release is called after tlb invalidate range, is it still
> > necessary to invalidate again?
>
> No, provided all mappings from the address space are unmapped and
> invalidated. I'll double check, but in my tests invalidate range
> didn't seem to be called for all mappings on mm exit, so I believe we
> do need this.
>
I think it is safe to invalidate again. There was a concern that mm
release may delete IOMMU driver from the notification list and miss tlb
invalidate range. I had a hard time to confirm that with ftrace while
killing a process, many lost events.
> Thanks,
> Jean
>
[Jacob Pan]
