From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bjorn Andersson Subject: Re: [PATCH v2] firmware: qcom: scm: Peripheral Authentication Service Date: Wed, 15 Jul 2015 18:22:11 -0700 Message-ID: <20150716012210.GK32767@usrtlx11787.corpusers.net> References: <1435693579-16109-1-git-send-email-bjorn.andersson@sonymobile.com> <1436986686-18304-1-git-send-email-bjorn.andersson@sonymobile.com> <20150715234351.GS30412@codeaurora.org> <20150716003535.GH32767@usrtlx11787.corpusers.net> <55A700F9.5070103@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Return-path: Content-Disposition: inline In-Reply-To: <55A700F9.5070103@codeaurora.org> Sender: linux-kernel-owner@vger.kernel.org To: Stephen Boyd Cc: Andy Gross , linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org On Wed 15 Jul 17:55 PDT 2015, Stephen Boyd wrote: > On 07/15/2015 05:35 PM, Bjorn Andersson wrote: > >On Wed 15 Jul 16:43 PDT 2015, Stephen Boyd wrote: > > > >>On 07/15, Bjorn Andersson wrote: [..] > >>Also, dma_alloc_coherent() doesn't do enough to prevent XPU > >>violations because memory returned from that function on ARM is > >>not guaranteed to be device memory and so we could speculatively > >>access the locked down metadata region. This is why we added the > >>strongly ordered mapping property and pass that to > >>dma_alloc_attrs in the downstream code so we can change the page > >>table attributes of the mapping to be device memory. Not doing > >>this can lead to random crashes when some read speculates on the > >>metadata and the secure world intercepts it and shuts the system > >>down. > >> > >The code is taken verbatim from msm-3.4 and the comment is picked from > >the git log, sorry to hear that this is not enough. > > Please move up to msm-3.14 or msm-3.10. Try to find the newest stuff if it's > code like this that isn't specific for a particular SoC. Otherwise we're > going to miss random bug fixes that haven't trickled down to trees for chips > that are two to three years old. > Right, with the introduction of the 64 bit platforms this code was altered to specify the strictly ordered attribute. I have to look at how this should be done in mainline, as I'm moving this out to the common code. > > > >>I was going to say we could try to use the carveout/reserved > >>memory code but that doesn't look fool proof. From what I can > >>tell CMA doesn't use the same page table attributes for the > >>mapping that dma-coherent does, so if we use dma-coherent it will > >>use ioremap and work but if we use CMA it won't (at least it > >>looks like bufferable memory there). Can we add a way to request > >>memory doesn't allow speculatioan through the DMA APIs? > >> > >I haven't looked enough at dma allocations, but this is what worries me > >when using the clients dev pointer (I'm under the impression that these > >choices follow the dev*). > > Yes it does. If the device is cache coherent (e.g. the video processor may > be cache coherent) or even if we want to have two different regions of > memory carved out for the device then using the client's dev pointer won't > work well. > I would like to allocate the peripheral memory in PIL from CMA, if so I guess we have this issue ;) > I think for this sort of allocation it makes sense to make SCM into a > platform driver/device so that we can assign the right attributes to a > memory carveout associated with it. It will also help when we need to max > out crypto clocks and bus bandwidth or other things that are strictly > related to what the firmware needs and not the remote processor. The trouble > is probe defer, so we may need to have some sort of get/put API that returns > EPROBE_DEFER so that client drivers can figure out when they need to wait > for SCM to be ready. > Right, it would definitely clarify the ownership and handling of the crypto clocks. Regards, Bjorn