Re: [PATCH 00/19] iommu/arm-smmu + drm/msm: per-process GPU pgtables

[Jordan Crouse <jcrouse@codeaurora.org>]

On Thu, Aug 13, 2020 at 07:40:55PM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> NOTE: Since Jordan was out today, and I wanted to keep things moving on
>       this, I took the liberty of respinning his series (originally
>       titled "iommu/arm-smmu: Add Adreno SMMU specific implementation")
>       with updates based on Will's review comments, and some fixes and
>       extra bits that I found in stress testing the series.  Original
>       commit msg and updated version history below.
> 
>       In general I like the private interface between adreno-smmu and
>       the GPU driver.  It should make for a more straightforward way
>       to extend things to optimize TLB invalidation in the future, for
>       example, rather than shoe-horning everything thru domain attrs.
>       And it lets us describe the get_ttbr1_cfg/set_ttrb0_cfg dance
>       more clearly.  Although it is going to make landing this via
>       iommu vs drm tree a bit more difficult.  Maybe there are some
>       arm-smmu parts of this series that could be pulled out to make
>       it not conflicty to land the private interface and adreno-smmu
>       bits via the drm tree?  (But I'm jumping a bit ahead here.  Just
>       wanted to point out that issue.)
> 
>       The complete series can be found at:
>       https://gitlab.freedesktop.org/drm/msm/-/commits/msm-next-pgtables
> 
> This series adds an Adreno SMMU implementation to arm-smmu to allow GPU hardware
> pagetable switching.
> 
> The Adreno GPU has built in capabilities to switch the TTBR0 pagetable during
> runtime to allow each individual instance or application to have its own
> pagetable.  In order to take advantage of the HW capabilities there are certain
> requirements needed of the SMMU hardware.
> 
> This series adds support for an Adreno specific arm-smmu implementation. The new
> implementation 1) ensures that the GPU domain is always assigned context bank 0,
> 2) enables split pagetable support (TTBR1) so that the instance specific
> pagetable can be swapped while the global memory remains in place and 3) shares
> the current pagetable configuration with the GPU driver to allow it to create
> its own io-pgtable instances.
> 
> The series then adds the drm/msm code to enable these features. For targets that
> support it allocate new pagetables using the io-pgtable configuration shared by
> the arm-smmu driver and swap them in during runtime.
> 
> This version of the series merges the previous patchset(s) [1] and [2]
> with the following improvements:
> 
> v13: (Respin by Rob)
>   - Switch to a private interface between adreno-smmu and GPU driver,
>     dropping the custom domain attr (Will Deacon)
>   - Rework the SCTLR.HUPCF patch to add new fields in smmu_domain->cfg
>     rather than adding new impl hook (Will Deacon)
>   - Drop for_each_cfg_sme() in favor of plain for() loop (Will Deacon)
>   - Fix context refcnt'ing issue which was causing problems with GPU
>     crash recover stress testing.
>   - Spiff up $debugfs/gem to show process information associated with
>     VMAs

I'll add the tags to Rob's code but in general I ack all these changes. I also 
like the private interface - it gives us the most flexibility without either
changing the IOMMU API or giving up entirely and making an internal SMMU
implementation inside drm/msm.

<snip>

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project