Hi,
i have upported a wifi-driver (mt6625l for armhf) for some time and fall now (at least 5.18) in the
"rejecting DMA map of vmalloc memory" error [1].
maybe anybody here can guide me on how to nail it down and maybe fix it.
as far as i have debugged it, it uses dma_map_single [2] to get dma memory from a previous
allocated memory region.
this function "kalDevPortRead" in [2] is used via macro HAL_PORT_RD [3] (used in HAL_READ_RX_PORT
and HAL_READ_INTR_STATUS in same hal.h file)
HAL_READ_INTR_STATUS is always called with an empty int array as buf which i guess is not the problem.
I think the issue is using the use with an preallocated prSDIOCtrl struct (have not completely traced
it back where it is allocated).
calls of HAL_PORT_RD/HAL_READ_RX_PORT are in nic{,_rx}.c (with sdio-struct) ([4] as example)
maybe there is a simple way to get an address in preallocated memory as replacement for the dma_map_simple call (and the unmap of course).
regards Frank
[1] https://elixir.bootlin.com/linux/latest/source/include/linux/dma-mapping.h#L327
[2] https://github.com/frank-w/BPI-R2-4.14/blob/5.18-main/drivers/misc/mediatek/connectivity/wlan/gen2/os/linux/hif/ahb/ahb.c#L940
[3] https://github.com/frank-w/BPI-R2-4.14/blob/5.18-main/drivers/misc/mediatek/connectivity/wlan/gen2/include/nic/hal.h#L176
[4] https://github.com/frank-w/BPI-R2-4.14/blob/5.18-main/drivers/misc/mediatek/connectivity/wlan/gen2/nic/nic_rx.c#L3604
On 2022-06-15 13:11, Frank Wunderlich wrote:
> Hi,
>
> i have upported a wifi-driver (mt6625l for armhf) for some time and fall now (at least 5.18) in the
> "rejecting DMA map of vmalloc memory" error [1].
>
> maybe anybody here can guide me on how to nail it down and maybe fix it.
>
> as far as i have debugged it, it uses dma_map_single [2] to get dma memory from a previous
> allocated memory region.
>
> this function "kalDevPortRead" in [2] is used via macro HAL_PORT_RD [3] (used in HAL_READ_RX_PORT
> and HAL_READ_INTR_STATUS in same hal.h file)
>
> HAL_READ_INTR_STATUS is always called with an empty int array as buf which i guess is not the problem.
> I think the issue is using the use with an preallocated prSDIOCtrl struct (have not completely traced
> it back where it is allocated).
Put simply, if you want to call dma_map_single() on a buffer, then that
buffer needs to be allocated with kmalloc() (or technically
alloc_pages(), but then dma_map_page() would make more sense when
dealing with entire pages.
Robin.
> calls of HAL_PORT_RD/HAL_READ_RX_PORT are in nic{,_rx}.c (with sdio-struct) ([4] as example)
>
> maybe there is a simple way to get an address in preallocated memory as replacement for the dma_map_simple call (and the unmap of course).
>
> regards Frank
>
> [1] https://elixir.bootlin.com/linux/latest/source/include/linux/dma-mapping.h#L327
> [2] https://github.com/frank-w/BPI-R2-4.14/blob/5.18-main/drivers/misc/mediatek/connectivity/wlan/gen2/os/linux/hif/ahb/ahb.c#L940
> [3] https://github.com/frank-w/BPI-R2-4.14/blob/5.18-main/drivers/misc/mediatek/connectivity/wlan/gen2/include/nic/hal.h#L176
> [4] https://github.com/frank-w/BPI-R2-4.14/blob/5.18-main/drivers/misc/mediatek/connectivity/wlan/gen2/nic/nic_rx.c#L3604
On Wed, Jun 15, 2022 at 02:15:33PM +0100, Robin Murphy wrote:
> Put simply, if you want to call dma_map_single() on a buffer, then that
> buffer needs to be allocated with kmalloc() (or technically alloc_pages(),
> but then dma_map_page() would make more sense when dealing with entire
> pages.
Yes. It sounds like the memory here comes from the dma coherent
allocator, in which case the code need to use the address returned
by that and not create another mapping.
Am 15. Juni 2022 15:17:00 MESZ schrieb Christoph Hellwig <[email protected]>:
>On Wed, Jun 15, 2022 at 02:15:33PM +0100, Robin Murphy wrote:
>> Put simply, if you want to call dma_map_single() on a buffer, then that
>> buffer needs to be allocated with kmalloc() (or technically alloc_pages(),
>> but then dma_map_page() would make more sense when dealing with entire
>> pages.
>
>Yes. It sounds like the memory here comes from the dma coherent
>allocator, in which case the code need to use the address returned
>by that and not create another mapping.
As i have not found position where memory is allocated (this is a very huge and dirty driver) is it maybe possible to check if buf is such "allready dma" memory (maybe is_vmalloc_addr) and call dma_single_map only if not (using original buf if yes)?
But i guess it should map only a part of available (pre-allocated) memory and other parts of this are used somewhere else. So i can ran into some issues caused by sharing this full block in different functions.
Hi,
Thanks for first suggestions.
regards Frank
Am 15. Juni 2022 15:17:00 MESZ schrieb Christoph Hellwig <[email protected]>:
>On Wed, Jun 15, 2022 at 02:15:33PM +0100, Robin Murphy wrote:
>> Put simply, if you want to call dma_map_single() on a buffer, then that
>> buffer needs to be allocated with kmalloc() (or technically alloc_pages(),
>> but then dma_map_page() would make more sense when dealing with entire
>> pages.
>
>Yes. It sounds like the memory here comes from the dma coherent
>allocator, in which case the code need to use the address returned
>by that and not create another mapping.
Hi
traced it to buffer allocated as simple uint8-array [1]:
UINT_8 aucBuffer[sizeof(INIT_HIF_RX_HEADER_T) + sizeof(INIT_EVENT_CMD_RESULT)];
and then called as
nicRxWaitResponse(prAdapter,0,aucBuffer,sizeof(INIT_HIF_RX_HEADER_T) + sizeof(INIT_EVENT_CMD_RESULT),/* 4B + 4B */
&u4RxPktLength)
this calls [2]:
WLAN_STATUS
nicRxWaitResponse(IN P_ADAPTER_T prAdapter,
IN UINT_8 ucPortIdx, OUT PUINT_8 pucRspBuffer, IN UINT_32 u4MaxRespBufferLen, OUT PUINT_32 pu4Length)
{
...
HAL_PORT_RD(prAdapter,ucPortIdx == 0 ? MCR_WRDR0 : MCR_WRDR1, u4PktLen, pucRspBuffer, u4MaxRespBufferLen);
}
nicRxWaitResponse contains a do-while(true)-loop, but it looks like this is failing on first call (i see my debug before call of HAL_PORT_RD only once)...
do i need kmalloc before call of nicRxWaitResponse and if yes which flags are right here?
https://www.kernel.org/doc/htmldocs/kernel-api/API-kmalloc.html
callstack is like this:
[ 126.919569] __dma_page_dev_to_cpu from kalDevPortRead+0x3a0/0x6b4 [wlan_gen2]
[ 126.928643] kalDevPortRead [wlan_gen2] from nicRxWaitResponse+0x4c0/0x61c [wlan_gen2]
[ 126.939752] nicRxWaitResponse [wlan_gen2] from wlanImageSectionDownloadStatus.part.0+0xd0/0x26c [wlan_gen2]
[ 126.952783] wlanImageSectionDownloadStatus.part.0 [wlan_gen2] from wlanImageSectionDownload+0x168/0x36c [wlan_gen2]
[ 126.966511] wlanImageSectionDownload [wlan_gen2] from wlanAdapterStart+0x674/0xf94 [wlan_gen2]
[ 126.978367] wlanAdapterStart [wlan_gen2] from wlanProbe+0x318/0xbe8 [wlan_gen2]
[ 126.988951] wlanProbe [wlan_gen2] from HifAhbProbe+0x54/0x88 [wlan_gen2]
[ 126.998905] HifAhbProbe [wlan_gen2] from wmt_func_wifi_on+0x4c/0x150
regards Frank
[1] https://github.com/frank-w/BPI-R2-4.14/blob/5.18-main/drivers/misc/mediatek/connectivity/wlan/gen2/common/wlan_lib.c#L3046
[2] https://github.com/frank-w/BPI-R2-4.14/blob/5.18-main/drivers/misc/mediatek/connectivity/wlan/gen2/nic/nic_rx.c#L3604
On 2022-06-17 17:17, Frank Wunderlich wrote:
> Am 15. Juni 2022 15:17:00 MESZ schrieb Christoph Hellwig <[email protected]>:
>> On Wed, Jun 15, 2022 at 02:15:33PM +0100, Robin Murphy wrote:
>>> Put simply, if you want to call dma_map_single() on a buffer, then that
>>> buffer needs to be allocated with kmalloc() (or technically alloc_pages(),
>>> but then dma_map_page() would make more sense when dealing with entire
>>> pages.
>>
>> Yes. It sounds like the memory here comes from the dma coherent
>> allocator, in which case the code need to use the address returned
>> by that and not create another mapping.
>
> Hi
>
> traced it to buffer allocated as simple uint8-array [1]:
>
> UINT_8 aucBuffer[sizeof(INIT_HIF_RX_HEADER_T) + sizeof(INIT_EVENT_CMD_RESULT)];
Ah, so it's trying to do DMA with a stack variable? CONFIG_DMA_API_DEBUG
is your friend; it should have screamed about that specifically.
Allocate this buffer properly to begin with, and free it again on the
way out of the function (it's surely not worth having to make a
temporary copy further down the callchain). The kmalloc flags can
probably be regular GFP_KERNEL, unless this can be called from more
restrictive contexts like an IRQ handler - the fact that it might be
mapped for DMA later is essentially irrelevant in that respect.
Thanks,
Robin.
>
> and then called as
>
> nicRxWaitResponse(prAdapter,0,aucBuffer,sizeof(INIT_HIF_RX_HEADER_T) + sizeof(INIT_EVENT_CMD_RESULT),/* 4B + 4B */
> &u4RxPktLength)
>
> this calls [2]:
>
> WLAN_STATUS
> nicRxWaitResponse(IN P_ADAPTER_T prAdapter,
> IN UINT_8 ucPortIdx, OUT PUINT_8 pucRspBuffer, IN UINT_32 u4MaxRespBufferLen, OUT PUINT_32 pu4Length)
> {
> ...
> HAL_PORT_RD(prAdapter,ucPortIdx == 0 ? MCR_WRDR0 : MCR_WRDR1, u4PktLen, pucRspBuffer, u4MaxRespBufferLen);
> }
>
>
> nicRxWaitResponse contains a do-while(true)-loop, but it looks like this is failing on first call (i see my debug before call of HAL_PORT_RD only once)...
>
> do i need kmalloc before call of nicRxWaitResponse and if yes which flags are right here?
> https://www.kernel.org/doc/htmldocs/kernel-api/API-kmalloc.html
>
> callstack is like this:
>
> [ 126.919569] __dma_page_dev_to_cpu from kalDevPortRead+0x3a0/0x6b4 [wlan_gen2]
> [ 126.928643] kalDevPortRead [wlan_gen2] from nicRxWaitResponse+0x4c0/0x61c [wlan_gen2]
> [ 126.939752] nicRxWaitResponse [wlan_gen2] from wlanImageSectionDownloadStatus.part.0+0xd0/0x26c [wlan_gen2]
> [ 126.952783] wlanImageSectionDownloadStatus.part.0 [wlan_gen2] from wlanImageSectionDownload+0x168/0x36c [wlan_gen2]
> [ 126.966511] wlanImageSectionDownload [wlan_gen2] from wlanAdapterStart+0x674/0xf94 [wlan_gen2]
> [ 126.978367] wlanAdapterStart [wlan_gen2] from wlanProbe+0x318/0xbe8 [wlan_gen2]
> [ 126.988951] wlanProbe [wlan_gen2] from HifAhbProbe+0x54/0x88 [wlan_gen2]
> [ 126.998905] HifAhbProbe [wlan_gen2] from wmt_func_wifi_on+0x4c/0x150
>
> regards Frank
>
> [1] https://github.com/frank-w/BPI-R2-4.14/blob/5.18-main/drivers/misc/mediatek/connectivity/wlan/gen2/common/wlan_lib.c#L3046
> [2] https://github.com/frank-w/BPI-R2-4.14/blob/5.18-main/drivers/misc/mediatek/connectivity/wlan/gen2/nic/nic_rx.c#L3604
> Gesendet: Freitag, 17. Juni 2022 um 19:22 Uhr
> Von: "Robin Murphy" <[email protected]>
> An: "Frank Wunderlich" <[email protected]>, "Christoph Hellwig" <[email protected]>
> Cc: [email protected], [email protected], "Marek Szyprowski" <[email protected]>
> Betreff: Re: helping with remapping vmem for dma
>
> On 2022-06-17 17:17, Frank Wunderlich wrote:
> > Am 15. Juni 2022 15:17:00 MESZ schrieb Christoph Hellwig <[email protected]>:
> >> On Wed, Jun 15, 2022 at 02:15:33PM +0100, Robin Murphy wrote:
> >>> Put simply, if you want to call dma_map_single() on a buffer, then that
> >>> buffer needs to be allocated with kmalloc() (or technically alloc_pages(),
> >>> but then dma_map_page() would make more sense when dealing with entire
> >>> pages.
> >>
> >> Yes. It sounds like the memory here comes from the dma coherent
> >> allocator, in which case the code need to use the address returned
> >> by that and not create another mapping.
> >
> > Hi
> >
> > traced it to buffer allocated as simple uint8-array [1]:
> >
> > UINT_8 aucBuffer[sizeof(INIT_HIF_RX_HEADER_T) + sizeof(INIT_EVENT_CMD_RESULT)];
>
> Ah, so it's trying to do DMA with a stack variable? CONFIG_DMA_API_DEBUG
> is your friend; it should have screamed about that specifically.
> Allocate this buffer properly to begin with, and free it again on the
> way out of the function (it's surely not worth having to make a
> temporary copy further down the callchain). The kmalloc flags can
> probably be regular GFP_KERNEL, unless this can be called from more
> restrictive contexts like an IRQ handler - the fact that it might be
> mapped for DMA later is essentially irrelevant in that respect.
Hi,
simply replaced the stack-vars to uint_8-pointers and using kmalloc/kfree for
memory handling (needed to replace some returns to goto to always free memory).
Thanks very much for support, driver is now working again :)
https://github.com/frank-w/BPI-R2-4.14/commit/7f3a721d5b0d8ca44935c23d5513a19cc57786c0
> Thanks,
> Robin.