Re: [PATCH v7 net] octeontx2-af: switch qmem from coherent DMA alloc to streaming DMA mapping

[Leon Romanovsky]

On Mon, Sep 07, 2026 at 04:12:37PM +0530, Ratheesh Kannoth wrote:
> On 2026-09-07 at 12:50:55, Leon Romanovsky (leon@xxxxxxxxxx) wrote:
> > On Mon, Sep 07, 2026 at 09:56:16AM +0530, Ratheesh Kannoth wrote:
> > > qmem_alloc() uses dma_alloc_attrs() with DMA_ATTR_FORCE_CONTIGUOUS, which
> > > allocates CPU-cache-coherent DMA memory and, with CMA enabled, draws from
> > > the CMA pool. qmem backs NIX/NPA queue contexts, admin queues, and LMTST
> > > regions (including CN10K LMTST areas that span page boundaries), so
> > > consumption grows with enabled interfaces and is hard to provision in CMA.
> > >
> > >
> > > v1 -> v2: Rewrote patch as per sashiko comment
> >
> > Your changelog says nothing. It should contain bullet points describing
> > what was actually changed. A generic "Addressed Sashiko comments" is not
> > sufficient.
> 
> Thanks for pointing this out. I relied on the Sashiko link to avoid
> over-cluttering the notes, as sashiko coments are many.
> I will make sure to explicitly list all notable changes in the
> changelog in future revisions.
> 
> > >
> > > +static inline bool otx2_dma_phys_in_mask(struct device *dev, phys_addr_t paddr,
> > > +					 size_t size)
> > > +{
> > > +	dma_addr_t dma_addr = phys_to_dma(dev, paddr);
> > > +
> > > +	return dma_capable(dev, dma_addr, size, true, 0);
> >
> > This line makes no sense in the driver code.
> 
> ACK. will remove internal APIs
> 
> >
> > > +}
> > > +
> > > +static inline void *otx2_dma_alloc_coherent(struct device *dev, size_t size,
> > > +					    dma_addr_t *dma_handle)
> > > +{
> > > +	dma_addr_t dma_addr;
> > > +	unsigned int order;
> > > +	gfp_t alloc_gfp;
> > > +	void *vaddr;
> > > +
> > > +	if (!dev || !dma_handle || !size)
> > > +		return NULL;
> >
> > Please remove defensive programming style, how can you call to DMA API
> > without device, dma_handle or size?
> ACK.
> 
> >
> > > +
> > > +	if (!dev_is_dma_coherent(dev))
> > > +		return NULL;
> > > +
> > > +	size = PAGE_ALIGN(size);
> > > +	order = get_order(size);
> > > +
> > > +	/* Octeontx2 qmem call sites size their allocations within
> > > +	 * MAX_PAGE_ORDER; mailbox, queue context, and ring memory
> > > +	 * requirements stay below the buddy allocator's limit.
> > > +	 */
> > > +	if (order > MAX_PAGE_ORDER) {
> >
> > Size is coming from the kernel, how can it be with order more than MAX_PAGE_ORDER?
> There is contigious memory allocation request from driver for PF-to-VF mail box memory.
> It is crossing max page order in newer platforms as number of VFs per PF increased.

I'm not sure what this means. You can't create a VF without assigning it
enough memory for DMA. You shouldn't get an "order > MAX_PAGE_ORDER"
error at this stage. If you do, there is likely another bug involved.

Thanks

> 
> >
> > > +		dev_err(dev,
> > > +			"CONFIG_ARCH_FORCE_MAX_ORDER is set to %u, minimum needed is %u\n",
> > > +			MAX_PAGE_ORDER, order);
> > > +		return NULL;
> > > +	}
> > > +
> > > +	if (size > dma_max_mapping_size(dev))
> > > +		return NULL;
> >
> > Same comment.
> Will remove this code and depend on dma_map_page_attrs().
> 
> >
> > > +
> > > +	alloc_gfp = GFP_KERNEL | __GFP_ZERO | __GFP_COMP;
> >
> > __GFP_COMP???
> Will remove this flag.
> 
> >
> > > +
> > > +	vaddr = (void *)__get_free_pages(alloc_gfp, order);
> >
> > You should use plain ksmalloc(). There was ongoing effort to remove
> > useless __get_free_pages().
> > https://lore.kernel.org/all/20260713-b4-rdma-v2-0-65d2a1a5180c@xxxxxxxxxx/
> will replace get_free_pages with kmalloc.
> 
> >
> > > +	while (vaddr &&
> > > +	       !otx2_dma_phys_in_mask(dev, virt_to_phys(vaddr), size)) {
> > > +		free_pages((unsigned long)vaddr, order);
> > > +		if (alloc_gfp & GFP_DMA32)
> > > +			return NULL;
> > > +		alloc_gfp |= GFP_DMA32;
> > > +		vaddr = (void *)__get_free_pages(alloc_gfp, order);
> > > +	}
> >
> > If you want to use streamline API, please use it like any other driver
> > without these DMA32 hacks.
> Will kmalloc() with GFP_ZERO | GFP_KERNEL
> 
> >
> > > +	if (!vaddr)
> > > +		return NULL;
> > > +
> > > +	/* dev_is_dma_coherent() only guarantees cache coherency, not that the
> > > +	 * mapped DMA address aliases qmem->base.  Require a coherent mapping
> > > +	 * so the DMA API rejects SWIOTLB bounce buffers.
> > > +	 */
> > > +	dma_addr = dma_map_page_attrs(dev, virt_to_page(vaddr), 0, size,
> > > +				      DMA_BIDIRECTIONAL, DMA_ATTR_REQUIRE_COHERENT);
> >
> > I don't understand why you insist on DMA_ATTR_REQUIRE_COHERENT. It is
> > clearly documented as being required for UAPI-visible memory.
> ACK.
> I overlooked this uAPI part, as  AI tool recommended adding
> this logic to reject allocations with SWIOTLB or cache management
> support.