On 7 June 2018 at 18:18, Bjorn Andersson <bjorn.andersson@xxxxxxxxxx> 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? -- Ard. >> Perhaps someone who has this hardware can find out empirically for us, as >> follows (mm folks is this right?): >> >> page = virt_to_page(address); >> if (!page) >> fail closed... >> if (page_zone(page) == ZONE_DMA || page_zone(page) == ZONE_DMA32) >> this is a DMA buffer >> else >> not DMA! >> > > Where do you want to put this? > >> Note that when request_firmware_into_buf() was being reviewed Mimi had asked back >> in 2016 [0] that if a DMA buffer was going to be used READING_FIRMWARE_DMA should be >> used otherwise READING_FIRMWARE_PREALLOC_BUFFER was fine. >> >> If it is a DMA buffer *now*, why / how did this change? >> > > From what I can see [0] says is to use READING_FIRMWARE_PREALLOC_BUFFER > regardless of where the memory comes from. > > Regards, > Bjorn > >> [0] https://patchwork.kernel.org/patch/9074611/ >> >> Luis _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
