[PATCH net-next] net: airoha: Reset PPE cpu port configuration in airoha_ppe_hw_init()

[PATCH net-next] net: airoha: Reset PPE cpu port configuration in airoha_ppe_hw_init()

[Lorenzo Bianconi]

Before this patch, the PPE cpu port configuration used for a specific GDM
device was set just running ndo_init() callback during the device
initialization. The selected PPE cpu port configuration depends on the QDMA
block assigned to the GDM port. The QDMA block is selected according to
the GDM port LAN/WAN configuration as specified in the commit
'8737d7194d6d ("net: airoha: select QDMA block according LAN/WAN
configuration"). However, the user selected PPE cpu port configuration can
be different with respect to the one hardcoded in the NPU firmware binary.
The hardcoded NPU PPE cpu port configuration is loaded initializing the PPE
engine running the NPU ops ppe_init() callback in airoha_ppe_offload_setup
routine (this is executed at runtime by the netfilter flowtable
infrastructure during flow offloading).
Reset the PPE cpu port configuration in airoha_ppe_hw_init routine in
order to apply the user requested setup according to the device DTS.
Please note this patch is fixing an issue not visible to the user (so we
do not need to backport it) since airoha_eth driver currently supports just
the internal phy available via the MT7530 DSA switch and there are no WAN
interfaces officially supported since PCS/external phy is not merged
mainline yet (it will be posted with following patches).

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/airoha/airoha_eth.c  | 28 ++++++----------------------
 drivers/net/ethernet/airoha/airoha_eth.h  |  2 ++
 drivers/net/ethernet/airoha/airoha_ppe.c  | 23 ++++++++++++++++++++++-
 drivers/net/ethernet/airoha/airoha_regs.h |  7 +++----
 4 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index b4563b5d53bbf174d45548a5efc6fdfb79d33c19..21a866e3cc05522104c0a7dc49a6b7a3c9f42165 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c
@@ -1755,8 +1755,7 @@ static int airoha_dev_init(struct net_device *dev)
 {
 	struct airoha_gdm_port *port = netdev_priv(dev);
 	struct airoha_eth *eth = port->eth;
-	u32 fe_cpu_port;
-	u8 ppe_id;
+	int i;
 
 	/* QDMA0 is used for lan ports while QDMA1 is used for WAN ports */
 	port->qdma = &eth->qdma[!airoha_is_lan_gdm_port(port)];
@@ -1774,28 +1773,13 @@ static int airoha_dev_init(struct net_device *dev)
 			if (err)
 				return err;
 		}
-		fallthrough;
-	case AIROHA_GDM2_IDX:
-		if (airoha_ppe_is_enabled(eth, 1)) {
-			/* For PPE2 always use secondary cpu port. */
-			fe_cpu_port = FE_PSE_PORT_CDM2;
-			ppe_id = 1;
-			break;
-		}
-		fallthrough;
-	default: {
-		u8 qdma_id = port->qdma - &eth->qdma[0];
-
-		/* For PPE1 select cpu port according to the running QDMA. */
-		fe_cpu_port = qdma_id ? FE_PSE_PORT_CDM2 : FE_PSE_PORT_CDM1;
-		ppe_id = 0;
 		break;
-	}
+	default:
+		break;
 	}
 
-	airoha_fe_rmw(eth, REG_PPE_DFT_CPORT0(ppe_id),
-		      DFT_CPORT_MASK(port->id),
-		      __field_prep(DFT_CPORT_MASK(port->id), fe_cpu_port));
+	for (i = 0; i < eth->soc->num_ppe; i++)
+		airoha_ppe_set_cpu_port(port, i);
 
 	return 0;
 }
@@ -1898,7 +1882,7 @@ static u32 airoha_get_dsa_tag(struct sk_buff *skb, struct net_device *dev)
 #endif
 }
 
-static int airoha_get_fe_port(struct airoha_gdm_port *port)
+int airoha_get_fe_port(struct airoha_gdm_port *port)
 {
 	struct airoha_qdma *qdma = port->qdma;
 	struct airoha_eth *eth = qdma->eth;
diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h
index 8cfa94ec084a16d6b2d3c94c404f25904c1150e3..7df4dbcd8861856c54c2a38bc89c69180ac2f6dc 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.h
+++ b/drivers/net/ethernet/airoha/airoha_eth.h
@@ -646,9 +646,11 @@ static inline bool airoha_is_7583(struct airoha_eth *eth)
 	return eth->soc->version == 0x7583;
 }
 
+int airoha_get_fe_port(struct airoha_gdm_port *port);
 bool airoha_is_valid_gdm_port(struct airoha_eth *eth,
 			      struct airoha_gdm_port *port);
 
+void airoha_ppe_set_cpu_port(struct airoha_gdm_port *port, u8 ppe_id);
 bool airoha_ppe_is_enabled(struct airoha_eth *eth, int index);
 void airoha_ppe_check_skb(struct airoha_ppe_dev *dev, struct sk_buff *skb,
 			  u16 hash, bool rx_wlan);
diff --git a/drivers/net/ethernet/airoha/airoha_ppe.c b/drivers/net/ethernet/airoha/airoha_ppe.c
index ec5ce41dad80827c8b467b8be112eebd14914744..7666b1d2f4f6ea758683181ab90d6fffb7bcd19d 100644
--- a/drivers/net/ethernet/airoha/airoha_ppe.c
+++ b/drivers/net/ethernet/airoha/airoha_ppe.c
@@ -85,6 +85,20 @@ static u32 airoha_ppe_get_timestamp(struct airoha_ppe *ppe)
 	return FIELD_GET(AIROHA_FOE_IB1_BIND_TIMESTAMP, timestamp);
 }
 
+void airoha_ppe_set_cpu_port(struct airoha_gdm_port *port, u8 ppe_id)
+{
+	struct airoha_qdma *qdma = port->qdma;
+	u8 fport = airoha_get_fe_port(port);
+	struct airoha_eth *eth = qdma->eth;
+	u8 qdma_id = qdma - &eth->qdma[0];
+	u32 fe_cpu_port;
+
+	fe_cpu_port = qdma_id ? FE_PSE_PORT_CDM2 : FE_PSE_PORT_CDM1;
+	airoha_fe_rmw(eth, REG_PPE_DFT_CPORT(ppe_id, fport),
+		      DFT_CPORT_MASK(fport),
+		      __field_prep(DFT_CPORT_MASK(fport), fe_cpu_port));
+}
+
 static void airoha_ppe_hw_init(struct airoha_ppe *ppe)
 {
 	u32 sram_ppe_num_data_entries = PPE_SRAM_NUM_ENTRIES, sram_num_entries;
@@ -147,7 +161,9 @@ static void airoha_ppe_hw_init(struct airoha_ppe *ppe)
 
 		airoha_fe_wr(eth, REG_PPE_HASH_SEED(i), PPE_HASH_SEED);
 
-		for (p = 0; p < ARRAY_SIZE(eth->ports); p++)
+		for (p = 0; p < ARRAY_SIZE(eth->ports); p++) {
+			struct airoha_gdm_port *port = eth->ports[p];
+
 			airoha_fe_rmw(eth, REG_PPE_MTU(i, p),
 				      FP0_EGRESS_MTU_MASK |
 				      FP1_EGRESS_MTU_MASK,
@@ -155,6 +171,11 @@ static void airoha_ppe_hw_init(struct airoha_ppe *ppe)
 						 AIROHA_MAX_MTU) |
 				      FIELD_PREP(FP1_EGRESS_MTU_MASK,
 						 AIROHA_MAX_MTU));
+			if (!port)
+				continue;
+
+			airoha_ppe_set_cpu_port(port, i);
+		}
 	}
 }
 
diff --git a/drivers/net/ethernet/airoha/airoha_regs.h b/drivers/net/ethernet/airoha/airoha_regs.h
index 29878b954c77c79ae45e63357840a10c23c517f2..436f3c8779c102e900139e9f015816ebeb64eb98 100644
--- a/drivers/net/ethernet/airoha/airoha_regs.h
+++ b/drivers/net/ethernet/airoha/airoha_regs.h
@@ -312,10 +312,9 @@
 #define REG_PPE_HASH_SEED(_n)			(((_n) ? PPE2_BASE : PPE1_BASE) + 0x244)
 #define PPE_HASH_SEED				0x12345678
 
-#define REG_PPE_DFT_CPORT0(_n)			(((_n) ? PPE2_BASE : PPE1_BASE) + 0x248)
-#define DFT_CPORT_MASK(_n)			GENMASK(3 + ((_n) << 2), ((_n) << 2))
-
-#define REG_PPE_DFT_CPORT1(_n)			(((_n) ? PPE2_BASE : PPE1_BASE) + 0x24c)
+#define REG_PPE_DFT_CPORT_BASE(_n)		(((_n) ? PPE2_BASE : PPE1_BASE) + 0x248)
+#define REG_PPE_DFT_CPORT(_m, _n)		(REG_PPE_DFT_CPORT_BASE(_m) + (((_n) / 8) << 2))
+#define DFT_CPORT_MASK(_n)			GENMASK(3 + (((_n) % 8) << 2), (((_n) % 8) << 2))
 
 #define REG_PPE_TB_HASH_CFG(_n)			(((_n) ? PPE2_BASE : PPE1_BASE) + 0x250)
 #define PPE_DRAM_HASH1_MODE_MASK		GENMASK(31, 28)

---
base-commit: 8737d7194d6d5947c3d7d8813895b44a25b84477
change-id: 20260317-airoha-fix-ppe-def-cpu-0280f76ddcbb

Best regards,
-- 
Lorenzo Bianconi <lorenzo@kernel.org>

Re: [PATCH net-next] net: airoha: Reset PPE cpu port configuration in airoha_ppe_hw_init()

[Simon Horman]

On Tue, Mar 17, 2026 at 05:40:47PM +0100, Lorenzo Bianconi wrote:
> Before this patch, the PPE cpu port configuration used for a specific GDM
> device was set just running ndo_init() callback during the device
> initialization. The selected PPE cpu port configuration depends on the QDMA
> block assigned to the GDM port. The QDMA block is selected according to
> the GDM port LAN/WAN configuration as specified in the commit
> '8737d7194d6d ("net: airoha: select QDMA block according LAN/WAN
> configuration"). However, the user selected PPE cpu port configuration can
> be different with respect to the one hardcoded in the NPU firmware binary.
> The hardcoded NPU PPE cpu port configuration is loaded initializing the PPE
> engine running the NPU ops ppe_init() callback in airoha_ppe_offload_setup
> routine (this is executed at runtime by the netfilter flowtable
> infrastructure during flow offloading).
> Reset the PPE cpu port configuration in airoha_ppe_hw_init routine in
> order to apply the user requested setup according to the device DTS.
> Please note this patch is fixing an issue not visible to the user (so we
> do not need to backport it) since airoha_eth driver currently supports just
> the internal phy available via the MT7530 DSA switch and there are no WAN
> interfaces officially supported since PCS/external phy is not merged
> mainline yet (it will be posted with following patches).
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Hi Lorenzo,

Thanks for the detailed commit message.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next] net: airoha: Reset PPE cpu port configuration in airoha_ppe_hw_init()

[Jakub Kicinski]

On Tue, 17 Mar 2026 17:40:47 +0100 Lorenzo Bianconi wrote:
> @@ -155,6 +171,11 @@ static void airoha_ppe_hw_init(struct airoha_ppe *ppe)
>  						 AIROHA_MAX_MTU) |
>  				      FIELD_PREP(FP1_EGRESS_MTU_MASK,
>  						 AIROHA_MAX_MTU));
> +			if (!port)
> +				continue;
> +
> +			airoha_ppe_set_cpu_port(port, i);

AI says:

Can this lead to a NULL pointer dereference if a port is not fully
initialized?

In airoha_probe(), all GDM ports defined in the device tree are allocated
and the eth->ports[] array is populated with pointers, but port->qdma is
left as NULL.

During airoha_register_gdm_devices(), the ports are registered sequentially
with register_netdev(). Since register_netdev() drops the rtnl_lock(),
userspace could react to the RTM_NEWLINK event of the first registered port
and apply a tc flow offload rule.

This would trigger the following call chain:
  .ndo_setup_tc() -> airoha_ppe_setup_tc_block_cb() -> airoha_ppe_offload_setup() 
   -> airoha_ppe_hw_init()

If airoha_ppe_hw_init() iterates over the array, it will find the subsequent
port that has been allocated but not yet registered, meaning its port->qdma
is still NULL. The call to airoha_ppe_set_cpu_port(port, i) will then
dereference the NULL port->qdma.

Would it be better to check if (!port || !port->qdma) before calling
airoha_ppe_set_cpu_port()?
-- 
pw-bot: cr

Re: [PATCH net-next] net: airoha: Reset PPE cpu port configuration in airoha_ppe_hw_init()

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 3912 bytes --]

> On Tue, 17 Mar 2026 17:40:47 +0100 Lorenzo Bianconi wrote:
> > @@ -155,6 +171,11 @@ static void airoha_ppe_hw_init(struct airoha_ppe *ppe)
> >  						 AIROHA_MAX_MTU) |
> >  				      FIELD_PREP(FP1_EGRESS_MTU_MASK,
> >  						 AIROHA_MAX_MTU));
> > +			if (!port)
> > +				continue;
> > +
> > +			airoha_ppe_set_cpu_port(port, i);
> 
> AI says:
> 
> Can this lead to a NULL pointer dereference if a port is not fully
> initialized?
> 
> In airoha_probe(), all GDM ports defined in the device tree are allocated
> and the eth->ports[] array is populated with pointers, but port->qdma is
> left as NULL.
> 
> During airoha_register_gdm_devices(), the ports are registered sequentially
> with register_netdev(). Since register_netdev() drops the rtnl_lock(),
> userspace could react to the RTM_NEWLINK event of the first registered port
> and apply a tc flow offload rule.
> 
> This would trigger the following call chain:
>   .ndo_setup_tc() -> airoha_ppe_setup_tc_block_cb() -> airoha_ppe_offload_setup() 
>    -> airoha_ppe_hw_init()
> 
> If airoha_ppe_hw_init() iterates over the array, it will find the subsequent
> port that has been allocated but not yet registered, meaning its port->qdma
> is still NULL. The call to airoha_ppe_set_cpu_port(port, i) will then
> dereference the NULL port->qdma.
> 
> Would it be better to check if (!port || !port->qdma) before calling
> airoha_ppe_set_cpu_port()?

Hi Jakub,

I agree there is a race between airoha_register_gdm_devices() and
airoha_ppe_setup_tc_block_cb(). Instead of not running
airoha_ppe_set_cpu_port() in airoha_ppe_hw_init() for a given port and not
configuring the hw correctly, I guess it would be better to grab
flow_offload_mutex mutex in airoha_register_gdm_devices routine and wait for
all devices to be properly registered before executing
airoha_ppe_setup_tc_block_cb(). Something like:

diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index 961325da833b..f5bb917f0f6e 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c
@@ -2927,21 +2927,24 @@ static int airoha_alloc_gdm_port(struct airoha_eth *eth,
 
 static int airoha_register_gdm_devices(struct airoha_eth *eth)
 {
-	int i;
+	int i, err = 0;
+
+	mutex_lock(&flow_offload_mutex);
 
 	for (i = 0; i < ARRAY_SIZE(eth->ports); i++) {
 		struct airoha_gdm_port *port = eth->ports[i];
-		int err;
 
 		if (!port)
 			continue;
 
 		err = register_netdev(port->dev);
 		if (err)
-			return err;
+			break;
 	}
 
-	return 0;
+	mutex_unlock(&flow_offload_mutex);
+
+	return err;
 }
 
 static int airoha_probe(struct platform_device *pdev)
diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h
index 7df4dbcd8861..3330ee54e20e 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.h
+++ b/drivers/net/ethernet/airoha/airoha_eth.h
@@ -551,6 +551,8 @@ struct airoha_gdm_port {
 #define AIROHA_RXD4_PPE_CPU_REASON	GENMASK(20, 16)
 #define AIROHA_RXD4_FOE_ENTRY		GENMASK(15, 0)
 
+extern struct mutex flow_offload_mutex;
+
 struct airoha_ppe {
 	struct airoha_ppe_dev dev;
 	struct airoha_eth *eth;
diff --git a/drivers/net/ethernet/airoha/airoha_ppe.c b/drivers/net/ethernet/airoha/airoha_ppe.c
index 7666b1d2f4f6..96d092f7214e 100644
--- a/drivers/net/ethernet/airoha/airoha_ppe.c
+++ b/drivers/net/ethernet/airoha/airoha_ppe.c
@@ -15,7 +15,7 @@
 #include "airoha_regs.h"
 #include "airoha_eth.h"
 
-static DEFINE_MUTEX(flow_offload_mutex);
+DEFINE_MUTEX(flow_offload_mutex);
 static DEFINE_SPINLOCK(ppe_lock);
 
 static const struct rhashtable_params airoha_flow_table_params = {


What do you think?
I noticed commit f44218cd5e6a is already applied to net-next. I will post a
follow-up patch, agree?

Regards,
Lorenzo

> -- 
> pw-bot: cr

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next] net: airoha: Reset PPE cpu port configuration in airoha_ppe_hw_init()

[Jakub Kicinski]

On Sat, 21 Mar 2026 13:41:06 +0100 Lorenzo Bianconi wrote:
> What do you think?
> I noticed commit f44218cd5e6a is already applied to net-next. I will post a
> follow-up patch, agree?

Sounds good, and yes please.