diff mbox series

[RESEND,v2,for-next,1/7] RDMA/hns: Fix potential compile warnings on hr_reg_write()

Message ID 1622624265-44796-2-git-send-email-liweihang@huawei.com (mailing list archive)
State Superseded
Headers show
Series RDMA/hns: Use new interfaces to write/read fields | expand

Commit Message

Weihang Li June 2, 2021, 8:57 a.m. UTC 
From: Lang Cheng <chenglang@huawei.com>

GCC may reports an running time assert error when a value calculated from
ib_mtu_enum_to_int() is using as 'val' in FIELD_PREDP:

include/linux/compiler_types.h:328:38: error: call to
'__compiletime_assert_1524' declared with attribute error: FIELD_PREP:
value too large for the field

But actually this error will still exists even if the driver can ensure
that ib_mtu_enum_to_int() returns a legal value. So add a mask in
hr_reg_write() to avoid above warning.

Also fix complains from sparse about "dubious: x & !y" when
hr_reg_write(ctx, field, !!val)，Use temporary variables to avoid this.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Lang Cheng <chenglang@huawei.com>
Signed-off-by: Weihang Li <liweihang@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_common.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Jason Gunthorpe June 16, 2021, 7:40 p.m. UTC | #1 
On Wed, Jun 02, 2021 at 04:57:39PM +0800, Weihang Li wrote:
> From: Lang Cheng <chenglang@huawei.com>
> 
> GCC may reports an running time assert error when a value calculated from
> ib_mtu_enum_to_int() is using as 'val' in FIELD_PREDP:
> 
> include/linux/compiler_types.h:328:38: error: call to
> '__compiletime_assert_1524' declared with attribute error: FIELD_PREP:
> value too large for the field

Huh? This warning looks reliable to me, you should not suppress it
like this.

> But actually this error will still exists even if the driver can ensure
> that ib_mtu_enum_to_int() returns a legal value. So add a mask in
> hr_reg_write() to avoid above warning.

I think you need to have an if that ib_mtu_enum_to_int() is not
negative, that should satisfy the CFA?

Jason
Weihang Li June 17, 2021, 6:33 a.m. UTC | #2 
On 2021/6/17 3:40, Jason Gunthorpe wrote:
> On Wed, Jun 02, 2021 at 04:57:39PM +0800, Weihang Li wrote:
>> From: Lang Cheng <chenglang@huawei.com>
>>
>> GCC may reports an running time assert error when a value calculated from
>> ib_mtu_enum_to_int() is using as 'val' in FIELD_PREDP:
>>
>> include/linux/compiler_types.h:328:38: error: call to
>> '__compiletime_assert_1524' declared with attribute error: FIELD_PREP:
>> value too large for the field
> 
> Huh? This warning looks reliable to me, you should not suppress it
> like this.
> 
>> But actually this error will still exists even if the driver can ensure
>> that ib_mtu_enum_to_int() returns a legal value. So add a mask in
>> hr_reg_write() to avoid above warning.
> 
> I think you need to have an if that ib_mtu_enum_to_int() is not
> negative, that should satisfy the CFA?
> 
> Jason
> 

I add a check for the integer mtu from ib_mtu_enum_to_int(), and gcc stop to
complain about that. I will add a patch to this series to fix it. Thank you.

Weihang
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/hns/hns_roce_common.h b/drivers/infiniband/hw/hns/hns_roce_common.h
index 3a5658f..dc7a9fe 100644
--- a/drivers/infiniband/hw/hns/hns_roce_common.h
+++ b/drivers/infiniband/hw/hns/hns_roce_common.h
@@ -79,9 +79,11 @@ 
 
 #define _hr_reg_write(ptr, field_type, field_h, field_l, val)                  \
 	({                                                                     \
+		u32 _val = val;                                                \
 		_hr_reg_clear(ptr, field_type, field_h, field_l);              \
-		*((__le32 *)ptr + (field_h) / 32) |= cpu_to_le32(FIELD_PREP(   \
-			GENMASK((field_h) % 32, (field_l) % 32), val));        \
+		*((__le32 *)ptr + (field_h) / 32) |= cpu_to_le32(              \
+			FIELD_PREP(GENMASK((field_h) % 32, (field_l) % 32),    \
+				   _val & GENMASK((field_h) - (field_l), 0))); \
 	})
 
 #define hr_reg_write(ptr, field, val) _hr_reg_write(ptr, field, val)