The branch main has been updated by kbowling:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=1c45a62a2f667b45ec10a92ad58ff5a34e68b569

commit 1c45a62a2f667b45ec10a92ad58ff5a34e68b569
Author:     Keith Reynolds <keith.reyno...@hpe.com>
AuthorDate: 2024-05-28 06:41:05 +0000
Commit:     Kevin Bowling <kbowl...@freebsd.org>
CommitDate: 2024-05-28 06:41:05 +0000

    qlnxe: Fix multiple locking issues
    
    Multiple issues are reported with WITNESS and code inspection of the
    locking and lock initialization.
    
    PR:             278084
    MFC after:      1 week
---
 sys/dev/qlnx/qlnxe/bcm_osal.h  |  8 +++----
 sys/dev/qlnx/qlnxe/ecore.h     |  1 +
 sys/dev/qlnx/qlnxe/ecore_mcp.c | 48 +++++++++++++++++++++---------------------
 sys/dev/qlnx/qlnxe/ecore_mcp.h |  6 +++---
 sys/dev/qlnx/qlnxe/qlnx_def.h  |  2 +-
 sys/dev/qlnx/qlnxe/qlnx_os.c   |  9 ++++----
 sys/dev/qlnx/qlnxe/qlnx_os.h   |  4 ++--
 7 files changed, 40 insertions(+), 38 deletions(-)

diff --git a/sys/dev/qlnx/qlnxe/bcm_osal.h b/sys/dev/qlnx/qlnxe/bcm_osal.h
index 5d940d3272d6..c820532c9e0a 100644
--- a/sys/dev/qlnx/qlnxe/bcm_osal.h
+++ b/sys/dev/qlnx/qlnxe/bcm_osal.h
@@ -72,7 +72,7 @@ extern void qlnx_dma_free_coherent(void *ecore_dev, void 
*v_addr,
                         bus_addr_t phys, uint32_t size);
 
 extern void qlnx_link_update(void *p_hwfn);
-extern void qlnx_barrier(void *p_hwfn);
+extern void qlnx_barrier(void *p_dev);
 
 extern void *qlnx_zalloc(uint32_t size);
 
@@ -213,14 +213,14 @@ typedef struct osal_list_t
 #define OSAL_SPIN_LOCK_ALLOC(p_hwfn, mutex)
 #define OSAL_SPIN_LOCK_DEALLOC(mutex) mtx_destroy(mutex)
 #define OSAL_SPIN_LOCK_INIT(lock) {\
-               mtx_init(lock, __func__, MTX_NETWORK_LOCK, MTX_SPIN); \
+               mtx_init(lock, __func__, "OSAL spin lock", MTX_SPIN); \
        }
 
 #define OSAL_SPIN_UNLOCK(lock) {\
-               mtx_unlock(lock); \
+               mtx_unlock_spin(lock); \
        }
 #define OSAL_SPIN_LOCK(lock) {\
-               mtx_lock(lock); \
+               mtx_lock_spin(lock); \
        }
 
 #define OSAL_MUTEX_ALLOC(p_hwfn, mutex)
diff --git a/sys/dev/qlnx/qlnxe/ecore.h b/sys/dev/qlnx/qlnxe/ecore.h
index 8fcbc1f8d8a0..eda7c260ba99 100644
--- a/sys/dev/qlnx/qlnxe/ecore.h
+++ b/sys/dev/qlnx/qlnxe/ecore.h
@@ -790,6 +790,7 @@ struct ecore_dev {
        u8                              dp_level;
        char                            name[NAME_SIZE];
        void                            *dp_ctx;
+       void                            *ha;
 
        enum ecore_dev_type             type;
 /* Translate type/revision combo into the proper conditions */
diff --git a/sys/dev/qlnx/qlnxe/ecore_mcp.c b/sys/dev/qlnx/qlnxe/ecore_mcp.c
index d94c7382edc5..ab14b1eb5186 100644
--- a/sys/dev/qlnx/qlnxe/ecore_mcp.c
+++ b/sys/dev/qlnx/qlnxe/ecore_mcp.c
@@ -191,17 +191,17 @@ enum _ecore_status_t ecore_mcp_free(struct ecore_hwfn 
*p_hwfn)
                OSAL_FREE(p_hwfn->p_dev, p_hwfn->mcp_info->mfw_mb_cur);
                OSAL_FREE(p_hwfn->p_dev, p_hwfn->mcp_info->mfw_mb_shadow);
 
-               OSAL_SPIN_LOCK(&p_hwfn->mcp_info->cmd_lock);
+               OSAL_MUTEX_ACQUIRE(&p_hwfn->mcp_info->cmd_lock);
                OSAL_LIST_FOR_EACH_ENTRY_SAFE(p_cmd_elem, p_tmp,
                                              &p_hwfn->mcp_info->cmd_list, list,
                                              struct ecore_mcp_cmd_elem) {
                        ecore_mcp_cmd_del_elem(p_hwfn, p_cmd_elem);
                }
-               OSAL_SPIN_UNLOCK(&p_hwfn->mcp_info->cmd_lock);
+               OSAL_MUTEX_RELEASE(&p_hwfn->mcp_info->cmd_lock);
 
 #ifdef CONFIG_ECORE_LOCK_ALLOC
-               OSAL_SPIN_LOCK_DEALLOC(&p_hwfn->mcp_info->cmd_lock);
-               OSAL_SPIN_LOCK_DEALLOC(&p_hwfn->mcp_info->link_lock);
+               OSAL_MUTEX_DEALLOC(&p_hwfn->mcp_info->cmd_lock);
+               OSAL_MUTEX_DEALLOC(&p_hwfn->mcp_info->link_lock);
 #endif
        }
 
@@ -308,18 +308,18 @@ enum _ecore_status_t ecore_mcp_cmd_init(struct ecore_hwfn 
*p_hwfn,
 
        /* Initialize the MFW spinlocks */
 #ifdef CONFIG_ECORE_LOCK_ALLOC
-       if (OSAL_SPIN_LOCK_ALLOC(p_hwfn, &p_info->cmd_lock)) {
+       if (OSAL_MUTEX_LOCK_ALLOC(p_hwfn, &p_info->cmd_lock)) {
                OSAL_FREE(p_hwfn->p_dev, p_hwfn->mcp_info);
                return ECORE_NOMEM;
        }
-       if (OSAL_SPIN_LOCK_ALLOC(p_hwfn, &p_info->link_lock)) {
-               OSAL_SPIN_LOCK_DEALLOC(&p_info->cmd_lock);
+       if (OSAL_MUTEX_ALLOC(p_hwfn, &p_info->link_lock)) {
+               OSAL_MUTEX_DEALLOC(&p_info->cmd_lock);
                OSAL_FREE(p_hwfn->p_dev, p_hwfn->mcp_info);
                return ECORE_NOMEM;
        }
 #endif
-       OSAL_SPIN_LOCK_INIT(&p_info->cmd_lock);
-       OSAL_SPIN_LOCK_INIT(&p_info->link_lock);
+       OSAL_MUTEX_INIT(&p_info->cmd_lock);
+       OSAL_MUTEX_INIT(&p_info->link_lock);
 
        OSAL_LIST_INIT(&p_info->cmd_list);
 
@@ -381,7 +381,7 @@ enum _ecore_status_t ecore_mcp_reset(struct ecore_hwfn 
*p_hwfn,
        }
 
        /* Ensure that only a single thread is accessing the mailbox */
-       OSAL_SPIN_LOCK(&p_hwfn->mcp_info->cmd_lock);
+       OSAL_MUTEX_ACQUIRE(&p_hwfn->mcp_info->cmd_lock);
 
        org_mcp_reset_seq = ecore_rd(p_hwfn, p_ptt, MISCS_REG_GENERIC_POR_0);
 
@@ -407,7 +407,7 @@ enum _ecore_status_t ecore_mcp_reset(struct ecore_hwfn 
*p_hwfn,
                rc = ECORE_AGAIN;
        }
 
-       OSAL_SPIN_UNLOCK(&p_hwfn->mcp_info->cmd_lock);
+       OSAL_MUTEX_RELEASE(&p_hwfn->mcp_info->cmd_lock);
 
        return rc;
 }
@@ -551,7 +551,7 @@ _ecore_mcp_cmd_and_union(struct ecore_hwfn *p_hwfn, struct 
ecore_ptt *p_ptt,
                 * The spinlock stays locked until the command is sent.
                 */
 
-               OSAL_SPIN_LOCK(&p_hwfn->mcp_info->cmd_lock);
+               OSAL_MUTEX_ACQUIRE(&p_hwfn->mcp_info->cmd_lock);
 
                if (!ecore_mcp_has_pending_cmd(p_hwfn))
                        break;
@@ -562,7 +562,7 @@ _ecore_mcp_cmd_and_union(struct ecore_hwfn *p_hwfn, struct 
ecore_ptt *p_ptt,
                else if (rc != ECORE_AGAIN)
                        goto err;
 
-               OSAL_SPIN_UNLOCK(&p_hwfn->mcp_info->cmd_lock);
+               OSAL_MUTEX_RELEASE(&p_hwfn->mcp_info->cmd_lock);
                if (ECORE_MB_FLAGS_IS_SET(p_mb_params, CAN_SLEEP)) {
                        OSAL_MSLEEP(msecs);
                } else {
@@ -588,7 +588,7 @@ _ecore_mcp_cmd_and_union(struct ecore_hwfn *p_hwfn, struct 
ecore_ptt *p_ptt,
        }
 
        __ecore_mcp_cmd_and_union(p_hwfn, p_ptt, p_mb_params, seq_num);
-       OSAL_SPIN_UNLOCK(&p_hwfn->mcp_info->cmd_lock);
+       OSAL_MUTEX_RELEASE(&p_hwfn->mcp_info->cmd_lock);
 
        /* Wait for the MFW response */
        do {
@@ -602,7 +602,7 @@ _ecore_mcp_cmd_and_union(struct ecore_hwfn *p_hwfn, struct 
ecore_ptt *p_ptt,
                } else {
                        OSAL_UDELAY(usecs);
                }
-               OSAL_SPIN_LOCK(&p_hwfn->mcp_info->cmd_lock);
+               OSAL_MUTEX_ACQUIRE(&p_hwfn->mcp_info->cmd_lock);
 
                if (p_cmd_elem->b_is_completed)
                        break;
@@ -613,7 +613,7 @@ _ecore_mcp_cmd_and_union(struct ecore_hwfn *p_hwfn, struct 
ecore_ptt *p_ptt,
                else if (rc != ECORE_AGAIN)
                        goto err;
 
-               OSAL_SPIN_UNLOCK(&p_hwfn->mcp_info->cmd_lock);
+               OSAL_MUTEX_RELEASE(&p_hwfn->mcp_info->cmd_lock);
                OSAL_MFW_CMD_PREEMPT(p_hwfn);
        } while (++cnt < max_retries);
 
@@ -623,9 +623,9 @@ _ecore_mcp_cmd_and_union(struct ecore_hwfn *p_hwfn, struct 
ecore_ptt *p_ptt,
                          p_mb_params->cmd, p_mb_params->param);
                ecore_mcp_print_cpu_info(p_hwfn, p_ptt);
 
-               OSAL_SPIN_LOCK(&p_hwfn->mcp_info->cmd_lock);
+               OSAL_MUTEX_ACQUIRE(&p_hwfn->mcp_info->cmd_lock);
                ecore_mcp_cmd_del_elem(p_hwfn, p_cmd_elem);
-               OSAL_SPIN_UNLOCK(&p_hwfn->mcp_info->cmd_lock);
+               OSAL_MUTEX_RELEASE(&p_hwfn->mcp_info->cmd_lock);
 
                if (!ECORE_MB_FLAGS_IS_SET(p_mb_params, AVOID_BLOCK))
                        ecore_mcp_cmd_set_blocking(p_hwfn, true);
@@ -634,7 +634,7 @@ _ecore_mcp_cmd_and_union(struct ecore_hwfn *p_hwfn, struct 
ecore_ptt *p_ptt,
        }
 
        ecore_mcp_cmd_del_elem(p_hwfn, p_cmd_elem);
-       OSAL_SPIN_UNLOCK(&p_hwfn->mcp_info->cmd_lock);
+       OSAL_MUTEX_RELEASE(&p_hwfn->mcp_info->cmd_lock);
 
        DP_VERBOSE(p_hwfn, ECORE_MSG_SP,
                   "MFW mailbox: response 0x%08x param 0x%08x [after %d.%03d 
ms]\n",
@@ -647,7 +647,7 @@ _ecore_mcp_cmd_and_union(struct ecore_hwfn *p_hwfn, struct 
ecore_ptt *p_ptt,
        return ECORE_SUCCESS;
 
 err:
-       OSAL_SPIN_UNLOCK(&p_hwfn->mcp_info->cmd_lock);
+       OSAL_MUTEX_RELEASE(&p_hwfn->mcp_info->cmd_lock);
        return rc;
 }
 
@@ -1439,7 +1439,7 @@ static void ecore_mcp_handle_link_change(struct 
ecore_hwfn *p_hwfn,
        u32 status = 0;
 
        /* Prevent SW/attentions from doing this at the same time */
-       OSAL_SPIN_LOCK(&p_hwfn->mcp_info->link_lock);
+       OSAL_MUTEX_ACQUIRE(&p_hwfn->mcp_info->link_lock);
 
        p_link = &p_hwfn->mcp_info->link_output;
        OSAL_MEMSET(p_link, 0, sizeof(*p_link));
@@ -1585,7 +1585,7 @@ static void ecore_mcp_handle_link_change(struct 
ecore_hwfn *p_hwfn,
 
        OSAL_LINK_UPDATE(p_hwfn, p_ptt);
 out:
-       OSAL_SPIN_UNLOCK(&p_hwfn->mcp_info->link_lock);
+       OSAL_MUTEX_RELEASE(&p_hwfn->mcp_info->link_lock);
 }
 
 enum _ecore_status_t ecore_mcp_set_link(struct ecore_hwfn *p_hwfn,
@@ -1774,7 +1774,7 @@ ecore_mcp_update_bw(struct ecore_hwfn *p_hwfn, struct 
ecore_ptt *p_ptt)
        struct public_func shmem_info;
        u32 resp = 0, param = 0;
 
-       OSAL_SPIN_LOCK(&p_hwfn->mcp_info->link_lock);
+       OSAL_MUTEX_ACQUIRE(&p_hwfn->mcp_info->link_lock);
 
        ecore_mcp_get_shmem_func(p_hwfn, p_ptt, &shmem_info,
                                 MCP_PF_ID(p_hwfn));
@@ -1787,7 +1787,7 @@ ecore_mcp_update_bw(struct ecore_hwfn *p_hwfn, struct 
ecore_ptt *p_ptt)
 
        ecore_configure_pf_max_bandwidth(p_hwfn->p_dev, p_info->bandwidth_max);
 
-       OSAL_SPIN_UNLOCK(&p_hwfn->mcp_info->link_lock);
+       OSAL_MUTEX_RELEASE(&p_hwfn->mcp_info->link_lock);
 
        /* Acknowledge the MFW */
        ecore_mcp_cmd(p_hwfn, p_ptt, DRV_MSG_CODE_BW_UPDATE_ACK, 0, &resp,
diff --git a/sys/dev/qlnx/qlnxe/ecore_mcp.h b/sys/dev/qlnx/qlnxe/ecore_mcp.h
index c94583cdfba3..edb1f9083467 100644
--- a/sys/dev/qlnx/qlnxe/ecore_mcp.h
+++ b/sys/dev/qlnx/qlnxe/ecore_mcp.h
@@ -51,10 +51,10 @@ struct ecore_mcp_info {
        /* List for mailbox commands which were sent and wait for a response */
        osal_list_t cmd_list;
 
-       /* Spinlock used for protecting the access to the mailbox commands list
+       /* Lock used for protecting the access to the mailbox commands list
         * and the sending of the commands.
         */
-       osal_spinlock_t cmd_lock;
+       osal_mutex_t cmd_lock;
 
        /* Flag to indicate whether sending a MFW mailbox command is blocked */
        bool b_block_cmd;
@@ -62,7 +62,7 @@ struct ecore_mcp_info {
        /* Spinlock used for syncing SW link-changes and link-changes
         * originating from attention context.
         */
-       osal_spinlock_t link_lock;
+       osal_mutex_t link_lock;
 
        /* Address of the MCP public area */
        u32 public_base;
diff --git a/sys/dev/qlnx/qlnxe/qlnx_def.h b/sys/dev/qlnx/qlnxe/qlnx_def.h
index 8ac403ab49dc..4342bba89587 100644
--- a/sys/dev/qlnx/qlnxe/qlnx_def.h
+++ b/sys/dev/qlnx/qlnxe/qlnx_def.h
@@ -391,7 +391,7 @@ struct qlnx_host {
 
        int                     msix_count;
 
-       struct mtx              hw_lock;
+       struct sx               hw_lock;
 
        /* debug */
 
diff --git a/sys/dev/qlnx/qlnxe/qlnx_os.c b/sys/dev/qlnx/qlnxe/qlnx_os.c
index de64aaef1b4c..2b3732e748fd 100644
--- a/sys/dev/qlnx/qlnxe/qlnx_os.c
+++ b/sys/dev/qlnx/qlnxe/qlnx_os.c
@@ -763,7 +763,7 @@ qlnx_pci_attach(device_t dev)
 
         ha->pci_dev = dev;
 
-       mtx_init(&ha->hw_lock, "qlnx_hw_lock", MTX_NETWORK_LOCK, MTX_DEF);
+       sx_init(&ha->hw_lock, "qlnx_hw_lock");
 
         ha->flags.lock_init = 1;
 
@@ -1207,6 +1207,7 @@ qlnx_init_hw(qlnx_host_t *ha)
        int                             rval = 0;
        struct ecore_hw_prepare_params  params;
 
+        ha->cdev.ha = ha;
        ecore_init_struct(&ha->cdev);
 
        /* ha->dp_module = ECORE_MSG_PROBE |
@@ -1351,7 +1352,7 @@ qlnx_release(qlnx_host_t *ha)
                 pci_release_msi(dev);
 
         if (ha->flags.lock_init) {
-                mtx_destroy(&ha->hw_lock);
+                sx_destroy(&ha->hw_lock);
         }
 
         if (ha->pci_reg)
@@ -5396,11 +5397,11 @@ qlnx_zalloc(uint32_t size)
 }
 
 void
-qlnx_barrier(void *p_hwfn)
+qlnx_barrier(void *p_dev)
 {
        qlnx_host_t     *ha;
 
-       ha = (qlnx_host_t *)((struct ecore_hwfn *)p_hwfn)->p_dev;
+       ha = ((struct ecore_dev *) p_dev)->ha;
        bus_barrier(ha->pci_reg,  0, 0, BUS_SPACE_BARRIER_WRITE);
 }
 
diff --git a/sys/dev/qlnx/qlnxe/qlnx_os.h b/sys/dev/qlnx/qlnxe/qlnx_os.h
index 261283fb6eaf..6d717d0e70bf 100644
--- a/sys/dev/qlnx/qlnxe/qlnx_os.h
+++ b/sys/dev/qlnx/qlnxe/qlnx_os.h
@@ -130,8 +130,8 @@ MALLOC_DECLARE(M_QLNXBUF);
 /*
  * Locks
  */
-#define QLNX_LOCK(ha)          mtx_lock(&ha->hw_lock)
-#define QLNX_UNLOCK(ha)                mtx_unlock(&ha->hw_lock)
+#define QLNX_LOCK(ha)          sx_xlock(&ha->hw_lock)
+#define QLNX_UNLOCK(ha)                sx_xunlock(&ha->hw_lock)
 
 /*
  * structure encapsulating a DMA buffer

Reply via email to