[PATCH v1 1/1] crypto: cavium/nitrox - don't cast parameter in bit operations

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH v1 1/1] crypto: cavium/nitrox - don't cast parameter in bit operations
@ 2022-02-15 16:06 Andy Shevchenko
  2022-02-15 19:03 ` kernel test robot
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Andy Shevchenko @ 2022-02-15 16:06 UTC (permalink / raw)
  To: Andy Shevchenko, linux-crypto, linux-kernel; +Cc: Herbert Xu, David S. Miller

While in this particular case it would not be a (critical) issue,
the pattern itself is bad and error prone in case the location
of the parameter is changed.

Don't cast parameter to unsigned long pointer in the bit operations.
Instead copy to a local variable on stack of a proper type and use.

Fixes: cf718eaa8f9b ("crypto: cavium/nitrox - Enabled Mailbox support")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/crypto/cavium/nitrox/nitrox_mbx.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/cavium/nitrox/nitrox_mbx.c b/drivers/crypto/cavium/nitrox/nitrox_mbx.c
index 2e9c0d214363..4531476a585a 100644
--- a/drivers/crypto/cavium/nitrox/nitrox_mbx.c
+++ b/drivers/crypto/cavium/nitrox/nitrox_mbx.c
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
+#include <linux/bitmap.h>
 #include <linux/workqueue.h>
 
 #include "nitrox_csr.h"
@@ -122,6 +123,7 @@ void nitrox_pf2vf_mbox_handler(struct nitrox_device *ndev)
 {
 	struct nitrox_vfdev *vfdev;
 	struct pf2vf_work *pfwork;
+	DEFINE_BITMAP(csr, 64);
 	u64 value, reg_addr;
 	u32 i;
 	int vfno;
@@ -129,7 +131,8 @@ void nitrox_pf2vf_mbox_handler(struct nitrox_device *ndev)
 	/* loop for VF(0..63) */
 	reg_addr = NPS_PKT_MBOX_INT_LO;
 	value = nitrox_read_csr(ndev, reg_addr);
-	for_each_set_bit(i, (const unsigned long *)&value, BITS_PER_LONG) {
+	bitmap_from_u64(csr, value);
+	for_each_set_bit(i, csr, BITS_PER_TYPE(csr)) {
 		/* get the vfno from ring */
 		vfno = RING_TO_VFNO(i, ndev->iov.max_vf_queues);
 		vfdev = ndev->iov.vfdev + vfno;
@@ -151,7 +154,8 @@ void nitrox_pf2vf_mbox_handler(struct nitrox_device *ndev)
 	/* loop for VF(64..127) */
 	reg_addr = NPS_PKT_MBOX_INT_HI;
 	value = nitrox_read_csr(ndev, reg_addr);
-	for_each_set_bit(i, (const unsigned long *)&value, BITS_PER_LONG) {
+	bitmap_from_u64(csr, value);
+	for_each_set_bit(i, csr, BITS_PER_TYPE(csr)) {
 		/* get the vfno from ring */
 		vfno = RING_TO_VFNO(i + 64, ndev->iov.max_vf_queues);
 		vfdev = ndev->iov.vfdev + vfno;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v1 1/1] crypto: cavium/nitrox - don't cast parameter in bit operations
  2022-02-15 16:06 [PATCH v1 1/1] crypto: cavium/nitrox - don't cast parameter in bit operations Andy Shevchenko
@ 2022-02-15 19:03 ` kernel test robot
  2022-02-15 20:35 ` kernel test robot
  2022-02-23  2:52 ` Herbert Xu
  2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2022-02-15 19:03 UTC (permalink / raw)
  To: Andy Shevchenko, linux-crypto, linux-kernel; +Cc: kbuild-all, Herbert Xu

Hi Andy,

I love your patch! Perhaps something to improve:

[auto build test WARNING on herbert-cryptodev-2.6/master]
[also build test WARNING on herbert-crypto-2.6/master linux/master linus/master v5.17-rc4 next-20220215]
[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]

url:    https://github.com/0day-ci/linux/commits/Andy-Shevchenko/crypto-cavium-nitrox-don-t-cast-parameter-in-bit-operations/20220216-000941
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220216/202202160202.jG0FQRzi-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/51e27f6f33d377023ec5e097d18acb18f992f551
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Andy-Shevchenko/crypto-cavium-nitrox-don-t-cast-parameter-in-bit-operations/20220216-000941
        git checkout 51e27f6f33d377023ec5e097d18acb18f992f551
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash drivers/crypto/cavium/nitrox/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/crypto/cavium/nitrox/nitrox_mbx.c: In function 'nitrox_pf2vf_mbox_handler':
   drivers/crypto/cavium/nitrox/nitrox_mbx.c:126:9: error: implicit declaration of function 'DEFINE_BITMAP'; did you mean 'DEFINE_IDA'? [-Werror=implicit-function-declaration]
     126 |         DEFINE_BITMAP(csr, 64);
         |         ^~~~~~~~~~~~~
         |         DEFINE_IDA
   drivers/crypto/cavium/nitrox/nitrox_mbx.c:126:23: error: 'csr' undeclared (first use in this function)
     126 |         DEFINE_BITMAP(csr, 64);
         |                       ^~~
   drivers/crypto/cavium/nitrox/nitrox_mbx.c:126:23: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/crypto/cavium/nitrox/nitrox_mbx.c:127:9: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     127 |         u64 value, reg_addr;
         |         ^~~
   cc1: some warnings being treated as errors


vim +127 drivers/crypto/cavium/nitrox/nitrox_mbx.c

cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  121  
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  122  void nitrox_pf2vf_mbox_handler(struct nitrox_device *ndev)
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  123  {
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  124  	struct nitrox_vfdev *vfdev;
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  125  	struct pf2vf_work *pfwork;
51e27f6f33d377 Andy Shevchenko   2022-02-15  126  	DEFINE_BITMAP(csr, 64);
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04 @127  	u64 value, reg_addr;
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  128  	u32 i;
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  129  	int vfno;
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  130  
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  131  	/* loop for VF(0..63) */
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  132  	reg_addr = NPS_PKT_MBOX_INT_LO;
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  133  	value = nitrox_read_csr(ndev, reg_addr);
51e27f6f33d377 Andy Shevchenko   2022-02-15  134  	bitmap_from_u64(csr, value);
51e27f6f33d377 Andy Shevchenko   2022-02-15  135  	for_each_set_bit(i, csr, BITS_PER_TYPE(csr)) {
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  136  		/* get the vfno from ring */
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  137  		vfno = RING_TO_VFNO(i, ndev->iov.max_vf_queues);
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  138  		vfdev = ndev->iov.vfdev + vfno;
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  139  		vfdev->ring = i;
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  140  		/* fill the vf mailbox data */
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  141  		vfdev->msg.value = pf2vf_read_mbox(ndev, vfdev->ring);
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  142  		pfwork = kzalloc(sizeof(*pfwork), GFP_ATOMIC);
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  143  		if (!pfwork)
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  144  			continue;
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  145  
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  146  		pfwork->vfdev = vfdev;
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  147  		pfwork->ndev = ndev;
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  148  		INIT_WORK(&pfwork->pf2vf_resp, pf2vf_resp_handler);
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  149  		queue_work(ndev->iov.pf2vf_wq, &pfwork->pf2vf_resp);
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  150  		/* clear the corresponding vf bit */
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  151  		nitrox_write_csr(ndev, reg_addr, BIT_ULL(i));
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  152  	}
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  153  
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  154  	/* loop for VF(64..127) */
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  155  	reg_addr = NPS_PKT_MBOX_INT_HI;
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  156  	value = nitrox_read_csr(ndev, reg_addr);
51e27f6f33d377 Andy Shevchenko   2022-02-15  157  	bitmap_from_u64(csr, value);
51e27f6f33d377 Andy Shevchenko   2022-02-15  158  	for_each_set_bit(i, csr, BITS_PER_TYPE(csr)) {
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  159  		/* get the vfno from ring */
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  160  		vfno = RING_TO_VFNO(i + 64, ndev->iov.max_vf_queues);
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  161  		vfdev = ndev->iov.vfdev + vfno;
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  162  		vfdev->ring = (i + 64);
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  163  		/* fill the vf mailbox data */
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  164  		vfdev->msg.value = pf2vf_read_mbox(ndev, vfdev->ring);
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  165  
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  166  		pfwork = kzalloc(sizeof(*pfwork), GFP_ATOMIC);
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  167  		if (!pfwork)
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  168  			continue;
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  169  
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  170  		pfwork->vfdev = vfdev;
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  171  		pfwork->ndev = ndev;
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  172  		INIT_WORK(&pfwork->pf2vf_resp, pf2vf_resp_handler);
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  173  		queue_work(ndev->iov.pf2vf_wq, &pfwork->pf2vf_resp);
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  174  		/* clear the corresponding vf bit */
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  175  		nitrox_write_csr(ndev, reg_addr, BIT_ULL(i));
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  176  	}
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  177  }
cf718eaa8f9b2c Srikanth, Jampala 2018-12-04  178  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1 1/1] crypto: cavium/nitrox - don't cast parameter in bit operations
  2022-02-15 16:06 [PATCH v1 1/1] crypto: cavium/nitrox - don't cast parameter in bit operations Andy Shevchenko
  2022-02-15 19:03 ` kernel test robot
@ 2022-02-15 20:35 ` kernel test robot
  2022-02-23  2:52 ` Herbert Xu
  2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2022-02-15 20:35 UTC (permalink / raw)
  To: Andy Shevchenko, linux-crypto, linux-kernel; +Cc: kbuild-all, Herbert Xu

Hi Andy,

I love your patch! Yet something to improve:

[auto build test ERROR on herbert-cryptodev-2.6/master]
[also build test ERROR on herbert-crypto-2.6/master linux/master linus/master v5.17-rc4 next-20220215]
[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]

url:    https://github.com/0day-ci/linux/commits/Andy-Shevchenko/crypto-cavium-nitrox-don-t-cast-parameter-in-bit-operations/20220216-000941
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20220216/202202160443.hqpSAgbP-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/51e27f6f33d377023ec5e097d18acb18f992f551
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Andy-Shevchenko/crypto-cavium-nitrox-don-t-cast-parameter-in-bit-operations/20220216-000941
        git checkout 51e27f6f33d377023ec5e097d18acb18f992f551
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/crypto/cavium/nitrox/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/crypto/cavium/nitrox/nitrox_mbx.c: In function 'nitrox_pf2vf_mbox_handler':
>> drivers/crypto/cavium/nitrox/nitrox_mbx.c:126:2: error: implicit declaration of function 'DEFINE_BITMAP'; did you mean 'DEFINE_TIMER'? [-Werror=implicit-function-declaration]
     126 |  DEFINE_BITMAP(csr, 64);
         |  ^~~~~~~~~~~~~
         |  DEFINE_TIMER
>> drivers/crypto/cavium/nitrox/nitrox_mbx.c:126:16: error: 'csr' undeclared (first use in this function); did you mean 'msr'?
     126 |  DEFINE_BITMAP(csr, 64);
         |                ^~~
         |                msr
   drivers/crypto/cavium/nitrox/nitrox_mbx.c:126:16: note: each undeclared identifier is reported only once for each function it appears in
   drivers/crypto/cavium/nitrox/nitrox_mbx.c:127:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
     127 |  u64 value, reg_addr;
         |  ^~~
   cc1: some warnings being treated as errors


vim +126 drivers/crypto/cavium/nitrox/nitrox_mbx.c

   121	
   122	void nitrox_pf2vf_mbox_handler(struct nitrox_device *ndev)
   123	{
   124		struct nitrox_vfdev *vfdev;
   125		struct pf2vf_work *pfwork;
 > 126		DEFINE_BITMAP(csr, 64);
   127		u64 value, reg_addr;
   128		u32 i;
   129		int vfno;
   130	
   131		/* loop for VF(0..63) */
   132		reg_addr = NPS_PKT_MBOX_INT_LO;
   133		value = nitrox_read_csr(ndev, reg_addr);
   134		bitmap_from_u64(csr, value);
   135		for_each_set_bit(i, csr, BITS_PER_TYPE(csr)) {
   136			/* get the vfno from ring */
   137			vfno = RING_TO_VFNO(i, ndev->iov.max_vf_queues);
   138			vfdev = ndev->iov.vfdev + vfno;
   139			vfdev->ring = i;
   140			/* fill the vf mailbox data */
   141			vfdev->msg.value = pf2vf_read_mbox(ndev, vfdev->ring);
   142			pfwork = kzalloc(sizeof(*pfwork), GFP_ATOMIC);
   143			if (!pfwork)
   144				continue;
   145	
   146			pfwork->vfdev = vfdev;
   147			pfwork->ndev = ndev;
   148			INIT_WORK(&pfwork->pf2vf_resp, pf2vf_resp_handler);
   149			queue_work(ndev->iov.pf2vf_wq, &pfwork->pf2vf_resp);
   150			/* clear the corresponding vf bit */
   151			nitrox_write_csr(ndev, reg_addr, BIT_ULL(i));
   152		}
   153	
   154		/* loop for VF(64..127) */
   155		reg_addr = NPS_PKT_MBOX_INT_HI;
   156		value = nitrox_read_csr(ndev, reg_addr);
   157		bitmap_from_u64(csr, value);
   158		for_each_set_bit(i, csr, BITS_PER_TYPE(csr)) {
   159			/* get the vfno from ring */
   160			vfno = RING_TO_VFNO(i + 64, ndev->iov.max_vf_queues);
   161			vfdev = ndev->iov.vfdev + vfno;
   162			vfdev->ring = (i + 64);
   163			/* fill the vf mailbox data */
   164			vfdev->msg.value = pf2vf_read_mbox(ndev, vfdev->ring);
   165	
   166			pfwork = kzalloc(sizeof(*pfwork), GFP_ATOMIC);
   167			if (!pfwork)
   168				continue;
   169	
   170			pfwork->vfdev = vfdev;
   171			pfwork->ndev = ndev;
   172			INIT_WORK(&pfwork->pf2vf_resp, pf2vf_resp_handler);
   173			queue_work(ndev->iov.pf2vf_wq, &pfwork->pf2vf_resp);
   174			/* clear the corresponding vf bit */
   175			nitrox_write_csr(ndev, reg_addr, BIT_ULL(i));
   176		}
   177	}
   178	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1 1/1] crypto: cavium/nitrox - don't cast parameter in bit operations
  2022-02-15 16:06 [PATCH v1 1/1] crypto: cavium/nitrox - don't cast parameter in bit operations Andy Shevchenko
  2022-02-15 19:03 ` kernel test robot
  2022-02-15 20:35 ` kernel test robot
@ 2022-02-23  2:52 ` Herbert Xu
  2022-02-23 11:09   ` Andy Shevchenko
  2 siblings, 1 reply; 5+ messages in thread
From: Herbert Xu @ 2022-02-23  2:52 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-crypto, linux-kernel, David S. Miller

On Tue, Feb 15, 2022 at 06:06:41PM +0200, Andy Shevchenko wrote:
>
> @@ -122,6 +123,7 @@ void nitrox_pf2vf_mbox_handler(struct nitrox_device *ndev)
>  {
>  	struct nitrox_vfdev *vfdev;
>  	struct pf2vf_work *pfwork;
> +	DEFINE_BITMAP(csr, 64);

Perhaps you mean DECLARE_BITMAP?

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1 1/1] crypto: cavium/nitrox - don't cast parameter in bit operations
  2022-02-23  2:52 ` Herbert Xu
@ 2022-02-23 11:09   ` Andy Shevchenko
  0 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2022-02-23 11:09 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto, linux-kernel, David S. Miller

On Wed, Feb 23, 2022 at 02:52:52PM +1200, Herbert Xu wrote:
> On Tue, Feb 15, 2022 at 06:06:41PM +0200, Andy Shevchenko wrote:
> >
> > @@ -122,6 +123,7 @@ void nitrox_pf2vf_mbox_handler(struct nitrox_device *ndev)
> >  {
> >  	struct nitrox_vfdev *vfdev;
> >  	struct pf2vf_work *pfwork;
> > +	DEFINE_BITMAP(csr, 64);
> 
> Perhaps you mean DECLARE_BITMAP?

Yes, sorry. I have missed the build by some reason. I'll send a new version.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-02-23 11:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-15 16:06 [PATCH v1 1/1] crypto: cavium/nitrox - don't cast parameter in bit operations Andy Shevchenko
2022-02-15 19:03 ` kernel test robot
2022-02-15 20:35 ` kernel test robot
2022-02-23  2:52 ` Herbert Xu
2022-02-23 11:09   ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox