| Message ID | 20220215160641.51683-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | [v1,1/1] crypto: cavium/nitrox - don't cast parameter in bit operations | expand |
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
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
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,
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.
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;
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(-)