Do Qualcomm drivers use DMA buffers for request_firmware_into_buf()?

[Ard Biesheuvel]

On 7 June 2018 at 20:21, Bjorn Andersson <bjorn.andersson at linaro.org> wrote:
> On Thu 07 Jun 09:33 PDT 2018, Greg Kroah-Hartman wrote:
>
>> On Thu, Jun 07, 2018 at 06:23:01PM +0200, Ard Biesheuvel wrote:
>> > On 7 June 2018 at 18:18, Bjorn Andersson <bjorn.andersson at linaro.org> wrote:
>> > > On Wed 06 Jun 13:32 PDT 2018, Luis R. Rodriguez wrote:
>> > >
>> > >> On Fri, Jun 01, 2018 at 09:23:46PM +0200, Luis R. Rodriguez wrote:
>> > >> > On Tue, May 08, 2018 at 03:38:05PM +0000, Luis R. Rodriguez wrote:
>> > >> > > On Fri, May 04, 2018 at 12:44:37PM -0700, Martijn Coenen wrote:
>> > >> > > >
>> > >> > > > I think the Qualcomm folks owning this (Andy, David, Bjorn, already
>> > >> > > > cc'd here) are better suited to answer that question.
>> > >> > >
>> > >> > > Andy, David, Bjorn?
>> > >> >
>> > >> > Andy, David, Bjorn?
>> > >>
>> > >> A month now with no answer...
>> > >>
>> > >
>> > > The patch at the top of this thread doesn't interest me and you didn't
>> > > bother sending your question To me.
>> > >
>> > > As a matter of fact I'm confused to what the actual question is.
>> > >
>> >
>> > The actual question is whether it is really required that the firmware
>> > is loaded by the kernel into a buffer that is already mapped for DMA
>> > at that point, and thus accessible by the device.
>> >
>> > To me, it is not entirely clear what the nature is of the firmware
>> > that we are talking about, since it seems to be getting passed to the
>> > secure world as well?
>> >
>> > In any case, the preferred model in terms of validation/sig checking is
>> >
>> > 1) allocate a CPU accessible buffer
>> >
>> > 2) request the firmware into it (which may include a sig check under the hood)
>> >
>> > 3) map the buffer for DMA to the device so it can load the firmware.
>> >
>> > 4) kick off the DMA transfer.
>> >
>> > The use of dma_alloc_coherent() for this purpose seems unnecessary,
>> > given that the DMA transfer is not bidirectional. Would it be possible
>> > to replace it with something like the above sequence?
>>
>> Why not just use kmalloc, it will always return a DMAable buffer.
>>
>
> For the buffers being targeted by request_firmware_into_buf() the
> problem is that some of them has requirements of physical placement and
> they are all too big for kmalloc() (i.e. tens of mb).
>
>
> For the dma_alloc_coherent() buffer that was mentioned earlier, which is
> not related to the firmware loading, it's not used because the buffer is
> passed to secure world, which temporarily locks Linux out from the
> memory region. Traditionally this region was kmalloc'ed downstream, but
> due to speculative access violations this code moved to use the DMA
> streaming API, although there's no actual DMA going on.
>

OK, so you are relying on the fact that dma_alloc_coherent() gives you
a device mapping (because the qcom_scm device is described as non
cache coherent), but this sounds risky to me. The linear alias of that
memory will still be mapped cacheable, and could potentially still be
accessed speculatively AFAIK.

> For this a way to allocate a chunk of physical memory dynamically and
> then unmapping and remapping it dynamically in Linux sounds like a
> solution, instead of (ab)using the DMA API. This could also serve as
> basis for the firmware memory, where firmware is position independent -
> in which case this would be passed to request_firmware_into_buf().
>