All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] bnxt_en: Fix 64-bit doorbell operation on 32-bit kernels
@ 2021-08-31 22:28 Michael Chan
  2021-08-31 23:59 ` Jakub Kicinski
  2021-09-01 19:13 ` kernel test robot
  0 siblings, 2 replies; 3+ messages in thread
From: Michael Chan @ 2021-08-31 22:28 UTC (permalink / raw)
  To: davem; +Cc: netdev, kuba, edwin.peer, gospo

[-- Attachment #1: Type: text/plain, Size: 4771 bytes --]

The driver requires 64-bit doorbell writes to be atomic on 32-bit
architectures.  So we redefined writeq as a new macro with spinlock
protection on 32-bit architectures.  This created a new warning when
we added a new file in a recent patchset.  writeq is defined on many
32-bit architectures to do the memory write non-atomically and it
generated a new macro redefined warning.  This warning was fixed
incorrectly in the recent patch.

Fix this properly by adding a new bnxt_writeq() function that will
do the non-atomic write under spinlock on 32-bit systems.  All callers
in the driver will now call bnxt_writeq() instead.

Reported-by: kernel test robot <lkp@intel.com>
Fixes: f9ff578251dc ("bnxt_en: introduce new firmware message API based on DMA pools")
Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c | 14 +++++-----
 drivers/net/ethernet/broadcom/bnxt/bnxt.h | 33 ++++++++++++++---------
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 627f85ee3922..5b145077d21a 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -305,13 +305,15 @@ static bool bnxt_vf_pciid(enum board_idx idx)
 	writel(DB_CP_FLAGS | RING_CMP(idx), (db)->doorbell)
 
 #define BNXT_DB_NQ_P5(db, idx)						\
-	writeq((db)->db_key64 | DBR_TYPE_NQ | RING_CMP(idx), (db)->doorbell)
+	bnxt_writeq((db)->db_key64 | DBR_TYPE_NQ | RING_CMP(idx),	\
+		    (db)->doorbell)
 
 #define BNXT_DB_CQ_ARM(db, idx)						\
 	writel(DB_CP_REARM_FLAGS | RING_CMP(idx), (db)->doorbell)
 
 #define BNXT_DB_NQ_ARM_P5(db, idx)					\
-	writeq((db)->db_key64 | DBR_TYPE_NQ_ARM | RING_CMP(idx), (db)->doorbell)
+	bnxt_writeq((db)->db_key64 | DBR_TYPE_NQ_ARM | RING_CMP(idx),	\
+		    (db)->doorbell)
 
 static void bnxt_db_nq(struct bnxt *bp, struct bnxt_db_info *db, u32 idx)
 {
@@ -332,8 +334,8 @@ static void bnxt_db_nq_arm(struct bnxt *bp, struct bnxt_db_info *db, u32 idx)
 static void bnxt_db_cq(struct bnxt *bp, struct bnxt_db_info *db, u32 idx)
 {
 	if (bp->flags & BNXT_FLAG_CHIP_P5)
-		writeq(db->db_key64 | DBR_TYPE_CQ_ARMALL | RING_CMP(idx),
-		       db->doorbell);
+		bnxt_writeq(db->db_key64 | DBR_TYPE_CQ_ARMALL | RING_CMP(idx),
+			    db->doorbell);
 	else
 		BNXT_DB_CQ(db, idx);
 }
@@ -2638,8 +2640,8 @@ static void __bnxt_poll_cqs_done(struct bnxt *bp, struct bnxt_napi *bnapi,
 
 		if (cpr2 && cpr2->had_work_done) {
 			db = &cpr2->cp_db;
-			writeq(db->db_key64 | dbr_type |
-			       RING_CMP(cpr2->cp_raw_cons), db->doorbell);
+			bnxt_writeq(db->db_key64 | dbr_type |
+				    RING_CMP(cpr2->cp_raw_cons), db->doorbell);
 			cpr2->had_work_done = 0;
 		}
 	}
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index a8212dcdad5f..e69d4a1d0c63 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1981,7 +1981,7 @@ struct bnxt {
 	struct mutex		sriov_lock;
 #endif
 
-#ifndef writeq
+#if BITS_PER_LONG == 32
 	/* ensure atomic 64-bit doorbell writes on 32-bit systems. */
 	spinlock_t		db_lock;
 #endif
@@ -2110,24 +2110,33 @@ static inline u32 bnxt_tx_avail(struct bnxt *bp, struct bnxt_tx_ring_info *txr)
 		((txr->tx_prod - txr->tx_cons) & bp->tx_ring_mask);
 }
 
-#ifndef writeq
-#define writeq(val64, db)			\
-do {						\
-	spin_lock(&bp->db_lock);		\
-	writel((val64) & 0xffffffff, db);	\
-	writel((val64) >> 32, (db) + 4);	\
-	spin_unlock(&bp->db_lock);		\
-} while (0)
+static inline void bnxt_writeq(u64 val, volatile void __iomem *addr)
+{
+#if BITS_PER_LONG == 32
+	spin_lock(&bp->db_lock);
+	writel(val & 0xffffffff, addr);
+	writel(val >> 32, addr + 4);
+	spin_unlock(&bp->db_lock);
+#else
+	writeq(val, addr);
+#endif
+}
 
-#define writeq_relaxed writeq
+static inline void bnxt_writeq_relaxed(u64 val, volatile void __iomem *addr)
+{
+#if BITS_PER_LONG == 32
+	bnxt_writeq(val, addr);
+#else
+	writeq_relaxed(val, addr);
 #endif
+}
 
 /* For TX and RX ring doorbells with no ordering guarantee*/
 static inline void bnxt_db_write_relaxed(struct bnxt *bp,
 					 struct bnxt_db_info *db, u32 idx)
 {
 	if (bp->flags & BNXT_FLAG_CHIP_P5) {
-		writeq_relaxed(db->db_key64 | idx, db->doorbell);
+		bnxt_writeq_relaxed(db->db_key64 | idx, db->doorbell);
 	} else {
 		u32 db_val = db->db_key32 | idx;
 
@@ -2142,7 +2151,7 @@ static inline void bnxt_db_write(struct bnxt *bp, struct bnxt_db_info *db,
 				 u32 idx)
 {
 	if (bp->flags & BNXT_FLAG_CHIP_P5) {
-		writeq(db->db_key64 | idx, db->doorbell);
+		bnxt_writeq(db->db_key64 | idx, db->doorbell);
 	} else {
 		u32 db_val = db->db_key32 | idx;
 
-- 
2.18.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4209 bytes --]

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

* Re: [PATCH net-next] bnxt_en: Fix 64-bit doorbell operation on 32-bit kernels
  2021-08-31 22:28 [PATCH net-next] bnxt_en: Fix 64-bit doorbell operation on 32-bit kernels Michael Chan
@ 2021-08-31 23:59 ` Jakub Kicinski
  2021-09-01 19:13 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2021-08-31 23:59 UTC (permalink / raw)
  To: Michael Chan; +Cc: davem, netdev, edwin.peer, gospo

On Tue, 31 Aug 2021 18:28:45 -0400 Michael Chan wrote:
> The driver requires 64-bit doorbell writes to be atomic on 32-bit
> architectures.  So we redefined writeq as a new macro with spinlock
> protection on 32-bit architectures.  This created a new warning when
> we added a new file in a recent patchset.  writeq is defined on many
> 32-bit architectures to do the memory write non-atomically and it
> generated a new macro redefined warning.  This warning was fixed
> incorrectly in the recent patch.
> 
> Fix this properly by adding a new bnxt_writeq() function that will
> do the non-atomic write under spinlock on 32-bit systems.  All callers
> in the driver will now call bnxt_writeq() instead.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Fixes: f9ff578251dc ("bnxt_en: introduce new firmware message API based on DMA pools")
> Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>

Lots of these:

drivers/net/ethernet/broadcom/bnxt/bnxt.h: In function ‘bnxt_writeq’:
drivers/net/ethernet/broadcom/bnxt/bnxt.h:2116:13: error: ‘bp’ undeclared (first use in this function); did you mean ‘up’?
 2116 |  spin_lock(&bp->db_lock);
      |             ^~
      |             up

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

* Re: [PATCH net-next] bnxt_en: Fix 64-bit doorbell operation on 32-bit kernels
  2021-08-31 22:28 [PATCH net-next] bnxt_en: Fix 64-bit doorbell operation on 32-bit kernels Michael Chan
  2021-08-31 23:59 ` Jakub Kicinski
@ 2021-09-01 19:13 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2021-09-01 19:13 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2383 bytes --]

Hi Michael,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Michael-Chan/bnxt_en-Fix-64-bit-doorbell-operation-on-32-bit-kernels/20210901-062918
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 29ce8f9701072fc221d9c38ad952de1a9578f95c
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-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/0f11c81bbeda35da6bb94d87ac7d2333b9f6760d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Michael-Chan/bnxt_en-Fix-64-bit-doorbell-operation-on-32-bit-kernels/20210901-062918
        git checkout 0f11c81bbeda35da6bb94d87ac7d2333b9f6760d
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash

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 >>):

   In file included from drivers/net/ethernet/broadcom/bnxt/bnxt.c:62:
   drivers/net/ethernet/broadcom/bnxt/bnxt.h: In function 'bnxt_writeq':
>> drivers/net/ethernet/broadcom/bnxt/bnxt.h:2116:20: error: 'bp' undeclared (first use in this function); did you mean 'up'?
    2116 |         spin_lock(&bp->db_lock);
         |                    ^~
         |                    up
   drivers/net/ethernet/broadcom/bnxt/bnxt.h:2116:20: note: each undeclared identifier is reported only once for each function it appears in


vim +2116 drivers/net/ethernet/broadcom/bnxt/bnxt.h

  2112	
  2113	static inline void bnxt_writeq(u64 val, volatile void __iomem *addr)
  2114	{
  2115	#if BITS_PER_LONG == 32
> 2116		spin_lock(&bp->db_lock);
  2117		writel(val & 0xffffffff, addr);
  2118		writel(val >> 32, addr + 4);
  2119		spin_unlock(&bp->db_lock);
  2120	#else
  2121		writeq(val, addr);
  2122	#endif
  2123	}
  2124	

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

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 68510 bytes --]

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

end of thread, other threads:[~2021-09-01 19:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-31 22:28 [PATCH net-next] bnxt_en: Fix 64-bit doorbell operation on 32-bit kernels Michael Chan
2021-08-31 23:59 ` Jakub Kicinski
2021-09-01 19:13 ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.