Re: [PATCH net-next v16 2/5] net: rnpgbe: Add basic TX packet transmission support

From
Yibo Dong <dong100@mucse.com>
Date
2026-10-14 05:44:20
Also in
lkml
Reply to
netdev-bot+sashiko@kernel.org, 2026-10-14
On Wed, Oct 14, 2026 at 02:59:13AM +0000, netdev-bot+sashiko@kernel.org wrote:
[ ... ]
quoted
@@ -556,12 +680,173 @@ static void rnpgbe_napi_disable_all(struct mucse *mucse)
 		napi_disable(&mucse->q_vector[i]->napi);
 }
 
[ ... ]
quoted
+static int rnpgbe_wait_tx_dma_idle(struct mucse *mucse)
+{
+	struct mucse_hw *hw = &mucse->hw;
+	u32 dma_status;
+	int err;
+
+	/* A timeout indicates a terminal AXI fault. Hardware stops all PCIe
+	 * DMA requests in this state, including requests accepted before the
+	 * fault. Teardown may therefore release mappings and descriptor memory.
+	 * Recovery requires a chip-level reset.
+	 */
+	err = readl_poll_timeout(hw->hw_addr + RNPGBE_DMA_STATUS,
+				 dma_status,
+				 (dma_status & RNPGBE_DMA_TX_STATUS) ==
+				 RNPGBE_DMA_TX_STATUS,
+				 10, 100000);
+	if (err) {
+		set_bit(__MUCSE_AXI_FAULT, &mucse->state);
+		dev_err(&mucse->pdev->dev,
+			"TX DMA failed to quiesce, status %#x\n", dma_status);
+	}
+
+	return err;
+}
[ ... ]
quoted
+	if (mucse->num_tx_queues &&
+	    !test_bit(__MUCSE_AXI_FAULT, &mucse->state))
+		err = rnpgbe_wait_tx_dma_idle(mucse);
+
+	dma_axi_ctl = mucse_hw_rd32(hw, RNPGBE_DMA_AXI_EN);
+	dma_axi_ctl &= ~TX_AXI_RW_EN;
+	mucse_hw_wr32(hw, RNPGBE_DMA_AXI_EN, dma_axi_ctl);
+	/* Flush the posted write before continuing. */
+	(void)mucse_hw_rd32(hw, RNPGBE_DMA_AXI_EN);
[ ... ]
quoted
+void rnpgbe_clean_all_tx_rings(struct mucse *mucse)
+{
+	/* rnpgbe_stop_all_tx_rings() logs and latches any AXI fault. Hardware
+	 * stops DMA requests in this state, so cleanup intentionally ignores
+	 * an error and releases the mappings and descriptor memory.
+	 */
+	rnpgbe_stop_all_tx_rings(mucse);
+
+	for (int i = 0; i < mucse->num_tx_queues; i++)
+		rnpgbe_clean_tx_ring(mucse->tx_ring[i]);
+}
[Severity: Medium]
After rnpgbe_wait_tx_dma_idle() times out, what guarantees that the
device has actually stopped DMA?

On a timeout the driver only sets __MUCSE_AXI_FAULT and clears the
internal TX_AXI_RW_EN gate in rnpgbe_stop_all_tx_rings().
rnpgbe_clean_all_tx_rings() then ignores the error. It unmaps the TX
buffers, frees the skbs and memsets the descriptor ring.

After that, rnpgbe_close() or the err_free_tx path in rnpgbe_open()
continues with:

rnpgbe_free_all_tx_resources()
  rnpgbe_free_tx_resources()
    dma_free_coherent(tx_ring->dev, tx_ring->size, tx_ring->desc,
                      tx_ring->dma);

The only reason this would be safe is the hardware claim in the comment
and the commit message, that the hardware "stops all PCIe DMA requests in
this state". Nothing on the host side enforces that:

- rnpgbe_probe() calls pci_set_master(), but the driver never calls
  pci_clear_master() and never resets the function.
- TX_BASE_ADDR_HI/LO keep pointing at the freed ring. Once the bit is
  set, rnpgbe_open() returns -EIO, so nothing reprograms them.
- Bus Master Enable is only cleared by pci_disable_device() in
  rnpgbe_remove() or rnpgbe_dev_shutdown(), long after the memory was
  freed.

The timeout also only means the done bits weren't seen within 100ms. A
slow drain looks the same as a real fault.

If the device does write DD status back late, or reads a buffer after
the unmap, wouldn't that land in freed and possibly reused memory?

Would it be safer to call pci_clear_master() (or reset the function) on
the fault path before releasing DMA memory, or to leak the ring and
buffers in this case instead of freeing them?

The later patch "net: rnpgbe: Add basic RX data path support" in this
series uses the same approach for RX.
This is illegitimate.
Once START=0 has taken effect, hardware does not initiate any new
PCIe memory read or write requests, including for prefetched descriptors.
The 100 ms wait only covers transactions already in flight.
For this chip, failure to reach the done state within that window
indicates a terminal AXI fault, not a recoverable slow drain. As
documented in the code comments, this fault prevents further host-memory
accesses, including from previously accepted requests, and requires a
chip-level reset. Cleanup therefore relies on this hardware guarantee.
The same to RX.
-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/3B42BE25B3696EAE%2B20261013022920.63529-1-dong100%40mucse.com
Thread (11 messages) 11 messages, 3 authors, last reply 22d ago
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help