Re: [PATCH 2/5] iommu/arm-smmu: Emulate bypass by using context banks

From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Rob Clark <robdclark@gmail.com>
Cc: Jonathan Marek <jonathan@marek.ca>, Will Deacon <will@kernel.org>,
	Joerg Roedel <joro@8bytes.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>,
	Joerg Roedel <joro@8bytes.org>,
	" <iommu@lists.linux-foundation.org>,
	Thierry Reding <thierry.reding@gmail.com>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	Robin Murphy <robin.murphy@arm.com>,
	"moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE"
	<linux-arm-kernel@lists.infradead.org>,
	Laurentiu Tudor <laurentiu.tudor@nxp.com>
Subject: Re: [PATCH 2/5] iommu/arm-smmu: Emulate bypass by using context banks
Date: Thu, 9 Jul 2020 09:48:37 -0700	[thread overview]
Message-ID: <20200709164833.GR11847@yoga> (raw)
In-Reply-To: <CAF6AEGugJYmMpeH_9mWXcegq1awzraUzBTa0S1Rqcr3=Bmk0uQ@mail.gmail.com>

On Thu 09 Jul 09:17 PDT 2020, Rob Clark wrote:

> On Wed, Jul 8, 2020 at 10:01 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> 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.

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@lists.linux-foundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/iommu

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel