Add basic devlink and devlink health reporters.
Devlink health reporters are added for NPA and NIX blocks.
These reporters report the error count in respective blocks.
Address Jakub's comment to add devlink support for error reporting.
https://www.spinics.net/lists/netdev/msg670712.html
Change-log:
- Address Willem's comments on v1.
- Fixed the sparse issues, reported by Jakub.
George Cherian (3):
octeontx2-af: Add devlink suppoort to af driver
octeontx2-af: Add devlink health reporters for NPA
octeontx2-af: Add devlink health reporters for NIX
.../net/ethernet/marvell/octeontx2/Kconfig | 1 +
.../ethernet/marvell/octeontx2/af/Makefile | 3 +-
.../net/ethernet/marvell/octeontx2/af/rvu.c | 9 +-
.../net/ethernet/marvell/octeontx2/af/rvu.h | 4 +
.../marvell/octeontx2/af/rvu_devlink.c | 860 ++++++++++++++++++
.../marvell/octeontx2/af/rvu_devlink.h | 67 ++
.../marvell/octeontx2/af/rvu_struct.h | 33 +
7 files changed, 975 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
--
2.25.4
Add devlink support to AF driver. Basic devlink support is added.
Currently info_get is the only supported devlink ops.
devlink ouptput looks like this
# devlink dev
pci/0002:01:00.0
# devlink dev info
pci/0002:01:00.0:
driver octeontx2-af
versions:
fixed:
mbox version: 9
Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
Signed-off-by: Jerin Jacob <[email protected]>
Signed-off-by: George Cherian <[email protected]>
---
.../net/ethernet/marvell/octeontx2/Kconfig | 1 +
.../ethernet/marvell/octeontx2/af/Makefile | 3 +-
.../net/ethernet/marvell/octeontx2/af/rvu.c | 9 ++-
.../net/ethernet/marvell/octeontx2/af/rvu.h | 4 ++
.../marvell/octeontx2/af/rvu_devlink.c | 72 +++++++++++++++++++
.../marvell/octeontx2/af/rvu_devlink.h | 20 ++++++
6 files changed, 107 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
diff --git a/drivers/net/ethernet/marvell/octeontx2/Kconfig b/drivers/net/ethernet/marvell/octeontx2/Kconfig
index 543a1d047567..16caa02095fe 100644
--- a/drivers/net/ethernet/marvell/octeontx2/Kconfig
+++ b/drivers/net/ethernet/marvell/octeontx2/Kconfig
@@ -9,6 +9,7 @@ config OCTEONTX2_MBOX
config OCTEONTX2_AF
tristate "Marvell OcteonTX2 RVU Admin Function driver"
select OCTEONTX2_MBOX
+ select NET_DEVLINK
depends on (64BIT && COMPILE_TEST) || ARM64
depends on PCI
help
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/Makefile b/drivers/net/ethernet/marvell/octeontx2/af/Makefile
index 2f7a861d0c7b..20135f1d3387 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/Makefile
+++ b/drivers/net/ethernet/marvell/octeontx2/af/Makefile
@@ -9,4 +9,5 @@ obj-$(CONFIG_OCTEONTX2_AF) += octeontx2_af.o
octeontx2_mbox-y := mbox.o rvu_trace.o
octeontx2_af-y := cgx.o rvu.o rvu_cgx.o rvu_npa.o rvu_nix.o \
- rvu_reg.o rvu_npc.o rvu_debugfs.o ptp.o
+ rvu_reg.o rvu_npc.o rvu_debugfs.o ptp.o \
+ rvu_devlink.o
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
index f0ce2ec0993b..cfff7d3fb705 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
@@ -2816,17 +2816,23 @@ static int rvu_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (err)
goto err_flr;
+ err = rvu_register_dl(rvu);
+ if (err)
+ goto err_irq;
+
rvu_setup_rvum_blk_revid(rvu);
/* Enable AF's VFs (if any) */
err = rvu_enable_sriov(rvu);
if (err)
- goto err_irq;
+ goto err_dl;
/* Initialize debugfs */
rvu_dbg_init(rvu);
return 0;
+err_dl:
+ rvu_unregister_dl(rvu);
err_irq:
rvu_unregister_interrupts(rvu);
err_flr:
@@ -2858,6 +2864,7 @@ static void rvu_remove(struct pci_dev *pdev)
rvu_dbg_exit(rvu);
rvu_unregister_interrupts(rvu);
+ rvu_unregister_dl(rvu);
rvu_flr_wq_destroy(rvu);
rvu_cgx_exit(rvu);
rvu_fwdata_exit(rvu);
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
index 5ac9bb12415f..282566235918 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
@@ -12,7 +12,10 @@
#define RVU_H
#include <linux/pci.h>
+#include <net/devlink.h>
+
#include "rvu_struct.h"
+#include "rvu_devlink.h"
#include "common.h"
#include "mbox.h"
@@ -376,6 +379,7 @@ struct rvu {
#ifdef CONFIG_DEBUG_FS
struct rvu_debugfs rvu_dbg;
#endif
+ struct rvu_devlink *rvu_dl;
};
static inline void rvu_write64(struct rvu *rvu, u64 block, u64 offset, u64 val)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
new file mode 100644
index 000000000000..596bb9c533b5
--- /dev/null
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Marvell OcteonTx2 RVU Devlink
+ *
+ * Copyright (C) 2020 Marvell International Ltd.
+ *
+ */
+
+#include "rvu.h"
+
+#define DRV_NAME "octeontx2-af"
+
+static int rvu_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
+ struct netlink_ext_ack *extack)
+{
+ char buf[10];
+ int err;
+
+ err = devlink_info_driver_name_put(req, DRV_NAME);
+ if (err)
+ return err;
+
+ sprintf(buf, "%X", OTX2_MBOX_VERSION);
+ return devlink_info_version_fixed_put(req, "mbox version:", buf);
+}
+
+static const struct devlink_ops rvu_devlink_ops = {
+ .info_get = rvu_devlink_info_get,
+};
+
+int rvu_register_dl(struct rvu *rvu)
+{
+ struct rvu_devlink *rvu_dl;
+ struct devlink *dl;
+ int err;
+
+ rvu_dl = kzalloc(sizeof(*rvu_dl), GFP_KERNEL);
+ if (!rvu_dl)
+ return -ENOMEM;
+
+ dl = devlink_alloc(&rvu_devlink_ops, sizeof(struct rvu_devlink));
+ if (!dl) {
+ dev_warn(rvu->dev, "devlink_alloc failed\n");
+ kfree(rvu_dl);
+ return -ENOMEM;
+ }
+
+ err = devlink_register(dl, rvu->dev);
+ if (err) {
+ dev_err(rvu->dev, "devlink register failed with error %d\n", err);
+ devlink_free(dl);
+ kfree(rvu_dl);
+ return err;
+ }
+
+ rvu_dl->dl = dl;
+ rvu_dl->rvu = rvu;
+ rvu->rvu_dl = rvu_dl;
+ return 0;
+}
+
+void rvu_unregister_dl(struct rvu *rvu)
+{
+ struct rvu_devlink *rvu_dl = rvu->rvu_dl;
+ struct devlink *dl = rvu_dl->dl;
+
+ if (!dl)
+ return;
+
+ devlink_unregister(dl);
+ devlink_free(dl);
+ kfree(rvu_dl);
+}
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
new file mode 100644
index 000000000000..b0a0dfeb99c2
--- /dev/null
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Marvell OcteonTx2 RVU Devlink
+ *
+ * Copyright (C) 2020 Marvell International Ltd.
+ *
+ */
+
+#ifndef RVU_DEVLINK_H
+#define RVU_DEVLINK_H
+
+struct rvu_devlink {
+ struct devlink *dl;
+ struct rvu *rvu;
+};
+
+/* Devlink APIs */
+int rvu_register_dl(struct rvu *rvu);
+void rvu_unregister_dl(struct rvu *rvu);
+
+#endif /* RVU_DEVLINK_H */
--
2.25.4
Add health reporters for RVU NPA block.
Only reporter dump is supported
Output:
# devlink health
pci/0002:01:00.0:
reporter npa
state healthy error 0 recover 0
# devlink health dump show pci/0002:01:00.0 reporter npa
NPA_AF_GENERAL:
Unmap PF Error: 0
Free Disabled for NIX0 RX: 0
Free Disabled for NIX0 TX: 0
Free Disabled for NIX1 RX: 0
Free Disabled for NIX1 TX: 0
Free Disabled for SSO: 0
Free Disabled for TIM: 0
Free Disabled for DPI: 0
Free Disabled for AURA: 0
Alloc Disabled for Resvd: 0
NPA_AF_ERR:
Memory Fault on NPA_AQ_INST_S read: 0
Memory Fault on NPA_AQ_RES_S write: 0
AQ Doorbell Error: 0
Poisoned data on NPA_AQ_INST_S read: 0
Poisoned data on NPA_AQ_RES_S write: 0
Poisoned data on HW context read: 0
NPA_AF_RVU:
Unmap Slot Error: 0
Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
Signed-off-by: Jerin Jacob <[email protected]>
Signed-off-by: George Cherian <[email protected]>
---
.../marvell/octeontx2/af/rvu_devlink.c | 432 +++++++++++++++++-
.../marvell/octeontx2/af/rvu_devlink.h | 23 +
.../marvell/octeontx2/af/rvu_struct.h | 23 +
3 files changed, 477 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
index 596bb9c533b5..bf9efe1f6aec 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
@@ -5,10 +5,438 @@
*
*/
+#include<linux/bitfield.h>
+
#include "rvu.h"
+#include "rvu_reg.h"
+#include "rvu_struct.h"
#define DRV_NAME "octeontx2-af"
+static int rvu_report_pair_start(struct devlink_fmsg *fmsg, const char *name)
+{
+ int err;
+
+ err = devlink_fmsg_pair_nest_start(fmsg, name);
+ if (err)
+ return err;
+
+ return devlink_fmsg_obj_nest_start(fmsg);
+}
+
+static int rvu_report_pair_end(struct devlink_fmsg *fmsg)
+{
+ int err;
+
+ err = devlink_fmsg_obj_nest_end(fmsg);
+ if (err)
+ return err;
+
+ return devlink_fmsg_pair_nest_end(fmsg);
+}
+
+static bool rvu_common_request_irq(struct rvu *rvu, int offset,
+ const char *name, irq_handler_t fn)
+{
+ struct rvu_devlink *rvu_dl = rvu->rvu_dl;
+ int rc;
+
+ sprintf(&rvu->irq_name[offset * NAME_SIZE], name);
+ rc = request_irq(pci_irq_vector(rvu->pdev, offset), fn, 0,
+ &rvu->irq_name[offset * NAME_SIZE], rvu_dl);
+ if (rc)
+ dev_warn(rvu->dev, "Failed to register %s irq\n", name);
+ else
+ rvu->irq_allocated[offset] = true;
+
+ return rvu->irq_allocated[offset];
+}
+
+static irqreturn_t rvu_npa_af_rvu_intr_handler(int irq, void *rvu_irq)
+{
+ struct rvu_npa_event_cnt *npa_event_count;
+ struct rvu_devlink *rvu_dl = rvu_irq;
+ struct rvu *rvu;
+ int blkaddr;
+ u64 intr;
+
+ rvu = rvu_dl->rvu;
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPA, 0);
+ if (blkaddr < 0)
+ return IRQ_NONE;
+
+ npa_event_count = rvu_dl->npa_event_cnt;
+ intr = rvu_read64(rvu, blkaddr, NPA_AF_RVU_INT);
+
+ if (intr & BIT_ULL(0))
+ npa_event_count->unmap_slot_count++;
+ /* Clear interrupts */
+ rvu_write64(rvu, blkaddr, NPA_AF_RVU_INT, intr);
+ return IRQ_HANDLED;
+}
+
+static int rvu_npa_inpq_to_cnt(u16 in,
+ struct rvu_npa_event_cnt *npa_event_count)
+{
+ switch (in) {
+ case 0:
+ return 0;
+ case BIT(NPA_INPQ_NIX0_RX):
+ return npa_event_count->free_dis_nix0_rx_count++;
+ case BIT(NPA_INPQ_NIX0_TX):
+ return npa_event_count->free_dis_nix0_tx_count++;
+ case BIT(NPA_INPQ_NIX1_RX):
+ return npa_event_count->free_dis_nix1_rx_count++;
+ case BIT(NPA_INPQ_NIX1_TX):
+ return npa_event_count->free_dis_nix1_tx_count++;
+ case BIT(NPA_INPQ_SSO):
+ return npa_event_count->free_dis_sso_count++;
+ case BIT(NPA_INPQ_TIM):
+ return npa_event_count->free_dis_tim_count++;
+ case BIT(NPA_INPQ_DPI):
+ return npa_event_count->free_dis_dpi_count++;
+ case BIT(NPA_INPQ_AURA_OP):
+ return npa_event_count->free_dis_aura_count++;
+ case BIT(NPA_INPQ_INTERNAL_RSV):
+ return npa_event_count->free_dis_rsvd_count++;
+ }
+
+ return npa_event_count->alloc_dis_rsvd_count++;
+}
+
+static irqreturn_t rvu_npa_af_gen_intr_handler(int irq, void *rvu_irq)
+{
+ struct rvu_npa_event_cnt *npa_event_count;
+ struct rvu_devlink *rvu_dl = rvu_irq;
+ struct rvu *rvu;
+ int blkaddr, val;
+ u64 intr;
+
+ rvu = rvu_dl->rvu;
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPA, 0);
+ if (blkaddr < 0)
+ return IRQ_NONE;
+
+ npa_event_count = rvu_dl->npa_event_cnt;
+ intr = rvu_read64(rvu, blkaddr, NPA_AF_GEN_INT);
+
+ if (intr & BIT_ULL(32))
+ npa_event_count->unmap_pf_count++;
+
+ val = FIELD_GET(GENMASK(31, 16), intr);
+ rvu_npa_inpq_to_cnt(val, npa_event_count);
+
+ val = FIELD_GET(GENMASK(15, 0), intr);
+ rvu_npa_inpq_to_cnt(val, npa_event_count);
+
+ /* Clear interrupts */
+ rvu_write64(rvu, blkaddr, NPA_AF_GEN_INT, intr);
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t rvu_npa_af_err_intr_handler(int irq, void *rvu_irq)
+{
+ struct rvu_npa_event_cnt *npa_event_count;
+ struct rvu_devlink *rvu_dl = rvu_irq;
+ struct rvu *rvu;
+ int blkaddr;
+ u64 intr;
+
+ rvu = rvu_dl->rvu;
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPA, 0);
+ if (blkaddr < 0)
+ return IRQ_NONE;
+
+ npa_event_count = rvu_dl->npa_event_cnt;
+ intr = rvu_read64(rvu, blkaddr, NPA_AF_ERR_INT);
+
+ if (intr & BIT_ULL(14))
+ npa_event_count->aq_inst_count++;
+
+ if (intr & BIT_ULL(13))
+ npa_event_count->aq_res_count++;
+
+ if (intr & BIT_ULL(12))
+ npa_event_count->aq_db_count++;
+
+ /* Clear interrupts */
+ rvu_write64(rvu, blkaddr, NPA_AF_ERR_INT, intr);
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t rvu_npa_af_ras_intr_handler(int irq, void *rvu_irq)
+{
+ struct rvu_npa_event_cnt *npa_event_count;
+ struct rvu_devlink *rvu_dl = rvu_irq;
+ struct rvu *rvu;
+ int blkaddr;
+ u64 intr;
+
+ rvu = rvu_dl->rvu;
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPA, 0);
+ if (blkaddr < 0)
+ return IRQ_NONE;
+
+ npa_event_count = rvu_dl->npa_event_cnt;
+ intr = rvu_read64(rvu, blkaddr, NPA_AF_RAS);
+
+ if (intr & BIT_ULL(34))
+ npa_event_count->poison_aq_inst_count++;
+
+ if (intr & BIT_ULL(33))
+ npa_event_count->poison_aq_res_count++;
+
+ if (intr & BIT_ULL(32))
+ npa_event_count->poison_aq_cxt_count++;
+
+ /* Clear interrupts */
+ rvu_write64(rvu, blkaddr, NPA_AF_RAS, intr);
+ return IRQ_HANDLED;
+}
+
+static void rvu_npa_unregister_interrupts(struct rvu *rvu)
+{
+ struct rvu_devlink *rvu_dl = rvu->rvu_dl;
+ int i, offs, blkaddr;
+ u64 reg;
+
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPA, 0);
+ if (blkaddr < 0)
+ return;
+
+ reg = rvu_read64(rvu, blkaddr, NPA_PRIV_AF_INT_CFG);
+ offs = reg & 0x3FF;
+
+ rvu_write64(rvu, blkaddr, NPA_AF_RVU_INT_ENA_W1C, ~0ULL);
+ rvu_write64(rvu, blkaddr, NPA_AF_GEN_INT_ENA_W1C, ~0ULL);
+ rvu_write64(rvu, blkaddr, NPA_AF_ERR_INT_ENA_W1C, ~0ULL);
+ rvu_write64(rvu, blkaddr, NPA_AF_RAS_ENA_W1C, ~0ULL);
+
+ for (i = 0; i < NPA_AF_INT_VEC_CNT; i++)
+ if (rvu->irq_allocated[offs + i]) {
+ free_irq(pci_irq_vector(rvu->pdev, offs + i), rvu_dl);
+ rvu->irq_allocated[offs + i] = false;
+ }
+}
+
+static int rvu_npa_register_interrupts(struct rvu *rvu)
+{
+ int blkaddr, base;
+ bool rc;
+
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPA, 0);
+ if (blkaddr < 0)
+ return blkaddr;
+
+ /* Get NPA AF MSIX vectors offset. */
+ base = rvu_read64(rvu, blkaddr, NPA_PRIV_AF_INT_CFG) & 0x3ff;
+ if (!base) {
+ dev_warn(rvu->dev,
+ "Failed to get NPA_AF_INT vector offsets\n");
+ return 0;
+ }
+
+ /* Register and enable NPA_AF_RVU_INT interrupt */
+ rc = rvu_common_request_irq(rvu, base + NPA_AF_INT_VEC_RVU,
+ "NPA_AF_RVU_INT",
+ rvu_npa_af_rvu_intr_handler);
+ if (!rc)
+ goto err;
+ rvu_write64(rvu, blkaddr, NPA_AF_RVU_INT_ENA_W1S, ~0ULL);
+
+ /* Register and enable NPA_AF_GEN_INT interrupt */
+ rc = rvu_common_request_irq(rvu, base + NPA_AF_INT_VEC_GEN,
+ "NPA_AF_RVU_GEN",
+ rvu_npa_af_gen_intr_handler);
+ if (!rc)
+ goto err;
+ rvu_write64(rvu, blkaddr, NPA_AF_GEN_INT_ENA_W1S, ~0ULL);
+
+ /* Register and enable NPA_AF_ERR_INT interrupt */
+ rc = rvu_common_request_irq(rvu, base + NPA_AF_INT_VEC_AF_ERR,
+ "NPA_AF_ERR_INT",
+ rvu_npa_af_err_intr_handler);
+ if (!rc)
+ goto err;
+ rvu_write64(rvu, blkaddr, NPA_AF_ERR_INT_ENA_W1S, ~0ULL);
+
+ /* Register and enable NPA_AF_RAS interrupt */
+ rc = rvu_common_request_irq(rvu, base + NPA_AF_INT_VEC_POISON,
+ "NPA_AF_RAS",
+ rvu_npa_af_ras_intr_handler);
+ if (!rc)
+ goto err;
+ rvu_write64(rvu, blkaddr, NPA_AF_RAS_ENA_W1S, ~0ULL);
+
+ return 0;
+err:
+ rvu_npa_unregister_interrupts(rvu);
+ return rc;
+}
+
+static int rvu_npa_report_show(struct devlink_fmsg *fmsg, struct rvu *rvu)
+{
+ struct rvu_npa_event_cnt *npa_event_count;
+ struct rvu_devlink *rvu_dl = rvu->rvu_dl;
+ int err;
+
+ npa_event_count = rvu_dl->npa_event_cnt;
+ err = rvu_report_pair_start(fmsg, "NPA_AF_GENERAL");
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\tUnmap PF Error",
+ npa_event_count->unmap_pf_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tFree Disabled for NIX0 RX",
+ npa_event_count->free_dis_nix0_rx_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tFree Disabled for NIX0 TX",
+ npa_event_count->free_dis_nix0_tx_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tFree Disabled for NIX1 RX",
+ npa_event_count->free_dis_nix1_rx_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tFree Disabled for NIX1 TX",
+ npa_event_count->free_dis_nix1_tx_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tFree Disabled for SSO",
+ npa_event_count->free_dis_sso_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tFree Disabled for TIM",
+ npa_event_count->free_dis_tim_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tFree Disabled for DPI",
+ npa_event_count->free_dis_dpi_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tFree Disabled for AURA",
+ npa_event_count->free_dis_aura_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tAlloc Disabled for Resvd",
+ npa_event_count->alloc_dis_rsvd_count);
+ if (err)
+ return err;
+ err = rvu_report_pair_end(fmsg);
+ if (err)
+ return err;
+ err = rvu_report_pair_start(fmsg, "NPA_AF_ERR");
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\tMemory Fault on NPA_AQ_INST_S read",
+ npa_event_count->aq_inst_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tMemory Fault on NPA_AQ_RES_S write",
+ npa_event_count->aq_res_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tAQ Doorbell Error",
+ npa_event_count->aq_db_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on NPA_AQ_INST_S read",
+ npa_event_count->poison_aq_inst_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on NPA_AQ_RES_S write",
+ npa_event_count->poison_aq_res_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on HW context read",
+ npa_event_count->poison_aq_cxt_count);
+ if (err)
+ return err;
+ err = rvu_report_pair_end(fmsg);
+ if (err)
+ return err;
+ err = rvu_report_pair_start(fmsg, "NPA_AF_RVU");
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\tUnmap Slot Error",
+ npa_event_count->unmap_slot_count);
+ if (err)
+ return err;
+ return rvu_report_pair_end(fmsg);
+}
+
+static int rvu_npa_reporter_dump(struct devlink_health_reporter *reporter,
+ struct devlink_fmsg *fmsg, void *ctx,
+ struct netlink_ext_ack *netlink_extack)
+{
+ struct rvu *rvu = devlink_health_reporter_priv(reporter);
+
+ return rvu_npa_report_show(fmsg, rvu);
+}
+
+static const struct devlink_health_reporter_ops rvu_npa_hw_fault_reporter_ops = {
+ .name = "npa",
+ .dump = rvu_npa_reporter_dump,
+};
+
+static int rvu_npa_health_reporters_create(struct rvu_devlink *rvu_dl)
+{
+ struct devlink_health_reporter *rvu_npa_health_reporter;
+ struct rvu_npa_event_cnt *npa_event_count;
+ struct rvu *rvu = rvu_dl->rvu;
+
+ npa_event_count = kzalloc(sizeof(*npa_event_count), GFP_KERNEL);
+ if (!npa_event_count)
+ return -ENOMEM;
+
+ rvu_dl->npa_event_cnt = npa_event_count;
+ rvu_npa_health_reporter = devlink_health_reporter_create(rvu_dl->dl,
+ &rvu_npa_hw_fault_reporter_ops,
+ 0, rvu);
+ if (IS_ERR(rvu_npa_health_reporter)) {
+ dev_warn(rvu->dev, "Failed to create npa reporter, err =%ld\n",
+ PTR_ERR(rvu_npa_health_reporter));
+ return PTR_ERR(rvu_npa_health_reporter);
+ }
+
+ rvu_dl->rvu_npa_health_reporter = rvu_npa_health_reporter;
+ rvu_npa_register_interrupts(rvu);
+
+ return 0;
+}
+
+static void rvu_npa_health_reporters_destroy(struct rvu_devlink *rvu_dl)
+{
+ struct rvu *rvu = rvu_dl->rvu;
+
+ if (!rvu_dl->rvu_npa_health_reporter)
+ return;
+
+ devlink_health_reporter_destroy(rvu_dl->rvu_npa_health_reporter);
+ rvu_npa_unregister_interrupts(rvu);
+}
+
+static int rvu_health_reporters_create(struct rvu *rvu)
+{
+ struct rvu_devlink *rvu_dl;
+
+ rvu_dl = rvu->rvu_dl;
+ return rvu_npa_health_reporters_create(rvu_dl);
+}
+
+static void rvu_health_reporters_destroy(struct rvu *rvu)
+{
+ struct rvu_devlink *rvu_dl;
+
+ if (!rvu->rvu_dl)
+ return;
+
+ rvu_dl = rvu->rvu_dl;
+ rvu_npa_health_reporters_destroy(rvu_dl);
+}
+
static int rvu_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
struct netlink_ext_ack *extack)
{
@@ -55,7 +483,8 @@ int rvu_register_dl(struct rvu *rvu)
rvu_dl->dl = dl;
rvu_dl->rvu = rvu;
rvu->rvu_dl = rvu_dl;
- return 0;
+
+ return rvu_health_reporters_create(rvu);
}
void rvu_unregister_dl(struct rvu *rvu)
@@ -66,6 +495,7 @@ void rvu_unregister_dl(struct rvu *rvu)
if (!dl)
return;
+ rvu_health_reporters_destroy(rvu);
devlink_unregister(dl);
devlink_free(dl);
kfree(rvu_dl);
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
index b0a0dfeb99c2..b3ce1a8fff57 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
@@ -8,9 +8,32 @@
#ifndef RVU_DEVLINK_H
#define RVU_DEVLINK_H
+struct rvu_npa_event_cnt {
+ unsigned long unmap_slot_count;
+ unsigned long unmap_pf_count;
+ unsigned long free_dis_nix0_rx_count;
+ unsigned long free_dis_nix0_tx_count;
+ unsigned long free_dis_nix1_rx_count;
+ unsigned long free_dis_nix1_tx_count;
+ unsigned long free_dis_sso_count;
+ unsigned long free_dis_tim_count;
+ unsigned long free_dis_dpi_count;
+ unsigned long free_dis_aura_count;
+ unsigned long free_dis_rsvd_count;
+ unsigned long alloc_dis_rsvd_count;
+ unsigned long aq_inst_count;
+ unsigned long aq_res_count;
+ unsigned long aq_db_count;
+ unsigned long poison_aq_inst_count;
+ unsigned long poison_aq_res_count;
+ unsigned long poison_aq_cxt_count;
+};
+
struct rvu_devlink {
struct devlink *dl;
struct rvu *rvu;
+ struct devlink_health_reporter *rvu_npa_health_reporter;
+ struct rvu_npa_event_cnt *npa_event_cnt;
};
/* Devlink APIs */
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h
index 9a7eb074cdc2..995add5d8bff 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h
@@ -64,6 +64,16 @@ enum rvu_af_int_vec_e {
RVU_AF_INT_VEC_CNT = 0x5,
};
+/* NPA Admin function Interrupt Vector Enumeration */
+enum npa_af_int_vec_e {
+ NPA_AF_INT_VEC_RVU = 0x0,
+ NPA_AF_INT_VEC_GEN = 0x1,
+ NPA_AF_INT_VEC_AQ_DONE = 0x2,
+ NPA_AF_INT_VEC_AF_ERR = 0x3,
+ NPA_AF_INT_VEC_POISON = 0x4,
+ NPA_AF_INT_VEC_CNT = 0x5,
+};
+
/**
* RVU PF Interrupt Vector Enumeration
*/
@@ -104,6 +114,19 @@ enum npa_aq_instop {
NPA_AQ_INSTOP_UNLOCK = 0x5,
};
+/* ALLOC/FREE input queues Enumeration from coprocessors */
+enum npa_inpq {
+ NPA_INPQ_NIX0_RX = 0x0,
+ NPA_INPQ_NIX0_TX = 0x1,
+ NPA_INPQ_NIX1_RX = 0x2,
+ NPA_INPQ_NIX1_TX = 0x3,
+ NPA_INPQ_SSO = 0x4,
+ NPA_INPQ_TIM = 0x5,
+ NPA_INPQ_DPI = 0x6,
+ NPA_INPQ_AURA_OP = 0xe,
+ NPA_INPQ_INTERNAL_RSV = 0xf,
+};
+
/* NPA admin queue instruction structure */
struct npa_aq_inst_s {
#if defined(__BIG_ENDIAN_BITFIELD)
--
2.25.4
Add health reporters for RVU NPA block.
Only reporter dump is supported.
Output:
# ./devlink health
pci/0002:01:00.0:
reporter npa
state healthy error 0 recover 0
reporter nix
state healthy error 0 recover 0
# ./devlink health dump show pci/0002:01:00.0 reporter nix
NIX_AF_GENERAL:
Memory Fault on NIX_AQ_INST_S read: 0
Memory Fault on NIX_AQ_RES_S write: 0
AQ Doorbell error: 0
Rx on unmapped PF_FUNC: 0
Rx multicast replication error: 0
Memory fault on NIX_RX_MCE_S read: 0
Memory fault on multicast WQE read: 0
Memory fault on mirror WQE read: 0
Memory fault on mirror pkt write: 0
Memory fault on multicast pkt write: 0
NIX_AF_RAS:
Poisoned data on NIX_AQ_INST_S read: 0
Poisoned data on NIX_AQ_RES_S write: 0
Poisoned data on HW context read: 0
Poisoned data on packet read from mirror buffer: 0
Poisoned data on packet read from mcast buffer: 0
Poisoned data on WQE read from mirror buffer: 0
Poisoned data on WQE read from multicast buffer: 0
Poisoned data on NIX_RX_MCE_S read: 0
NIX_AF_RVU:
Unmap Slot Error: 0
Signed-off-by: Sunil Kovvuri Goutham <[email protected]>
Signed-off-by: Jerin Jacob <[email protected]>
Signed-off-by: George Cherian <[email protected]>
---
.../marvell/octeontx2/af/rvu_devlink.c | 360 +++++++++++++++++-
.../marvell/octeontx2/af/rvu_devlink.h | 24 ++
.../marvell/octeontx2/af/rvu_struct.h | 10 +
3 files changed, 393 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
index bf9efe1f6aec..49e51d1bd7d5 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
@@ -35,6 +35,110 @@ static int rvu_report_pair_end(struct devlink_fmsg *fmsg)
return devlink_fmsg_pair_nest_end(fmsg);
}
+static irqreturn_t rvu_nix_af_rvu_intr_handler(int irq, void *rvu_irq)
+{
+ struct rvu_nix_event_cnt *nix_event_count;
+ struct rvu_devlink *rvu_dl = rvu_irq;
+ struct rvu *rvu;
+ int blkaddr;
+ u64 intr;
+
+ rvu = rvu_dl->rvu;
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, 0);
+ if (blkaddr < 0)
+ return IRQ_NONE;
+
+ nix_event_count = rvu_dl->nix_event_cnt;
+ intr = rvu_read64(rvu, blkaddr, NIX_AF_RVU_INT);
+
+ if (intr & BIT_ULL(0))
+ nix_event_count->unmap_slot_count++;
+
+ /* Clear interrupts */
+ rvu_write64(rvu, blkaddr, NIX_AF_RVU_INT, intr);
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t rvu_nix_af_err_intr_handler(int irq, void *rvu_irq)
+{
+ struct rvu_nix_event_cnt *nix_event_count;
+ struct rvu_devlink *rvu_dl = rvu_irq;
+ struct rvu *rvu;
+ int blkaddr;
+ u64 intr;
+
+ rvu = rvu_dl->rvu;
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, 0);
+ if (blkaddr < 0)
+ return IRQ_NONE;
+
+ nix_event_count = rvu_dl->nix_event_cnt;
+ intr = rvu_read64(rvu, blkaddr, NIX_AF_ERR_INT);
+
+ if (intr & BIT_ULL(14))
+ nix_event_count->aq_inst_count++;
+ if (intr & BIT_ULL(13))
+ nix_event_count->aq_res_count++;
+ if (intr & BIT_ULL(12))
+ nix_event_count->aq_db_count++;
+ if (intr & BIT_ULL(6))
+ nix_event_count->rx_on_unmap_pf_count++;
+ if (intr & BIT_ULL(5))
+ nix_event_count->rx_mcast_repl_count++;
+ if (intr & BIT_ULL(4))
+ nix_event_count->rx_mcast_memfault_count++;
+ if (intr & BIT_ULL(3))
+ nix_event_count->rx_mcast_wqe_memfault_count++;
+ if (intr & BIT_ULL(2))
+ nix_event_count->rx_mirror_wqe_memfault_count++;
+ if (intr & BIT_ULL(1))
+ nix_event_count->rx_mirror_pktw_memfault_count++;
+ if (intr & BIT_ULL(0))
+ nix_event_count->rx_mcast_pktw_memfault_count++;
+
+ /* Clear interrupts */
+ rvu_write64(rvu, blkaddr, NIX_AF_ERR_INT, intr);
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t rvu_nix_af_ras_intr_handler(int irq, void *rvu_irq)
+{
+ struct rvu_nix_event_cnt *nix_event_count;
+ struct rvu_devlink *rvu_dl = rvu_irq;
+ struct rvu *rvu;
+ int blkaddr;
+ u64 intr;
+
+ rvu = rvu_dl->rvu;
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, 0);
+ if (blkaddr < 0)
+ return IRQ_NONE;
+
+ nix_event_count = rvu_dl->nix_event_cnt;
+ intr = rvu_read64(rvu, blkaddr, NIX_AF_RAS);
+
+ if (intr & BIT_ULL(34))
+ nix_event_count->poison_aq_inst_count++;
+ if (intr & BIT_ULL(33))
+ nix_event_count->poison_aq_res_count++;
+ if (intr & BIT_ULL(32))
+ nix_event_count->poison_aq_cxt_count++;
+ if (intr & BIT_ULL(4))
+ nix_event_count->rx_mirror_data_poison_count++;
+ if (intr & BIT_ULL(3))
+ nix_event_count->rx_mcast_data_poison_count++;
+ if (intr & BIT_ULL(2))
+ nix_event_count->rx_mirror_wqe_poison_count++;
+ if (intr & BIT_ULL(1))
+ nix_event_count->rx_mcast_wqe_poison_count++;
+ if (intr & BIT_ULL(0))
+ nix_event_count->rx_mce_poison_count++;
+
+ /* Clear interrupts */
+ rvu_write64(rvu, blkaddr, NIX_AF_RAS, intr);
+ return IRQ_HANDLED;
+}
+
static bool rvu_common_request_irq(struct rvu *rvu, int offset,
const char *name, irq_handler_t fn)
{
@@ -52,6 +156,254 @@ static bool rvu_common_request_irq(struct rvu *rvu, int offset,
return rvu->irq_allocated[offset];
}
+static void rvu_nix_blk_unregister_interrupts(struct rvu *rvu,
+ int blkaddr)
+{
+ struct rvu_devlink *rvu_dl = rvu->rvu_dl;
+ int offs, i;
+
+ offs = rvu_read64(rvu, blkaddr, NIX_PRIV_AF_INT_CFG) & 0x3ff;
+ if (!offs)
+ return;
+
+ rvu_write64(rvu, blkaddr, NIX_AF_RVU_INT_ENA_W1C, ~0ULL);
+ rvu_write64(rvu, blkaddr, NIX_AF_ERR_INT_ENA_W1C, ~0ULL);
+ rvu_write64(rvu, blkaddr, NIX_AF_RAS_ENA_W1C, ~0ULL);
+
+ if (rvu->irq_allocated[offs + NIX_AF_INT_VEC_RVU]) {
+ free_irq(pci_irq_vector(rvu->pdev, offs + NIX_AF_INT_VEC_RVU),
+ rvu_dl);
+ rvu->irq_allocated[offs + NIX_AF_INT_VEC_RVU] = false;
+ }
+
+ for (i = NIX_AF_INT_VEC_AF_ERR; i < NIX_AF_INT_VEC_CNT; i++)
+ if (rvu->irq_allocated[offs + i]) {
+ free_irq(pci_irq_vector(rvu->pdev, offs + i), rvu_dl);
+ rvu->irq_allocated[offs + i] = false;
+ }
+}
+
+static void rvu_nix_unregister_interrupts(struct rvu *rvu)
+{
+ int blkaddr = 0;
+
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, 0);
+ if (blkaddr < 0)
+ return;
+
+ rvu_nix_blk_unregister_interrupts(rvu, blkaddr);
+}
+
+static int rvu_nix_blk_register_interrupts(struct rvu *rvu,
+ int blkaddr)
+{
+ int base;
+ bool rc;
+
+ /* Get NIX AF MSIX vectors offset. */
+ base = rvu_read64(rvu, blkaddr, NIX_PRIV_AF_INT_CFG) & 0x3ff;
+ if (!base) {
+ dev_warn(rvu->dev,
+ "Failed to get NIX%d NIX_AF_INT vector offsets\n",
+ blkaddr - BLKADDR_NIX0);
+ return 0;
+ }
+ /* Register and enable NIX_AF_RVU_INT interrupt */
+ rc = rvu_common_request_irq(rvu, base + NIX_AF_INT_VEC_RVU,
+ "NIX_AF_RVU_INT",
+ rvu_nix_af_rvu_intr_handler);
+ if (!rc)
+ goto err;
+ rvu_write64(rvu, blkaddr, NIX_AF_RVU_INT_ENA_W1S, ~0ULL);
+
+ /* Register and enable NIX_AF_ERR_INT interrupt */
+ rc = rvu_common_request_irq(rvu, base + NIX_AF_INT_VEC_AF_ERR,
+ "NIX_AF_ERR_INT",
+ rvu_nix_af_err_intr_handler);
+ if (!rc)
+ goto err;
+ rvu_write64(rvu, blkaddr, NIX_AF_ERR_INT_ENA_W1S, ~0ULL);
+
+ /* Register and enable NIX_AF_RAS interrupt */
+ rc = rvu_common_request_irq(rvu, base + NIX_AF_INT_VEC_POISON,
+ "NIX_AF_RAS",
+ rvu_nix_af_ras_intr_handler);
+ if (!rc)
+ goto err;
+ rvu_write64(rvu, blkaddr, NIX_AF_RAS_ENA_W1S, ~0ULL);
+
+ return 0;
+err:
+ rvu_nix_unregister_interrupts(rvu);
+ return -1;
+}
+
+static int rvu_nix_register_interrupts(struct rvu *rvu)
+{
+ int blkaddr = 0;
+
+ blkaddr = rvu_get_blkaddr(rvu, blkaddr, 0);
+ if (blkaddr < 0)
+ return blkaddr;
+
+ rvu_nix_blk_register_interrupts(rvu, blkaddr);
+
+ return 0;
+}
+
+static int rvu_nix_report_show(struct devlink_fmsg *fmsg, struct rvu *rvu)
+{
+ struct rvu_devlink *rvu_dl = rvu->rvu_dl;
+ struct rvu_nix_event_cnt *nix_event_count = rvu_dl->nix_event_cnt;
+ int err;
+
+ err = rvu_report_pair_start(fmsg, "NIX_AF_GENERAL");
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\tMemory Fault on NIX_AQ_INST_S read",
+ nix_event_count->aq_inst_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tMemory Fault on NIX_AQ_RES_S write",
+ nix_event_count->aq_res_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tAQ Doorbell error",
+ nix_event_count->aq_db_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tRx on unmapped PF_FUNC",
+ nix_event_count->rx_on_unmap_pf_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tRx multicast replication error",
+ nix_event_count->rx_mcast_repl_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tMemory fault on NIX_RX_MCE_S read",
+ nix_event_count->rx_mcast_memfault_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tMemory fault on multicast WQE read",
+ nix_event_count->rx_mcast_wqe_memfault_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tMemory fault on mirror WQE read",
+ nix_event_count->rx_mirror_wqe_memfault_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tMemory fault on mirror pkt write",
+ nix_event_count->rx_mirror_pktw_memfault_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tMemory fault on multicast pkt write",
+ nix_event_count->rx_mcast_pktw_memfault_count);
+ if (err)
+ return err;
+ err = rvu_report_pair_end(fmsg);
+ if (err)
+ return err;
+ err = rvu_report_pair_start(fmsg, "NIX_AF_RAS");
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\tPoisoned data on NIX_AQ_INST_S read",
+ nix_event_count->poison_aq_inst_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on NIX_AQ_RES_S write",
+ nix_event_count->poison_aq_res_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on HW context read",
+ nix_event_count->poison_aq_cxt_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on packet read from mirror buffer",
+ nix_event_count->rx_mirror_data_poison_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on packet read from mcast buffer",
+ nix_event_count->rx_mcast_data_poison_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on WQE read from mirror buffer",
+ nix_event_count->rx_mirror_wqe_poison_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on WQE read from multicast buffer",
+ nix_event_count->rx_mcast_wqe_poison_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on NIX_RX_MCE_S read",
+ nix_event_count->rx_mce_poison_count);
+ if (err)
+ return err;
+ err = rvu_report_pair_end(fmsg);
+ if (err)
+ return err;
+ err = rvu_report_pair_start(fmsg, "NIX_AF_RVU");
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\tUnmap Slot Error",
+ nix_event_count->unmap_slot_count);
+ if (err)
+ return err;
+ err = rvu_report_pair_end(fmsg);
+ if (err)
+ return err;
+ return 0;
+}
+
+static int rvu_nix_reporter_dump(struct devlink_health_reporter *reporter,
+ struct devlink_fmsg *fmsg, void *ctx,
+ struct netlink_ext_ack *netlink_extack)
+{
+ struct rvu *rvu = devlink_health_reporter_priv(reporter);
+
+ return rvu_nix_report_show(fmsg, rvu);
+}
+
+static const struct devlink_health_reporter_ops rvu_nix_fault_reporter_ops = {
+ .name = "nix",
+ .dump = rvu_nix_reporter_dump,
+};
+
+static int rvu_nix_health_reporters_create(struct rvu_devlink *rvu_dl)
+{
+ struct devlink_health_reporter *rvu_nix_health_reporter;
+ struct rvu_nix_event_cnt *nix_event_count;
+ struct rvu *rvu = rvu_dl->rvu;
+
+ nix_event_count = kzalloc(sizeof(*nix_event_count), GFP_KERNEL);
+ if (!nix_event_count)
+ return -ENOMEM;
+
+ rvu_dl->nix_event_cnt = nix_event_count;
+ rvu_nix_health_reporter = devlink_health_reporter_create(rvu_dl->dl,
+ &rvu_nix_fault_reporter_ops,
+ 0, rvu);
+ if (IS_ERR(rvu_nix_health_reporter)) {
+ dev_warn(rvu->dev, "Failed to create nix reporter, err = %ld\n",
+ PTR_ERR(rvu_nix_health_reporter));
+ return PTR_ERR(rvu_nix_health_reporter);
+ }
+
+ rvu_dl->rvu_nix_health_reporter = rvu_nix_health_reporter;
+ rvu_nix_register_interrupts(rvu);
+ return 0;
+}
+
+static void rvu_nix_health_reporters_destroy(struct rvu_devlink *rvu_dl)
+{
+ struct rvu *rvu = rvu_dl->rvu;
+
+ if (!rvu_dl->rvu_nix_health_reporter)
+ return;
+
+ devlink_health_reporter_destroy(rvu_dl->rvu_nix_health_reporter);
+ rvu_nix_unregister_interrupts(rvu);
+}
+
static irqreturn_t rvu_npa_af_rvu_intr_handler(int irq, void *rvu_irq)
{
struct rvu_npa_event_cnt *npa_event_count;
@@ -421,9 +773,14 @@ static void rvu_npa_health_reporters_destroy(struct rvu_devlink *rvu_dl)
static int rvu_health_reporters_create(struct rvu *rvu)
{
struct rvu_devlink *rvu_dl;
+ int err;
rvu_dl = rvu->rvu_dl;
- return rvu_npa_health_reporters_create(rvu_dl);
+ err = rvu_npa_health_reporters_create(rvu_dl);
+ if (err)
+ return err;
+
+ return rvu_nix_health_reporters_create(rvu_dl);
}
static void rvu_health_reporters_destroy(struct rvu *rvu)
@@ -435,6 +792,7 @@ static void rvu_health_reporters_destroy(struct rvu *rvu)
rvu_dl = rvu->rvu_dl;
rvu_npa_health_reporters_destroy(rvu_dl);
+ rvu_nix_health_reporters_destroy(rvu_dl);
}
static int rvu_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
index b3ce1a8fff57..15724ad2ed44 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
@@ -29,11 +29,35 @@ struct rvu_npa_event_cnt {
unsigned long poison_aq_cxt_count;
};
+struct rvu_nix_event_cnt {
+ unsigned long unmap_slot_count;
+ unsigned long aq_inst_count;
+ unsigned long aq_res_count;
+ unsigned long aq_db_count;
+ unsigned long rx_on_unmap_pf_count;
+ unsigned long rx_mcast_repl_count;
+ unsigned long rx_mcast_memfault_count;
+ unsigned long rx_mcast_wqe_memfault_count;
+ unsigned long rx_mirror_wqe_memfault_count;
+ unsigned long rx_mirror_pktw_memfault_count;
+ unsigned long rx_mcast_pktw_memfault_count;
+ unsigned long poison_aq_inst_count;
+ unsigned long poison_aq_res_count;
+ unsigned long poison_aq_cxt_count;
+ unsigned long rx_mirror_data_poison_count;
+ unsigned long rx_mcast_data_poison_count;
+ unsigned long rx_mirror_wqe_poison_count;
+ unsigned long rx_mcast_wqe_poison_count;
+ unsigned long rx_mce_poison_count;
+};
+
struct rvu_devlink {
struct devlink *dl;
struct rvu *rvu;
struct devlink_health_reporter *rvu_npa_health_reporter;
struct rvu_npa_event_cnt *npa_event_cnt;
+ struct devlink_health_reporter *rvu_nix_health_reporter;
+ struct rvu_nix_event_cnt *nix_event_cnt;
};
/* Devlink APIs */
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h
index 995add5d8bff..b5944199faf5 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h
@@ -74,6 +74,16 @@ enum npa_af_int_vec_e {
NPA_AF_INT_VEC_CNT = 0x5,
};
+/* NIX Admin function Interrupt Vector Enumeration */
+enum nix_af_int_vec_e {
+ NIX_AF_INT_VEC_RVU = 0x0,
+ NIX_AF_INT_VEC_GEN = 0x1,
+ NIX_AF_INT_VEC_AQ_DONE = 0x2,
+ NIX_AF_INT_VEC_AF_ERR = 0x3,
+ NIX_AF_INT_VEC_POISON = 0x4,
+ NIX_AF_INT_VEC_CNT = 0x5,
+};
+
/**
* RVU PF Interrupt Vector Enumeration
*/
--
2.25.4
On Wed, 2020-11-04 at 17:57 +0530, George Cherian wrote:
> Add health reporters for RVU NPA block.
^^^ NIX ?
Cc: Jiri
Anyway, could you please spare some words on what is NPA and what is
NIX?
Regarding the reporters names, all drivers register well known generic
names such as (fw,hw,rx,tx), I don't know if it is a good idea to use
vendor specific names, if you are reporting for hw/fw units then just
use "hw" or "fw" as the reporter name and append the unit NPA/NIX to
the counter/error names.
> Only reporter dump is supported.
>
> Output:
> # ./devlink health
> pci/0002:01:00.0:
> reporter npa
> state healthy error 0 recover 0
> reporter nix
> state healthy error 0 recover 0
> # ./devlink health dump show pci/0002:01:00.0 reporter nix
> NIX_AF_GENERAL:
> Memory Fault on NIX_AQ_INST_S read: 0
> Memory Fault on NIX_AQ_RES_S write: 0
> AQ Doorbell error: 0
> Rx on unmapped PF_FUNC: 0
> Rx multicast replication error: 0
> Memory fault on NIX_RX_MCE_S read: 0
> Memory fault on multicast WQE read: 0
> Memory fault on mirror WQE read: 0
> Memory fault on mirror pkt write: 0
> Memory fault on multicast pkt write: 0
> NIX_AF_RAS:
> Poisoned data on NIX_AQ_INST_S read: 0
> Poisoned data on NIX_AQ_RES_S write: 0
> Poisoned data on HW context read: 0
> Poisoned data on packet read from mirror buffer: 0
> Poisoned data on packet read from mcast buffer: 0
> Poisoned data on WQE read from mirror buffer: 0
> Poisoned data on WQE read from multicast buffer: 0
> Poisoned data on NIX_RX_MCE_S read: 0
> NIX_AF_RVU:
> Unmap Slot Error: 0
>
Now i am a little bit skeptic here, devlink health reporter
infrastructure was never meant to deal with dump op only, the main
purpose is to diagnose/dump and recover.
especially in your use case where you only report counters, i don't
believe devlink health dump is a proper interface for this.
Many of these counters if not most are data path packet based and maybe
they should belong to ethtool.
Hi Saeed,
Thanks for the review.
> -----Original Message-----
> From: Saeed Mahameed <[email protected]>
> Sent: Thursday, November 5, 2020 10:39 AM
> To: George Cherian <[email protected]>; [email protected];
> [email protected]; Jiri Pirko <[email protected]>
> Cc: [email protected]; [email protected]; Sunil Kovvuri Goutham
> <[email protected]>; Linu Cherian <[email protected]>;
> Geethasowjanya Akula <[email protected]>; [email protected];
> [email protected]
> Subject: Re: [PATCH v2 net-next 3/3] octeontx2-af: Add devlink health
> reporters for NIX
>
> On Wed, 2020-11-04 at 17:57 +0530, George Cherian wrote:
> > Add health reporters for RVU NPA block.
> ^^^ NIX ?
>
Yes, it's NIX.
> Cc: Jiri
>
> Anyway, could you please spare some words on what is NPA and what is
> NIX?
>
> Regarding the reporters names, all drivers register well known generic names
> such as (fw,hw,rx,tx), I don't know if it is a good idea to use vendor specific
> names, if you are reporting for hw/fw units then just use "hw" or "fw" as the
> reporter name and append the unit NPA/NIX to the counter/error names.
Okay. These are hw units, I will rename them as hw_npa/hw_nix.
>
> > Only reporter dump is supported.
> >
> > Output:
> > # ./devlink health
> > pci/0002:01:00.0:
> > reporter npa
> > state healthy error 0 recover 0
> > reporter nix
> > state healthy error 0 recover 0
> > # ./devlink health dump show pci/0002:01:00.0 reporter nix
> > NIX_AF_GENERAL:
> > Memory Fault on NIX_AQ_INST_S read: 0
> > Memory Fault on NIX_AQ_RES_S write: 0
> > AQ Doorbell error: 0
> > Rx on unmapped PF_FUNC: 0
> > Rx multicast replication error: 0
> > Memory fault on NIX_RX_MCE_S read: 0
> > Memory fault on multicast WQE read: 0
> > Memory fault on mirror WQE read: 0
> > Memory fault on mirror pkt write: 0
> > Memory fault on multicast pkt write: 0
> > NIX_AF_RAS:
> > Poisoned data on NIX_AQ_INST_S read: 0
> > Poisoned data on NIX_AQ_RES_S write: 0
> > Poisoned data on HW context read: 0
> > Poisoned data on packet read from mirror buffer: 0
> > Poisoned data on packet read from mcast buffer: 0
> > Poisoned data on WQE read from mirror buffer: 0
> > Poisoned data on WQE read from multicast buffer: 0
> > Poisoned data on NIX_RX_MCE_S read: 0
> > NIX_AF_RVU:
> > Unmap Slot Error: 0
> >
>
> Now i am a little bit skeptic here, devlink health reporter infrastructure was
> never meant to deal with dump op only, the main purpose is to
> diagnose/dump and recover.
>
> especially in your use case where you only report counters, i don't believe
> devlink health dump is a proper interface for this.
These are not counters. These are error interrupts raised by HW blocks.
The count is provided to understand on how frequently the errors are seen.
Error recovery for some of the blocks happen internally. That is the reason,
Currently only dump op is added.
> Many of these counters if not most are data path packet based and maybe
> they should belong to ethtool.
Regards,
-George
On Thu, 5 Nov 2020 13:36:56 +0000 George Cherian wrote:
> > Now i am a little bit skeptic here, devlink health reporter infrastructure was
> > never meant to deal with dump op only, the main purpose is to
> > diagnose/dump and recover.
> >
> > especially in your use case where you only report counters, i don't believe
> > devlink health dump is a proper interface for this.
> These are not counters. These are error interrupts raised by HW blocks.
> The count is provided to understand on how frequently the errors are seen.
> Error recovery for some of the blocks happen internally. That is the reason,
> Currently only dump op is added.
The previous incarnation was printing messages to logs, so I assume
these errors are expected to be relatively low rate.
The point of using devlink health was that you can generate a netlink
notification when the error happens. IOW you need some calls to
devlink_health_report() or such.
At least that's my thinking, others may disagree.
On Thu, 2020-11-05 at 13:36 +0000, George Cherian wrote:
> Hi Saeed,
>
> Thanks for the review.
>
> > -----Original Message-----
> > From: Saeed Mahameed <[email protected]>
> > Sent: Thursday, November 5, 2020 10:39 AM
> > To: George Cherian <[email protected]>; [email protected];
> > [email protected]; Jiri Pirko <[email protected]>
> > Cc: [email protected]; [email protected]; Sunil Kovvuri Goutham
> > <[email protected]>; Linu Cherian <[email protected]>;
> > Geethasowjanya Akula <[email protected]>; [email protected];
> > [email protected]
> > Subject: Re: [PATCH v2 net-next 3/3] octeontx2-af: Add devlink
> > health
> > reporters for NIX
> >
> > On Wed, 2020-11-04 at 17:57 +0530, George Cherian wrote:
> > > Add health reporters for RVU NPA block.
> > ^^^ NIX ?
> >
> Yes, it's NIX.
>
> > Cc: Jiri
> >
> > Anyway, could you please spare some words on what is NPA and what
> > is
> > NIX?
> >
> > Regarding the reporters names, all drivers register well known
> > generic names
> > such as (fw,hw,rx,tx), I don't know if it is a good idea to use
> > vendor specific
> > names, if you are reporting for hw/fw units then just use "hw" or
> > "fw" as the
> > reporter name and append the unit NPA/NIX to the counter/error
> > names.
> Okay. These are hw units, I will rename them as hw_npa/hw_nix.
What i meant is have one reporter named "hw" and inside it report
counters with their unit name appended to the counter name.
./devlink health
pci/0002:01:00.0:
reporter hw
state healthy error 0 recover 0
./devlink health dump show pci/0002:01:00.0 reporter hw
NIX:
nix_counter_a: 0
...
NPA:
npa_counter_a: 0
...
> > > Only reporter dump is supported.
> > >
> > > Output:
> > > # ./devlink health
> > > pci/0002:01:00.0:
> > > reporter npa
> > > state healthy error 0 recover 0
> > > reporter nix
> > > state healthy error 0 recover 0
> > > # ./devlink health dump show pci/0002:01:00.0 reporter nix
> > > NIX_AF_GENERAL:
> > > Memory Fault on NIX_AQ_INST_S read: 0
> > > Memory Fault on NIX_AQ_RES_S write: 0
> > > AQ Doorbell error: 0
> > > Rx on unmapped PF_FUNC: 0
> > > Rx multicast replication error: 0
> > > Memory fault on NIX_RX_MCE_S read: 0
> > > Memory fault on multicast WQE read: 0
> > > Memory fault on mirror WQE read: 0
> > > Memory fault on mirror pkt write: 0
> > > Memory fault on multicast pkt write: 0
> > > NIX_AF_RAS:
> > > Poisoned data on NIX_AQ_INST_S read: 0
> > > Poisoned data on NIX_AQ_RES_S write: 0
> > > Poisoned data on HW context read: 0
> > > Poisoned data on packet read from mirror buffer: 0
> > > Poisoned data on packet read from mcast buffer: 0
> > > Poisoned data on WQE read from mirror buffer: 0
> > > Poisoned data on WQE read from multicast buffer: 0
> > > Poisoned data on NIX_RX_MCE_S read: 0
> > > NIX_AF_RVU:
> > > Unmap Slot Error: 0
> > >
> >
> > Now i am a little bit skeptic here, devlink health reporter
> > infrastructure was
> > never meant to deal with dump op only, the main purpose is to
> > diagnose/dump and recover.
> >
> > especially in your use case where you only report counters, i don't
> > believe
> > devlink health dump is a proper interface for this.
> These are not counters. These are error interrupts raised by HW
> blocks.
> The count is provided to understand on how frequently the errors are
> seen.
> Error recovery for some of the blocks happen internally. That is the
> reason,
> Currently only dump op is added.
So you are counting these events in driver, sounds like a counter to
me, i really think this shouldn't belong to devlink, unless you really
utilize devlink health ops for actual reporting and recovery.
what's wrong with just dumping these counters to ethtool ?
> > Many of these counters if not most are data path packet based and
> > maybe
> > they should belong to ethtool.
>
> Regards,
> -George
>
>
>
On Thu, 2020-11-05 at 09:07 -0800, Jakub Kicinski wrote:
> On Thu, 5 Nov 2020 13:36:56 +0000 George Cherian wrote:
> > > Now i am a little bit skeptic here, devlink health reporter
> > > infrastructure was
> > > never meant to deal with dump op only, the main purpose is to
> > > diagnose/dump and recover.
> > >
> > > especially in your use case where you only report counters, i
> > > don't believe
> > > devlink health dump is a proper interface for this.
> > These are not counters. These are error interrupts raised by HW
> > blocks.
> > The count is provided to understand on how frequently the errors
> > are seen.
> > Error recovery for some of the blocks happen internally. That is
> > the reason,
> > Currently only dump op is added.
>
> The previous incarnation was printing messages to logs, so I assume
> these errors are expected to be relatively low rate.
>
> The point of using devlink health was that you can generate a netlink
> notification when the error happens. IOW you need some calls to
> devlink_health_report() or such.
>
> At least that's my thinking, others may disagree.
If you report an error without recovering, devlink health will report a
bad device state
$ ./devlink health
pci/0002:01:00.0:
reporter npa
state error error 1 recover 0
So you will need to implement an empty recover op.
so if these events are informational only and they don't indicate
device health issues, why would you report them via devlink health ?
> > > > Output:
> > > > # ./devlink health
> > > > pci/0002:01:00.0:
> > > > reporter npa
> > > > state healthy error 0 recover 0
> > > > reporter nix
> > > > state healthy error 0 recover 0
> > > > # ./devlink health dump show pci/0002:01:00.0 reporter nix
> > > > NIX_AF_GENERAL:
> > > > Memory Fault on NIX_AQ_INST_S read: 0
> > > > Memory Fault on NIX_AQ_RES_S write: 0
> > > > AQ Doorbell error: 0
> > > > Rx on unmapped PF_FUNC: 0
> > > > Rx multicast replication error: 0
> > > > Memory fault on NIX_RX_MCE_S read: 0
> > > > Memory fault on multicast WQE read: 0
> > > > Memory fault on mirror WQE read: 0
> > > > Memory fault on mirror pkt write: 0
> > > > Memory fault on multicast pkt write: 0
> > > > NIX_AF_RAS:
> > > > Poisoned data on NIX_AQ_INST_S read: 0
> > > > Poisoned data on NIX_AQ_RES_S write: 0
> > > > Poisoned data on HW context read: 0
> > > > Poisoned data on packet read from mirror buffer: 0
> > > > Poisoned data on packet read from mcast buffer: 0
> > > > Poisoned data on WQE read from mirror buffer: 0
> > > > Poisoned data on WQE read from multicast buffer: 0
> > > > Poisoned data on NIX_RX_MCE_S read: 0
> > > > NIX_AF_RVU:
> > > > Unmap Slot Error: 0
> > > >
> > >
> > > Now i am a little bit skeptic here, devlink health reporter
> > > infrastructure was
> > > never meant to deal with dump op only, the main purpose is to
> > > diagnose/dump and recover.
> > >
> > > especially in your use case where you only report counters, i don't
> > > believe
> > > devlink health dump is a proper interface for this.
> > These are not counters. These are error interrupts raised by HW
> > blocks.
> > The count is provided to understand on how frequently the errors are
> > seen.
> > Error recovery for some of the blocks happen internally. That is the
> > reason,
> > Currently only dump op is added.
>
> So you are counting these events in driver, sounds like a counter to
> me, i really think this shouldn't belong to devlink, unless you really
> utilize devlink health ops for actual reporting and recovery.
>
> what's wrong with just dumping these counters to ethtool ?
This driver is a administrative driver which handles all the resources
in the system and doesn't do any IO.
NIX and NPA are key co-processor blocks which this driver handles.
With NIX and NPA, there are pieces
which gets attached to a PCI device to make it a networking device. We
have netdev drivers registered to this
networking device. Some more information about the drivers is available at
https://www.kernel.org/doc/html/latest/networking/device_drivers/ethernet/marvell/octeontx2.html
So we don't have a netdev here to report these co-processor block
level errors over ethtool.
Thanks,
Sunil.
On Thu, 05 Nov 2020 11:23:54 -0800 Saeed Mahameed wrote:
> If you report an error without recovering, devlink health will report a
> bad device state
>
> $ ./devlink health
> pci/0002:01:00.0:
> reporter npa
> state error error 1 recover 0
Actually, the counter in the driver is unnecessary, right? Devlink
counts errors.
> So you will need to implement an empty recover op.
> so if these events are informational only and they don't indicate
> device health issues, why would you report them via devlink health ?
I see devlink health reporters a way of collecting errors reports which
for the most part are just shared with the vendor. IOW firmware (or
hardware) bugs.
Obviously as you say without recover and additional context in the
report the value is quite diminished. But _if_ these are indeed "report
me to the vendor" kind of events then at least they should use our
current mechanics for such reports - which is dl-health.
Without knowing what these events are it's quite hard to tell if
devlink health is an overkill or counter is sufficient.
Either way - printing these to the logs is definitely the worst choice
:)
On Thu, 2020-11-05 at 12:42 -0800, Jakub Kicinski wrote:
> On Thu, 05 Nov 2020 11:23:54 -0800 Saeed Mahameed wrote:
> > If you report an error without recovering, devlink health will
> > report a
> > bad device state
> >
> > $ ./devlink health
> > pci/0002:01:00.0:
> > reporter npa
> > state error error 1 recover 0
>
> Actually, the counter in the driver is unnecessary, right? Devlink
> counts errors.
>
if you mean error and recover counters, then yes. they are managed by
devlink health
every call to dl-health-report will do:
devlink_health_report(reporter, err_ctx, msg)
{
reproter.error++;
devlink_trigger_event(reporter, msg);
reporter.dump(err_ctx, msg);
reporter.diag(err_ctx);
if (!reporter.recover(err_ctx))
reporter.recover++;
}
so dl-health reports without a recover op will confuse the user if user
sees error count > recover count.
error count should only be grater than recover count when recover
procedure fails which now will indicate the device is not in a healthy
state.
also i want to clarify one small note about devlink dump.
devlink health dump semantics:
on devlink health dump, the devlink health will check if previous dump
exists and will just return it without actually calling the driver, if
not then it will call the driver to perform a new dump and will cache
it.
user has to explicitly clear the devlink health dump of that reporter
in order to allow for newer dump to get generated.
this is done this way because we want the driver to store the dump of
the previously reported errors at the moment the erorrs are reported by
driver, so when a user issue a dump command the dump of the previous
error will be reported to user form memory without the need to access
driver/hw who might be in a bad state.
so this is why using devlink dump for reporting counters doesn't really
work, it will only report the first time the counters are accessed via
devlink health dump, after that it will report the same cached values
over and over until the user clears it up.
> > So you will need to implement an empty recover op.
> > so if these events are informational only and they don't indicate
> > device health issues, why would you report them via devlink health
> > ?
>
> I see devlink health reporters a way of collecting errors reports
> which
> for the most part are just shared with the vendor. IOW firmware (or
> hardware) bugs.
>
> Obviously as you say without recover and additional context in the
> report the value is quite diminished. But _if_ these are indeed
> "report
> me to the vendor" kind of events then at least they should use our
> current mechanics for such reports - which is dl-health.
>
> Without knowing what these events are it's quite hard to tell if
> devlink health is an overkill or counter is sufficient.
>
> Either way - printing these to the logs is definitely the worst
> choice
> :)
Sure, I don't mind using devlink health for dump only, I don't really
have strong feelings against this, they can always extend it in the
future.
it just doesn't make sense to me to have it mainly used for dumping
counters and without using devlik helath utilities, like events,
reports and recover.
so maybe Sunil et al. could polish this patchset and provide more
devlink health support, like diagnose for these errors, dump HW
information and contexts related to these errors so they could debug
root causes, etc ..
Then the use for dl health in this series can be truly justified.
On Thu, 05 Nov 2020 15:52:32 -0800 Saeed Mahameed wrote:
> On Thu, 2020-11-05 at 12:42 -0800, Jakub Kicinski wrote:
> > On Thu, 05 Nov 2020 11:23:54 -0800 Saeed Mahameed wrote:
> > > If you report an error without recovering, devlink health will
> > > report a
> > > bad device state
> > >
> > > $ ./devlink health
> > > pci/0002:01:00.0:
> > > reporter npa
> > > state error error 1 recover 0
> >
> > Actually, the counter in the driver is unnecessary, right? Devlink
> > counts errors.
>
> if you mean error and recover counters, then yes. they are managed by
> devlink health
>
> every call to dl-health-report will do:
>
> devlink_health_report(reporter, err_ctx, msg)
> {
> reproter.error++;
>
> devlink_trigger_event(reporter, msg);
>
> reporter.dump(err_ctx, msg);
> reporter.diag(err_ctx);
>
> if (!reporter.recover(err_ctx))
> reporter.recover++;
> }
>
> so dl-health reports without a recover op will confuse the user if user
> sees error count > recover count.
>
> error count should only be grater than recover count when recover
> procedure fails which now will indicate the device is not in a healthy
> state.
Good point, as is the internal devlink counter mismatch looks pretty
strange.
> also i want to clarify one small note about devlink dump.
>
> devlink health dump semantics:
> on devlink health dump, the devlink health will check if previous dump
> exists and will just return it without actually calling the driver, if
> not then it will call the driver to perform a new dump and will cache
> it.
>
> user has to explicitly clear the devlink health dump of that reporter
> in order to allow for newer dump to get generated.
>
> this is done this way because we want the driver to store the dump of
> the previously reported errors at the moment the erorrs are reported by
> driver, so when a user issue a dump command the dump of the previous
> error will be reported to user form memory without the need to access
> driver/hw who might be in a bad state.
>
> so this is why using devlink dump for reporting counters doesn't really
> work, it will only report the first time the counters are accessed via
> devlink health dump, after that it will report the same cached values
> over and over until the user clears it up.
Agreed, if only counters are reported driver should rely on the
devlink counters. Dump contents are for context of the event.
> > > So you will need to implement an empty recover op.
> > > so if these events are informational only and they don't indicate
> > > device health issues, why would you report them via devlink health
> > > ?
> >
> > I see devlink health reporters a way of collecting errors reports
> > which
> > for the most part are just shared with the vendor. IOW firmware (or
> > hardware) bugs.
> >
> > Obviously as you say without recover and additional context in the
> > report the value is quite diminished. But _if_ these are indeed
> > "report
> > me to the vendor" kind of events then at least they should use our
> > current mechanics for such reports - which is dl-health.
> >
> > Without knowing what these events are it's quite hard to tell if
> > devlink health is an overkill or counter is sufficient.
> >
> > Either way - printing these to the logs is definitely the worst
> > choice
> > :)
>
> Sure, I don't mind using devlink health for dump only, I don't really
> have strong feelings against this, they can always extend it in the
> future.
>
> it just doesn't make sense to me to have it mainly used for dumping
> counters and without using devlik helath utilities, like events,
> reports and recover.
>
> so maybe Sunil et al. could polish this patchset and provide more
> devlink health support, like diagnose for these errors, dump HW
> information and contexts related to these errors so they could debug
> root causes, etc ..
> Then the use for dl health in this series can be truly justified.
That'd indeed be optimal.
On Fri, 2020-11-06 at 00:59 +0530, Sunil Kovvuri wrote:
> > > > > Output:
> > > > > # ./devlink health
> > > > > pci/0002:01:00.0:
> > > > > reporter npa
> > > > > state healthy error 0 recover 0
> > > > > reporter nix
> > > > > state healthy error 0 recover 0
> > > > > # ./devlink health dump show pci/0002:01:00.0 reporter nix
> > > > > NIX_AF_GENERAL:
> > > > > Memory Fault on NIX_AQ_INST_S read: 0
> > > > > Memory Fault on NIX_AQ_RES_S write: 0
> > > > > AQ Doorbell error: 0
> > > > > Rx on unmapped PF_FUNC: 0
> > > > > Rx multicast replication error: 0
> > > > > Memory fault on NIX_RX_MCE_S read: 0
> > > > > Memory fault on multicast WQE read: 0
> > > > > Memory fault on mirror WQE read: 0
> > > > > Memory fault on mirror pkt write: 0
> > > > > Memory fault on multicast pkt write: 0
> > > > > NIX_AF_RAS:
> > > > > Poisoned data on NIX_AQ_INST_S read: 0
> > > > > Poisoned data on NIX_AQ_RES_S write: 0
> > > > > Poisoned data on HW context read: 0
> > > > > Poisoned data on packet read from mirror buffer: 0
> > > > > Poisoned data on packet read from mcast buffer: 0
> > > > > Poisoned data on WQE read from mirror buffer: 0
> > > > > Poisoned data on WQE read from multicast buffer: 0
> > > > > Poisoned data on NIX_RX_MCE_S read: 0
> > > > > NIX_AF_RVU:
> > > > > Unmap Slot Error: 0
> > > > >
> > > >
> > > > Now i am a little bit skeptic here, devlink health reporter
> > > > infrastructure was
> > > > never meant to deal with dump op only, the main purpose is to
> > > > diagnose/dump and recover.
> > > >
> > > > especially in your use case where you only report counters, i
> > > > don't
> > > > believe
> > > > devlink health dump is a proper interface for this.
> > > These are not counters. These are error interrupts raised by HW
> > > blocks.
> > > The count is provided to understand on how frequently the errors
> > > are
> > > seen.
> > > Error recovery for some of the blocks happen internally. That is
> > > the
> > > reason,
> > > Currently only dump op is added.
> >
> > So you are counting these events in driver, sounds like a counter
> > to
> > me, i really think this shouldn't belong to devlink, unless you
> > really
> > utilize devlink health ops for actual reporting and recovery.
> >
> > what's wrong with just dumping these counters to ethtool ?
>
> This driver is a administrative driver which handles all the
> resources
> in the system and doesn't do any IO.
> NIX and NPA are key co-processor blocks which this driver handles.
> With NIX and NPA, there are pieces
> which gets attached to a PCI device to make it a networking device.
> We
> have netdev drivers registered to this
> networking device. Some more information about the drivers is
> available at
> https://www.kernel.org/doc/html/latest/networking/device_drivers/ethernet/marvell/octeontx2.html
>
> So we don't have a netdev here to report these co-processor block
> level errors over ethtool.
>
but AF driver can't be standalone to operate your hw, it must have a
PF/VF with netdev interface to do io, so even if your model is modular,
a common user of this driver will always see a netdev.
On Sat, Nov 7, 2020 at 2:28 AM Saeed Mahameed <[email protected]> wrote:
>
> On Fri, 2020-11-06 at 00:59 +0530, Sunil Kovvuri wrote:
> > > > > > Output:
> > > > > > # ./devlink health
> > > > > > pci/0002:01:00.0:
> > > > > > reporter npa
> > > > > > state healthy error 0 recover 0
> > > > > > reporter nix
> > > > > > state healthy error 0 recover 0
> > > > > > # ./devlink health dump show pci/0002:01:00.0 reporter nix
> > > > > > NIX_AF_GENERAL:
> > > > > > Memory Fault on NIX_AQ_INST_S read: 0
> > > > > > Memory Fault on NIX_AQ_RES_S write: 0
> > > > > > AQ Doorbell error: 0
> > > > > > Rx on unmapped PF_FUNC: 0
> > > > > > Rx multicast replication error: 0
> > > > > > Memory fault on NIX_RX_MCE_S read: 0
> > > > > > Memory fault on multicast WQE read: 0
> > > > > > Memory fault on mirror WQE read: 0
> > > > > > Memory fault on mirror pkt write: 0
> > > > > > Memory fault on multicast pkt write: 0
> > > > > > NIX_AF_RAS:
> > > > > > Poisoned data on NIX_AQ_INST_S read: 0
> > > > > > Poisoned data on NIX_AQ_RES_S write: 0
> > > > > > Poisoned data on HW context read: 0
> > > > > > Poisoned data on packet read from mirror buffer: 0
> > > > > > Poisoned data on packet read from mcast buffer: 0
> > > > > > Poisoned data on WQE read from mirror buffer: 0
> > > > > > Poisoned data on WQE read from multicast buffer: 0
> > > > > > Poisoned data on NIX_RX_MCE_S read: 0
> > > > > > NIX_AF_RVU:
> > > > > > Unmap Slot Error: 0
> > > > > >
> > > > >
> > > > > Now i am a little bit skeptic here, devlink health reporter
> > > > > infrastructure was
> > > > > never meant to deal with dump op only, the main purpose is to
> > > > > diagnose/dump and recover.
> > > > >
> > > > > especially in your use case where you only report counters, i
> > > > > don't
> > > > > believe
> > > > > devlink health dump is a proper interface for this.
> > > > These are not counters. These are error interrupts raised by HW
> > > > blocks.
> > > > The count is provided to understand on how frequently the errors
> > > > are
> > > > seen.
> > > > Error recovery for some of the blocks happen internally. That is
> > > > the
> > > > reason,
> > > > Currently only dump op is added.
> > >
> > > So you are counting these events in driver, sounds like a counter
> > > to
> > > me, i really think this shouldn't belong to devlink, unless you
> > > really
> > > utilize devlink health ops for actual reporting and recovery.
> > >
> > > what's wrong with just dumping these counters to ethtool ?
> >
> > This driver is a administrative driver which handles all the
> > resources
> > in the system and doesn't do any IO.
> > NIX and NPA are key co-processor blocks which this driver handles.
> > With NIX and NPA, there are pieces
> > which gets attached to a PCI device to make it a networking device.
> > We
> > have netdev drivers registered to this
> > networking device. Some more information about the drivers is
> > available at
> > https://www.kernel.org/doc/html/latest/networking/device_drivers/ethernet/marvell/octeontx2.html
> >
> > So we don't have a netdev here to report these co-processor block
> > level errors over ethtool.
> >
>
> but AF driver can't be standalone to operate your hw, it must have a
> PF/VF with netdev interface to do io, so even if your model is modular,
> a common user of this driver will always see a netdev.
>
That's right, user will always see a netdev, but
The co-processor blocks are like this
- Each co-processor has two parts, AF unit and LF units (local function)
- Each of the co-processor can have multiple LFs, incase of NIX
co-processor, each of the LF provides RQ, SQ, CQs etc.
- So the AF driver handles the co-processor's AF unit and upon
receiving requests from PF/VF attaches the LFs to them, so that they
can do network IO.
- Within co-processor, AF unit specific errors (global) are reported
to AF driver and LF specific errors are reported to netdev driver.
- There can be 10s of netdev driver instances in the system, so these
AF unit global errors cannot be routed and shown in one of the
netdev's ethtool.
Thanks,
Sunil.