crypto: cavium/nitrox - don't cast parameter in bit operations

PATCH Superseded Andy Shevchenko 3 participants 5 messages Feb 15, 2022 – Feb 23, 2022 Crypto API

Share & tools

Andy Shevchenko

author

2022-02-15 16:06 UTC #0 lore raw

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

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 <[email protected]>

---
 drivers/crypto/cavium/nitrox/nitrox_mbx.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

drivers/crypto/cavium/nitrox/nitrox_mbx.c +6 -2

index 2e9c0d214363..4531476a585a 100644
@@ -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;

Signature · 1 line

2.34.1

kernel test robot

build bot

2022-02-15 19:03 UTC #1 lore raw

I love your patch! Perhaps something to improve:

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(a)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 <[email protected]>

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(a)lists.01.org

kernel test robot

build bot

2022-02-15 20:35 UTC #2 lore raw

In reply to Andy Shevchenko

I love your patch! Yet something to improve:

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(a)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 <[email protected]>

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(a)lists.01.org

Herbert Xu

maintainer

2022-02-23 02:52 UTC #3 lore raw

In reply to Andy Shevchenko

drivers/crypto/cavium/nitrox/nitrox_mbx.c void nitrox_pf2vf_mbox_handler(struct nitrox_device *ndev) · line 126

Quoted 6 lines · Andy Shevchenko

drivers/crypto/cavium/nitrox/nitrox_mbx.c +1 -0

@@ -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,

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Andy Shevchenko

author

2022-02-23 11:09 UTC #4 lore raw

Quoted 9 lines · Herbert Xu

drivers/crypto/cavium/nitrox/nitrox_mbx.c void nitrox_pf2vf_mbox_handler(struct nitrox_device *ndev) · line 126
Quoted 6 lines · Andy Shevchenko
drivers/crypto/cavium/nitrox/nitrox_mbx.c +1 -0
@@ -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.

Signature · 2 lines

With Best Regards,
Andy Shevchenko