[PATCH iwl] ice: Restore Ordered MMIO Writes for Tx Doorbells

Bryan Fraschetti posted 1 patch 1 month, 4 weeks ago
drivers/net/ethernet/intel/ice/ice_txrx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH iwl] ice: Restore Ordered MMIO Writes for Tx Doorbells
Posted by Bryan Fraschetti 1 month, 4 weeks ago
The DQL accounting state which tracks the number of bytes queued for the
NIC to transmit, namely dql->num_queued, is updated by the ICE driver
when it invokes __netdev_tx_sent_queue(). Subsequently the driver
updates the hardware queue tail allowing the NIC to begin processing the
work queue. The queue accounting update should be ordered before the NIC
begins transmitting descriptors.

The transmit path currently updates the hardware doorbell using
writel_relaxed(), which (on arm64) does not provide the same ordering
guarantees between writes to normal memory and writes to MMIO registers
that writel() does. This introduces a potential race where the NIC
begins transmitting descriptors before the dql->num_queued update is
globally visible. If the NIC finishes before the update is observed by
dql_completed(), it detects an invalid state where more bytes have been
completed than have been queued. When this happens the following BUG_ON
is triggered.

  BUG_ON(count > num_queued - dql->num_completed);

This has been observed and manifests as the following crash (note that
the trace has been trimmed) in an environment with sustained network
load that uses an Intel Corporation Ethernet Controller E810-XXV for SFP
(rev 02) on an arm64 machine. Replacing writel_relaxed() with writel()
in a test kernel eliminated the crash in the user's workload, which
previously reproduced the issue reliably.

  kernel BUG at lib/dynamic_queue_limits.c:99

Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
pc : dql_completed+0x268/0x2a0
lr : ice_clean_tx_irq+0x1d4/0x620 [ice]
Call trace:
dql_completed+0x268/0x2a0 (P)
ice_napi_poll+0x94/0x520 [ice]
__napi_poll+0x48/0x3f0
net_rx_action+0x194/0x420

This restores the behaviour prior to commit ccde82e90946 ("ice: add E830
Earliest TxTime First Offload support"), which changed the notification
mechanism from writel() to writel_relaxed().

Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2161572
Fixes: ccde82e90946 ("ice: add E830 Earliest TxTime First Offload support")
Signed-off-by: Bryan Fraschetti <bryan.fraschetti@canonical.com>
---
 drivers/net/ethernet/intel/ice/ice_txrx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
index c04c5856dad6..4e26ace793ec 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.c
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
@@ -1561,10 +1561,10 @@ ice_tx_map(struct ice_tx_ring *tx_ring, struct ice_tx_buf *first,
 			}
 		}
 		tstamp_ring->next_to_use = j;
-		writel_relaxed(j, tstamp_ring->tail);
+		writel(j, tstamp_ring->tail);
 	} else {
 ring_kick:
-		writel_relaxed(i, tx_ring->tail);
+		writel(i, tx_ring->tail);
 	}
 	return;
 
-- 
2.53.0
RE: [Intel-wired-lan] [PATCH iwl] ice: Restore Ordered MMIO Writes for Tx Doorbells
Posted by Nowlin, Alexander 2 weeks ago
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Bryan Fraschetti via Intel-wired-lan
> Sent: Monday, August 3, 2026 8:29 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Alice Michael <alice.michael@intel.com>; Greenwalt, Paul <paul.greenwalt@intel.com>; Fijalkowski, Maciej <maciej.fijalkowski@intel.com>; Loktionov, Aleksandr <aleksandr.loktionov@intel.com>
> Cc: Bryan Fraschetti <bryan.fraschetti@canonical.com>; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH iwl] ice: Restore Ordered MMIO Writes for Tx Doorbells
> 
> The DQL accounting state which tracks the number of bytes queued for the NIC to transmit, namely dql->num_queued, is updated by the ICE driver when it invokes __netdev_tx_sent_queue(). Subsequently the > driver updates the hardware queue tail allowing the NIC to begin processing the work queue. The queue accounting update should be ordered before the NIC begins transmitting descriptors.
> 
> The transmit path currently updates the hardware doorbell using writel_relaxed(), which (on arm64) does not provide the same ordering guarantees between writes to normal memory and writes to MMIO registers that writel() does. This introduces a potential race where the NIC begins transmitting descriptors before the dql->num_queued update is globally visible. If the NIC finishes before the update is observed > by dql_completed(), it detects an invalid state where more bytes have been completed than have been queued. When this happens the following BUG_ON is triggered.
> 
>   BUG_ON(count > num_queued - dql->num_completed);
> 
> This has been observed and manifests as the following crash (note that the trace has been trimmed) in an environment with sustained network load that uses an Intel Corporation Ethernet Controller E810-XXV > for SFP (rev 02) on an arm64 machine. Replacing writel_relaxed() with writel() in a test kernel eliminated the crash in the user's workload, which previously reproduced the issue reliably.
> 
>   kernel BUG at lib/dynamic_queue_limits.c:99
> 
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP pc : dql_completed+0x268/0x2a0 lr : ice_clean_tx_irq+0x1d4/0x620 [ice] Call trace:
> dql_completed+0x268/0x2a0 (P)
> ice_napi_poll+0x94/0x520 [ice]
> __napi_poll+0x48/0x3f0
> net_rx_action+0x194/0x420
> 
> This restores the behaviour prior to commit ccde82e90946 ("ice: add E830 Earliest TxTime First Offload support"), which changed the notification mechanism from writel() to writel_relaxed().
> 
> Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2161572
> Fixes: ccde82e90946 ("ice: add E830 Earliest TxTime First Offload support")
> Signed-off-by: Bryan Fraschetti <bryan.fraschetti@canonical.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_txrx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Tested-by: Alexander Nowlin <alexander.nowlin@intel.com>