On Tue, Jan 29, 2019 at 8:34 PM Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote: > > (+ Bjorn) > > On Mon, 28 Jan 2019 at 12:27, Vivek Gautam <vivek.gautam@xxxxxxxxxxxxxx> wrote: > > > > Hi Ard, > > > > On Thu, Jan 24, 2019 at 1:25 PM Ard Biesheuvel > > <ard.biesheuvel@xxxxxxxxxx> wrote: > > > > > > On Thu, 24 Jan 2019 at 07:58, Vivek Gautam <vivek.gautam@xxxxxxxxxxxxxx> wrote: > > > > > > > > On Mon, Jan 21, 2019 at 7:55 PM Ard Biesheuvel > > > > <ard.biesheuvel@xxxxxxxxxx> wrote: > > > > > > > > > > On Mon, 21 Jan 2019 at 14:56, Robin Murphy <robin.murphy@xxxxxxx> wrote: > > > > > > > > > > > > On 21/01/2019 13:36, Ard Biesheuvel wrote: > > > > > > > On Mon, 21 Jan 2019 at 14:25, Robin Murphy <robin.murphy@xxxxxxx> wrote: > > > > > > >> > > > > > > >> On 21/01/2019 10:50, Ard Biesheuvel wrote: > > > > > > >>> On Mon, 21 Jan 2019 at 11:17, Vivek Gautam <vivek.gautam@xxxxxxxxxxxxxx> wrote: > > > > > > >>>> > > > > > > >>>> Hi, > > > > > > >>>> > > > > > > >>>> > > > > > > >>>> On Mon, Jan 21, 2019 at 12:56 PM Ard Biesheuvel > > > > > > >>>> <ard.biesheuvel@xxxxxxxxxx> wrote: > > > > > > >>>>> > > > > > > >>>>> On Mon, 21 Jan 2019 at 06:54, Vivek Gautam <vivek.gautam@xxxxxxxxxxxxxx> wrote: > > > > > > >>>>>> > > > > > > >>>>>> Qualcomm SoCs have an additional level of cache called as > > > > > > >>>>>> System cache, aka. Last level cache (LLC). This cache sits right > > > > > > >>>>>> before the DDR, and is tightly coupled with the memory controller. > > > > > > >>>>>> The clients using this cache request their slices from this > > > > > > >>>>>> system cache, make it active, and can then start using it. > > > > > > >>>>>> For these clients with smmu, to start using the system cache for > > > > > > >>>>>> buffers and, related page tables [1], memory attributes need to be > > > > > > >>>>>> set accordingly. This series add the required support. > > > > > > >>>>>> > > > > > > >>>>> > > > > > > >>>>> Does this actually improve performance on reads from a device? The > > > > > > >>>>> non-cache coherent DMA routines perform an unconditional D-cache > > > > > > >>>>> invalidate by VA to the PoC before reading from the buffers filled by > > > > > > >>>>> the device, and I would expect the PoC to be defined as lying beyond > > > > > > >>>>> the LLC to still guarantee the architected behavior. > > > > > > >>>> > > > > > > >>>> We have seen performance improvements when running Manhattan > > > > > > >>>> GFXBench benchmarks. > > > > > > >>>> > > > > > > >>> > > > > > > >>> Ah ok, that makes sense, since in that case, the data flow is mostly > > > > > > >>> to the device, not from the device. > > > > > > >>> > > > > > > >>>> As for the PoC, from my knowledge on sdm845 the system cache, aka > > > > > > >>>> Last level cache (LLC) lies beyond the point of coherency. > > > > > > >>>> Non-cache coherent buffers will not be cached to system cache also, and > > > > > > >>>> no additional software cache maintenance ops are required for system cache. > > > > > > >>>> Pratik can add more if I am missing something. > > > > > > >>>> > > > > > > >>>> To take care of the memory attributes from DMA APIs side, we can add a > > > > > > >>>> DMA_ATTR definition to take care of any dma non-coherent APIs calls. > > > > > > >>>> > > > > > > >>> > > > > > > >>> So does the device use the correct inner non-cacheable, outer > > > > > > >>> writeback cacheable attributes if the SMMU is in pass-through? > > > > > > >>> > > > > > > >>> We have been looking into another use case where the fact that the > > > > > > >>> SMMU overrides memory attributes is causing issues (WC mappings used > > > > > > >>> by the radeon and amdgpu driver). So if the SMMU would honour the > > > > > > >>> existing attributes, would you still need the SMMU changes? > > > > > > >> > > > > > > >> Even if we could force a stage 2 mapping with the weakest pagetable > > > > > > >> attributes (such that combining would work), there would still need to > > > > > > >> be a way to set the TCR attributes appropriately if this behaviour is > > > > > > >> wanted for the SMMU's own table walks as well. > > > > > > >> > > > > > > > > > > > > > > Isn't that just a matter of implementing support for SMMUs that lack > > > > > > > the 'dma-coherent' attribute? > > > > > > > > > > > > Not quite - in general they need INC-ONC attributes in case there > > > > > > actually is something in the architectural outer-cacheable domain. > > > > > > > > > > But is it a problem to use INC-ONC attributes for the SMMU PTW on this > > > > > chip? AIUI, the reason for the SMMU changes is to avoid the > > > > > performance hit of snooping, which is more expensive than cache > > > > > maintenance of SMMU page tables. So are you saying the by-VA cache > > > > > maintenance is not relayed to this system cache, resulting in page > > > > > table updates to be invisible to masters using INC-ONC attributes? > > > > > > > > The reason for this SMMU changes is that the non-coherent devices > > > > can't access the inner caches at all. But they have a way to allocate > > > > and lookup in system cache. > > > > > > > > CPU will by default make use of system cache when the inner-cacheable > > > > and outer-cacheable memory attribute is set. > > > > > > > > So for SMMU page tables to be visible to PTW, > > > > -- For IO coherent clients, the CPU cache maintenance operations are not > > > > required for buffers marked Normal Cached to achieve a coherent view of > > > > memory. However, client-specific cache maintenance may still be > > > > required for devices > > > > with local caches (for example, compute DSP local L1 or L2). > > > > > > Why would devices need to access the SMMU page tables? > > > > No, the devices don't need to access the page tables, rather the PTW does. > > Sorry for mixing it up. > > > > > > > > > -- For non-IO coherent clients, the CPU cache maintenance operations (cleans > > > > and/or invalidates) are required at buffer handoff points for buffers marked as > > > > Normal Cached in any CPU page table in order to observe the latest updates. > > > > > > > > > > Indeed, and this is what your non-coherent SMMU PTW requires, and what > > > you /should/ get when you omit the 'dma-coherent' property from its DT > > > node (and if you don't, it is a bug in the SMMU driver that should get > > > fixed) > > > > > > The question is whether using inner-non-cached/outer-cacheable > > > attributes for the PTW is required for correctness, or whether it is > > > merely an optimization (since the point of this exercise was to avoid > > > snoop latency from the SMMU PTW). If it is an optimization, I would > > > like to understand whether the performance delta between SMMU page > > > tables in DRAM vs SMMU page tables in the LLC justifies these > > > intrusive changes to the SMMU driver. > > > > IIUC, SMMU uses the TCR configurations to decide how PTW should access > > the memory. TCR doesn't direct CPU whether to use cacheable or non -cacheable > > memory to allocate page tables. Is that right? > > Correct > > > Currently, these TCR configurations are set for inner-cacheable, and > > outer-cacheable. > > With this, is it assumed that PTW would snoop into the CPU caches for > > any updates > > of the page tables? > > > ` > Yes, and if I understand the issue correctly, this snooping is costly, > which is why you want to avoid it, right? > > > When we omit 'dma-coherent', CPU will allocate non-coherent memory > > for these page tables, and software has to explicitly flush CPU caches to > > make the changes visible to SMMU. > > Indeed. But I would expect the TCR configuration to reflect this as > well, and that doesn't appear the case. Yea, we are discussing this case of TCR configurations for non-coherent memory in another thread [1]. [1] https://lore.kernel.org/patchwork/patch/1032939/ > > > The CPU will still mark this memory as Normal Cached, i.e. inner cached, > > outer cached, and the non-IO coherent SMMU PTW won't be able to snoop into > > CPU caches. Does the following code in io-pgtable-arm.c ensures that SMMU > > sees the latest page tables? > > > > } else if (!(cfg->quirks & IO_PGTABLE_QUIRK_NO_DMA) && > > !(pte & ARM_LPAE_PTE_SW_SYNC)) { > > __arm_lpae_sync_pte(ptep, cfg); > > } > > > > I don't know the history of why NO_DMA is implemented as a quirk (and > why it is called like that in the first place). > But it indeed appears that this is where the cache maintenance occurs > for non-coherent PTWs. > > > This change is mostly to get optimized PTW. As seen in the patch [1] for GPU, > > there's a separate slice for page tables - "gpuhtw_llc_slice". > > Let me try to get the numbers for this optimization. > > > > Yes, please. We'd need to compare page tables in the LLC with page > tables in system RAM, and for completeness, it would be nice to > include the cache-coherent configuration as well. Sure, let me ping Pratik and Patrick to check if they have already have these numbers available, else I can check it. Regards Vivek -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
