[PATCH v5 fwctl 0/5] fwctl/bnxt_fwctl: fwctl for Broadcom Netxtreme devices

[PATCH v5 fwctl 0/5] fwctl/bnxt_fwctl: fwctl for Broadcom Netxtreme devices

[Pavan Chebbi]

Introducing bnxt_fwctl which follows along Jason's work [1].
It is an aux bus driver that enables fwctl for Broadcom
NetXtreme 574xx, 575xx and 576xx series chipsets by using
bnxt driver's capability to talk to devices' firmware.

The first patch moves the ULP definitions to a common place
inside include/linux/bnxt/. The second and third patches
refactor and extend the existing bnxt aux bus functions to
be able to add more than one auxiliary device. The last three
patches create an additional bnxt aux device, add bnxt_fwctl,
and the documentation.

v5: In patch #4, removed support for any commands that require additional
DMA buffers to be passed in the command input. Only commands that have
the entire input message inlined in the request, are allowed.

v4: In patch #2 fix use after free of auxdev ID in bnxt_aux_dev_release()
In patch #4, added new entries of Makefile, Kconfig and MAINTAINERS in a
sorted manner.

v3: Addressed Leon's comment on patch#2 to have the lock
cover entire lifespan of the auxdevice, so that it is 100pc
synchronized with auxdev removing.

v2: Addressed Leon's comments as below:
Patch #2: Converted atomic_t based aux device state handling to
lock based functions. Drop passing 'err' to bnxt_ulp_start()
Patch #4: Zero-initialized the fw rpc variable making unwind
safer

v1: Based on the feedback received on v5 here:
https://lore.kernel.org/netdev/20251014081033.1175053-1-pavan.chebbi@broadcom.com/ ,
posting this series to fwctl tree.
This series has the below changes from v5 that was posted to netdev:
Patch #2: Addressed Leon's comment for defensive prograaming style
for aux dev functions
Patch #4: Removed support for commands that don't comply with fwctl
guidelines. Also addressed additional comments from Jonathan
Patch #5: Added info for example python3 program

v5: Addressed the v4's review comments as below:
Patch #2 and #3: Simplified aux bus device creation logic by
having the core maintain arrays of pointers to aux devices and
their contexts, thereby avoiding function calls from aux dev.
[thanks Leon]
Patch #4: Used memdup_user() as suggested by cocci. Addressed
additional review comments from Jonathon and Dave. Collected
Rb tags from Dave.

v4: In patch #4, added the missing kfree on error for response
buffer. Improved documentation in patch #5 based on comments
from Dave.

v3: Addressed the review comments as below
Patch #1: Removed redundant common.h [thanks Saeed]
Patch #2 and #3 merged into a single patch [thanks Jonathan]
Patch #3: Addressed comments from Jonathan
Patch #4 and #5: Addressed comments from Jonathan and Dave

v2: In patch #5, fixed a sparse warning where a __le16 was
degraded to an integer. Also addressed kdoc warnings for
include/uapi/fwctl/bnxt.h in the same patch.

[1] https://lore.kernel.org/netdev/0-v5-642aa0c94070+4447f-fwctl_jgg@nvidia.com/

The following are changes since commit f4d0ec0aa20d49f09dc01d82894ce80d72de0560
    Merge tag 'erofs-for-7.0-rc2-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs
and are available in the git repository at:
  https://github.com/pavanchebbi/linux/tree/fwctl-bnxt-v5

Pavan Chebbi (5):
  fwctl/bnxt_en: Move common definitions to include/linux/bnxt/
  fwctl/bnxt_en: Refactor aux bus functions to be more generic
  fwctl/bnxt_en: Create an aux device for fwctl
  fwctl/bnxt_fwctl: Add bnxt fwctl device
  fwctl/bnxt_fwctl: Add documentation entries

 .../userspace-api/fwctl/bnxt_fwctl.rst        |  83 +++++
 Documentation/userspace-api/fwctl/fwctl.rst   |   1 +
 Documentation/userspace-api/fwctl/index.rst   |   1 +
 MAINTAINERS                                   |   6 +
 drivers/fwctl/Kconfig                         |  11 +
 drivers/fwctl/Makefile                        |   1 +
 drivers/fwctl/bnxt/Makefile                   |   4 +
 drivers/fwctl/bnxt/main.c                     | 278 ++++++++++++++
 drivers/infiniband/hw/bnxt_re/debugfs.c       |   2 +-
 drivers/infiniband/hw/bnxt_re/main.c          |   2 +-
 drivers/infiniband/hw/bnxt_re/qplib_fp.c      |   2 +-
 drivers/infiniband/hw/bnxt_re/qplib_res.h     |   2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  49 ++-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  19 +-
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c |  10 +-
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c |   4 +-
 .../net/ethernet/broadcom/bnxt/bnxt_sriov.c   |   2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 349 +++++++++++-------
 .../bnxt_ulp.h => include/linux/bnxt/ulp.h    |  26 +-
 include/uapi/fwctl/bnxt.h                     |  44 +++
 include/uapi/fwctl/fwctl.h                    |   1 +
 21 files changed, 721 insertions(+), 176 deletions(-)
 create mode 100644 Documentation/userspace-api/fwctl/bnxt_fwctl.rst
 create mode 100644 drivers/fwctl/bnxt/Makefile
 create mode 100644 drivers/fwctl/bnxt/main.c
 rename drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h => include/linux/bnxt/ulp.h (86%)
 create mode 100644 include/uapi/fwctl/bnxt.h

-- 
2.39.1

[PATCH v5 fwctl 1/5] fwctl/bnxt_en: Move common definitions to include/linux/bnxt/

[Pavan Chebbi]

We have common definitions that are now going to be used
by more than one component outside of bnxt (bnxt_re and
fwctl)

Move bnxt_ulp.h to include/linux/bnxt/ as ulp.h.

Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Cc: linux-rdma@vger.kernel.org
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
---
 drivers/infiniband/hw/bnxt_re/debugfs.c                         | 2 +-
 drivers/infiniband/hw/bnxt_re/main.c                            | 2 +-
 drivers/infiniband/hw/bnxt_re/qplib_fp.c                        | 2 +-
 drivers/infiniband/hw/bnxt_re/qplib_res.h                       | 2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.c                       | 2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c               | 2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c               | 2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c                 | 2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c                   | 2 +-
 .../broadcom/bnxt/bnxt_ulp.h => include/linux/bnxt/ulp.h        | 0
 10 files changed, 9 insertions(+), 9 deletions(-)
 rename drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h => include/linux/bnxt/ulp.h (100%)

diff --git a/drivers/infiniband/hw/bnxt_re/debugfs.c b/drivers/infiniband/hw/bnxt_re/debugfs.c
index 88817c86ae24..c57bbe3492a8 100644
--- a/drivers/infiniband/hw/bnxt_re/debugfs.c
+++ b/drivers/infiniband/hw/bnxt_re/debugfs.c
@@ -10,8 +10,8 @@
 #include <linux/pci.h>
 #include <linux/seq_file.h>
 #include <rdma/ib_addr.h>
+#include <linux/bnxt/ulp.h>
 
-#include "bnxt_ulp.h"
 #include "roce_hsi.h"
 #include "qplib_res.h"
 #include "qplib_sp.h"
diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 73003ad25ee8..79ca734a1377 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -55,8 +55,8 @@
 #include <rdma/ib_umem.h>
 #include <rdma/ib_addr.h>
 #include <linux/hashtable.h>
+#include <linux/bnxt/ulp.h>
 
-#include "bnxt_ulp.h"
 #include "roce_hsi.h"
 #include "qplib_res.h"
 #include "qplib_sp.h"
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_fp.c b/drivers/infiniband/hw/bnxt_re/qplib_fp.c
index c88f049136fc..5a28946e4668 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_fp.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_fp.c
@@ -46,6 +46,7 @@
 #include <linux/delay.h>
 #include <linux/prefetch.h>
 #include <linux/if_ether.h>
+#include <linux/bnxt/ulp.h>
 #include <rdma/ib_mad.h>
 
 #include "roce_hsi.h"
@@ -55,7 +56,6 @@
 #include "qplib_sp.h"
 #include "qplib_fp.h"
 #include <rdma/ib_addr.h>
-#include "bnxt_ulp.h"
 #include "bnxt_re.h"
 #include "ib_verbs.h"
 
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_res.h b/drivers/infiniband/hw/bnxt_re/qplib_res.h
index 2ea3b7f232a3..ebe8893937f6 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_res.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_res.h
@@ -39,7 +39,7 @@
 #ifndef __BNXT_QPLIB_RES_H__
 #define __BNXT_QPLIB_RES_H__
 
-#include "bnxt_ulp.h"
+#include <linux/bnxt/ulp.h>
 
 extern const struct bnxt_qplib_gid bnxt_qplib_gid_zero;
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index d17d0ea89c36..4481d80cdfc2 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -59,10 +59,10 @@
 #include <net/netdev_rx_queue.h>
 #include <linux/pci-tph.h>
 #include <linux/bnxt/hsi.h>
+#include <linux/bnxt/ulp.h>
 
 #include "bnxt.h"
 #include "bnxt_hwrm.h"
-#include "bnxt_ulp.h"
 #include "bnxt_sriov.h"
 #include "bnxt_ethtool.h"
 #include "bnxt_dcb.h"
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 15de802bbac4..230cd95d30a2 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -13,12 +13,12 @@
 #include <net/devlink.h>
 #include <net/netdev_lock.h>
 #include <linux/bnxt/hsi.h>
+#include <linux/bnxt/ulp.h>
 #include "bnxt.h"
 #include "bnxt_hwrm.h"
 #include "bnxt_vfr.h"
 #include "bnxt_devlink.h"
 #include "bnxt_ethtool.h"
-#include "bnxt_ulp.h"
 #include "bnxt_ptp.h"
 #include "bnxt_coredump.h"
 #include "bnxt_nvm_defs.h"
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 068e191ede19..8cad7b982664 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -27,9 +27,9 @@
 #include <net/netdev_queues.h>
 #include <net/netlink.h>
 #include <linux/bnxt/hsi.h>
+#include <linux/bnxt/ulp.h>
 #include "bnxt.h"
 #include "bnxt_hwrm.h"
-#include "bnxt_ulp.h"
 #include "bnxt_xdp.h"
 #include "bnxt_ptp.h"
 #include "bnxt_ethtool.h"
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
index be7deb9cc410..83884af32249 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
@@ -17,9 +17,9 @@
 #include <linux/etherdevice.h>
 #include <net/dcbnl.h>
 #include <linux/bnxt/hsi.h>
+#include <linux/bnxt/ulp.h>
 #include "bnxt.h"
 #include "bnxt_hwrm.h"
-#include "bnxt_ulp.h"
 #include "bnxt_sriov.h"
 #include "bnxt_vfr.h"
 #include "bnxt_ethtool.h"
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index 927971c362f1..fa513892db50 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -22,10 +22,10 @@
 #include <linux/auxiliary_bus.h>
 #include <net/netdev_lock.h>
 #include <linux/bnxt/hsi.h>
+#include <linux/bnxt/ulp.h>
 
 #include "bnxt.h"
 #include "bnxt_hwrm.h"
-#include "bnxt_ulp.h"
 
 static DEFINE_IDA(bnxt_aux_dev_ids);
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h b/include/linux/bnxt/ulp.h
similarity index 100%
rename from drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.h
rename to include/linux/bnxt/ulp.h
-- 
2.39.1

[PATCH v5 fwctl 2/5] fwctl/bnxt_en: Refactor aux bus functions to be more generic

[Pavan Chebbi]

Up until now there was only one auxiliary device that bnxt
created and that was for RoCE driver. bnxt fwctl is also
going to use an aux bus device that bnxt should create.
This requires some nomenclature changes and refactoring of
the existing bnxt aux dev functions.

Convert 'aux_priv' and 'edev' members of struct bnxt into
arrays where each element contains supported auxbus device's
data. Move struct bnxt_aux_priv from bnxt.h to ulp.h because
that is where it belongs. Make aux bus init/uninit/add/del
functions more generic which will loop through all the aux
device types. Make bnxt_ulp_start/stop functions (the only
other common functions applicable to any aux device) loop
through the aux devices to update their config and states.
Make callers of bnxt_ulp_start() call it only when there
are no errors.

Also, as an improvement in code, bnxt_register_dev() can skip
unnecessary dereferencing of edev from bp, instead use the
edev pointer from the function parameter.

Future patches will reuse these functions to add an aux bus
device for fwctl.

Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c     |  47 ++-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  19 +-
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c |   8 +-
 .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c |   2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 339 +++++++++++-------
 include/linux/bnxt/ulp.h                      |  25 +-
 6 files changed, 273 insertions(+), 167 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 4481d80cdfc2..a9bfbfabf121 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -6859,7 +6859,8 @@ int bnxt_hwrm_vnic_cfg(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 #endif
 	if ((bp->flags & BNXT_FLAG_STRIP_VLAN) || def_vlan)
 		req->flags |= cpu_to_le32(VNIC_CFG_REQ_FLAGS_VLAN_STRIP_MODE);
-	if (vnic->vnic_id == BNXT_VNIC_DEFAULT && bnxt_ulp_registered(bp->edev))
+	if (vnic->vnic_id == BNXT_VNIC_DEFAULT &&
+	    bnxt_ulp_registered(bp->edev[BNXT_AUXDEV_RDMA]))
 		req->flags |= cpu_to_le32(bnxt_get_roce_vnic_mode(bp));
 
 	return hwrm_req_send(bp, req);
@@ -7974,6 +7975,7 @@ static int bnxt_get_avail_msix(struct bnxt *bp, int num);
 
 static int __bnxt_reserve_rings(struct bnxt *bp)
 {
+	struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
 	struct bnxt_hw_rings hwr = {0};
 	int rx_rings, old_rx_rings, rc;
 	int cp = bp->cp_nr_rings;
@@ -7984,7 +7986,7 @@ static int __bnxt_reserve_rings(struct bnxt *bp)
 	if (!bnxt_need_reserve_rings(bp))
 		return 0;
 
-	if (BNXT_NEW_RM(bp) && !bnxt_ulp_registered(bp->edev)) {
+	if (BNXT_NEW_RM(bp) && !bnxt_ulp_registered(edev)) {
 		ulp_msix = bnxt_get_avail_msix(bp, bp->ulp_num_msix_want);
 		if (!ulp_msix)
 			bnxt_set_ulp_stat_ctxs(bp, 0);
@@ -8035,8 +8037,7 @@ static int __bnxt_reserve_rings(struct bnxt *bp)
 	}
 	rx_rings = min_t(int, rx_rings, hwr.grp);
 	hwr.cp = min_t(int, hwr.cp, bp->cp_nr_rings);
-	if (bnxt_ulp_registered(bp->edev) &&
-	    hwr.stat > bnxt_get_ulp_stat_ctxs(bp))
+	if (bnxt_ulp_registered(edev) && hwr.stat > bnxt_get_ulp_stat_ctxs(bp))
 		hwr.stat -= bnxt_get_ulp_stat_ctxs(bp);
 	hwr.cp = min_t(int, hwr.cp, hwr.stat);
 	rc = bnxt_trim_rings(bp, &rx_rings, &hwr.tx, hwr.cp, sh);
@@ -8075,7 +8076,7 @@ static int __bnxt_reserve_rings(struct bnxt *bp)
 	    !netif_is_rxfh_configured(bp->dev))
 		bnxt_set_dflt_rss_indir_tbl(bp, NULL);
 
-	if (!bnxt_ulp_registered(bp->edev) && BNXT_NEW_RM(bp)) {
+	if (!bnxt_ulp_registered(edev) && BNXT_NEW_RM(bp)) {
 		int resv_msix, resv_ctx, ulp_ctxs;
 		struct bnxt_hw_resc *hw_resc;
 
@@ -11430,6 +11431,7 @@ static void bnxt_clear_int_mode(struct bnxt *bp)
 
 int bnxt_reserve_rings(struct bnxt *bp, bool irq_re_init)
 {
+	struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
 	bool irq_cleared = false;
 	bool irq_change = false;
 	int tcs = bp->num_tc;
@@ -11439,7 +11441,7 @@ int bnxt_reserve_rings(struct bnxt *bp, bool irq_re_init)
 	if (!bnxt_need_reserve_rings(bp))
 		return 0;
 
-	if (BNXT_NEW_RM(bp) && !bnxt_ulp_registered(bp->edev)) {
+	if (BNXT_NEW_RM(bp) && !bnxt_ulp_registered(edev)) {
 		int ulp_msix = bnxt_get_avail_msix(bp, bp->ulp_num_msix_want);
 
 		if (ulp_msix > bp->ulp_num_msix_want)
@@ -14522,7 +14524,7 @@ static void bnxt_fw_echo_reply(struct bnxt *bp)
 static void bnxt_ulp_restart(struct bnxt *bp)
 {
 	bnxt_ulp_stop(bp);
-	bnxt_ulp_start(bp, 0);
+	bnxt_ulp_start(bp);
 }
 
 static void bnxt_sp_task(struct work_struct *work)
@@ -14679,7 +14681,7 @@ int bnxt_check_rings(struct bnxt *bp, int tx, int rx, bool sh, int tcs,
 		hwr.cp_p5 = hwr.tx + rx;
 	rc = bnxt_hwrm_check_rings(bp, &hwr);
 	if (!rc && pci_msix_can_alloc_dyn(bp->pdev)) {
-		if (!bnxt_ulp_registered(bp->edev)) {
+		if (!bnxt_ulp_registered(bp->edev[BNXT_AUXDEV_RDMA])) {
 			hwr.cp += bnxt_get_ulp_msix_num(bp);
 			hwr.cp = min_t(int, hwr.cp, bnxt_get_max_func_irqs(bp));
 		}
@@ -15199,7 +15201,7 @@ static void bnxt_fw_reset_task(struct work_struct *work)
 			bnxt_dl_health_fw_status_update(bp, true);
 		}
 		netdev_unlock(bp->dev);
-		bnxt_ulp_start(bp, 0);
+		bnxt_ulp_start(bp);
 		bnxt_reenable_sriov(bp);
 		netdev_lock(bp->dev);
 		bnxt_vf_reps_alloc(bp);
@@ -15221,7 +15223,8 @@ static void bnxt_fw_reset_task(struct work_struct *work)
 	bnxt_fw_reset_abort(bp, rc);
 	netdev_unlock(bp->dev);
 ulp_start:
-	bnxt_ulp_start(bp, rc);
+	if (!rc)
+		bnxt_ulp_start(bp);
 }
 
 static int bnxt_init_board(struct pci_dev *pdev, struct net_device *dev)
@@ -16211,12 +16214,13 @@ static void bnxt_remove_one(struct pci_dev *pdev)
 	if (BNXT_PF(bp))
 		__bnxt_sriov_disable(bp);
 
-	bnxt_rdma_aux_device_del(bp);
+	bnxt_aux_devices_del(bp);
 
 	unregister_netdev(dev);
 	bnxt_ptp_clear(bp);
 
-	bnxt_rdma_aux_device_uninit(bp);
+	bnxt_aux_devices_uninit(bp);
+	bnxt_auxdev_id_free(bp, bp->auxdev_id);
 
 	bnxt_free_l2_filters(bp, true);
 	bnxt_free_ntp_fltrs(bp, true);
@@ -16802,7 +16806,9 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	bnxt_set_tpa_flags(bp);
 	bnxt_init_ring_params(bp);
 	bnxt_set_ring_params(bp);
-	bnxt_rdma_aux_device_init(bp);
+	mutex_init(&bp->auxdev_lock);
+	if (!bnxt_auxdev_id_alloc(bp))
+		bnxt_aux_devices_init(bp);
 	rc = bnxt_set_dflt_rings(bp, true);
 	if (rc) {
 		if (BNXT_VF(bp) && rc == -ENODEV) {
@@ -16866,7 +16872,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	bnxt_dl_fw_reporters_create(bp);
 
-	bnxt_rdma_aux_device_add(bp);
+	bnxt_aux_devices_add(bp);
 
 	bnxt_print_device_info(bp);
 
@@ -16874,7 +16880,8 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	return 0;
 init_err_cleanup:
-	bnxt_rdma_aux_device_uninit(bp);
+	bnxt_aux_devices_uninit(bp);
+	bnxt_auxdev_id_free(bp, bp->auxdev_id);
 	bnxt_dl_unregister(bp);
 init_err_dl:
 	bnxt_shutdown_tc(bp);
@@ -17008,9 +17015,10 @@ static int bnxt_resume(struct device *device)
 
 resume_exit:
 	netdev_unlock(bp->dev);
-	bnxt_ulp_start(bp, rc);
-	if (!rc)
+	if (!rc) {
+		bnxt_ulp_start(bp);
 		bnxt_reenable_sriov(bp);
+	}
 	return rc;
 }
 
@@ -17190,9 +17198,10 @@ static void bnxt_io_resume(struct pci_dev *pdev)
 		netif_device_attach(netdev);
 
 	netdev_unlock(netdev);
-	bnxt_ulp_start(bp, err);
-	if (!err)
+	if (!err) {
+		bnxt_ulp_start(bp);
 		bnxt_reenable_sriov(bp);
+	}
 }
 
 static const struct pci_error_handlers bnxt_err_handler = {
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index f5f07a7e6b29..a739e7d52bdc 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -24,12 +24,12 @@
 #include <linux/interrupt.h>
 #include <linux/rhashtable.h>
 #include <linux/crash_dump.h>
-#include <linux/auxiliary_bus.h>
 #include <net/devlink.h>
 #include <net/dst_metadata.h>
 #include <net/xdp.h>
 #include <linux/dim.h>
 #include <linux/io-64-nonatomic-lo-hi.h>
+#include <linux/bnxt/ulp.h>
 #ifdef CONFIG_TEE_BNXT_FW
 #include <linux/firmware/broadcom/tee_bnxt_fw.h>
 #endif
@@ -2076,12 +2076,6 @@ struct bnxt_fw_health {
 #define BNXT_FW_IF_RETRY		10
 #define BNXT_FW_SLOT_RESET_RETRY	4
 
-struct bnxt_aux_priv {
-	struct auxiliary_device aux_dev;
-	struct bnxt_en_dev *edev;
-	int id;
-};
-
 enum board_idx {
 	BCM57301,
 	BCM57302,
@@ -2341,8 +2335,8 @@ struct bnxt {
 #define BNXT_CHIP_P5_AND_MINUS(bp)		\
 	(BNXT_CHIP_P3(bp) || BNXT_CHIP_P4(bp) || BNXT_CHIP_P5(bp))
 
-	struct bnxt_aux_priv	*aux_priv;
-	struct bnxt_en_dev	*edev;
+	struct bnxt_aux_priv	*aux_priv[__BNXT_AUXDEV_MAX];
+	struct bnxt_en_dev	*edev[__BNXT_AUXDEV_MAX];
 
 	struct bnxt_napi	**bnapi;
 
@@ -2751,6 +2745,13 @@ struct bnxt {
 	struct bnxt_ctx_pg_info	*fw_crash_mem;
 	u32			fw_crash_len;
 	struct bnxt_bs_trace_info bs_trace[BNXT_TRACE_MAX];
+	int			auxdev_id;
+	/* synchronize validity checks of available aux devices */
+	struct mutex		auxdev_lock;
+	u8			auxdev_state[__BNXT_AUXDEV_MAX];
+#define	BNXT_ADEV_STATE_NONE	0
+#define	BNXT_ADEV_STATE_INIT	1
+#define	BNXT_ADEV_STATE_ADD	2
 };
 
 #define BNXT_NUM_RX_RING_STATS			8
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 230cd95d30a2..835f2b413931 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -440,13 +440,13 @@ static int bnxt_dl_reload_down(struct devlink *dl, bool netns_change,
 					   "reload is unsupported while VFs are allocated or being configured");
 			netdev_unlock(bp->dev);
 			rtnl_unlock();
-			bnxt_ulp_start(bp, 0);
+			bnxt_ulp_start(bp);
 			return -EOPNOTSUPP;
 		}
 		if (bp->dev->reg_state == NETREG_UNREGISTERED) {
 			netdev_unlock(bp->dev);
 			rtnl_unlock();
-			bnxt_ulp_start(bp, 0);
+			bnxt_ulp_start(bp);
 			return -ENODEV;
 		}
 		if (netif_running(bp->dev))
@@ -578,8 +578,8 @@ static int bnxt_dl_reload_up(struct devlink *dl, enum devlink_reload_action acti
 	}
 	netdev_unlock(bp->dev);
 	rtnl_unlock();
-	if (action == DEVLINK_RELOAD_ACTION_DRIVER_REINIT)
-		bnxt_ulp_start(bp, rc);
+	if (!rc && action == DEVLINK_RELOAD_ACTION_DRIVER_REINIT)
+		bnxt_ulp_start(bp);
 	return rc;
 }
 
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 8cad7b982664..064d7bc4ce8d 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -5100,7 +5100,7 @@ static void bnxt_self_test(struct net_device *dev, struct ethtool_test *etest,
 
 	memset(buf, 0, sizeof(u64) * bp->num_tests);
 	if (etest->flags & ETH_TEST_FL_OFFLINE &&
-	    bnxt_ulp_registered(bp->edev)) {
+	    bnxt_ulp_registered(bp->edev[BNXT_AUXDEV_RDMA])) {
 		etest->flags |= ETH_TEST_FL_FAILED;
 		netdev_warn(dev, "Offline tests cannot be run with RoCE driver loaded\n");
 		return;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index fa513892db50..d05a1c62fd0b 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -29,9 +29,32 @@
 
 static DEFINE_IDA(bnxt_aux_dev_ids);
 
+struct bnxt_aux_device {
+	const char *name;
+};
+
+static void bnxt_auxdev_set_state(struct bnxt *bp, int idx, int state)
+{
+	bp->auxdev_state[idx] = state;
+}
+
+static bool bnxt_auxdev_is_init(struct bnxt *bp, int idx)
+{
+	return (bp->auxdev_state[idx] == BNXT_ADEV_STATE_INIT);
+}
+
+static bool bnxt_auxdev_is_active(struct bnxt *bp, int idx)
+{
+	return (bp->auxdev_state[idx] == BNXT_ADEV_STATE_ADD);
+}
+
+static struct bnxt_aux_device bnxt_aux_devices[__BNXT_AUXDEV_MAX] = {{
+	.name		= "rdma",
+}};
+
 static void bnxt_fill_msix_vecs(struct bnxt *bp, struct bnxt_msix_entry *ent)
 {
-	struct bnxt_en_dev *edev = bp->edev;
+	struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
 	int num_msix, i;
 
 	if (!edev->ulp_tbl->msix_requested) {
@@ -51,61 +74,75 @@ static void bnxt_fill_msix_vecs(struct bnxt *bp, struct bnxt_msix_entry *ent)
 
 int bnxt_get_ulp_msix_num(struct bnxt *bp)
 {
-	if (bp->edev)
-		return bp->edev->ulp_num_msix_vec;
+	struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
+
+	if (edev)
+		return edev->ulp_num_msix_vec;
 	return 0;
 }
 
 void bnxt_set_ulp_msix_num(struct bnxt *bp, int num)
 {
-	if (bp->edev)
-		bp->edev->ulp_num_msix_vec = num;
+	struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
+
+	if (edev)
+		edev->ulp_num_msix_vec = num;
 }
 
 int bnxt_get_ulp_msix_num_in_use(struct bnxt *bp)
 {
-	if (bnxt_ulp_registered(bp->edev))
-		return bp->edev->ulp_num_msix_vec;
+	struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
+
+	if (bnxt_ulp_registered(edev))
+		return edev->ulp_num_msix_vec;
 	return 0;
 }
 
 int bnxt_get_ulp_stat_ctxs(struct bnxt *bp)
 {
-	if (bp->edev)
-		return bp->edev->ulp_num_ctxs;
+	struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
+
+	if (edev)
+		return edev->ulp_num_ctxs;
 	return 0;
 }
 
 void bnxt_set_ulp_stat_ctxs(struct bnxt *bp, int num_ulp_ctx)
 {
-	if (bp->edev)
-		bp->edev->ulp_num_ctxs = num_ulp_ctx;
+	struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
+
+	if (edev)
+		edev->ulp_num_ctxs = num_ulp_ctx;
 }
 
 int bnxt_get_ulp_stat_ctxs_in_use(struct bnxt *bp)
 {
-	if (bnxt_ulp_registered(bp->edev))
-		return bp->edev->ulp_num_ctxs;
+	struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
+
+	if (bnxt_ulp_registered(edev))
+		return edev->ulp_num_ctxs;
 	return 0;
 }
 
 void bnxt_set_dflt_ulp_stat_ctxs(struct bnxt *bp)
 {
-	if (bp->edev) {
-		bp->edev->ulp_num_ctxs = BNXT_MIN_ROCE_STAT_CTXS;
+	struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
+
+	if (edev) {
+		edev->ulp_num_ctxs = BNXT_MIN_ROCE_STAT_CTXS;
 		/* Reserve one additional stat_ctx for PF0 (except
 		 * on 1-port NICs) as it also creates one stat_ctx
 		 * for PF1 in case of RoCE bonding.
 		 */
 		if (BNXT_PF(bp) && !bp->pf.port_id &&
 		    bp->port_count > 1)
-			bp->edev->ulp_num_ctxs++;
+			edev->ulp_num_ctxs++;
 
 		/* Reserve one additional stat_ctx when the device is capable
 		 * of supporting port mirroring on RDMA device.
 		 */
 		if (BNXT_MIRROR_ON_ROCE_CAP(bp))
-			bp->edev->ulp_num_ctxs++;
+			edev->ulp_num_ctxs++;
 	}
 }
 
@@ -141,7 +178,7 @@ int bnxt_register_dev(struct bnxt_en_dev *edev,
 
 	edev->ulp_tbl->msix_requested = bnxt_get_ulp_msix_num(bp);
 
-	bnxt_fill_msix_vecs(bp, bp->edev->msix_entries);
+	bnxt_fill_msix_vecs(bp, edev->msix_entries);
 exit:
 	mutex_unlock(&edev->en_dev_lock);
 	netdev_unlock(dev);
@@ -227,20 +264,27 @@ EXPORT_SYMBOL(bnxt_send_msg);
 
 void bnxt_ulp_stop(struct bnxt *bp)
 {
-	struct bnxt_aux_priv *aux_priv = bp->aux_priv;
-	struct bnxt_en_dev *edev = bp->edev;
+	int i;
 
-	if (!edev)
-		return;
-
-	mutex_lock(&edev->en_dev_lock);
-	if (!bnxt_ulp_registered(edev) ||
-	    (edev->flags & BNXT_EN_FLAG_ULP_STOPPED))
-		goto ulp_stop_exit;
-
-	edev->flags |= BNXT_EN_FLAG_ULP_STOPPED;
-	if (aux_priv) {
+	mutex_lock(&bp->auxdev_lock);
+	for (i = 0; i < __BNXT_AUXDEV_MAX; i++) {
+		struct bnxt_aux_priv *aux_priv;
 		struct auxiliary_device *adev;
+		struct bnxt_en_dev *edev;
+
+		if (!bnxt_auxdev_is_active(bp, i))
+			continue;
+
+		aux_priv = bp->aux_priv[i];
+		edev = bp->edev[i];
+		mutex_lock(&edev->en_dev_lock);
+		if (!bnxt_ulp_registered(edev) ||
+		    (edev->flags & BNXT_EN_FLAG_ULP_STOPPED)) {
+			mutex_unlock(&edev->en_dev_lock);
+			continue;
+		}
+
+		edev->flags |= BNXT_EN_FLAG_ULP_STOPPED;
 
 		adev = &aux_priv->aux_dev;
 		if (adev->dev.driver) {
@@ -251,29 +295,35 @@ void bnxt_ulp_stop(struct bnxt *bp)
 			edev->en_state = bp->state;
 			adrv->suspend(adev, pm);
 		}
+		mutex_unlock(&edev->en_dev_lock);
 	}
-ulp_stop_exit:
-	mutex_unlock(&edev->en_dev_lock);
+	mutex_unlock(&bp->auxdev_lock);
 }
 
-void bnxt_ulp_start(struct bnxt *bp, int err)
+void bnxt_ulp_start(struct bnxt *bp)
 {
-	struct bnxt_aux_priv *aux_priv = bp->aux_priv;
-	struct bnxt_en_dev *edev = bp->edev;
+	int i;
 
-	if (!edev || err)
-		return;
+	mutex_lock(&bp->auxdev_lock);
+	for (i = 0; i < __BNXT_AUXDEV_MAX; i++) {
+		struct bnxt_aux_priv *aux_priv;
+		struct auxiliary_device *adev;
+		struct bnxt_en_dev *edev;
 
-	mutex_lock(&edev->en_dev_lock);
-	if (!bnxt_ulp_registered(edev) ||
-	    !(edev->flags & BNXT_EN_FLAG_ULP_STOPPED))
-		goto ulp_start_exit;
+		if (!bnxt_auxdev_is_active(bp, i))
+			continue;
 
-	if (edev->ulp_tbl->msix_requested)
-		bnxt_fill_msix_vecs(bp, edev->msix_entries);
+		aux_priv = bp->aux_priv[i];
+		edev = bp->edev[i];
+		mutex_lock(&edev->en_dev_lock);
+		if (!bnxt_ulp_registered(edev) ||
+		    !(edev->flags & BNXT_EN_FLAG_ULP_STOPPED)) {
+			goto clear_flag_continue;
+		}
+
+		if (edev->ulp_tbl->msix_requested)
+			bnxt_fill_msix_vecs(bp, edev->msix_entries);
 
-	if (aux_priv) {
-		struct auxiliary_device *adev;
 
 		adev = &aux_priv->aux_dev;
 		if (adev->dev.driver) {
@@ -283,22 +333,23 @@ void bnxt_ulp_start(struct bnxt *bp, int err)
 			edev->en_state = bp->state;
 			adrv->resume(adev);
 		}
+clear_flag_continue:
+		edev->flags &= ~BNXT_EN_FLAG_ULP_STOPPED;
+		mutex_unlock(&edev->en_dev_lock);
 	}
-ulp_start_exit:
-	edev->flags &= ~BNXT_EN_FLAG_ULP_STOPPED;
-	mutex_unlock(&edev->en_dev_lock);
+	mutex_unlock(&bp->auxdev_lock);
 }
 
 void bnxt_ulp_irq_stop(struct bnxt *bp)
 {
-	struct bnxt_en_dev *edev = bp->edev;
+	struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
 	struct bnxt_ulp_ops *ops;
 	bool reset = false;
 
 	if (!edev)
 		return;
 
-	if (bnxt_ulp_registered(bp->edev)) {
+	if (bnxt_ulp_registered(edev)) {
 		struct bnxt_ulp *ulp = edev->ulp_tbl;
 
 		if (!ulp->msix_requested)
@@ -315,13 +366,13 @@ void bnxt_ulp_irq_stop(struct bnxt *bp)
 
 void bnxt_ulp_irq_restart(struct bnxt *bp, int err)
 {
-	struct bnxt_en_dev *edev = bp->edev;
+	struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
 	struct bnxt_ulp_ops *ops;
 
 	if (!edev)
 		return;
 
-	if (bnxt_ulp_registered(bp->edev)) {
+	if (bnxt_ulp_registered(edev)) {
 		struct bnxt_ulp *ulp = edev->ulp_tbl;
 		struct bnxt_msix_entry *ent = NULL;
 
@@ -347,7 +398,7 @@ void bnxt_ulp_irq_restart(struct bnxt *bp, int err)
 void bnxt_ulp_async_events(struct bnxt *bp, struct hwrm_async_event_cmpl *cmpl)
 {
 	u16 event_id = le16_to_cpu(cmpl->event_id);
-	struct bnxt_en_dev *edev = bp->edev;
+	struct bnxt_en_dev *edev = bp->edev[BNXT_AUXDEV_RDMA];
 	struct bnxt_ulp_ops *ops;
 	struct bnxt_ulp *ulp;
 
@@ -388,18 +439,21 @@ void bnxt_register_async_events(struct bnxt_en_dev *edev,
 }
 EXPORT_SYMBOL(bnxt_register_async_events);
 
-void bnxt_rdma_aux_device_uninit(struct bnxt *bp)
+void bnxt_aux_devices_uninit(struct bnxt *bp)
 {
 	struct bnxt_aux_priv *aux_priv;
 	struct auxiliary_device *adev;
-
-	/* Skip if no auxiliary device init was done. */
-	if (!bp->aux_priv)
-		return;
-
-	aux_priv = bp->aux_priv;
-	adev = &aux_priv->aux_dev;
-	auxiliary_device_uninit(adev);
+	int idx;
+
+	mutex_lock(&bp->auxdev_lock);
+	for (idx = 0; idx < __BNXT_AUXDEV_MAX; idx++) {
+		if (bnxt_auxdev_is_init(bp, idx)) {
+			aux_priv = bp->aux_priv[idx];
+			adev = &aux_priv->aux_dev;
+			auxiliary_device_uninit(adev);
+		}
+	}
+	mutex_unlock(&bp->auxdev_lock);
 }
 
 static void bnxt_aux_dev_release(struct device *dev)
@@ -408,20 +462,25 @@ static void bnxt_aux_dev_release(struct device *dev)
 		container_of(dev, struct bnxt_aux_priv, aux_dev.dev);
 	struct bnxt *bp = netdev_priv(aux_priv->edev->net);
 
-	ida_free(&bnxt_aux_dev_ids, aux_priv->id);
 	kfree(aux_priv->edev->ulp_tbl);
-	bp->edev = NULL;
+	bp->edev[aux_priv->id] = NULL;
 	kfree(aux_priv->edev);
+	bp->aux_priv[aux_priv->id] = NULL;
 	kfree(aux_priv);
-	bp->aux_priv = NULL;
 }
 
-void bnxt_rdma_aux_device_del(struct bnxt *bp)
+void bnxt_aux_devices_del(struct bnxt *bp)
 {
-	if (!bp->edev)
-		return;
+	int idx;
 
-	auxiliary_device_delete(&bp->aux_priv->aux_dev);
+	mutex_lock(&bp->auxdev_lock);
+	for (idx = 0; idx < __BNXT_AUXDEV_MAX; idx++) {
+		if (bnxt_auxdev_is_active(bp, idx)) {
+			auxiliary_device_delete(&bp->aux_priv[idx]->aux_dev);
+			bnxt_auxdev_set_state(bp, idx, BNXT_ADEV_STATE_INIT);
+		}
+	}
+	mutex_unlock(&bp->auxdev_lock);
 }
 
 static void bnxt_set_edev_info(struct bnxt_en_dev *edev, struct bnxt *bp)
@@ -451,83 +510,105 @@ static void bnxt_set_edev_info(struct bnxt_en_dev *edev, struct bnxt *bp)
 	edev->bar0 = bp->bar0;
 }
 
-void bnxt_rdma_aux_device_add(struct bnxt *bp)
+void bnxt_aux_devices_add(struct bnxt *bp)
 {
 	struct auxiliary_device *aux_dev;
-	int rc;
-
-	if (!bp->edev)
-		return;
-
-	aux_dev = &bp->aux_priv->aux_dev;
-	rc = auxiliary_device_add(aux_dev);
-	if (rc) {
-		netdev_warn(bp->dev, "Failed to add auxiliary device for ROCE\n");
-		auxiliary_device_uninit(aux_dev);
-		bp->flags &= ~BNXT_FLAG_ROCE_CAP;
+	int rc, idx;
+
+	mutex_lock(&bp->auxdev_lock);
+	for (idx = 0; idx < __BNXT_AUXDEV_MAX; idx++) {
+		if (bnxt_auxdev_is_init(bp, idx)) {
+			aux_dev = &bp->aux_priv[idx]->aux_dev;
+			rc = auxiliary_device_add(aux_dev);
+			if (rc) {
+				netdev_warn(bp->dev, "Failed to add auxiliary device for ROCE\n");
+				auxiliary_device_uninit(aux_dev);
+				if (idx == BNXT_AUXDEV_RDMA)
+					bp->flags &= ~BNXT_FLAG_ROCE_CAP;
+				continue;
+			}
+			bnxt_auxdev_set_state(bp, idx, BNXT_ADEV_STATE_ADD);
+		}
 	}
+	mutex_unlock(&bp->auxdev_lock);
 }
 
-void bnxt_rdma_aux_device_init(struct bnxt *bp)
+void bnxt_aux_devices_init(struct bnxt *bp)
 {
 	struct auxiliary_device *aux_dev;
 	struct bnxt_aux_priv *aux_priv;
 	struct bnxt_en_dev *edev;
 	struct bnxt_ulp *ulp;
-	int rc;
+	int rc, idx;
+
+	mutex_lock(&bp->auxdev_lock);
+	for (idx = 0; idx < __BNXT_AUXDEV_MAX; idx++) {
+		bnxt_auxdev_set_state(bp, idx, BNXT_ADEV_STATE_NONE);
+
+		if (idx == BNXT_AUXDEV_RDMA &&
+		    !(bp->flags & BNXT_FLAG_ROCE_CAP))
+			continue;
+
+		aux_priv = kzalloc(sizeof(*aux_priv), GFP_KERNEL);
+		if (!aux_priv)
+			goto next_auxdev;
+
+		aux_dev = &aux_priv->aux_dev;
+		aux_dev->id = bp->auxdev_id;
+		aux_dev->name = bnxt_aux_devices[idx].name;
+		aux_dev->dev.parent = &bp->pdev->dev;
+		aux_dev->dev.release = bnxt_aux_dev_release;
+
+		rc = auxiliary_device_init(aux_dev);
+		if (rc) {
+			kfree(aux_priv);
+			goto next_auxdev;
+		}
+		bp->aux_priv[idx] = aux_priv;
 
-	if (!(bp->flags & BNXT_FLAG_ROCE_CAP))
-		return;
+		/* From this point, all cleanup will happen via the .release
+		 * callback & any error unwinding will need to include a call
+		 * to auxiliary_device_uninit.
+		 */
+		edev = kzalloc(sizeof(*edev), GFP_KERNEL);
+		if (!edev)
+			goto aux_dev_uninit;
 
-	aux_priv = kzalloc(sizeof(*bp->aux_priv), GFP_KERNEL);
-	if (!aux_priv)
-		goto exit;
+		aux_priv->edev = edev;
+		bnxt_set_edev_info(edev, bp);
 
-	aux_priv->id = ida_alloc(&bnxt_aux_dev_ids, GFP_KERNEL);
-	if (aux_priv->id < 0) {
-		netdev_warn(bp->dev,
-			    "ida alloc failed for ROCE auxiliary device\n");
-		kfree(aux_priv);
-		goto exit;
-	}
+		ulp = kzalloc(sizeof(*ulp), GFP_KERNEL);
+		if (!ulp)
+			goto aux_dev_uninit;
 
-	aux_dev = &aux_priv->aux_dev;
-	aux_dev->id = aux_priv->id;
-	aux_dev->name = "rdma";
-	aux_dev->dev.parent = &bp->pdev->dev;
-	aux_dev->dev.release = bnxt_aux_dev_release;
+		edev->ulp_tbl = ulp;
+		bp->edev[idx] = edev;
+		if (idx == BNXT_AUXDEV_RDMA)
+			bp->ulp_num_msix_want = bnxt_set_dflt_ulp_msix(bp);
+		aux_priv->id = idx;
+		bnxt_auxdev_set_state(bp, idx, BNXT_ADEV_STATE_INIT);
 
-	rc = auxiliary_device_init(aux_dev);
-	if (rc) {
-		ida_free(&bnxt_aux_dev_ids, aux_priv->id);
-		kfree(aux_priv);
-		goto exit;
+		continue;
+aux_dev_uninit:
+		auxiliary_device_uninit(aux_dev);
+next_auxdev:
+		if (idx == BNXT_AUXDEV_RDMA)
+			bp->flags &= ~BNXT_FLAG_ROCE_CAP;
 	}
-	bp->aux_priv = aux_priv;
-
-	/* From this point, all cleanup will happen via the .release callback &
-	 * any error unwinding will need to include a call to
-	 * auxiliary_device_uninit.
-	 */
-	edev = kzalloc(sizeof(*edev), GFP_KERNEL);
-	if (!edev)
-		goto aux_dev_uninit;
-
-	aux_priv->edev = edev;
-
-	ulp = kzalloc(sizeof(*ulp), GFP_KERNEL);
-	if (!ulp)
-		goto aux_dev_uninit;
+	mutex_unlock(&bp->auxdev_lock);
+}
 
-	edev->ulp_tbl = ulp;
-	bp->edev = edev;
-	bnxt_set_edev_info(edev, bp);
-	bp->ulp_num_msix_want = bnxt_set_dflt_ulp_msix(bp);
+int bnxt_auxdev_id_alloc(struct bnxt *bp)
+{
+	bp->auxdev_id = ida_alloc(&bnxt_aux_dev_ids, GFP_KERNEL);
+	if (bp->auxdev_id < 0)
+		return bp->auxdev_id;
 
-	return;
+	return 0;
+}
 
-aux_dev_uninit:
-	auxiliary_device_uninit(aux_dev);
-exit:
-	bp->flags &= ~BNXT_FLAG_ROCE_CAP;
+void bnxt_auxdev_id_free(struct bnxt *bp, int id)
+{
+	if (bp->auxdev_id >= 0)
+		ida_free(&bnxt_aux_dev_ids, id);
 }
diff --git a/include/linux/bnxt/ulp.h b/include/linux/bnxt/ulp.h
index 3c5b8a53f715..1a4643c46f86 100644
--- a/include/linux/bnxt/ulp.h
+++ b/include/linux/bnxt/ulp.h
@@ -10,6 +10,8 @@
 #ifndef BNXT_ULP_H
 #define BNXT_ULP_H
 
+#include <linux/auxiliary_bus.h>
+
 #define BNXT_MIN_ROCE_CP_RINGS	2
 #define BNXT_MIN_ROCE_STAT_CTXS	1
 
@@ -20,6 +22,17 @@
 struct hwrm_async_event_cmpl;
 struct bnxt;
 
+enum bnxt_auxdev_type {
+	BNXT_AUXDEV_RDMA = 0,
+	__BNXT_AUXDEV_MAX
+};
+
+struct bnxt_aux_priv {
+	struct auxiliary_device aux_dev;
+	struct bnxt_en_dev *edev;
+	int id;
+};
+
 struct bnxt_msix_entry {
 	u32	vector;
 	u32	ring_idx;
@@ -110,19 +123,21 @@ void bnxt_set_ulp_stat_ctxs(struct bnxt *bp, int num_ctxs);
 int bnxt_get_ulp_stat_ctxs_in_use(struct bnxt *bp);
 void bnxt_set_dflt_ulp_stat_ctxs(struct bnxt *bp);
 void bnxt_ulp_stop(struct bnxt *bp);
-void bnxt_ulp_start(struct bnxt *bp, int err);
+void bnxt_ulp_start(struct bnxt *bp);
 void bnxt_ulp_sriov_cfg(struct bnxt *bp, int num_vfs);
 void bnxt_ulp_irq_stop(struct bnxt *bp);
 void bnxt_ulp_irq_restart(struct bnxt *bp, int err);
 void bnxt_ulp_async_events(struct bnxt *bp, struct hwrm_async_event_cmpl *cmpl);
-void bnxt_rdma_aux_device_uninit(struct bnxt *bp);
-void bnxt_rdma_aux_device_del(struct bnxt *bp);
-void bnxt_rdma_aux_device_add(struct bnxt *bp);
-void bnxt_rdma_aux_device_init(struct bnxt *bp);
+void bnxt_aux_devices_uninit(struct bnxt *bp);
+void bnxt_aux_devices_del(struct bnxt *bp);
+void bnxt_aux_devices_add(struct bnxt *bp);
+void bnxt_aux_devices_init(struct bnxt *bp);
 int bnxt_register_dev(struct bnxt_en_dev *edev, struct bnxt_ulp_ops *ulp_ops,
 		      void *handle);
 void bnxt_unregister_dev(struct bnxt_en_dev *edev);
 int bnxt_send_msg(struct bnxt_en_dev *edev, struct bnxt_fw_msg *fw_msg);
 void bnxt_register_async_events(struct bnxt_en_dev *edev,
 				unsigned long *events_bmap, u16 max_id);
+int bnxt_auxdev_id_alloc(struct bnxt *bp);
+void bnxt_auxdev_id_free(struct bnxt *bp, int id);
 #endif
-- 
2.39.1

[PATCH v5 fwctl 3/5] fwctl/bnxt_en: Create an aux device for fwctl

[Pavan Chebbi]

Create an additional auxiliary device to support fwctl.
The next patch will create bnxt_fwctl and bind to this
device.

Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 12 ++++++++++--
 include/linux/bnxt/ulp.h                      |  1 +
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index d05a1c62fd0b..9eaf01177efb 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -50,6 +50,8 @@ static bool bnxt_auxdev_is_active(struct bnxt *bp, int idx)
 
 static struct bnxt_aux_device bnxt_aux_devices[__BNXT_AUXDEV_MAX] = {{
 	.name		= "rdma",
+}, {
+	.name		= "fwctl",
 }};
 
 static void bnxt_fill_msix_vecs(struct bnxt *bp, struct bnxt_msix_entry *ent)
@@ -278,6 +280,11 @@ void bnxt_ulp_stop(struct bnxt *bp)
 		aux_priv = bp->aux_priv[i];
 		edev = bp->edev[i];
 		mutex_lock(&edev->en_dev_lock);
+		if (i == BNXT_AUXDEV_FWCTL) {
+			edev->flags |= BNXT_EN_FLAG_ULP_STOPPED;
+			mutex_unlock(&edev->en_dev_lock);
+			continue;
+		}
 		if (!bnxt_ulp_registered(edev) ||
 		    (edev->flags & BNXT_EN_FLAG_ULP_STOPPED)) {
 			mutex_unlock(&edev->en_dev_lock);
@@ -316,7 +323,7 @@ void bnxt_ulp_start(struct bnxt *bp)
 		aux_priv = bp->aux_priv[i];
 		edev = bp->edev[i];
 		mutex_lock(&edev->en_dev_lock);
-		if (!bnxt_ulp_registered(edev) ||
+		if (i == BNXT_AUXDEV_FWCTL || !bnxt_ulp_registered(edev) ||
 		    !(edev->flags & BNXT_EN_FLAG_ULP_STOPPED)) {
 			goto clear_flag_continue;
 		}
@@ -521,7 +528,8 @@ void bnxt_aux_devices_add(struct bnxt *bp)
 			aux_dev = &bp->aux_priv[idx]->aux_dev;
 			rc = auxiliary_device_add(aux_dev);
 			if (rc) {
-				netdev_warn(bp->dev, "Failed to add auxiliary device for ROCE\n");
+				netdev_warn(bp->dev, "Failed to add auxiliary device for auxdev type %d\n",
+					    idx);
 				auxiliary_device_uninit(aux_dev);
 				if (idx == BNXT_AUXDEV_RDMA)
 					bp->flags &= ~BNXT_FLAG_ROCE_CAP;
diff --git a/include/linux/bnxt/ulp.h b/include/linux/bnxt/ulp.h
index 1a4643c46f86..0851ad3394b0 100644
--- a/include/linux/bnxt/ulp.h
+++ b/include/linux/bnxt/ulp.h
@@ -24,6 +24,7 @@ struct bnxt;
 
 enum bnxt_auxdev_type {
 	BNXT_AUXDEV_RDMA = 0,
+	BNXT_AUXDEV_FWCTL,
 	__BNXT_AUXDEV_MAX
 };
 
-- 
2.39.1

[PATCH v5 fwctl 4/5] fwctl/bnxt_fwctl: Add bnxt fwctl device

[Pavan Chebbi]

Create bnxt_fwctl device. This will bind to bnxt's aux device.
On the upper edge, it will register with the fwctl subsystem.
It will make use of bnxt's ULP functions to send FW commands.

Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
---
 MAINTAINERS                 |   6 +
 drivers/fwctl/Kconfig       |  11 ++
 drivers/fwctl/Makefile      |   1 +
 drivers/fwctl/bnxt/Makefile |   4 +
 drivers/fwctl/bnxt/main.c   | 278 ++++++++++++++++++++++++++++++++++++
 include/uapi/fwctl/bnxt.h   |  44 ++++++
 include/uapi/fwctl/fwctl.h  |   1 +
 7 files changed, 345 insertions(+)
 create mode 100644 drivers/fwctl/bnxt/Makefile
 create mode 100644 drivers/fwctl/bnxt/main.c
 create mode 100644 include/uapi/fwctl/bnxt.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 12f49de7fe03..15cc43ccb919 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10395,6 +10395,12 @@ F:	drivers/fwctl/
 F:	include/linux/fwctl.h
 F:	include/uapi/fwctl/
 
+FWCTL BNXT DRIVER
+M:	Pavan Chebbi <pavan.chebbi@broadcom.com>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	drivers/fwctl/bnxt/
+
 FWCTL MLX5 DRIVER
 M:	Saeed Mahameed <saeedm@nvidia.com>
 R:	Itay Avraham <itayavr@nvidia.com>
diff --git a/drivers/fwctl/Kconfig b/drivers/fwctl/Kconfig
index b5583b12a011..d1b1925bdaec 100644
--- a/drivers/fwctl/Kconfig
+++ b/drivers/fwctl/Kconfig
@@ -9,6 +9,17 @@ menuconfig FWCTL
 	  fit neatly into an existing subsystem.
 
 if FWCTL
+config FWCTL_BNXT
+	tristate "bnxt control fwctl driver"
+	depends on BNXT
+	help
+	  BNXT provides interface for the user process to access the debug and
+	  configuration registers of the Broadcom NIC hardware family.
+	  This will allow configuration and debug tools to work out of the box on
+	  mainstream kernel.
+
+	  If you don't know what to do here, say N.
+
 config FWCTL_MLX5
 	tristate "mlx5 ConnectX control fwctl driver"
 	depends on MLX5_CORE
diff --git a/drivers/fwctl/Makefile b/drivers/fwctl/Makefile
index c093b5f661d6..692e4b8d7beb 100644
--- a/drivers/fwctl/Makefile
+++ b/drivers/fwctl/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_FWCTL) += fwctl.o
+obj-$(CONFIG_FWCTL_BNXT) += bnxt/
 obj-$(CONFIG_FWCTL_MLX5) += mlx5/
 obj-$(CONFIG_FWCTL_PDS) += pds/
 
diff --git a/drivers/fwctl/bnxt/Makefile b/drivers/fwctl/bnxt/Makefile
new file mode 100644
index 000000000000..b47172761f1e
--- /dev/null
+++ b/drivers/fwctl/bnxt/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_FWCTL_BNXT) += bnxt_fwctl.o
+
+bnxt_fwctl-y += main.o
diff --git a/drivers/fwctl/bnxt/main.c b/drivers/fwctl/bnxt/main.c
new file mode 100644
index 000000000000..4d7449b9d686
--- /dev/null
+++ b/drivers/fwctl/bnxt/main.c
@@ -0,0 +1,278 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2026, Broadcom Corporation
+ */
+
+#include <linux/auxiliary_bus.h>
+#include <linux/slab.h>
+#include <linux/pci.h>
+#include <linux/fwctl.h>
+#include <linux/bnxt/hsi.h>
+#include <linux/bnxt/ulp.h>
+#include <uapi/fwctl/fwctl.h>
+#include <uapi/fwctl/bnxt.h>
+
+struct bnxtctl_uctx {
+	struct fwctl_uctx uctx;
+	u32 uctx_caps;
+};
+
+struct bnxtctl_dev {
+	struct fwctl_device fwctl;
+	struct bnxt_aux_priv *aux_priv;
+};
+
+DEFINE_FREE(bnxtctl, struct bnxtctl_dev *, if (_T) fwctl_put(&_T->fwctl))
+
+static int bnxtctl_open_uctx(struct fwctl_uctx *uctx)
+{
+	struct bnxtctl_uctx *bnxtctl_uctx =
+		container_of(uctx, struct bnxtctl_uctx, uctx);
+
+	bnxtctl_uctx->uctx_caps = BIT(FWCTL_BNXT_INLINE_COMMANDS) |
+				  BIT(FWCTL_BNXT_QUERY_COMMANDS) |
+				  BIT(FWCTL_BNXT_SEND_COMMANDS);
+	return 0;
+}
+
+static void bnxtctl_close_uctx(struct fwctl_uctx *uctx)
+{
+}
+
+static void *bnxtctl_info(struct fwctl_uctx *uctx, size_t *length)
+{
+	struct bnxtctl_uctx *bnxtctl_uctx =
+		container_of(uctx, struct bnxtctl_uctx, uctx);
+	struct fwctl_info_bnxt *info;
+
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return ERR_PTR(-ENOMEM);
+
+	info->uctx_caps = bnxtctl_uctx->uctx_caps;
+
+	*length = sizeof(*info);
+	return info;
+}
+
+static bool bnxtctl_validate_rpc(struct bnxt_en_dev *edev,
+				 struct bnxt_fw_msg *hwrm_in,
+				 enum fwctl_rpc_scope scope)
+{
+	struct input *req = (struct input *)hwrm_in->msg;
+
+	guard(mutex)(&edev->en_dev_lock);
+	if (edev->flags & BNXT_EN_FLAG_ULP_STOPPED)
+		return false;
+
+	switch (le16_to_cpu(req->req_type)) {
+	case HWRM_FUNC_RESET:
+	case HWRM_PORT_CLR_STATS:
+	case HWRM_FW_RESET:
+	case HWRM_FW_SYNC:
+	case HWRM_FW_SET_TIME:
+	case HWRM_DBG_LOG_BUFFER_FLUSH:
+	case HWRM_DBG_ERASE_NVM:
+	case HWRM_DBG_CFG:
+	case HWRM_NVM_DEFRAG:
+	case HWRM_NVM_FACTORY_DEFAULTS:
+	case HWRM_NVM_FLUSH:
+	case HWRM_NVM_VERIFY_UPDATE:
+	case HWRM_NVM_ERASE_DIR_ENTRY:
+	case HWRM_NVM_MOD_DIR_ENTRY:
+	case HWRM_NVM_FIND_DIR_ENTRY:
+		return scope >= FWCTL_RPC_CONFIGURATION;
+
+	case HWRM_VER_GET:
+	case HWRM_ERROR_RECOVERY_QCFG:
+	case HWRM_FUNC_QCAPS:
+	case HWRM_FUNC_QCFG:
+	case HWRM_FUNC_QSTATS:
+	case HWRM_PORT_PHY_QCFG:
+	case HWRM_PORT_MAC_QCFG:
+	case HWRM_PORT_PHY_QCAPS:
+	case HWRM_PORT_PHY_I2C_READ:
+	case HWRM_PORT_PHY_MDIO_READ:
+	case HWRM_QUEUE_PRI2COS_QCFG:
+	case HWRM_QUEUE_COS2BW_QCFG:
+	case HWRM_VNIC_RSS_QCFG:
+	case HWRM_QUEUE_GLOBAL_QCFG:
+	case HWRM_QUEUE_ADPTV_QOS_RX_FEATURE_QCFG:
+	case HWRM_QUEUE_ADPTV_QOS_TX_FEATURE_QCFG:
+	case HWRM_QUEUE_QCAPS:
+	case HWRM_QUEUE_ADPTV_QOS_RX_TUNING_QCFG:
+	case HWRM_QUEUE_ADPTV_QOS_TX_TUNING_QCFG:
+	case HWRM_TUNNEL_DST_PORT_QUERY:
+	case HWRM_PORT_TX_FIR_QCFG:
+	case HWRM_FW_LIVEPATCH_QUERY:
+	case HWRM_FW_QSTATUS:
+	case HWRM_FW_HEALTH_CHECK:
+	case HWRM_FW_GET_TIME:
+	case HWRM_PORT_EP_TX_QCFG:
+	case HWRM_PORT_QCFG:
+	case HWRM_PORT_MAC_QCAPS:
+	case HWRM_TEMP_MONITOR_QUERY:
+	case HWRM_REG_POWER_QUERY:
+	case HWRM_CORE_FREQUENCY_QUERY:
+	case HWRM_CFA_REDIRECT_QUERY_TUNNEL_TYPE:
+	case HWRM_CFA_ADV_FLOW_MGNT_QCAPS:
+	case HWRM_FUNC_RESOURCE_QCAPS:
+	case HWRM_FUNC_BACKING_STORE_QCAPS:
+	case HWRM_FUNC_BACKING_STORE_QCFG:
+	case HWRM_FUNC_QSTATS_EXT:
+	case HWRM_FUNC_PTP_PIN_QCFG:
+	case HWRM_FUNC_PTP_EXT_QCFG:
+	case HWRM_FUNC_BACKING_STORE_QCFG_V2:
+	case HWRM_FUNC_BACKING_STORE_QCAPS_V2:
+	case HWRM_FUNC_SYNCE_QCFG:
+	case HWRM_FUNC_TTX_PACING_RATE_PROF_QUERY:
+	case HWRM_PORT_PHY_FDRSTAT:
+	case HWRM_DBG_RING_INFO_GET:
+	case HWRM_DBG_QCAPS:
+	case HWRM_DBG_QCFG:
+	case HWRM_DBG_USEQ_FLUSH:
+	case HWRM_DBG_USEQ_QCAPS:
+	case HWRM_DBG_SIM_CABLE_STATE:
+	case HWRM_DBG_TOKEN_QUERY_AUTH_IDS:
+	case HWRM_NVM_GET_DEV_INFO:
+	case HWRM_NVM_GET_DIR_INFO:
+	case HWRM_SELFTEST_QLIST:
+		return scope >= FWCTL_RPC_DEBUG_READ_ONLY;
+
+	case HWRM_PORT_PHY_I2C_WRITE:
+	case HWRM_PORT_PHY_MDIO_WRITE:
+		return scope >= FWCTL_RPC_DEBUG_WRITE;
+
+	default:
+		return false;
+	}
+}
+
+static void *bnxtctl_fw_rpc(struct fwctl_uctx *uctx,
+			    enum fwctl_rpc_scope scope,
+			    void *in, size_t in_len, size_t *out_len)
+{
+	struct bnxtctl_dev *bnxtctl =
+		container_of(uctx->fwctl, struct bnxtctl_dev, fwctl);
+	struct bnxt_aux_priv *bnxt_aux_priv = bnxtctl->aux_priv;
+	size_t off = offsetof(struct fwctl_rpc_bnxt, reserved1);
+	struct fwctl_rpc_bnxt *msg = in;
+	struct bnxt_fw_msg rpc_in = {0};
+	int rc, err = 0;
+
+	if (in_len < sizeof(*msg))
+		return ERR_PTR(-EINVAL);
+
+	if (memchr_inv((unsigned char *)msg + off, 0, in_len - off))
+		return ERR_PTR(-EINVAL);
+
+	if (msg->req_len < sizeof(struct input) ||
+	    msg->req_len > HWRM_MAX_REQ_LEN)
+		return ERR_PTR(-EINVAL);
+
+	rpc_in.msg = memdup_user(u64_to_user_ptr(msg->req), msg->req_len);
+	if (IS_ERR(rpc_in.msg))
+		return rpc_in.msg;
+
+	if (!bnxtctl_validate_rpc(bnxt_aux_priv->edev, &rpc_in, scope)) {
+		err = -EPERM;
+		goto free_msg_out;
+	}
+
+	rpc_in.msg_len = msg->req_len;
+	rpc_in.resp = kzalloc(*out_len, GFP_KERNEL);
+	if (!rpc_in.resp) {
+		err = -ENOMEM;
+		goto free_msg_out;
+	}
+
+	rpc_in.resp_max_len = *out_len;
+	if (!msg->timeout)
+		rpc_in.timeout = FWCTL_BNXT_HWRM_DFLT_TIMEOUT;
+	else
+		rpc_in.timeout = msg->timeout;
+
+	rc = bnxt_send_msg(bnxt_aux_priv->edev, &rpc_in);
+	if (rc) {
+		struct output *resp = rpc_in.resp;
+
+		/* Copy the response to user always, as it contains
+		 * detailed status of the command failure
+		 */
+		if (!resp->error_code)
+			/* bnxt_send_msg() returned much before FW
+			 * received the command.
+			 */
+			resp->error_code = rc;
+	}
+free_msg_out:
+	kfree(rpc_in.msg);
+
+	if (err) {
+		kfree(rpc_in.resp);
+		return ERR_PTR(err);
+	}
+
+	return rpc_in.resp;
+}
+
+static const struct fwctl_ops bnxtctl_ops = {
+	.device_type = FWCTL_DEVICE_TYPE_BNXT,
+	.uctx_size = sizeof(struct bnxtctl_uctx),
+	.open_uctx = bnxtctl_open_uctx,
+	.close_uctx = bnxtctl_close_uctx,
+	.info = bnxtctl_info,
+	.fw_rpc = bnxtctl_fw_rpc,
+};
+
+static int bnxtctl_probe(struct auxiliary_device *adev,
+			 const struct auxiliary_device_id *id)
+{
+	struct bnxt_aux_priv *aux_priv =
+		container_of(adev, struct bnxt_aux_priv, aux_dev);
+	struct bnxtctl_dev *bnxtctl __free(bnxtctl) =
+		fwctl_alloc_device(&aux_priv->edev->pdev->dev, &bnxtctl_ops,
+				   struct bnxtctl_dev, fwctl);
+	int rc;
+
+	if (!bnxtctl)
+		return -ENOMEM;
+
+	bnxtctl->aux_priv = aux_priv;
+
+	rc = fwctl_register(&bnxtctl->fwctl);
+	if (rc)
+		return rc;
+
+	auxiliary_set_drvdata(adev, no_free_ptr(bnxtctl));
+	return 0;
+}
+
+static void bnxtctl_remove(struct auxiliary_device *adev)
+{
+	struct bnxtctl_dev *ctldev = auxiliary_get_drvdata(adev);
+
+	fwctl_unregister(&ctldev->fwctl);
+	fwctl_put(&ctldev->fwctl);
+}
+
+static const struct auxiliary_device_id bnxtctl_id_table[] = {
+	{ .name = "bnxt_en.fwctl", },
+	{}
+};
+MODULE_DEVICE_TABLE(auxiliary, bnxtctl_id_table);
+
+static struct auxiliary_driver bnxtctl_driver = {
+	.name = "bnxt_fwctl",
+	.probe = bnxtctl_probe,
+	.remove = bnxtctl_remove,
+	.id_table = bnxtctl_id_table,
+};
+
+module_auxiliary_driver(bnxtctl_driver);
+
+MODULE_IMPORT_NS("FWCTL");
+MODULE_DESCRIPTION("BNXT fwctl driver");
+MODULE_AUTHOR("Pavan Chebbi <pavan.chebbi@broadcom.com>");
+MODULE_AUTHOR("Andy Gospodarek <gospo@broadcom.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/uapi/fwctl/bnxt.h b/include/uapi/fwctl/bnxt.h
new file mode 100644
index 000000000000..bfb27e8f6779
--- /dev/null
+++ b/include/uapi/fwctl/bnxt.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright (c) 2026, Broadcom Inc
+ */
+
+#ifndef _UAPI_FWCTL_BNXT_H_
+#define _UAPI_FWCTL_BNXT_H_
+
+#include <linux/types.h>
+
+#define FWCTL_BNXT_HWRM_DFLT_TIMEOUT    500 /* ms */
+
+enum fwctl_bnxt_commands {
+	FWCTL_BNXT_INLINE_COMMANDS = 0,
+	FWCTL_BNXT_QUERY_COMMANDS,
+	FWCTL_BNXT_SEND_COMMANDS,
+};
+
+/**
+ * struct fwctl_info_bnxt - ioctl(FWCTL_INFO) out_device_data
+ * @uctx_caps: The command capabilities driver accepts.
+ *
+ * Return basic information about the FW interface available.
+ */
+struct fwctl_info_bnxt {
+	__u32 uctx_caps;
+};
+
+/**
+ * struct fwctl_rpc_bnxt - describe the fwctl message for bnxt
+ * @req: FW (HWRM) command input structure
+ * @req_len: length of @req
+ * @timeout: in ms to override the driver's default, 0 otherwise
+ * @reserved: must be 0
+ * @reserved1: must be 0
+ */
+struct fwctl_rpc_bnxt {
+	__aligned_u64 req;
+	__u32 req_len;
+	__u32 timeout;
+	__u32 reserved[2];
+	__aligned_u64 reserved1;
+};
+#endif
diff --git a/include/uapi/fwctl/fwctl.h b/include/uapi/fwctl/fwctl.h
index 716ac0eee42d..2d6d4049c205 100644
--- a/include/uapi/fwctl/fwctl.h
+++ b/include/uapi/fwctl/fwctl.h
@@ -44,6 +44,7 @@ enum fwctl_device_type {
 	FWCTL_DEVICE_TYPE_ERROR = 0,
 	FWCTL_DEVICE_TYPE_MLX5 = 1,
 	FWCTL_DEVICE_TYPE_CXL = 2,
+	FWCTL_DEVICE_TYPE_BNXT = 3,
 	FWCTL_DEVICE_TYPE_PDS = 4,
 };
 
-- 
2.39.1

[PATCH v5 fwctl 5/5] fwctl/bnxt_fwctl: Add documentation entries

[Pavan Chebbi]

Add bnxt_fwctl to the driver and fwctl documentation pages.

Reviewed-by: Andy Gospodarek <gospo@broadcom.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
---
 .../userspace-api/fwctl/bnxt_fwctl.rst        | 83 +++++++++++++++++++
 Documentation/userspace-api/fwctl/fwctl.rst   |  1 +
 Documentation/userspace-api/fwctl/index.rst   |  1 +
 3 files changed, 85 insertions(+)
 create mode 100644 Documentation/userspace-api/fwctl/bnxt_fwctl.rst

diff --git a/Documentation/userspace-api/fwctl/bnxt_fwctl.rst b/Documentation/userspace-api/fwctl/bnxt_fwctl.rst
new file mode 100644
index 000000000000..15a0b06b3dd8
--- /dev/null
+++ b/Documentation/userspace-api/fwctl/bnxt_fwctl.rst
@@ -0,0 +1,83 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=================
+fwctl bnxt driver
+=================
+
+:Author: Pavan Chebbi
+
+Overview
+========
+
+BNXT driver makes a fwctl service available through an auxiliary_device.
+The bnxt_fwctl driver binds to this device and registers itself with the
+fwctl subsystem.
+
+The bnxt_fwctl driver is agnostic to the device firmware internals. It
+uses the Upper Layer Protocol (ULP) conduit provided by bnxt to send
+HardWare Resource Manager (HWRM) commands to firmware.
+
+These commands can query or change firmware driven device configurations
+and read/write registers that are useful for debugging.
+
+bnxt_fwctl User API
+===================
+
+Each RPC request contains a message request structure (HWRM input),
+its length, optional request timeout, and dma buffers' information
+if the command needs any DMA. The request is then put together with
+the request data and sent through bnxt's message queue to the firmware,
+and the results are returned to the caller.
+
+A typical user application can send a FWCTL_INFO command using ioctl()
+to discover bnxt_fwctl's RPC capabilities as shown below:
+
+        ioctl(fd, FWCTL_INFO, &fwctl_info_msg);
+
+where fwctl_info_msg (of type struct fwctl_info) describes bnxt_info_msg
+(of type struct fwctl_info_bnxt). fwctl_info_msg is set up as follows:
+
+        size = sizeof(struct fwctl_info);
+        flags = 0;
+        device_data_len = sizeof(bnxt_info_msg);
+        out_device_data = (__aligned_u64)&bnxt_info_msg;
+
+The uctx_caps of bnxt_info_msg represents the capabilities as described
+in fwctl_bnxt_commands of include/uapi/fwctl/bnxt.h
+
+The FW RPC itself, FWCTL_RPC can be sent using ioctl() as:
+
+        ioctl(fd, FWCTL_RPC, &fwctl_rpc_msg);
+
+where fwctl_rpc_msg (of type struct fwctl_rpc) encapsulates fwctl_rpc_bnxt
+(see bnxt_rpc_msg below). fwctl_rpc_bnxt members are set up as per the
+requirements of specific HWRM commands described in include/linux/bnxt/hsi.h.
+An example for HWRM_VER_GET is shown below:
+
+        struct fwctl_rpc_bnxt bnxt_rpc_msg;
+        struct hwrm_ver_get_output resp;
+        struct fwctl_rpc fwctl_rpc_msg;
+        struct hwrm_ver_get_input req;
+
+        req.req_type = HWRM_VER_GET;
+        req.hwrm_intf_maj = HWRM_VERSION_MAJOR;
+        req.hwrm_intf_min = HWRM_VERSION_MINOR;
+        req.hwrm_intf_upd = HWRM_VERSION_UPDATE;
+        req.cmpl_ring = -1;
+        req.target_id = -1;
+
+        bnxt_rpc_msg.req_len = sizeof(struct hwrm_ver_get_input);
+        bnxt_rpc_msg.num_dma = 0;
+        bnxt_rpc_msg.req = (__aligned_u64)&req;
+
+        fwctl_rpc_msg.size = sizeof(struct fwctl_rpc);
+        fwctl_rpc_msg.scope = FWCTL_RPC_DEBUG_READ_ONLY;
+        fwctl_rpc_msg.in_len = sizeof(bnxt_rpc_msg);
+        fwctl_rpc_msg.out_len = sizeof(struct hwrm_ver_get_output);
+        fwctl_rpc_msg.in = (__aligned_u64)&bnxt_rpc_msg;
+        fwctl_rpc_msg.out = (__aligned_u64)&resp;
+
+An example python3 program that can exercise this interface can be found in
+the following git repository:
+
+https://github.com/Broadcom/fwctl-tools
diff --git a/Documentation/userspace-api/fwctl/fwctl.rst b/Documentation/userspace-api/fwctl/fwctl.rst
index a74eab8d14c6..826817bfd54d 100644
--- a/Documentation/userspace-api/fwctl/fwctl.rst
+++ b/Documentation/userspace-api/fwctl/fwctl.rst
@@ -148,6 +148,7 @@ area resulting in clashes will be resolved in favour of a kernel implementation.
 fwctl User API
 ==============
 
+.. kernel-doc:: include/uapi/fwctl/bnxt.h
 .. kernel-doc:: include/uapi/fwctl/fwctl.h
 .. kernel-doc:: include/uapi/fwctl/mlx5.h
 .. kernel-doc:: include/uapi/fwctl/pds.h
diff --git a/Documentation/userspace-api/fwctl/index.rst b/Documentation/userspace-api/fwctl/index.rst
index 316ac456ad3b..8062f7629654 100644
--- a/Documentation/userspace-api/fwctl/index.rst
+++ b/Documentation/userspace-api/fwctl/index.rst
@@ -10,5 +10,6 @@ to securely construct and execute RPCs inside device firmware.
    :maxdepth: 1
 
    fwctl
+   bnxt_fwctl
    fwctl-cxl
    pds_fwctl
-- 
2.39.1

Re: [PATCH v5 fwctl 2/5] fwctl/bnxt_en: Refactor aux bus functions to be more generic

[Jason Gunthorpe]

On Thu, Feb 26, 2026 at 12:23:15AM -0800, Pavan Chebbi wrote:
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 4481d80cdfc2..a9bfbfabf121 100644

I don't know what you've done, but this doesn't apply.

Applying: fwctl/bnxt_en: Move common definitions to include/linux/bnxt/
Applying: fwctl/bnxt_en: Refactor aux bus functions to be more generic
error: sha1 information is lacking or useless (drivers/net/ethernet/broadcom/bnxt/bnxt.c).
error: could not build fake ancestor
Patch failed at 0002 fwctl/bnxt_en: Refactor aux bus functions to be more generic
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

The github the cover letter points at has a different patch, that shouldn't happen.

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 728f893689eaa2..3005c4bb8aed3d 100644

Jason

Re: [PATCH v5 fwctl 2/5] fwctl/bnxt_en: Refactor aux bus functions to be more generic

[Pavan Chebbi]

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

On Fri, Mar 13, 2026 at 9:16 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Thu, Feb 26, 2026 at 12:23:15AM -0800, Pavan Chebbi wrote:
> > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> > index 4481d80cdfc2..a9bfbfabf121 100644
>
> I don't know what you've done, but this doesn't apply.
>
> Applying: fwctl/bnxt_en: Move common definitions to include/linux/bnxt/
> Applying: fwctl/bnxt_en: Refactor aux bus functions to be more generic
> error: sha1 information is lacking or useless (drivers/net/ethernet/broadcom/bnxt/bnxt.c).
> error: could not build fake ancestor

Really not sure what is happening here. Let me try and see what could
be the issue..

> Patch failed at 0002 fwctl/bnxt_en: Refactor aux bus functions to be more generic
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
> The github the cover letter points at has a different patch, that shouldn't happen.

Ok there was one difference which I had to make because the github
tree had an additional patch that was not in fwctl tree, which was
about kzalloc_obj() replacing kzalloc()
There should be no other difference.

>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 728f893689eaa2..3005c4bb8aed3d 100644
>
> Jason

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

Re: [PATCH v5 fwctl 4/5] fwctl/bnxt_fwctl: Add bnxt fwctl device

[Jason Gunthorpe]

On Thu, Feb 26, 2026 at 12:23:17AM -0800, Pavan Chebbi wrote:
> +static void *bnxtctl_fw_rpc(struct fwctl_uctx *uctx,
> +			    enum fwctl_rpc_scope scope,
> +			    void *in, size_t in_len, size_t *out_len)
> +{
> +	struct bnxtctl_dev *bnxtctl =
> +		container_of(uctx->fwctl, struct bnxtctl_dev, fwctl);
> +	struct bnxt_aux_priv *bnxt_aux_priv = bnxtctl->aux_priv;
> +	size_t off = offsetof(struct fwctl_rpc_bnxt, reserved1);
> +	struct fwctl_rpc_bnxt *msg = in;
> +	struct bnxt_fw_msg rpc_in = {0};
> +	int rc, err = 0;
> +
> +	if (in_len < sizeof(*msg))
> +		return ERR_PTR(-EINVAL);
> +
> +	if (memchr_inv((unsigned char *)msg + off, 0, in_len - off))
> +		return ERR_PTR(-EINVAL);

EOPNOTSUPP

> +/**
> + * struct fwctl_rpc_bnxt - describe the fwctl message for bnxt
> + * @req: FW (HWRM) command input structure
> + * @req_len: length of @req
> + * @timeout: in ms to override the driver's default, 0 otherwise
> + * @reserved: must be 0
> + * @reserved1: must be 0
> + */
> +struct fwctl_rpc_bnxt {
> +	__aligned_u64 req;
> +	__u32 req_len;
> +	__u32 timeout;
> +	__u32 reserved[2];
> +	__aligned_u64 reserved1;

Why? Upstream isn't going to try to be compatible with OOT drivers.

> +};

I know pensando did it this way, but every time I see this I don't
like it.

struct fwctl_rpc {
        __u32 size;
        __u32 scope;
        __u32 in_len;
        __u32 out_len;
        __aligned_u64 in;
        __aligned_u64 out;
         ^^^^^^^^^^^^^^^^^

These two were supposed to be the actual message, not a layer of
driver indirection, that is why the buffer mangement works the way
it does.

"timeout" just doesn't seem like a great reason to put a wrapper
struct..

Would you be happy to add a
   __aligned_u64 driver_data

To fwctl_rpc to use for your timeout and future stuff and leave the
message buffer as it was intended?

Jason

Re: [PATCH v5 fwctl 4/5] fwctl/bnxt_fwctl: Add bnxt fwctl device

[Jason Gunthorpe]

On Thu, Feb 26, 2026 at 12:23:17AM -0800, Pavan Chebbi wrote:
> +	rpc_in.msg_len = msg->req_len;
> +	rpc_in.resp = kzalloc(*out_len, GFP_KERNEL);
> +	if (!rpc_in.resp) {
> +		err = -ENOMEM;
> +		goto free_msg_out;
> +	}
> +
> +	rpc_in.resp_max_len = *out_len;
> +	if (!msg->timeout)
> +		rpc_in.timeout = FWCTL_BNXT_HWRM_DFLT_TIMEOUT;
> +	else
> +		rpc_in.timeout = msg->timeout;
> +
> +	rc = bnxt_send_msg(bnxt_aux_priv->edev, &rpc_in);
> +	if (rc) {
> +		struct output *resp = rpc_in.resp;
> +
> +		/* Copy the response to user always, as it contains
> +		 * detailed status of the command failure
> +		 */
> +		if (!resp->error_code)
> +			/* bnxt_send_msg() returned much before FW
> +			 * received the command.
> +			 */
> +			resp->error_code = rc;
> +	}
> +free_msg_out:
> +	kfree(rpc_in.msg);

Timeout is such a complicated thing to add to a HW RPC interface. Does
bnxt do it right? Claude says no, and it looks compelling to me..

So don't give userspace an easy ability to trigger timeout, by
lowering the timeout value, and causing corruption in the kernel.

If you hit a FW timeout you probably have to FLR the device to recover
from it, if fwctl is going to have a timeout knob it has to leave the
request active in the kernel and orphan it to avoid a FLR - not sure
that makes much sense.

>> I want to investigate what the state is after this function fails,
>> particularly if it fails for a timeout. Is there any cahnce that
>> fw_msg->resp can be written too after the function returns because
>> it is still programmined into HW as a DMA?

DMA Safety Analysis: bnxt_send_msg() Timeout Path

Overview

Investigation of whether firmware can DMA into a freed buffer after
bnxt_send_msg() returns due to a timeout.

How resp_addr Gets Programmed Into HW

At bnxt_hwrm.c:90, the DMA address of the response buffer is embedded in
the request message itself:

    ctx->req->resp_addr = cpu_to_le64(dma_handle + BNXT_HWRM_RESP_OFFSET);

This address is sent to the firmware as part of the HWRM command. The
firmware caches it and will DMA the response to that address when it
completes.

What Happens on Timeout

There are multiple timeout goto exit paths (lines 587, 645, 668 in
bnxt_hwrm.c). All jump to the exit label at line 685:

    exit:
        if (token)
            __hwrm_release_token(bp, token);
        if (ctx->flags & BNXT_HWRM_INTERNAL_CTX_OWNED)
            ctx->flags |= BNXT_HWRM_INTERNAL_RESP_DIRTY;
        else
            __hwrm_ctx_drop(bp, ctx);

No abort command is sent to the firmware. No mechanism exists to tell
the HW to stop using resp_addr.

The Specific Flow in bnxt_send_msg()

Since hwrm_req_hold() is called, the request is “owned”, so on timeout:

1.  __hwrm_send() sets BNXT_HWRM_INTERNAL_RESP_DIRTY flag and returns
    error.
2.  Back in bnxt_send_msg(), resp_len is read from resp->resp_len. On a
    timeout this is likely 0 (buffer was zeroed at alloc), so the memcpy
    is skipped.
3.  hwrm_req_drop() calls __hwrm_ctx_drop() which calls dma_pool_free()
    at line 313.

The DMA Hazard

After dma_pool_free(), the buffer goes back to the pool. But the
firmware still has resp_addr and may still DMA the response to it. This
means:

1.  The pool buffer can be reallocated for a new HWRM request via
    dma_pool_alloc().
2.  The firmware can write to it at any time – corrupting the new
    request’s data or response.

The BNXT_HWRM_INTERNAL_RESP_DIRTY flag only causes a memset on the next
use of the same buffer (around line 476), but that doesn’t help – it’s a
race. The firmware DMA can arrive after the memset, after the new
request is in flight, or even after the new response is written.

Regarding fw_msg->resp Specifically

In bnxt_send_msg() itself, fw_msg->resp is not a DMA target – it’s a
plain kernel buffer that gets memcpy’d into. So fw_msg->resp won’t be
directly corrupted by late DMA. The DMA target is the internal ctx->resp
buffer from the pool.

Summary

  -----------------------------------------------------------------------
  Aspect                              Finding
  ----------------------------------- -----------------------------------
  fw_msg->resp written by late DMA?   No – it’s a memcpy destination, not
                                      a DMA target

  Pool DMA buffer at risk?            Yes – firmware retains resp_addr
                                      after timeout

  Firmware abort on timeout?          No – no abort/stop mechanism exists

  DMA unmap on timeout?               No – dma_pool_free() returns buffer
                                      to pool without unmapping

  Synchronization barrier?            No – no barriers or delays before
                                      freeing

  Corruption scenario                 Firmware completes late, DMA writes
                                      into reallocated pool buffer used
                                      by a new HWRM command
  -----------------------------------------------------------------------

Re: [PATCH v5 fwctl 4/5] fwctl/bnxt_fwctl: Add bnxt fwctl device

[Pavan Chebbi]

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

>
> Would you be happy to add a
>    __aligned_u64 driver_data
>
> To fwctl_rpc to use for your timeout and future stuff and leave the
> message buffer as it was intended?

OK.. it sounds like a good idea. I can make this change.

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

Re: [PATCH v5 fwctl 4/5] fwctl/bnxt_fwctl: Add bnxt fwctl device

[Pavan Chebbi]

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

>
> Timeout is such a complicated thing to add to a HW RPC interface. Does
> bnxt do it right? Claude says no, and it looks compelling to me..
>
> So don't give userspace an easy ability to trigger timeout, by
> lowering the timeout value, and causing corruption in the kernel.

Yea I see what you are saying. I think it should be fine if
bnxtctl_fw_rpc() itself increases the timeout for the required
commands.
I can make this change. But if I drop the "timeout" now, I won't need
the driver_data we discussed previously, at least until I actually add
the future enhancements.
So I guess I should defer that change to when I actually use driver_data, right?

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

Re: [PATCH v5 fwctl 4/5] fwctl/bnxt_fwctl: Add bnxt fwctl device

[Jason Gunthorpe]

On Fri, Mar 13, 2026 at 11:44:05PM +0530, Pavan Chebbi wrote:
> >
> > Timeout is such a complicated thing to add to a HW RPC interface. Does
> > bnxt do it right? Claude says no, and it looks compelling to me..
> >
> > So don't give userspace an easy ability to trigger timeout, by
> > lowering the timeout value, and causing corruption in the kernel.
> 
> Yea I see what you are saying. I think it should be fine if
> bnxtctl_fw_rpc() itself increases the timeout for the required
> commands.

Ah, you wanted to increase the timeout - yes, I think the kernel needs
to do this itself to avoid userspace triggering timeouts that don't
work right..

> I can make this change. But if I drop the "timeout" now, I won't need
> the driver_data we discussed previously, at least until I actually add
> the future enhancements.
> So I guess I should defer that change to when I actually use
> driver_data, right?

Yes all that makes sense. Without timeout and the dma list thing there
is no reason to wrapper the commands. Comme with some driver_data
later if you find you need more info

Jason