From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Rob Clark <robdclark@gmail.com>
Cc: Jonathan Marek <jonathan@marek.ca>, Will Deacon <will@kernel.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>
Subject: Re: [PATCH 2/5] iommu/arm-smmu: Emulate bypass by using context banks
Date: Thu, 9 Jul 2020 12:40:12 -0700 [thread overview]
Message-ID: <20200709194012.GX388985@builder.lan> (raw)
In-Reply-To: <CAF6AEGvHjiS8r32FkVs5yK+nk+caEr4dikCnrE205nqToTA4tg@mail.gmail.com>
On Thu 09 Jul 11:55 PDT 2020, Rob Clark wrote:
> On Thu, Jul 9, 2020 at 9:56 AM Rob Clark <robdclark@gmail.com> wrote:
> >
> > On Thu, Jul 9, 2020 at 9:48 AM Bjorn Andersson
> > <bjorn.andersson@linaro.org> 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@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.
> >
> > 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.
>
I might need some help to really understand the plumbing here, but this
is what I read from the code and can see in my debugging...
The display device will upon creation be allocated a default_domain, of
type IOMMU_DOMAIN_IDENTITY - per the qcom-impl. Then you then allocate a
new iommu domain on the platform bus in the display driver and attach
this to the device. This will result in the group's domain being of type
IOMMU_DOMAIN_UNMANAGED.
But when you then invoke dma_map_single() on the display device, the map
operation will acquire the iommu_group's default_domain (not domain) and
as such attempt to map stuff on the IDENTITY domain.
__iommu_map() will however reject this, given that the type does not
have __IOMMU_DOMAIN_PAGING set. Afaict, this would happen regardless of
this patch actually setting up a page table for the default_domain or
not.
So, afaict, you can't use dma_map_single()/dma_unmap() to operate your
page table on a lately attached iommu domain.
This would also imply that the display device consumes two context
banks, which worries me more than the waste of page tables etc.
Regards,
Bjorn
> 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@lists.linux-foundation.org
> > > > > https://lists.linuxfoundation.org/mailman/listinfo/iommu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2020-07-09 19:43 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-09 5:01 [PATCH 0/5] iommu/arm-smmu: Support maintaining bootloader mappings Bjorn Andersson
2020-07-09 5:01 ` [PATCH 1/5] iommu/arm-smmu: Make all valid stream mappings BYPASS Bjorn Andersson
2020-07-13 21:25 ` kernel test robot
2020-07-13 21:25 ` [RFC PATCH] iommu/arm-smmu: arm_smmu_setup_identity() can be static kernel test robot
2020-07-15 23:20 ` [PATCH 1/5] iommu/arm-smmu: Make all valid stream mappings BYPASS kernel test robot
2020-07-09 5:01 ` [PATCH 2/5] iommu/arm-smmu: Emulate bypass by using context banks Bjorn Andersson
2020-07-09 16:17 ` Rob Clark
2020-07-09 16:48 ` Bjorn Andersson
2020-07-09 16:56 ` Rob Clark
2020-07-09 18:55 ` Rob Clark
2020-07-09 19:40 ` Bjorn Andersson [this message]
2020-07-09 5:01 ` [PATCH 3/5] iommu/arm-smmu: Move SMR and S2CR definitions to header file Bjorn Andersson
2020-07-09 5:01 ` [PATCH 4/5] iommu/arm-smmu-qcom: Consstently initialize stream mappings Bjorn Andersson
2020-07-09 7:32 ` Vinod Koul
2020-07-09 5:01 ` [PATCH 5/5] iommu/arm-smmu: Setup identity domain for boot mappings Bjorn Andersson
2020-07-09 15:50 ` Laurentiu Tudor
2020-07-09 19:57 ` Bjorn Andersson
2020-07-28 15:27 ` Laurentiu Tudor
2020-07-09 7:33 ` [PATCH 0/5] iommu/arm-smmu: Support maintaining bootloader mappings Vinod Koul
2020-07-10 5:25 ` John Stultz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200709194012.GX388985@builder.lan \
--to=bjorn.andersson@linaro.org \
--cc=iommu@lists.linux-foundation.org \
--cc=jonathan@marek.ca \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robdclark@gmail.com \
--cc=robin.murphy@arm.com \
--cc=thierry.reding@gmail.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).