Messages in this thread | | | From | Jesper Dangaard Brouer <> | Date | Mon, 15 Nov 2021 15:39:35 +0100 | Subject | Re: [PATCH net-next v6] page_pool: disable dma mapping support for 32-bit arch with 64-bit DMA |
| |
On 15/11/2021 13.21, Yunsheng Lin wrote: > On 2021/11/15 20:10, Ilias Apalodimas wrote: >> On Mon, 15 Nov 2021 at 13:53, Ilias Apalodimas >> <ilias.apalodimas@linaro.org> wrote: >>> >>> On Mon, Nov 15, 2021 at 11:34:59AM +0800, Yunsheng Lin wrote: >>>> On 2021/11/12 17:21, Guillaume Tucker wrote: >>>>> On 09/11/2021 12:02, Yunsheng Lin wrote: >>>>>> On 2021/11/9 17:58, Guillaume Tucker wrote: >>>>>>> Hi Yunsheng, >>>>>>> >>>>>>> Please see the bisection report below about a boot failure on >>>>>>> rk3288-rock2-square which is pointing to this patch. The issue >>>>>>> appears to only happen with CONFIG_ARM_LPAE=y. >>>>>>> >>>>>>> Reports aren't automatically sent to the public while we're >>>>>>> trialing new bisection features on kernelci.org but this one >>>>>>> looks valid. >>>>>>> >>>>>>> Some more details can be found here: >>>>>>> >>>>>>> https://linux.kernelci.org/test/case/id/6189968c3ec0a3c06e3358fe/ >>>>>>> >>>>>>> Here's the same revision on the same platform booting fine with a >>>>>>> plain multi_v7_defconfig build: >>>>>>> >>>>>>> https://linux.kernelci.org/test/plan/id/61899d322c0e9fee7e3358ec/ >>>>>>> >>>>>>> Please let us know if you need any help debugging this issue or >>>>>>> if you have a fix to try. >>>>>> >>>>>> The patch below is removing the dma mapping support in page pool >>>>>> for 32 bit systems with 64 bit dma address, so it seems there >>>>>> is indeed a a drvier using the the page pool with PP_FLAG_DMA_MAP >>>>>> flags set in a 32 bit systems with 64 bit dma address. >>>>>> >>>>>> It seems we might need to revert the below patch or implement the >>>>>> DMA-mapping tracking support in the driver as mentioned in the below >>>>>> commit log. >>>>>> >>>>>> which ethernet driver do you use in your system? >>>>> >>>>> Thanks for taking a look and sorry for the slow reply. Here's a >>>>> booting test job with LPAE disabled: >>>>> >>>>> https://linux.kernelci.org/test/plan/id/618dbb81c60c4d94503358f1/ >>>>> https://storage.kernelci.org/mainline/master/v5.15-12452-g5833291ab6de/arm/multi_v7_defconfig/gcc-10/lab-collabora/baseline-nfs-rk3288-rock2-square.html#L812 >>>>> >>>>> [ 8.314523] rk_gmac-dwmac ff290000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx >>>>> >>>>> So the driver is drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c >>>> >>>> Thanks for the report, this patch seems to cause problem for 32-bit >>>> system with LPAE enabled. >>>> >>>> As LPAE seems like a common feature for 32 bits system, this patch >>>> might need to be reverted. >>>> >>>> @Jesper, @Ilias, what do you think? >>> >>> >>> So enabling LPAE also enables CONFIG_ARCH_DMA_ADDR_T_64BIT on that board? >>> Doing a quick grep only selects that for XEN. I am ok reverting that, but >>> I think we need to understand how the dma address ended up being 64bit. >> >> So looking a bit closer, indeed enabling LPAE always enables this. So >> we need to revert the patch. >> Yunsheng will you send that? > > Sure.
Why don't we change that driver[1] to not use page_pool_get_dma_addr() ?
[1] drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
I took a closer look and it seems the driver have struct stmmac_rx_buffer in which is stored the dma_addr it gets from page_pool_get_dma_addr().
See func: stmmac_init_rx_buffers
static int stmmac_init_rx_buffers(struct stmmac_priv *priv, struct dma_desc *p, int i, gfp_t flags, u32 queue) {
if (!buf->page) { buf->page = page_pool_dev_alloc_pages(rx_q->page_pool); if (!buf->page) return -ENOMEM; buf->page_offset = stmmac_rx_offset(priv); } [...]
buf->addr = page_pool_get_dma_addr(buf->page) + buf->page_offset;
stmmac_set_desc_addr(priv, p, buf->addr); [...] }
I question if this driver really to use page_pool for storing the dma_addr as it just extract it and store it outside page_pool?
@Ilias it looks like you added part of the page_pool support in this driver, so I hope you can give a qualified guess on: How much work will it be to let driver do the DMA-map itself? (and not depend on the DMA-map feature provided by page_pool)
--Jesper
| |