Re: [PATCH v2] firmware: qcom: scm: Peripheral Authentication Service

[Bjorn Andersson <bjorn.andersson@sonymobile.com>]

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