Re: [PATCH net-next v3 2/5] net: enetc: Enable probing of version 4.4 devices

[Simon Horman <horms@kernel.org>]

On Tue, Nov 03, 2026 at 04:09:08PM +0200, Claudiu Manoil wrote:
> The ENETC v4.4 devices are present on S32N79 SoCs and feature only
> Pseudo-MAC ENETC ports, whose PFs use either the 0xe101 or the 0xe110
> PCI device ID, and the VFs answer to the usual device ID of 0xef00.
> 
> While the ENETC PF instances are usually designated by the 0xe101 ID,
> in newer IP versions the 0xe110 ID was introduced to indicate the
> power-on reset default NETC switch management interface. These IDs
> do not indicate any functional difference between the designated
> ENETC instances, and can be interchanged and re-assigned at boot time
> (i.e. from firmware).
> 
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
> ---
> v3:
>  - Added sysclk_freq info for the v4.4 ENETC devices.
> 
> v2:
>  - Described why there are two ENETC PF IDs in the commit message, as per
>    review.

I realise this was forwarded somewhere by an automation, but it seems not
to the full CC list, so I'm reproducing it here:

  commit 6de2e15f2fb313f3ab9a84d0bfd5a867de11c72c
  Author: Claudiu Manoil <claudiu.manoil@nxp.com>
  
  net: enetc: Enable probing of version 4.4 devices
  
  This commit adds driver data and platform info entries for ENETC v4.4 PF and
  VF devices on S32N79 SoCs with a 500 MHz system clock frequency.
  
  > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
  > index c6571b5cc8425..b989823859ff2 100644
  > --- a/drivers/net/ethernet/freescale/enetc/enetc.c
  > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
  > @@ -3832,6 +3832,20 @@ static const struct enetc_drvdata enetc4_vf_data = {
  >  	.eth_ops = &enetc_vf_ethtool_ops,
  >  };
  >
  > +static const struct enetc_drvdata enetc4_s32n79_ppm_data = {
  > +	.sysclk_freq = ENETC_CLK_500M,
  > +	.tx_csum = true,
  > +	.max_frags = ENETC4_MAX_SKB_FRAGS,
  > +	.eth_ops = &enetc4_ppm_ethtool_ops,
  > +};
  > +
  > +static const struct enetc_drvdata enetc4_s32n79_vf_data = {
  > +	.sysclk_freq = ENETC_CLK_500M,
  > +	.tx_csum = true,
  > +	.max_frags = ENETC4_MAX_SKB_FRAGS,
  > +	.eth_ops = &enetc_vf_ethtool_ops,
  > +};
  > +
  >  static const struct enetc_platform_info enetc_info[] = {
  >  	{ .revision = ENETC_REV_1_0,
  >  	  .dev_id = ENETC_DEV_ID_PF,
  > @@ -3862,6 +3876,18 @@ static const struct enetc_platform_info enetc_info[] = {
  >  	  .dev_id = NXP_ENETC_VF_DEV_ID,
  >  	  .data = &enetc4_vf_data,
  >  	},
  > +	{ .revision = ENETC_REV_4_4,
  > +	  .dev_id = NXP_ENETC_PPM_DEV_ID,
  > +	  .data = &enetc4_s32n79_ppm_data,
  > +	},
  > +	{ .revision = ENETC_REV_4_4,
  > +	  .dev_id = NXP_ENETC_PF_DEV_ID,
  > +	  .data = &enetc4_s32n79_ppm_data,
  > +	},
  > +	{ .revision = ENETC_REV_4_4,
  > +	  .dev_id = NXP_ENETC_VF_DEV_ID,
  > +	  .data = &enetc4_s32n79_vf_data,
  > +	},
  
  Can probing ENETC v4.4 on an 8-core S32N79 SoC overflow priv->int_vector[] in
  enetc_int_vector_init()?
  
  In drivers/net/ethernet/freescale/enetc/enetc.h, ENETC_MAX_BDR_INT is still
  defined as 6:
  
  #define ENETC_MAX_BDR_INT	6 /* fixed to max # of available cpus */
  ...
  struct enetc_ndev_priv {
  	...
  	int bdr_int_num; /* number of Rx/Tx ring interrupts */
  	struct enetc_int_vector *int_vector[ENETC_MAX_BDR_INT];
  	u16 num_rx_rings, num_tx_rings;
  	u16 rx_bd_count, tx_bd_count;
  
  Previously supported SoCs had at most 6 CPUs, whereas S32N79 has 8 CPUs and
  enetc4_default_rings_allocation() allocates up to ENETC_SI_MAX_RING_NUM (8)
  RX rings to the PF.
  
  When enetc_init_si_rings_params() runs on an 8-CPU system with 7 or 8 RX
  rings:
  
  	int cpus = num_online_cpus();
  	...
  	priv->num_rx_rings = min_t(int, cpus, si->num_rx_rings);
  	priv->num_tx_rings = si->num_tx_rings;
  	priv->bdr_int_num = priv->num_rx_rings;
  
  priv->bdr_int_num becomes 7 or 8 without being clamped to ENETC_MAX_BDR_INT.
  Then enetc_alloc_msix() -> enetc_int_vector_init() writes past the end of
  priv->int_vector[]:
  
  	for (i = 0; i < priv->bdr_int_num; i++) {
  		...
  		err = enetc_int_vector_init(priv, i, num_tx_rings);
  
  where enetc_int_vector_init() does:
  
  	priv->int_vector[i] = v;
  
  overwriting num_rx_rings, num_tx_rings, rx_bd_count, tx_bd_count, and
  subsequent fields in struct enetc_ndev_priv.
  
  Should ENETC_MAX_BDR_INT be increased (and/or priv->bdr_int_num bounded by
  ENETC_MAX_BDR_INT)?

Link: https://sashiko.dev/#/patchset/cover.1793712309.git.claudiu.manoil%40nxp.com?part=2

This does seem to be a legitimate concern to me, and I think it warrants a v4.

There is also a review of patch 1/5, here, which I think warrants
investigation too.

Link: https://sashiko.dev/#/patchset/cover.1793712309.git.claudiu.manoil%40nxp.com?part=1

-- 
pw-bot: changes-requested