diff mbox series

[v2] crypto: caam/qi2 - Add printing dpseci fq stats using debugfs

Message ID 20190719111821.21696-1-vakul.garg@nxp.com (mailing list archive)
State Superseded
Delegated to: Herbert Xu
Headers show
Series [v2] crypto: caam/qi2 - Add printing dpseci fq stats using debugfs | expand

Commit Message

Vakul Garg July 19, 2019, 11:23 a.m. UTC 
Add support of printing the dpseci frame queue statistics using debugfs.

Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
 drivers/crypto/caam/Kconfig          | 11 +++++
 drivers/crypto/caam/Makefile         |  1 +
 drivers/crypto/caam/caamalg_qi2.c    |  5 +++
 drivers/crypto/caam/caamalg_qi2.h    |  2 +
 drivers/crypto/caam/dpseci-debugfs.c | 80 ++++++++++++++++++++++++++++++++++++
 drivers/crypto/caam/dpseci-debugfs.h | 19 +++++++++
 6 files changed, 118 insertions(+)
 create mode 100644 drivers/crypto/caam/dpseci-debugfs.c
 create mode 100644 drivers/crypto/caam/dpseci-debugfs.h

Comments

Horia Geanta July 22, 2019, 2:25 p.m. UTC | #1 
On 7/19/2019 2:23 PM, Vakul Garg wrote:
[...]
> +if CRYPTO_DEV_FSL_DPAA2_CAAM
> +
> +config CRYPTO_DEV_FSL_DPAA2_CAAM_DEBUGFS
> +	depends on DEBUG_FS
> +	bool "Enable debugfs support"
> +	help
> +	  Selecting this will enable printing of various debug information
> +          in the DPAA2 CAAM driver.
> +
> +endif
Let's enable this based on CONFIG_DEBUG_FS.

> diff --git a/drivers/crypto/caam/Makefile b/drivers/crypto/caam/Makefile
> index 9ab4e81ea21e..e4e9fa481a44 100644
> --- a/drivers/crypto/caam/Makefile
> +++ b/drivers/crypto/caam/Makefile
> @@ -30,3 +30,4 @@ endif
>  obj-$(CONFIG_CRYPTO_DEV_FSL_DPAA2_CAAM) += dpaa2_caam.o
>  
>  dpaa2_caam-y    := caamalg_qi2.o dpseci.o
> +dpaa2_caam-$(CONFIG_CRYPTO_DEV_FSL_DPAA2_CAAM_DEBUGFS) += dpseci-debugfs.o
dpaa2_caam-$(CONFIG_DEBUG_FS)

[...]
> diff --git a/drivers/crypto/caam/caamalg_qi2.h b/drivers/crypto/caam/caamalg_qi2.h
> index 973f6296bc6f..b450e2a25c1f 100644
> --- a/drivers/crypto/caam/caamalg_qi2.h
> +++ b/drivers/crypto/caam/caamalg_qi2.h
> @@ -10,6 +10,7 @@
>  #include <soc/fsl/dpaa2-io.h>
>  #include <soc/fsl/dpaa2-fd.h>
>  #include <linux/threads.h>
> +#include <linux/netdevice.h>
How is this change related to current patch?

>  #include "dpseci.h"
>  #include "desc_constr.h"
>  
> @@ -64,6 +65,7 @@ struct dpaa2_caam_priv {
>  	struct iommu_domain *domain;
>  
>  	struct dpaa2_caam_priv_per_cpu __percpu *ppriv;
> +	struct dentry *dfs_root;
dfs_root is used only in dpseci-debugfs.c, let's have it there as global.

Horia
Vakul Garg July 22, 2019, 2:29 p.m. UTC | #2 
> -----Original Message-----
> From: Horia Geanta
> Sent: Monday, July 22, 2019 7:55 PM
> To: Vakul Garg <vakul.garg@nxp.com>; linux-crypto@vger.kernel.org
> Cc: Aymen Sghaier <aymen.sghaier@nxp.com>;
> herbert@gondor.apana.org.au
> Subject: Re: [PATCH v2] crypto: caam/qi2 - Add printing dpseci fq stats using
> debugfs
> 
> On 7/19/2019 2:23 PM, Vakul Garg wrote:
> [...]
> > +if CRYPTO_DEV_FSL_DPAA2_CAAM
> > +
> > +config CRYPTO_DEV_FSL_DPAA2_CAAM_DEBUGFS
> > +	depends on DEBUG_FS
> > +	bool "Enable debugfs support"
> > +	help
> > +	  Selecting this will enable printing of various debug information
> > +          in the DPAA2 CAAM driver.
> > +
> > +endif
> Let's enable this based on CONFIG_DEBUG_FS.

It is not clear to me.
Do you mean not have additional CRYPTO_DEV_FSL_DPAA2_CAAM_DEBUGFS?

> 
> > diff --git a/drivers/crypto/caam/Makefile
> > b/drivers/crypto/caam/Makefile index 9ab4e81ea21e..e4e9fa481a44
> 100644
> > --- a/drivers/crypto/caam/Makefile
> > +++ b/drivers/crypto/caam/Makefile
> > @@ -30,3 +30,4 @@ endif
> >  obj-$(CONFIG_CRYPTO_DEV_FSL_DPAA2_CAAM) += dpaa2_caam.o
> >
> >  dpaa2_caam-y    := caamalg_qi2.o dpseci.o
> > +dpaa2_caam-$(CONFIG_CRYPTO_DEV_FSL_DPAA2_CAAM_DEBUGFS) +=
> > +dpseci-debugfs.o
> dpaa2_caam-$(CONFIG_DEBUG_FS)
> 
> [...]
> > diff --git a/drivers/crypto/caam/caamalg_qi2.h
> > b/drivers/crypto/caam/caamalg_qi2.h
> > index 973f6296bc6f..b450e2a25c1f 100644
> > --- a/drivers/crypto/caam/caamalg_qi2.h
> > +++ b/drivers/crypto/caam/caamalg_qi2.h
> > @@ -10,6 +10,7 @@
> >  #include <soc/fsl/dpaa2-io.h>
> >  #include <soc/fsl/dpaa2-fd.h>
> >  #include <linux/threads.h>
> > +#include <linux/netdevice.h>
> How is this change related to current patch?
> 

It should have been here in first place because we have some napi related things in this file.
It is required as I got compilation errors now.

> >  #include "dpseci.h"
> >  #include "desc_constr.h"
> >
> > @@ -64,6 +65,7 @@ struct dpaa2_caam_priv {
> >  	struct iommu_domain *domain;
> >
> >  	struct dpaa2_caam_priv_per_cpu __percpu *ppriv;
> > +	struct dentry *dfs_root;
> dfs_root is used only in dpseci-debugfs.c, let's have it there as global.
> 
> Horia
Horia Geanta July 22, 2019, 3:20 p.m. UTC | #3 
On 7/22/2019 5:29 PM, Vakul Garg wrote:
>> -----Original Message-----
>> From: Horia Geanta
>> Sent: Monday, July 22, 2019 7:55 PM
>> To: Vakul Garg <vakul.garg@nxp.com>; linux-crypto@vger.kernel.org
>> Cc: Aymen Sghaier <aymen.sghaier@nxp.com>;
>> herbert@gondor.apana.org.au
>> Subject: Re: [PATCH v2] crypto: caam/qi2 - Add printing dpseci fq stats using
>> debugfs
>>
>> On 7/19/2019 2:23 PM, Vakul Garg wrote:
>> [...]
>>> +if CRYPTO_DEV_FSL_DPAA2_CAAM
>>> +
>>> +config CRYPTO_DEV_FSL_DPAA2_CAAM_DEBUGFS
>>> +	depends on DEBUG_FS
>>> +	bool "Enable debugfs support"
>>> +	help
>>> +	  Selecting this will enable printing of various debug information
>>> +          in the DPAA2 CAAM driver.
>>> +
>>> +endif
>> Let's enable this based on CONFIG_DEBUG_FS.
> 
> It is not clear to me.
> Do you mean not have additional CRYPTO_DEV_FSL_DPAA2_CAAM_DEBUGFS?
> 
Yes.

>>
>>> diff --git a/drivers/crypto/caam/Makefile
>>> b/drivers/crypto/caam/Makefile index 9ab4e81ea21e..e4e9fa481a44
>> 100644
>>> --- a/drivers/crypto/caam/Makefile
>>> +++ b/drivers/crypto/caam/Makefile
>>> @@ -30,3 +30,4 @@ endif
>>>  obj-$(CONFIG_CRYPTO_DEV_FSL_DPAA2_CAAM) += dpaa2_caam.o
>>>
>>>  dpaa2_caam-y    := caamalg_qi2.o dpseci.o
>>> +dpaa2_caam-$(CONFIG_CRYPTO_DEV_FSL_DPAA2_CAAM_DEBUGFS) +=
>>> +dpseci-debugfs.o
>> dpaa2_caam-$(CONFIG_DEBUG_FS)
>>
>> [...]
>>> diff --git a/drivers/crypto/caam/caamalg_qi2.h
>>> b/drivers/crypto/caam/caamalg_qi2.h
>>> index 973f6296bc6f..b450e2a25c1f 100644
>>> --- a/drivers/crypto/caam/caamalg_qi2.h
>>> +++ b/drivers/crypto/caam/caamalg_qi2.h
>>> @@ -10,6 +10,7 @@
>>>  #include <soc/fsl/dpaa2-io.h>
>>>  #include <soc/fsl/dpaa2-fd.h>
>>>  #include <linux/threads.h>
>>> +#include <linux/netdevice.h>
>> How is this change related to current patch?
>>
> 
> It should have been here in first place because we have some napi related things in this file.
> It is required as I got compilation errors now.
> 
Indeed, this is caused by including caamalg_qi2.h -> dpseci-debugfs.h ->
dpseci-debugfs.c.

Compiling this patch without the inclusion:

  CC      drivers/crypto/caam/dpseci-debugfs.o
In file included from drivers/crypto/caam/dpseci-debugfs.h:9:0,
                 from drivers/crypto/caam/dpseci-debugfs.c:8:
drivers/crypto/caam/caamalg_qi2.h:84:21: error: field 'napi' has incomplete type
  struct napi_struct napi;
                     ^
drivers/crypto/caam/caamalg_qi2.h:85:20: error: field 'net_dev' has incomplete type
  struct net_device net_dev;
                    ^
scripts/Makefile.build:278: recipe for target 'drivers/crypto/caam/dpseci-debugfs.o' failed


Other driver files include linux/netdevice.h indirectly, via compat.h ->
linux/rtnetlink.h.

Most *.c files in drivers/crypto/caam solve dependencies by including
compat.h, but this approach is messy.

Let's keep the explicit inclusion then.

Horia
Vakul Garg July 23, 2019, 1:20 a.m. UTC | #4 
> -----Original Message-----
> From: Horia Geanta
> Sent: Monday, July 22, 2019 7:55 PM
> To: Vakul Garg <vakul.garg@nxp.com>; linux-crypto@vger.kernel.org
> Cc: Aymen Sghaier <aymen.sghaier@nxp.com>;
> herbert@gondor.apana.org.au
> Subject: Re: [PATCH v2] crypto: caam/qi2 - Add printing dpseci fq stats using
> debugfs
> 
> On 7/19/2019 2:23 PM, Vakul Garg wrote:
> [...]
> > +if CRYPTO_DEV_FSL_DPAA2_CAAM
> > +
> > +config CRYPTO_DEV_FSL_DPAA2_CAAM_DEBUGFS
> > +	depends on DEBUG_FS
> > +	bool "Enable debugfs support"
> > +	help
> > +	  Selecting this will enable printing of various debug information
> > +          in the DPAA2 CAAM driver.
> > +
> > +endif
> Let's enable this based on CONFIG_DEBUG_FS.
> 
> > diff --git a/drivers/crypto/caam/Makefile
> > b/drivers/crypto/caam/Makefile index 9ab4e81ea21e..e4e9fa481a44
> 100644
> > --- a/drivers/crypto/caam/Makefile
> > +++ b/drivers/crypto/caam/Makefile
> > @@ -30,3 +30,4 @@ endif
> >  obj-$(CONFIG_CRYPTO_DEV_FSL_DPAA2_CAAM) += dpaa2_caam.o
> >
> >  dpaa2_caam-y    := caamalg_qi2.o dpseci.o
> > +dpaa2_caam-$(CONFIG_CRYPTO_DEV_FSL_DPAA2_CAAM_DEBUGFS) +=
> > +dpseci-debugfs.o
> dpaa2_caam-$(CONFIG_DEBUG_FS)
> 
> [...]
> > diff --git a/drivers/crypto/caam/caamalg_qi2.h
> > b/drivers/crypto/caam/caamalg_qi2.h
> > index 973f6296bc6f..b450e2a25c1f 100644
> > --- a/drivers/crypto/caam/caamalg_qi2.h
> > +++ b/drivers/crypto/caam/caamalg_qi2.h
> > @@ -10,6 +10,7 @@
> >  #include <soc/fsl/dpaa2-io.h>
> >  #include <soc/fsl/dpaa2-fd.h>
> >  #include <linux/threads.h>
> > +#include <linux/netdevice.h>
> How is this change related to current patch?
> 
> >  #include "dpseci.h"
> >  #include "desc_constr.h"
> >
> > @@ -64,6 +65,7 @@ struct dpaa2_caam_priv {
> >  	struct iommu_domain *domain;
> >
> >  	struct dpaa2_caam_priv_per_cpu __percpu *ppriv;
> > +	struct dentry *dfs_root;
> dfs_root is used only in dpseci-debugfs.c, let's have it there as global.
> 

I submitted this change in v3. There is still a minor issue with this patch version. 
Before submitting the next v4, I have a question.

Could there be a situation that there are multiple  dpseci objects assigned to kernel?
In that case, we need to maintain dfs_root for each separately.
 


> Horia
Horia Geanta July 23, 2019, 6:14 a.m. UTC | #5 
On 7/23/2019 4:20 AM, Vakul Garg wrote:
>>> @@ -64,6 +65,7 @@ struct dpaa2_caam_priv {
>>>  	struct iommu_domain *domain;
>>>
>>>  	struct dpaa2_caam_priv_per_cpu __percpu *ppriv;
>>> +	struct dentry *dfs_root;
>> dfs_root is used only in dpseci-debugfs.c, let's have it there as global.
>>
> 
> I submitted this change in v3. There is still a minor issue with this patch version. 
> Before submitting the next v4, I have a question.
> 
> Could there be a situation that there are multiple  dpseci objects assigned to kernel?
In theory, yes.
fsl-mc, the bus dpseci devices sit on, allows for multiple instances.

However, caam/qi2 (driver for dpseci devices) doesn't have support
for this.
For e.g., all dpseci instances would try to register the algorithms using
the same name & driver name - something that will trigger an error
in crypto API.
This could be easily fixed, however the real issue is that
there is no load balancing support - neither at crypto API level
nor at driver level.

> In that case, we need to maintain dfs_root for each separately.
>  
Ok, let's keep dfs_root per device.
For now this has no practical value, but at least makes the work easier
in case load balancing support is added at some point.

Horia
diff mbox series

Patch

diff --git a/drivers/crypto/caam/Kconfig b/drivers/crypto/caam/Kconfig
index 3720ddabb507..383e167e7272 100644
--- a/drivers/crypto/caam/Kconfig
+++ b/drivers/crypto/caam/Kconfig
@@ -168,3 +168,14 @@  config CRYPTO_DEV_FSL_DPAA2_CAAM
 
 	  To compile this as a module, choose M here: the module
 	  will be called dpaa2_caam.
+
+if CRYPTO_DEV_FSL_DPAA2_CAAM
+
+config CRYPTO_DEV_FSL_DPAA2_CAAM_DEBUGFS
+	depends on DEBUG_FS
+	bool "Enable debugfs support"
+	help
+	  Selecting this will enable printing of various debug information
+          in the DPAA2 CAAM driver.
+
+endif
diff --git a/drivers/crypto/caam/Makefile b/drivers/crypto/caam/Makefile
index 9ab4e81ea21e..e4e9fa481a44 100644
--- a/drivers/crypto/caam/Makefile
+++ b/drivers/crypto/caam/Makefile
@@ -30,3 +30,4 @@  endif
 obj-$(CONFIG_CRYPTO_DEV_FSL_DPAA2_CAAM) += dpaa2_caam.o
 
 dpaa2_caam-y    := caamalg_qi2.o dpseci.o
+dpaa2_caam-$(CONFIG_CRYPTO_DEV_FSL_DPAA2_CAAM_DEBUGFS) += dpseci-debugfs.o
diff --git a/drivers/crypto/caam/caamalg_qi2.c b/drivers/crypto/caam/caamalg_qi2.c
index 06bf32c32cbd..a78a36dfa7b9 100644
--- a/drivers/crypto/caam/caamalg_qi2.c
+++ b/drivers/crypto/caam/caamalg_qi2.c
@@ -15,6 +15,7 @@ 
 #include "key_gen.h"
 #include "caamalg_desc.h"
 #include "caamhash_desc.h"
+#include "dpseci-debugfs.h"
 #include <linux/fsl/mc.h>
 #include <soc/fsl/dpaa2-io.h>
 #include <soc/fsl/dpaa2-fd.h>
@@ -5098,6 +5099,8 @@  static int dpaa2_caam_probe(struct fsl_mc_device *dpseci_dev)
 		goto err_bind;
 	}
 
+	dpaa2_dpseci_debugfs_init(priv);
+
 	/* register crypto algorithms the device supports */
 	for (i = 0; i < ARRAY_SIZE(driver_algs); i++) {
 		struct caam_skcipher_alg *t_alg = driver_algs + i;
@@ -5265,6 +5268,8 @@  static int __cold dpaa2_caam_remove(struct fsl_mc_device *ls_dev)
 	dev = &ls_dev->dev;
 	priv = dev_get_drvdata(dev);
 
+	dpaa2_dpseci_debugfs_exit(priv);
+
 	for (i = 0; i < ARRAY_SIZE(driver_aeads); i++) {
 		struct caam_aead_alg *t_alg = driver_aeads + i;
 
diff --git a/drivers/crypto/caam/caamalg_qi2.h b/drivers/crypto/caam/caamalg_qi2.h
index 973f6296bc6f..b450e2a25c1f 100644
--- a/drivers/crypto/caam/caamalg_qi2.h
+++ b/drivers/crypto/caam/caamalg_qi2.h
@@ -10,6 +10,7 @@ 
 #include <soc/fsl/dpaa2-io.h>
 #include <soc/fsl/dpaa2-fd.h>
 #include <linux/threads.h>
+#include <linux/netdevice.h>
 #include "dpseci.h"
 #include "desc_constr.h"
 
@@ -64,6 +65,7 @@  struct dpaa2_caam_priv {
 	struct iommu_domain *domain;
 
 	struct dpaa2_caam_priv_per_cpu __percpu *ppriv;
+	struct dentry *dfs_root;
 };
 
 /**
diff --git a/drivers/crypto/caam/dpseci-debugfs.c b/drivers/crypto/caam/dpseci-debugfs.c
new file mode 100644
index 000000000000..a6b67e4499aa
--- /dev/null
+++ b/drivers/crypto/caam/dpseci-debugfs.c
@@ -0,0 +1,80 @@ 
+// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
+/* Copyright 2019 NXP
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/debugfs.h>
+#include "dpseci-debugfs.h"
+
+static int dpseci_dbg_fqs_show(struct seq_file *file, void *offset)
+{
+	struct dpaa2_caam_priv *priv = (struct dpaa2_caam_priv *)file->private;
+	u32 fqid, fcnt, bcnt;
+	int i, err;
+
+	seq_printf(file, "FQ stats for %s:\n", dev_name(priv->dev));
+	seq_printf(file, "%s%16s%16s\n",
+		   "Rx-VFQID",
+		   "Pending frames",
+		   "Pending bytes");
+
+	for (i = 0; i <  priv->num_pairs; i++) {
+		fqid = priv->rx_queue_attr[i].fqid;
+		err = dpaa2_io_query_fq_count(NULL, fqid, &fcnt, &bcnt);
+		if (err)
+			continue;
+
+		seq_printf(file, "%5d%16u%16u\n", fqid, fcnt, bcnt);
+	}
+
+	seq_printf(file, "%s%16s%16s\n",
+		   "Tx-VFQID",
+		   "Pending frames",
+		   "Pending bytes");
+
+	for (i = 0; i <  priv->num_pairs; i++) {
+		fqid = priv->tx_queue_attr[i].fqid;
+		err = dpaa2_io_query_fq_count(NULL, fqid, &fcnt, &bcnt);
+		if (err)
+			continue;
+
+		seq_printf(file, "%5d%16u%16u\n", fqid, fcnt, bcnt);
+	}
+
+	return 0;
+}
+
+static int dpseci_dbg_fqs_open(struct inode *inode, struct file *file)
+{
+	int err;
+	struct dpaa2_caam_priv *priv;
+
+	priv = (struct dpaa2_caam_priv *)inode->i_private;
+
+	err = single_open(file, dpseci_dbg_fqs_show, priv);
+	if (err < 0)
+		dev_err(priv->dev, "single_open() failed\n");
+
+	return err;
+}
+
+static const struct file_operations dpseci_dbg_fq_ops = {
+	.open = dpseci_dbg_fqs_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+void dpaa2_dpseci_debugfs_init(struct dpaa2_caam_priv *priv)
+{
+	priv->dfs_root = debugfs_create_dir(dev_name(priv->dev), NULL);
+
+	debugfs_create_file("fq_stats", 0444, priv->dfs_root, priv,
+			    &dpseci_dbg_fq_ops);
+}
+
+void dpaa2_dpseci_debugfs_exit(struct dpaa2_caam_priv *priv)
+{
+	debugfs_remove_recursive(priv->dfs_root);
+}
diff --git a/drivers/crypto/caam/dpseci-debugfs.h b/drivers/crypto/caam/dpseci-debugfs.h
new file mode 100644
index 000000000000..9f90819a7adf
--- /dev/null
+++ b/drivers/crypto/caam/dpseci-debugfs.h
@@ -0,0 +1,19 @@ 
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
+/* Copyright 2019 NXP
+ */
+
+#ifndef DPSECI_DEBUGFS_H
+#define DPSECI_DEBUGFS_H
+
+#include <linux/dcache.h>
+#include "caamalg_qi2.h"
+
+#ifdef CONFIG_CRYPTO_DEV_FSL_DPAA2_CAAM_DEBUGFS
+void dpaa2_dpseci_debugfs_init(struct dpaa2_caam_priv *priv);
+void dpaa2_dpseci_debugfs_exit(struct dpaa2_caam_priv *priv);
+#else
+static inline void dpaa2_dpseci_debugfs_init(struct dpaa2_caam_priv *priv) {}
+static inline void dpaa2_dpseci_debugfs_exit(struct dpaa2_caam_priv *priv) {}
+#endif /* CONFIG_CRYPTO_DEV_FSL_DPAA2_CAAM_DEBUGFS */
+
+#endif /* DPSECI_DEBUGFS_H */