mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v6 net] octeontx2-af: switch qmem from coherent DMA alloc to streaming DMA mapping
@ 2026-09-02  2:10 Ratheesh Kannoth
  2026-09-03  2:45 ` Ratheesh Kannoth
  2026-09-05 20:02 ` netdev-bot+sashiko
  0 siblings, 2 replies; 3+ messages in thread
From: Ratheesh Kannoth @ 2026-09-02  2:10 UTC (permalink / raw)
  To: davem, gakula, linux-kernel, netdev, sgoutham
  Cc: andrew+netdev, edumazet, kuba, pabeni, leon, Qqingfang.deng,
	Ratheesh Kannoth

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.

Switch qmem to a streaming-DMA-style path: allocate physically contiguous
compound pages from the buddy allocator via __get_free_pages(), then map
them for device access with dma_map_page_attrs(). Add
otx2_dma_alloc_coherent() and otx2_dma_free_coherent() helpers that
enforce dev_is_dma_coherent(), retry with GFP_DMA32 when the physical
range is outside the device DMA mask, and wire qmem_alloc()/qmem_free()
through them instead of dma_alloc_attrs()/dma_free_attrs().

This works on Octeon because the octeontx2 driver is written for
DMA-coherent devices: Octeon platforms provide IO coherency (via SMMU), so
the driver already uses streaming DMA APIs for packet data while
deliberately skipping explicit CPU cache sync (DMA_ATTR_SKIP_CPU_SYNC).
The same IO coherency lets qmem use a streaming map of buddy-allocated
pages instead of a dedicated coherent allocator or CMA reservation. That
is valid because the platform is DMA-coherent, not because omitting
dma_sync_* magically makes memory coherent.

Allocations requiring more than MAX_PAGE_ORDER pages are still rejected,
since the buddy allocator cannot serve them without CMA.

cc: Geetha sowjanya <gakula@marvell.com>
Fixes: 73d33dbc0723 ("octeontx2-af: Use DMA_ATTR_FORCE_CONTIGUOUS attribute in DMA alloc")
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>

---
v5 -> v6: Addressed Review comments
	https://lore.kernel.org/netdev/20260901015621.2708182-1-rkannoth@marvell.com/

v4 -> v5: Fixed compilation issues.
	https://lore.kernel.org/netdev/20260831024210.208447-1-rkannoth@marvell.com/

v3 -> v4: Fixed compilation issues.
	https://lore.kernel.org/netdev/apTpKcN_S1xIwRbZ@rkannoth-OptiPlex-7090/

v2 -> v3: Addressed sashiko comments
	https://sashiko.dev/#/patchset/20260825045616.3723078-1-rkannoth%40marvell.com

v1 -> v2: Rewrote patch as per sashiko comment
---
 .../ethernet/marvell/octeontx2/af/common.h    | 84 +++++++++++++++++--
 1 file changed, 78 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/common.h b/drivers/net/ethernet/marvell/octeontx2/af/common.h
index 779413a383b7..4ec20c3cfec0 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/common.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/common.h
@@ -7,6 +7,11 @@
 #ifndef COMMON_H
 #define COMMON_H
 
+#include <linux/dma-mapping.h>
+#include <linux/dma-map-ops.h>
+#include <linux/gfp.h>
+#include <linux/mm.h>
+
 #include "rvu_struct.h"
 
 #define OTX2_ALIGN			128  /* Align to cacheline */
@@ -44,6 +49,74 @@ struct qmem {
 	u32		qsize;
 };
 
+static inline bool otx2_dma_phys_in_mask(struct device *dev, phys_addr_t paddr,
+					 size_t size)
+{
+	u64 mask = dma_get_mask(dev);
+
+	return paddr + size - 1 <= mask;
+}
+
+static inline void *otx2_dma_alloc_coherent(struct device *dev, size_t size,
+					    dma_addr_t *dma_handle, gfp_t gfp)
+{
+	dma_addr_t dma_addr;
+	unsigned int order;
+	gfp_t alloc_gfp;
+	void *vaddr;
+
+	if (!dev || !dma_handle || !size)
+		return NULL;
+
+	if (!dev_is_dma_coherent(dev))
+		return NULL;
+
+	size = PAGE_ALIGN(size);
+	order = get_order(size);
+	if (order > MAX_PAGE_ORDER)
+		return NULL;
+
+	alloc_gfp = (gfp & ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM)) |
+		__GFP_ZERO | __GFP_COMP | __GFP_RECLAIM;
+
+	vaddr = (void *)__get_free_pages(alloc_gfp, order);
+	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 (!vaddr)
+		return NULL;
+
+	dma_addr = dma_map_page_attrs(dev, virt_to_page(vaddr), 0, size,
+				      DMA_BIDIRECTIONAL, 0);
+	if (dma_mapping_error(dev, dma_addr)) {
+		free_pages((unsigned long)vaddr, order);
+		return NULL;
+	}
+
+	*dma_handle = dma_addr;
+	return vaddr;
+}
+
+static inline void otx2_dma_free_coherent(struct device *dev, size_t size,
+					  void *vaddr, dma_addr_t dma_handle)
+{
+	unsigned int order;
+
+	if (!dev || !vaddr)
+		return;
+
+	size = PAGE_ALIGN(size);
+	order = get_order(size);
+
+	dma_unmap_page_attrs(dev, dma_handle, size, DMA_BIDIRECTIONAL, 0);
+	free_pages((unsigned long)vaddr, order);
+}
+
 static inline int qmem_alloc(struct device *dev, struct qmem **q,
 			     int qsize, int entry_sz)
 {
@@ -60,8 +133,8 @@ static inline int qmem_alloc(struct device *dev, struct qmem **q,
 
 	qmem->entry_sz = entry_sz;
 	qmem->alloc_sz = (qsize * entry_sz) + OTX2_ALIGN;
-	qmem->base = dma_alloc_attrs(dev, qmem->alloc_sz, &qmem->iova,
-				     GFP_KERNEL, DMA_ATTR_FORCE_CONTIGUOUS);
+	qmem->base = otx2_dma_alloc_coherent(dev, qmem->alloc_sz, &qmem->iova,
+					     GFP_KERNEL);
 	if (!qmem->base)
 		return -ENOMEM;
 
@@ -80,10 +153,9 @@ static inline void qmem_free(struct device *dev, struct qmem *qmem)
 		return;
 
 	if (qmem->base)
-		dma_free_attrs(dev, qmem->alloc_sz,
-			       qmem->base - qmem->align,
-			       qmem->iova - qmem->align,
-			       DMA_ATTR_FORCE_CONTIGUOUS);
+		otx2_dma_free_coherent(dev, qmem->alloc_sz,
+				       qmem->base - qmem->align,
+				       qmem->iova - qmem->align);
 	devm_kfree(dev, qmem);
 }
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v6 net] octeontx2-af: switch qmem from coherent DMA alloc to streaming DMA mapping
  2026-09-02  2:10 [PATCH v6 net] octeontx2-af: switch qmem from coherent DMA alloc to streaming DMA mapping Ratheesh Kannoth
@ 2026-09-03  2:45 ` Ratheesh Kannoth
  2026-09-05 20:02 ` netdev-bot+sashiko
  1 sibling, 0 replies; 3+ messages in thread
From: Ratheesh Kannoth @ 2026-09-03  2:45 UTC (permalink / raw)
  To: davem, gakula, linux-kernel, netdev, sgoutham
  Cc: andrew+netdev, edumazet, kuba, pabeni, leon, Qqingfang.deng

On 2026-09-02 at 07:40:23, Ratheesh Kannoth (rkannoth@marvell.com) 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.
>
> Switch qmem to a streaming-DMA-style path: allocate physically contiguous
> compound pages from the buddy allocator via __get_free_pages(), then map
> them for device access with dma_map_page_attrs(). Add
> otx2_dma_alloc_coherent() and otx2_dma_free_coherent() helpers that
> enforce dev_is_dma_coherent(), retry with GFP_DMA32 when the physical
> range is outside the device DMA mask, and wire qmem_alloc()/qmem_free()
> through them instead of dma_alloc_attrs()/dma_free_attrs().
>
> This works on Octeon because the octeontx2 driver is written for
> DMA-coherent devices: Octeon platforms provide IO coherency (via SMMU), so
> the driver already uses streaming DMA APIs for packet data while
> deliberately skipping explicit CPU cache sync (DMA_ATTR_SKIP_CPU_SYNC).
> The same IO coherency lets qmem use a streaming map of buddy-allocated
> pages instead of a dedicated coherent allocator or CMA reservation. That
> is valid because the platform is DMA-coherent, not because omitting
> dma_sync_* magically makes memory coherent.
>
> Allocations requiring more than MAX_PAGE_ORDER pages are still rejected,
> since the buddy allocator cannot serve them without CMA.

My reply to sashiko comments

+----+------------+------------+-----+----------+----------------+---------------+-----------------+
| #  | Location   | Issue      | Sev | Preexist | When it bites  | Suggested fix | My comments     |
+----+------------+------------+-----+----------+----------------+---------------+-----------------+
| 1  | otx2_dma_  | alloc_gfp  | Med | No       | GFP_ATOMIC/    | Preserve      |  All existing callers of this static function
|    | alloc_     | ORs in     |     |          | NOWAIT may     | caller GFP;   |  use GFP_KERNEL. So in sleep in context.
|    | coherent() | __GFP_     |     |          | sleep in       | add RECLAIM   |                 |
|    | GFP        | RECLAIM;   |     |          | atomic ctx     | only when     |                 |
|    | handling   | violates   |     |          | (sched while   | allowed.      |                 |
|    |            | caller GFP |     |          | atomic).       |               |                 |
|    |            | semantics  |     |          | qmem uses      |               |                 |
|    |            |            |     |          | GFP_KERNEL.    |               |                 |
+----+------------+------------+-----+----------+----------------+---------------+-----------------+
| 2  | otx2_dma_  | Coherent   | Hi  | No       | SWIOTLB bounce | Keep coherent |                 |
|    | alloc_     | DMA ->     |     |          | (swiotlb=force | DMA or add    | Commit message clearly explains the reason
|    | coherent() | streaming  |     |          | ARM CCA): CPU  | dma_sync_*;   |                 |
|    | DMA map    | map; no    |     |          | and device use | detect bounce |                 |
|    |            | dma_sync_* |     |          | disjoint mem;  | fallback.     |                 |
|    |            | in queue   |     |          | queue broken.  |               |                 |
+----+------------+------------+-----+----------+----------------+---------------+-----------------+

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v6 net] octeontx2-af: switch qmem from coherent DMA alloc to streaming DMA mapping
  2026-09-02  2:10 [PATCH v6 net] octeontx2-af: switch qmem from coherent DMA alloc to streaming DMA mapping Ratheesh Kannoth
  2026-09-03  2:45 ` Ratheesh Kannoth
@ 2026-09-05 20:02 ` netdev-bot+sashiko
  1 sibling, 0 replies; 3+ messages in thread
From: netdev-bot+sashiko @ 2026-09-05 20:02 UTC (permalink / raw)
  To: rkannoth
  Cc: davem, gakula, linux-kernel, netdev, sgoutham, andrew+netdev,
	edumazet, kuba, pabeni, leon, Qqingfang.deng

Thank you for your contribution! Sashiko AI review found 6 potential
issue(s) to consider:

Critical: 0 · High: 2 · Medium: 3 (1) · Low: 1

- [High] New hard MAX_PAGE_ORDER ceiling in otx2_dma_alloc_coherent()
  (drivers/net/ethernet/marvell/octeontx2/af/common.h:76-77) rejects…
- [High] qmem regions are long-lived, bidirectionally shared hardware
  structures (NIX/NPA context tables, AF admin-queue instruction/result…
- [Medium] otx2_dma_phys_in_mask() (af/common.h:52-57) compares a raw CPU
  physical address against dma_get_mask(dev).
- [Medium] otx2_dma_alloc_coherent() maps up to 1<<MAX_PAGE_ORDER bytes'
  worth of pages (4 MiB with 4 KiB pages) in a single…
- [Medium] otx2_dma_alloc_coherent() unconditionally forces __GFP_RECLAIM
  on all allocations, violating caller-provided GFP semantics.
- [Low] The patch adds '#include <linux/dma-map-ops.h>' to
  drivers/net/ethernet/marvell/octeontx2/af/common.h (line 11) solely to…

Pre-existing issues:
- [Medium] qmem_alloc() computes 'qmem->alloc_sz = (qsize * entry_sz) +
  OTX2_ALIGN' in signed int arithmetic (af/common.h:121,135) from values…

--

octeontx2-af: switch qmem from coherent DMA alloc to streaming DMA mapping

This replaces the dma_alloc_attrs(DMA_ATTR_FORCE_CONTIGUOUS) backing of
qmem with buddy-allocated compound pages that are then mapped for the
device with dma_map_page_attrs(DMA_BIDIRECTIONAL).  New helpers
otx2_dma_alloc_coherent() and otx2_dma_free_coherent() gate on
dev_is_dma_coherent() and retry with GFP_DMA32 when the physical range
falls outside the device DMA mask.  A few questions below.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/common.h b/drivers/net/ethernet/marvell/octeontx2/af/common.h
> index 779413a383b74..4ec20c3cfec03 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/common.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/common.h
> @@ -7,6 +7,11 @@
>  #ifndef COMMON_H
>  #define COMMON_H
>  
> +#include <linux/dma-mapping.h>
> +#include <linux/dma-map-ops.h>

[Severity: Low]
This isn't a bug, but is including linux/dma-map-ops.h from a driver
header intended?  That header opens with:

  /*
   * This header is for implementations of dma_map_ops and related code.
   * It should not be included in drivers just using the DMA API.
   */

common.h is included across af/ and nic/, so this pulls DMA
implementation internals into the whole driver, apparently only to reach
dev_is_dma_coherent().

Related to that, on builds without ARCH_HAS_SYNC_DMA_FOR_DEVICE and
ARCH_HAS_SYNC_DMA_FOR_CPU, the same header defines:

  static inline bool dev_is_dma_coherent(struct device *dev)
  {
	return true;
  }

so the guard below is a compile-time constant rather than a runtime
check on those configurations.  Is that the intent?

> +#include <linux/gfp.h>
> +#include <linux/mm.h>
> +
>  #include "rvu_struct.h"
>  
>  #define OTX2_ALIGN			128  /* Align to cacheline */
> @@ -44,6 +49,74 @@ struct qmem {
>  	u32		qsize;
>  };
>  
> +static inline bool otx2_dma_phys_in_mask(struct device *dev, phys_addr_t paddr,
> +					 size_t size)
> +{
> +	u64 mask = dma_get_mask(dev);
> +
> +	return paddr + size - 1 <= mask;
> +}

[Severity: Medium]
Is comparing a CPU physical address against dma_get_mask() the test that
was wanted here?  The DMA mask constrains device (bus or IOVA)
addresses, so the equivalent core-API check goes through phys_to_dma()
and dma_capable(), which also account for dev->dma_range_map offsets and
dev->bus_dma_limit.

Two consequences follow:

- Behind an SMMU (the configuration the changelog relies on) the IOVA is
  allocated inside the mask no matter where the page sits physically, so
  freeing the pages and retrying with GFP_DMA32 is a constraint the
  hardware does not have, and it can end in NULL when no high-order
  ZONE_DMA32 block is available.

- With a non-zero dma-ranges offset or a bus_dma_limit below the mask, an
  address that passes this check is still not addressable, and the real
  verdict (bounce or DMA_MAPPING_ERROR) only comes later from
  dma_map_page_attrs().

Also, every octeontx2 device sets DMA_BIT_MASK(48) (rvu_probe(), cgx
probe, otx2_pf/otx2_vf/rep probe), so on shipping hardware the check
always passes and the GFP_DMA32 retry body is unreachable.  Could the
retry simply be dropped and the decision left to dma_map_page_attrs()?

> +
> +static inline void *otx2_dma_alloc_coherent(struct device *dev, size_t size,
> +					    dma_addr_t *dma_handle, gfp_t gfp)
> +{
> +	dma_addr_t dma_addr;
> +	unsigned int order;
> +	gfp_t alloc_gfp;
> +	void *vaddr;
> +
> +	if (!dev || !dma_handle || !size)
> +		return NULL;
> +
> +	if (!dev_is_dma_coherent(dev))
> +		return NULL;
> +
> +	size = PAGE_ALIGN(size);
> +	order = get_order(size);
> +	if (order > MAX_PAGE_ORDER)
> +		return NULL;

[Severity: High]
Does this cap reject qmem allocations that the previous
dma_alloc_attrs(DMA_ATTR_FORCE_CONTIGUOUS) path served from CMA?  With
4 KiB pages and the arm64 default ARCH_FORCE_MAX_ORDER, the ceiling is
4 MiB, while dma_alloc_contiguous() had no such limit:

  kernel/dma/contiguous.c:dma_alloc_contiguous() {
	if (dev->cma_area)
		return cma_alloc_aligned(dev->cma_area, size, gfp);
	...
	return cma_alloc_aligned(dma_contiguous_default_area, size, gfp);
  }

Call sites that appear to cross 4 MiB:

  af/cn20k/mbox_init.c:rvu_alloc_mbox_memory()
	err = qmem_alloc(rvu->dev, &mbox_addr, ndevs, mbox_size);

  reached from cn20k_rvu_mbox_init(TYPE_AFPF, rvu->hw->total_pfs) with
  mbox_size = MBOX_SIZE = SZ_64K, so total_pfs >= 64 gives order 11 and
  AF probe fails.  The CN20K mailbox IRQ code already handles PF indices
  up to 96.

  nic/otx2_pf.c:cn20k_pfvf_mbox_alloc()
	err = qmem_alloc(&pf->pdev->dev, &mbox_addr, numvfs, MBOX_SIZE);

  fails once numvfs >= 64, i.e. on enabling SR-IOV.

  nic/otx2_common.c:otx2_sq_init()
	err = qmem_alloc(pfvf->dev, &sq->tso_hdrs, qset->sqe_cnt,
			 TSO_HEADER_SIZE);

  nic/otx2_common.c:otx2_cq_init()
	err = qmem_alloc(pfvf->dev, &cq->cqe, cq->cqe_cnt, cq->cqe_size);

  and otx2_set_ringparam() still accepts up to a million entries:

	tx_count = clamp_t(u32, ring->tx_pending,
			   Q_COUNT(Q_SIZE_4K), Q_COUNT(Q_SIZE_MAX));

  so the TSO header area (sqe_cnt * 128), the SQE ring and the CQE ring
  (cqe_cnt * 128 or 512) pass 4 MiB somewhere around 16384 to 32768
  entries and now return -ENOMEM at interface bring-up.

Should these call sites be audited or capped before the allocator gains
a hard limit?

On the same subject, the changelog says:

  Allocations requiring more than MAX_PAGE_ORDER pages are still
  rejected, since the buddy allocator cannot serve them without CMA.

For CMA-enabled configurations that is a new restriction rather than
existing behaviour, and MAX_PAGE_ORDER is an order, not a page count.
Could that paragraph be reworded to describe the change?

> +
> +	alloc_gfp = (gfp & ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM)) |
> +		__GFP_ZERO | __GFP_COMP | __GFP_RECLAIM;
> +
> +	vaddr = (void *)__get_free_pages(alloc_gfp, order);
> +	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 (!vaddr)
> +		return NULL;
> +
> +	dma_addr = dma_map_page_attrs(dev, virt_to_page(vaddr), 0, size,
> +				      DMA_BIDIRECTIONAL, 0);

[Severity: Medium]
The helper accepts a gfp argument but then ORs in __GFP_RECLAIM
unconditionally, so whatever the caller asked for, the allocation ends
up able to block.  __GFP_RECLAIM is
__GFP_DIRECT_RECLAIM | __GFP_KSWAPD_RECLAIM, so a caller passing
GFP_ATOMIC or GFP_NOWAIT to otx2_dma_alloc_coherent() has
__GFP_DIRECT_RECLAIM added behind its back and __get_free_pages() may
sleep.  From an atomic or spinlock-held context that is a
scheduling-while-atomic BUG rather than the allocation failure the
caller was prepared for.

Note this is different in kind from the __GFP_ZERO and __GFP_COMP bits
added in the same expression: those describe how the pages are
formatted, while __GFP_RECLAIM changes whether the call can sleep,
which is the caller's decision and not the allocator wrapper's.
Stripping __GFP_DMA, __GFP_DMA32 and __GFP_HIGHMEM is likewise fine,
since the helper manages the zone itself.

There is no bug in this tree today, because the only caller is
qmem_alloc() and it passes GFP_KERNEL, which already contains
__GFP_RECLAIM.  But the helper is written as a general
dma_alloc_coherent() replacement, is placed in a header shared by af/
and nic/, and its name invites reuse, so the override is a trap for the
next caller.

Could the OR of __GFP_RECLAIM simply be dropped and the caller's flags
honoured, or, if the helper is only ever meant to serve the sleeping
qmem_alloc() path, would it be clearer to drop the gfp parameter
altogether and hardcode GFP_KERNEL inside the helper?

[Severity: High]
Can this mapping end up on a SWIOTLB bounce buffer, so that the device
and the CPU stop looking at the same memory?

qmem regions are long-lived structures that hardware and the driver
touch concurrently for the lifetime of the mapping: NIX/NPA contexts, AF
admin queue instruction and result rings, CQE rings, the NPA fc_addr
writeback area, CN10K LMT lines, and the mailbox regions.  The mailbox
case is documented in the driver itself:

  af/cn20k/mbox_init.c:rvu_alloc_mbox_memory() {
	/* Allocate contiguous memory for mailbox communication.
	 * ...
	 * AF will access mbox memory using direct physical addresses
	 * and PFs will access the same shared memory from BAR2.
  }

With attrs = 0, dma_map_page_attrs() is allowed to substitute a bounce
buffer:

  kernel/dma/direct.c:dma_direct_map_phys() {
	if (is_swiotlb_force_bounce(dev)) {
		if (attrs & (DMA_ATTR_MMIO | DMA_ATTR_REQUIRE_COHERENT))
			return DMA_MAPPING_ERROR;

		return swiotlb_map(dev, phys, size, dir, attrs);
	}
  }

  drivers/iommu/dma-iommu.c:dev_use_swiotlb() {
	return IS_ENABLED(CONFIG_SWIOTLB) &&
		(dev_is_untrusted(dev) ||
		 dma_kmalloc_needs_bounce(dev, size, dir));
  }

so swiotlb=force, restricted-DMA pools, confidential-computing guests,
or a device marked untrusted behind the SMMU all bounce.  Regions at or
below swiotlb_max_mapping_size() (fc_addr, AQ rings, qints contexts,
small mailbox regions) would bounce successfully and silently.  In that
case the AF programs NIX_AF_LFX_*_BASE, NPA_AF_LFX_LOC_AURAS_BASE or
RVU_MBOX_AF_PFX_ADDR with the bounce address while the driver keeps
using qmem->base, and there are no dma_sync_*() calls anywhere to
reconcile the two.

The unmap in otx2_dma_free_coherent() then copies the device-side buffer
back over the page the CPU has been writing:

	dma_unmap_page_attrs(dev, dma_handle, size, DMA_BIDIRECTIONAL, 0);
	free_pages((unsigned long)vaddr, order);

dev_is_dma_coherent() only reports cache coherency; it does not promise
that the mapping aliases the same physical memory, and the physical-range
check above cannot rule bouncing out either since force-bounce ignores
the address.  Would DMA_ATTR_REQUIRE_COHERENT be the attribute wanted
here, given its documented contract:

  /*
   * DMA_ATTR_REQUIRE_COHERENT: Indicates that DMA coherency is required.
   * All mappings that carry this attribute can't work with SWIOTLB and cache
   * flushing.
   */
  #define DMA_ATTR_REQUIRE_COHERENT	(1UL << 12)

[Severity: Medium]
Separately, should the size passed here be clamped by
dma_max_mapping_size(dev)?  Documentation/core-api/dma-api.rst says of
that helper:

  Returns the maximum size of a mapping for the device. The size parameter
  of the mapping functions like dma_map_single(), dma_map_page() and
  others should not be larger than the returned value.

The only bound applied is the buddy order, so up to 4 MiB can be handed
to a single dma_map_page_attrs() call, while

  kernel/dma/direct.c:dma_direct_max_mapping_size() {
	if (is_swiotlb_active(dev) &&
	    (dma_addressing_limited(dev) || is_swiotlb_force_bounce(dev) ||
	     force_dma_unencrypted(dev)))
		return swiotlb_max_mapping_size(dev);
  }

can be roughly 256 KiB.  The failure then surfaces as a generic
-ENOMEM from qmem_alloc() with no hint about the size limit.  The old
dma_alloc_attrs() path had no per-mapping size limit.

> +	if (dma_mapping_error(dev, dma_addr)) {
> +		free_pages((unsigned long)vaddr, order);
> +		return NULL;
> +	}
> +
> +	*dma_handle = dma_addr;
> +	return vaddr;
> +}
> +

[ ... ]

>  static inline int qmem_alloc(struct device *dev, struct qmem **q,
>  			     int qsize, int entry_sz)
>  {
> @@ -60,8 +133,8 @@ static inline int qmem_alloc(struct device *dev, struct qmem **q,
>  
>  	qmem->entry_sz = entry_sz;
>  	qmem->alloc_sz = (qsize * entry_sz) + OTX2_ALIGN;
> -	qmem->base = dma_alloc_attrs(dev, qmem->alloc_sz, &qmem->iova,
> -				     GFP_KERNEL, DMA_ATTR_FORCE_CONTIGUOUS);
> +	qmem->base = otx2_dma_alloc_coherent(dev, qmem->alloc_sz, &qmem->iova,
> +					     GFP_KERNEL);
>  	if (!qmem->base)
>  		return -ENOMEM;

[Severity: Medium]
This isn't a bug introduced by this patch, but since the size
computation is now the only thing bounding the new allocator, can
alloc_sz wrap here?  The product is signed int arithmetic over values
that come straight from mailbox requests:

  af/rvu_npa.c:rvu_mbox_handler_npa_lf_alloc()
	err = qmem_alloc(rvu->dev, &pfvf->pool_ctx, req->nr_pools, hwctx_size);

req->nr_pools is u32 and only checked for being non-zero, so
nr_pools = 2^25 with a 128-byte pool context wraps the product to zero
and leaves alloc_sz = 128.  A single page is then allocated while
pool_ctx->qsize keeps the huge requested count used by later bounds
checks and hardware programming.

For the NIX RQ/SQ/CQ paths the same wrap is blocked before any register
write, because the count-proportional allocation fails first:

  af/rvu_nix.c:rvu_mbox_handler_nix_lf_alloc() {
	pfvf->rq_bmap = kcalloc(req->rq_cnt, sizeof(long), GFP_KERNEL);
	if (!pfvf->rq_bmap) {
		rc = -ENOMEM;
		goto free_mem;
	}

	rvu_write64(rvu, blkaddr, NIX_AF_LFX_RQS_BASE(nixlf),
		    (u64)pfvf->rq_ctx->iova);
  }

The NPA pool_ctx path has no such companion allocation, and device
accesses past the mapping land on untranslated IOVA that the SMMU
faults, so this is an inconsistent qmem object rather than memory
corruption.  Would it make sense to add a size sanity check on
qsize * entry_sz while this code is being touched?

> @@ -80,10 +153,9 @@ static inline void qmem_free(struct device *dev, struct qmem *qmem)
>  		return;
>  
>  	if (qmem->base)
> -		dma_free_attrs(dev, qmem->alloc_sz,
> -			       qmem->base - qmem->align,
> -			       qmem->iova - qmem->align,
> -			       DMA_ATTR_FORCE_CONTIGUOUS);
> +		otx2_dma_free_coherent(dev, qmem->alloc_sz,
> +				       qmem->base - qmem->align,
> +				       qmem->iova - qmem->align);
>  	devm_kfree(dev, qmem);
>  }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260902021023.2987908-1-rkannoth%40marvell.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-05 20:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-02  2:10 [PATCH v6 net] octeontx2-af: switch qmem from coherent DMA alloc to streaming DMA mapping Ratheesh Kannoth
2026-09-03  2:45 ` Ratheesh Kannoth
2026-09-05 20:02 ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®