On 2023/8/22 2:35, Jakub Kicinski wrote:

On Mon, 21 Aug 2023 20:18:55 +0800 Yunsheng Lin wrote:

quoted

quoted

-	page_pool_set_dma_addr(page, dma);
+	if (page_pool_set_dma_addr(page, dma))
+		goto unmap_failed;  

What does the driver do when the above fails?
Does the driver still need to implement a fallback for 32 bit arch with
dma addr with more than 32 + 12 bits?
If yes, it does not seems to be very helpful from driver's point of view
as the driver might still need to call page allocator API directly when
the above fails.

I'd expect the driver to do nothing, we are operating under 
the assumption that "this will never happen". If it does 
the user should report it back to us. So maybe..

Digging a litte deeper:
From CPU's point of views, up to 40 bits is supported for LPAE.
From device's point of view, it seems up to 48-bits is supported
when iommu is enabled, see below:
https://elixir.free-electrons.com/linux/v6.5-rc7/source/drivers/iommu/Kconfig#L28

quoted

quoted

 	if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV)
 		page_pool_dma_sync_for_device(pool, page, pool->p.max_len);
 
 	return true;
+
+unmap_failed:

.. we should also add a:

	WARN_ONCE(1, "misaligned DMA address, please report to netdev@");

As the CONFIG_PHYS_ADDR_T_64BIT seems to used widely in x86/arm/mips/powerpc,
I am not sure if we can really make the above assumption.

https://elixir.free-electrons.com/linux/v6.4-rc6/K/ident/CONFIG_PHYS_ADDR_T_64BIT

here?

quoted

quoted

+	dma_unmap_page_attrs(pool->p.dev, dma,
+			     PAGE_SIZE << pool->p.order, pool->p.dma_dir,
+			     DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING);
+	return false;

.