On Thu, Jul 9, 2020 at 9:56 AM Rob Clark <robdclark@xxxxxxxxx> wrote:
>
> On Thu, Jul 9, 2020 at 9:48 AM Bjorn Andersson
> <bjorn.andersson@xxxxxxxxxx> wrote:
> >
> > On Thu 09 Jul 09:17 PDT 2020, Rob Clark wrote:
> >
> > > On Wed, Jul 8, 2020 at 10:01 PM Bjorn Andersson
> > > <bjorn.andersson@xxxxxxxxxx> wrote:
> > [..]
> > > > @@ -678,7 +680,11 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
> > > > if (smmu_domain->smmu)
> > > > goto out_unlock;
> > > >
> > > > - if (domain->type == IOMMU_DOMAIN_IDENTITY) {
> > > > + /*
> > > > + * Nothing to do for IDENTITY domains,unless disabled context banks are
> > > > + * used to emulate bypass mappings on Qualcomm platforms.
> > > > + */
> > > > + if (domain->type == IOMMU_DOMAIN_IDENTITY && !smmu->qcom_bypass_quirk) {
> > >
> > > maybe I'm overlooking something, but I think this would put us back to
> > > allocating pgtables (and making iommu->map/unmap() no longer no-ops),
> > > which I don't think we want
> > >
> >
> > You're right, we are allocating page tables for these contexts and
> > map/unmap would modify the page tables. But afaict traversal is never
> > performed, given that the banks are never enabled.
> >
> > But as drivers probe properly, or the direct mapped drivers sets up
> > their iommu domains explicitly with translation this would not be used.
> >
> > So afaict we're just wasting some memory - for the gain of not
> > overcomplicating this function.
>
> the problem is that it makes dma_map/unmap less of a no-op than it
> should be (for the case where the driver is explicitly managing it's
> own domain).. I was hoping to get rid of the hacks to use dma_sync go
> back to dma_map/unmap for cache cleaning
That said, it seems to cause less explosions than before.. either that
or I'm not trying hard enough. Still, I think it would probably
result in unnecessary tlb inv's.
Previously, *somehow* we seemed to end up with dma_map/unmap trying to
modify the domain that we had attached.
BR,
-R
> BR,
> -R
>
>
> >
> > Regards,
> > Bjorn
> >
> > > BR,
> > > -R
> > >
> > > > smmu_domain->stage = ARM_SMMU_DOMAIN_BYPASS;
> > > > smmu_domain->smmu = smmu;
> > > > goto out_unlock;
> > > > @@ -826,6 +832,10 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
> > > > domain->geometry.aperture_end = (1UL << ias) - 1;
> > > > domain->geometry.force_aperture = true;
> > > >
> > > > + /* Enable translation for non-identity context banks */
> > > > + if (domain->type != IOMMU_DOMAIN_IDENTITY)
> > > > + cfg->m = true;
> > > > +
> > > > /* Initialise the context bank with our page table cfg */
> > > > arm_smmu_init_context_bank(smmu_domain, &pgtbl_cfg);
> > > > arm_smmu_write_context_bank(smmu, cfg->cbndx);
> > > > diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
> > > > index d172c024be61..a71d193073e4 100644
> > > > --- a/drivers/iommu/arm-smmu.h
> > > > +++ b/drivers/iommu/arm-smmu.h
> > > > @@ -305,6 +305,8 @@ struct arm_smmu_device {
> > > >
> > > > /* IOMMU core code handle */
> > > > struct iommu_device iommu;
> > > > +
> > > > + bool qcom_bypass_quirk;
> > > > };
> > > >
> > > > enum arm_smmu_context_fmt {
> > > > @@ -323,6 +325,7 @@ struct arm_smmu_cfg {
> > > > };
> > > > enum arm_smmu_cbar_type cbar;
> > > > enum arm_smmu_context_fmt fmt;
> > > > + bool m;
> > > > };
> > > > #define ARM_SMMU_INVALID_IRPTNDX 0xff
> > > >
> > > > --
> > > > 2.26.2
> > > >
> > > > _______________________________________________
> > > > iommu mailing list
> > > > iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx
> > > > https://lists.linuxfoundation.org/mailman/listinfo/iommu
