* [PATCH net-next v2 0/3] xgmac: Support for 16 MTL/DMA queues [not found] <CGME20260601162617eucas1p2efc386a4d9f9e7e98e8acb83f9c550d6@eucas1p2.samsung.com> @ 2026-06-01 16:25 ` Jakub Raczynski 2026-06-01 16:25 ` [PATCH net-next v2 1/3] net/stmmac/dwxgmac: Modify DMA functions for future hardware Jakub Raczynski ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Jakub Raczynski @ 2026-06-01 16:25 UTC (permalink / raw) To: netdev Cc: andrew+netdev, davem, edumazet, kuba, pabeni, mcoquelin.stm32, alexandre.torgue, linux-kernel, k.domagalski, k.tegowski, cs0617.lee, Jakub Raczynski New XGMAC datasheets for XGMAC 3.20a and 3.40a list support up to 16 MTL/DMA queues. This patch series does modifications to accommodate for that, by following: - Increase MAX value macros (this will increase memory allocation for driver) - Add missing registers - Modify/fix some functions to split TC and MTL/DMA handling Biggest issue is that new hardware does support only 8 TC (traffic classes), while supporting more MTL/DMA queues. This is problematic, as current code does assume that these are equal which is not true anymore. Fix that by applying better context to functions, using TC count read from HW_FEATURE register where it applies. For DMA/MTL queues higher than TC count redirect traffic to TC 0. This is error in user configuration but quite possible to occur, if user does 'one DMA/MTL queue per TC'. This change has been tested on XGMAC based on 3.20a. --- Changes in v2: - Remove newly added num_tc value, replace with stmmac_priv pointer, to use numtc value from dma_features struct - Change all new WARN_ONCE calls to netdev_err - Add Signed-Off for colleague that did similar original patch internally and took part in new version of this patch - Apply recommended change for switch case for readability Link to v1: https://lore.kernel.org/netdev/20260511165416.3093015-1-j.raczynski@samsung.com/ Jakub Raczynski (3): net/stmmac/dwxgmac: Modify DMA functions for future hardware net/stmmac/dwxgmac: Extend MTL/DMA support to 16 queues include/stmmac: Increase max DMA/MTL channel count from 8 to 16 drivers/net/ethernet/stmicro/stmmac/common.h | 1 + .../net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 + .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 65 ++++++++++++++++++- .../ethernet/stmicro/stmmac/dwxgmac2_dma.c | 15 ++++- drivers/net/ethernet/stmicro/stmmac/hwif.c | 3 + .../net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- include/linux/stmmac.h | 6 +- 7 files changed, 85 insertions(+), 9 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next v2 1/3] net/stmmac/dwxgmac: Modify DMA functions for future hardware 2026-06-01 16:25 ` [PATCH net-next v2 0/3] xgmac: Support for 16 MTL/DMA queues Jakub Raczynski @ 2026-06-01 16:25 ` Jakub Raczynski 2026-06-03 11:46 ` Jakub Raczynski 2026-06-01 16:25 ` [PATCH net-next v2 2/3] net/stmmac/dwxgmac: Extend MTL/DMA support to 16 queues Jakub Raczynski 2026-06-01 16:25 ` [PATCH net-next v2 3/3] include/stmmac: Increase max DMA/MTL channel count from 8 to 16 Jakub Raczynski 2 siblings, 1 reply; 9+ messages in thread From: Jakub Raczynski @ 2026-06-01 16:25 UTC (permalink / raw) To: netdev Cc: andrew+netdev, davem, edumazet, kuba, pabeni, mcoquelin.stm32, alexandre.torgue, linux-kernel, k.domagalski, k.tegowski, cs0617.lee, Jakub Raczynski Datasheet for newer hardware of XGMAC (3.20a and 3.40a) list support for up to 16 DMA/MTL queues. Currently maximum amount of queues in 8 set by STMMAC_CH_MAX, MTL_MAX_TX_QUEUES, MTL_MAX_RX_QUEUES macros. But before we set these to higher value there are changes to be made. While newer hardware supports up to 16 MTL/DMA queues, there is support only for 8 TC's (traffic control) entries. Current source assumes these are equal, which might not be true. While in some cases it would be just incorrect value, there are possible wrong memory accesses. Fix this by saving number of TC supported by hardware in mac_device_info and verify it in related functions. Also use TC count rather than MTL_MAX values where it applies. Co-developed-by: Chang-Sub Lee <cs0617.lee@samsung.com> Signed-off-by: Chang-Sub Lee <cs0617.lee@samsung.com> Signed-off-by: Jakub Raczynski <j.raczynski@samsung.com> --- drivers/net/ethernet/stmicro/stmmac/common.h | 1 + .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 46 ++++++++++++++++++- .../ethernet/stmicro/stmmac/dwxgmac2_dma.c | 15 +++++- drivers/net/ethernet/stmicro/stmmac/hwif.c | 3 ++ .../net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- 5 files changed, 63 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h index 927ea6230073..359a76d3658c 100644 --- a/drivers/net/ethernet/stmicro/stmmac/common.h +++ b/drivers/net/ethernet/stmicro/stmmac/common.h @@ -620,6 +620,7 @@ struct mac_device_info { const struct stmmac_mmc_ops *mmc; const struct stmmac_est_ops *est; const struct stmmac_vlan_ops *vlan; + struct stmmac_priv *priv_data; struct dw_xpcs *xpcs; struct phylink_pcs *phylink_pcs; struct mii_regs mii; /* MII register Addresses */ diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c index f02b434bbd50..dbedf31bb2ab 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c @@ -97,11 +97,19 @@ static void dwxgmac2_rx_queue_enable(struct mac_device_info *hw, u8 mode, static void dwxgmac2_rx_queue_prio(struct mac_device_info *hw, u32 prio, u32 queue) { + const unsigned int numtc = hw->priv_data->dma_cap.numtc; + struct net_device *ndev = hw->priv_data->dev; void __iomem *ioaddr = hw->pcsr; u32 clear_mask = 0; u32 ctrl2, ctrl3; int i; + if (queue >= numtc) { + netdev_err(ndev, "%s: invalid TC queue %d, supported %d\n", + __func__, queue, numtc); + return; + } + ctrl2 = readl(ioaddr + XGMAC_RXQ_CTRL2); ctrl3 = readl(ioaddr + XGMAC_RXQ_CTRL3); @@ -138,9 +146,17 @@ static void dwxgmac2_rx_queue_prio(struct mac_device_info *hw, u32 prio, static void dwxgmac2_tx_queue_prio(struct mac_device_info *hw, u32 prio, u32 queue) { + const unsigned int numtc = hw->priv_data->dma_cap.numtc; + struct net_device *ndev = hw->priv_data->dev; void __iomem *ioaddr = hw->pcsr; u32 value, reg; + if (queue >= numtc) { + netdev_err(ndev, "%s: invalid TC queue %d, supported %d\n", + __func__, queue, numtc); + return; + } + reg = (queue < 4) ? XGMAC_TC_PRTY_MAP0 : XGMAC_TC_PRTY_MAP1; if (queue >= 4) queue -= 4; @@ -207,6 +223,7 @@ static void dwxgmac2_prog_mtl_rx_algorithms(struct mac_device_info *hw, static void dwxgmac2_prog_mtl_tx_algorithms(struct mac_device_info *hw, u32 tx_alg) { + const unsigned int numtc = hw->priv_data->dma_cap.numtc; void __iomem *ioaddr = hw->pcsr; bool ets = true; u32 value; @@ -233,7 +250,7 @@ static void dwxgmac2_prog_mtl_tx_algorithms(struct mac_device_info *hw, writel(value, ioaddr + XGMAC_MTL_OPMODE); /* Set ETS if desired */ - for (i = 0; i < MTL_MAX_TX_QUEUES; i++) { + for (i = 0; i < numtc; i++) { value = readl(ioaddr + XGMAC_MTL_TCx_ETS_CONTROL(i)); value &= ~XGMAC_TSA; if (ets) @@ -246,8 +263,15 @@ static void dwxgmac2_set_mtl_tx_queue_weight(struct stmmac_priv *priv, struct mac_device_info *hw, u32 weight, u32 queue) { + const unsigned int numtc = priv->dma_cap.numtc; void __iomem *ioaddr = hw->pcsr; + if (queue >= numtc) { + netdev_err(priv->dev, "%s: invalid TC queue %d, supported %d\n", + __func__, queue, numtc); + return; + } + writel(weight, ioaddr + XGMAC_MTL_TCx_QUANTUM_WEIGHT(queue)); } @@ -273,9 +297,16 @@ static void dwxgmac2_config_cbs(struct stmmac_priv *priv, u32 send_slope, u32 idle_slope, u32 high_credit, u32 low_credit, u32 queue) { + const unsigned int numtc = priv->dma_cap.numtc; void __iomem *ioaddr = hw->pcsr; u32 value; + if (queue >= numtc) { + netdev_err(priv->dev, "%s: invalid TC queue %d, supported %d\n", + __func__, queue, numtc); + return; + } + writel(send_slope, ioaddr + XGMAC_MTL_TCx_SENDSLOPE(queue)); writel(idle_slope, ioaddr + XGMAC_MTL_TCx_QUANTUM_WEIGHT(queue)); writel(high_credit, ioaddr + XGMAC_MTL_TCx_HICREDIT(queue)); @@ -357,6 +388,8 @@ static void dwxgmac2_flow_ctrl(struct mac_device_info *hw, unsigned int duplex, unsigned int fc, unsigned int pause_time, u8 tx_cnt) { + const unsigned int numtc = hw->priv_data->dma_cap.numtc; + struct net_device *ndev = hw->priv_data->dev; void __iomem *ioaddr = hw->pcsr; u8 i; @@ -366,6 +399,17 @@ static void dwxgmac2_flow_ctrl(struct mac_device_info *hw, unsigned int duplex, for (i = 0; i < tx_cnt; i++) { u32 value = XGMAC_TFE; + if (i >= numtc) { + netdev_err(ndev, + "%s: invalid TC queue %d, supported %d\n", + __func__, i, numtc); + /* This will skip all other wrong channels, + * but triggering this is preceded by warnings + * from other functions, so limit the spam. + */ + break; + } + if (duplex) value |= FIELD_PREP(XGMAC_PT, pause_time); diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c index 03437f1cf3df..f70eaa181dad 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c @@ -204,8 +204,19 @@ static void dwxgmac2_dma_tx_mode(struct stmmac_priv *priv, void __iomem *ioaddr, value = u32_replace_bits(value, ttc, XGMAC_TTC); } - /* Use static TC to Queue mapping */ - value |= FIELD_PREP(XGMAC_Q2TCMAP, channel); + /* Newer XGMAC hardware does support up to 16 MTL/DMA queues but + * only 8 traffic class queues. Redirect these, but this is error in + * configuration. + */ + if (channel >= priv->dma_cap.numtc) { + netdev_err(priv->dev, + "%s: Wrong channel set for TX mode redirecting to TC 0\n", + __func__); + value |= FIELD_PREP(XGMAC_Q2TCMAP, 0); + } else { + /* Use static TC to Queue mapping */ + value |= FIELD_PREP(XGMAC_Q2TCMAP, channel); + } if (qmode != MTL_QUEUE_AVB) txqen = 0x2; diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.c b/drivers/net/ethernet/stmicro/stmmac/hwif.c index 511b0fd5e834..52842366ae0c 100644 --- a/drivers/net/ethernet/stmicro/stmmac/hwif.c +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.c @@ -368,6 +368,9 @@ int stmmac_hwif_init(struct stmmac_priv *priv) return -EINVAL; } + /* Assign priv struct internally for easier processing */ + mac->priv_data = priv; + /* Only use generic HW helpers if needed */ mac->desc = mac->desc ? : entry->desc; mac->dma = mac->dma ? : entry->dma; diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 3591755ea30b..5917bf47c7de 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -4140,7 +4140,7 @@ static int __stmmac_open(struct net_device *dev, u8 chan; int ret; - for (int i = 0; i < MTL_MAX_TX_QUEUES; i++) + for (int i = 0; i < priv->plat->tx_queues_to_use; i++) if (priv->dma_conf.tx_queue[i].tbs & STMMAC_TBS_EN) dma_conf->tx_queue[i].tbs = priv->dma_conf.tx_queue[i].tbs; memcpy(&priv->dma_conf, dma_conf, sizeof(*dma_conf)); -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net-next v2 1/3] net/stmmac/dwxgmac: Modify DMA functions for future hardware 2026-06-01 16:25 ` [PATCH net-next v2 1/3] net/stmmac/dwxgmac: Modify DMA functions for future hardware Jakub Raczynski @ 2026-06-03 11:46 ` Jakub Raczynski 2026-06-03 12:45 ` Larysa Zaremba 0 siblings, 1 reply; 9+ messages in thread From: Jakub Raczynski @ 2026-06-03 11:46 UTC (permalink / raw) To: netdev Cc: andrew+netdev, davem, edumazet, kuba, pabeni, mcoquelin.stm32, alexandre.torgue, linux-kernel, k.domagalski, k.tegowski, cs0617.lee [-- Attachment #1: Type: text/plain, Size: 4735 bytes --] Gonna respond to Sashiko comments. > > --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > > @@ -97,11 +97,19 @@ static void dwxgmac2_rx_queue_enable(struct mac_device_info *hw, u8 mode, > > static void dwxgmac2_rx_queue_prio(struct mac_device_info *hw, u32 prio, > > u32 queue) > > { > > + const unsigned int numtc = hw->priv_data->dma_cap.numtc; > > + struct net_device *ndev = hw->priv_data->dev; > > void __iomem *ioaddr = hw->pcsr; > > u32 clear_mask = 0; > > u32 ctrl2, ctrl3; > > int i; > > > > + if (queue >= numtc) { > > + netdev_err(ndev, "%s: invalid TC queue %d, supported %d\n", > > + __func__, queue, numtc); > > + return; > > + } > Does this artificially restrict the configuration of valid RX queue priorities? > It looks like the queue index is being compared against the number of > traffic classes (numtc), which could prevent mapping priorities on valid > queues greater than the number of traffic classes. > [ ... ] No. These registers just don't exist to support more. Synopsys seems to have done this this way because there there is no free space after XGMAC_RXQ_CTRL3. So if this becomes a thing, we will have big new driver dwxgmac3. > > @@ -246,8 +263,15 @@ static void dwxgmac2_set_mtl_tx_queue_weight(struct stmmac_priv *priv, > > struct mac_device_info *hw, > > u32 weight, u32 queue) > > { > > + const unsigned int numtc = priv->dma_cap.numtc; > > void __iomem *ioaddr = hw->pcsr; > > > > + if (queue >= numtc) { > > + netdev_err(priv->dev, "%s: invalid TC queue %d, supported %d\n", > > + __func__, queue, numtc); > > + return; > > + } > > + > Because dwxgmac2_set_mtl_tx_queue_weight() returns void and is invoked via > the stmmac_do_void_callback() macro (which unconditionally returns 0), will > this early return cause ETS qdisc queue weighting on higher queue indices > to silently fail? > It appears userspace will receive a success code while the hardware is left > unconfigured. > [ ... ] As talked in previous mailing, answer is yes, there will be error in dmesg, but no fail, for very simple reason that we would need to rewrite A LOT to add proper error handling. I just hope that if user misconfigured this stuff they will read this error and fix it rather than complain. > > @@ -357,6 +388,8 @@ static void dwxgmac2_flow_ctrl(struct mac_device_info *hw, unsigned int duplex, > > unsigned int fc, unsigned int pause_time, > > u8 tx_cnt) > > { > > + const unsigned int numtc = hw->priv_data->dma_cap.numtc; > > + struct net_device *ndev = hw->priv_data->dev; > > void __iomem *ioaddr = hw->pcsr; > > u8 i; > > > > @@ -366,6 +399,17 @@ static void dwxgmac2_flow_ctrl(struct mac_device_info *hw, unsigned int duplex, > > for (i = 0; i < tx_cnt; i++) { > > u32 value = XGMAC_TFE; > > > > + if (i >= numtc) { > > + netdev_err(ndev, > > + "%s: invalid TC queue %d, supported %d\n", > > + __func__, i, numtc); > > + /* This will skip all other wrong channels, > > + * but triggering this is preceded by warnings > > + * from other functions, so limit the spam. > > + */ > > + break; > > + } > > + > If the hardware supports up to 16 transmit queues but only 8 traffic classes, > does breaking this loop at numtc skip TX flow control initialization for > valid higher queues? > The XGMAC_Qx_TX_FLOW_CTRL registers appear to be per-queue rather than > per-traffic-class. > [ ... ] No, queues are in order. If dynamic queues get implemented then maybe? Depends how this will get implemenented. > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > index 3591755ea30be..5917bf47c7de6 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > @@ -4140,7 +4140,7 @@ static int __stmmac_open(struct net_device *dev, > > u8 chan; > > int ret; > > > > - for (int i = 0; i < MTL_MAX_TX_QUEUES; i++) > > + for (int i = 0; i < priv->plat->tx_queues_to_use; i++) > > if (priv->dma_conf.tx_queue[i].tbs & STMMAC_TBS_EN) > > dma_conf->tx_queue[i].tbs = priv->dma_conf.tx_queue[i].tbs; > The tx_queue array inside struct stmmac_dma_conf is statically sized to > MTL_MAX_TX_QUEUES (8). If priv->plat->tx_queues_to_use is dynamically set > up to 16, does this cause an out-of-bounds access that corrupts adjacent > memory? > I see this is resolved later in the series by commit 9cc40baf0739 which > increases the macro to 16. Could this intermediate step in the series break > bisectability due to the buffer overflow? Nah. BR Jakub Raczynski [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next v2 1/3] net/stmmac/dwxgmac: Modify DMA functions for future hardware 2026-06-03 11:46 ` Jakub Raczynski @ 2026-06-03 12:45 ` Larysa Zaremba 2026-06-06 2:02 ` Jakub Kicinski 0 siblings, 1 reply; 9+ messages in thread From: Larysa Zaremba @ 2026-06-03 12:45 UTC (permalink / raw) To: Jakub Raczynski Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni, mcoquelin.stm32, alexandre.torgue, linux-kernel, k.domagalski, k.tegowski, cs0617.lee Overall looks fine, but some adjustments won't hurt. Reviewed-by: Larysa Zaremba <larysa.zaremba@intel.com> On Wed, Jun 03, 2026 at 01:46:53PM +0200, Jakub Raczynski wrote: > Gonna respond to Sashiko comments. > > > > --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > > > @@ -97,11 +97,19 @@ static void dwxgmac2_rx_queue_enable(struct mac_device_info *hw, u8 mode, > > > static void dwxgmac2_rx_queue_prio(struct mac_device_info *hw, u32 prio, > > > u32 queue) > > > { > > > + const unsigned int numtc = hw->priv_data->dma_cap.numtc; > > > + struct net_device *ndev = hw->priv_data->dev; > > > void __iomem *ioaddr = hw->pcsr; > > > u32 clear_mask = 0; > > > u32 ctrl2, ctrl3; > > > int i; > > > > > > + if (queue >= numtc) { > > > + netdev_err(ndev, "%s: invalid TC queue %d, supported %d\n", > > > + __func__, queue, numtc); > > > + return; > > > + } > > Does this artificially restrict the configuration of valid RX queue priorities? > > It looks like the queue index is being compared against the number of > > traffic classes (numtc), which could prevent mapping priorities on valid > > queues greater than the number of traffic classes. > > [ ... ] > > No. These registers just don't exist to support more. > Synopsys seems to have done this this way because there there is no free > space after XGMAC_RXQ_CTRL3. > So if this becomes a thing, we will have big new driver dwxgmac3. > > > > @@ -246,8 +263,15 @@ static void dwxgmac2_set_mtl_tx_queue_weight(struct stmmac_priv *priv, > > > struct mac_device_info *hw, > > > u32 weight, u32 queue) > > > { > > > + const unsigned int numtc = priv->dma_cap.numtc; > > > void __iomem *ioaddr = hw->pcsr; > > > > > > + if (queue >= numtc) { > > > + netdev_err(priv->dev, "%s: invalid TC queue %d, supported %d\n", > > > + __func__, queue, numtc); > > > + return; > > > + } > > > + > > Because dwxgmac2_set_mtl_tx_queue_weight() returns void and is invoked via > > the stmmac_do_void_callback() macro (which unconditionally returns 0), will > > this early return cause ETS qdisc queue weighting on higher queue indices > > to silently fail? > > It appears userspace will receive a success code while the hardware is left > > unconfigured. > > [ ... ] > > As talked in previous mailing, answer is yes, there will be error in dmesg, > but no fail, for very simple reason that we would need to rewrite A LOT to add > proper error handling. > I just hope that if user misconfigured this stuff they will read this error > and fix it rather than complain. > But previously those callbacks did not have any failure conditions, so I would say that adding a message-only failure does constitute a minor regression. > > > @@ -357,6 +388,8 @@ static void dwxgmac2_flow_ctrl(struct mac_device_info *hw, unsigned int duplex, > > > unsigned int fc, unsigned int pause_time, > > > u8 tx_cnt) > > > { > > > + const unsigned int numtc = hw->priv_data->dma_cap.numtc; > > > + struct net_device *ndev = hw->priv_data->dev; > > > void __iomem *ioaddr = hw->pcsr; > > > u8 i; > > > > > > @@ -366,6 +399,17 @@ static void dwxgmac2_flow_ctrl(struct mac_device_info *hw, unsigned int duplex, > > > for (i = 0; i < tx_cnt; i++) { > > > u32 value = XGMAC_TFE; > > > > > > + if (i >= numtc) { > > > + netdev_err(ndev, > > > + "%s: invalid TC queue %d, supported %d\n", > > > + __func__, i, numtc); > > > + /* This will skip all other wrong channels, > > > + * but triggering this is preceded by warnings > > > + * from other functions, so limit the spam. > > > + */ > > > + break; > > > + } > > > + > > If the hardware supports up to 16 transmit queues but only 8 traffic classes, > > does breaking this loop at numtc skip TX flow control initialization for > > valid higher queues? > > The XGMAC_Qx_TX_FLOW_CTRL registers appear to be per-queue rather than > > per-traffic-class. > > [ ... ] > > No, queues are in order. If dynamic queues get implemented then maybe? > Depends how this will get implemenented. > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > index 3591755ea30be..5917bf47c7de6 100644 > > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > @@ -4140,7 +4140,7 @@ static int __stmmac_open(struct net_device *dev, > > > u8 chan; > > > int ret; > > > > > > - for (int i = 0; i < MTL_MAX_TX_QUEUES; i++) > > > + for (int i = 0; i < priv->plat->tx_queues_to_use; i++) > > > if (priv->dma_conf.tx_queue[i].tbs & STMMAC_TBS_EN) > > > dma_conf->tx_queue[i].tbs = priv->dma_conf.tx_queue[i].tbs; > > The tx_queue array inside struct stmmac_dma_conf is statically sized to > > MTL_MAX_TX_QUEUES (8). If priv->plat->tx_queues_to_use is dynamically set > > up to 16, does this cause an out-of-bounds access that corrupts adjacent > > memory? > > I see this is resolved later in the series by commit 9cc40baf0739 which > > increases the macro to 16. Could this intermediate step in the series break > > bisectability due to the buffer overflow? > > Nah. Maybe worth adding a WARN_ON to indicate that such case can only come from a programming error? > > BR > Jakub Raczynski ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next v2 1/3] net/stmmac/dwxgmac: Modify DMA functions for future hardware 2026-06-03 12:45 ` Larysa Zaremba @ 2026-06-06 2:02 ` Jakub Kicinski 0 siblings, 0 replies; 9+ messages in thread From: Jakub Kicinski @ 2026-06-06 2:02 UTC (permalink / raw) To: Larysa Zaremba, Jakub Raczynski Cc: netdev, andrew+netdev, davem, edumazet, pabeni, mcoquelin.stm32, alexandre.torgue, linux-kernel, k.domagalski, k.tegowski, cs0617.lee On Wed, 3 Jun 2026 14:45:53 +0200 Larysa Zaremba wrote: > > > Because dwxgmac2_set_mtl_tx_queue_weight() returns void and is invoked via > > > the stmmac_do_void_callback() macro (which unconditionally returns 0), will > > > this early return cause ETS qdisc queue weighting on higher queue indices > > > to silently fail? > > > It appears userspace will receive a success code while the hardware is left > > > unconfigured. > > > > As talked in previous mailing, answer is yes, there will be error in dmesg, > > but no fail, for very simple reason that we would need to rewrite A LOT to add > > proper error handling. > > I just hope that if user misconfigured this stuff they will read this error > > and fix it rather than complain. > > But previously those callbacks did not have any failure conditions, so I would > say that adding a message-only failure does constitute a minor regression. +1, this feels a bit sloppy. The driver has some painful abstraction layers but if we cut ourselves some slack because of that that'd be basically encouraging shitty architectures. -- pw-bot: cr ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next v2 2/3] net/stmmac/dwxgmac: Extend MTL/DMA support to 16 queues 2026-06-01 16:25 ` [PATCH net-next v2 0/3] xgmac: Support for 16 MTL/DMA queues Jakub Raczynski 2026-06-01 16:25 ` [PATCH net-next v2 1/3] net/stmmac/dwxgmac: Modify DMA functions for future hardware Jakub Raczynski @ 2026-06-01 16:25 ` Jakub Raczynski 2026-06-03 14:23 ` Larysa Zaremba 2026-06-01 16:25 ` [PATCH net-next v2 3/3] include/stmmac: Increase max DMA/MTL channel count from 8 to 16 Jakub Raczynski 2 siblings, 1 reply; 9+ messages in thread From: Jakub Raczynski @ 2026-06-01 16:25 UTC (permalink / raw) To: netdev Cc: andrew+netdev, davem, edumazet, kuba, pabeni, mcoquelin.stm32, alexandre.torgue, linux-kernel, k.domagalski, k.tegowski, cs0617.lee, Jakub Raczynski New datasheets for XGMAC (3.20a and 3.40a, depending on product) support up to 16 MTL/DMA queues. Before we increase max amount through macro, prepare dwxgmac functions to handle that. Co-developed-by: Chang-Sub Lee <cs0617.lee@samsung.com> Signed-off-by: Chang-Sub Lee <cs0617.lee@samsung.com> Signed-off-by: Jakub Raczynski <j.raczynski@samsung.com> --- .../net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 ++ .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 23 +++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h index 51943705a2b0..bd333afe7e1b 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h @@ -257,6 +257,8 @@ #define XGMAC_MTL_INT_STATUS 0x00001020 #define XGMAC_MTL_RXQ_DMA_MAP0 0x00001030 #define XGMAC_MTL_RXQ_DMA_MAP1 0x00001034 +#define XGMAC_MTL_RXQ_DMA_MAP2 0x00001038 +#define XGMAC_MTL_RXQ_DMA_MAP3 0x0000103c #define XGMAC_QxMDMACH(x) GENMASK((x) * 8 + 7, (x) * 8) #define XGMAC_QxMDMACH_SHIFT(x) ((x) * 8) #define XGMAC_QDDMACH BIT(7) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c index dbedf31bb2ab..4aba62680938 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c @@ -278,12 +278,31 @@ static void dwxgmac2_set_mtl_tx_queue_weight(struct stmmac_priv *priv, static void dwxgmac2_map_mtl_to_dma(struct mac_device_info *hw, u32 queue, u32 chan) { + struct net_device *ndev = hw->priv_data->dev; void __iomem *ioaddr = hw->pcsr; u32 value, reg; - reg = (queue < 4) ? XGMAC_MTL_RXQ_DMA_MAP0 : XGMAC_MTL_RXQ_DMA_MAP1; - if (queue >= 4) + switch (queue) { + case 0 ... 3: + reg = XGMAC_MTL_RXQ_DMA_MAP0; + break; + case 4 ... 7: + reg = XGMAC_MTL_RXQ_DMA_MAP1; queue -= 4; + break; + case 8 ... 11: + reg = XGMAC_MTL_RXQ_DMA_MAP2; + queue -= 8; + break; + case 12 ... 15: + reg = XGMAC_MTL_RXQ_DMA_MAP3; + queue -= 12; + break; + default: + netdev_err(ndev, "%s: Incorrect queue mapping %d\n", + __func__, queue); + return; + } value = readl(ioaddr + reg); value &= ~XGMAC_QxMDMACH(queue); -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net-next v2 2/3] net/stmmac/dwxgmac: Extend MTL/DMA support to 16 queues 2026-06-01 16:25 ` [PATCH net-next v2 2/3] net/stmmac/dwxgmac: Extend MTL/DMA support to 16 queues Jakub Raczynski @ 2026-06-03 14:23 ` Larysa Zaremba 0 siblings, 0 replies; 9+ messages in thread From: Larysa Zaremba @ 2026-06-03 14:23 UTC (permalink / raw) To: Jakub Raczynski Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni, mcoquelin.stm32, alexandre.torgue, linux-kernel, k.domagalski, k.tegowski, cs0617.lee On Mon, Jun 01, 2026 at 06:25:36PM +0200, Jakub Raczynski wrote: > New datasheets for XGMAC (3.20a and 3.40a, depending on product) support up to > 16 MTL/DMA queues. Before we increase max amount through macro, > prepare dwxgmac functions to handle that. Reviewed-by: Larysa Zaremba <larysa.zaremba@intel.com> > > Co-developed-by: Chang-Sub Lee <cs0617.lee@samsung.com> > Signed-off-by: Chang-Sub Lee <cs0617.lee@samsung.com> > Signed-off-by: Jakub Raczynski <j.raczynski@samsung.com> > --- > .../net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 ++ > .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 23 +++++++++++++++++-- > 2 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h > index 51943705a2b0..bd333afe7e1b 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h > @@ -257,6 +257,8 @@ > #define XGMAC_MTL_INT_STATUS 0x00001020 > #define XGMAC_MTL_RXQ_DMA_MAP0 0x00001030 > #define XGMAC_MTL_RXQ_DMA_MAP1 0x00001034 > +#define XGMAC_MTL_RXQ_DMA_MAP2 0x00001038 > +#define XGMAC_MTL_RXQ_DMA_MAP3 0x0000103c > #define XGMAC_QxMDMACH(x) GENMASK((x) * 8 + 7, (x) * 8) > #define XGMAC_QxMDMACH_SHIFT(x) ((x) * 8) > #define XGMAC_QDDMACH BIT(7) > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > index dbedf31bb2ab..4aba62680938 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c > @@ -278,12 +278,31 @@ static void dwxgmac2_set_mtl_tx_queue_weight(struct stmmac_priv *priv, > static void dwxgmac2_map_mtl_to_dma(struct mac_device_info *hw, u32 queue, > u32 chan) > { > + struct net_device *ndev = hw->priv_data->dev; > void __iomem *ioaddr = hw->pcsr; > u32 value, reg; > > - reg = (queue < 4) ? XGMAC_MTL_RXQ_DMA_MAP0 : XGMAC_MTL_RXQ_DMA_MAP1; > - if (queue >= 4) > + switch (queue) { > + case 0 ... 3: > + reg = XGMAC_MTL_RXQ_DMA_MAP0; > + break; > + case 4 ... 7: > + reg = XGMAC_MTL_RXQ_DMA_MAP1; > queue -= 4; > + break; > + case 8 ... 11: > + reg = XGMAC_MTL_RXQ_DMA_MAP2; > + queue -= 8; > + break; > + case 12 ... 15: > + reg = XGMAC_MTL_RXQ_DMA_MAP3; > + queue -= 12; > + break; > + default: > + netdev_err(ndev, "%s: Incorrect queue mapping %d\n", > + __func__, queue); > + return; > + } > > value = readl(ioaddr + reg); > value &= ~XGMAC_QxMDMACH(queue); > -- > 2.34.1 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next v2 3/3] include/stmmac: Increase max DMA/MTL channel count from 8 to 16 2026-06-01 16:25 ` [PATCH net-next v2 0/3] xgmac: Support for 16 MTL/DMA queues Jakub Raczynski 2026-06-01 16:25 ` [PATCH net-next v2 1/3] net/stmmac/dwxgmac: Modify DMA functions for future hardware Jakub Raczynski 2026-06-01 16:25 ` [PATCH net-next v2 2/3] net/stmmac/dwxgmac: Extend MTL/DMA support to 16 queues Jakub Raczynski @ 2026-06-01 16:25 ` Jakub Raczynski 2026-06-04 13:16 ` Paolo Abeni 2 siblings, 1 reply; 9+ messages in thread From: Jakub Raczynski @ 2026-06-01 16:25 UTC (permalink / raw) To: netdev Cc: andrew+netdev, davem, edumazet, kuba, pabeni, mcoquelin.stm32, alexandre.torgue, linux-kernel, k.domagalski, k.tegowski, cs0617.lee, Jakub Raczynski Newer XGMAC hardware does support up to 16 DMA/MTL queues. Add support for these after previous modifications of driver to accommodate for that. Co-developed-by: Chang-Sub Lee <cs0617.lee@samsung.com> Signed-off-by: Chang-Sub Lee <cs0617.lee@samsung.com> Signed-off-by: Jakub Raczynski <j.raczynski@samsung.com> --- include/linux/stmmac.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h index 4430b967abde..a9d19e2c388c 100644 --- a/include/linux/stmmac.h +++ b/include/linux/stmmac.h @@ -15,9 +15,9 @@ #include <linux/platform_device.h> #include <linux/phylink.h> -#define MTL_MAX_RX_QUEUES 8 -#define MTL_MAX_TX_QUEUES 8 -#define STMMAC_CH_MAX 8 +#define MTL_MAX_RX_QUEUES 16 +#define MTL_MAX_TX_QUEUES 16 +#define STMMAC_CH_MAX 16 #define STMMAC_RX_COE_NONE 0 #define STMMAC_RX_COE_TYPE1 1 -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net-next v2 3/3] include/stmmac: Increase max DMA/MTL channel count from 8 to 16 2026-06-01 16:25 ` [PATCH net-next v2 3/3] include/stmmac: Increase max DMA/MTL channel count from 8 to 16 Jakub Raczynski @ 2026-06-04 13:16 ` Paolo Abeni 0 siblings, 0 replies; 9+ messages in thread From: Paolo Abeni @ 2026-06-04 13:16 UTC (permalink / raw) To: j.raczynski Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni, mcoquelin.stm32, alexandre.torgue, linux-kernel, k.domagalski, k.tegowski, cs0617.lee From: AI Reviewer <ai@example.com> This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev/ --- include/stmmac: Increase max DMA/MTL channel count from 8 to 16 Newer XGMAC hardware does support up to 16 DMA/MTL queues. Add support for these after previous modifications of driver to accommodate for that. > diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h > index 4430b967abdeb..a9d19e2c388c6 100644 > --- a/include/linux/stmmac.h > +++ b/include/linux/stmmac.h > @@ -15,9 +15,9 @@ > #include <linux/platform_device.h> > #include <linux/phylink.h> > > -#define MTL_MAX_RX_QUEUES 8 > -#define MTL_MAX_TX_QUEUES 8 > -#define STMMAC_CH_MAX 8 > +#define MTL_MAX_RX_QUEUES 16 > +#define MTL_MAX_TX_QUEUES 16 > +#define STMMAC_CH_MAX 16 [Severity: High] Does increasing these limits to 16 cause undefined behavior and silent register truncation in the hardware configuration functions when queue >= 8? In dwxgmac2_core.c:dwxgmac2_tx_queue_prio(), when queue >= 8, the logic only subtracts 4: if (queue >= 4) queue -= 4; This leaves queue >= 4, which causes XGMAC_PSTC_SHIFT(queue) to shift by >= 32 bits, overflowing the 32-bit integer. A similar regression occurs in dwmac4_core.c:dwmac4_tx_queue_priority() where it shifts by GMAC_TXQCTRL_PSTQX_SHIFT(queue). Additionally, in dwmac4_core.c:dwmac4_rx_queue_routing(), the queue variable is masked against a 3-bit bitmask, which silently truncates any queue >= 8: value |= (queue << route_possibilities[packet-1].reg_shift) & route_possibilities[packet - 1].reg_mask; Finally, dwmac4_core.c:dwmac4_map_mtl_dma() only maps queues up to 7 (using MAP0 and MAP1): if (queue < 4) { ... } else { value = readl(ioaddr + MTL_RXQ_DMA_MAP1); value &= ~MTL_RXQ_DMA_QXMDMACH_MASK(queue - 4); For queue >= 8, the shift calculation overflows and corrupts register data. Should these hardware configuration functions be updated to handle the expanded queue limits? [Severity: Critical] This is a pre-existing issue, but does missing bounds checking on the device tree properties expose an out-of-bounds array access during device probe? In stmmac_platform.c:stmmac_mtl_setup(), the device tree property snps,tx-queues-to-use is parsed and clamped only to 255 (U8_MAX): if (!of_property_read_u32(tx_node, "snps,tx-queues-to-use", &value)) { if (value > U8_MAX) value = U8_MAX; plat->tx_queues_to_use = value; } Later in stmmac_main.c:stmmac_hw_setup(), it attempts to dynamically clamp this value to the hardware capabilities: if (priv->dma_cap.number_tx_queues && priv->plat->tx_queues_to_use > priv->dma_cap.number_tx_queues) { priv->plat->tx_queues_to_use = priv->dma_cap.number_tx_queues; } However, if the hardware feature registers report 0 (which happens for older GMAC versions), this check is bypassed. The driver fails to clamp the requested queues against the static internal array limit MTL_MAX_TX_QUEUES. As a result, in stmmac_main.c:init_dma_tx_desc_rings(), the initialization loop iterates up to the unvalidated tx_queues_to_use: for (queue = 0; queue < tx_queue_cnt; queue++) __init_dma_tx_desc_rings(priv, dma_conf, queue); This will overrun the bounds of arrays embedded in struct stmmac_priv (like dma_conf->tx_queue) if the device tree configuration exceeds the static array limits. Should we add a strict bounds check against MTL_MAX_TX_QUEUES and MTL_MAX_RX_QUEUES during probe? -- This is an AI-generated review. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-06-06 2:02 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20260601162617eucas1p2efc386a4d9f9e7e98e8acb83f9c550d6@eucas1p2.samsung.com>
2026-06-01 16:25 ` [PATCH net-next v2 0/3] xgmac: Support for 16 MTL/DMA queues Jakub Raczynski
2026-06-01 16:25 ` [PATCH net-next v2 1/3] net/stmmac/dwxgmac: Modify DMA functions for future hardware Jakub Raczynski
2026-06-03 11:46 ` Jakub Raczynski
2026-06-03 12:45 ` Larysa Zaremba
2026-06-06 2:02 ` Jakub Kicinski
2026-06-01 16:25 ` [PATCH net-next v2 2/3] net/stmmac/dwxgmac: Extend MTL/DMA support to 16 queues Jakub Raczynski
2026-06-03 14:23 ` Larysa Zaremba
2026-06-01 16:25 ` [PATCH net-next v2 3/3] include/stmmac: Increase max DMA/MTL channel count from 8 to 16 Jakub Raczynski
2026-06-04 13:16 ` Paolo Abeni
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox