Re: helping with remapping vmem for dma

Re: helping with remapping vmem for dma

[Frank Wunderlich]

Am 15. Juni 2022 15:17:00 MESZ schrieb Christoph Hellwig <hch@lst.de>:
>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

Re: helping with remapping vmem for dma

[Frank Wunderlich]

Am 15. Juni 2022 15:17:00 MESZ schrieb Christoph Hellwig <hch@lst.de>:
>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
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: helping with remapping vmem for dma

[Robin Murphy]

On 2022-06-17 17:17, Frank Wunderlich wrote:
> Am 15. Juni 2022 15:17:00 MESZ schrieb Christoph Hellwig <hch@lst.de>:
>> 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

Re: helping with remapping vmem for dma

[Robin Murphy]

On 2022-06-17 17:17, Frank Wunderlich wrote:
> Am 15. Juni 2022 15:17:00 MESZ schrieb Christoph Hellwig <hch@lst.de>:
>> 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
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Aw: Re: helping with remapping vmem for dma

[Frank Wunderlich]

> Gesendet: Freitag, 17. Juni 2022 um 19:22 Uhr
> Von: "Robin Murphy" <robin.murphy@arm.com>
> An: "Frank Wunderlich" <frank-w@public-files.de>, "Christoph Hellwig" <hch@lst.de>
> Cc: linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, "Marek Szyprowski" <m.szyprowski@samsung.com>
> 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 <hch@lst.de>:
> >> 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.

Aw: Re: helping with remapping vmem for dma

[Frank Wunderlich]

> Gesendet: Freitag, 17. Juni 2022 um 19:22 Uhr
> Von: "Robin Murphy" <robin.murphy@arm.com>
> An: "Frank Wunderlich" <frank-w@public-files.de>, "Christoph Hellwig" <hch@lst.de>
> Cc: linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, "Marek Szyprowski" <m.szyprowski@samsung.com>
> 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 <hch@lst.de>:
> >> 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.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu