linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Vivek Gautam <vivek.gautam@codeaurora.org>,
	Will Deacon <will.deacon@arm.com>, Joerg Roedel <joro@8bytes.org>,
	"list@263.net:IOMMU DRIVERS" <iommu@lists.linux-foundation.org>,
	pdaly@codeaurora.org,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Tomasz Figa <tfiga@chromium.org>,
	Jordan Crouse <jcrouse@codeaurora.org>,
	pratikp@codeaurora.org,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 0/3] iommu/arm-smmu: Add support to use Last level cache
Date: Mon, 21 Jan 2019 15:15:15 +0000	[thread overview]
Message-ID: <55213eaf-d90d-0f2f-9f01-5e964e087d3f@arm.com> (raw)
In-Reply-To: <CAKv+Gu_Oz-QEFnq9KiOBHQrC8o+0ykkEZBm0vCWfYDfFB8QTcQ@mail.gmail.com>

On 21/01/2019 14:24, Ard Biesheuvel wrote:
> On Mon, 21 Jan 2019 at 14:56, Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 21/01/2019 13:36, Ard Biesheuvel wrote:
>>> On Mon, 21 Jan 2019 at 14:25, Robin Murphy <robin.murphy@arm.com> wrote:
>>>>
>>>> On 21/01/2019 10:50, Ard Biesheuvel wrote:
>>>>> On Mon, 21 Jan 2019 at 11:17, Vivek Gautam <vivek.gautam@codeaurora.org> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>>
>>>>>> On Mon, Jan 21, 2019 at 12:56 PM Ard Biesheuvel
>>>>>> <ard.biesheuvel@linaro.org> wrote:
>>>>>>>
>>>>>>> On Mon, 21 Jan 2019 at 06:54, Vivek Gautam <vivek.gautam@codeaurora.org> 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?

I only have a relatively vague impression of how this Qcom interconnect 
actually behaves, but AIUI the outer attribute has no correctness impact 
(it's effectively mismatched between CPU and devices already), only some 
degree of latency improvement which is effectively the opposite of 
no-snoop, in allowing certain non-coherent device traffic to still 
allocate in the LLC. I'm assuming that if that latency matters for the 
device accesses themselves, it might also matter for the associated 
table walks depending on the TLB miss rate.

Robin.

  reply	other threads:[~2019-01-21 15:15 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-21  5:53 [PATCH 0/3] iommu/arm-smmu: Add support to use Last level cache Vivek Gautam
     [not found] ` <20190121055335.15430-1-vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2019-01-21  5:53   ` [PATCH 1/3] iommu/arm-smmu: Move to bitmap for arm_smmu_domain atrributes Vivek Gautam
2019-01-21 13:51     ` Robin Murphy
2019-01-22 17:06       ` Vivek Gautam
2019-01-21  5:53   ` [PATCH 2/3] iommu/io-pgtable-arm: Add support to use system cache Vivek Gautam
2019-01-21  5:53   ` [PATCH 3/3] iommu/arm-smmu: " Vivek Gautam
2019-01-21  7:26 ` [PATCH 0/3] iommu/arm-smmu: Add support to use Last level cache Ard Biesheuvel
2019-01-21 10:17   ` Vivek Gautam
2019-01-21 10:50     ` Ard Biesheuvel
2019-01-21 13:25       ` Robin Murphy
2019-01-21 13:36         ` Ard Biesheuvel
2019-01-21 13:56           ` Robin Murphy
2019-01-21 14:24             ` Ard Biesheuvel
2019-01-21 15:15               ` Robin Murphy [this message]
2019-01-24  6:58               ` Vivek Gautam
2019-01-24  7:54                 ` Ard Biesheuvel
2019-01-28 11:27                   ` Vivek Gautam
2019-01-29 15:02                     ` Ard Biesheuvel
2019-01-30  5:39                       ` Vivek Gautam

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=55213eaf-d90d-0f2f-9f01-5e964e087d3f@arm.com \
    --to=robin.murphy@arm.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jcrouse@codeaurora.org \
    --cc=joro@8bytes.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pdaly@codeaurora.org \
    --cc=pratikp@codeaurora.org \
    --cc=tfiga@chromium.org \
    --cc=vivek.gautam@codeaurora.org \
    --cc=will.deacon@arm.com \
    /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).