[PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST

[PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST

[Breno Leitao]

As most of the drivers that depend on ARCH_LAYERSCAPE, make FSL_DPAA
depend on COMPILE_TEST for compilation and testing.

	# grep -r depends.\*ARCH_LAYERSCAPE.\*COMPILE_TEST | wc -l
	29

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/soc/fsl/qbman/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/fsl/qbman/Kconfig b/drivers/soc/fsl/qbman/Kconfig
index bdecb86bb656..27774ec6ff90 100644
--- a/drivers/soc/fsl/qbman/Kconfig
+++ b/drivers/soc/fsl/qbman/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 menuconfig FSL_DPAA
 	bool "QorIQ DPAA1 framework support"
-	depends on ((FSL_SOC_BOOKE || ARCH_LAYERSCAPE) && ARCH_DMA_ADDR_T_64BIT)
+	depends on ((FSL_SOC_BOOKE || ARCH_LAYERSCAPE || COMPILE_TEST) && ARCH_DMA_ADDR_T_64BIT)
 	select GENERIC_ALLOCATOR
 	help
 	  The Freescale Data Path Acceleration Architecture (DPAA) is a set of
-- 
2.43.0

[PATCH 2/4] crypto: caam: Depend on COMPILE_TEST also

[Breno Leitao]

As most of the drivers that depend on ARCH_LAYERSCAPE, make
CRYPTO_DEV_FSL_CAAM depend on COMPILE_TEST for compilation and testing.

    # grep -r depends.\*ARCH_LAYERSCAPE.\*COMPILE_TEST | wc -l
    29

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/crypto/caam/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig
index c631f99e415f..05210a0edb8a 100644
--- a/drivers/crypto/caam/Kconfig
+++ b/drivers/crypto/caam/Kconfig
@@ -10,7 +10,7 @@ config CRYPTO_DEV_FSL_CAAM_AHASH_API_DESC
 
 config CRYPTO_DEV_FSL_CAAM
 	tristate "Freescale CAAM-Multicore platform driver backend"
-	depends on FSL_SOC || ARCH_MXC || ARCH_LAYERSCAPE
+	depends on FSL_SOC || ARCH_MXC || ARCH_LAYERSCAPE || COMPILE_TEST
 	select SOC_BUS
 	select CRYPTO_DEV_FSL_CAAM_COMMON
 	imply FSL_MC_BUS
-- 
2.43.0

[PATCH 3/4] crypto: caam: Unembed net_dev structure from qi

[Breno Leitao]

Embedding net_device into structures prohibits the usage of flexible
arrays in the net_device structure. For more details, see the discussion
at [1].

Un-embed the net_devices from struct caam_qi_pcpu_priv by converting them
into pointers, and allocating them dynamically. Use the leverage
alloc_netdev_dummy() to allocate the net_device object at
caam_qi_init().

The free of the device occurs at caam_qi_shutdown().

Link: https://lore.kernel.org/all/20240229225910.79e224cf@kernel.org/ [1]
Signed-off-by: Breno Leitao <leitao@debian.org>
---
PS: Unfortunately due to lack of hardware, this was not tested in real
hardware.

 drivers/crypto/caam/qi.c | 43 ++++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/drivers/crypto/caam/qi.c b/drivers/crypto/caam/qi.c
index 46a083849a8e..0c13ffc81862 100644
--- a/drivers/crypto/caam/qi.c
+++ b/drivers/crypto/caam/qi.c
@@ -57,7 +57,7 @@ struct caam_napi {
  */
 struct caam_qi_pcpu_priv {
 	struct caam_napi caam_napi;
-	struct net_device net_dev;
+	struct net_device *net_dev;
 	struct qman_fq *rsp_fq;
 } ____cacheline_aligned;
 
@@ -144,7 +144,7 @@ static void caam_fq_ern_cb(struct qman_portal *qm, struct qman_fq *fq,
 {
 	const struct qm_fd *fd;
 	struct caam_drv_req *drv_req;
-	struct device *qidev = &(raw_cpu_ptr(&pcpu_qipriv)->net_dev.dev);
+	struct device *qidev = &(raw_cpu_ptr(&pcpu_qipriv)->net_dev->dev);
 	struct caam_drv_private *priv = dev_get_drvdata(qidev);
 
 	fd = &msg->ern.fd;
@@ -530,6 +530,7 @@ static void caam_qi_shutdown(void *data)
 
 		if (kill_fq(qidev, per_cpu(pcpu_qipriv.rsp_fq, i)))
 			dev_err(qidev, "Rsp FQ kill failed, cpu: %d\n", i);
+		free_netdev(pcpu_qipriv.net_dev);
 	}
 
 	qman_delete_cgr_safe(&priv->cgr);
@@ -573,7 +574,7 @@ static enum qman_cb_dqrr_result caam_rsp_fq_dqrr_cb(struct qman_portal *p,
 	struct caam_napi *caam_napi = raw_cpu_ptr(&pcpu_qipriv.caam_napi);
 	struct caam_drv_req *drv_req;
 	const struct qm_fd *fd;
-	struct device *qidev = &(raw_cpu_ptr(&pcpu_qipriv)->net_dev.dev);
+	struct device *qidev = &(raw_cpu_ptr(&pcpu_qipriv)->net_dev->dev);
 	struct caam_drv_private *priv = dev_get_drvdata(qidev);
 	u32 status;
 
@@ -718,12 +719,24 @@ static void free_rsp_fqs(void)
 		kfree(per_cpu(pcpu_qipriv.rsp_fq, i));
 }
 
+static void free_caam_qi_pcpu_netdev(const cpumask_t *cpus)
+{
+	struct caam_qi_pcpu_priv *priv;
+	int i;
+
+	for_each_cpu(i, cpus) {
+		priv = per_cpu_ptr(&pcpu_qipriv, i);
+		free_netdev(priv->net_dev);
+	}
+}
+
 int caam_qi_init(struct platform_device *caam_pdev)
 {
 	int err, i;
 	struct device *ctrldev = &caam_pdev->dev, *qidev;
 	struct caam_drv_private *ctrlpriv;
 	const cpumask_t *cpus = qman_affine_cpus();
+	cpumask_t clean_mask;
 
 	ctrlpriv = dev_get_drvdata(ctrldev);
 	qidev = ctrldev;
@@ -743,6 +756,8 @@ int caam_qi_init(struct platform_device *caam_pdev)
 		return err;
 	}
 
+	cpumask_clear(&clean_mask);
+
 	/*
 	 * Enable the NAPI contexts on each of the core which has an affine
 	 * portal.
@@ -751,10 +766,16 @@ int caam_qi_init(struct platform_device *caam_pdev)
 		struct caam_qi_pcpu_priv *priv = per_cpu_ptr(&pcpu_qipriv, i);
 		struct caam_napi *caam_napi = &priv->caam_napi;
 		struct napi_struct *irqtask = &caam_napi->irqtask;
-		struct net_device *net_dev = &priv->net_dev;
+		struct net_device *net_dev;
 
+		net_dev = alloc_netdev_dummy(0);
+		if (!net_dev) {
+			err = -ENOMEM;
+			goto fail;
+		}
+		cpumask_set_cpu(i, &clean_mask);
+		priv->net_dev = net_dev;
 		net_dev->dev = *qidev;
-		INIT_LIST_HEAD(&net_dev->napi_list);
 
 		netif_napi_add_tx_weight(net_dev, irqtask, caam_qi_poll,
 					 CAAM_NAPI_WEIGHT);
@@ -766,16 +787,22 @@ int caam_qi_init(struct platform_device *caam_pdev)
 				     dma_get_cache_alignment(), 0, NULL);
 	if (!qi_cache) {
 		dev_err(qidev, "Can't allocate CAAM cache\n");
-		free_rsp_fqs();
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto fail2;
 	}
 
 	caam_debugfs_qi_init(ctrlpriv);
 
 	err = devm_add_action_or_reset(qidev, caam_qi_shutdown, ctrlpriv);
 	if (err)
-		return err;
+		goto fail2;
 
 	dev_info(qidev, "Linux CAAM Queue I/F driver initialised\n");
 	return 0;
+
+fail2:
+	free_rsp_fqs();
+fail:
+	free_caam_qi_pcpu_netdev(&clean_mask);
+	return err;
 }
-- 
2.43.0

[PATCH 4/4] crypto: caam: Unembed net_dev structure in dpaa2

[Breno Leitao]

Embedding net_device into structures prohibits the usage of flexible
arrays in the net_device structure. For more details, see the discussion
at [1].

Un-embed the net_devices from struct dpaa2_caam_priv_per_cpu by
converting them into pointers, and allocating them dynamically. Use the
leverage alloc_netdev_dummy() to allocate the net_device object at
dpaa2_dpseci_setup().

The free of the device occurs at dpaa2_dpseci_disable().

Link: https://lore.kernel.org/all/20240229225910.79e224cf@kernel.org/ [1]
Signed-off-by: Breno Leitao <leitao@debian.org>
---
PS: Unfortunately due to lack of hardware, this was not tested in real
hardware.

 drivers/crypto/caam/caamalg_qi2.c | 28 +++++++++++++++++++++++++---
 drivers/crypto/caam/caamalg_qi2.h |  2 +-
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c
index a4f6884416a0..207dc422785a 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -4990,11 +4990,23 @@ static int dpaa2_dpseci_congestion_setup(struct dpaa2_caam_priv *priv,
 	return err;
 }
 
+static void free_dpaa2_pcpu_netdev(struct dpaa2_caam_priv *priv, const cpumask_t *cpus)
+{
+	struct dpaa2_caam_priv_per_cpu *ppriv;
+	int i;
+
+	for_each_cpu(i, cpus) {
+		ppriv = per_cpu_ptr(priv->ppriv, i);
+		free_netdev(ppriv->net_dev);
+	}
+}
+
 static int __cold dpaa2_dpseci_setup(struct fsl_mc_device *ls_dev)
 {
 	struct device *dev = &ls_dev->dev;
 	struct dpaa2_caam_priv *priv;
 	struct dpaa2_caam_priv_per_cpu *ppriv;
+	cpumask_t clean_mask;
 	int err, cpu;
 	u8 i;
 
@@ -5073,6 +5085,7 @@ static int __cold dpaa2_dpseci_setup(struct fsl_mc_device *ls_dev)
 		}
 	}
 
+	cpumask_clear(&clean_mask);
 	i = 0;
 	for_each_online_cpu(cpu) {
 		u8 j;
@@ -5096,15 +5109,23 @@ static int __cold dpaa2_dpseci_setup(struct fsl_mc_device *ls_dev)
 			priv->rx_queue_attr[j].fqid,
 			priv->tx_queue_attr[j].fqid);
 
-		ppriv->net_dev.dev = *dev;
-		INIT_LIST_HEAD(&ppriv->net_dev.napi_list);
-		netif_napi_add_tx_weight(&ppriv->net_dev, &ppriv->napi,
+		ppriv->net_dev = alloc_netdev_dummy(0);
+		if (!ppriv->net_dev) {
+			err = -ENOMEM;
+			goto err_alloc_netdev;
+		}
+		cpumask_set_cpu(cpu, &clean_mask);
+		ppriv->net_dev->dev = *dev;
+
+		netif_napi_add_tx_weight(ppriv->net_dev, &ppriv->napi,
 					 dpaa2_dpseci_poll,
 					 DPAA2_CAAM_NAPI_WEIGHT);
 	}
 
 	return 0;
 
+err_alloc_netdev:
+	free_dpaa2_pcpu_netdev(priv, &clean_mask);
 err_get_rx_queue:
 	dpaa2_dpseci_congestion_free(priv);
 err_get_vers:
@@ -5153,6 +5174,7 @@ static int __cold dpaa2_dpseci_disable(struct dpaa2_caam_priv *priv)
 		ppriv = per_cpu_ptr(priv->ppriv, i);
 		napi_disable(&ppriv->napi);
 		netif_napi_del(&ppriv->napi);
+		free_netdev(ppriv->net_dev);
 	}
 
 	return 0;
diff --git a/drivers/crypto/caam/caamalg_qi2.h b/drivers/crypto/caam/caamalg_qi2.h
index abb502bb675c..61d1219a202f 100644
--- a/drivers/crypto/caam/caamalg_qi2.h
+++ b/drivers/crypto/caam/caamalg_qi2.h
@@ -81,7 +81,7 @@ struct dpaa2_caam_priv {
  */
 struct dpaa2_caam_priv_per_cpu {
 	struct napi_struct napi;
-	struct net_device net_dev;
+	struct net_device *net_dev;
 	int req_fqid;
 	int rsp_fqid;
 	int prio;
-- 
2.43.0

Re: [PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST

[Jakub Kicinski]

On Mon, 24 Jun 2024 09:21:19 -0700 Breno Leitao wrote:
> As most of the drivers that depend on ARCH_LAYERSCAPE, make FSL_DPAA
> depend on COMPILE_TEST for compilation and testing.
> 
> 	# grep -r depends.\*ARCH_LAYERSCAPE.\*COMPILE_TEST | wc -l
> 	29

Cover letter would be good..

Herbert, (Pankaj | Gaurav | Horia) - no rush but once reviewed can we
take this via netdev (or a shared branch)? As Breno linked we want to
change the netdev allocation API, this is the last chunk of drivers
we need to convert.

Re: [PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST

[Herbert Xu]

On Tue, Jun 25, 2024 at 07:39:26AM -0700, Jakub Kicinski wrote:
> On Mon, 24 Jun 2024 09:21:19 -0700 Breno Leitao wrote:
> > As most of the drivers that depend on ARCH_LAYERSCAPE, make FSL_DPAA
> > depend on COMPILE_TEST for compilation and testing.
> > 
> > 	# grep -r depends.\*ARCH_LAYERSCAPE.\*COMPILE_TEST | wc -l
> > 	29
> 
> Cover letter would be good..
> 
> Herbert, (Pankaj | Gaurav | Horia) - no rush but once reviewed can we
> take this via netdev (or a shared branch)? As Breno linked we want to
> change the netdev allocation API, this is the last chunk of drivers
> we need to convert.

Sure, please feel free to take this via netdev.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST

[kernel test robot]

Hi Breno,

kernel test robot noticed the following build warnings:

[auto build test WARNING on herbert-cryptodev-2.6/master]
[also build test WARNING on soc/for-next linus/master v6.10-rc5 next-20240625]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Breno-Leitao/crypto-caam-Depend-on-COMPILE_TEST-also/20240625-223834
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
patch link:    https://lore.kernel.org/r/20240624162128.1665620-1-leitao%40debian.org
patch subject: [PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20240626/202406261920.l5pzM1rj-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240626/202406261920.l5pzM1rj-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406261920.l5pzM1rj-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c:3280:12: warning: stack frame size (16664) exceeds limit (2048) in 'dpaa_eth_probe' [-Wframe-larger-than]
    3280 | static int dpaa_eth_probe(struct platform_device *pdev)
         |            ^
   1 warning generated.
--
>> drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c:454:12: warning: stack frame size (8264) exceeds limit (2048) in 'dpaa_set_coalesce' [-Wframe-larger-than]
     454 | static int dpaa_set_coalesce(struct net_device *dev,
         |            ^
   1 warning generated.


vim +/dpaa_eth_probe +3280 drivers/net/ethernet/freescale/dpaa/dpaa_eth.c

9ad1a37493338c Madalin Bucur   2016-11-15  3279  
9ad1a37493338c Madalin Bucur   2016-11-15 @3280  static int dpaa_eth_probe(struct platform_device *pdev)
9ad1a37493338c Madalin Bucur   2016-11-15  3281  {
9ad1a37493338c Madalin Bucur   2016-11-15  3282  	struct net_device *net_dev = NULL;
f07f30042f8e0f Madalin Bucur   2019-10-31  3283  	struct dpaa_bp *dpaa_bp = NULL;
9ad1a37493338c Madalin Bucur   2016-11-15  3284  	struct dpaa_fq *dpaa_fq, *tmp;
9ad1a37493338c Madalin Bucur   2016-11-15  3285  	struct dpaa_priv *priv = NULL;
9ad1a37493338c Madalin Bucur   2016-11-15  3286  	struct fm_port_fqs port_fqs;
9ad1a37493338c Madalin Bucur   2016-11-15  3287  	struct mac_device *mac_dev;
f07f30042f8e0f Madalin Bucur   2019-10-31  3288  	int err = 0, channel;
9ad1a37493338c Madalin Bucur   2016-11-15  3289  	struct device *dev;
9ad1a37493338c Madalin Bucur   2016-11-15  3290  
060ad66f97954f Madalin Bucur   2019-10-23  3291  	dev = &pdev->dev;
060ad66f97954f Madalin Bucur   2019-10-23  3292  
5537b329857676 Laurentiu Tudor 2019-10-23  3293  	err = bman_is_probed();
5537b329857676 Laurentiu Tudor 2019-10-23  3294  	if (!err)
5537b329857676 Laurentiu Tudor 2019-10-23  3295  		return -EPROBE_DEFER;
5537b329857676 Laurentiu Tudor 2019-10-23  3296  	if (err < 0) {
060ad66f97954f Madalin Bucur   2019-10-23  3297  		dev_err(dev, "failing probe due to bman probe error\n");
5537b329857676 Laurentiu Tudor 2019-10-23  3298  		return -ENODEV;
5537b329857676 Laurentiu Tudor 2019-10-23  3299  	}
5537b329857676 Laurentiu Tudor 2019-10-23  3300  	err = qman_is_probed();
5537b329857676 Laurentiu Tudor 2019-10-23  3301  	if (!err)
5537b329857676 Laurentiu Tudor 2019-10-23  3302  		return -EPROBE_DEFER;
5537b329857676 Laurentiu Tudor 2019-10-23  3303  	if (err < 0) {
060ad66f97954f Madalin Bucur   2019-10-23  3304  		dev_err(dev, "failing probe due to qman probe error\n");
5537b329857676 Laurentiu Tudor 2019-10-23  3305  		return -ENODEV;
5537b329857676 Laurentiu Tudor 2019-10-23  3306  	}
5537b329857676 Laurentiu Tudor 2019-10-23  3307  	err = bman_portals_probed();
5537b329857676 Laurentiu Tudor 2019-10-23  3308  	if (!err)
5537b329857676 Laurentiu Tudor 2019-10-23  3309  		return -EPROBE_DEFER;
5537b329857676 Laurentiu Tudor 2019-10-23  3310  	if (err < 0) {
060ad66f97954f Madalin Bucur   2019-10-23  3311  		dev_err(dev,
5537b329857676 Laurentiu Tudor 2019-10-23  3312  			"failing probe due to bman portals probe error\n");
5537b329857676 Laurentiu Tudor 2019-10-23  3313  		return -ENODEV;
5537b329857676 Laurentiu Tudor 2019-10-23  3314  	}
5537b329857676 Laurentiu Tudor 2019-10-23  3315  	err = qman_portals_probed();
5537b329857676 Laurentiu Tudor 2019-10-23  3316  	if (!err)
5537b329857676 Laurentiu Tudor 2019-10-23  3317  		return -EPROBE_DEFER;
5537b329857676 Laurentiu Tudor 2019-10-23  3318  	if (err < 0) {
060ad66f97954f Madalin Bucur   2019-10-23  3319  		dev_err(dev,
5537b329857676 Laurentiu Tudor 2019-10-23  3320  			"failing probe due to qman portals probe error\n");
5537b329857676 Laurentiu Tudor 2019-10-23  3321  		return -ENODEV;
5537b329857676 Laurentiu Tudor 2019-10-23  3322  	}
5537b329857676 Laurentiu Tudor 2019-10-23  3323  
9ad1a37493338c Madalin Bucur   2016-11-15  3324  	/* Allocate this early, so we can store relevant information in
9ad1a37493338c Madalin Bucur   2016-11-15  3325  	 * the private area
9ad1a37493338c Madalin Bucur   2016-11-15  3326  	 */
9ad1a37493338c Madalin Bucur   2016-11-15  3327  	net_dev = alloc_etherdev_mq(sizeof(*priv), DPAA_ETH_TXQ_NUM);
9ad1a37493338c Madalin Bucur   2016-11-15  3328  	if (!net_dev) {
9ad1a37493338c Madalin Bucur   2016-11-15  3329  		dev_err(dev, "alloc_etherdev_mq() failed\n");
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3330  		return -ENOMEM;
9ad1a37493338c Madalin Bucur   2016-11-15  3331  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3332  
9ad1a37493338c Madalin Bucur   2016-11-15  3333  	/* Do this here, so we can be verbose early */
5d14c304bfc14b Vladimir Oltean 2020-05-25  3334  	SET_NETDEV_DEV(net_dev, dev->parent);
9ad1a37493338c Madalin Bucur   2016-11-15  3335  	dev_set_drvdata(dev, net_dev);
9ad1a37493338c Madalin Bucur   2016-11-15  3336  
9ad1a37493338c Madalin Bucur   2016-11-15  3337  	priv = netdev_priv(net_dev);
9ad1a37493338c Madalin Bucur   2016-11-15  3338  	priv->net_dev = net_dev;
9ad1a37493338c Madalin Bucur   2016-11-15  3339  
9ad1a37493338c Madalin Bucur   2016-11-15  3340  	priv->msg_enable = netif_msg_init(debug, DPAA_MSG_DEFAULT);
9ad1a37493338c Madalin Bucur   2016-11-15  3341  
9ad1a37493338c Madalin Bucur   2016-11-15  3342  	mac_dev = dpaa_mac_dev_get(pdev);
9ad1a37493338c Madalin Bucur   2016-11-15  3343  	if (IS_ERR(mac_dev)) {
060ad66f97954f Madalin Bucur   2019-10-23  3344  		netdev_err(net_dev, "dpaa_mac_dev_get() failed\n");
9ad1a37493338c Madalin Bucur   2016-11-15  3345  		err = PTR_ERR(mac_dev);
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3346  		goto free_netdev;
9ad1a37493338c Madalin Bucur   2016-11-15  3347  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3348  
060ad66f97954f Madalin Bucur   2019-10-23  3349  	/* Devices used for DMA mapping */
060ad66f97954f Madalin Bucur   2019-10-23  3350  	priv->rx_dma_dev = fman_port_get_device(mac_dev->port[RX]);
060ad66f97954f Madalin Bucur   2019-10-23  3351  	priv->tx_dma_dev = fman_port_get_device(mac_dev->port[TX]);
060ad66f97954f Madalin Bucur   2019-10-23  3352  	err = dma_coerce_mask_and_coherent(priv->rx_dma_dev, DMA_BIT_MASK(40));
060ad66f97954f Madalin Bucur   2019-10-23  3353  	if (!err)
060ad66f97954f Madalin Bucur   2019-10-23  3354  		err = dma_coerce_mask_and_coherent(priv->tx_dma_dev,
060ad66f97954f Madalin Bucur   2019-10-23  3355  						   DMA_BIT_MASK(40));
060ad66f97954f Madalin Bucur   2019-10-23  3356  	if (err) {
060ad66f97954f Madalin Bucur   2019-10-23  3357  		netdev_err(net_dev, "dma_coerce_mask_and_coherent() failed\n");
6790711f8ac5fa Liu Jian        2020-07-20  3358  		goto free_netdev;
060ad66f97954f Madalin Bucur   2019-10-23  3359  	}
060ad66f97954f Madalin Bucur   2019-10-23  3360  
9ad1a37493338c Madalin Bucur   2016-11-15  3361  	/* If fsl_fm_max_frm is set to a higher value than the all-common 1500,
9ad1a37493338c Madalin Bucur   2016-11-15  3362  	 * we choose conservatively and let the user explicitly set a higher
9ad1a37493338c Madalin Bucur   2016-11-15  3363  	 * MTU via ifconfig. Otherwise, the user may end up with different MTUs
9ad1a37493338c Madalin Bucur   2016-11-15  3364  	 * in the same LAN.
9ad1a37493338c Madalin Bucur   2016-11-15  3365  	 * If on the other hand fsl_fm_max_frm has been chosen below 1500,
9ad1a37493338c Madalin Bucur   2016-11-15  3366  	 * start with the maximum allowed.
9ad1a37493338c Madalin Bucur   2016-11-15  3367  	 */
9ad1a37493338c Madalin Bucur   2016-11-15  3368  	net_dev->mtu = min(dpaa_get_max_mtu(), ETH_DATA_LEN);
9ad1a37493338c Madalin Bucur   2016-11-15  3369  
9ad1a37493338c Madalin Bucur   2016-11-15  3370  	netdev_dbg(net_dev, "Setting initial MTU on net device: %d\n",
9ad1a37493338c Madalin Bucur   2016-11-15  3371  		   net_dev->mtu);
9ad1a37493338c Madalin Bucur   2016-11-15  3372  
9ad1a37493338c Madalin Bucur   2016-11-15  3373  	priv->buf_layout[RX].priv_data_size = DPAA_RX_PRIV_DATA_SIZE; /* Rx */
9ad1a37493338c Madalin Bucur   2016-11-15  3374  	priv->buf_layout[TX].priv_data_size = DPAA_TX_PRIV_DATA_SIZE; /* Tx */
9ad1a37493338c Madalin Bucur   2016-11-15  3375  
9ad1a37493338c Madalin Bucur   2016-11-15  3376  	/* bp init */
f07f30042f8e0f Madalin Bucur   2019-10-31  3377  	dpaa_bp = dpaa_bp_alloc(dev);
f07f30042f8e0f Madalin Bucur   2019-10-31  3378  	if (IS_ERR(dpaa_bp)) {
f07f30042f8e0f Madalin Bucur   2019-10-31  3379  		err = PTR_ERR(dpaa_bp);
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3380  		goto free_dpaa_bps;
29130853fe6dee Wei Yongjun     2017-11-06  3381  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3382  	/* the raw size of the buffers used for reception */
f07f30042f8e0f Madalin Bucur   2019-10-31  3383  	dpaa_bp->raw_size = DPAA_BP_RAW_SIZE;
9ad1a37493338c Madalin Bucur   2016-11-15  3384  	/* avoid runtime computations by keeping the usable size here */
f07f30042f8e0f Madalin Bucur   2019-10-31  3385  	dpaa_bp->size = dpaa_bp_size(dpaa_bp->raw_size);
f07f30042f8e0f Madalin Bucur   2019-10-31  3386  	dpaa_bp->priv = priv;
9ad1a37493338c Madalin Bucur   2016-11-15  3387  
f07f30042f8e0f Madalin Bucur   2019-10-31  3388  	err = dpaa_bp_alloc_pool(dpaa_bp);
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3389  	if (err < 0)
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3390  		goto free_dpaa_bps;
f07f30042f8e0f Madalin Bucur   2019-10-31  3391  	priv->dpaa_bp = dpaa_bp;
9ad1a37493338c Madalin Bucur   2016-11-15  3392  
9ad1a37493338c Madalin Bucur   2016-11-15  3393  	INIT_LIST_HEAD(&priv->dpaa_fq_list);
9ad1a37493338c Madalin Bucur   2016-11-15  3394  
9ad1a37493338c Madalin Bucur   2016-11-15  3395  	memset(&port_fqs, 0, sizeof(port_fqs));
9ad1a37493338c Madalin Bucur   2016-11-15  3396  
9ad1a37493338c Madalin Bucur   2016-11-15  3397  	err = dpaa_alloc_all_fqs(dev, &priv->dpaa_fq_list, &port_fqs);
9ad1a37493338c Madalin Bucur   2016-11-15  3398  	if (err < 0) {
9ad1a37493338c Madalin Bucur   2016-11-15  3399  		dev_err(dev, "dpaa_alloc_all_fqs() failed\n");
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3400  		goto free_dpaa_bps;
9ad1a37493338c Madalin Bucur   2016-11-15  3401  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3402  
9ad1a37493338c Madalin Bucur   2016-11-15  3403  	priv->mac_dev = mac_dev;
9ad1a37493338c Madalin Bucur   2016-11-15  3404  
9ad1a37493338c Madalin Bucur   2016-11-15  3405  	channel = dpaa_get_channel();
9ad1a37493338c Madalin Bucur   2016-11-15  3406  	if (channel < 0) {
9ad1a37493338c Madalin Bucur   2016-11-15  3407  		dev_err(dev, "dpaa_get_channel() failed\n");
9ad1a37493338c Madalin Bucur   2016-11-15  3408  		err = channel;
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3409  		goto free_dpaa_bps;
9ad1a37493338c Madalin Bucur   2016-11-15  3410  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3411  
9ad1a37493338c Madalin Bucur   2016-11-15  3412  	priv->channel = (u16)channel;
9ad1a37493338c Madalin Bucur   2016-11-15  3413  
d75de7b6e73714 Jake Moroni     2018-02-12  3414  	/* Walk the CPUs with affine portals
9ad1a37493338c Madalin Bucur   2016-11-15  3415  	 * and add this pool channel to each's dequeue mask.
9ad1a37493338c Madalin Bucur   2016-11-15  3416  	 */
e06eea555b878f Madalin Bucur   2019-10-31  3417  	dpaa_eth_add_channel(priv->channel, &pdev->dev);
9ad1a37493338c Madalin Bucur   2016-11-15  3418  
9ad1a37493338c Madalin Bucur   2016-11-15  3419  	dpaa_fq_setup(priv, &dpaa_fq_cbs, priv->mac_dev->port[TX]);
9ad1a37493338c Madalin Bucur   2016-11-15  3420  
9ad1a37493338c Madalin Bucur   2016-11-15  3421  	/* Create a congestion group for this netdev, with
9ad1a37493338c Madalin Bucur   2016-11-15  3422  	 * dynamically-allocated CGR ID.
9ad1a37493338c Madalin Bucur   2016-11-15  3423  	 * Must be executed after probing the MAC, but before
9ad1a37493338c Madalin Bucur   2016-11-15  3424  	 * assigning the egress FQs to the CGRs.
9ad1a37493338c Madalin Bucur   2016-11-15  3425  	 */
9ad1a37493338c Madalin Bucur   2016-11-15  3426  	err = dpaa_eth_cgr_init(priv);
9ad1a37493338c Madalin Bucur   2016-11-15  3427  	if (err < 0) {
9ad1a37493338c Madalin Bucur   2016-11-15  3428  		dev_err(dev, "Error initializing CGR\n");
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3429  		goto free_dpaa_bps;
9ad1a37493338c Madalin Bucur   2016-11-15  3430  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3431  
9ad1a37493338c Madalin Bucur   2016-11-15  3432  	err = dpaa_ingress_cgr_init(priv);
9ad1a37493338c Madalin Bucur   2016-11-15  3433  	if (err < 0) {
9ad1a37493338c Madalin Bucur   2016-11-15  3434  		dev_err(dev, "Error initializing ingress CGR\n");
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3435  		goto delete_egress_cgr;
9ad1a37493338c Madalin Bucur   2016-11-15  3436  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3437  
9ad1a37493338c Madalin Bucur   2016-11-15  3438  	/* Add the FQs to the interface, and make them active */
9ad1a37493338c Madalin Bucur   2016-11-15  3439  	list_for_each_entry_safe(dpaa_fq, tmp, &priv->dpaa_fq_list, list) {
9ad1a37493338c Madalin Bucur   2016-11-15  3440  		err = dpaa_fq_init(dpaa_fq, false);
9ad1a37493338c Madalin Bucur   2016-11-15  3441  		if (err < 0)
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3442  			goto free_dpaa_fqs;
9ad1a37493338c Madalin Bucur   2016-11-15  3443  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3444  
7834e494f42627 Camelia Groza   2020-11-02  3445  	priv->tx_headroom = dpaa_get_headroom(priv->buf_layout, TX);
7834e494f42627 Camelia Groza   2020-11-02  3446  	priv->rx_headroom = dpaa_get_headroom(priv->buf_layout, RX);
9ad1a37493338c Madalin Bucur   2016-11-15  3447  
9ad1a37493338c Madalin Bucur   2016-11-15  3448  	/* All real interfaces need their ports initialized */
f07f30042f8e0f Madalin Bucur   2019-10-31  3449  	err = dpaa_eth_init_ports(mac_dev, dpaa_bp, &port_fqs,
9ad1a37493338c Madalin Bucur   2016-11-15  3450  				  &priv->buf_layout[0], dev);
7f8a6a1b8fa491 Madalin Bucur   2017-02-13  3451  	if (err)
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3452  		goto free_dpaa_fqs;
9ad1a37493338c Madalin Bucur   2016-11-15  3453  
056057e288e707 Madalin Bucur   2017-08-27  3454  	/* Rx traffic distribution based on keygen hashing defaults to on */
056057e288e707 Madalin Bucur   2017-08-27  3455  	priv->keygen_in_use = true;
056057e288e707 Madalin Bucur   2017-08-27  3456  
9ad1a37493338c Madalin Bucur   2016-11-15  3457  	priv->percpu_priv = devm_alloc_percpu(dev, *priv->percpu_priv);
9ad1a37493338c Madalin Bucur   2016-11-15  3458  	if (!priv->percpu_priv) {
9ad1a37493338c Madalin Bucur   2016-11-15  3459  		dev_err(dev, "devm_alloc_percpu() failed\n");
9ad1a37493338c Madalin Bucur   2016-11-15  3460  		err = -ENOMEM;
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3461  		goto free_dpaa_fqs;
9ad1a37493338c Madalin Bucur   2016-11-15  3462  	}
9ad1a37493338c Madalin Bucur   2016-11-15  3463  
c44efa1d75e4c0 Camelia Groza   2016-07-25  3464  	priv->num_tc = 1;
c44efa1d75e4c0 Camelia Groza   2016-07-25  3465  	netif_set_real_num_tx_queues(net_dev, priv->num_tc * DPAA_TC_TXQ_NUM);
c44efa1d75e4c0 Camelia Groza   2016-07-25  3466  
9ad1a37493338c Madalin Bucur   2016-11-15  3467  	/* Initialize NAPI */
9ad1a37493338c Madalin Bucur   2016-11-15  3468  	err = dpaa_napi_add(net_dev);
9ad1a37493338c Madalin Bucur   2016-11-15  3469  	if (err < 0)
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3470  		goto delete_dpaa_napi;
9ad1a37493338c Madalin Bucur   2016-11-15  3471  
9ad1a37493338c Madalin Bucur   2016-11-15  3472  	err = dpaa_netdev_init(net_dev, &dpaa_ops, tx_timeout);
9ad1a37493338c Madalin Bucur   2016-11-15  3473  	if (err < 0)
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3474  		goto delete_dpaa_napi;
9ad1a37493338c Madalin Bucur   2016-11-15  3475  
846a86e20123b0 Madalin Bucur   2016-11-15  3476  	dpaa_eth_sysfs_init(&net_dev->dev);
846a86e20123b0 Madalin Bucur   2016-11-15  3477  
9ad1a37493338c Madalin Bucur   2016-11-15  3478  	netif_info(priv, probe, net_dev, "Probed interface %s\n",
9ad1a37493338c Madalin Bucur   2016-11-15  3479  		   net_dev->name);
9ad1a37493338c Madalin Bucur   2016-11-15  3480  
9ad1a37493338c Madalin Bucur   2016-11-15  3481  	return 0;
9ad1a37493338c Madalin Bucur   2016-11-15  3482  
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3483  delete_dpaa_napi:
9ad1a37493338c Madalin Bucur   2016-11-15  3484  	dpaa_napi_del(net_dev);
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3485  free_dpaa_fqs:
9ad1a37493338c Madalin Bucur   2016-11-15  3486  	dpaa_fq_free(dev, &priv->dpaa_fq_list);
9ad1a37493338c Madalin Bucur   2016-11-15  3487  	qman_delete_cgr_safe(&priv->ingress_cgr);
9ad1a37493338c Madalin Bucur   2016-11-15  3488  	qman_release_cgrid(priv->ingress_cgr.cgrid);
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3489  delete_egress_cgr:
9ad1a37493338c Madalin Bucur   2016-11-15  3490  	qman_delete_cgr_safe(&priv->cgr_data.cgr);
9ad1a37493338c Madalin Bucur   2016-11-15  3491  	qman_release_cgrid(priv->cgr_data.cgr.cgrid);
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3492  free_dpaa_bps:
9ad1a37493338c Madalin Bucur   2016-11-15  3493  	dpaa_bps_free(priv);
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3494  free_netdev:
9ad1a37493338c Madalin Bucur   2016-11-15  3495  	dev_set_drvdata(dev, NULL);
9ad1a37493338c Madalin Bucur   2016-11-15  3496  	free_netdev(net_dev);
8b9b5a2c27e1a7 Madalin Bucur   2017-10-16  3497  
9ad1a37493338c Madalin Bucur   2016-11-15  3498  	return err;
9ad1a37493338c Madalin Bucur   2016-11-15  3499  }
9ad1a37493338c Madalin Bucur   2016-11-15  3500  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST

[Vladimir Oltean]

On Wed, Jun 26, 2024 at 08:09:53PM +0800, kernel test robot wrote:
> Hi Breno,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on herbert-cryptodev-2.6/master]
> [also build test WARNING on soc/for-next linus/master v6.10-rc5 next-20240625]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Breno-Leitao/crypto-caam-Depend-on-COMPILE_TEST-also/20240625-223834
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
> patch link:    https://lore.kernel.org/r/20240624162128.1665620-1-leitao%40debian.org
> patch subject: [PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST
> config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20240626/202406261920.l5pzM1rj-lkp@intel.com/config)
> compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240626/202406261920.l5pzM1rj-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202406261920.l5pzM1rj-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
> >> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c:3280:12: warning: stack frame size (16664) exceeds limit (2048) in 'dpaa_eth_probe' [-Wframe-larger-than]
>     3280 | static int dpaa_eth_probe(struct platform_device *pdev)
>          |            ^
>    1 warning generated.
> --
> >> drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c:454:12: warning: stack frame size (8264) exceeds limit (2048) in 'dpaa_set_coalesce' [-Wframe-larger-than]
>      454 | static int dpaa_set_coalesce(struct net_device *dev,
>          |            ^
>    1 warning generated.

Arrays of NR_CPUS elements are what it probably doesn't like?
In the attached Kconfig, CONFIG_NR_CPUS is 8192, which is clearly
excessive compared to the SoCs that the driver is written for and
expects to run on (1-24 cores).

static int dpaa_set_coalesce(struct net_device *dev,
			     struct ethtool_coalesce *c,
			     struct kernel_ethtool_coalesce *kernel_coal,
			     struct netlink_ext_ack *extack)
{
	const cpumask_t *cpus = qman_affine_cpus();
	bool needs_revert[NR_CPUS] = {false};
	...
}

static void dpaa_fq_setup(struct dpaa_priv *priv,
			  const struct dpaa_fq_cbs *fq_cbs,
			  struct fman_port *tx_port)
{
	int egress_cnt = 0, conf_cnt = 0, num_portals = 0, portal_cnt = 0, cpu;
	const cpumask_t *affine_cpus = qman_affine_cpus();
	u16 channels[NR_CPUS];
	...
}

While 'needs_revert' can probably easily be converted to a bitmask which
consumes 8 times less space, I don't know what to say about the "channels"
array. It could probably be rewritten to use dynamic allocation for the
array. I don't have any better idea...

Re: [PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST

[Breno Leitao]

Hello Vladimir,

On Wed, Jun 26, 2024 at 05:06:23PM +0300, Vladimir Oltean wrote:
> On Wed, Jun 26, 2024 at 08:09:53PM +0800, kernel test robot wrote:

> > All warnings (new ones prefixed by >>):
> > 
> > >> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c:3280:12: warning: stack frame size (16664) exceeds limit (2048) in 'dpaa_eth_probe' [-Wframe-larger-than]
> >     3280 | static int dpaa_eth_probe(struct platform_device *pdev)
> >          |            ^
> >    1 warning generated.
> > --
> > >> drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c:454:12: warning: stack frame size (8264) exceeds limit (2048) in 'dpaa_set_coalesce' [-Wframe-larger-than]
> >      454 | static int dpaa_set_coalesce(struct net_device *dev,
> >          |            ^
> >    1 warning generated.
> 
> Arrays of NR_CPUS elements are what it probably doesn't like?

Can it use the number of online CPUs instead of NR_CPUS?

Other than that, I would say we can drop this patch in the meantime, so,
we can move with the others, while this one is being addressed.

Re: [PATCH 3/4] crypto: caam: Unembed net_dev structure from qi

[Simon Horman]

On Mon, Jun 24, 2024 at 09:21:21AM -0700, Breno Leitao wrote:
> Embedding net_device into structures prohibits the usage of flexible
> arrays in the net_device structure. For more details, see the discussion
> at [1].
> 
> Un-embed the net_devices from struct caam_qi_pcpu_priv by converting them
> into pointers, and allocating them dynamically. Use the leverage
> alloc_netdev_dummy() to allocate the net_device object at
> caam_qi_init().
> 
> The free of the device occurs at caam_qi_shutdown().
> 
> Link: https://lore.kernel.org/all/20240229225910.79e224cf@kernel.org/ [1]
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> PS: Unfortunately due to lack of hardware, this was not tested in real
> hardware.
> 
>  drivers/crypto/caam/qi.c | 43 ++++++++++++++++++++++++++++++++--------
>  1 file changed, 35 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/crypto/caam/qi.c b/drivers/crypto/caam/qi.c

...

> @@ -530,6 +530,7 @@ static void caam_qi_shutdown(void *data)
>  
>  		if (kill_fq(qidev, per_cpu(pcpu_qipriv.rsp_fq, i)))
>  			dev_err(qidev, "Rsp FQ kill failed, cpu: %d\n", i);
> +		free_netdev(pcpu_qipriv.net_dev);

Hi Breno,

I don't think you can access pcpu_qipriv.net_dev like this,
as pcpu_qipriv is a per-cpu variable. Perhaps this?

	free_netdev(per_cpu(pcpu_qipriv.net_dev, i));

Flagged by Sparse.

>  	}
>  
>  	qman_delete_cgr_safe(&priv->cgr);

...

Re: [PATCH 3/4] crypto: caam: Unembed net_dev structure from qi

[Breno Leitao]

Hello Simon,

On Fri, Jun 28, 2024 at 05:32:26PM +0100, Simon Horman wrote:
> On Mon, Jun 24, 2024 at 09:21:21AM -0700, Breno Leitao wrote:

> > @@ -530,6 +530,7 @@ static void caam_qi_shutdown(void *data)
> >  
> >  		if (kill_fq(qidev, per_cpu(pcpu_qipriv.rsp_fq, i)))
> >  			dev_err(qidev, "Rsp FQ kill failed, cpu: %d\n", i);
> > +		free_netdev(pcpu_qipriv.net_dev);

> Hi Breno,
> 
> I don't think you can access pcpu_qipriv.net_dev like this,
> as pcpu_qipriv is a per-cpu variable. Perhaps this?
> 
> 	free_netdev(per_cpu(pcpu_qipriv.net_dev, i));

You are absolutely correct. Let me fix it.

> Flagged by Sparse.

Thanks. I've just added sparse to my development workflow, and I can see
it also:

	drivers/crypto/caam/qi.c:533:29: warning: dereference of noderef expression

Thanks for the review. I will send an updated version.

--breno

Re: [PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST

[Vladimir Oltean]

On Thu, Jun 27, 2024 at 11:40:24AM -0700, Breno Leitao wrote:
> Hello Vladimir,
> 
> On Wed, Jun 26, 2024 at 05:06:23PM +0300, Vladimir Oltean wrote:
> > On Wed, Jun 26, 2024 at 08:09:53PM +0800, kernel test robot wrote:
> 
> > > All warnings (new ones prefixed by >>):
> > > 
> > > >> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c:3280:12: warning: stack frame size (16664) exceeds limit (2048) in 'dpaa_eth_probe' [-Wframe-larger-than]
> > >     3280 | static int dpaa_eth_probe(struct platform_device *pdev)
> > >          |            ^
> > >    1 warning generated.
> > > --
> > > >> drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c:454:12: warning: stack frame size (8264) exceeds limit (2048) in 'dpaa_set_coalesce' [-Wframe-larger-than]
> > >      454 | static int dpaa_set_coalesce(struct net_device *dev,
> > >          |            ^
> > >    1 warning generated.
> > 
> > Arrays of NR_CPUS elements are what it probably doesn't like?
> 
> Can it use the number of online CPUs instead of NR_CPUS?

I don't see how, given that variable length arrays are something which
should be avoided in the kernel?

Re: [PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST

[Breno Leitao]

Hello Vladimir,

On Mon, Jul 08, 2024 at 04:37:46PM +0300, Vladimir Oltean wrote:
> On Thu, Jun 27, 2024 at 11:40:24AM -0700, Breno Leitao wrote:

> > > >      454 | static int dpaa_set_coalesce(struct net_device *dev,
> > > >          |            ^
> > > >    1 warning generated.
> > > 
> > > Arrays of NR_CPUS elements are what it probably doesn't like?

> > Can it use the number of online CPUs instead of NR_CPUS?

> I don't see how, given that variable length arrays are something which
> should be avoided in the kernel?

I thought about a patch like the following (compile tested only). What
do you think?

	Author: Breno Leitao <leitao@debian.org>
	Date:   Mon Jul 8 11:57:33 2024 -0700

	    net: dpaa: Allocate only for online CPUs in dpaa_set_coalesce
	    
	    Currently, dpaa_set_coalesce allocates a boolean for every possible CPU
	    (NR_CPUS). This approach is suboptimal and causes failures in COMPILE_TEST.
	    For reference, see:
	    https://lore.kernel.org/all/202406261920.l5pzM1rj-lkp@intel.com/
	    
	    Modify the allocation to consider only online CPUs instead of
	    NR_CPUs. This change reduces the function's memory footprint and resolves
	    the COMPILE_TEST issues.
	    
	    Signed-off-by: Breno Leitao <leitao@debian.org>

	diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
	index 5bd0b36d1feb..7202a5310045 100644
	--- a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
	+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
	@@ -457,7 +457,7 @@ static int dpaa_set_coalesce(struct net_device *dev,
				     struct netlink_ext_ack *extack)
	 {
		const cpumask_t *cpus = qman_affine_cpus();
	-	bool needs_revert[NR_CPUS] = {false};
	+	bool *needs_revert;
		struct qman_portal *portal;
		u32 period, prev_period;
		u8 thresh, prev_thresh;
	@@ -466,6 +466,11 @@ static int dpaa_set_coalesce(struct net_device *dev,
		period = c->rx_coalesce_usecs;
		thresh = c->rx_max_coalesced_frames;
	 
	+	needs_revert = kmalloc_array(num_possible_cpus(), sizeof(bool), GFP_KERNEL);
	+	if (!needs_revert)
	+		return -ENOMEM;
	+	memset(needs_revert, 0, num_online_cpus() * sizeof(bool));
	+
		/* save previous values */
		portal = qman_get_affine_portal(smp_processor_id());
		qman_portal_get_iperiod(portal, &prev_period);
	@@ -498,6 +503,7 @@ static int dpaa_set_coalesce(struct net_device *dev,
			qman_dqrr_set_ithresh(portal, prev_thresh);
		}
	 
	+	kfree(needs_revert);
		return res;
	 }
	 

Re: [PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST

[Vladimir Oltean]

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

Hi Breno,

On Mon, Jul 08, 2024 at 12:08:05PM -0700, Breno Leitao wrote:
> I thought about a patch like the following (compile tested only). What
> do you think?

To be honest, there are several things I don't really like about this
patch.

- I really struggled with applying it in the current format. Could you
  please post the output of git format-patch in the future?
- You addressed dpaa_set_coalesce() but not also dpaa_fq_setup()
- You misrepresented the patch content by saying you only allocate size
  for online CPUs in the commit message. But you allocate for all
  possible CPUs.
- You only kfree(needs_revert) in the error (revert_values) case, but
  not in the normal (return 0) case.
- The netdev coding style is to sort the lines with variable
  declarations in reverse order of line length (they call this "reverse
  Christmas tree"). Your patch broke that order.
- You should use kcalloc() instead of kmalloc_array() + memset()

I have prepared and tested the attached alternative patch on a board and
I am preparing to submit it myself, if you don't have any objection.

Thanks,
Vladimir

[-- Attachment #2: 0001-net-dpaa-avoid-on-stack-arrays-of-NR_CPUS-elements.patch --]
[-- Type: text/x-diff, Size: 4199 bytes --]

From 00b942829ee283baa602011a05b02d18c6988171 Mon Sep 17 00:00:00 2001
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: Mon, 8 Jul 2024 11:57:33 -0700
Subject: [PATCH] net: dpaa: avoid on-stack arrays of NR_CPUS elements

The dpaa-eth driver is written for PowerPC and Arm SoCs which have 1-24
CPUs. It depends on CONFIG_NR_CPUS having a reasonably small value in
Kconfig. Otherwise, there are 2 functions which allocate on-stack arrays
of NR_CPUS elements, and these can quickly explode in size, leading to
warnings such as:

  drivers/net/ethernet/freescale/dpaa/dpaa_eth.c:3280:12: warning:
  stack frame size (16664) exceeds limit (2048) in 'dpaa_eth_probe' [-Wframe-larger-than]

The problem is twofold:
- Reducing the array size to the boot-time num_possible_cpus() (rather
  than the compile-time NR_CPUS) creates a variable-length array,
  avoidable in the Linux kernel.
- Using NR_CPUS as an array size makes the driver blow up in stack
  consumption with generic, as opposed to hand-crafted, .config files.

A simple solution is to use dynamic allocation for num_possible_cpus()
elements (aka a small number determined at runtime).

Link: https://lore.kernel.org/all/202406261920.l5pzM1rj-lkp@intel.com/
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 .../net/ethernet/freescale/dpaa/dpaa_eth.c    | 20 ++++++++++++++-----
 .../ethernet/freescale/dpaa/dpaa_ethtool.c    | 10 +++++++++-
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
index ddeb0a5f2317..c856b556929d 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
@@ -931,14 +931,18 @@ static inline void dpaa_setup_egress(const struct dpaa_priv *priv,
 	}
 }
 
-static void dpaa_fq_setup(struct dpaa_priv *priv,
-			  const struct dpaa_fq_cbs *fq_cbs,
-			  struct fman_port *tx_port)
+static int dpaa_fq_setup(struct dpaa_priv *priv,
+			 const struct dpaa_fq_cbs *fq_cbs,
+			 struct fman_port *tx_port)
 {
 	int egress_cnt = 0, conf_cnt = 0, num_portals = 0, portal_cnt = 0, cpu;
 	const cpumask_t *affine_cpus = qman_affine_cpus();
-	u16 channels[NR_CPUS];
 	struct dpaa_fq *fq;
+	u16 *channels;
+
+	channels = kcalloc(num_possible_cpus(), sizeof(u16), GFP_KERNEL);
+	if (!channels)
+		return -ENOMEM;
 
 	for_each_cpu_and(cpu, affine_cpus, cpu_online_mask)
 		channels[num_portals++] = qman_affine_channel(cpu);
@@ -997,6 +1001,10 @@ static void dpaa_fq_setup(struct dpaa_priv *priv,
 				break;
 		}
 	}
+
+	kfree(channels);
+
+	return 0;
 }
 
 static inline int dpaa_tx_fq_to_id(const struct dpaa_priv *priv,
@@ -3416,7 +3424,9 @@ static int dpaa_eth_probe(struct platform_device *pdev)
 	 */
 	dpaa_eth_add_channel(priv->channel, &pdev->dev);
 
-	dpaa_fq_setup(priv, &dpaa_fq_cbs, priv->mac_dev->port[TX]);
+	err = dpaa_fq_setup(priv, &dpaa_fq_cbs, priv->mac_dev->port[TX]);
+	if (err)
+		goto free_dpaa_bps;
 
 	/* Create a congestion group for this netdev, with
 	 * dynamically-allocated CGR ID.
diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
index 5bd0b36d1feb..3f8cd4a7d845 100644
--- a/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
+++ b/drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c
@@ -457,12 +457,16 @@ static int dpaa_set_coalesce(struct net_device *dev,
 			     struct netlink_ext_ack *extack)
 {
 	const cpumask_t *cpus = qman_affine_cpus();
-	bool needs_revert[NR_CPUS] = {false};
 	struct qman_portal *portal;
 	u32 period, prev_period;
 	u8 thresh, prev_thresh;
+	bool *needs_revert;
 	int cpu, res;
 
+	needs_revert = kcalloc(num_possible_cpus(), sizeof(bool), GFP_KERNEL);
+	if (!needs_revert)
+		return -ENOMEM;
+
 	period = c->rx_coalesce_usecs;
 	thresh = c->rx_max_coalesced_frames;
 
@@ -485,6 +489,8 @@ static int dpaa_set_coalesce(struct net_device *dev,
 		needs_revert[cpu] = true;
 	}
 
+	kfree(needs_revert);
+
 	return 0;
 
 revert_values:
@@ -498,6 +504,8 @@ static int dpaa_set_coalesce(struct net_device *dev,
 		qman_dqrr_set_ithresh(portal, prev_thresh);
 	}
 
+	kfree(needs_revert);
+
 	return res;
 }
 
-- 
2.34.1

Re: [PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST

[Breno Leitao]

Hello Vladimir,

On Tue, Jul 09, 2024 at 04:58:11PM +0300, Vladimir Oltean wrote:

> On Mon, Jul 08, 2024 at 12:08:05PM -0700, Breno Leitao wrote:
> > I thought about a patch like the following (compile tested only). What
> > do you think?
> 
> To be honest, there are several things I don't really like about this
> patch.
> 
> - I really struggled with applying it in the current format. Could you
>   please post the output of git format-patch in the future?

This is the output of `git format-patch` shifted right by a tab.

> - You addressed dpaa_set_coalesce() but not also dpaa_fq_setup()
> - You misrepresented the patch content by saying you only allocate size
>   for online CPUs in the commit message. But you allocate for all
>   possible CPUs.
> - You only kfree(needs_revert) in the error (revert_values) case, but
>   not in the normal (return 0) case.
> - The netdev coding style is to sort the lines with variable
>   declarations in reverse order of line length (they call this "reverse
>   Christmas tree"). Your patch broke that order.
> - You should use kcalloc() instead of kmalloc_array() + memset()
> 
> I have prepared and tested the attached alternative patch on a board and
> I am preparing to submit it myself, if you don't have any objection.

Sure, not a problem. You just asked how that would be possible, and I
decided to craft patch to show what I had in mind. I am glad we have a
way moving forward.

Thanks for solving it.

Re: [PATCH 1/4] soc: fsl: qbman: FSL_DPAA depends on COMPILE_TEST

[Vladimir Oltean]

On Tue, Jul 09, 2024 at 08:15:23AM -0700, Breno Leitao wrote:
> Hello Vladimir,
> 
> On Tue, Jul 09, 2024 at 04:58:11PM +0300, Vladimir Oltean wrote:
> 
> > On Mon, Jul 08, 2024 at 12:08:05PM -0700, Breno Leitao wrote:
> > > I thought about a patch like the following (compile tested only). What
> > > do you think?
> > 
> > To be honest, there are several things I don't really like about this
> > patch.
> > 
> > - I really struggled with applying it in the current format. Could you
> >   please post the output of git format-patch in the future?
> 
> This is the output of `git format-patch` shifted right by a tab.

I don't want to insist too much on it, but this is not correct. In the
process of shifting the patch to the right, something ate the leading
space on each patch context line. The patch is ill-formatted even if the
first tab is removed. Try to keep it simple and either attach it or post
it without any change whatsoever.

> > I have prepared and tested the attached alternative patch on a board and
> > I am preparing to submit it myself, if you don't have any objection.
> 
> Sure, not a problem. You just asked how that would be possible, and I
> decided to craft patch to show what I had in mind. I am glad we have a
> way moving forward.
> 
> Thanks for solving it.

I mean I did suggest dynamic allocation for the array since my first
reply in this thread, which is essentially what the patch is..
Given that dynamic allocation was already mentioned and then you
suggested to replace NR_CPUS with something dynamic, I took that as an
alternative proposal and an invitation at using VLAs, which is what I
was commenting on, and what I was saying I don't think would work.

By VLAs I mean:
-	u16 channels[NR_CPUS];
+	u16 channels[num_possible_cpus()];

Anyway...