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