[PATCH 0/5] net/cxgbe: updates and bug fixes

[PATCH 0/5] net/cxgbe: updates and bug fixes

[Rahul Lakkireddy]

This series of patches add the following updates and bug fixes to
the cxgbe PMD.

Patch 1 fixes an issue with wrong port id being filled in mbufs
allocated in Rx path.

Patch 2 fixes an issue with Txq getting stuck when trying to coalesce
mbufs with chains.

Patch 3 reworks and simplifies the code for posting mbufs and their
payload buffer sizes to hardware in Rx path.

Patch 4 adds support to track packets in Rx path dropped due to
congestion at Rx queues that may not get propagated to the rest
of the Rx pipeline.

Patch 5 adds support to read firmware configuration file from
/lib/firmware/cxgb4/ to allow changing firmware parameters without
having to flash the configuration file onto the adapter.

Thanks,
Rahul

Rahul Lakkireddy (5):
  net/cxgbe: fill correct port info in mbufs for Rx
  net/cxgbe: fix Tx queue stuck with mbuf chain coalescing
  net/cxgbe: simplify Rx payload buffer size posting
  net/cxgbe: track packets dropped by TP due to congestion
  net/cxgbe: read firmware configuration file from filesystem

 drivers/net/cxgbe/base/adapter.h        |   6 +-
 drivers/net/cxgbe/base/common.h         |   4 +-
 drivers/net/cxgbe/base/t4_hw.c          |  57 +---
 drivers/net/cxgbe/base/t4_hw.h          |   1 +
 drivers/net/cxgbe/base/t4_regs.h        |   4 +
 drivers/net/cxgbe/base/t4fw_interface.h |   1 +
 drivers/net/cxgbe/base/t4vf_hw.c        |  40 ---
 drivers/net/cxgbe/cxgbe_ethdev.c        |  11 +
 drivers/net/cxgbe/cxgbe_main.c          | 332 ++++++++++++++++--------
 drivers/net/cxgbe/sge.c                 | 296 +++++++--------------
 10 files changed, 351 insertions(+), 401 deletions(-)

-- 
2.27.0

[PATCH 1/5] net/cxgbe: fill correct port info in mbufs for Rx

[Rahul Lakkireddy]

Fill the correct DPDK ethdev port_id, instead of local adapter
physical port_id in mbufs allocated for Rx.

Fixes: 78fc1a716ae8 ("cxgbe: improve Rx performance")
Cc: stable@dpdk.org

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
 drivers/net/cxgbe/sge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/cxgbe/sge.c b/drivers/net/cxgbe/sge.c
index 1c76b8e4d0..5c176004f9 100644
--- a/drivers/net/cxgbe/sge.c
+++ b/drivers/net/cxgbe/sge.c
@@ -1910,7 +1910,7 @@ int t4_sge_alloc_rxq(struct adapter *adap, struct sge_rspq *iq, bool fwevtq,
 	iq->stat = (void *)&iq->desc[iq->size * 8];
 	iq->eth_dev = eth_dev;
 	iq->handler = hnd;
-	iq->port_id = pi->pidx;
+	iq->port_id = eth_dev->data->port_id;
 	iq->mb_pool = mp;
 
 	/* set offset to -1 to distinguish ingress queues without FL */
-- 
2.27.0

[PATCH 2/5] net/cxgbe: fix Tx queue stuck with mbuf chain coalescing

[Rahul Lakkireddy]

When trying to coalesce mbufs with chain on Tx side, it is possible
to get stuck during queue wrap around. When coalescing this mbuf
chain fails, the Tx path returns EBUSY and when the same packet
is retried again, it couldn't get coalesced again, and the loop
repeats. Fix by pushing the packet through the normal Tx path.
Also use FW_ETH_TX_PKTS_WR to handle mbufs with chain for FW
to optimize.

Fixes: 6c2809628cd5 ("net/cxgbe: improve latency for slow traffic")
Cc: stable@dpdk.org

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
 drivers/net/cxgbe/sge.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/drivers/net/cxgbe/sge.c b/drivers/net/cxgbe/sge.c
index 5c176004f9..566cd48406 100644
--- a/drivers/net/cxgbe/sge.c
+++ b/drivers/net/cxgbe/sge.c
@@ -789,9 +789,9 @@ static inline void txq_advance(struct sge_txq *q, unsigned int n)
 
 #define MAX_COALESCE_LEN 64000
 
-static inline int wraps_around(struct sge_txq *q, int ndesc)
+static inline bool wraps_around(struct sge_txq *q, int ndesc)
 {
-	return (q->pidx + ndesc) > q->size ? 1 : 0;
+	return (q->pidx + ndesc) > q->size ? true : false;
 }
 
 static void tx_timer_cb(void *data)
@@ -842,7 +842,6 @@ static inline void ship_tx_pkt_coalesce_wr(struct adapter *adap,
 
 	/* fill the pkts WR header */
 	wr = (void *)&q->desc[q->pidx];
-	wr->op_pkd = htonl(V_FW_WR_OP(FW_ETH_TX_PKTS2_WR));
 	vmwr = (void *)&q->desc[q->pidx];
 
 	wr_mid = V_FW_WR_LEN16(DIV_ROUND_UP(q->coalesce.flits, 2));
@@ -852,8 +851,11 @@ static inline void ship_tx_pkt_coalesce_wr(struct adapter *adap,
 	wr->npkt = q->coalesce.idx;
 	wr->r3 = 0;
 	if (is_pf4(adap)) {
-		wr->op_pkd = htonl(V_FW_WR_OP(FW_ETH_TX_PKTS2_WR));
 		wr->type = q->coalesce.type;
+		if (likely(wr->type != 0))
+			wr->op_pkd = htonl(V_FW_WR_OP(FW_ETH_TX_PKTS2_WR));
+		else
+			wr->op_pkd = htonl(V_FW_WR_OP(FW_ETH_TX_PKTS_WR));
 	} else {
 		wr->op_pkd = htonl(V_FW_WR_OP(FW_ETH_TX_PKTS_VM_WR));
 		vmwr->r4 = 0;
@@ -932,13 +934,16 @@ static inline int should_tx_packet_coalesce(struct sge_eth_txq *txq,
 		ndesc = DIV_ROUND_UP(q->coalesce.flits + flits, 8);
 		credits = txq_avail(q) - ndesc;
 
+		if (unlikely(wraps_around(q, ndesc)))
+			return 0;
+
 		/* If we are wrapping or this is last mbuf then, send the
 		 * already coalesced mbufs and let the non-coalesce pass
 		 * handle the mbuf.
 		 */
-		if (unlikely(credits < 0 || wraps_around(q, ndesc))) {
+		if (unlikely(credits < 0)) {
 			ship_tx_pkt_coalesce_wr(adap, txq);
-			return 0;
+			return -EBUSY;
 		}
 
 		/* If the max coalesce len or the max WR len is reached
@@ -962,8 +967,12 @@ static inline int should_tx_packet_coalesce(struct sge_eth_txq *txq,
 	ndesc = flits_to_desc(q->coalesce.flits + flits);
 	credits = txq_avail(q) - ndesc;
 
-	if (unlikely(credits < 0 || wraps_around(q, ndesc)))
+	if (unlikely(wraps_around(q, ndesc)))
 		return 0;
+
+	if (unlikely(credits < 0))
+		return -EBUSY;
+
 	q->coalesce.flits += wr_size / sizeof(__be64);
 	q->coalesce.type = type;
 	q->coalesce.ptr = (unsigned char *)&q->desc[q->pidx] +
@@ -1106,7 +1115,7 @@ int t4_eth_xmit(struct sge_eth_txq *txq, struct rte_mbuf *mbuf,
 	unsigned int flits, ndesc, cflits;
 	int l3hdr_len, l4hdr_len, eth_xtra_len;
 	int len, last_desc;
-	int credits;
+	int should_coal, credits;
 	u32 wr_mid;
 	u64 cntrl, *end;
 	bool v6;
@@ -1138,9 +1147,9 @@ int t4_eth_xmit(struct sge_eth_txq *txq, struct rte_mbuf *mbuf,
 	/* align the end of coalesce WR to a 512 byte boundary */
 	txq->q.coalesce.max = (8 - (txq->q.pidx & 7)) * 8;
 
-	if (!((m->ol_flags & RTE_MBUF_F_TX_TCP_SEG) ||
-			m->pkt_len > RTE_ETHER_MAX_LEN)) {
-		if (should_tx_packet_coalesce(txq, mbuf, &cflits, adap)) {
+	if ((m->ol_flags & RTE_MBUF_F_TX_TCP_SEG) == 0) {
+		should_coal = should_tx_packet_coalesce(txq, mbuf, &cflits, adap);
+		if (should_coal > 0) {
 			if (unlikely(map_mbuf(mbuf, addr) < 0)) {
 				dev_warn(adap, "%s: mapping err for coalesce\n",
 					 __func__);
@@ -1149,8 +1158,8 @@ int t4_eth_xmit(struct sge_eth_txq *txq, struct rte_mbuf *mbuf,
 			}
 			return tx_do_packet_coalesce(txq, mbuf, cflits, adap,
 						     pi, addr, nb_pkts);
-		} else {
-			return -EBUSY;
+		} else if (should_coal < 0) {
+			return should_coal;
 		}
 	}
 
@@ -1197,8 +1206,7 @@ int t4_eth_xmit(struct sge_eth_txq *txq, struct rte_mbuf *mbuf,
 		end = (u64 *)vmwr + flits;
 	}
 
-	len = 0;
-	len += sizeof(*cpl);
+	len = sizeof(*cpl);
 
 	/* Coalescing skipped and we send through normal path */
 	if (!(m->ol_flags & RTE_MBUF_F_TX_TCP_SEG)) {
-- 
2.27.0

[PATCH 3/5] net/cxgbe: simplify Rx payload buffer size posting

[Rahul Lakkireddy]

Match the closest supported Rx payload buffer size with the mempool
data size and program it for the Rx queue. This removes unnecessary
need for handling additional padding, packing, and alignment, when
posting Rx buffers to hardware.

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
 drivers/net/cxgbe/base/adapter.h |   6 +-
 drivers/net/cxgbe/base/common.h  |   2 -
 drivers/net/cxgbe/base/t4_hw.c   |  43 ------
 drivers/net/cxgbe/base/t4_hw.h   |   1 +
 drivers/net/cxgbe/base/t4vf_hw.c |  40 -----
 drivers/net/cxgbe/cxgbe_main.c   |   3 +-
 drivers/net/cxgbe/sge.c          | 256 ++++++++++---------------------
 7 files changed, 81 insertions(+), 270 deletions(-)

diff --git a/drivers/net/cxgbe/base/adapter.h b/drivers/net/cxgbe/base/adapter.h
index 97963422bf..1c016a7115 100644
--- a/drivers/net/cxgbe/base/adapter.h
+++ b/drivers/net/cxgbe/base/adapter.h
@@ -98,6 +98,7 @@ struct sge_fl {                     /* SGE free-buffer queue state */
 
 	unsigned long alloc_failed; /* # of times buffer allocation failed */
 	unsigned long low;          /* # of times momentarily starving */
+	u8 fl_buf_size_idx;         /* Selected SGE_FL_BUFFER_SIZE index */
 };
 
 #define MAX_MBUF_FRAGS (16384 / 512 + 2)
@@ -110,7 +111,6 @@ struct pkt_gl {
 	void *va;                         /* virtual address of first byte */
 	unsigned int nfrags;              /* # of fragments */
 	unsigned int tot_len;             /* total length of fragments */
-	bool usembufs;                    /* use mbufs for fragments */
 };
 
 typedef int (*rspq_handler_t)(struct sge_rspq *q, const __be64 *rsp,
@@ -160,7 +160,6 @@ struct sge_eth_rxq {                /* a SW Ethernet Rx queue */
 	struct sge_rspq rspq;
 	struct sge_fl fl;
 	struct sge_eth_rx_stats stats;
-	bool usembufs;               /* one ingress packet per mbuf FL buffer */
 } __rte_cache_aligned;
 
 /*
@@ -286,9 +285,8 @@ struct sge {
 	u16 timer_val[SGE_NTIMERS];
 	u8  counter_val[SGE_NCOUNTERS];
 
-	u32 fl_align;               /* response queue message alignment */
-	u32 fl_pg_order;            /* large page allocation size */
 	u32 fl_starve_thres;        /* Free List starvation threshold */
+	u32 fl_buffer_size[SGE_FL_BUFFER_SIZE_NUM]; /* Free List buffer sizes */
 };
 
 /*
diff --git a/drivers/net/cxgbe/base/common.h b/drivers/net/cxgbe/base/common.h
index af987b0d5d..980d78e4a4 100644
--- a/drivers/net/cxgbe/base/common.h
+++ b/drivers/net/cxgbe/base/common.h
@@ -343,8 +343,6 @@ int t4_fw_reset(struct adapter *adap, unsigned int mbox, int reset);
 int t4vf_fw_reset(struct adapter *adap);
 int t4_fw_halt(struct adapter *adap, unsigned int mbox, int reset);
 int t4_fw_restart(struct adapter *adap, unsigned int mbox, int reset);
-int t4_fl_pkt_align(struct adapter *adap);
-int t4vf_fl_pkt_align(struct adapter *adap, u32 sge_control, u32 sge_control2);
 int t4vf_get_vfres(struct adapter *adap);
 int t4_fixup_host_params_compat(struct adapter *adap, unsigned int page_size,
 				unsigned int cache_line_size,
diff --git a/drivers/net/cxgbe/base/t4_hw.c b/drivers/net/cxgbe/base/t4_hw.c
index 645833765a..84c4316e68 100644
--- a/drivers/net/cxgbe/base/t4_hw.c
+++ b/drivers/net/cxgbe/base/t4_hw.c
@@ -3477,49 +3477,6 @@ int t4_fw_restart(struct adapter *adap, unsigned int mbox, int reset)
 	return 0;
 }
 
-/**
- * t4_fl_pkt_align - return the fl packet alignment
- * @adap: the adapter
- *
- * T4 has a single field to specify the packing and padding boundary.
- * T5 onwards has separate fields for this and hence the alignment for
- * next packet offset is maximum of these two.
- */
-int t4_fl_pkt_align(struct adapter *adap)
-{
-	u32 sge_control, sge_control2;
-	unsigned int ingpadboundary, ingpackboundary, fl_align, ingpad_shift;
-
-	sge_control = t4_read_reg(adap, A_SGE_CONTROL);
-
-	/* T4 uses a single control field to specify both the PCIe Padding and
-	 * Packing Boundary.  T5 introduced the ability to specify these
-	 * separately.  The actual Ingress Packet Data alignment boundary
-	 * within Packed Buffer Mode is the maximum of these two
-	 * specifications.
-	 */
-	if (CHELSIO_CHIP_VERSION(adap->params.chip) <= CHELSIO_T5)
-		ingpad_shift = X_INGPADBOUNDARY_SHIFT;
-	else
-		ingpad_shift = X_T6_INGPADBOUNDARY_SHIFT;
-
-	ingpadboundary = 1 << (G_INGPADBOUNDARY(sge_control) + ingpad_shift);
-
-	fl_align = ingpadboundary;
-	if (!is_t4(adap->params.chip)) {
-		sge_control2 = t4_read_reg(adap, A_SGE_CONTROL2);
-		ingpackboundary = G_INGPACKBOUNDARY(sge_control2);
-		if (ingpackboundary == X_INGPACKBOUNDARY_16B)
-			ingpackboundary = 16;
-		else
-			ingpackboundary = 1 << (ingpackboundary +
-					X_INGPACKBOUNDARY_SHIFT);
-
-		fl_align = max(ingpadboundary, ingpackboundary);
-	}
-	return fl_align;
-}
-
 /**
  * t4_fixup_host_params_compat - fix up host-dependent parameters
  * @adap: the adapter
diff --git a/drivers/net/cxgbe/base/t4_hw.h b/drivers/net/cxgbe/base/t4_hw.h
index e77563dfa1..db5a31cf82 100644
--- a/drivers/net/cxgbe/base/t4_hw.h
+++ b/drivers/net/cxgbe/base/t4_hw.h
@@ -28,6 +28,7 @@ enum {
 enum {
 	SGE_NTIMERS = 6,          /* # of interrupt holdoff timer values */
 	SGE_NCOUNTERS = 4,        /* # of interrupt packet counter values */
+	SGE_FL_BUFFER_SIZE_NUM = 16, /* # of freelist buffser size regs */
 };
 
 /* PCI-e memory window access */
diff --git a/drivers/net/cxgbe/base/t4vf_hw.c b/drivers/net/cxgbe/base/t4vf_hw.c
index 7dbd4deb79..46d24a6339 100644
--- a/drivers/net/cxgbe/base/t4vf_hw.c
+++ b/drivers/net/cxgbe/base/t4vf_hw.c
@@ -468,46 +468,6 @@ int t4vf_set_params(struct adapter *adapter, unsigned int nparams,
 	return t4vf_wr_mbox(adapter, &cmd, sizeof(cmd), NULL);
 }
 
-/**
- * t4vf_fl_pkt_align - return the fl packet alignment
- * @adapter: the adapter
- *
- * T4 has a single field to specify the packing and padding boundary.
- * T5 onwards has separate fields for this and hence the alignment for
- * next packet offset is maximum of these two.
- */
-int t4vf_fl_pkt_align(struct adapter *adapter, u32 sge_control,
-		      u32 sge_control2)
-{
-	unsigned int ingpadboundary, ingpackboundary, fl_align, ingpad_shift;
-
-	/* T4 uses a single control field to specify both the PCIe Padding and
-	 * Packing Boundary.  T5 introduced the ability to specify these
-	 * separately.  The actual Ingress Packet Data alignment boundary
-	 * within Packed Buffer Mode is the maximum of these two
-	 * specifications.
-	 */
-	if (CHELSIO_CHIP_VERSION(adapter->params.chip) <= CHELSIO_T5)
-		ingpad_shift = X_INGPADBOUNDARY_SHIFT;
-	else
-		ingpad_shift = X_T6_INGPADBOUNDARY_SHIFT;
-
-	ingpadboundary = 1 << (G_INGPADBOUNDARY(sge_control) + ingpad_shift);
-
-	fl_align = ingpadboundary;
-	if (!is_t4(adapter->params.chip)) {
-		ingpackboundary = G_INGPACKBOUNDARY(sge_control2);
-		if (ingpackboundary == X_INGPACKBOUNDARY_16B)
-			ingpackboundary = 16;
-		else
-			ingpackboundary = 1 << (ingpackboundary +
-					X_INGPACKBOUNDARY_SHIFT);
-
-		fl_align = max(ingpadboundary, ingpackboundary);
-	}
-	return fl_align;
-}
-
 unsigned int t4vf_get_pf_from_vf(struct adapter *adapter)
 {
 	u32 whoami;
diff --git a/drivers/net/cxgbe/cxgbe_main.c b/drivers/net/cxgbe/cxgbe_main.c
index ab06e30a5e..e2a2ccb781 100644
--- a/drivers/net/cxgbe/cxgbe_main.c
+++ b/drivers/net/cxgbe/cxgbe_main.c
@@ -623,8 +623,7 @@ int cxgbe_cfg_queues(struct rte_eth_dev *eth_dev)
 			struct sge_eth_txq *t = &s->ethtxq[i];
 
 			init_rspq(adap, &r->rspq, 5, 32, 1024, 64);
-			r->usembufs = 1;
-			r->fl.size = (r->usembufs ? 1024 : 72);
+			r->fl.size = 1024;
 
 			t->q.size = 1024;
 		}
diff --git a/drivers/net/cxgbe/sge.c b/drivers/net/cxgbe/sge.c
index 566cd48406..5d91355c9a 100644
--- a/drivers/net/cxgbe/sge.c
+++ b/drivers/net/cxgbe/sge.c
@@ -58,27 +58,6 @@ static inline void ship_tx_pkt_coalesce_wr(struct adapter *adap,
  */
 #define MAX_CTRL_WR_LEN SGE_MAX_WR_LEN
 
-/*
- * Rx buffer sizes for "usembufs" Free List buffers (one ingress packet
- * per mbuf buffer).  We currently only support two sizes for 1500- and
- * 9000-byte MTUs. We could easily support more but there doesn't seem to be
- * much need for that ...
- */
-#define FL_MTU_SMALL 1500
-#define FL_MTU_LARGE 9000
-
-static inline unsigned int fl_mtu_bufsize(struct adapter *adapter,
-					  unsigned int mtu)
-{
-	struct sge *s = &adapter->sge;
-
-	return CXGBE_ALIGN(s->pktshift + RTE_ETHER_HDR_LEN + RTE_VLAN_HLEN + mtu,
-			   s->fl_align);
-}
-
-#define FL_MTU_SMALL_BUFSIZE(adapter) fl_mtu_bufsize(adapter, FL_MTU_SMALL)
-#define FL_MTU_LARGE_BUFSIZE(adapter) fl_mtu_bufsize(adapter, FL_MTU_LARGE)
-
 /*
  * Bits 0..3 of rx_sw_desc.dma_addr have special meaning.  The hardware uses
  * these to specify the buffer size as an index into the SGE Free List Buffer
@@ -93,19 +72,6 @@ enum {
 	RX_BUF_FLAGS     = 0x1f,   /* bottom five bits are special */
 	RX_BUF_SIZE      = 0x0f,   /* bottom three bits are for buf sizes */
 	RX_UNMAPPED_BUF  = 0x10,   /* buffer is not mapped */
-
-	/*
-	 * XXX We shouldn't depend on being able to use these indices.
-	 * XXX Especially when some other Master PF has initialized the
-	 * XXX adapter or we use the Firmware Configuration File.  We
-	 * XXX should really search through the Host Buffer Size register
-	 * XXX array for the appropriately sized buffer indices.
-	 */
-	RX_SMALL_PG_BUF  = 0x0,   /* small (PAGE_SIZE) page buffer */
-	RX_LARGE_PG_BUF  = 0x1,   /* buffer large page buffer */
-
-	RX_SMALL_MTU_BUF = 0x2,   /* small MTU buffer */
-	RX_LARGE_MTU_BUF = 0x3,   /* large MTU buffer */
 };
 
 /**
@@ -225,24 +191,7 @@ static inline bool fl_starving(const struct adapter *adapter,
 static inline unsigned int get_buf_size(struct adapter *adapter,
 					const struct rx_sw_desc *d)
 {
-	unsigned int rx_buf_size_idx = d->dma_addr & RX_BUF_SIZE;
-	unsigned int buf_size = 0;
-
-	switch (rx_buf_size_idx) {
-	case RX_SMALL_MTU_BUF:
-		buf_size = FL_MTU_SMALL_BUFSIZE(adapter);
-		break;
-
-	case RX_LARGE_MTU_BUF:
-		buf_size = FL_MTU_LARGE_BUFSIZE(adapter);
-		break;
-
-	default:
-		BUG_ON(1);
-		/* NOT REACHED */
-	}
-
-	return buf_size;
+	return adapter->sge.fl_buffer_size[d->dma_addr & RX_BUF_SIZE];
 }
 
 /**
@@ -358,18 +307,11 @@ static unsigned int refill_fl_usembufs(struct adapter *adap, struct sge_fl *q,
 				       int n)
 {
 	struct sge_eth_rxq *rxq = container_of(q, struct sge_eth_rxq, fl);
-	unsigned int cred = q->avail;
-	__be64 *d = &q->desc[q->pidx];
 	struct rx_sw_desc *sd = &q->sdesc[q->pidx];
-	unsigned int buf_size_idx = RX_SMALL_MTU_BUF;
+	__be64 *d = &q->desc[q->pidx];
 	struct rte_mbuf *buf_bulk[n];
+	unsigned int cred = q->avail;
 	int ret, i;
-	struct rte_pktmbuf_pool_private *mbp_priv;
-
-	/* Use jumbo mtu buffers if mbuf data room size can fit jumbo data. */
-	mbp_priv = rte_mempool_get_priv(rxq->rspq.mb_pool);
-	if ((mbp_priv->mbuf_data_room_size - RTE_PKTMBUF_HEADROOM) >= 9000)
-		buf_size_idx = RX_LARGE_MTU_BUF;
 
 	ret = rte_mempool_get_bulk(rxq->rspq.mb_pool, (void *)buf_bulk, n);
 	if (unlikely(ret != 0)) {
@@ -392,20 +334,13 @@ static unsigned int refill_fl_usembufs(struct adapter *adap, struct sge_fl *q,
 		}
 
 		rte_mbuf_refcnt_set(mbuf, 1);
-		mbuf->data_off =
-			(uint16_t)((char *)
-				   RTE_PTR_ALIGN((char *)mbuf->buf_addr +
-						 RTE_PKTMBUF_HEADROOM,
-						 adap->sge.fl_align) -
-				   (char *)mbuf->buf_addr);
+		mbuf->data_off = RTE_PKTMBUF_HEADROOM;
 		mbuf->next = NULL;
 		mbuf->nb_segs = 1;
 		mbuf->port = rxq->rspq.port_id;
 
-		mapping = (dma_addr_t)RTE_ALIGN(mbuf->buf_iova +
-						mbuf->data_off,
-						adap->sge.fl_align);
-		mapping |= buf_size_idx;
+		mapping = (dma_addr_t)(mbuf->buf_iova + mbuf->data_off);
+		mapping |= q->fl_buf_size_idx;
 		*d++ = cpu_to_be64(mapping);
 		set_rx_sw_desc(sd, mbuf, mapping);
 		sd++;
@@ -1782,6 +1717,38 @@ int t4_sge_eth_rxq_stop(struct adapter *adap, struct sge_eth_rxq *rxq)
 				rxq->rspq.cntxt_id, fl_id, 0xffff);
 }
 
+static int cxgbe_sge_fl_buf_size_index(const struct adapter *adap,
+				       struct rte_mempool *mp)
+{
+	int fl_buf_size, size, delta, min_delta = INT_MAX;
+	unsigned int i, match = UINT_MAX;
+	const struct sge *s = &adap->sge;
+
+	size = rte_pktmbuf_data_room_size(mp) - RTE_PKTMBUF_HEADROOM;
+	for (i = 0; i < RTE_DIM(s->fl_buffer_size); i++) {
+		fl_buf_size = s->fl_buffer_size[i];
+		if (fl_buf_size > size || fl_buf_size == 0)
+			continue;
+
+		delta = size - fl_buf_size;
+		if (delta < 0)
+			delta = -delta;
+		if (delta < min_delta) {
+			min_delta = delta;
+			match = i;
+		}
+	}
+
+	if (match == UINT_MAX) {
+		dev_err(adap,
+			"Could not find valid buffer size for mbuf data room: %d\n",
+			size);
+		return -EINVAL;
+	}
+
+	return match;
+}
+
 /*
  * @intr_idx: MSI/MSI-X vector if >=0, -(absolute qid + 1) if < 0
  * @cong: < 0 -> no congestion feedback, >= 0 -> congestion channel map
@@ -1791,12 +1758,20 @@ int t4_sge_alloc_rxq(struct adapter *adap, struct sge_rspq *iq, bool fwevtq,
 		     struct sge_fl *fl, rspq_handler_t hnd, int cong,
 		     struct rte_mempool *mp, int queue_id, int socket_id)
 {
-	int ret, flsz = 0;
-	struct fw_iq_cmd c;
-	struct sge *s = &adap->sge;
 	struct port_info *pi = eth_dev->data->dev_private;
+	u8 pciechan, fl_buf_size_idx = 0;
+	struct sge *s = &adap->sge;
 	unsigned int nb_refill;
-	u8 pciechan;
+	struct fw_iq_cmd c;
+	int ret, flsz = 0;
+
+	if (fl != NULL) {
+		ret = cxgbe_sge_fl_buf_size_index(adap, mp);
+		if (ret < 0)
+			return ret;
+
+		fl_buf_size_idx = ret;
+	}
 
 	/* Size needs to be multiple of 16, including status entry. */
 	iq->size = cxgbe_roundup(iq->size, 16);
@@ -1845,8 +1820,6 @@ int t4_sge_alloc_rxq(struct adapter *adap, struct sge_rspq *iq, bool fwevtq,
 	c.iqaddr = cpu_to_be64(iq->phys_addr);
 
 	if (fl) {
-		struct sge_eth_rxq *rxq = container_of(fl, struct sge_eth_rxq,
-						       fl);
 		unsigned int chip_ver = CHELSIO_CHIP_VERSION(adap->params.chip);
 
 		/*
@@ -1873,10 +1846,7 @@ int t4_sge_alloc_rxq(struct adapter *adap, struct sge_rspq *iq, bool fwevtq,
 		flsz = fl->size / 8 + s->stat_len / sizeof(struct tx_desc);
 		c.iqns_to_fl0congen |=
 			htonl(V_FW_IQ_CMD_FL0HOSTFCMODE(X_HOSTFCMODE_NONE) |
-			      (unlikely(rxq->usembufs) ?
-			       0 : F_FW_IQ_CMD_FL0PACKEN) |
-			      F_FW_IQ_CMD_FL0FETCHRO | F_FW_IQ_CMD_FL0DATARO |
-			      F_FW_IQ_CMD_FL0PADEN);
+			      F_FW_IQ_CMD_FL0FETCHRO | F_FW_IQ_CMD_FL0DATARO);
 		if (is_pf4(adap) && cong >= 0)
 			c.iqns_to_fl0congen |=
 				htonl(V_FW_IQ_CMD_FL0CNGCHMAP(cong) |
@@ -1931,6 +1901,7 @@ int t4_sge_alloc_rxq(struct adapter *adap, struct sge_rspq *iq, bool fwevtq,
 		fl->pidx = 0;
 		fl->cidx = 0;
 		fl->alloc_failed = 0;
+		fl->fl_buf_size_idx = fl_buf_size_idx;
 
 		/*
 		 * Note, we must initialize the BAR2 Free List User Doorbell
@@ -2341,10 +2312,10 @@ void t4_free_sge_resources(struct adapter *adap)
  */
 static int t4_sge_init_soft(struct adapter *adap)
 {
-	struct sge *s = &adap->sge;
-	u32 fl_small_pg, fl_large_pg, fl_small_mtu, fl_large_mtu;
 	u32 timer_value_0_and_1, timer_value_2_and_3, timer_value_4_and_5;
+	struct sge *s = &adap->sge;
 	u32 ingress_rx_threshold;
+	u8 i;
 
 	/*
 	 * Verify that CPL messages are going to the Ingress Queue for
@@ -2358,59 +2329,13 @@ static int t4_sge_init_soft(struct adapter *adap)
 	}
 
 	/*
-	 * Validate the Host Buffer Register Array indices that we want to
+	 * Read the Host Buffer Register Array indices that we want to
 	 * use ...
-	 *
-	 * XXX Note that we should really read through the Host Buffer Size
-	 * XXX register array and find the indices of the Buffer Sizes which
-	 * XXX meet our needs!
 	 */
-#define READ_FL_BUF(x) \
-	t4_read_reg(adap, A_SGE_FL_BUFFER_SIZE0 + (x) * sizeof(u32))
-
-	fl_small_pg = READ_FL_BUF(RX_SMALL_PG_BUF);
-	fl_large_pg = READ_FL_BUF(RX_LARGE_PG_BUF);
-	fl_small_mtu = READ_FL_BUF(RX_SMALL_MTU_BUF);
-	fl_large_mtu = READ_FL_BUF(RX_LARGE_MTU_BUF);
-
-	/*
-	 * We only bother using the Large Page logic if the Large Page Buffer
-	 * is larger than our Page Size Buffer.
-	 */
-	if (fl_large_pg <= fl_small_pg)
-		fl_large_pg = 0;
-
-#undef READ_FL_BUF
-
-	/*
-	 * The Page Size Buffer must be exactly equal to our Page Size and the
-	 * Large Page Size Buffer should be 0 (per above) or a power of 2.
-	 */
-	if (fl_small_pg != CXGBE_PAGE_SIZE ||
-	    (fl_large_pg & (fl_large_pg - 1)) != 0) {
-		dev_err(adap, "bad SGE FL page buffer sizes [%d, %d]\n",
-			fl_small_pg, fl_large_pg);
-		return -EINVAL;
-	}
-	if (fl_large_pg)
-		s->fl_pg_order = ilog2(fl_large_pg) - PAGE_SHIFT;
-
-	if (adap->use_unpacked_mode) {
-		int err = 0;
-
-		if (fl_small_mtu < FL_MTU_SMALL_BUFSIZE(adap)) {
-			dev_err(adap, "bad SGE FL small MTU %d\n",
-				fl_small_mtu);
-			err = -EINVAL;
-		}
-		if (fl_large_mtu < FL_MTU_LARGE_BUFSIZE(adap)) {
-			dev_err(adap, "bad SGE FL large MTU %d\n",
-				fl_large_mtu);
-			err = -EINVAL;
-		}
-		if (err)
-			return err;
-	}
+	for (i = 0; i < RTE_DIM(s->fl_buffer_size); i++)
+		s->fl_buffer_size[i] = t4_read_reg(adap,
+						   A_SGE_FL_BUFFER_SIZE0 +
+						   i * sizeof(u32));
 
 	/*
 	 * Retrieve our RX interrupt holdoff timer values and counter
@@ -2454,7 +2379,6 @@ int t4_sge_init(struct adapter *adap)
 	sge_control = t4_read_reg(adap, A_SGE_CONTROL);
 	s->pktshift = G_PKTSHIFT(sge_control);
 	s->stat_len = (sge_control & F_EGRSTATUSPAGESIZE) ? 128 : 64;
-	s->fl_align = t4_fl_pkt_align(adap);
 	ret = t4_sge_init_soft(adap);
 	if (ret < 0) {
 		dev_err(adap, "%s: t4_sge_init_soft failed, error %d\n",
@@ -2489,8 +2413,6 @@ int t4vf_sge_init(struct adapter *adap)
 	struct sge_params *sge_params = &adap->params.sge;
 	u32 sge_ingress_queues_per_page;
 	u32 sge_egress_queues_per_page;
-	u32 sge_control, sge_control2;
-	u32 fl_small_pg, fl_large_pg;
 	u32 sge_ingress_rx_threshold;
 	u32 sge_timer_value_0_and_1;
 	u32 sge_timer_value_2_and_3;
@@ -2500,7 +2422,9 @@ int t4vf_sge_init(struct adapter *adap)
 	unsigned int s_hps, s_qpp;
 	u32 sge_host_page_size;
 	u32 params[7], vals[7];
+	u32 sge_control;
 	int v;
+	u8 i;
 
 	/* query basic params from fw */
 	params[0] = (V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
@@ -2508,14 +2432,10 @@ int t4vf_sge_init(struct adapter *adap)
 	params[1] = (V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
 		     V_FW_PARAMS_PARAM_XYZ(A_SGE_HOST_PAGE_SIZE));
 	params[2] = (V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
-		     V_FW_PARAMS_PARAM_XYZ(A_SGE_FL_BUFFER_SIZE0));
-	params[3] = (V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
-		     V_FW_PARAMS_PARAM_XYZ(A_SGE_FL_BUFFER_SIZE1));
-	params[4] = (V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
 		     V_FW_PARAMS_PARAM_XYZ(A_SGE_TIMER_VALUE_0_AND_1));
-	params[5] = (V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
+	params[3] = (V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
 		     V_FW_PARAMS_PARAM_XYZ(A_SGE_TIMER_VALUE_2_AND_3));
-	params[6] = (V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
+	params[4] = (V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
 		     V_FW_PARAMS_PARAM_XYZ(A_SGE_TIMER_VALUE_4_AND_5));
 	v = t4vf_query_params(adap, 7, params, vals);
 	if (v != FW_SUCCESS)
@@ -2523,51 +2443,32 @@ int t4vf_sge_init(struct adapter *adap)
 
 	sge_control = vals[0];
 	sge_host_page_size = vals[1];
-	fl_small_pg = vals[2];
-	fl_large_pg = vals[3];
-	sge_timer_value_0_and_1 = vals[4];
-	sge_timer_value_2_and_3 = vals[5];
-	sge_timer_value_4_and_5 = vals[6];
+	sge_timer_value_0_and_1 = vals[2];
+	sge_timer_value_2_and_3 = vals[3];
+	sge_timer_value_4_and_5 = vals[4];
+
+	for (i = 0; i < RTE_DIM(s->fl_buffer_size); i++) {
+		params[0] = (V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
+			     V_FW_PARAMS_PARAM_XYZ(A_SGE_FL_BUFFER_SIZE0 +
+						   i * sizeof(u32)));
+		v = t4vf_query_params(adap, 1, params, vals);
+		if (v != FW_SUCCESS)
+			return v;
+
+		s->fl_buffer_size[i] = vals[0];
+	}
 
 	/*
 	 * Start by vetting the basic SGE parameters which have been set up by
 	 * the Physical Function Driver.
 	 */
 
-	/* We only bother using the Large Page logic if the Large Page Buffer
-	 * is larger than our Page Size Buffer.
-	 */
-	if (fl_large_pg <= fl_small_pg)
-		fl_large_pg = 0;
-
-	/* The Page Size Buffer must be exactly equal to our Page Size and the
-	 * Large Page Size Buffer should be 0 (per above) or a power of 2.
-	 */
-	if (fl_small_pg != CXGBE_PAGE_SIZE ||
-	    (fl_large_pg & (fl_large_pg - 1)) != 0) {
-		dev_err(adapter->pdev_dev, "bad SGE FL buffer sizes [%d, %d]\n",
-			fl_small_pg, fl_large_pg);
-		return -EINVAL;
-	}
-
 	if ((sge_control & F_RXPKTCPLMODE) !=
 	    V_RXPKTCPLMODE(X_RXPKTCPLMODE_SPLIT)) {
 		dev_err(adapter->pdev_dev, "bad SGE CPL MODE\n");
 		return -EINVAL;
 	}
 
-
-	/* Grab ingress packing boundary from SGE_CONTROL2 for */
-	params[0] = (V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
-		     V_FW_PARAMS_PARAM_XYZ(A_SGE_CONTROL2));
-	v = t4vf_query_params(adap, 1, params, vals);
-	if (v != FW_SUCCESS) {
-		dev_err(adapter, "Unable to get SGE Control2; "
-			"probably old firmware.\n");
-		return v;
-	}
-	sge_control2 = vals[0];
-
 	params[0] = (V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
 		     V_FW_PARAMS_PARAM_XYZ(A_SGE_INGRESS_RX_THRESHOLD));
 	params[1] = (V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
@@ -2612,12 +2513,9 @@ int t4vf_sge_init(struct adapter *adap)
 	/*
 	 * Now translate the queried parameters into our internal forms.
 	 */
-	if (fl_large_pg)
-		s->fl_pg_order = ilog2(fl_large_pg) - PAGE_SHIFT;
 	s->stat_len = ((sge_control & F_EGRSTATUSPAGESIZE)
 			? 128 : 64);
 	s->pktshift = G_PKTSHIFT(sge_control);
-	s->fl_align = t4vf_fl_pkt_align(adap, sge_control, sge_control2);
 
 	/*
 	 * A FL with <= fl_starve_thres buffers is starving and a periodic
-- 
2.27.0

[PATCH 4/5] net/cxgbe: track packets dropped by TP due to congestion

[Rahul Lakkireddy]

Rx packets can get dropped at TP due to congestion and this info
will not get propagated to MPS. Track these Rx dropped packets
in imissed counter. Also add xstats for these counters.

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
 drivers/net/cxgbe/base/common.h  |  2 ++
 drivers/net/cxgbe/base/t4_hw.c   | 14 ++++++++++++--
 drivers/net/cxgbe/base/t4_regs.h |  4 ++++
 drivers/net/cxgbe/cxgbe_ethdev.c | 11 +++++++++++
 4 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/net/cxgbe/base/common.h b/drivers/net/cxgbe/base/common.h
index 980d78e4a4..4482ddbf56 100644
--- a/drivers/net/cxgbe/base/common.h
+++ b/drivers/net/cxgbe/base/common.h
@@ -101,6 +101,8 @@ struct port_stats {
 	u64 rx_trunc1;            /* buffer-group 1 truncated packets */
 	u64 rx_trunc2;            /* buffer-group 2 truncated packets */
 	u64 rx_trunc3;            /* buffer-group 3 truncated packets */
+
+	u64 rx_tp_tnl_cong_drops[NCHAN]; /* TP frame drops due to congestion */
 };
 
 struct sge_params {
diff --git a/drivers/net/cxgbe/base/t4_hw.c b/drivers/net/cxgbe/base/t4_hw.c
index 84c4316e68..384080e6d3 100644
--- a/drivers/net/cxgbe/base/t4_hw.c
+++ b/drivers/net/cxgbe/base/t4_hw.c
@@ -3040,8 +3040,10 @@ unsigned int t4_get_tp_ch_map(struct adapter *adapter, unsigned int pidx)
  */
 void t4_get_port_stats(struct adapter *adap, int idx, struct port_stats *p)
 {
-	u32 bgmap = t4_get_mps_bg_map(adap, idx);
 	u32 stat_ctl = t4_read_reg(adap, A_MPS_STAT_CTL);
+	u32 bgmap = t4_get_mps_bg_map(adap, idx);
+	u32 val[NCHAN] = { 0 };
+	u8 i;
 
 #define GET_STAT(name) \
 	t4_read_reg64(adap, \
@@ -3129,6 +3131,11 @@ void t4_get_port_stats(struct adapter *adap, int idx, struct port_stats *p)
 	p->rx_trunc2 = (bgmap & 4) ? GET_STAT_COM(RX_BG_2_MAC_TRUNC_FRAME) : 0;
 	p->rx_trunc3 = (bgmap & 8) ? GET_STAT_COM(RX_BG_3_MAC_TRUNC_FRAME) : 0;
 
+	t4_read_indirect(adap, A_TP_MIB_INDEX, A_TP_MIB_DATA, &val[idx], 1,
+			 A_TP_MIB_TNL_CNG_DROP_0 + idx);
+
+	for (i = 0; i < NCHAN; i++)
+		p->rx_tp_tnl_cong_drops[i] = val[i];
 #undef GET_STAT
 #undef GET_STAT_COM
 }
@@ -3163,9 +3170,10 @@ void t4_get_port_stats_offset(struct adapter *adap, int idx,
  */
 void t4_clr_port_stats(struct adapter *adap, int idx)
 {
-	unsigned int i;
 	u32 bgmap = t4_get_mps_bg_map(adap, idx);
 	u32 port_base_addr;
+	unsigned int i;
+	u32 val = 0;
 
 	if (is_t4(adap->params.chip))
 		port_base_addr = PORT_BASE(idx);
@@ -3187,6 +3195,8 @@ void t4_clr_port_stats(struct adapter *adap, int idx)
 				     A_MPS_STAT_RX_BG_0_MAC_TRUNC_FRAME_L +
 				     i * 8, 0);
 		}
+	t4_write_indirect(adap, A_TP_MIB_INDEX, A_TP_MIB_DATA,
+			  &val, 1, A_TP_MIB_TNL_CNG_DROP_0 + idx);
 }
 
 /**
diff --git a/drivers/net/cxgbe/base/t4_regs.h b/drivers/net/cxgbe/base/t4_regs.h
index 8a14d09a15..ff60dc1bb3 100644
--- a/drivers/net/cxgbe/base/t4_regs.h
+++ b/drivers/net/cxgbe/base/t4_regs.h
@@ -525,6 +525,8 @@
 
 #define A_TP_PIO_ADDR 0x7e40
 #define A_TP_PIO_DATA 0x7e44
+#define A_TP_MIB_INDEX 0x7e50
+#define A_TP_MIB_DATA 0x7e54
 
 #define A_TP_RSS_SECRET_KEY0 0x40
 
@@ -587,6 +589,8 @@
 #define S_RM_OVLAN	9
 #define V_RM_OVLAN(x)	((x) << S_RM_OVLAN)
 
+#define A_TP_MIB_TNL_CNG_DROP_0 0x18
+
 /* registers for module MA */
 #define A_MA_EDRAM0_BAR 0x77c0
 
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index e7ea76180f..cf9a2fdc19 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -709,6 +709,9 @@ static int cxgbe_dev_stats_get(struct rte_eth_dev *eth_dev,
 			      ps.rx_ovflow2 + ps.rx_ovflow3 +
 			      ps.rx_trunc0 + ps.rx_trunc1 +
 			      ps.rx_trunc2 + ps.rx_trunc3;
+	for (i = 0; i < NCHAN; i++)
+		eth_stats->imissed += ps.rx_tp_tnl_cong_drops[i];
+
 	eth_stats->ierrors  = ps.rx_symbol_err + ps.rx_fcs_err +
 			      ps.rx_jabber + ps.rx_too_long + ps.rx_runt +
 			      ps.rx_len_err;
@@ -851,6 +854,14 @@ static const struct cxgbe_dev_xstats_name_off cxgbe_dev_port_stats_strings[] = {
 	{"rx_bg1_truncated_packets", offsetof(struct port_stats, rx_trunc1)},
 	{"rx_bg2_truncated_packets", offsetof(struct port_stats, rx_trunc2)},
 	{"rx_bg3_truncated_packets", offsetof(struct port_stats, rx_trunc3)},
+	{"rx_tp_tnl_cong_drops0",
+	 offsetof(struct port_stats, rx_tp_tnl_cong_drops[0])},
+	{"rx_tp_tnl_cong_drops1",
+	 offsetof(struct port_stats, rx_tp_tnl_cong_drops[1])},
+	{"rx_tp_tnl_cong_drops2",
+	 offsetof(struct port_stats, rx_tp_tnl_cong_drops[2])},
+	{"rx_tp_tnl_cong_drops3",
+	 offsetof(struct port_stats, rx_tp_tnl_cong_drops[3])},
 };
 
 static const struct cxgbe_dev_xstats_name_off
-- 
2.27.0

Re: [PATCH 4/5] net/cxgbe: track packets dropped by TP due to congestion

[Ferruh Yigit]

On 4/18/2022 11:24 PM, Rahul Lakkireddy wrote:
> Rx packets can get dropped at TP due to congestion and this info
> will not get propagated to MPS. Track these Rx dropped packets
> in imissed counter. Also add xstats for these counters.
> 
> Signed-off-by: Rahul Lakkireddy<rahul.lakkireddy@chelsio.com>

What 'TP' stands for? As far as I understand it is a kind of FW (TP 
Microcode) but I didn't able to find any reference to it.

Re: [PATCH 4/5] net/cxgbe: track packets dropped by TP due to congestion

[Rahul Lakkireddy]

On Thursday, May 05/05/22, 2022 at 17:28:55 +0100, Ferruh Yigit wrote:
> On 4/18/2022 11:24 PM, Rahul Lakkireddy wrote:
> > Rx packets can get dropped at TP due to congestion and this info
> > will not get propagated to MPS. Track these Rx dropped packets
> > in imissed counter. Also add xstats for these counters.
> > 
> > Signed-off-by: Rahul Lakkireddy<rahul.lakkireddy@chelsio.com>
> 
> What 'TP' stands for? As far as I understand it is a kind of FW (TP
> Microcode) but I didn't able to find any reference to it.

Transport Processor (TP) on the NIC delivers the incoming packets
from the wire to NIC's DMA engine to place the packets in Rx buffers.
TP sends signal towards the Multi-Port Switch (MPS) near the MAC when
the Rxqs run out of Rx buffers posted by driver. These MPS buffer drop
stats are already accounted for in imissed counters. However, if a
large number of Rxqs run out of Rx buffers simultaneously, then the
TP can start dropping packets by itself without informing to the MPS
if there is heavy congestion on the channel. These packets dropped by
TP that could not be informed to MPS are now being accounted for in
imissed counters in this patch.

Thanks,
Rahul

Re: [PATCH 4/5] net/cxgbe: track packets dropped by TP due to congestion

[Ferruh Yigit]

On 5/6/2022 12:09 PM, Rahul Lakkireddy wrote:
> On Thursday, May 05/05/22, 2022 at 17:28:55 +0100, Ferruh Yigit wrote:
>> On 4/18/2022 11:24 PM, Rahul Lakkireddy wrote:
>>> Rx packets can get dropped at TP due to congestion and this info
>>> will not get propagated to MPS. Track these Rx dropped packets
>>> in imissed counter. Also add xstats for these counters.
>>>
>>> Signed-off-by: Rahul Lakkireddy<rahul.lakkireddy@chelsio.com>
>>
>> What 'TP' stands for? As far as I understand it is a kind of FW (TP
>> Microcode) but I didn't able to find any reference to it.
> 
> Transport Processor (TP) on the NIC delivers the incoming packets
> from the wire to NIC's DMA engine to place the packets in Rx buffers.
> TP sends signal towards the Multi-Port Switch (MPS) near the MAC when
> the Rxqs run out of Rx buffers posted by driver. These MPS buffer drop
> stats are already accounted for in imissed counters. However, if a
> large number of Rxqs run out of Rx buffers simultaneously, then the
> TP can start dropping packets by itself without informing to the MPS
> if there is heavy congestion on the channel. These packets dropped by
> TP that could not be informed to MPS are now being accounted for in
> imissed counters in this patch.
> 

Thanks Rahul, this is clearer explanation than the original commit log.

Can you please put what abbreviations stands for (as you did in above 
paragraph) in the commit log in next version? Like:
'Transport Processor (TP)', 'Multi-Port Switch (MPS)'

[PATCH v2] net/cxgbe: track packets dropped by TP due to congestion

[Rahul Lakkireddy]

Transport Processor (TP) on the NIC delivers the incoming packets
from the wire to NIC's DMA engine to place the packets in Rx buffers.
TP sends signal towards the Multi-Port Switch (MPS) near the MAC when
the Rxqs run out of Rx buffers posted by driver. These MPS buffer drop
stats are already accounted for in imissed counters. However, if a
large number of Rxqs run out of Rx buffers simultaneously, then the
TP can start dropping packets by itself when there is heavy congestion
on the channel and hence could not inform to the MPS. So, track these
packets dropped by TP in imissed counters. Also add xstats for these
counters.

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
v2:
- Update commit message with more info about the TP counters.

 drivers/net/cxgbe/base/common.h  |  2 ++
 drivers/net/cxgbe/base/t4_hw.c   | 14 ++++++++++++--
 drivers/net/cxgbe/base/t4_regs.h |  4 ++++
 drivers/net/cxgbe/cxgbe_ethdev.c | 11 +++++++++++
 4 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/net/cxgbe/base/common.h b/drivers/net/cxgbe/base/common.h
index 980d78e4a4..4482ddbf56 100644
--- a/drivers/net/cxgbe/base/common.h
+++ b/drivers/net/cxgbe/base/common.h
@@ -101,6 +101,8 @@ struct port_stats {
 	u64 rx_trunc1;            /* buffer-group 1 truncated packets */
 	u64 rx_trunc2;            /* buffer-group 2 truncated packets */
 	u64 rx_trunc3;            /* buffer-group 3 truncated packets */
+
+	u64 rx_tp_tnl_cong_drops[NCHAN]; /* TP frame drops due to congestion */
 };
 
 struct sge_params {
diff --git a/drivers/net/cxgbe/base/t4_hw.c b/drivers/net/cxgbe/base/t4_hw.c
index 84c4316e68..384080e6d3 100644
--- a/drivers/net/cxgbe/base/t4_hw.c
+++ b/drivers/net/cxgbe/base/t4_hw.c
@@ -3040,8 +3040,10 @@ unsigned int t4_get_tp_ch_map(struct adapter *adapter, unsigned int pidx)
  */
 void t4_get_port_stats(struct adapter *adap, int idx, struct port_stats *p)
 {
-	u32 bgmap = t4_get_mps_bg_map(adap, idx);
 	u32 stat_ctl = t4_read_reg(adap, A_MPS_STAT_CTL);
+	u32 bgmap = t4_get_mps_bg_map(adap, idx);
+	u32 val[NCHAN] = { 0 };
+	u8 i;
 
 #define GET_STAT(name) \
 	t4_read_reg64(adap, \
@@ -3129,6 +3131,11 @@ void t4_get_port_stats(struct adapter *adap, int idx, struct port_stats *p)
 	p->rx_trunc2 = (bgmap & 4) ? GET_STAT_COM(RX_BG_2_MAC_TRUNC_FRAME) : 0;
 	p->rx_trunc3 = (bgmap & 8) ? GET_STAT_COM(RX_BG_3_MAC_TRUNC_FRAME) : 0;
 
+	t4_read_indirect(adap, A_TP_MIB_INDEX, A_TP_MIB_DATA, &val[idx], 1,
+			 A_TP_MIB_TNL_CNG_DROP_0 + idx);
+
+	for (i = 0; i < NCHAN; i++)
+		p->rx_tp_tnl_cong_drops[i] = val[i];
 #undef GET_STAT
 #undef GET_STAT_COM
 }
@@ -3163,9 +3170,10 @@ void t4_get_port_stats_offset(struct adapter *adap, int idx,
  */
 void t4_clr_port_stats(struct adapter *adap, int idx)
 {
-	unsigned int i;
 	u32 bgmap = t4_get_mps_bg_map(adap, idx);
 	u32 port_base_addr;
+	unsigned int i;
+	u32 val = 0;
 
 	if (is_t4(adap->params.chip))
 		port_base_addr = PORT_BASE(idx);
@@ -3187,6 +3195,8 @@ void t4_clr_port_stats(struct adapter *adap, int idx)
 				     A_MPS_STAT_RX_BG_0_MAC_TRUNC_FRAME_L +
 				     i * 8, 0);
 		}
+	t4_write_indirect(adap, A_TP_MIB_INDEX, A_TP_MIB_DATA,
+			  &val, 1, A_TP_MIB_TNL_CNG_DROP_0 + idx);
 }
 
 /**
diff --git a/drivers/net/cxgbe/base/t4_regs.h b/drivers/net/cxgbe/base/t4_regs.h
index 8a14d09a15..ff60dc1bb3 100644
--- a/drivers/net/cxgbe/base/t4_regs.h
+++ b/drivers/net/cxgbe/base/t4_regs.h
@@ -525,6 +525,8 @@
 
 #define A_TP_PIO_ADDR 0x7e40
 #define A_TP_PIO_DATA 0x7e44
+#define A_TP_MIB_INDEX 0x7e50
+#define A_TP_MIB_DATA 0x7e54
 
 #define A_TP_RSS_SECRET_KEY0 0x40
 
@@ -587,6 +589,8 @@
 #define S_RM_OVLAN	9
 #define V_RM_OVLAN(x)	((x) << S_RM_OVLAN)
 
+#define A_TP_MIB_TNL_CNG_DROP_0 0x18
+
 /* registers for module MA */
 #define A_MA_EDRAM0_BAR 0x77c0
 
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index e7ea76180f..cf9a2fdc19 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -709,6 +709,9 @@ static int cxgbe_dev_stats_get(struct rte_eth_dev *eth_dev,
 			      ps.rx_ovflow2 + ps.rx_ovflow3 +
 			      ps.rx_trunc0 + ps.rx_trunc1 +
 			      ps.rx_trunc2 + ps.rx_trunc3;
+	for (i = 0; i < NCHAN; i++)
+		eth_stats->imissed += ps.rx_tp_tnl_cong_drops[i];
+
 	eth_stats->ierrors  = ps.rx_symbol_err + ps.rx_fcs_err +
 			      ps.rx_jabber + ps.rx_too_long + ps.rx_runt +
 			      ps.rx_len_err;
@@ -851,6 +854,14 @@ static const struct cxgbe_dev_xstats_name_off cxgbe_dev_port_stats_strings[] = {
 	{"rx_bg1_truncated_packets", offsetof(struct port_stats, rx_trunc1)},
 	{"rx_bg2_truncated_packets", offsetof(struct port_stats, rx_trunc2)},
 	{"rx_bg3_truncated_packets", offsetof(struct port_stats, rx_trunc3)},
+	{"rx_tp_tnl_cong_drops0",
+	 offsetof(struct port_stats, rx_tp_tnl_cong_drops[0])},
+	{"rx_tp_tnl_cong_drops1",
+	 offsetof(struct port_stats, rx_tp_tnl_cong_drops[1])},
+	{"rx_tp_tnl_cong_drops2",
+	 offsetof(struct port_stats, rx_tp_tnl_cong_drops[2])},
+	{"rx_tp_tnl_cong_drops3",
+	 offsetof(struct port_stats, rx_tp_tnl_cong_drops[3])},
 };
 
 static const struct cxgbe_dev_xstats_name_off
-- 
2.27.0

Re: [PATCH v2] net/cxgbe: track packets dropped by TP due to congestion

[Ferruh Yigit]

On 5/6/2022 2:18 PM, Rahul Lakkireddy wrote:
> Transport Processor (TP) on the NIC delivers the incoming packets
> from the wire to NIC's DMA engine to place the packets in Rx buffers.
> TP sends signal towards the Multi-Port Switch (MPS) near the MAC when
> the Rxqs run out of Rx buffers posted by driver. These MPS buffer drop
> stats are already accounted for in imissed counters. However, if a
> large number of Rxqs run out of Rx buffers simultaneously, then the
> TP can start dropping packets by itself when there is heavy congestion
> on the channel and hence could not inform to the MPS. So, track these
> packets dropped by TP in imissed counters. Also add xstats for these
> counters.
> 
> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>

Applied to dpdk-next-net/main, thanks.

[PATCH 5/5] net/cxgbe: read firmware configuration file from filesystem

[Rahul Lakkireddy]

Add support to read firmware configuration file from
/lib/firmware/cxgb4/ path in the filesystem.

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
 drivers/net/cxgbe/base/t4fw_interface.h |   1 +
 drivers/net/cxgbe/cxgbe_main.c          | 329 ++++++++++++++++--------
 2 files changed, 217 insertions(+), 113 deletions(-)

diff --git a/drivers/net/cxgbe/base/t4fw_interface.h b/drivers/net/cxgbe/base/t4fw_interface.h
index a0a9292c0c..76f58d7c77 100644
--- a/drivers/net/cxgbe/base/t4fw_interface.h
+++ b/drivers/net/cxgbe/base/t4fw_interface.h
@@ -697,6 +697,7 @@ enum fw_params_param_dev {
 						 */
 	FW_PARAMS_PARAM_DEV_FWREV	= 0x0B, /* fw version */
 	FW_PARAMS_PARAM_DEV_TPREV	= 0x0C, /* tp version */
+	FW_PARAMS_PARAM_DEV_CF		= 0x0D,
 	FW_PARAMS_PARAM_DEV_ULPTX_MEMWRITE_DSGL = 0x17,
 	FW_PARAMS_PARAM_DEV_FILTER2_WR	= 0x1D,
 	FW_PARAMS_PARAM_DEV_OPAQUE_VIID_SMT_EXTN = 0x27,
diff --git a/drivers/net/cxgbe/cxgbe_main.c b/drivers/net/cxgbe/cxgbe_main.c
index e2a2ccb781..7b162af3e7 100644
--- a/drivers/net/cxgbe/cxgbe_main.c
+++ b/drivers/net/cxgbe/cxgbe_main.c
@@ -4,6 +4,7 @@
  */
 
 #include <sys/queue.h>
+#include <sys/stat.h>
 #include <stdio.h>
 #include <errno.h>
 #include <stdint.h>
@@ -11,6 +12,7 @@
 #include <unistd.h>
 #include <stdarg.h>
 #include <inttypes.h>
+#include <fcntl.h>
 #include <netinet/in.h>
 
 #include <rte_byteorder.h>
@@ -1006,6 +1008,218 @@ static int configure_filter_mode_mask(struct adapter *adap)
 			     params, val);
 }
 
+#define CXGBE_FW_CONFIG_PATH_T5 "/lib/firmware/cxgb4/t5-config.txt"
+#define CXGBE_FW_CONFIG_PATH_T6 "/lib/firmware/cxgb4/t6-config.txt"
+
+/*
+ * Load firmware configuration from file in /lib/firmware/cxgb4/ path,
+ * if it is present.
+ */
+static int cxgbe_load_fw_config_from_filesystem(struct adapter *adap,
+						const char **config_name,
+						u32 *mem_type, u32 *mem_addr)
+{
+	u32 param, val, mtype, maddr;
+	const char *fw_cfg_path;
+	char *fw_cfg = NULL;
+	struct stat st;
+	int ret, fd;
+
+	switch (CHELSIO_CHIP_VERSION(adap->params.chip)) {
+	case CHELSIO_T5:
+		fw_cfg_path = CXGBE_FW_CONFIG_PATH_T5;
+		break;
+	case CHELSIO_T6:
+		fw_cfg_path = CXGBE_FW_CONFIG_PATH_T6;
+		break;
+	default:
+		return -ENOENT;
+	}
+
+	ret = open(fw_cfg_path, O_RDONLY);
+	if (ret < 0) {
+		dev_debug(adap, "Couldn't open FW config file\n");
+		return ret;
+	}
+
+	fd = ret;
+
+	ret = fstat(fd, &st);
+	if (ret < 0) {
+		dev_debug(adap, "Couldn't get FW config file size\n");
+		goto out_err;
+	}
+
+	if (st.st_size >= FLASH_CFG_MAX_SIZE) {
+		dev_debug(adap, "FW config file size >= max(%u)\n",
+			  FLASH_CFG_MAX_SIZE);
+		ret = -ENOMEM;
+		goto out_err;
+	}
+
+	fw_cfg = rte_zmalloc(NULL, st.st_size, 0);
+	if (fw_cfg == NULL) {
+		ret = -ENOMEM;
+		goto out_err;
+	}
+
+	if (read(fd, fw_cfg, st.st_size) != st.st_size) {
+		dev_debug(adap, "Couldn't read FW config file data\n");
+		ret = -EIO;
+		goto out_err;
+	}
+
+	close(fd);
+
+	/* Send it to FW to verify and update to new configuration */
+	param = V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_DEV) |
+		V_FW_PARAMS_PARAM_X(FW_PARAMS_PARAM_DEV_CF);
+	ret = t4_query_params(adap, adap->mbox, adap->pf, 0, 1, &param, &val);
+	if (ret < 0) {
+		dev_debug(adap, "FW config param query failed: %d\n", ret);
+		goto out_free;
+	}
+
+	mtype = val >> 8;
+	maddr = (val & 0xff) << 16;
+
+	t4_os_lock(&adap->win0_lock);
+	ret = t4_memory_rw(adap, MEMWIN_NIC, mtype, maddr, st.st_size,
+			   fw_cfg, T4_MEMORY_WRITE);
+	t4_os_unlock(&adap->win0_lock);
+	if (ret < 0) {
+		dev_debug(adap, "FW config file update failed: %d\n", ret);
+		goto out_free;
+	}
+
+	rte_free(fw_cfg);
+
+	*mem_type = mtype;
+	*mem_addr = maddr;
+	*config_name = fw_cfg_path;
+	return 0;
+
+out_err:
+	close(fd);
+out_free:
+	rte_free(fw_cfg);
+	return ret;
+}
+
+static int cxgbe_load_fw_config(struct adapter *adap)
+{
+	u32 finiver, finicsum, cfcsum, mtype, maddr, param, val;
+	struct fw_caps_config_cmd caps_cmd = { 0 };
+	const char *config_name = NULL;
+	int ret;
+
+	ret = cxgbe_load_fw_config_from_filesystem(adap, &config_name,
+						   &mtype, &maddr);
+	if (ret < 0) {
+		config_name = "On Flash";
+
+		ret = t4_flash_cfg_addr(adap);
+		if (ret < 0) {
+			dev_warn(adap,
+				 "Finding address for FW config file in flash failed: %d\n",
+				 ret);
+			goto out_default_config;
+		}
+
+		mtype = FW_MEMTYPE_CF_FLASH;
+		maddr = ret;
+	}
+
+	/* Enable HASH filter region when support is available. */
+	val = 1;
+	param = CXGBE_FW_PARAM_DEV(HASHFILTER_WITH_OFLD);
+	t4_set_params(adap, adap->mbox, adap->pf, 0, 1, &param, &val);
+
+	/*
+	 * Issue a Capability Configuration command to the firmware to get it
+	 * to parse the Configuration File.
+	 */
+	caps_cmd.op_to_write = cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
+					   F_FW_CMD_REQUEST | F_FW_CMD_READ);
+	caps_cmd.cfvalid_to_len16 =
+		cpu_to_be32(F_FW_CAPS_CONFIG_CMD_CFVALID |
+			    V_FW_CAPS_CONFIG_CMD_MEMTYPE_CF(mtype) |
+			    V_FW_CAPS_CONFIG_CMD_MEMADDR64K_CF(maddr >> 16) |
+			    FW_LEN16(caps_cmd));
+	ret = t4_wr_mbox(adap, adap->mbox, &caps_cmd, sizeof(caps_cmd),
+			 &caps_cmd);
+
+out_default_config:
+	/*
+	 * If the CAPS_CONFIG failed with an ENOENT (for a Firmware
+	 * Configuration File in filesystem or FLASH), our last gasp
+	 * effort is to use the Firmware Configuration File which is
+	 * embedded in the firmware.
+	 */
+	if (ret == -ENOENT) {
+		config_name = "Firmware Default";
+
+		memset(&caps_cmd, 0, sizeof(caps_cmd));
+		caps_cmd.op_to_write =
+			cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
+				    F_FW_CMD_REQUEST | F_FW_CMD_READ);
+		caps_cmd.cfvalid_to_len16 = cpu_to_be32(FW_LEN16(caps_cmd));
+		ret = t4_wr_mbox(adap, adap->mbox, &caps_cmd, sizeof(caps_cmd),
+				 &caps_cmd);
+	}
+
+	if (ret < 0) {
+		dev_info(adap,
+			 "Failed to configure using %s Firmware Configuration file: %d\n",
+			 config_name, ret);
+		return ret;
+	}
+
+	finiver = be32_to_cpu(caps_cmd.finiver);
+	finicsum = be32_to_cpu(caps_cmd.finicsum);
+	cfcsum = be32_to_cpu(caps_cmd.cfcsum);
+	if (finicsum != cfcsum)
+		dev_warn(adap,
+			 "Configuration File checksum mismatch: [fini] csum=0x%x, computed csum=0x%x\n",
+			 finicsum, cfcsum);
+
+	/*
+	 * If we're a pure NIC driver then disable all offloading facilities.
+	 * This will allow the firmware to optimize aspects of the hardware
+	 * configuration which will result in improved performance.
+	 */
+	caps_cmd.niccaps &= cpu_to_be16(~FW_CAPS_CONFIG_NIC_ETHOFLD);
+	caps_cmd.toecaps = 0;
+	caps_cmd.iscsicaps = 0;
+	caps_cmd.rdmacaps = 0;
+	caps_cmd.fcoecaps = 0;
+	caps_cmd.cryptocaps = 0;
+
+	/*
+	 * And now tell the firmware to use the configuration we just loaded.
+	 */
+	caps_cmd.op_to_write = cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
+					   F_FW_CMD_REQUEST | F_FW_CMD_WRITE);
+	caps_cmd.cfvalid_to_len16 = htonl(FW_LEN16(caps_cmd));
+	ret = t4_wr_mbox(adap, adap->mbox, &caps_cmd, sizeof(caps_cmd),
+			 NULL);
+	if (ret < 0) {
+		dev_warn(adap, "Unable to finalize Firmware Capabilities %d\n",
+			 ret);
+		return ret;
+	}
+
+	/*
+	 * Return successfully and note that we're operating with parameters
+	 * not supplied by the driver, rather than from hard-wired
+	 * initialization constants buried in the driver.
+	 */
+	dev_info(adap,
+		 "Successfully configured using Firmware Configuration File \"%s\", version: 0x%x, computed csum: 0x%x\n",
+		 config_name, finiver, cfcsum);
+	return 0;
+}
+
 static void configure_pcie_ext_tag(struct adapter *adapter)
 {
 	u16 v;
@@ -1119,12 +1333,7 @@ static int adap_init0_tweaks(struct adapter *adapter)
  */
 static int adap_init0_config(struct adapter *adapter, int reset)
 {
-	u32 finiver, finicsum, cfcsum, param, val;
-	struct fw_caps_config_cmd caps_cmd;
-	unsigned long mtype = 0, maddr = 0;
-	u8 config_issued = 0;
-	char config_name[20];
-	int cfg_addr, ret;
+	int ret;
 
 	/*
 	 * Reset device if necessary.
@@ -1139,98 +1348,10 @@ static int adap_init0_config(struct adapter *adapter, int reset)
 		}
 	}
 
-	cfg_addr = t4_flash_cfg_addr(adapter);
-	if (cfg_addr < 0) {
-		ret = cfg_addr;
-		dev_warn(adapter, "Finding address for firmware config file in flash failed, error %d\n",
-			 -ret);
-		goto bye;
-	}
-
-	strcpy(config_name, "On Flash");
-	mtype = FW_MEMTYPE_CF_FLASH;
-	maddr = cfg_addr;
-
-	/* Enable HASH filter region when support is available. */
-	val = 1;
-	param = CXGBE_FW_PARAM_DEV(HASHFILTER_WITH_OFLD);
-	t4_set_params(adapter, adapter->mbox, adapter->pf, 0, 1,
-		      &param, &val);
-
-	/*
-	 * Issue a Capability Configuration command to the firmware to get it
-	 * to parse the Configuration File.  We don't use t4_fw_config_file()
-	 * because we want the ability to modify various features after we've
-	 * processed the configuration file ...
-	 */
-	memset(&caps_cmd, 0, sizeof(caps_cmd));
-	caps_cmd.op_to_write = cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
-					   F_FW_CMD_REQUEST | F_FW_CMD_READ);
-	caps_cmd.cfvalid_to_len16 =
-		cpu_to_be32(F_FW_CAPS_CONFIG_CMD_CFVALID |
-			    V_FW_CAPS_CONFIG_CMD_MEMTYPE_CF(mtype) |
-			    V_FW_CAPS_CONFIG_CMD_MEMADDR64K_CF(maddr >> 16) |
-			    FW_LEN16(caps_cmd));
-	ret = t4_wr_mbox(adapter, adapter->mbox, &caps_cmd, sizeof(caps_cmd),
-			 &caps_cmd);
-	/*
-	 * If the CAPS_CONFIG failed with an ENOENT (for a Firmware
-	 * Configuration File in FLASH), our last gasp effort is to use the
-	 * Firmware Configuration File which is embedded in the firmware.  A
-	 * very few early versions of the firmware didn't have one embedded
-	 * but we can ignore those.
-	 */
-	if (ret == -ENOENT) {
-		dev_info(adapter, "%s: Going for embedded config in firmware..\n",
-			 __func__);
-
-		memset(&caps_cmd, 0, sizeof(caps_cmd));
-		caps_cmd.op_to_write =
-			cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
-				    F_FW_CMD_REQUEST | F_FW_CMD_READ);
-		caps_cmd.cfvalid_to_len16 = cpu_to_be32(FW_LEN16(caps_cmd));
-		ret = t4_wr_mbox(adapter, adapter->mbox, &caps_cmd,
-				 sizeof(caps_cmd), &caps_cmd);
-		strcpy(config_name, "Firmware Default");
-	}
-
-	config_issued = 1;
+	ret = cxgbe_load_fw_config(adapter);
 	if (ret < 0)
 		goto bye;
 
-	finiver = be32_to_cpu(caps_cmd.finiver);
-	finicsum = be32_to_cpu(caps_cmd.finicsum);
-	cfcsum = be32_to_cpu(caps_cmd.cfcsum);
-	if (finicsum != cfcsum)
-		dev_warn(adapter, "Configuration File checksum mismatch: [fini] csum=%#x, computed csum=%#x\n",
-			 finicsum, cfcsum);
-
-	/*
-	 * If we're a pure NIC driver then disable all offloading facilities.
-	 * This will allow the firmware to optimize aspects of the hardware
-	 * configuration which will result in improved performance.
-	 */
-	caps_cmd.niccaps &= cpu_to_be16(~FW_CAPS_CONFIG_NIC_ETHOFLD);
-	caps_cmd.toecaps = 0;
-	caps_cmd.iscsicaps = 0;
-	caps_cmd.rdmacaps = 0;
-	caps_cmd.fcoecaps = 0;
-	caps_cmd.cryptocaps = 0;
-
-	/*
-	 * And now tell the firmware to use the configuration we just loaded.
-	 */
-	caps_cmd.op_to_write = cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
-					   F_FW_CMD_REQUEST | F_FW_CMD_WRITE);
-	caps_cmd.cfvalid_to_len16 = htonl(FW_LEN16(caps_cmd));
-	ret = t4_wr_mbox(adapter, adapter->mbox, &caps_cmd, sizeof(caps_cmd),
-			 NULL);
-	if (ret < 0) {
-		dev_warn(adapter, "Unable to finalize Firmware Capabilities %d\n",
-			 -ret);
-		goto bye;
-	}
-
 	/*
 	 * Tweak configuration based on system architecture, etc.
 	 */
@@ -1251,27 +1372,9 @@ static int adap_init0_config(struct adapter *adapter, int reset)
 		goto bye;
 	}
 
-	/*
-	 * Return successfully and note that we're operating with parameters
-	 * not supplied by the driver, rather than from hard-wired
-	 * initialization constants buried in the driver.
-	 */
-	dev_info(adapter,
-		 "Successfully configured using Firmware Configuration File \"%s\", version %#x, computed checksum %#x\n",
-		 config_name, finiver, cfcsum);
-
 	return 0;
 
-	/*
-	 * Something bad happened.  Return the error ...  (If the "error"
-	 * is that there's no Configuration File on the adapter we don't
-	 * want to issue a warning since this is fairly common.)
-	 */
 bye:
-	if (config_issued && ret != -ENOENT)
-		dev_warn(adapter, "\"%s\" configuration file error %d\n",
-			 config_name, -ret);
-
 	dev_debug(adapter, "%s: returning ret = %d ..\n", __func__, ret);
 	return ret;
 }
-- 
2.27.0

Re: [PATCH 5/5] net/cxgbe: read firmware configuration file from filesystem

[Ferruh Yigit]

On 4/18/2022 11:24 PM, Rahul Lakkireddy wrote:
> Add support to read firmware configuration file from
> /lib/firmware/cxgb4/ path in the filesystem.
> 

Hi Rahul,

Can you please document the FW config file in the driver documentation?
Please add:
- Path of the config file
- Content of the config file. As far as I can see from the code the 
config file directly sent to the FW, does this mean config file is binary?
- What happens when config file is not found

Also are these values overlap with the devargs that PMD has? If so what 
happens in that case, which one is used, devargs one or config file one?

Previously there was 'cxgbtool' tool to send the config file, is this 
method replacing it? Why not keep using 'cxgbtool'?

Thanks,
ferruh


> Signed-off-by: Rahul Lakkireddy<rahul.lakkireddy@chelsio.com>

<...>

Re: [PATCH 5/5] net/cxgbe: read firmware configuration file from filesystem

[Ferruh Yigit]

On 5/5/2022 5:29 PM, Ferruh Yigit wrote:
> On 4/18/2022 11:24 PM, Rahul Lakkireddy wrote:
>> Add support to read firmware configuration file from
>> /lib/firmware/cxgb4/ path in the filesystem.
>>
> 
> Hi Rahul,
> 
> Can you please document the FW config file in the driver documentation?
> Please add:
> - Path of the config file
> - Content of the config file. As far as I can see from the code the 
> config file directly sent to the FW, does this mean config file is binary?
> - What happens when config file is not found
> 
> Also are these values overlap with the devargs that PMD has? If so what 
> happens in that case, which one is used, devargs one or config file one?
> 
> Previously there was 'cxgbtool' tool to send the config file, is this 
> method replacing it? Why not keep using 'cxgbtool'?
> 

cc'ed more folks.

This patch introduces a userspace config file for runtime FW config.

What do you think about this approach?
Should we formalize this method more, like introducing an ethdev level 
config option to hold the config file, which can be used for driver 
and/or FW. And perhaps with a defined syntax (yaml?).
Can this be an alternative to PMD devargs?

Cheers,
ferruh

Re: [PATCH 5/5] net/cxgbe: read firmware configuration file from filesystem

[Rahul Lakkireddy]

On Thursday, May 05/05/22, 2022 at 17:36:06 +0100, Ferruh Yigit wrote:
> On 5/5/2022 5:29 PM, Ferruh Yigit wrote:
> > On 4/18/2022 11:24 PM, Rahul Lakkireddy wrote:
> > > Add support to read firmware configuration file from
> > > /lib/firmware/cxgb4/ path in the filesystem.
> > > 
> > 
> > Hi Rahul,
> > 
> > Can you please document the FW config file in the driver documentation?
> > Please add:
> > - Path of the config file
> > - Content of the config file. As far as I can see from the code the
> > config file directly sent to the FW, does this mean config file is
> > binary?
> > - What happens when config file is not found
> > 
> > Also are these values overlap with the devargs that PMD has? If so what
> > happens in that case, which one is used, devargs one or config file one?
> > 
> > Previously there was 'cxgbtool' tool to send the config file, is this
> > method replacing it? Why not keep using 'cxgbtool'?
> > 

The Chelsio FW config file contains a list of register=value pairs to
change configuration of the NIC before firmware is initialized.
It closely resembles the INI file format. It is mainly used to aid
in debugging FW initialization failures and to optimally partition
NIC hardware resources for specific requirements. Partitioning
generally involves moving resources on unused Physical Functions
(PFs) to the main PF, like redistributing queues, hardware TCAMs,
etc., before firmware begins initialization. Once the configuration
looks good, then the final FW config file is flashed onto the NIC
using the cxgbtool. The FW config file can then be removed from the
/lib/firmware/cxgb4/ directory and the FW will begin initializing
with the flashed FW config file on the NIC from next time onwards.

With this patch, the FW config file is selected in following order.

1) Check and select FW config file present in /lib/firmware/cxgb4/
   directory.

2) Otherwise, check and select FW config file flashed onto the
   NIC.

3) Otherwise, select the default FW config file embedded within the
   FW binary on the NIC.

Since this is pretty low-level hardware configuration, the users are
not expected to change this file without expert guidance. So,
exporting such a low-level configuration via devargs API does not
feel like the right fit for this specific requirement.

Once FW is up and running with the FW config file, some of the driver
and FW runtime features can be enabled/disabled via devargs API during
driver probe.

To summarize, the FW config file is intended to debug FW initialization
failures and/or aid in resources partitioning before FW starts
initialization. Once the FW is running, the whole or smaller slices
of these partitioned resources can be further redistributed across
the multiple physical ports controlled by the same underlying PF.

> 
> cc'ed more folks.
> 
> This patch introduces a userspace config file for runtime FW config.
> 
> What do you think about this approach?
> Should we formalize this method more, like introducing an ethdev level
> config option to hold the config file, which can be used for driver and/or
> FW. And perhaps with a defined syntax (yaml?).
> Can this be an alternative to PMD devargs?
> 
> Cheers,
> ferruh

Thanks,
Rahul

Re: [PATCH 5/5] net/cxgbe: read firmware configuration file from filesystem

[Thomas Monjalon]

06/05/2022 13:36, Rahul Lakkireddy:
> The Chelsio FW config file contains a list of register=value pairs to
> change configuration of the NIC before firmware is initialized.
> It closely resembles the INI file format. It is mainly used to aid
> in debugging FW initialization failures and to optimally partition
> NIC hardware resources for specific requirements. Partitioning
> generally involves moving resources on unused Physical Functions
> (PFs) to the main PF, like redistributing queues, hardware TCAMs,
> etc., before firmware begins initialization. Once the configuration
> looks good, then the final FW config file is flashed onto the NIC
> using the cxgbtool. The FW config file can then be removed from the
> /lib/firmware/cxgb4/ directory and the FW will begin initializing
> with the flashed FW config file on the NIC from next time onwards.
> 
> With this patch, the FW config file is selected in following order.
> 
> 1) Check and select FW config file present in /lib/firmware/cxgb4/
>    directory.
> 
> 2) Otherwise, check and select FW config file flashed onto the
>    NIC.
> 
> 3) Otherwise, select the default FW config file embedded within the
>    FW binary on the NIC.
> 
> Since this is pretty low-level hardware configuration, the users are
> not expected to change this file without expert guidance. So,
> exporting such a low-level configuration via devargs API does not
> feel like the right fit for this specific requirement.
> 
> Once FW is up and running with the FW config file, some of the driver
> and FW runtime features can be enabled/disabled via devargs API during
> driver probe.
> 
> To summarize, the FW config file is intended to debug FW initialization
> failures and/or aid in resources partitioning before FW starts
> initialization. Once the FW is running, the whole or smaller slices
> of these partitioned resources can be further redistributed across
> the multiple physical ports controlled by the same underlying PF.

Sorry it is not clear to me.
The FW file is used by cxgbtool to flash it, right?
The PMD may use the same FW file for debug diagnostics?
How is it used by the kernel?

Re: [PATCH 5/5] net/cxgbe: read firmware configuration file from filesystem

[Rahul Lakkireddy]

On Tuesday, May 05/10/22, 2022 at 11:02:05 +0200, Thomas Monjalon wrote:
> 06/05/2022 13:36, Rahul Lakkireddy:
> > The Chelsio FW config file contains a list of register=value pairs to
> > change configuration of the NIC before firmware is initialized.
> > It closely resembles the INI file format. It is mainly used to aid
> > in debugging FW initialization failures and to optimally partition
> > NIC hardware resources for specific requirements. Partitioning
> > generally involves moving resources on unused Physical Functions
> > (PFs) to the main PF, like redistributing queues, hardware TCAMs,
> > etc., before firmware begins initialization. Once the configuration
> > looks good, then the final FW config file is flashed onto the NIC
> > using the cxgbtool. The FW config file can then be removed from the
> > /lib/firmware/cxgb4/ directory and the FW will begin initializing
> > with the flashed FW config file on the NIC from next time onwards.
> > 
> > With this patch, the FW config file is selected in following order.
> > 
> > 1) Check and select FW config file present in /lib/firmware/cxgb4/
> >    directory.
> > 
> > 2) Otherwise, check and select FW config file flashed onto the
> >    NIC.
> > 
> > 3) Otherwise, select the default FW config file embedded within the
> >    FW binary on the NIC.
> > 
> > Since this is pretty low-level hardware configuration, the users are
> > not expected to change this file without expert guidance. So,
> > exporting such a low-level configuration via devargs API does not
> > feel like the right fit for this specific requirement.
> > 
> > Once FW is up and running with the FW config file, some of the driver
> > and FW runtime features can be enabled/disabled via devargs API during
> > driver probe.
> > 
> > To summarize, the FW config file is intended to debug FW initialization
> > failures and/or aid in resources partitioning before FW starts
> > initialization. Once the FW is running, the whole or smaller slices
> > of these partitioned resources can be further redistributed across
> > the multiple physical ports controlled by the same underlying PF.
> 
> Sorry it is not clear to me.
> The FW file is used by cxgbtool to flash it, right?
> The PMD may use the same FW file for debug diagnostics?


There are 2 FW related files in /lib/firmware/cxgb4 directory that
can be written into the NIC flash (at different locations in the flash)
using cxgbtool:

1) FW config file (t6-config.txt): This file contains register=value
   pairs to configure some HW registers before initializing FW
   (t6fw.bin). These register=value pairs are very low level and
   specific to HW. The FW config file can be used to enable/disable
   some paths in HW to debug FW initialization issues. Since it
   can disable some paths in HW, it can also be used to partition
   and redistribute resources from other unused PFs to main PF,
   before FW initialization.

2) FW binary (t6fw.bin): This contains the actual FW. This binary also
   has an embedded "default" FW config file that will be used if no
   FW config file (t6-config.txt) is present in NIC "flash" or in the
   "/lib/firmware/cxgb4/" directory.

> How is it used by the kernel?
> 

In kernel cxgb4 driver also, we follow the same sequence when
initializing FW. If any FW config file (t6-config.txt) is present
in "/lib/firmware/cxgb4" directory, then it is picked up first.
Otherwise, a FW config file that has been "flashed" onto the NIC
using cxgbtool, is picked up next. Otherwise, FW will init with
the "default" FW config file embedded inside the FW binary (t6fw.bin).

Currently, the DPDK cxgbe PMD only picks up the FW config file from
NIC "flash" or the "default" one inside the FW binary. This patch adds
a way to pick FW config file from "lib/firmware/cxgb4" directory when
a FW config file is placed there.

Thanks,
Rahul

Re: [PATCH 5/5] net/cxgbe: read firmware configuration file from filesystem

[Thomas Monjalon]

10/05/2022 16:11, Rahul Lakkireddy:
> On Tuesday, May 05/10/22, 2022 at 11:02:05 +0200, Thomas Monjalon wrote:
> > 06/05/2022 13:36, Rahul Lakkireddy:
> > > The Chelsio FW config file contains a list of register=value pairs to
> > > change configuration of the NIC before firmware is initialized.
> > > It closely resembles the INI file format. It is mainly used to aid
> > > in debugging FW initialization failures and to optimally partition
> > > NIC hardware resources for specific requirements. Partitioning
> > > generally involves moving resources on unused Physical Functions
> > > (PFs) to the main PF, like redistributing queues, hardware TCAMs,
> > > etc., before firmware begins initialization. Once the configuration
> > > looks good, then the final FW config file is flashed onto the NIC
> > > using the cxgbtool. The FW config file can then be removed from the
> > > /lib/firmware/cxgb4/ directory and the FW will begin initializing
> > > with the flashed FW config file on the NIC from next time onwards.
> > > 
> > > With this patch, the FW config file is selected in following order.
> > > 
> > > 1) Check and select FW config file present in /lib/firmware/cxgb4/
> > >    directory.
> > > 
> > > 2) Otherwise, check and select FW config file flashed onto the
> > >    NIC.
> > > 
> > > 3) Otherwise, select the default FW config file embedded within the
> > >    FW binary on the NIC.
> > > 
> > > Since this is pretty low-level hardware configuration, the users are
> > > not expected to change this file without expert guidance. So,
> > > exporting such a low-level configuration via devargs API does not
> > > feel like the right fit for this specific requirement.
> > > 
> > > Once FW is up and running with the FW config file, some of the driver
> > > and FW runtime features can be enabled/disabled via devargs API during
> > > driver probe.
> > > 
> > > To summarize, the FW config file is intended to debug FW initialization
> > > failures and/or aid in resources partitioning before FW starts
> > > initialization. Once the FW is running, the whole or smaller slices
> > > of these partitioned resources can be further redistributed across
> > > the multiple physical ports controlled by the same underlying PF.
> > 
> > Sorry it is not clear to me.
> > The FW file is used by cxgbtool to flash it, right?
> > The PMD may use the same FW file for debug diagnostics?
> 
> 
> There are 2 FW related files in /lib/firmware/cxgb4 directory that
> can be written into the NIC flash (at different locations in the flash)
> using cxgbtool:
> 
> 1) FW config file (t6-config.txt): This file contains register=value
>    pairs to configure some HW registers before initializing FW
>    (t6fw.bin). These register=value pairs are very low level and
>    specific to HW. The FW config file can be used to enable/disable
>    some paths in HW to debug FW initialization issues. Since it
>    can disable some paths in HW, it can also be used to partition
>    and redistribute resources from other unused PFs to main PF,
>    before FW initialization.
> 
> 2) FW binary (t6fw.bin): This contains the actual FW. This binary also
>    has an embedded "default" FW config file that will be used if no
>    FW config file (t6-config.txt) is present in NIC "flash" or in the
>    "/lib/firmware/cxgb4/" directory.
> 
> > How is it used by the kernel?
> > 
> 
> In kernel cxgb4 driver also, we follow the same sequence when
> initializing FW. If any FW config file (t6-config.txt) is present
> in "/lib/firmware/cxgb4" directory, then it is picked up first.
> Otherwise, a FW config file that has been "flashed" onto the NIC
> using cxgbtool, is picked up next. Otherwise, FW will init with
> the "default" FW config file embedded inside the FW binary (t6fw.bin).
> 
> Currently, the DPDK cxgbe PMD only picks up the FW config file from
> NIC "flash" or the "default" one inside the FW binary. This patch adds
> a way to pick FW config file from "lib/firmware/cxgb4" directory when
> a FW config file is placed there.

OK, so both Linux and DPDK drivers read the FW config file
to provide better diagnostics. Am I right?

Re: [PATCH 5/5] net/cxgbe: read firmware configuration file from filesystem

[Rahul Lakkireddy]

On Tuesday, May 05/10/22, 2022 at 16:30:59 +0200, Thomas Monjalon wrote:
> 10/05/2022 16:11, Rahul Lakkireddy:
> > On Tuesday, May 05/10/22, 2022 at 11:02:05 +0200, Thomas Monjalon wrote:
> > > 06/05/2022 13:36, Rahul Lakkireddy:
> > > > The Chelsio FW config file contains a list of register=value pairs to
> > > > change configuration of the NIC before firmware is initialized.
> > > > It closely resembles the INI file format. It is mainly used to aid
> > > > in debugging FW initialization failures and to optimally partition
> > > > NIC hardware resources for specific requirements. Partitioning
> > > > generally involves moving resources on unused Physical Functions
> > > > (PFs) to the main PF, like redistributing queues, hardware TCAMs,
> > > > etc., before firmware begins initialization. Once the configuration
> > > > looks good, then the final FW config file is flashed onto the NIC
> > > > using the cxgbtool. The FW config file can then be removed from the
> > > > /lib/firmware/cxgb4/ directory and the FW will begin initializing
> > > > with the flashed FW config file on the NIC from next time onwards.
> > > > 
> > > > With this patch, the FW config file is selected in following order.
> > > > 
> > > > 1) Check and select FW config file present in /lib/firmware/cxgb4/
> > > >    directory.
> > > > 
> > > > 2) Otherwise, check and select FW config file flashed onto the
> > > >    NIC.
> > > > 
> > > > 3) Otherwise, select the default FW config file embedded within the
> > > >    FW binary on the NIC.
> > > > 
> > > > Since this is pretty low-level hardware configuration, the users are
> > > > not expected to change this file without expert guidance. So,
> > > > exporting such a low-level configuration via devargs API does not
> > > > feel like the right fit for this specific requirement.
> > > > 
> > > > Once FW is up and running with the FW config file, some of the driver
> > > > and FW runtime features can be enabled/disabled via devargs API during
> > > > driver probe.
> > > > 
> > > > To summarize, the FW config file is intended to debug FW initialization
> > > > failures and/or aid in resources partitioning before FW starts
> > > > initialization. Once the FW is running, the whole or smaller slices
> > > > of these partitioned resources can be further redistributed across
> > > > the multiple physical ports controlled by the same underlying PF.
> > > 
> > > Sorry it is not clear to me.
> > > The FW file is used by cxgbtool to flash it, right?
> > > The PMD may use the same FW file for debug diagnostics?
> > 
> > 
> > There are 2 FW related files in /lib/firmware/cxgb4 directory that
> > can be written into the NIC flash (at different locations in the flash)
> > using cxgbtool:
> > 
> > 1) FW config file (t6-config.txt): This file contains register=value
> >    pairs to configure some HW registers before initializing FW
> >    (t6fw.bin). These register=value pairs are very low level and
> >    specific to HW. The FW config file can be used to enable/disable
> >    some paths in HW to debug FW initialization issues. Since it
> >    can disable some paths in HW, it can also be used to partition
> >    and redistribute resources from other unused PFs to main PF,
> >    before FW initialization.
> > 
> > 2) FW binary (t6fw.bin): This contains the actual FW. This binary also
> >    has an embedded "default" FW config file that will be used if no
> >    FW config file (t6-config.txt) is present in NIC "flash" or in the
> >    "/lib/firmware/cxgb4/" directory.
> > 
> > > How is it used by the kernel?
> > > 
> > 
> > In kernel cxgb4 driver also, we follow the same sequence when
> > initializing FW. If any FW config file (t6-config.txt) is present
> > in "/lib/firmware/cxgb4" directory, then it is picked up first.
> > Otherwise, a FW config file that has been "flashed" onto the NIC
> > using cxgbtool, is picked up next. Otherwise, FW will init with
> > the "default" FW config file embedded inside the FW binary (t6fw.bin).
> > 
> > Currently, the DPDK cxgbe PMD only picks up the FW config file from
> > NIC "flash" or the "default" one inside the FW binary. This patch adds
> > a way to pick FW config file from "lib/firmware/cxgb4" directory when
> > a FW config file is placed there.
> 
> OK, so both Linux and DPDK drivers read the FW config file
> to provide better diagnostics. Am I right?
> 

Yes, both use the FW config file to enable/disable specific paths in
HW to get better diagnostics for FW init failures.

Thanks,
Rahul

Re: [PATCH 5/5] net/cxgbe: read firmware configuration file from filesystem

[Thomas Monjalon]

10/05/2022 17:05, Rahul Lakkireddy:
> On Tuesday, May 05/10/22, 2022 at 16:30:59 +0200, Thomas Monjalon wrote:
> > 10/05/2022 16:11, Rahul Lakkireddy:
> > > On Tuesday, May 05/10/22, 2022 at 11:02:05 +0200, Thomas Monjalon wrote:
> > > > 06/05/2022 13:36, Rahul Lakkireddy:
> > > > > The Chelsio FW config file contains a list of register=value pairs to
> > > > > change configuration of the NIC before firmware is initialized.
> > > > > It closely resembles the INI file format. It is mainly used to aid
> > > > > in debugging FW initialization failures and to optimally partition
> > > > > NIC hardware resources for specific requirements. Partitioning
> > > > > generally involves moving resources on unused Physical Functions
> > > > > (PFs) to the main PF, like redistributing queues, hardware TCAMs,
> > > > > etc., before firmware begins initialization. Once the configuration
> > > > > looks good, then the final FW config file is flashed onto the NIC
> > > > > using the cxgbtool. The FW config file can then be removed from the
> > > > > /lib/firmware/cxgb4/ directory and the FW will begin initializing
> > > > > with the flashed FW config file on the NIC from next time onwards.
> > > > > 
> > > > > With this patch, the FW config file is selected in following order.
> > > > > 
> > > > > 1) Check and select FW config file present in /lib/firmware/cxgb4/
> > > > >    directory.
> > > > > 
> > > > > 2) Otherwise, check and select FW config file flashed onto the
> > > > >    NIC.
> > > > > 
> > > > > 3) Otherwise, select the default FW config file embedded within the
> > > > >    FW binary on the NIC.
> > > > > 
> > > > > Since this is pretty low-level hardware configuration, the users are
> > > > > not expected to change this file without expert guidance. So,
> > > > > exporting such a low-level configuration via devargs API does not
> > > > > feel like the right fit for this specific requirement.
> > > > > 
> > > > > Once FW is up and running with the FW config file, some of the driver
> > > > > and FW runtime features can be enabled/disabled via devargs API during
> > > > > driver probe.
> > > > > 
> > > > > To summarize, the FW config file is intended to debug FW initialization
> > > > > failures and/or aid in resources partitioning before FW starts
> > > > > initialization. Once the FW is running, the whole or smaller slices
> > > > > of these partitioned resources can be further redistributed across
> > > > > the multiple physical ports controlled by the same underlying PF.
> > > > 
> > > > Sorry it is not clear to me.
> > > > The FW file is used by cxgbtool to flash it, right?
> > > > The PMD may use the same FW file for debug diagnostics?
> > > 
> > > 
> > > There are 2 FW related files in /lib/firmware/cxgb4 directory that
> > > can be written into the NIC flash (at different locations in the flash)
> > > using cxgbtool:
> > > 
> > > 1) FW config file (t6-config.txt): This file contains register=value
> > >    pairs to configure some HW registers before initializing FW
> > >    (t6fw.bin). These register=value pairs are very low level and
> > >    specific to HW. The FW config file can be used to enable/disable
> > >    some paths in HW to debug FW initialization issues. Since it
> > >    can disable some paths in HW, it can also be used to partition
> > >    and redistribute resources from other unused PFs to main PF,
> > >    before FW initialization.
> > > 
> > > 2) FW binary (t6fw.bin): This contains the actual FW. This binary also
> > >    has an embedded "default" FW config file that will be used if no
> > >    FW config file (t6-config.txt) is present in NIC "flash" or in the
> > >    "/lib/firmware/cxgb4/" directory.
> > > 
> > > > How is it used by the kernel?
> > > > 
> > > 
> > > In kernel cxgb4 driver also, we follow the same sequence when
> > > initializing FW. If any FW config file (t6-config.txt) is present
> > > in "/lib/firmware/cxgb4" directory, then it is picked up first.
> > > Otherwise, a FW config file that has been "flashed" onto the NIC
> > > using cxgbtool, is picked up next. Otherwise, FW will init with
> > > the "default" FW config file embedded inside the FW binary (t6fw.bin).
> > > 
> > > Currently, the DPDK cxgbe PMD only picks up the FW config file from
> > > NIC "flash" or the "default" one inside the FW binary. This patch adds
> > > a way to pick FW config file from "lib/firmware/cxgb4" directory when
> > > a FW config file is placed there.
> > 
> > OK, so both Linux and DPDK drivers read the FW config file
> > to provide better diagnostics. Am I right?
> > 
> 
> Yes, both use the FW config file to enable/disable specific paths in
> HW to get better diagnostics for FW init failures.

OK thanks for the explanation.
The idea looks OK.

[PATCH v2] net/cxgbe: read firmware configuration file from filesystem

[Rahul Lakkireddy]

Add support to read firmware configuration file from
/lib/firmware/cxgb4/ path in the filesystem. The firmware
config file is used to enable or disable NIC features before
firmware initialization to help retrieve better debug data to
analyze firmware init failures. The config file can also
be used to redistribute resources, like queues, TCAMs, etc.,
from disabled physical functions (PFs) to main PF, before
firmware init.

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
v2:
- Update cxgbe pmd doc about the firmware config file
- Update commit message to explain more about firmware config file

 doc/guides/nics/cxgbe.rst               |  28 ++
 drivers/net/cxgbe/base/t4fw_interface.h |   1 +
 drivers/net/cxgbe/cxgbe_main.c          | 329 ++++++++++++++++--------
 3 files changed, 245 insertions(+), 113 deletions(-)

diff --git a/doc/guides/nics/cxgbe.rst b/doc/guides/nics/cxgbe.rst
index a1d30c488b..fc8a5751f1 100644
--- a/doc/guides/nics/cxgbe.rst
+++ b/doc/guides/nics/cxgbe.rst
@@ -838,3 +838,31 @@ to configure the mtu of all the ports with a single command.
 
      testpmd> port stop all
      testpmd> port config all max-pkt-len 9000
+
+Hardware Configuration and Debugging
+------------------------------------
+
+Firmware Configuration File
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+To enable or disable Chelsio NIC features before firmware initialization,
+the Chelsio firmware configuration file can be placed in following
+directory. The CXGBE PMD will search and pick up the firmware
+configuration file during the Chelsio NIC probe, before initializing
+the firmware.
+
+.. code-block:: console
+
+   cp <path_to_config_file>/t6-config.txt /lib/firmware/cxgb4/t6-config.txt
+
+The firmware configuration file is mainly intended to be used to debug
+firmware initialization failures. It can also be used to redistribute
+NIC resources from disabled physical functions (PFs) to main PF before
+initializing firmware.
+
+.. warning::
+
+   Note that the Chelsio firmware configuration file contains very low
+   level details that is specific to the Chelsio NIC. Hence, the
+   firmware configuration file must not be modified without expert
+   guidance from Chelsio support team.
diff --git a/drivers/net/cxgbe/base/t4fw_interface.h b/drivers/net/cxgbe/base/t4fw_interface.h
index a0a9292c0c..76f58d7c77 100644
--- a/drivers/net/cxgbe/base/t4fw_interface.h
+++ b/drivers/net/cxgbe/base/t4fw_interface.h
@@ -697,6 +697,7 @@ enum fw_params_param_dev {
 						 */
 	FW_PARAMS_PARAM_DEV_FWREV	= 0x0B, /* fw version */
 	FW_PARAMS_PARAM_DEV_TPREV	= 0x0C, /* tp version */
+	FW_PARAMS_PARAM_DEV_CF		= 0x0D,
 	FW_PARAMS_PARAM_DEV_ULPTX_MEMWRITE_DSGL = 0x17,
 	FW_PARAMS_PARAM_DEV_FILTER2_WR	= 0x1D,
 	FW_PARAMS_PARAM_DEV_OPAQUE_VIID_SMT_EXTN = 0x27,
diff --git a/drivers/net/cxgbe/cxgbe_main.c b/drivers/net/cxgbe/cxgbe_main.c
index e2a2ccb781..7b162af3e7 100644
--- a/drivers/net/cxgbe/cxgbe_main.c
+++ b/drivers/net/cxgbe/cxgbe_main.c
@@ -4,6 +4,7 @@
  */
 
 #include <sys/queue.h>
+#include <sys/stat.h>
 #include <stdio.h>
 #include <errno.h>
 #include <stdint.h>
@@ -11,6 +12,7 @@
 #include <unistd.h>
 #include <stdarg.h>
 #include <inttypes.h>
+#include <fcntl.h>
 #include <netinet/in.h>
 
 #include <rte_byteorder.h>
@@ -1006,6 +1008,218 @@ static int configure_filter_mode_mask(struct adapter *adap)
 			     params, val);
 }
 
+#define CXGBE_FW_CONFIG_PATH_T5 "/lib/firmware/cxgb4/t5-config.txt"
+#define CXGBE_FW_CONFIG_PATH_T6 "/lib/firmware/cxgb4/t6-config.txt"
+
+/*
+ * Load firmware configuration from file in /lib/firmware/cxgb4/ path,
+ * if it is present.
+ */
+static int cxgbe_load_fw_config_from_filesystem(struct adapter *adap,
+						const char **config_name,
+						u32 *mem_type, u32 *mem_addr)
+{
+	u32 param, val, mtype, maddr;
+	const char *fw_cfg_path;
+	char *fw_cfg = NULL;
+	struct stat st;
+	int ret, fd;
+
+	switch (CHELSIO_CHIP_VERSION(adap->params.chip)) {
+	case CHELSIO_T5:
+		fw_cfg_path = CXGBE_FW_CONFIG_PATH_T5;
+		break;
+	case CHELSIO_T6:
+		fw_cfg_path = CXGBE_FW_CONFIG_PATH_T6;
+		break;
+	default:
+		return -ENOENT;
+	}
+
+	ret = open(fw_cfg_path, O_RDONLY);
+	if (ret < 0) {
+		dev_debug(adap, "Couldn't open FW config file\n");
+		return ret;
+	}
+
+	fd = ret;
+
+	ret = fstat(fd, &st);
+	if (ret < 0) {
+		dev_debug(adap, "Couldn't get FW config file size\n");
+		goto out_err;
+	}
+
+	if (st.st_size >= FLASH_CFG_MAX_SIZE) {
+		dev_debug(adap, "FW config file size >= max(%u)\n",
+			  FLASH_CFG_MAX_SIZE);
+		ret = -ENOMEM;
+		goto out_err;
+	}
+
+	fw_cfg = rte_zmalloc(NULL, st.st_size, 0);
+	if (fw_cfg == NULL) {
+		ret = -ENOMEM;
+		goto out_err;
+	}
+
+	if (read(fd, fw_cfg, st.st_size) != st.st_size) {
+		dev_debug(adap, "Couldn't read FW config file data\n");
+		ret = -EIO;
+		goto out_err;
+	}
+
+	close(fd);
+
+	/* Send it to FW to verify and update to new configuration */
+	param = V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_DEV) |
+		V_FW_PARAMS_PARAM_X(FW_PARAMS_PARAM_DEV_CF);
+	ret = t4_query_params(adap, adap->mbox, adap->pf, 0, 1, &param, &val);
+	if (ret < 0) {
+		dev_debug(adap, "FW config param query failed: %d\n", ret);
+		goto out_free;
+	}
+
+	mtype = val >> 8;
+	maddr = (val & 0xff) << 16;
+
+	t4_os_lock(&adap->win0_lock);
+	ret = t4_memory_rw(adap, MEMWIN_NIC, mtype, maddr, st.st_size,
+			   fw_cfg, T4_MEMORY_WRITE);
+	t4_os_unlock(&adap->win0_lock);
+	if (ret < 0) {
+		dev_debug(adap, "FW config file update failed: %d\n", ret);
+		goto out_free;
+	}
+
+	rte_free(fw_cfg);
+
+	*mem_type = mtype;
+	*mem_addr = maddr;
+	*config_name = fw_cfg_path;
+	return 0;
+
+out_err:
+	close(fd);
+out_free:
+	rte_free(fw_cfg);
+	return ret;
+}
+
+static int cxgbe_load_fw_config(struct adapter *adap)
+{
+	u32 finiver, finicsum, cfcsum, mtype, maddr, param, val;
+	struct fw_caps_config_cmd caps_cmd = { 0 };
+	const char *config_name = NULL;
+	int ret;
+
+	ret = cxgbe_load_fw_config_from_filesystem(adap, &config_name,
+						   &mtype, &maddr);
+	if (ret < 0) {
+		config_name = "On Flash";
+
+		ret = t4_flash_cfg_addr(adap);
+		if (ret < 0) {
+			dev_warn(adap,
+				 "Finding address for FW config file in flash failed: %d\n",
+				 ret);
+			goto out_default_config;
+		}
+
+		mtype = FW_MEMTYPE_CF_FLASH;
+		maddr = ret;
+	}
+
+	/* Enable HASH filter region when support is available. */
+	val = 1;
+	param = CXGBE_FW_PARAM_DEV(HASHFILTER_WITH_OFLD);
+	t4_set_params(adap, adap->mbox, adap->pf, 0, 1, &param, &val);
+
+	/*
+	 * Issue a Capability Configuration command to the firmware to get it
+	 * to parse the Configuration File.
+	 */
+	caps_cmd.op_to_write = cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
+					   F_FW_CMD_REQUEST | F_FW_CMD_READ);
+	caps_cmd.cfvalid_to_len16 =
+		cpu_to_be32(F_FW_CAPS_CONFIG_CMD_CFVALID |
+			    V_FW_CAPS_CONFIG_CMD_MEMTYPE_CF(mtype) |
+			    V_FW_CAPS_CONFIG_CMD_MEMADDR64K_CF(maddr >> 16) |
+			    FW_LEN16(caps_cmd));
+	ret = t4_wr_mbox(adap, adap->mbox, &caps_cmd, sizeof(caps_cmd),
+			 &caps_cmd);
+
+out_default_config:
+	/*
+	 * If the CAPS_CONFIG failed with an ENOENT (for a Firmware
+	 * Configuration File in filesystem or FLASH), our last gasp
+	 * effort is to use the Firmware Configuration File which is
+	 * embedded in the firmware.
+	 */
+	if (ret == -ENOENT) {
+		config_name = "Firmware Default";
+
+		memset(&caps_cmd, 0, sizeof(caps_cmd));
+		caps_cmd.op_to_write =
+			cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
+				    F_FW_CMD_REQUEST | F_FW_CMD_READ);
+		caps_cmd.cfvalid_to_len16 = cpu_to_be32(FW_LEN16(caps_cmd));
+		ret = t4_wr_mbox(adap, adap->mbox, &caps_cmd, sizeof(caps_cmd),
+				 &caps_cmd);
+	}
+
+	if (ret < 0) {
+		dev_info(adap,
+			 "Failed to configure using %s Firmware Configuration file: %d\n",
+			 config_name, ret);
+		return ret;
+	}
+
+	finiver = be32_to_cpu(caps_cmd.finiver);
+	finicsum = be32_to_cpu(caps_cmd.finicsum);
+	cfcsum = be32_to_cpu(caps_cmd.cfcsum);
+	if (finicsum != cfcsum)
+		dev_warn(adap,
+			 "Configuration File checksum mismatch: [fini] csum=0x%x, computed csum=0x%x\n",
+			 finicsum, cfcsum);
+
+	/*
+	 * If we're a pure NIC driver then disable all offloading facilities.
+	 * This will allow the firmware to optimize aspects of the hardware
+	 * configuration which will result in improved performance.
+	 */
+	caps_cmd.niccaps &= cpu_to_be16(~FW_CAPS_CONFIG_NIC_ETHOFLD);
+	caps_cmd.toecaps = 0;
+	caps_cmd.iscsicaps = 0;
+	caps_cmd.rdmacaps = 0;
+	caps_cmd.fcoecaps = 0;
+	caps_cmd.cryptocaps = 0;
+
+	/*
+	 * And now tell the firmware to use the configuration we just loaded.
+	 */
+	caps_cmd.op_to_write = cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
+					   F_FW_CMD_REQUEST | F_FW_CMD_WRITE);
+	caps_cmd.cfvalid_to_len16 = htonl(FW_LEN16(caps_cmd));
+	ret = t4_wr_mbox(adap, adap->mbox, &caps_cmd, sizeof(caps_cmd),
+			 NULL);
+	if (ret < 0) {
+		dev_warn(adap, "Unable to finalize Firmware Capabilities %d\n",
+			 ret);
+		return ret;
+	}
+
+	/*
+	 * Return successfully and note that we're operating with parameters
+	 * not supplied by the driver, rather than from hard-wired
+	 * initialization constants buried in the driver.
+	 */
+	dev_info(adap,
+		 "Successfully configured using Firmware Configuration File \"%s\", version: 0x%x, computed csum: 0x%x\n",
+		 config_name, finiver, cfcsum);
+	return 0;
+}
+
 static void configure_pcie_ext_tag(struct adapter *adapter)
 {
 	u16 v;
@@ -1119,12 +1333,7 @@ static int adap_init0_tweaks(struct adapter *adapter)
  */
 static int adap_init0_config(struct adapter *adapter, int reset)
 {
-	u32 finiver, finicsum, cfcsum, param, val;
-	struct fw_caps_config_cmd caps_cmd;
-	unsigned long mtype = 0, maddr = 0;
-	u8 config_issued = 0;
-	char config_name[20];
-	int cfg_addr, ret;
+	int ret;
 
 	/*
 	 * Reset device if necessary.
@@ -1139,98 +1348,10 @@ static int adap_init0_config(struct adapter *adapter, int reset)
 		}
 	}
 
-	cfg_addr = t4_flash_cfg_addr(adapter);
-	if (cfg_addr < 0) {
-		ret = cfg_addr;
-		dev_warn(adapter, "Finding address for firmware config file in flash failed, error %d\n",
-			 -ret);
-		goto bye;
-	}
-
-	strcpy(config_name, "On Flash");
-	mtype = FW_MEMTYPE_CF_FLASH;
-	maddr = cfg_addr;
-
-	/* Enable HASH filter region when support is available. */
-	val = 1;
-	param = CXGBE_FW_PARAM_DEV(HASHFILTER_WITH_OFLD);
-	t4_set_params(adapter, adapter->mbox, adapter->pf, 0, 1,
-		      &param, &val);
-
-	/*
-	 * Issue a Capability Configuration command to the firmware to get it
-	 * to parse the Configuration File.  We don't use t4_fw_config_file()
-	 * because we want the ability to modify various features after we've
-	 * processed the configuration file ...
-	 */
-	memset(&caps_cmd, 0, sizeof(caps_cmd));
-	caps_cmd.op_to_write = cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
-					   F_FW_CMD_REQUEST | F_FW_CMD_READ);
-	caps_cmd.cfvalid_to_len16 =
-		cpu_to_be32(F_FW_CAPS_CONFIG_CMD_CFVALID |
-			    V_FW_CAPS_CONFIG_CMD_MEMTYPE_CF(mtype) |
-			    V_FW_CAPS_CONFIG_CMD_MEMADDR64K_CF(maddr >> 16) |
-			    FW_LEN16(caps_cmd));
-	ret = t4_wr_mbox(adapter, adapter->mbox, &caps_cmd, sizeof(caps_cmd),
-			 &caps_cmd);
-	/*
-	 * If the CAPS_CONFIG failed with an ENOENT (for a Firmware
-	 * Configuration File in FLASH), our last gasp effort is to use the
-	 * Firmware Configuration File which is embedded in the firmware.  A
-	 * very few early versions of the firmware didn't have one embedded
-	 * but we can ignore those.
-	 */
-	if (ret == -ENOENT) {
-		dev_info(adapter, "%s: Going for embedded config in firmware..\n",
-			 __func__);
-
-		memset(&caps_cmd, 0, sizeof(caps_cmd));
-		caps_cmd.op_to_write =
-			cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
-				    F_FW_CMD_REQUEST | F_FW_CMD_READ);
-		caps_cmd.cfvalid_to_len16 = cpu_to_be32(FW_LEN16(caps_cmd));
-		ret = t4_wr_mbox(adapter, adapter->mbox, &caps_cmd,
-				 sizeof(caps_cmd), &caps_cmd);
-		strcpy(config_name, "Firmware Default");
-	}
-
-	config_issued = 1;
+	ret = cxgbe_load_fw_config(adapter);
 	if (ret < 0)
 		goto bye;
 
-	finiver = be32_to_cpu(caps_cmd.finiver);
-	finicsum = be32_to_cpu(caps_cmd.finicsum);
-	cfcsum = be32_to_cpu(caps_cmd.cfcsum);
-	if (finicsum != cfcsum)
-		dev_warn(adapter, "Configuration File checksum mismatch: [fini] csum=%#x, computed csum=%#x\n",
-			 finicsum, cfcsum);
-
-	/*
-	 * If we're a pure NIC driver then disable all offloading facilities.
-	 * This will allow the firmware to optimize aspects of the hardware
-	 * configuration which will result in improved performance.
-	 */
-	caps_cmd.niccaps &= cpu_to_be16(~FW_CAPS_CONFIG_NIC_ETHOFLD);
-	caps_cmd.toecaps = 0;
-	caps_cmd.iscsicaps = 0;
-	caps_cmd.rdmacaps = 0;
-	caps_cmd.fcoecaps = 0;
-	caps_cmd.cryptocaps = 0;
-
-	/*
-	 * And now tell the firmware to use the configuration we just loaded.
-	 */
-	caps_cmd.op_to_write = cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
-					   F_FW_CMD_REQUEST | F_FW_CMD_WRITE);
-	caps_cmd.cfvalid_to_len16 = htonl(FW_LEN16(caps_cmd));
-	ret = t4_wr_mbox(adapter, adapter->mbox, &caps_cmd, sizeof(caps_cmd),
-			 NULL);
-	if (ret < 0) {
-		dev_warn(adapter, "Unable to finalize Firmware Capabilities %d\n",
-			 -ret);
-		goto bye;
-	}
-
 	/*
 	 * Tweak configuration based on system architecture, etc.
 	 */
@@ -1251,27 +1372,9 @@ static int adap_init0_config(struct adapter *adapter, int reset)
 		goto bye;
 	}
 
-	/*
-	 * Return successfully and note that we're operating with parameters
-	 * not supplied by the driver, rather than from hard-wired
-	 * initialization constants buried in the driver.
-	 */
-	dev_info(adapter,
-		 "Successfully configured using Firmware Configuration File \"%s\", version %#x, computed checksum %#x\n",
-		 config_name, finiver, cfcsum);
-
 	return 0;
 
-	/*
-	 * Something bad happened.  Return the error ...  (If the "error"
-	 * is that there's no Configuration File on the adapter we don't
-	 * want to issue a warning since this is fairly common.)
-	 */
 bye:
-	if (config_issued && ret != -ENOENT)
-		dev_warn(adapter, "\"%s\" configuration file error %d\n",
-			 config_name, -ret);
-
 	dev_debug(adapter, "%s: returning ret = %d ..\n", __func__, ret);
 	return ret;
 }
-- 
2.27.0

Re: [PATCH v2] net/cxgbe: read firmware configuration file from filesystem

[Ferruh Yigit]

On 5/16/2022 11:27 AM, Rahul Lakkireddy wrote:
> Add support to read firmware configuration file from
> /lib/firmware/cxgb4/ path in the filesystem. The firmware
> config file is used to enable or disable NIC features before
> firmware initialization to help retrieve better debug data to
> analyze firmware init failures. The config file can also
> be used to redistribute resources, like queues, TCAMs, etc.,
> from disabled physical functions (PFs) to main PF, before
> firmware init.

Hi Rahul,

Please find comments below.

Also can you please send a new version for both 4/5 and 5/5 (this patch) 
from original series?

> 
> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> ---
> v2:
> - Update cxgbe pmd doc about the firmware config file
> - Update commit message to explain more about firmware config file
> 
>   doc/guides/nics/cxgbe.rst               |  28 ++
>   drivers/net/cxgbe/base/t4fw_interface.h |   1 +
>   drivers/net/cxgbe/cxgbe_main.c          | 329 ++++++++++++++++--------
>   3 files changed, 245 insertions(+), 113 deletions(-)
> 
> diff --git a/doc/guides/nics/cxgbe.rst b/doc/guides/nics/cxgbe.rst
> index a1d30c488b..fc8a5751f1 100644
> --- a/doc/guides/nics/cxgbe.rst
> +++ b/doc/guides/nics/cxgbe.rst
> @@ -838,3 +838,31 @@ to configure the mtu of all the ports with a single command.
>   
>        testpmd> port stop all
>        testpmd> port config all max-pkt-len 9000
> +
> +Hardware Configuration and Debugging
> +------------------------------------
> +
> +Firmware Configuration File
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +To enable or disable Chelsio NIC features before firmware initialization,
> +the Chelsio firmware configuration file can be placed in following
> +directory. The CXGBE PMD will search and pick up the firmware
> +configuration file during the Chelsio NIC probe, before initializing
> +the firmware.
> +

Does it worth to mention what happens if the FW config file doesn't 
exist? Or mention from FW config file load order, as you described in 
mail list, to understand the relation with 'cxgbtool'?

> +.. code-block:: console
> +
> +   cp <path_to_config_file>/t6-config.txt /lib/firmware/cxgb4/t6-config.txt
> +

There is also 't5-config.txt' in the code.

> +The firmware configuration file is mainly intended to be used to debug
> +firmware initialization failures. It can also be used to redistribute
> +NIC resources from disabled physical functions (PFs) to main PF before
> +initializing firmware.
> +
> +.. warning::
> +
> +   Note that the Chelsio firmware configuration file contains very low
> +   level details that is specific to the Chelsio NIC. Hence, the
> +   firmware configuration file must not be modified without expert
> +   guidance from Chelsio support team.

Will it be too much detail to document what config can be changed via 
this FW config?

<...>

Re: [PATCH v2] net/cxgbe: read firmware configuration file from filesystem

[Rahul Lakkireddy]

On Monday, May 05/16/22, 2022 at 12:06:01 +0100, Ferruh Yigit wrote:
> On 5/16/2022 11:27 AM, Rahul Lakkireddy wrote:
> > Add support to read firmware configuration file from
> > /lib/firmware/cxgb4/ path in the filesystem. The firmware
> > config file is used to enable or disable NIC features before
> > firmware initialization to help retrieve better debug data to
> > analyze firmware init failures. The config file can also
> > be used to redistribute resources, like queues, TCAMs, etc.,
> > from disabled physical functions (PFs) to main PF, before
> > firmware init.
> 
> Hi Rahul,
> 
> Please find comments below.
> 
> Also can you please send a new version for both 4/5 and 5/5 (this patch)
> from original series?
> 

I had already posted v2 for 4/5 from original series at below location.

https://patches.dpdk.org/project/dpdk/patch/61ae717665c2f4e38712652bec2f9d0fe5ca7d32.1651842841.git.rahul.lakkireddy@chelsio.com/

Do you want me to send a new patch series with 4/5 and 5/5 in a
single patch series?

> > 
> > Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> > ---
> > v2:
> > - Update cxgbe pmd doc about the firmware config file
> > - Update commit message to explain more about firmware config file
> > 
> >   doc/guides/nics/cxgbe.rst               |  28 ++
> >   drivers/net/cxgbe/base/t4fw_interface.h |   1 +
> >   drivers/net/cxgbe/cxgbe_main.c          | 329 ++++++++++++++++--------
> >   3 files changed, 245 insertions(+), 113 deletions(-)
> > 
> > diff --git a/doc/guides/nics/cxgbe.rst b/doc/guides/nics/cxgbe.rst
> > index a1d30c488b..fc8a5751f1 100644
> > --- a/doc/guides/nics/cxgbe.rst
> > +++ b/doc/guides/nics/cxgbe.rst
> > @@ -838,3 +838,31 @@ to configure the mtu of all the ports with a single command.
> >        testpmd> port stop all
> >        testpmd> port config all max-pkt-len 9000
> > +
> > +Hardware Configuration and Debugging
> > +------------------------------------
> > +
> > +Firmware Configuration File
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +To enable or disable Chelsio NIC features before firmware initialization,
> > +the Chelsio firmware configuration file can be placed in following
> > +directory. The CXGBE PMD will search and pick up the firmware
> > +configuration file during the Chelsio NIC probe, before initializing
> > +the firmware.
> > +
> 
> Does it worth to mention what happens if the FW config file doesn't exist?
> Or mention from FW config file load order, as you described in mail list, to
> understand the relation with 'cxgbtool'?
> 

Will do.

> > +.. code-block:: console
> > +
> > +   cp <path_to_config_file>/t6-config.txt /lib/firmware/cxgb4/t6-config.txt
> > +
> 
> There is also 't5-config.txt' in the code.
> 

Yes, t5-config.txt is for Chelsio T5 NIC series and t6-config.txt
is for Chelsio T6 NIC series. Will update t5-config.txt as well.

> > +The firmware configuration file is mainly intended to be used to debug
> > +firmware initialization failures. It can also be used to redistribute
> > +NIC resources from disabled physical functions (PFs) to main PF before
> > +initializing firmware.
> > +
> > +.. warning::
> > +
> > +   Note that the Chelsio firmware configuration file contains very low
> > +   level details that is specific to the Chelsio NIC. Hence, the
> > +   firmware configuration file must not be modified without expert
> > +   guidance from Chelsio support team.
> 
> Will it be too much detail to document what config can be changed via this
> FW config?
> 

Yes, this will be difficult to document. The config file just looks like
register=value pairs and are often dependent on each other based on
the selected configuration. A portion of sample config file is given
below. I'm not sure how to document this kind of info.

[global]
    reg[0x100c] = 0x22222222
    reg[0x10a0] = 0x01040810
    reg[0x1044] = 4096
    reg[0x1048] = 65536
    reg[0x104c] = 1536
    reg[0x1050] = 9024
    reg[0x1054] = 9216
    reg[0x1058] = 2048
    reg[0x105c] = 128
    reg[0x1060] = 8192
    reg[0x1064] = 16384
[...]

Thanks,
Rahul

Re: [PATCH v2] net/cxgbe: read firmware configuration file from filesystem

[Ferruh Yigit]

On 5/16/2022 12:56 PM, Rahul Lakkireddy wrote:
> On Monday, May 05/16/22, 2022 at 12:06:01 +0100, Ferruh Yigit wrote:
>> On 5/16/2022 11:27 AM, Rahul Lakkireddy wrote:
>>> Add support to read firmware configuration file from
>>> /lib/firmware/cxgb4/ path in the filesystem. The firmware
>>> config file is used to enable or disable NIC features before
>>> firmware initialization to help retrieve better debug data to
>>> analyze firmware init failures. The config file can also
>>> be used to redistribute resources, like queues, TCAMs, etc.,
>>> from disabled physical functions (PFs) to main PF, before
>>> firmware init.
>>
>> Hi Rahul,
>>
>> Please find comments below.
>>
>> Also can you please send a new version for both 4/5 and 5/5 (this patch)
>> from original series?
>>
> 
> I had already posted v2 for 4/5 from original series at below location.
> 
> https://patches.dpdk.org/project/dpdk/patch/61ae717665c2f4e38712652bec2f9d0fe5ca7d32.1651842841.git.rahul.lakkireddy@chelsio.com/
> 
> Do you want me to send a new patch series with 4/5 and 5/5 in a
> single patch series?
> 

Nope, I missed above patch, it is good as separate patches.

>>>
>>> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
>>> ---
>>> v2:
>>> - Update cxgbe pmd doc about the firmware config file
>>> - Update commit message to explain more about firmware config file
>>>
>>>    doc/guides/nics/cxgbe.rst               |  28 ++
>>>    drivers/net/cxgbe/base/t4fw_interface.h |   1 +
>>>    drivers/net/cxgbe/cxgbe_main.c          | 329 ++++++++++++++++--------
>>>    3 files changed, 245 insertions(+), 113 deletions(-)
>>>
>>> diff --git a/doc/guides/nics/cxgbe.rst b/doc/guides/nics/cxgbe.rst
>>> index a1d30c488b..fc8a5751f1 100644
>>> --- a/doc/guides/nics/cxgbe.rst
>>> +++ b/doc/guides/nics/cxgbe.rst
>>> @@ -838,3 +838,31 @@ to configure the mtu of all the ports with a single command.
>>>         testpmd> port stop all
>>>         testpmd> port config all max-pkt-len 9000
>>> +
>>> +Hardware Configuration and Debugging
>>> +------------------------------------
>>> +
>>> +Firmware Configuration File
>>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> +
>>> +To enable or disable Chelsio NIC features before firmware initialization,
>>> +the Chelsio firmware configuration file can be placed in following
>>> +directory. The CXGBE PMD will search and pick up the firmware
>>> +configuration file during the Chelsio NIC probe, before initializing
>>> +the firmware.
>>> +
>>
>> Does it worth to mention what happens if the FW config file doesn't exist?
>> Or mention from FW config file load order, as you described in mail list, to
>> understand the relation with 'cxgbtool'?
>>
> 
> Will do.
> 
>>> +.. code-block:: console
>>> +
>>> +   cp <path_to_config_file>/t6-config.txt /lib/firmware/cxgb4/t6-config.txt
>>> +
>>
>> There is also 't5-config.txt' in the code.
>>
> 
> Yes, t5-config.txt is for Chelsio T5 NIC series and t6-config.txt
> is for Chelsio T6 NIC series. Will update t5-config.txt as well.
> 
>>> +The firmware configuration file is mainly intended to be used to debug
>>> +firmware initialization failures. It can also be used to redistribute
>>> +NIC resources from disabled physical functions (PFs) to main PF before
>>> +initializing firmware.
>>> +
>>> +.. warning::
>>> +
>>> +   Note that the Chelsio firmware configuration file contains very low
>>> +   level details that is specific to the Chelsio NIC. Hence, the
>>> +   firmware configuration file must not be modified without expert
>>> +   guidance from Chelsio support team.
>>
>> Will it be too much detail to document what config can be changed via this
>> FW config?
>>
> 
> Yes, this will be difficult to document. The config file just looks like
> register=value pairs and are often dependent on each other based on
> the selected configuration. A portion of sample config file is given
> below. I'm not sure how to document this kind of info.
> 
> [global]
>      reg[0x100c] = 0x22222222
>      reg[0x10a0] = 0x01040810
>      reg[0x1044] = 4096
>      reg[0x1048] = 65536
>      reg[0x104c] = 1536
>      reg[0x1050] = 9024
>      reg[0x1054] = 9216
>      reg[0x1058] = 2048
>      reg[0x105c] = 128
>      reg[0x1060] = 8192
>      reg[0x1064] = 16384
> [...]
> 

Above won't be much useful to user anyway (without description of the 
values), if there is already some documentation a link to that can be 
good, otherwise OK to skip this.

[PATCH v3] net/cxgbe: read firmware configuration file from filesystem

[Rahul Lakkireddy]

Add support to read firmware configuration file from
/lib/firmware/cxgb4/ path in the filesystem. The firmware
config file is used to enable or disable NIC features before
firmware initialization to help retrieve better debug data to
analyze firmware init failures. The config file can also
be used to redistribute resources, like queues, TCAMs, etc.,
from disabled physical functions (PFs) to main PF, before
firmware init.

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
v3:
- Update cxgbe pmd doc about copying t5-config.txt to
  /lib/firmware/cxgb4/ directory.
- Update cxgbe pmd doc about firmware config file search and
  selection order.

v2:
- Update cxgbe pmd doc about the firmware config file
- Update commit message to explain more about firmware config file

 doc/guides/nics/cxgbe.rst               |  48 ++++
 drivers/net/cxgbe/base/t4fw_interface.h |   1 +
 drivers/net/cxgbe/cxgbe_main.c          | 329 ++++++++++++++++--------
 3 files changed, 265 insertions(+), 113 deletions(-)

diff --git a/doc/guides/nics/cxgbe.rst b/doc/guides/nics/cxgbe.rst
index a1d30c488b..6d20384b70 100644
--- a/doc/guides/nics/cxgbe.rst
+++ b/doc/guides/nics/cxgbe.rst
@@ -838,3 +838,51 @@ to configure the mtu of all the ports with a single command.
 
      testpmd> port stop all
      testpmd> port config all max-pkt-len 9000
+
+Hardware Configuration and Debugging
+------------------------------------
+
+Firmware Configuration File
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+To enable or disable Chelsio NIC features before firmware initialization,
+the Chelsio firmware configuration file can be placed in following
+directory.
+
+.. code-block:: console
+
+   # For Chelsio T5 NIC series
+   cp <path_to_config_file>/t5-config.txt /lib/firmware/cxgb4/t5-config.txt
+
+   # For Chelsio T6 NIC series
+   cp <path_to_config_file>/t6-config.txt /lib/firmware/cxgb4/t6-config.txt
+
+The firmware configuration file is mainly intended to be used to debug
+firmware initialization failures. It can also be used to redistribute
+NIC resources from disabled physical functions (PFs) to main PF before
+initializing firmware.
+
+The CXGBE PMD will search and pick up the firmware configuration file
+during the Chelsio NIC probe, in following order.
+
+#. If the firmware configuration file is present in /lib/firmware/cxgb4/
+   directory, then this file is downloaded to temporary location in
+   NIC's on-chip RAM. When firmware is initializing, it picks up the
+   file from the temporary on-chip RAM location.
+
+#. Otherwise, if the firmware configuration file has been written
+   onto the NIC persistent flash area using cxgbtool, then this
+   file is picked up from the persistent flash area during
+   firmware initialization.
+
+#. If no firmware configuration file is found at /lib/firmware/cxgb4/
+   directory or on the NIC persistent flash area, then the firmware
+   will initialize with the default configuration file embedded inside
+   the firmware binary.
+
+.. warning::
+
+   Note that the Chelsio firmware configuration file contains very low
+   level details that is specific to the Chelsio NIC. Hence, the
+   firmware configuration file must not be modified without expert
+   guidance from Chelsio support team.
diff --git a/drivers/net/cxgbe/base/t4fw_interface.h b/drivers/net/cxgbe/base/t4fw_interface.h
index a0a9292c0c..76f58d7c77 100644
--- a/drivers/net/cxgbe/base/t4fw_interface.h
+++ b/drivers/net/cxgbe/base/t4fw_interface.h
@@ -697,6 +697,7 @@ enum fw_params_param_dev {
 						 */
 	FW_PARAMS_PARAM_DEV_FWREV	= 0x0B, /* fw version */
 	FW_PARAMS_PARAM_DEV_TPREV	= 0x0C, /* tp version */
+	FW_PARAMS_PARAM_DEV_CF		= 0x0D,
 	FW_PARAMS_PARAM_DEV_ULPTX_MEMWRITE_DSGL = 0x17,
 	FW_PARAMS_PARAM_DEV_FILTER2_WR	= 0x1D,
 	FW_PARAMS_PARAM_DEV_OPAQUE_VIID_SMT_EXTN = 0x27,
diff --git a/drivers/net/cxgbe/cxgbe_main.c b/drivers/net/cxgbe/cxgbe_main.c
index e2a2ccb781..7b162af3e7 100644
--- a/drivers/net/cxgbe/cxgbe_main.c
+++ b/drivers/net/cxgbe/cxgbe_main.c
@@ -4,6 +4,7 @@
  */
 
 #include <sys/queue.h>
+#include <sys/stat.h>
 #include <stdio.h>
 #include <errno.h>
 #include <stdint.h>
@@ -11,6 +12,7 @@
 #include <unistd.h>
 #include <stdarg.h>
 #include <inttypes.h>
+#include <fcntl.h>
 #include <netinet/in.h>
 
 #include <rte_byteorder.h>
@@ -1006,6 +1008,218 @@ static int configure_filter_mode_mask(struct adapter *adap)
 			     params, val);
 }
 
+#define CXGBE_FW_CONFIG_PATH_T5 "/lib/firmware/cxgb4/t5-config.txt"
+#define CXGBE_FW_CONFIG_PATH_T6 "/lib/firmware/cxgb4/t6-config.txt"
+
+/*
+ * Load firmware configuration from file in /lib/firmware/cxgb4/ path,
+ * if it is present.
+ */
+static int cxgbe_load_fw_config_from_filesystem(struct adapter *adap,
+						const char **config_name,
+						u32 *mem_type, u32 *mem_addr)
+{
+	u32 param, val, mtype, maddr;
+	const char *fw_cfg_path;
+	char *fw_cfg = NULL;
+	struct stat st;
+	int ret, fd;
+
+	switch (CHELSIO_CHIP_VERSION(adap->params.chip)) {
+	case CHELSIO_T5:
+		fw_cfg_path = CXGBE_FW_CONFIG_PATH_T5;
+		break;
+	case CHELSIO_T6:
+		fw_cfg_path = CXGBE_FW_CONFIG_PATH_T6;
+		break;
+	default:
+		return -ENOENT;
+	}
+
+	ret = open(fw_cfg_path, O_RDONLY);
+	if (ret < 0) {
+		dev_debug(adap, "Couldn't open FW config file\n");
+		return ret;
+	}
+
+	fd = ret;
+
+	ret = fstat(fd, &st);
+	if (ret < 0) {
+		dev_debug(adap, "Couldn't get FW config file size\n");
+		goto out_err;
+	}
+
+	if (st.st_size >= FLASH_CFG_MAX_SIZE) {
+		dev_debug(adap, "FW config file size >= max(%u)\n",
+			  FLASH_CFG_MAX_SIZE);
+		ret = -ENOMEM;
+		goto out_err;
+	}
+
+	fw_cfg = rte_zmalloc(NULL, st.st_size, 0);
+	if (fw_cfg == NULL) {
+		ret = -ENOMEM;
+		goto out_err;
+	}
+
+	if (read(fd, fw_cfg, st.st_size) != st.st_size) {
+		dev_debug(adap, "Couldn't read FW config file data\n");
+		ret = -EIO;
+		goto out_err;
+	}
+
+	close(fd);
+
+	/* Send it to FW to verify and update to new configuration */
+	param = V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_DEV) |
+		V_FW_PARAMS_PARAM_X(FW_PARAMS_PARAM_DEV_CF);
+	ret = t4_query_params(adap, adap->mbox, adap->pf, 0, 1, &param, &val);
+	if (ret < 0) {
+		dev_debug(adap, "FW config param query failed: %d\n", ret);
+		goto out_free;
+	}
+
+	mtype = val >> 8;
+	maddr = (val & 0xff) << 16;
+
+	t4_os_lock(&adap->win0_lock);
+	ret = t4_memory_rw(adap, MEMWIN_NIC, mtype, maddr, st.st_size,
+			   fw_cfg, T4_MEMORY_WRITE);
+	t4_os_unlock(&adap->win0_lock);
+	if (ret < 0) {
+		dev_debug(adap, "FW config file update failed: %d\n", ret);
+		goto out_free;
+	}
+
+	rte_free(fw_cfg);
+
+	*mem_type = mtype;
+	*mem_addr = maddr;
+	*config_name = fw_cfg_path;
+	return 0;
+
+out_err:
+	close(fd);
+out_free:
+	rte_free(fw_cfg);
+	return ret;
+}
+
+static int cxgbe_load_fw_config(struct adapter *adap)
+{
+	u32 finiver, finicsum, cfcsum, mtype, maddr, param, val;
+	struct fw_caps_config_cmd caps_cmd = { 0 };
+	const char *config_name = NULL;
+	int ret;
+
+	ret = cxgbe_load_fw_config_from_filesystem(adap, &config_name,
+						   &mtype, &maddr);
+	if (ret < 0) {
+		config_name = "On Flash";
+
+		ret = t4_flash_cfg_addr(adap);
+		if (ret < 0) {
+			dev_warn(adap,
+				 "Finding address for FW config file in flash failed: %d\n",
+				 ret);
+			goto out_default_config;
+		}
+
+		mtype = FW_MEMTYPE_CF_FLASH;
+		maddr = ret;
+	}
+
+	/* Enable HASH filter region when support is available. */
+	val = 1;
+	param = CXGBE_FW_PARAM_DEV(HASHFILTER_WITH_OFLD);
+	t4_set_params(adap, adap->mbox, adap->pf, 0, 1, &param, &val);
+
+	/*
+	 * Issue a Capability Configuration command to the firmware to get it
+	 * to parse the Configuration File.
+	 */
+	caps_cmd.op_to_write = cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
+					   F_FW_CMD_REQUEST | F_FW_CMD_READ);
+	caps_cmd.cfvalid_to_len16 =
+		cpu_to_be32(F_FW_CAPS_CONFIG_CMD_CFVALID |
+			    V_FW_CAPS_CONFIG_CMD_MEMTYPE_CF(mtype) |
+			    V_FW_CAPS_CONFIG_CMD_MEMADDR64K_CF(maddr >> 16) |
+			    FW_LEN16(caps_cmd));
+	ret = t4_wr_mbox(adap, adap->mbox, &caps_cmd, sizeof(caps_cmd),
+			 &caps_cmd);
+
+out_default_config:
+	/*
+	 * If the CAPS_CONFIG failed with an ENOENT (for a Firmware
+	 * Configuration File in filesystem or FLASH), our last gasp
+	 * effort is to use the Firmware Configuration File which is
+	 * embedded in the firmware.
+	 */
+	if (ret == -ENOENT) {
+		config_name = "Firmware Default";
+
+		memset(&caps_cmd, 0, sizeof(caps_cmd));
+		caps_cmd.op_to_write =
+			cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
+				    F_FW_CMD_REQUEST | F_FW_CMD_READ);
+		caps_cmd.cfvalid_to_len16 = cpu_to_be32(FW_LEN16(caps_cmd));
+		ret = t4_wr_mbox(adap, adap->mbox, &caps_cmd, sizeof(caps_cmd),
+				 &caps_cmd);
+	}
+
+	if (ret < 0) {
+		dev_info(adap,
+			 "Failed to configure using %s Firmware Configuration file: %d\n",
+			 config_name, ret);
+		return ret;
+	}
+
+	finiver = be32_to_cpu(caps_cmd.finiver);
+	finicsum = be32_to_cpu(caps_cmd.finicsum);
+	cfcsum = be32_to_cpu(caps_cmd.cfcsum);
+	if (finicsum != cfcsum)
+		dev_warn(adap,
+			 "Configuration File checksum mismatch: [fini] csum=0x%x, computed csum=0x%x\n",
+			 finicsum, cfcsum);
+
+	/*
+	 * If we're a pure NIC driver then disable all offloading facilities.
+	 * This will allow the firmware to optimize aspects of the hardware
+	 * configuration which will result in improved performance.
+	 */
+	caps_cmd.niccaps &= cpu_to_be16(~FW_CAPS_CONFIG_NIC_ETHOFLD);
+	caps_cmd.toecaps = 0;
+	caps_cmd.iscsicaps = 0;
+	caps_cmd.rdmacaps = 0;
+	caps_cmd.fcoecaps = 0;
+	caps_cmd.cryptocaps = 0;
+
+	/*
+	 * And now tell the firmware to use the configuration we just loaded.
+	 */
+	caps_cmd.op_to_write = cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
+					   F_FW_CMD_REQUEST | F_FW_CMD_WRITE);
+	caps_cmd.cfvalid_to_len16 = htonl(FW_LEN16(caps_cmd));
+	ret = t4_wr_mbox(adap, adap->mbox, &caps_cmd, sizeof(caps_cmd),
+			 NULL);
+	if (ret < 0) {
+		dev_warn(adap, "Unable to finalize Firmware Capabilities %d\n",
+			 ret);
+		return ret;
+	}
+
+	/*
+	 * Return successfully and note that we're operating with parameters
+	 * not supplied by the driver, rather than from hard-wired
+	 * initialization constants buried in the driver.
+	 */
+	dev_info(adap,
+		 "Successfully configured using Firmware Configuration File \"%s\", version: 0x%x, computed csum: 0x%x\n",
+		 config_name, finiver, cfcsum);
+	return 0;
+}
+
 static void configure_pcie_ext_tag(struct adapter *adapter)
 {
 	u16 v;
@@ -1119,12 +1333,7 @@ static int adap_init0_tweaks(struct adapter *adapter)
  */
 static int adap_init0_config(struct adapter *adapter, int reset)
 {
-	u32 finiver, finicsum, cfcsum, param, val;
-	struct fw_caps_config_cmd caps_cmd;
-	unsigned long mtype = 0, maddr = 0;
-	u8 config_issued = 0;
-	char config_name[20];
-	int cfg_addr, ret;
+	int ret;
 
 	/*
 	 * Reset device if necessary.
@@ -1139,98 +1348,10 @@ static int adap_init0_config(struct adapter *adapter, int reset)
 		}
 	}
 
-	cfg_addr = t4_flash_cfg_addr(adapter);
-	if (cfg_addr < 0) {
-		ret = cfg_addr;
-		dev_warn(adapter, "Finding address for firmware config file in flash failed, error %d\n",
-			 -ret);
-		goto bye;
-	}
-
-	strcpy(config_name, "On Flash");
-	mtype = FW_MEMTYPE_CF_FLASH;
-	maddr = cfg_addr;
-
-	/* Enable HASH filter region when support is available. */
-	val = 1;
-	param = CXGBE_FW_PARAM_DEV(HASHFILTER_WITH_OFLD);
-	t4_set_params(adapter, adapter->mbox, adapter->pf, 0, 1,
-		      &param, &val);
-
-	/*
-	 * Issue a Capability Configuration command to the firmware to get it
-	 * to parse the Configuration File.  We don't use t4_fw_config_file()
-	 * because we want the ability to modify various features after we've
-	 * processed the configuration file ...
-	 */
-	memset(&caps_cmd, 0, sizeof(caps_cmd));
-	caps_cmd.op_to_write = cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
-					   F_FW_CMD_REQUEST | F_FW_CMD_READ);
-	caps_cmd.cfvalid_to_len16 =
-		cpu_to_be32(F_FW_CAPS_CONFIG_CMD_CFVALID |
-			    V_FW_CAPS_CONFIG_CMD_MEMTYPE_CF(mtype) |
-			    V_FW_CAPS_CONFIG_CMD_MEMADDR64K_CF(maddr >> 16) |
-			    FW_LEN16(caps_cmd));
-	ret = t4_wr_mbox(adapter, adapter->mbox, &caps_cmd, sizeof(caps_cmd),
-			 &caps_cmd);
-	/*
-	 * If the CAPS_CONFIG failed with an ENOENT (for a Firmware
-	 * Configuration File in FLASH), our last gasp effort is to use the
-	 * Firmware Configuration File which is embedded in the firmware.  A
-	 * very few early versions of the firmware didn't have one embedded
-	 * but we can ignore those.
-	 */
-	if (ret == -ENOENT) {
-		dev_info(adapter, "%s: Going for embedded config in firmware..\n",
-			 __func__);
-
-		memset(&caps_cmd, 0, sizeof(caps_cmd));
-		caps_cmd.op_to_write =
-			cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
-				    F_FW_CMD_REQUEST | F_FW_CMD_READ);
-		caps_cmd.cfvalid_to_len16 = cpu_to_be32(FW_LEN16(caps_cmd));
-		ret = t4_wr_mbox(adapter, adapter->mbox, &caps_cmd,
-				 sizeof(caps_cmd), &caps_cmd);
-		strcpy(config_name, "Firmware Default");
-	}
-
-	config_issued = 1;
+	ret = cxgbe_load_fw_config(adapter);
 	if (ret < 0)
 		goto bye;
 
-	finiver = be32_to_cpu(caps_cmd.finiver);
-	finicsum = be32_to_cpu(caps_cmd.finicsum);
-	cfcsum = be32_to_cpu(caps_cmd.cfcsum);
-	if (finicsum != cfcsum)
-		dev_warn(adapter, "Configuration File checksum mismatch: [fini] csum=%#x, computed csum=%#x\n",
-			 finicsum, cfcsum);
-
-	/*
-	 * If we're a pure NIC driver then disable all offloading facilities.
-	 * This will allow the firmware to optimize aspects of the hardware
-	 * configuration which will result in improved performance.
-	 */
-	caps_cmd.niccaps &= cpu_to_be16(~FW_CAPS_CONFIG_NIC_ETHOFLD);
-	caps_cmd.toecaps = 0;
-	caps_cmd.iscsicaps = 0;
-	caps_cmd.rdmacaps = 0;
-	caps_cmd.fcoecaps = 0;
-	caps_cmd.cryptocaps = 0;
-
-	/*
-	 * And now tell the firmware to use the configuration we just loaded.
-	 */
-	caps_cmd.op_to_write = cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
-					   F_FW_CMD_REQUEST | F_FW_CMD_WRITE);
-	caps_cmd.cfvalid_to_len16 = htonl(FW_LEN16(caps_cmd));
-	ret = t4_wr_mbox(adapter, adapter->mbox, &caps_cmd, sizeof(caps_cmd),
-			 NULL);
-	if (ret < 0) {
-		dev_warn(adapter, "Unable to finalize Firmware Capabilities %d\n",
-			 -ret);
-		goto bye;
-	}
-
 	/*
 	 * Tweak configuration based on system architecture, etc.
 	 */
@@ -1251,27 +1372,9 @@ static int adap_init0_config(struct adapter *adapter, int reset)
 		goto bye;
 	}
 
-	/*
-	 * Return successfully and note that we're operating with parameters
-	 * not supplied by the driver, rather than from hard-wired
-	 * initialization constants buried in the driver.
-	 */
-	dev_info(adapter,
-		 "Successfully configured using Firmware Configuration File \"%s\", version %#x, computed checksum %#x\n",
-		 config_name, finiver, cfcsum);
-
 	return 0;
 
-	/*
-	 * Something bad happened.  Return the error ...  (If the "error"
-	 * is that there's no Configuration File on the adapter we don't
-	 * want to issue a warning since this is fairly common.)
-	 */
 bye:
-	if (config_issued && ret != -ENOENT)
-		dev_warn(adapter, "\"%s\" configuration file error %d\n",
-			 config_name, -ret);
-
 	dev_debug(adapter, "%s: returning ret = %d ..\n", __func__, ret);
 	return ret;
 }
-- 
2.27.0

Re: [PATCH v3] net/cxgbe: read firmware configuration file from filesystem

[Ferruh Yigit]

On 5/16/2022 8:34 PM, Rahul Lakkireddy wrote:
> Add support to read firmware configuration file from
> /lib/firmware/cxgb4/ path in the filesystem. The firmware
> config file is used to enable or disable NIC features before
> firmware initialization to help retrieve better debug data to
> analyze firmware init failures. The config file can also
> be used to redistribute resources, like queues, TCAMs, etc.,
> from disabled physical functions (PFs) to main PF, before
> firmware init.
> 
> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>

Applied to dpdk-next-net/main, thanks.

Re: [PATCH 0/5] net/cxgbe: updates and bug fixes

[Ferruh Yigit]

On 4/18/2022 11:24 PM, Rahul Lakkireddy wrote:
> This series of patches add the following updates and bug fixes to
> the cxgbe PMD.
> 
> Patch 1 fixes an issue with wrong port id being filled in mbufs
> allocated in Rx path.
> 
> Patch 2 fixes an issue with Txq getting stuck when trying to coalesce
> mbufs with chains.
> 
> Patch 3 reworks and simplifies the code for posting mbufs and their
> payload buffer sizes to hardware in Rx path.
> 
> Patch 4 adds support to track packets in Rx path dropped due to
> congestion at Rx queues that may not get propagated to the rest
> of the Rx pipeline.
> 
> Patch 5 adds support to read firmware configuration file from
> /lib/firmware/cxgb4/ to allow changing firmware parameters without
> having to flash the configuration file onto the adapter.
> 
> Thanks,
> Rahul
> 
> Rahul Lakkireddy (5):
>    net/cxgbe: fill correct port info in mbufs for Rx
>    net/cxgbe: fix Tx queue stuck with mbuf chain coalescing
>    net/cxgbe: simplify Rx payload buffer size posting
>    net/cxgbe: track packets dropped by TP due to congestion
>    net/cxgbe: read firmware configuration file from filesystem
> 

Except from 4/5 & 5/5,
Series applied to dpdk-next-net/main, thanks.

Please check comments for 4/5 & 5/5. Since patches are independent, I 
think it would be OK to send a new series for remaining patches.

Thanks,
ferruh