On Mon, Dec 15, 2025 at 05:06:02PM +0100, Stefan Sperling wrote:
> On Wed, Nov 26, 2025 at 05:35:45PM +1000, Jonathan Matthew wrote:
> > The diff below adds support for BCM5750x devices, also called Phase 5
> > and Thor in various places, to bnxt(4).  These are showing up in new
> > servers from Dell and others, particularly if you want a 200GbE
> > OCP 3.0 nic for some reason, or 4x 25GbE.
> > 
> > There are a couple of significant differences to earlier generation
> > bnxt hardware.  Instead of completion queues generating interrupts,
> > they now generate events that feed into notification queues, which
> > generate interrupts.  Notification queues are very similar to
> > completion queues, so this isn't overly difficult to deal with.
> > 
> > The host is also required to allocate some memory as backing store for
> > the nic (on the order of a few MB) before it can do anything useful.
> > This required some changes to the initial setup process - in particular,
> > I had to move the function reset call earlier because it was clearing
> > the backing store config, which made the whole thing useless.
> > This version of the diff probably allocates a bit more backing store than
> > strictly needed, so I'll try to cut it down later.
> > 
> > Support for BCM5760x (Phase 7, or Thor2) is another set of incremental
> > changes on top of this, but I don't have any hardware to work on that.
> > 
> > I'll break this up into some smaller pieces to commit, but in case
> > anyone wants to try this against new or old bnxt, here it is.
> 
> This device still is working fine with your diff:
> 
> bnxt0 at pci14 dev 0 function 0 "Broadcom BCM57414" rev 0x01: fw ver 234.0.150, msix, 8 queues, address xx:xx:xx:xx:xx:xx

Thanks for testing.

> 
> The diff reads fine, too. Though I noticed that bnxt_backing_store_cfg()
> contains 7 calls to bnxt_dmamem_alloc() all of which lack NULL return checks
> and corresponding cleanup in bnxt_attach() on failure.

Good point. It might make sense to change it to do a single
allocation, since only the nic cares about what all the different
pieces of backing store are.

> 
> I do see the following during 'ifconfig bnxt0 down up' while tcpbench
> is running:
> 
> bnxt0: unexpected completion type 3
> bnxt0: unexpected completion type 18
> 
> But I doubt that is a new problem.
> 
> 18 decimal corresponds to CMPL_BASE_TYPE_RX_AGG.
> Not sure which event type 3 is supposed to be.
>

I think I've seen something like this before. Perhaps we're not
shutting down the completion ring properly, so we're getting junk
written to it somehow. I'll look into this.

Here's the first piece I'd like to commit, moving the func reset
and qportcfg operations around to suit the P5 setup process.

ok?


Index: if_bnxt.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_bnxt.c,v
diff -u -p -r1.62 if_bnxt.c
--- if_bnxt.c	19 Nov 2025 07:28:52 -0000	1.62
+++ if_bnxt.c	12 Jan 2026 06:54:56 -0000
@@ -512,6 +512,11 @@ bnxt_attach(struct device *parent, struc
 		goto free_resp;
 	}
 
+	if (bnxt_hwrm_func_reset(sc) != 0) {
+		printf(": reset failed\n");
+		goto free_resp;
+	}
+
 	if (bnxt_hwrm_nvm_get_dev_info(sc, NULL, NULL, NULL, NULL, NULL, NULL)
 	    != 0) {
 		printf(": failed to get nvram info\n");
@@ -533,6 +538,11 @@ bnxt_attach(struct device *parent, struc
 		goto free_resp;
 	}
 
+	if (bnxt_hwrm_queue_qportcfg(sc) != 0) {
+		printf(": failed to query port config\n");
+		goto free_resp;
+	}
+
 	/*
 	 * devices advertise msi support, but there's no way to tell a
 	 * completion queue to use msi mode, only legacy or msi-x.
@@ -581,16 +591,6 @@ bnxt_attach(struct device *parent, struc
 
 	if (bnxt_hwrm_func_qcfg(sc) != 0) {
 		printf("%s: failed to query function config\n", DEVNAME(sc));
-		goto deintr;
-	}
-
-	if (bnxt_hwrm_queue_qportcfg(sc) != 0) {
-		printf("%s: failed to query port config\n", DEVNAME(sc));
-		goto deintr;
-	}
-
-	if (bnxt_hwrm_func_reset(sc) != 0) {
-		printf("%s: reset failed\n", DEVNAME(sc));
 		goto deintr;
 	}