Message-ID: <2b7263db-ac5e-4eda-a599-2535242be24a@redhat.com>
Date: Tue, 10 Feb 2026 13:23:13 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Meghana Malladi <m-malladi@...com>, vadim.fedorenko@...ux.dev,
 jacob.e.keller@...el.com, horms@...nel.org, basharath@...thit.com,
 parvathi@...thit.com, afd@...com, vladimir.oltean@....com,
 rogerq@...nel.org, danishanwar@...com, kuba@...nel.org, edumazet@...gle.com,
 davem@...emloft.net, andrew+netdev@...n.ch
Cc: linux-arm-kernel@...ts.infradead.org, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, srk@...com,
 Vignesh Raghavendra <vigneshr@...com>
Subject: Re: [PATCH net-next v3 1/2] net: ti: icssg-prueth: Add Frame
 Preemption MAC Merge support

On 2/5/26 7:47 PM, Meghana Malladi wrote:
> From: MD Danish Anwar <danishanwar@...com>
> 
> This patch introduces qos support for the icssg driver. This
> includes adding support to configure mqprio qdisc and IET FPE.
> By default all the queues are marked as express which can be
> overwritten by the mqprio tc mask passed by tc qdisc.
> 
> icssg_config_ietfpe() work thread takes care of configuring
> IET FPE in the firmware and triggering the verify state machine
> based on the MAC Merge sublayer parameters set by the ethtool.
> The firmware handles the cleanup after successful mac verification.
> And in case the remote peer fails to respond to verify command
> before the timeout (5secs), then FPE is disabled by firmware.
> 
> During link up/down, verify state machine gets triggered again
> based on the state of fpe_enabled and fpe_active.
> 
> Signed-off-by: MD Danish Anwar <danishanwar@...com>
> Signed-off-by: Meghana Malladi <m-malladi@...com>
> ---
> 
> v3-v1:
> - Move icssg_qos_link_up/down() to if-statement right above as suggested by
> Vadim Fedorenko <vadim.fedorenko@...ux.dev>
> - Fix p_mqprio dereference without initialization as flagged by Simon Horman <horms@...nel.org>
> - Following changes as suggested by Vladimir Oltean <vladimir.oltean@....com>:
> * Provide more detailed commit message
> * Fix the ordering of the functions to avoid forward declarations
> * Fix the bug w.r.t scheduling work: fpe_config_task without initialising it
> * Fix the logic for icssg_iet_set_preempt_mask() - configure the queues based on user requests only.
> * Remove prueth_mqprio_validate() as this duplicates what "caps->validate_queue_counts = true" does.
> * Remove clearing PRE_EMPTION_ENABLE_TX on failure which is irrelevant - ACTIVE status never transitions to true.
> * Fix handling of these flags: iet->configured<->iet->fpe_enabled and iet->fpe_enabled<->iet->fpe_active
> * Schedule asynchronous work without waiting for any timeout
> * Remove cancel_fpe_config argument from prueth_qos_iet structure - no reader
> * Remove mentioning "priv flags" in the comments for prueth_qos_iet struct
> 
>  drivers/net/ethernet/ti/Makefile             |   2 +-
>  drivers/net/ethernet/ti/icssg/icssg_config.h |   9 -
>  drivers/net/ethernet/ti/icssg/icssg_prueth.c |   5 +
>  drivers/net/ethernet/ti/icssg/icssg_prueth.h |   2 +
>  drivers/net/ethernet/ti/icssg/icssg_qos.c    | 216 +++++++++++++++++++
>  drivers/net/ethernet/ti/icssg/icssg_qos.h    |  58 +++++
>  6 files changed, 282 insertions(+), 10 deletions(-)
>  create mode 100644 drivers/net/ethernet/ti/icssg/icssg_qos.c
>  create mode 100644 drivers/net/ethernet/ti/icssg/icssg_qos.h
> 
> diff --git a/drivers/net/ethernet/ti/Makefile b/drivers/net/ethernet/ti/Makefile
> index 6da50f4b7c2e..6893baf47d46 100644
> --- a/drivers/net/ethernet/ti/Makefile
> +++ b/drivers/net/ethernet/ti/Makefile
> @@ -35,7 +35,7 @@ ti-am65-cpsw-nuss-$(CONFIG_TI_K3_AM65_CPSW_SWITCHDEV) += am65-cpsw-switchdev.o
>  obj-$(CONFIG_TI_K3_AM65_CPTS) += am65-cpts.o
>  
>  obj-$(CONFIG_TI_ICSSG_PRUETH) += icssg-prueth.o icssg.o
> -icssg-prueth-y := icssg/icssg_prueth.o icssg/icssg_switchdev.o
> +icssg-prueth-y := icssg/icssg_prueth.o icssg/icssg_switchdev.o icssg/icssg_qos.o
>  
>  obj-$(CONFIG_TI_ICSSG_PRUETH_SR1) += icssg-prueth-sr1.o icssg.o
>  icssg-prueth-sr1-y := icssg/icssg_prueth_sr1.o
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_config.h b/drivers/net/ethernet/ti/icssg/icssg_config.h
> index 60d69744ffae..1ac202f855ed 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_config.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_config.h
> @@ -323,13 +323,4 @@ struct prueth_fdb_slot {
>  	u8 fid;
>  	u8 fid_c2;
>  } __packed;
> -
> -enum icssg_ietfpe_verify_states {
> -	ICSSG_IETFPE_STATE_UNKNOWN = 0,
> -	ICSSG_IETFPE_STATE_INITIAL,
> -	ICSSG_IETFPE_STATE_VERIFYING,
> -	ICSSG_IETFPE_STATE_SUCCEEDED,
> -	ICSSG_IETFPE_STATE_FAILED,
> -	ICSSG_IETFPE_STATE_DISABLED
> -};
>  #endif /* __NET_TI_ICSSG_CONFIG_H */
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> index f65041662173..1e4bc5fd636e 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> @@ -374,9 +374,11 @@ static void emac_adjust_link(struct net_device *ndev)
>  			spin_unlock_irqrestore(&emac->lock, flags);
>  			icssg_config_set_speed(emac);
>  			icssg_set_port_state(emac, ICSSG_EMAC_PORT_FORWARD);
> +			icssg_qos_link_up(ndev);
>  
>  		} else {
>  			icssg_set_port_state(emac, ICSSG_EMAC_PORT_DISABLE);
> +			icssg_qos_link_down(ndev);
>  		}
>  	}
>  
> @@ -967,6 +969,8 @@ static int emac_ndo_open(struct net_device *ndev)
>  	if (ret)
>  		goto destroy_rxq;
>  
> +	icssg_qos_init(ndev);
> +
>  	/* start PHY */
>  	phy_start(ndev->phydev);
>  
> @@ -1421,6 +1425,7 @@ static const struct net_device_ops emac_netdev_ops = {
>  	.ndo_hwtstamp_get = icssg_ndo_get_ts_config,
>  	.ndo_hwtstamp_set = icssg_ndo_set_ts_config,
>  	.ndo_xsk_wakeup = prueth_xsk_wakeup,
> +	.ndo_setup_tc = icssg_qos_ndo_setup_tc,
>  };
>  
>  static int prueth_netdev_init(struct prueth *prueth,
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> index 10eadd356650..7a586038adf8 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
> @@ -44,6 +44,7 @@
>  #include "icssg_config.h"
>  #include "icss_iep.h"
>  #include "icssg_switch_map.h"
> +#include "icssg_qos.h"
>  
>  #define PRUETH_MAX_MTU          (2000 - ETH_HLEN - ETH_FCS_LEN)
>  #define PRUETH_MIN_PKT_SIZE     (VLAN_ETH_ZLEN)
> @@ -255,6 +256,7 @@ struct prueth_emac {
>  	struct bpf_prog *xdp_prog;
>  	struct xdp_attachment_info xdpi;
>  	int xsk_qid;
> +	struct prueth_qos qos;
>  };
>  
>  /* The buf includes headroom compatible with both skb and xdpf */
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_qos.c b/drivers/net/ethernet/ti/icssg/icssg_qos.c
> new file mode 100644
> index 000000000000..8d17b28357f8
> --- /dev/null
> +++ b/drivers/net/ethernet/ti/icssg/icssg_qos.c
> @@ -0,0 +1,216 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Texas Instruments ICSSG PRUETH QoS submodule
> + * Copyright (C) 2023 Texas Instruments Incorporated - http://www.ti.com/
> + */
> +
> +#include "icssg_prueth.h"
> +#include "icssg_switch_map.h"
> +
> +static void icssg_iet_set_preempt_mask(struct prueth_emac *emac, u8 preemptible_tcs)
> +{
> +	void __iomem *config = emac->dram.va + ICSSG_CONFIG_OFFSET;
> +	struct prueth_qos_mqprio *p_mqprio = &emac->qos.mqprio;
> +	struct tc_mqprio_qopt *qopt = &p_mqprio->mqprio.qopt;
> +	int prempt_mask = 0, i;
> +	u8 tc;
> +
> +	/* Configure the queues based on the preemptible tc map set by the user */
> +	for (tc = 0; tc < p_mqprio->mqprio.qopt.num_tc; tc++) {
> +		/* check if the tc is preemptive or not */
> +		if (preemptible_tcs & BIT(tc)) {
> +			for (i = qopt->offset[tc]; i < qopt->offset[tc] + qopt->count[tc]; i++) {
> +				/* Set all the queues in this tc as preemptive queues */
> +				writeb(BIT(4), config + EXPRESS_PRE_EMPTIVE_Q_MAP + i);
> +				prempt_mask &= ~BIT(i);
> +			}
> +		} else {
> +			/* Set all the queues in this tc as express queues */
> +			for (i = qopt->offset[tc]; i < qopt->offset[tc] + qopt->count[tc]; i++) {
> +				writeb(0, config + EXPRESS_PRE_EMPTIVE_Q_MAP + i);
> +				prempt_mask |= BIT(i);
> +			}
> +		}
> +		writeb(prempt_mask, config + EXPRESS_PRE_EMPTIVE_Q_MASK);
> +		netdev_set_tc_queue(emac->ndev, tc, qopt->count[tc], qopt->offset[tc]);
> +	}
> +}
> +
> +static void icssg_config_ietfpe(struct work_struct *work)
> +{
> +	struct prueth_qos_iet *iet =
> +		container_of(work, struct prueth_qos_iet, fpe_config_task);
> +	void __iomem *config = iet->emac->dram.va + ICSSG_CONFIG_OFFSET;
> +	struct prueth_qos_mqprio *p_mqprio =  &iet->emac->qos.mqprio;
> +	bool enable = !!atomic_read(&iet->enable_fpe_config);
> +	int ret;
> +	u8 val;
> +
> +	if (!netif_running(iet->emac->ndev))
> +		return;
> +
> +	/* Update FPE Tx enable bit (PRE_EMPTION_ACTIVE_TX) if
> +	 * fpe_enabled is set to enable MM in Tx direction
> +	 */
> +	writeb(enable ? 1 : 0, config + PRE_EMPTION_ENABLE_TX);
> +
> +	/* If FPE is to be enabled, first configure MAC Verify state
> +	 * machine in firmware as firmware kicks the Verify process
> +	 * as soon as ICSSG_EMAC_PORT_PREMPT_TX_ENABLE command is
> +	 * received.
> +	 */
> +	if (enable && iet->mac_verify_configure) {
> +		writeb(1, config + PRE_EMPTION_ENABLE_VERIFY);
> +		writew(iet->tx_min_frag_size, config + PRE_EMPTION_ADD_FRAG_SIZE_LOCAL);
> +		writel(iet->verify_time_ms, config + PRE_EMPTION_VERIFY_TIME);
> +	}
> +
> +	/* Send command to enable FPE Tx side. Rx is always enabled */
> +	ret = icssg_set_port_state(iet->emac,
> +				   enable ? ICSSG_EMAC_PORT_PREMPT_TX_ENABLE :
> +					    ICSSG_EMAC_PORT_PREMPT_TX_DISABLE);
> +	if (ret) {
> +		netdev_err(iet->emac->ndev, "TX preempt %s command failed\n",
> +			   str_enable_disable(enable));
> +		writeb(0, config + PRE_EMPTION_ENABLE_VERIFY);
> +		iet->verify_status = ICSSG_IETFPE_STATE_DISABLED;
> +		return;
> +	}
> +
> +	if (enable && iet->mac_verify_configure) {
> +		ret = readb_poll_timeout(config + PRE_EMPTION_VERIFY_STATUS, iet->verify_status,
> +					 (iet->verify_status == ICSSG_IETFPE_STATE_SUCCEEDED),
> +					 USEC_PER_MSEC, 5 * USEC_PER_SEC);
> +		if (ret) {
> +			iet->verify_status = ICSSG_IETFPE_STATE_FAILED;
> +			netdev_err(iet->emac->ndev,
> +				   "timeout for MAC Verify: status %x\n",
> +				   iet->verify_status);
> +			return;
> +		}
> +	} else if (enable) {
> +		/* Give f/w some time to update PRE_EMPTION_ACTIVE_TX state */
> +		usleep_range(100, 200);
> +	}
> +
> +	if (enable) {
> +		val = readb(config + PRE_EMPTION_ACTIVE_TX);
> +		if (val != 1) {
> +			netdev_err(iet->emac->ndev,
> +				   "F/w fails to activate IET/FPE\n");
> +			return;
> +		}
> +		iet->fpe_active = true;
> +	} else {
> +		iet->fpe_active = false;
> +	}
> +
> +	netdev_info(iet->emac->ndev, "IET FPE %s successfully\n",
> +		    str_enable_disable(iet->fpe_active));
> +	icssg_iet_set_preempt_mask(iet->emac, p_mqprio->preemptible_tcs);
> +}
> +
> +void icssg_qos_init(struct net_device *ndev)
> +{
> +	struct prueth_emac *emac = netdev_priv(ndev);
> +	struct prueth_qos_iet *iet = &emac->qos.iet;
> +
> +	/* Init work queue for IET MAC verify process */
> +	iet->emac = emac;
> +	INIT_WORK(&iet->fpe_config_task, icssg_config_ietfpe);
> +}
> +
> +static int emac_tc_query_caps(struct net_device *ndev, void *type_data)
> +{
> +	struct tc_query_caps_base *base = type_data;
> +
> +	switch (base->type) {
> +	case TC_SETUP_QDISC_MQPRIO: {
> +		struct tc_mqprio_caps *caps = base->caps;
> +
> +		caps->validate_queue_counts = true;
> +		return 0;
> +	}
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int emac_tc_setup_mqprio(struct net_device *ndev, void *type_data)
> +{
> +	struct tc_mqprio_qopt_offload *mqprio = type_data;
> +	struct prueth_emac *emac = netdev_priv(ndev);
> +	struct tc_mqprio_qopt *qopt = &mqprio->qopt;
> +	struct prueth_qos_mqprio *p_mqprio;
> +	u8 num_tc = mqprio->qopt.num_tc;
> +	int tc, offset, count;
> +
> +	p_mqprio = &emac->qos.mqprio;
> +
> +	if (!num_tc) {
> +		netdev_reset_tc(ndev);
> +		p_mqprio->preemptible_tcs = 0;
> +		goto reset_tcs;
> +	}
> +
> +	memcpy(&p_mqprio->mqprio, mqprio, sizeof(*mqprio));
> +	p_mqprio->preemptible_tcs = mqprio->preemptible_tcs;
> +	netdev_set_num_tc(ndev, mqprio->qopt.num_tc);
> +
> +	for (tc = 0; tc < num_tc; tc++) {
> +		count = qopt->count[tc];
> +		offset = qopt->offset[tc];
> +		netdev_set_tc_queue(ndev, tc, count, offset);
> +	}
> +
> +reset_tcs:
> +	icssg_iet_set_preempt_mask(emac, p_mqprio->preemptible_tcs);
> +
> +	return 0;
> +}
> +
> +int icssg_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type type,
> +			   void *type_data)
> +{
> +	switch (type) {
> +	case TC_QUERY_CAPS:
> +		return emac_tc_query_caps(ndev, type_data);
> +	case TC_SETUP_QDISC_MQPRIO:
> +		return emac_tc_setup_mqprio(ndev, type_data);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(icssg_qos_ndo_setup_tc);
> +
> +void icssg_qos_link_up(struct net_device *ndev)
> +{
> +	struct prueth_emac *emac = netdev_priv(ndev);
> +	struct prueth_qos_iet *iet = &emac->qos.iet;
> +
> +	/* Enable FPE if not active but fpe_enabled is true
> +	 * and disable FPE if active but fpe_enabled is false
> +	 */
> +	if (!iet->fpe_active && iet->fpe_enabled) {
> +		/* Schedule IET FPE IET FPE enable */
> +		atomic_set(&iet->enable_fpe_config, 1);
> +	} else if (iet->fpe_active && !iet->fpe_enabled) {
> +		/* Schedule IET FPE IET FPE disable */
> +		atomic_set(&iet->enable_fpe_config, 0);
> +	} else {
> +		return;
> +	}
> +	schedule_work(&iet->fpe_config_task);

AFAICS the scheduled work could still be running at module removal time.
If so, you need to cancel it properly to avoid possible UaF.

/P

Powered by blists - more mailing lists