Re: [PATCH v1 2/6] arm/smmu: Add auxiliary domain support for arm-smmuv2

[Rob Clark]

On Wed, May 20, 2020 at 8:13 AM Jordan Crouse <jcrouse@xxxxxxxxxxxxxx> wrote:
>
> On Wed, May 20, 2020 at 01:57:01PM +0100, Will Deacon wrote:
> > On Mon, May 18, 2020 at 08:50:27AM -0700, Rob Clark wrote:
> > > On Mon, May 18, 2020 at 8:18 AM Will Deacon <will@xxxxxxxxxx> wrote:
> > > > On Wed, Mar 18, 2020 at 04:43:07PM -0700, Rob Clark wrote:
> > > > > We do in fact need live domain switching, that is really the whole
> > > > > point.  The GPU CP (command processor/parser) is directly updating
> > > > > TTBR0 and triggering TLB flush, asynchronously from the CPU.
> > > > >
> > > > > And I think the answer about ASID is easy (on current hw).. it must be zero[*].
> > > >
> > > > Using ASID zero is really bad, because it means that you will end up sharing
> > > > TLB entries with whichever device is using context bank 0.
> > > >
> > > > Is the SMMU only used by the GPU in your SoC?
> > > >
> > >
> > > yes, the snapdragon SoCs have two SMMU instances, one used by the GPU,
> > > where ASID0/cb0 is the gpu itself, and another cb is the GMU
> > > (basically power control for the gpu), and the second SMMU is
> > > everything else.
> >
> > Right, in which case I'm starting to think that we should treat this GPU
> > SMMU instance specially. Give it its own compatible string (looks like you
> > need this for HUPCFG anyway) and hook in via arm_smmu_impl_init(). You can
> > then set IO_PGTABLE_QUIRK_ARM_TTBR1 when talking to the io-pgtable code
> > without having to add a domain attribute.
>
> If we did this via a special GPU SMMU instance then we could also create and
> register a dummy TTBR0 instance along with the TTBR1 instance and then we
> wouldn't need to worry about the aux domains at all.
>
> > With that. you'll need to find a way to allow the GPU driver to call into
> > your own hooks for getting at the TTBR0 tables -- given that you're
> > programming these in the hardware, I don't think it makes sense to expose
> > that in the IOMMU API, since most devices won't be able to do anything with
> > that data. Perhaps you could install a couple of function pointers
> > (subdomain_alloc/subdomain_free) in the GPU device when you see it appear
> > from the SMMU driver? Alternatively, you could make an io_pgtable_cfg
> > available so that the GPU driver can interface with io-pgtable directly.
>
> I don't want to speak for Rob but I think that this is the same direction we've
> landed on. If we use the implementation specific code to initialize the base
> pagetables then the GPU driver can use io-pgtable directly. We can easily
> construct an io_pgtable_cfg. This feature will only be available for opt-in
> GPU targets that will have a known configuration.

Agreed about using io-pgtable helpers directly.. the gpu's use-case is
pretty far different from anything normal/sane, and I don't think it
is worth designing some generic iommu interfaces with precisely one
user[*].  We just need enough in arm-smmu(/-impl) to bootstrap things
when we power up the gpu.

BR,
-R

[*] all the other gpu's that I've seen so far, even if they sit behind
an iommu, they have their own internal mmu

> The only gotcha is TLB maintenance but Rob and I have ideas about coordinating
> with the GPU hardware (which has to do a TLBIALL during a switch anyway) and we
> can always use the iommu_tlb_flush_all() hammer from software if we really need
> it. It might take a bit of thought, but it is doable.
>
> > Yes, it's ugly, but I don't think it's worth trying to abstract this.
>
> I'm not sure how ugly it is. I've always operated under the assumption that the
> GPU SMMU was special (though it had generic registers) just because of where it
> was and how it it was used.  In the long run baking in a implementation specific
> solution would probably be preferable to lots of domain attributes and aux
> domains that would never be used except by us.
>
> > Thoughts? It's taken me a long time to figure out what's going on here,
> > so sorry if it feels like I'm leading you round the houses.
>
> I'll hack on this and try to get something in place. It might be dumber on the
> GPU side than we would like but it would at least spur some more conversation.
>
> Jordan
>
> > Will
>
> --
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project