All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] qed*: enhancements
@ 2021-11-24  9:43 Manish Chopra
  2021-11-24  9:43 ` [PATCH net-next 1/2] qed*: enhance tx timeout debug info Manish Chopra
  2021-11-24  9:43 ` [PATCH net-next 2/2] qed*: esl priv flag support through ethtool Manish Chopra
  0 siblings, 2 replies; 7+ messages in thread
From: Manish Chopra @ 2021-11-24  9:43 UTC (permalink / raw)
  To: kuba; +Cc: netdev, aelior, palok, pkushwaha

Hello Jakub,

This series adds below enhancements for qed/qede drivers

patch 1: Improves tx timeout debug data logs.
patch 2: Add ESL priv flag cap/status support via ethtool.

Please consider applying it to "net-next"

Manish Chopra (2):
  qed*: enhance tx timeout debug info
  qed*: esl priv flag support through ethtool

 drivers/net/ethernet/qlogic/qed/qed_int.c     | 23 +++++
 drivers/net/ethernet/qlogic/qed/qed_int.h     | 13 +++
 drivers/net/ethernet/qlogic/qed/qed_main.c    | 72 +++++++++++++-
 drivers/net/ethernet/qlogic/qed/qed_mcp.c     | 22 +++++
 drivers/net/ethernet/qlogic/qed/qed_mcp.h     | 22 +++++
 drivers/net/ethernet/qlogic/qed/qed_mfw_hsi.h |  1 +
 .../net/ethernet/qlogic/qed/qed_reg_addr.h    |  2 +
 .../net/ethernet/qlogic/qede/qede_ethtool.c   | 13 +++
 drivers/net/ethernet/qlogic/qede/qede_main.c  | 94 ++++++++++++++++---
 include/linux/qed/qed_if.h                    | 14 +++
 10 files changed, 260 insertions(+), 16 deletions(-)

-- 
2.27.0


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

* [PATCH net-next 1/2] qed*: enhance tx timeout debug info
  2021-11-24  9:43 [PATCH net-next 0/2] qed*: enhancements Manish Chopra
@ 2021-11-24  9:43 ` Manish Chopra
  2021-11-25  2:51   ` Jakub Kicinski
  2021-11-24  9:43 ` [PATCH net-next 2/2] qed*: esl priv flag support through ethtool Manish Chopra
  1 sibling, 1 reply; 7+ messages in thread
From: Manish Chopra @ 2021-11-24  9:43 UTC (permalink / raw)
  To: kuba; +Cc: netdev, aelior, palok, pkushwaha

This patch add some new qed APIs to query status block
info and report various data to MFW on tx timeout event

Along with that it enhances qede to dump more debug logs
(not just specific to the queue which was reported by stack)
on tx timeout which includes various other basic metadata about
all tx queues and other info (like status block etc.)

Signed-off-by: Manish Chopra <manishc@marvell.com>
Signed-off-by: Prabhakar Kushwaha <pkushwaha@marvell.com>
Signed-off-by: Alok Prasad <palok@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 drivers/net/ethernet/qlogic/qed/qed_int.c     | 23 +++++
 drivers/net/ethernet/qlogic/qed/qed_int.h     | 13 +++
 drivers/net/ethernet/qlogic/qed/qed_main.c    | 47 ++++++++++
 drivers/net/ethernet/qlogic/qed/qed_mcp.h     |  2 +
 .../net/ethernet/qlogic/qed/qed_reg_addr.h    |  2 +
 drivers/net/ethernet/qlogic/qede/qede_main.c  | 94 ++++++++++++++++---
 include/linux/qed/qed_if.h                    | 11 +++
 7 files changed, 177 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_int.c b/drivers/net/ethernet/qlogic/qed/qed_int.c
index a97f691839e0..f9744c089f1f 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_int.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_int.c
@@ -2399,3 +2399,26 @@ int qed_int_set_timer_res(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
 
 	return rc;
 }
+
+int qed_int_get_sb_dbg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
+		       struct qed_sb_info *p_sb, struct qed_sb_info_dbg *p_info)
+{
+	u16 sbid = p_sb->igu_sb_id;
+	u32 i;
+
+	if (IS_VF(p_hwfn->cdev))
+		return -EINVAL;
+
+	if (sbid >= NUM_OF_SBS(p_hwfn->cdev))
+		return -EINVAL;
+
+	p_info->igu_prod = qed_rd(p_hwfn, p_ptt, IGU_REG_PRODUCER_MEMORY + sbid * 4);
+	p_info->igu_cons = qed_rd(p_hwfn, p_ptt, IGU_REG_CONSUMER_MEM + sbid * 4);
+
+	for (i = 0; i < PIS_PER_SB; i++) {
+		p_info->pi[i] = (u16)qed_rd(p_hwfn, p_ptt,
+					    CAU_REG_PI_MEMORY + sbid * 4 * PIS_PER_SB + i * 4);
+	}
+
+	return 0;
+}
diff --git a/drivers/net/ethernet/qlogic/qed/qed_int.h b/drivers/net/ethernet/qlogic/qed/qed_int.h
index 84c17e97f569..49ebb5d9f767 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_int.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_int.h
@@ -185,6 +185,19 @@ void qed_int_disable_post_isr_release(struct qed_dev *cdev);
  */
 void qed_int_attn_clr_enable(struct qed_dev *cdev, bool clr_enable);
 
+/**
+ * qed_int_get_sb_dbg: Read debug information regarding a given SB
+ *
+ * @p_hwfn: hw function pointer
+ * @p_ptt: ptt resource
+ * @p_sb: pointer to status block for which we want to get info
+ * @p_info: pointer to struct to fill with information regarding SB
+ *
+ * Return: Int
+ */
+int qed_int_get_sb_dbg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
+		       struct qed_sb_info *p_sb, struct qed_sb_info_dbg *p_info);
+
 /**
  * qed_db_rec_handler(): Doorbell Recovery handler.
  *          Run doorbell recovery in case of PF overflow (and flush DORQ if
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index 7673b3e07736..a18d2ea96b26 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -2936,6 +2936,30 @@ static int qed_update_mtu(struct qed_dev *cdev, u16 mtu)
 	return status;
 }
 
+static int
+qed_get_sb_info(struct qed_dev *cdev, struct qed_sb_info *sb,
+		u16 qid, struct qed_sb_info_dbg *sb_dbg)
+{
+	struct qed_hwfn *hwfn = &cdev->hwfns[qid % cdev->num_hwfns];
+	struct qed_ptt *ptt;
+	int rc;
+
+	if (IS_VF(cdev))
+		return -EINVAL;
+
+	ptt = qed_ptt_acquire(hwfn);
+	if (!ptt) {
+		DP_NOTICE(hwfn, "Can't acquire PTT\n");
+		return -EAGAIN;
+	}
+
+	memset(sb_dbg, 0, sizeof(*sb_dbg));
+	rc = qed_int_get_sb_dbg(hwfn, ptt, sb, sb_dbg);
+
+	qed_ptt_release(hwfn, ptt);
+	return rc;
+}
+
 static int qed_read_module_eeprom(struct qed_dev *cdev, char *buf,
 				  u8 dev_addr, u32 offset, u32 len)
 {
@@ -2978,6 +3002,27 @@ static int qed_set_grc_config(struct qed_dev *cdev, u32 cfg_id, u32 val)
 	return rc;
 }
 
+static __printf(2, 3) void qed_mfw_report(struct qed_dev *cdev, char *fmt, ...)
+{
+	char buf[QED_MFW_REPORT_STR_SIZE];
+	struct qed_hwfn *p_hwfn;
+	struct qed_ptt *p_ptt;
+	va_list vl;
+
+	va_start(vl, fmt);
+	vsnprintf(buf, QED_MFW_REPORT_STR_SIZE, fmt, vl);
+	va_end(vl);
+
+	if (IS_PF(cdev)) {
+		p_hwfn = QED_LEADING_HWFN(cdev);
+		p_ptt = qed_ptt_acquire(p_hwfn);
+		if (p_ptt) {
+			qed_mcp_send_raw_debug_data(p_hwfn, p_ptt, buf, strlen(buf));
+			qed_ptt_release(p_hwfn, p_ptt);
+		}
+	}
+}
+
 static u8 qed_get_affin_hwfn_idx(struct qed_dev *cdev)
 {
 	return QED_AFFIN_HWFN_IDX(cdev);
@@ -3038,6 +3083,8 @@ const struct qed_common_ops qed_common_ops_pass = {
 	.read_nvm_cfg = &qed_nvm_flash_cfg_read,
 	.read_nvm_cfg_len = &qed_nvm_flash_cfg_len,
 	.set_grc_config = &qed_set_grc_config,
+	.mfw_report = &qed_mfw_report,
+	.get_sb_info = &qed_get_sb_info,
 };
 
 void qed_get_protocol_stats(struct qed_dev *cdev,
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.h b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
index 564723800d15..2c28d5f86497 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
@@ -15,6 +15,8 @@
 #include "qed_hsi.h"
 #include "qed_dev_api.h"
 
+#define QED_MFW_REPORT_STR_SIZE	256
+
 struct qed_mcp_link_speed_params {
 	bool					autoneg;
 
diff --git a/drivers/net/ethernet/qlogic/qed/qed_reg_addr.h b/drivers/net/ethernet/qlogic/qed/qed_reg_addr.h
index 6f1a52e6beb2..b5e35f433a20 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_reg_addr.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_reg_addr.h
@@ -550,6 +550,8 @@
 		0x1 << 1)
 #define  IGU_REG_BLOCK_CONFIGURATION_PXP_TPH_INTERFACE_EN	( \
 		0x1 << 0)
+#define IGU_REG_PRODUCER_MEMORY 0x182000UL
+#define IGU_REG_CONSUMER_MEM 0x183000UL
 #define  IGU_REG_MAPPING_MEMORY \
 	0x184000UL
 #define IGU_REG_STATISTIC_NUM_VF_MSG_SENT \
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index 06c6a5813606..f37604da79e9 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -509,34 +509,98 @@ static int qede_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	return 0;
 }
 
-static void qede_tx_log_print(struct qede_dev *edev, struct qede_tx_queue *txq)
+static void qede_fp_sb_dump(struct qede_dev *edev, struct qede_fastpath *fp)
 {
+	char *p_sb = (char *)fp->sb_info->sb_virt;
+	u32 sb_size, i;
+
+	sb_size = sizeof(struct status_block);
+
+	for (i = 0; i < sb_size; i += 8) {
+		DP_NOTICE(edev,
+			  "%02hhX %02hhX %02hhX %02hhX  %02hhX %02hhX %02hhX %02hhX\n",
+			  p_sb[i], p_sb[i + 1], p_sb[i + 2], p_sb[i + 3],
+			  p_sb[i + 4], p_sb[i + 5], p_sb[i + 6], p_sb[i + 7]);
+	}
+}
+
+static void
+qede_txq_fp_log_metadata(struct qede_dev *edev,
+			 struct qede_fastpath *fp, struct qede_tx_queue *txq)
+{
+	struct qed_chain *p_chain = &txq->tx_pbl;
+
+	/* Dump txq/fp/sb ids etc. other metadata */
 	DP_NOTICE(edev,
-		  "Txq[%d]: FW cons [host] %04x, SW cons %04x, SW prod %04x [Jiffies %lu]\n",
-		  txq->index, le16_to_cpu(*txq->hw_cons_ptr),
-		  qed_chain_get_cons_idx(&txq->tx_pbl),
-		  qed_chain_get_prod_idx(&txq->tx_pbl),
-		  jiffies);
+		  "fpid 0x%x sbid 0x%x txqid [0x%x] ndev_qid [0x%x] cos [0x%x] p_chain %p cap %d size %d jiffies %lu HZ 0x%x\n",
+		  fp->id, fp->sb_info->igu_sb_id, txq->index, txq->ndev_txq_id, txq->cos,
+		  p_chain, p_chain->capacity, p_chain->size, jiffies, HZ);
+
+	/* Dump all the relevant prod/cons indexes */
+	DP_NOTICE(edev,
+		  "hw cons %04x sw_tx_prod=0x%x, sw_tx_cons=0x%x, bd_prod 0x%x bd_cons 0x%x\n",
+		  le16_to_cpu(*txq->hw_cons_ptr), txq->sw_tx_prod, txq->sw_tx_cons,
+		  qed_chain_get_prod_idx(p_chain), qed_chain_get_cons_idx(p_chain));
+}
+
+static void
+qede_tx_log_print(struct qede_dev *edev, struct qede_fastpath *fp, struct qede_tx_queue *txq)
+{
+	struct qed_sb_info_dbg sb_dbg;
+	int rc;
+
+	/* sb info */
+	qede_fp_sb_dump(edev, fp);
+
+	memset(&sb_dbg, 0, sizeof(sb_dbg));
+	rc = edev->ops->common->get_sb_info(edev->cdev, fp->sb_info, (u16)fp->id, &sb_dbg);
+
+	DP_NOTICE(edev, "IGU: prod %08x cons %08x CAU Tx %04x\n",
+		  sb_dbg.igu_prod, sb_dbg.igu_cons, sb_dbg.pi[TX_PI(txq->cos)]);
+
+	/* report to mfw */
+	edev->ops->common->mfw_report(edev->cdev,
+				      "Txq[%d]: FW cons [host] %04x, SW cons %04x, SW prod %04x [Jiffies %lu]\n",
+				      txq->index, le16_to_cpu(*txq->hw_cons_ptr),
+				      qed_chain_get_cons_idx(&txq->tx_pbl),
+				      qed_chain_get_prod_idx(&txq->tx_pbl), jiffies);
+	if (!rc) {
+		edev->ops->common->mfw_report(edev->cdev,
+					      "Txq[%d]: SB[0x%04x] - IGU: prod %08x cons %08x CAU Tx %04x\n",
+					      txq->index, fp->sb_info->igu_sb_id,
+					      sb_dbg.igu_prod, sb_dbg.igu_cons,
+					      sb_dbg.pi[TX_PI(txq->cos)]);
+	}
 }
 
 static void qede_tx_timeout(struct net_device *dev, unsigned int txqueue)
 {
 	struct qede_dev *edev = netdev_priv(dev);
-	struct qede_tx_queue *txq;
-	int cos;
+	int i;
 
 	netif_carrier_off(dev);
 	DP_NOTICE(edev, "TX timeout on queue %u!\n", txqueue);
 
-	if (!(edev->fp_array[txqueue].type & QEDE_FASTPATH_TX))
-		return;
+	for_each_queue(i) {
+		struct qede_tx_queue *txq;
+		struct qede_fastpath *fp;
+		int cos;
 
-	for_each_cos_in_txq(edev, cos) {
-		txq = &edev->fp_array[txqueue].txq[cos];
+		fp = &edev->fp_array[i];
+		if (!(fp->type & QEDE_FASTPATH_TX))
+			continue;
+
+		for_each_cos_in_txq(edev, cos) {
+			txq = &fp->txq[cos];
 
-		if (qed_chain_get_cons_idx(&txq->tx_pbl) !=
-		    qed_chain_get_prod_idx(&txq->tx_pbl))
-			qede_tx_log_print(edev, txq);
+			/* Dump basic metadata for all queues */
+			qede_txq_fp_log_metadata(edev, fp, txq);
+
+			if (qed_chain_get_cons_idx(&txq->tx_pbl) !=
+			    qed_chain_get_prod_idx(&txq->tx_pbl)) {
+				qede_tx_log_print(edev, fp, txq);
+			}
+		}
 	}
 
 	if (IS_VF(edev))
diff --git a/include/linux/qed/qed_if.h b/include/linux/qed/qed_if.h
index 0dae7fcc5ef2..9f4bfa2a4829 100644
--- a/include/linux/qed/qed_if.h
+++ b/include/linux/qed/qed_if.h
@@ -807,6 +807,12 @@ struct qed_devlink {
 	struct devlink_health_reporter *fw_reporter;
 };
 
+struct qed_sb_info_dbg {
+	u32 igu_prod;
+	u32 igu_cons;
+	u16 pi[PIS_PER_SB];
+};
+
 struct qed_common_cb_ops {
 	void (*arfs_filter_op)(void *dev, void *fltr, u8 fw_rc);
 	void (*link_update)(void *dev, struct qed_link_output *link);
@@ -1194,6 +1200,11 @@ struct qed_common_ops {
 	struct devlink* (*devlink_register)(struct qed_dev *cdev);
 
 	void (*devlink_unregister)(struct devlink *devlink);
+
+	__printf(2, 3) void (*mfw_report)(struct qed_dev *cdev, char *fmt, ...);
+
+	int (*get_sb_info)(struct qed_dev *cdev, struct qed_sb_info *sb,
+			   u16 qid, struct qed_sb_info_dbg *sb_dbg);
 };
 
 #define MASK_FIELD(_name, _value) \
-- 
2.27.0


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

* [PATCH net-next 2/2] qed*: esl priv flag support through ethtool
  2021-11-24  9:43 [PATCH net-next 0/2] qed*: enhancements Manish Chopra
  2021-11-24  9:43 ` [PATCH net-next 1/2] qed*: enhance tx timeout debug info Manish Chopra
@ 2021-11-24  9:43 ` Manish Chopra
  2021-11-25  2:53   ` Jakub Kicinski
  1 sibling, 1 reply; 7+ messages in thread
From: Manish Chopra @ 2021-11-24  9:43 UTC (permalink / raw)
  To: kuba; +Cc: netdev, aelior, palok, pkushwaha

This patch adds relevant qed APIs to get ESL (enhance system lockdown)
capabilities and current status for ESL, which can be queried via qede
.get_priv_flags() ethtool interface

Signed-off-by: Manish Chopra <manishc@marvell.com>
Signed-off-by: Prabhakar Kushwaha <pkushwaha@marvell.com>
Signed-off-by: Alok Prasad <palok@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 drivers/net/ethernet/qlogic/qed/qed_main.c    | 25 ++++++++++++++++++-
 drivers/net/ethernet/qlogic/qed/qed_mcp.c     | 22 ++++++++++++++++
 drivers/net/ethernet/qlogic/qed/qed_mcp.h     | 20 +++++++++++++++
 drivers/net/ethernet/qlogic/qed/qed_mfw_hsi.h |  1 +
 .../net/ethernet/qlogic/qede/qede_ethtool.c   | 13 ++++++++++
 include/linux/qed/qed_if.h                    |  3 +++
 6 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index a18d2ea96b26..46d4207f22a3 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -447,7 +447,7 @@ int qed_fill_dev_info(struct qed_dev *cdev,
 			dev_info->wol_support = true;
 
 		dev_info->smart_an = qed_mcp_is_smart_an_supported(p_hwfn);
-
+		dev_info->esl = qed_mcp_is_esl_supported(p_hwfn);
 		dev_info->abs_pf_id = QED_LEADING_HWFN(cdev)->abs_pf_id;
 	} else {
 		qed_vf_get_fw_version(&cdev->hwfns[0], &dev_info->fw_major,
@@ -3028,6 +3028,28 @@ static u8 qed_get_affin_hwfn_idx(struct qed_dev *cdev)
 	return QED_AFFIN_HWFN_IDX(cdev);
 }
 
+static int qed_get_esl_status(struct qed_dev *cdev, bool *esl_active)
+{
+	struct qed_hwfn *hwfn = QED_LEADING_HWFN(cdev);
+	struct qed_ptt *ptt;
+	int rc = 0;
+
+	*esl_active = false;
+
+	if (IS_VF(cdev))
+		return 0;
+
+	ptt = qed_ptt_acquire(hwfn);
+	if (!ptt)
+		return -EAGAIN;
+
+	rc = qed_mcp_get_esl_status(hwfn, ptt, esl_active);
+
+	qed_ptt_release(hwfn, ptt);
+
+	return rc;
+}
+
 static struct qed_selftest_ops qed_selftest_ops_pass = {
 	.selftest_memory = &qed_selftest_memory,
 	.selftest_interrupt = &qed_selftest_interrupt,
@@ -3085,6 +3107,7 @@ const struct qed_common_ops qed_common_ops_pass = {
 	.set_grc_config = &qed_set_grc_config,
 	.mfw_report = &qed_mfw_report,
 	.get_sb_info = &qed_get_sb_info,
+	.get_esl_status = &qed_get_esl_status,
 };
 
 void qed_get_protocol_stats(struct qed_dev *cdev,
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.c b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
index 64678a256f3b..da1eadabcb41 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
@@ -4158,3 +4158,25 @@ qed_mcp_send_raw_debug_data(struct qed_hwfn *p_hwfn,
 	return qed_mcp_send_debug_data(p_hwfn, p_ptt,
 				       QED_MCP_DBG_DATA_TYPE_RAW, p_buf, size);
 }
+
+bool qed_mcp_is_esl_supported(struct qed_hwfn *p_hwfn)
+{
+	return !!(p_hwfn->mcp_info->capabilities &
+		  FW_MB_PARAM_FEATURE_SUPPORT_ENHANCED_SYS_LCK);
+}
+
+int qed_mcp_get_esl_status(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt, bool *active)
+{
+	u32 resp = 0, param = 0;
+	int rc;
+
+	rc = qed_mcp_cmd(p_hwfn, p_ptt, DRV_MSG_CODE_GET_MANAGEMENT_STATUS, 0, &resp, &param);
+	if (rc) {
+		DP_NOTICE(p_hwfn, "Failed to send ESL command, rc = %d\n", rc);
+		return rc;
+	}
+
+	*active = !!(param & FW_MB_PARAM_MANAGEMENT_STATUS_LOCKDOWN_ENABLED);
+
+	return 0;
+}
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.h b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
index 2c28d5f86497..3cf6dab9e582 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
@@ -1339,4 +1339,24 @@ int qed_mcp_nvm_get_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
 int qed_mcp_nvm_set_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
 			u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
 			u32 len);
+
+/**
+ * qed_mcp_is_esl_supported(): Return whether management firmware support ESL or not.
+ *
+ * @p_hwfn: hw function pointer
+ *
+ * Return: bool
+ */
+bool qed_mcp_is_esl_supported(struct qed_hwfn *p_hwfn);
+
+/**
+ * qed_mcp_get_esl_status(): Get enhanced system lockdown status
+ *
+ * @p_hwfn: hw function pointer
+ * @p_ptt: ptt resource pointer
+ * @active: ESL active status data pointer
+ *
+ * Return: Int
+ */
+int qed_mcp_get_esl_status(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt, bool *active);
 #endif
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mfw_hsi.h b/drivers/net/ethernet/qlogic/qed/qed_mfw_hsi.h
index 8a0e3c5d4bda..b70ee8200e15 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mfw_hsi.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_mfw_hsi.h
@@ -1191,6 +1191,7 @@ enum drv_msg_code_enum {
 	DRV_MSG_CODE_CFG_VF_MSIX = DRV_MSG_CODE(0xc001),
 	DRV_MSG_CODE_CFG_PF_VFS_MSIX = DRV_MSG_CODE(0xc002),
 	DRV_MSG_CODE_DEBUG_DATA_SEND = DRV_MSG_CODE(0xc004),
+	DRV_MSG_CODE_GET_MANAGEMENT_STATUS = DRV_MSG_CODE(0xc007),
 };
 
 #define DRV_MSG_CODE_VMAC_TYPE_SHIFT            4
diff --git a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
index 100c9c52c20b..97a7ab0826ed 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_ethtool.c
@@ -168,6 +168,8 @@ enum {
 	QEDE_PRI_FLAG_CMT,
 	QEDE_PRI_FLAG_SMART_AN_SUPPORT, /* MFW supports SmartAN */
 	QEDE_PRI_FLAG_RECOVER_ON_ERROR,
+	QEDE_PRI_FLAG_ESL_SUPPORT, /* MFW supports Enhanced System Lockdown */
+	QEDE_PRI_FLAG_ESL_ACTIVE, /* Enhanced System Lockdown Active status */
 	QEDE_PRI_FLAG_LEN,
 };
 
@@ -175,6 +177,8 @@ static const char qede_private_arr[QEDE_PRI_FLAG_LEN][ETH_GSTRING_LEN] = {
 	"Coupled-Function",
 	"SmartAN capable",
 	"Recover on error",
+	"ESL capable",
+	"ESL active",
 };
 
 enum qede_ethtool_tests {
@@ -478,6 +482,7 @@ static int qede_get_sset_count(struct net_device *dev, int stringset)
 static u32 qede_get_priv_flags(struct net_device *dev)
 {
 	struct qede_dev *edev = netdev_priv(dev);
+	bool esl_active;
 	u32 flags = 0;
 
 	if (edev->dev_info.common.num_hwfns > 1)
@@ -489,6 +494,14 @@ static u32 qede_get_priv_flags(struct net_device *dev)
 	if (edev->err_flags & BIT(QEDE_ERR_IS_RECOVERABLE))
 		flags |= BIT(QEDE_PRI_FLAG_RECOVER_ON_ERROR);
 
+	if (edev->dev_info.common.esl)
+		flags |= BIT(QEDE_PRI_FLAG_ESL_SUPPORT);
+
+	edev->ops->common->get_esl_status(edev->cdev, &esl_active);
+
+	if (esl_active)
+		flags |= BIT(QEDE_PRI_FLAG_ESL_ACTIVE);
+
 	return flags;
 }
 
diff --git a/include/linux/qed/qed_if.h b/include/linux/qed/qed_if.h
index 9f4bfa2a4829..6dc4943d8aec 100644
--- a/include/linux/qed/qed_if.h
+++ b/include/linux/qed/qed_if.h
@@ -652,6 +652,7 @@ struct qed_dev_info {
 
 	bool wol_support;
 	bool smart_an;
+	bool esl;
 
 	/* MBI version */
 	u32 mbi_version;
@@ -1205,6 +1206,8 @@ struct qed_common_ops {
 
 	int (*get_sb_info)(struct qed_dev *cdev, struct qed_sb_info *sb,
 			   u16 qid, struct qed_sb_info_dbg *sb_dbg);
+
+	int (*get_esl_status)(struct qed_dev *cdev, bool *esl_active);
 };
 
 #define MASK_FIELD(_name, _value) \
-- 
2.27.0


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

* Re: [PATCH net-next 1/2] qed*: enhance tx timeout debug info
  2021-11-24  9:43 ` [PATCH net-next 1/2] qed*: enhance tx timeout debug info Manish Chopra
@ 2021-11-25  2:51   ` Jakub Kicinski
  2021-11-26 22:04     ` [EXT] " Manish Chopra
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2021-11-25  2:51 UTC (permalink / raw)
  To: Manish Chopra; +Cc: netdev, aelior, palok, pkushwaha

On Wed, 24 Nov 2021 01:43:02 -0800 Manish Chopra wrote:
> This patch add some new qed APIs to query status block
> info and report various data to MFW on tx timeout event
> 
> Along with that it enhances qede to dump more debug logs
> (not just specific to the queue which was reported by stack)
> on tx timeout which includes various other basic metadata about
> all tx queues and other info (like status block etc.)
> 
> Signed-off-by: Manish Chopra <manishc@marvell.com>
> Signed-off-by: Prabhakar Kushwaha <pkushwaha@marvell.com>
> Signed-off-by: Alok Prasad <palok@marvell.com>
> Signed-off-by: Ariel Elior <aelior@marvell.com>

Please consider using devlink health if you want to communicate more
data to the user.

> diff --git a/drivers/net/ethernet/qlogic/qed/qed_int.c b/drivers/net/ethernet/qlogic/qed/qed_int.c
> index a97f691839e0..f9744c089f1f 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_int.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_int.c
> @@ -2399,3 +2399,26 @@ int qed_int_set_timer_res(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
>  
>  	return rc;
>  }
> +
> +int qed_int_get_sb_dbg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
> +		       struct qed_sb_info *p_sb, struct qed_sb_info_dbg *p_info)
> +{
> +	u16 sbid = p_sb->igu_sb_id;
> +	u32 i;
> +
> +	if (IS_VF(p_hwfn->cdev))
> +		return -EINVAL;
> +
> +	if (sbid >= NUM_OF_SBS(p_hwfn->cdev))
> +		return -EINVAL;
> +
> +	p_info->igu_prod = qed_rd(p_hwfn, p_ptt, IGU_REG_PRODUCER_MEMORY + sbid * 4);
> +	p_info->igu_cons = qed_rd(p_hwfn, p_ptt, IGU_REG_CONSUMER_MEM + sbid * 4);
> +
> +	for (i = 0; i < PIS_PER_SB; i++) {
> +		p_info->pi[i] = (u16)qed_rd(p_hwfn, p_ptt,
> +					    CAU_REG_PI_MEMORY + sbid * 4 * PIS_PER_SB + i * 4);
> +	}

bracket unnecessary, there's a lot of those, please fix all of them.

> +
> +	return 0;
> +}
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_int.h b/drivers/net/ethernet/qlogic/qed/qed_int.h
> index 84c17e97f569..49ebb5d9f767 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_int.h
> +++ b/drivers/net/ethernet/qlogic/qed/qed_int.h
> @@ -185,6 +185,19 @@ void qed_int_disable_post_isr_release(struct qed_dev *cdev);
>   */
>  void qed_int_attn_clr_enable(struct qed_dev *cdev, bool clr_enable);
>  
> +/**
> + * qed_int_get_sb_dbg: Read debug information regarding a given SB
> + *
> + * @p_hwfn: hw function pointer
> + * @p_ptt: ptt resource
> + * @p_sb: pointer to status block for which we want to get info
> + * @p_info: pointer to struct to fill with information regarding SB
> + *
> + * Return: Int

What's the point of documenting the return type?

> + */
> +int qed_int_get_sb_dbg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
> +		       struct qed_sb_info *p_sb, struct qed_sb_info_dbg *p_info);
> +
>  /**
>   * qed_db_rec_handler(): Doorbell Recovery handler.
>   *          Run doorbell recovery in case of PF overflow (and flush DORQ if
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
> index 7673b3e07736..a18d2ea96b26 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_main.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
> @@ -2936,6 +2936,30 @@ static int qed_update_mtu(struct qed_dev *cdev, u16 mtu)
>  	return status;
>  }
>  
> +static int
> +qed_get_sb_info(struct qed_dev *cdev, struct qed_sb_info *sb,
> +		u16 qid, struct qed_sb_info_dbg *sb_dbg)
> +{
> +	struct qed_hwfn *hwfn = &cdev->hwfns[qid % cdev->num_hwfns];
> +	struct qed_ptt *ptt;
> +	int rc;
> +
> +	if (IS_VF(cdev))
> +		return -EINVAL;
> +
> +	ptt = qed_ptt_acquire(hwfn);
> +	if (!ptt) {
> +		DP_NOTICE(hwfn, "Can't acquire PTT\n");
> +		return -EAGAIN;
> +	}
> +
> +	memset(sb_dbg, 0, sizeof(*sb_dbg));
> +	rc = qed_int_get_sb_dbg(hwfn, ptt, sb, sb_dbg);
> +
> +	qed_ptt_release(hwfn, ptt);
> +	return rc;
> +}
> +
>  static int qed_read_module_eeprom(struct qed_dev *cdev, char *buf,
>  				  u8 dev_addr, u32 offset, u32 len)
>  {
> @@ -2978,6 +3002,27 @@ static int qed_set_grc_config(struct qed_dev *cdev, u32 cfg_id, u32 val)
>  	return rc;
>  }
>  
> +static __printf(2, 3) void qed_mfw_report(struct qed_dev *cdev, char *fmt, ...)
> +{
> +	char buf[QED_MFW_REPORT_STR_SIZE];
> +	struct qed_hwfn *p_hwfn;
> +	struct qed_ptt *p_ptt;
> +	va_list vl;
> +
> +	va_start(vl, fmt);
> +	vsnprintf(buf, QED_MFW_REPORT_STR_SIZE, fmt, vl);
> +	va_end(vl);
> +
> +	if (IS_PF(cdev)) {
> +		p_hwfn = QED_LEADING_HWFN(cdev);
> +		p_ptt = qed_ptt_acquire(p_hwfn);
> +		if (p_ptt) {
> +			qed_mcp_send_raw_debug_data(p_hwfn, p_ptt, buf, strlen(buf));
> +			qed_ptt_release(p_hwfn, p_ptt);
> +		}
> +	}
> +}
> +
>  static u8 qed_get_affin_hwfn_idx(struct qed_dev *cdev)
>  {
>  	return QED_AFFIN_HWFN_IDX(cdev);
> @@ -3038,6 +3083,8 @@ const struct qed_common_ops qed_common_ops_pass = {
>  	.read_nvm_cfg = &qed_nvm_flash_cfg_read,
>  	.read_nvm_cfg_len = &qed_nvm_flash_cfg_len,
>  	.set_grc_config = &qed_set_grc_config,
> +	.mfw_report = &qed_mfw_report,
> +	.get_sb_info = &qed_get_sb_info,
>  };
>  
>  void qed_get_protocol_stats(struct qed_dev *cdev,
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.h b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
> index 564723800d15..2c28d5f86497 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_mcp.h
> +++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
> @@ -15,6 +15,8 @@
>  #include "qed_hsi.h"
>  #include "qed_dev_api.h"
>  
> +#define QED_MFW_REPORT_STR_SIZE	256
> +
>  struct qed_mcp_link_speed_params {
>  	bool					autoneg;
>  
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_reg_addr.h b/drivers/net/ethernet/qlogic/qed/qed_reg_addr.h
> index 6f1a52e6beb2..b5e35f433a20 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_reg_addr.h
> +++ b/drivers/net/ethernet/qlogic/qed/qed_reg_addr.h
> @@ -550,6 +550,8 @@
>  		0x1 << 1)
>  #define  IGU_REG_BLOCK_CONFIGURATION_PXP_TPH_INTERFACE_EN	( \
>  		0x1 << 0)
> +#define IGU_REG_PRODUCER_MEMORY 0x182000UL
> +#define IGU_REG_CONSUMER_MEM 0x183000UL
>  #define  IGU_REG_MAPPING_MEMORY \
>  	0x184000UL
>  #define IGU_REG_STATISTIC_NUM_VF_MSG_SENT \
> diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
> index 06c6a5813606..f37604da79e9 100644
> --- a/drivers/net/ethernet/qlogic/qede/qede_main.c
> +++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
> @@ -509,34 +509,98 @@ static int qede_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
>  	return 0;
>  }
>  
> -static void qede_tx_log_print(struct qede_dev *edev, struct qede_tx_queue *txq)
> +static void qede_fp_sb_dump(struct qede_dev *edev, struct qede_fastpath *fp)
>  {
> +	char *p_sb = (char *)fp->sb_info->sb_virt;
> +	u32 sb_size, i;
> +
> +	sb_size = sizeof(struct status_block);
> +
> +	for (i = 0; i < sb_size; i += 8) {
> +		DP_NOTICE(edev,
> +			  "%02hhX %02hhX %02hhX %02hhX  %02hhX %02hhX %02hhX %02hhX\n",
> +			  p_sb[i], p_sb[i + 1], p_sb[i + 2], p_sb[i + 3],
> +			  p_sb[i + 4], p_sb[i + 5], p_sb[i + 6], p_sb[i + 7]);
> +	}
> +}
> +
> +static void
> +qede_txq_fp_log_metadata(struct qede_dev *edev,
> +			 struct qede_fastpath *fp, struct qede_tx_queue *txq)
> +{
> +	struct qed_chain *p_chain = &txq->tx_pbl;
> +
> +	/* Dump txq/fp/sb ids etc. other metadata */
>  	DP_NOTICE(edev,
> -		  "Txq[%d]: FW cons [host] %04x, SW cons %04x, SW prod %04x [Jiffies %lu]\n",
> -		  txq->index, le16_to_cpu(*txq->hw_cons_ptr),
> -		  qed_chain_get_cons_idx(&txq->tx_pbl),
> -		  qed_chain_get_prod_idx(&txq->tx_pbl),
> -		  jiffies);
> +		  "fpid 0x%x sbid 0x%x txqid [0x%x] ndev_qid [0x%x] cos [0x%x] p_chain %p cap %d size %d jiffies %lu HZ 0x%x\n",
> +		  fp->id, fp->sb_info->igu_sb_id, txq->index, txq->ndev_txq_id, txq->cos,
> +		  p_chain, p_chain->capacity, p_chain->size, jiffies, HZ);
> +
> +	/* Dump all the relevant prod/cons indexes */
> +	DP_NOTICE(edev,
> +		  "hw cons %04x sw_tx_prod=0x%x, sw_tx_cons=0x%x, bd_prod 0x%x bd_cons 0x%x\n",
> +		  le16_to_cpu(*txq->hw_cons_ptr), txq->sw_tx_prod, txq->sw_tx_cons,
> +		  qed_chain_get_prod_idx(p_chain), qed_chain_get_cons_idx(p_chain));
> +}
> +
> +static void
> +qede_tx_log_print(struct qede_dev *edev, struct qede_fastpath *fp, struct qede_tx_queue *txq)
> +{
> +	struct qed_sb_info_dbg sb_dbg;
> +	int rc;
> +
> +	/* sb info */
> +	qede_fp_sb_dump(edev, fp);
> +
> +	memset(&sb_dbg, 0, sizeof(sb_dbg));
> +	rc = edev->ops->common->get_sb_info(edev->cdev, fp->sb_info, (u16)fp->id, &sb_dbg);
> +
> +	DP_NOTICE(edev, "IGU: prod %08x cons %08x CAU Tx %04x\n",
> +		  sb_dbg.igu_prod, sb_dbg.igu_cons, sb_dbg.pi[TX_PI(txq->cos)]);
> +
> +	/* report to mfw */
> +	edev->ops->common->mfw_report(edev->cdev,
> +				      "Txq[%d]: FW cons [host] %04x, SW cons %04x, SW prod %04x [Jiffies %lu]\n",
> +				      txq->index, le16_to_cpu(*txq->hw_cons_ptr),
> +				      qed_chain_get_cons_idx(&txq->tx_pbl),
> +				      qed_chain_get_prod_idx(&txq->tx_pbl), jiffies);
> +	if (!rc) {
> +		edev->ops->common->mfw_report(edev->cdev,
> +					      "Txq[%d]: SB[0x%04x] - IGU: prod %08x cons %08x CAU Tx %04x\n",
> +					      txq->index, fp->sb_info->igu_sb_id,
> +					      sb_dbg.igu_prod, sb_dbg.igu_cons,
> +					      sb_dbg.pi[TX_PI(txq->cos)]);
> +	}
>  }
>  
>  static void qede_tx_timeout(struct net_device *dev, unsigned int txqueue)
>  {
>  	struct qede_dev *edev = netdev_priv(dev);
> -	struct qede_tx_queue *txq;
> -	int cos;
> +	int i;
>  
>  	netif_carrier_off(dev);
>  	DP_NOTICE(edev, "TX timeout on queue %u!\n", txqueue);
>  
> -	if (!(edev->fp_array[txqueue].type & QEDE_FASTPATH_TX))
> -		return;
> +	for_each_queue(i) {

Please only dump state for the queue that timed out. 

> +		struct qede_tx_queue *txq;
> +		struct qede_fastpath *fp;
> +		int cos;
>  
> -	for_each_cos_in_txq(edev, cos) {
> -		txq = &edev->fp_array[txqueue].txq[cos];
> +		fp = &edev->fp_array[i];
> +		if (!(fp->type & QEDE_FASTPATH_TX))
> +			continue;
> +
> +		for_each_cos_in_txq(edev, cos) {
> +			txq = &fp->txq[cos];
>  
> -		if (qed_chain_get_cons_idx(&txq->tx_pbl) !=
> -		    qed_chain_get_prod_idx(&txq->tx_pbl))
> -			qede_tx_log_print(edev, txq);
> +			/* Dump basic metadata for all queues */
> +			qede_txq_fp_log_metadata(edev, fp, txq);
> +
> +			if (qed_chain_get_cons_idx(&txq->tx_pbl) !=
> +			    qed_chain_get_prod_idx(&txq->tx_pbl)) {
> +				qede_tx_log_print(edev, fp, txq);
> +			}
> +		}
>  	}
>  
>  	if (IS_VF(edev))
> diff --git a/include/linux/qed/qed_if.h b/include/linux/qed/qed_if.h
> index 0dae7fcc5ef2..9f4bfa2a4829 100644
> --- a/include/linux/qed/qed_if.h
> +++ b/include/linux/qed/qed_if.h
> @@ -807,6 +807,12 @@ struct qed_devlink {
>  	struct devlink_health_reporter *fw_reporter;
>  };
>  
> +struct qed_sb_info_dbg {
> +	u32 igu_prod;
> +	u32 igu_cons;

This info gets populated by the device, right? It should probably use
the correct endian types.

> +	u16 pi[PIS_PER_SB];
> +};
> +
>  struct qed_common_cb_ops {
>  	void (*arfs_filter_op)(void *dev, void *fltr, u8 fw_rc);
>  	void (*link_update)(void *dev, struct qed_link_output *link);


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

* Re: [PATCH net-next 2/2] qed*: esl priv flag support through ethtool
  2021-11-24  9:43 ` [PATCH net-next 2/2] qed*: esl priv flag support through ethtool Manish Chopra
@ 2021-11-25  2:53   ` Jakub Kicinski
  0 siblings, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2021-11-25  2:53 UTC (permalink / raw)
  To: Manish Chopra; +Cc: netdev, aelior, palok, pkushwaha

On Wed, 24 Nov 2021 01:43:03 -0800 Manish Chopra wrote:
> This patch adds relevant qed APIs to get ESL (enhance system lockdown)
> capabilities and current status for ESL, which can be queried via qede
> .get_priv_flags() ethtool interface

You need to spell out what this feature is and what it does.
Speaking of spelling s/enhance/enhanced/.

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

* RE: [EXT] Re: [PATCH net-next 1/2] qed*: enhance tx timeout debug info
  2021-11-25  2:51   ` Jakub Kicinski
@ 2021-11-26 22:04     ` Manish Chopra
  2021-11-27  0:41       ` Jakub Kicinski
  0 siblings, 1 reply; 7+ messages in thread
From: Manish Chopra @ 2021-11-26 22:04 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, Ariel Elior, Alok Prasad, Prabhakar Kushwaha

Hi Jakub,

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Thursday, November 25, 2021 8:22 AM
> To: Manish Chopra <manishc@marvell.com>
> Cc: netdev@vger.kernel.org; Ariel Elior <aelior@marvell.com>; Alok Prasad
> <palok@marvell.com>; Prabhakar Kushwaha <pkushwaha@marvell.com>
> Subject: [EXT] Re: [PATCH net-next 1/2] qed*: enhance tx timeout debug info
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Wed, 24 Nov 2021 01:43:02 -0800 Manish Chopra wrote:
> > This patch add some new qed APIs to query status block info and report
> > various data to MFW on tx timeout event
> >
> > Along with that it enhances qede to dump more debug logs (not just
> > specific to the queue which was reported by stack) on tx timeout which
> > includes various other basic metadata about all tx queues and other
> > info (like status block etc.)
> >
> > Signed-off-by: Manish Chopra <manishc@marvell.com>
> > Signed-off-by: Prabhakar Kushwaha <pkushwaha@marvell.com>
> > Signed-off-by: Alok Prasad <palok@marvell.com>
> > Signed-off-by: Ariel Elior <aelior@marvell.com>
> 
> Please consider using devlink health if you want to communicate more data to
> the user

It's not really that huge logs/data, these are just very basic metadata (few prints) about the TX queues logged to system logs.
Those can be looked easily from dmesg/var-log-messages files which can be made available conveniently.
Rest are the mailbox commands posted to management firmware with those basic information about the queues.

> 
> > diff --git a/drivers/net/ethernet/qlogic/qed/qed_int.c
> > b/drivers/net/ethernet/qlogic/qed/qed_int.c
> > index a97f691839e0..f9744c089f1f 100644
> > --- a/drivers/net/ethernet/qlogic/qed/qed_int.c
> > +++ b/drivers/net/ethernet/qlogic/qed/qed_int.c
> > @@ -2399,3 +2399,26 @@ int qed_int_set_timer_res(struct qed_hwfn
> > *p_hwfn, struct qed_ptt *p_ptt,
> >
> >  	return rc;
> >  }
> > +
> > +int qed_int_get_sb_dbg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
> > +		       struct qed_sb_info *p_sb, struct qed_sb_info_dbg *p_info) {
> > +	u16 sbid = p_sb->igu_sb_id;
> > +	u32 i;
> > +
> > +	if (IS_VF(p_hwfn->cdev))
> > +		return -EINVAL;
> > +
> > +	if (sbid >= NUM_OF_SBS(p_hwfn->cdev))
> > +		return -EINVAL;
> > +
> > +	p_info->igu_prod = qed_rd(p_hwfn, p_ptt,
> IGU_REG_PRODUCER_MEMORY + sbid * 4);
> > +	p_info->igu_cons = qed_rd(p_hwfn, p_ptt, IGU_REG_CONSUMER_MEM
> + sbid
> > +* 4);
> > +
> > +	for (i = 0; i < PIS_PER_SB; i++) {
> > +		p_info->pi[i] = (u16)qed_rd(p_hwfn, p_ptt,
> > +					    CAU_REG_PI_MEMORY + sbid * 4 *
> PIS_PER_SB + i * 4);
> > +	}
> 
> bracket unnecessary, there's a lot of those, please fix all of them.
> 
> > +
> > +	return 0;
> > +}
> > diff --git a/drivers/net/ethernet/qlogic/qed/qed_int.h
> > b/drivers/net/ethernet/qlogic/qed/qed_int.h
> > index 84c17e97f569..49ebb5d9f767 100644
> > --- a/drivers/net/ethernet/qlogic/qed/qed_int.h
> > +++ b/drivers/net/ethernet/qlogic/qed/qed_int.h
> > @@ -185,6 +185,19 @@ void qed_int_disable_post_isr_release(struct
> qed_dev *cdev);
> >   */
> >  void qed_int_attn_clr_enable(struct qed_dev *cdev, bool clr_enable);
> >
> > +/**
> > + * qed_int_get_sb_dbg: Read debug information regarding a given SB
> > + *
> > + * @p_hwfn: hw function pointer
> > + * @p_ptt: ptt resource
> > + * @p_sb: pointer to status block for which we want to get info
> > + * @p_info: pointer to struct to fill with information regarding SB
> > + *
> > + * Return: Int
> 
> What's the point of documenting the return type?

For ./scripts/kernel-doc, I will put some suitable description. 

> 
> > + */
> > +int qed_int_get_sb_dbg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
> > +		       struct qed_sb_info *p_sb, struct qed_sb_info_dbg *p_info);
> > +
> >  /**
> >   * qed_db_rec_handler(): Doorbell Recovery handler.
> >   *          Run doorbell recovery in case of PF overflow (and flush DORQ if
> > diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c
> > b/drivers/net/ethernet/qlogic/qed/qed_main.c
> > index 7673b3e07736..a18d2ea96b26 100644
> > --- a/drivers/net/ethernet/qlogic/qed/qed_main.c
> > +++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
> > @@ -2936,6 +2936,30 @@ static int qed_update_mtu(struct qed_dev *cdev,
> u16 mtu)
> >  	return status;
> >  }
> >
> > +static int
> > +qed_get_sb_info(struct qed_dev *cdev, struct qed_sb_info *sb,
> > +		u16 qid, struct qed_sb_info_dbg *sb_dbg) {
> > +	struct qed_hwfn *hwfn = &cdev->hwfns[qid % cdev->num_hwfns];
> > +	struct qed_ptt *ptt;
> > +	int rc;
> > +
> > +	if (IS_VF(cdev))
> > +		return -EINVAL;
> > +
> > +	ptt = qed_ptt_acquire(hwfn);
> > +	if (!ptt) {
> > +		DP_NOTICE(hwfn, "Can't acquire PTT\n");
> > +		return -EAGAIN;
> > +	}
> > +
> > +	memset(sb_dbg, 0, sizeof(*sb_dbg));
> > +	rc = qed_int_get_sb_dbg(hwfn, ptt, sb, sb_dbg);
> > +
> > +	qed_ptt_release(hwfn, ptt);
> > +	return rc;
> > +}
> > +
> >  static int qed_read_module_eeprom(struct qed_dev *cdev, char *buf,
> >  				  u8 dev_addr, u32 offset, u32 len)  { @@ -
> 2978,6 +3002,27 @@
> > static int qed_set_grc_config(struct qed_dev *cdev, u32 cfg_id, u32 val)
> >  	return rc;
> >  }
> >
> > +static __printf(2, 3) void qed_mfw_report(struct qed_dev *cdev, char
> > +*fmt, ...) {
> > +	char buf[QED_MFW_REPORT_STR_SIZE];
> > +	struct qed_hwfn *p_hwfn;
> > +	struct qed_ptt *p_ptt;
> > +	va_list vl;
> > +
> > +	va_start(vl, fmt);
> > +	vsnprintf(buf, QED_MFW_REPORT_STR_SIZE, fmt, vl);
> > +	va_end(vl);
> > +
> > +	if (IS_PF(cdev)) {
> > +		p_hwfn = QED_LEADING_HWFN(cdev);
> > +		p_ptt = qed_ptt_acquire(p_hwfn);
> > +		if (p_ptt) {
> > +			qed_mcp_send_raw_debug_data(p_hwfn, p_ptt, buf,
> strlen(buf));
> > +			qed_ptt_release(p_hwfn, p_ptt);
> > +		}
> > +	}
> > +}
> > +
> >  static u8 qed_get_affin_hwfn_idx(struct qed_dev *cdev)  {
> >  	return QED_AFFIN_HWFN_IDX(cdev);
> > @@ -3038,6 +3083,8 @@ const struct qed_common_ops
> qed_common_ops_pass = {
> >  	.read_nvm_cfg = &qed_nvm_flash_cfg_read,
> >  	.read_nvm_cfg_len = &qed_nvm_flash_cfg_len,
> >  	.set_grc_config = &qed_set_grc_config,
> > +	.mfw_report = &qed_mfw_report,
> > +	.get_sb_info = &qed_get_sb_info,
> >  };
> >
> >  void qed_get_protocol_stats(struct qed_dev *cdev, diff --git
> > a/drivers/net/ethernet/qlogic/qed/qed_mcp.h
> > b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
> > index 564723800d15..2c28d5f86497 100644
> > --- a/drivers/net/ethernet/qlogic/qed/qed_mcp.h
> > +++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
> > @@ -15,6 +15,8 @@
> >  #include "qed_hsi.h"
> >  #include "qed_dev_api.h"
> >
> > +#define QED_MFW_REPORT_STR_SIZE	256
> > +
> >  struct qed_mcp_link_speed_params {
> >  	bool					autoneg;
> >
> > diff --git a/drivers/net/ethernet/qlogic/qed/qed_reg_addr.h
> > b/drivers/net/ethernet/qlogic/qed/qed_reg_addr.h
> > index 6f1a52e6beb2..b5e35f433a20 100644
> > --- a/drivers/net/ethernet/qlogic/qed/qed_reg_addr.h
> > +++ b/drivers/net/ethernet/qlogic/qed/qed_reg_addr.h
> > @@ -550,6 +550,8 @@
> >  		0x1 << 1)
> >  #define  IGU_REG_BLOCK_CONFIGURATION_PXP_TPH_INTERFACE_EN	( \
> >  		0x1 << 0)
> > +#define IGU_REG_PRODUCER_MEMORY 0x182000UL #define
> > +IGU_REG_CONSUMER_MEM 0x183000UL
> >  #define  IGU_REG_MAPPING_MEMORY \
> >  	0x184000UL
> >  #define IGU_REG_STATISTIC_NUM_VF_MSG_SENT \ diff --git
> > a/drivers/net/ethernet/qlogic/qede/qede_main.c
> > b/drivers/net/ethernet/qlogic/qede/qede_main.c
> > index 06c6a5813606..f37604da79e9 100644
> > --- a/drivers/net/ethernet/qlogic/qede/qede_main.c
> > +++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
> > @@ -509,34 +509,98 @@ static int qede_ioctl(struct net_device *dev, struct
> ifreq *ifr, int cmd)
> >  	return 0;
> >  }
> >
> > -static void qede_tx_log_print(struct qede_dev *edev, struct
> > qede_tx_queue *txq)
> > +static void qede_fp_sb_dump(struct qede_dev *edev, struct
> > +qede_fastpath *fp)
> >  {
> > +	char *p_sb = (char *)fp->sb_info->sb_virt;
> > +	u32 sb_size, i;
> > +
> > +	sb_size = sizeof(struct status_block);
> > +
> > +	for (i = 0; i < sb_size; i += 8) {
> > +		DP_NOTICE(edev,
> > +			  "%02hhX %02hhX %02hhX %02hhX  %02hhX %02hhX
> %02hhX %02hhX\n",
> > +			  p_sb[i], p_sb[i + 1], p_sb[i + 2], p_sb[i + 3],
> > +			  p_sb[i + 4], p_sb[i + 5], p_sb[i + 6], p_sb[i + 7]);
> > +	}
> > +}
> > +
> > +static void
> > +qede_txq_fp_log_metadata(struct qede_dev *edev,
> > +			 struct qede_fastpath *fp, struct qede_tx_queue *txq) {
> > +	struct qed_chain *p_chain = &txq->tx_pbl;
> > +
> > +	/* Dump txq/fp/sb ids etc. other metadata */
> >  	DP_NOTICE(edev,
> > -		  "Txq[%d]: FW cons [host] %04x, SW cons %04x, SW prod %04x
> [Jiffies %lu]\n",
> > -		  txq->index, le16_to_cpu(*txq->hw_cons_ptr),
> > -		  qed_chain_get_cons_idx(&txq->tx_pbl),
> > -		  qed_chain_get_prod_idx(&txq->tx_pbl),
> > -		  jiffies);
> > +		  "fpid 0x%x sbid 0x%x txqid [0x%x] ndev_qid [0x%x] cos [0x%x]
> p_chain %p cap %d size %d jiffies %lu HZ 0x%x\n",
> > +		  fp->id, fp->sb_info->igu_sb_id, txq->index, txq->ndev_txq_id,
> txq->cos,
> > +		  p_chain, p_chain->capacity, p_chain->size, jiffies, HZ);
> > +
> > +	/* Dump all the relevant prod/cons indexes */
> > +	DP_NOTICE(edev,
> > +		  "hw cons %04x sw_tx_prod=0x%x, sw_tx_cons=0x%x, bd_prod
> 0x%x bd_cons 0x%x\n",
> > +		  le16_to_cpu(*txq->hw_cons_ptr), txq->sw_tx_prod, txq-
> >sw_tx_cons,
> > +		  qed_chain_get_prod_idx(p_chain),
> > +qed_chain_get_cons_idx(p_chain)); }
> > +
> > +static void
> > +qede_tx_log_print(struct qede_dev *edev, struct qede_fastpath *fp,
> > +struct qede_tx_queue *txq) {
> > +	struct qed_sb_info_dbg sb_dbg;
> > +	int rc;
> > +
> > +	/* sb info */
> > +	qede_fp_sb_dump(edev, fp);
> > +
> > +	memset(&sb_dbg, 0, sizeof(sb_dbg));
> > +	rc = edev->ops->common->get_sb_info(edev->cdev, fp->sb_info,
> > +(u16)fp->id, &sb_dbg);
> > +
> > +	DP_NOTICE(edev, "IGU: prod %08x cons %08x CAU Tx %04x\n",
> > +		  sb_dbg.igu_prod, sb_dbg.igu_cons, sb_dbg.pi[TX_PI(txq-
> >cos)]);
> > +
> > +	/* report to mfw */
> > +	edev->ops->common->mfw_report(edev->cdev,
> > +				      "Txq[%d]: FW cons [host] %04x, SW cons
> %04x, SW prod %04x [Jiffies %lu]\n",
> > +				      txq->index, le16_to_cpu(*txq-
> >hw_cons_ptr),
> > +				      qed_chain_get_cons_idx(&txq->tx_pbl),
> > +				      qed_chain_get_prod_idx(&txq->tx_pbl),
> jiffies);
> > +	if (!rc) {
> > +		edev->ops->common->mfw_report(edev->cdev,
> > +					      "Txq[%d]: SB[0x%04x] - IGU: prod
> %08x cons %08x CAU Tx %04x\n",
> > +					      txq->index, fp->sb_info->igu_sb_id,
> > +					      sb_dbg.igu_prod, sb_dbg.igu_cons,
> > +					      sb_dbg.pi[TX_PI(txq->cos)]);
> > +	}
> >  }
> >
> >  static void qede_tx_timeout(struct net_device *dev, unsigned int
> > txqueue)  {
> >  	struct qede_dev *edev = netdev_priv(dev);
> > -	struct qede_tx_queue *txq;
> > -	int cos;
> > +	int i;
> >
> >  	netif_carrier_off(dev);
> >  	DP_NOTICE(edev, "TX timeout on queue %u!\n", txqueue);
> >
> > -	if (!(edev->fp_array[txqueue].type & QEDE_FASTPATH_TX))
> > -		return;
> > +	for_each_queue(i) {
> 
> Please only dump state for the queue that timed out.

Generally, for debugging purpose getting at least basic metadata info (prod/cons indexes etc.) for all queues is good
on such tx timeout event. Down the flow driver only dump more info (status block etc.) and report to firmware
for the queue which is stuck (prod != cons).

> 
> > +		struct qede_tx_queue *txq;
> > +		struct qede_fastpath *fp;
> > +		int cos;
> >
> > -	for_each_cos_in_txq(edev, cos) {
> > -		txq = &edev->fp_array[txqueue].txq[cos];
> > +		fp = &edev->fp_array[i];
> > +		if (!(fp->type & QEDE_FASTPATH_TX))
> > +			continue;
> > +
> > +		for_each_cos_in_txq(edev, cos) {
> > +			txq = &fp->txq[cos];
> >
> > -		if (qed_chain_get_cons_idx(&txq->tx_pbl) !=
> > -		    qed_chain_get_prod_idx(&txq->tx_pbl))
> > -			qede_tx_log_print(edev, txq);
> > +			/* Dump basic metadata for all queues */
> > +			qede_txq_fp_log_metadata(edev, fp, txq);
> > +
> > +			if (qed_chain_get_cons_idx(&txq->tx_pbl) !=
> > +			    qed_chain_get_prod_idx(&txq->tx_pbl)) {
> > +				qede_tx_log_print(edev, fp, txq);
> > +			}
> > +		}
> >  	}
> >
> >  	if (IS_VF(edev))
> > diff --git a/include/linux/qed/qed_if.h b/include/linux/qed/qed_if.h
> > index 0dae7fcc5ef2..9f4bfa2a4829 100644
> > --- a/include/linux/qed/qed_if.h
> > +++ b/include/linux/qed/qed_if.h
> > @@ -807,6 +807,12 @@ struct qed_devlink {
> >  	struct devlink_health_reporter *fw_reporter;  };
> >
> > +struct qed_sb_info_dbg {
> > +	u32 igu_prod;
> > +	u32 igu_cons;
> 
> This info gets populated by the device, right? It should probably use the correct
> endian types.

These are the values read from BAR address (readl). I think this should be fine.
 
> > +	u16 pi[PIS_PER_SB];
> > +};
> > +
> >  struct qed_common_cb_ops {
> >  	void (*arfs_filter_op)(void *dev, void *fltr, u8 fw_rc);
> >  	void (*link_update)(void *dev, struct qed_link_output *link);


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

* Re: [EXT] Re: [PATCH net-next 1/2] qed*: enhance tx timeout debug info
  2021-11-26 22:04     ` [EXT] " Manish Chopra
@ 2021-11-27  0:41       ` Jakub Kicinski
  0 siblings, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2021-11-27  0:41 UTC (permalink / raw)
  To: Manish Chopra; +Cc: netdev, Ariel Elior, Alok Prasad, Prabhakar Kushwaha

On Fri, 26 Nov 2021 22:04:03 +0000 Manish Chopra wrote:
> > Please consider using devlink health if you want to communicate more data to
> > the user  
> 
> It's not really that huge logs/data, these are just very basic metadata (few prints) about the TX queues logged to system logs.
> Those can be looked easily from dmesg/var-log-messages files which can be made available conveniently.
> Rest are the mailbox commands posted to management firmware with those basic information about the queues.

Right, I meant "more" as in if it keeps growing in the future, not
necessarily to replace this patch.

> > > +/**
> > > + * qed_int_get_sb_dbg: Read debug information regarding a given SB
> > > + *
> > > + * @p_hwfn: hw function pointer
> > > + * @p_ptt: ptt resource
> > > + * @p_sb: pointer to status block for which we want to get info
> > > + * @p_info: pointer to struct to fill with information regarding SB
> > > + *
> > > + * Return: Int  
> > 
> > What's the point of documenting the return type?  
> 
> For ./scripts/kernel-doc, I will put some suitable description. 

I don't think it requires documenting return value. All arguments -
yes, but not documenting return value is fine. So you can as well
remove it, up to you.

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

end of thread, other threads:[~2021-11-27  0:43 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-24  9:43 [PATCH net-next 0/2] qed*: enhancements Manish Chopra
2021-11-24  9:43 ` [PATCH net-next 1/2] qed*: enhance tx timeout debug info Manish Chopra
2021-11-25  2:51   ` Jakub Kicinski
2021-11-26 22:04     ` [EXT] " Manish Chopra
2021-11-27  0:41       ` Jakub Kicinski
2021-11-24  9:43 ` [PATCH net-next 2/2] qed*: esl priv flag support through ethtool Manish Chopra
2021-11-25  2:53   ` Jakub Kicinski

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