* [PATCH 1/2] common/cnxk: fix mailbox timeout issues
@ 2024-11-04 13:07 Harman Kalra
2024-11-04 13:08 ` [PATCH 2/2] common/cnxk: add mailbox debug trace Harman Kalra
2024-11-14 9:31 ` [PATCH v2 1/2] common/cnxk: fix mailbox timeout issues Harman Kalra
0 siblings, 2 replies; 6+ messages in thread
From: Harman Kalra @ 2024-11-04 13:07 UTC (permalink / raw)
To: Nithin Dabilpuram, Kiran Kumar K, Sunil Kumar Kori, Satha Rao,
Harman Kalra
Cc: dev
Found couple of reasons causing mailbox timeout:
* msgs_acked value by interrupt thread was not getting synced to thread
polling it.
* mbox_data value sent by AF was seen 0 causing neither down nor up msg
processing.
Fixes: 9bd368ca311a ("common/cnxk: enable PF VF mailbox")
Fixes: 61deac72abbf ("common/cnxk: add CN20KA mbox support")
Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
drivers/common/cnxk/roc_dev.c | 106 ++++++++++++++++++++++++-----
drivers/common/cnxk/roc_dev_priv.h | 11 +++
drivers/common/cnxk/roc_mbox.c | 10 ++-
3 files changed, 104 insertions(+), 23 deletions(-)
diff --git a/drivers/common/cnxk/roc_dev.c b/drivers/common/cnxk/roc_dev.c
index 32409f2ef3..1127d8185c 100644
--- a/drivers/common/cnxk/roc_dev.c
+++ b/drivers/common/cnxk/roc_dev.c
@@ -401,8 +401,7 @@ vf_pf_process_up_msgs(struct dev *dev, uint16_t vf)
offset = mbox->rx_start + msg->next_msgoff;
}
mbox_reset(mbox, vf);
- mdev->msgs_acked = msgs_acked;
- plt_wmb();
+ __atomic_store_n(&mdev->msgs_acked, msgs_acked, __ATOMIC_RELEASE);
return i;
}
@@ -542,8 +541,7 @@ process_msgs(struct dev *dev, struct mbox *mbox)
mbox_reset(mbox, 0);
/* Update acked if someone is waiting a message - mbox_wait is waiting */
- mdev->msgs_acked = msgs_acked;
- plt_wmb();
+ __atomic_store_n(&mdev->msgs_acked, msgs_acked, __ATOMIC_RELEASE);
}
/* Copies the message received from AF and sends it to VF */
@@ -920,12 +918,59 @@ process_msgs_up(struct dev *dev, struct mbox *mbox)
}
}
+/* IRQ to VF from PF - VF context (interrupt thread) */
+static void
+roc_pf_vf_mbox_irq_cn20k(void *param)
+{
+ struct dev *dev = param;
+ uint64_t intr;
+
+ intr = plt_read64(dev->mbox_reg_base + RVU_VF_INT);
+ if (intr == 0)
+ plt_base_dbg("Proceeding to check mbox UP messages if any");
+
+ plt_write64(intr, dev->mbox_reg_base + RVU_VF_INT);
+ plt_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev->vf);
+
+ /* If interrupt occurred for down message */
+ if (intr & BIT_ULL(1))
+ /* First process all configuration messages */
+ process_msgs(dev, dev->mbox);
+
+ /* If interrupt occurred for UP message */
+ if (intr & BIT_ULL(0))
+ process_msgs_up(dev, &dev->mbox_up);
+}
+
+/* IRQ to PF from AF - PF context (interrupt thread) */
+static void
+roc_af_pf_mbox_irq_cn20k(void *param)
+{
+ struct dev *dev = param;
+ uint64_t intr;
+
+ intr = plt_read64(dev->mbox_reg_base + RVU_PF_INT);
+ if (intr == 0)
+ plt_base_dbg("Proceeding to check mbox UP messages if any");
+
+ plt_write64(intr, dev->mbox_reg_base + RVU_PF_INT);
+ plt_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev->vf);
+
+ /* If interrupt occurred for down message */
+ if (intr & BIT_ULL(1))
+ process_msgs(dev, dev->mbox);
+
+ /* If interrupt occurred for up message */
+ if (intr & BIT_ULL(0))
+ process_msgs_up(dev, &dev->mbox_up);
+}
+
/* IRQ to VF from PF - VF context (interrupt thread) */
static void
roc_pf_vf_mbox_irq(void *param)
{
struct dev *dev = param;
- uint64_t mbox_data;
+ uint64_t mbox_data = 0;
uint64_t intr;
intr = plt_read64(dev->mbox_reg_base + RVU_VF_INT);
@@ -940,7 +985,7 @@ roc_pf_vf_mbox_irq(void *param)
*/
mbox_data = plt_read64(dev->mbox_reg_base + RVU_VF_VFPF_MBOX0);
/* If interrupt occurred for down message */
- if (mbox_data & MBOX_DOWN_MSG || intr & BIT_ULL(1)) {
+ if (mbox_data & MBOX_DOWN_MSG) {
mbox_data &= ~MBOX_DOWN_MSG;
plt_write64(mbox_data, dev->mbox_reg_base + RVU_VF_VFPF_MBOX0);
@@ -948,7 +993,7 @@ roc_pf_vf_mbox_irq(void *param)
process_msgs(dev, dev->mbox);
}
/* If interrupt occurred for UP message */
- if (mbox_data & MBOX_UP_MSG || intr & BIT_ULL(0)) {
+ if (mbox_data & MBOX_UP_MSG) {
mbox_data &= ~MBOX_UP_MSG;
plt_write64(mbox_data, dev->mbox_reg_base + RVU_VF_VFPF_MBOX0);
@@ -962,7 +1007,7 @@ static void
roc_af_pf_mbox_irq(void *param)
{
struct dev *dev = param;
- uint64_t mbox_data;
+ uint64_t mbox_data = 0;
uint64_t intr;
intr = plt_read64(dev->mbox_reg_base + RVU_PF_INT);
@@ -977,7 +1022,7 @@ roc_af_pf_mbox_irq(void *param)
*/
mbox_data = plt_read64(dev->mbox_reg_base + RVU_PF_PFAF_MBOX0);
/* If interrupt occurred for down message */
- if (mbox_data & MBOX_DOWN_MSG || intr & BIT_ULL(1)) {
+ if (mbox_data & MBOX_DOWN_MSG) {
mbox_data &= ~MBOX_DOWN_MSG;
plt_write64(mbox_data, dev->mbox_reg_base + RVU_PF_PFAF_MBOX0);
@@ -985,7 +1030,7 @@ roc_af_pf_mbox_irq(void *param)
process_msgs(dev, dev->mbox);
}
/* If interrupt occurred for up message */
- if (mbox_data & MBOX_UP_MSG || intr & BIT_ULL(0)) {
+ if (mbox_data & MBOX_UP_MSG) {
mbox_data &= ~MBOX_UP_MSG;
plt_write64(mbox_data, dev->mbox_reg_base + RVU_PF_PFAF_MBOX0);
@@ -1045,7 +1090,8 @@ mbox_register_pf_irq(struct plt_pci_device *pci_dev, struct dev *dev)
}
}
/* MBOX interrupt AF <-> PF */
- rc = dev_irq_register(intr_handle, roc_af_pf_mbox_irq, dev, dev->mbox_plat->pfaf_vec);
+ rc = dev_irq_register(intr_handle, dev->mbox_plat->ops->af_pf_mbox_irq, dev,
+ dev->mbox_plat->pfaf_vec);
if (rc) {
plt_err("Fail to register AF<->PF mbox irq");
return rc;
@@ -1073,7 +1119,8 @@ mbox_register_vf_irq(struct plt_pci_device *pci_dev, struct dev *dev)
plt_write64(~0ull, dev->mbox_reg_base + RVU_VF_INT_ENA_W1C);
/* MBOX interrupt PF <-> VF */
- rc = dev_irq_register(intr_handle, roc_pf_vf_mbox_irq, dev, RVU_VF_INT_VEC_MBOX);
+ rc = dev_irq_register(intr_handle, dev->mbox_plat->ops->pf_vf_mbox_irq, dev,
+ RVU_VF_INT_VEC_MBOX);
if (rc) {
plt_err("Fail to register PF<->VF mbox irq");
return rc;
@@ -1127,7 +1174,8 @@ mbox_unregister_pf_irq(struct plt_pci_device *pci_dev, struct dev *dev)
}
/* MBOX interrupt AF <-> PF */
- dev_irq_unregister(intr_handle, roc_af_pf_mbox_irq, dev, dev->mbox_plat->pfaf_vec);
+ dev_irq_unregister(intr_handle, dev->mbox_plat->ops->af_pf_mbox_irq, dev,
+ dev->mbox_plat->pfaf_vec);
}
static void
@@ -1139,7 +1187,8 @@ mbox_unregister_vf_irq(struct plt_pci_device *pci_dev, struct dev *dev)
plt_write64(~0ull, dev->mbox_reg_base + RVU_VF_INT_ENA_W1C);
/* Unregister the interrupt handler */
- dev_irq_unregister(intr_handle, roc_pf_vf_mbox_irq, dev, RVU_VF_INT_VEC_MBOX);
+ dev_irq_unregister(intr_handle, dev->mbox_plat->ops->pf_vf_mbox_irq, dev,
+ RVU_VF_INT_VEC_MBOX);
}
void
@@ -1599,10 +1648,17 @@ dev_cache_line_size_valid(void)
return true;
}
-static void
+static int
mbox_platform_changes(struct mbox_platform *mbox_plat, uintptr_t bar2, uintptr_t bar4, bool is_vf)
{
- int i;
+ int i, rc = 0;
+
+ /* Allocate memory for device ops */
+ mbox_plat->ops = plt_zmalloc(sizeof(struct mbox_ops), 0);
+ if (mbox_plat->ops == NULL) {
+ rc = -ENOMEM;
+ goto fail;
+ }
if (roc_model_is_cn20k()) {
/* For CN20K, AF allocates mbox memory in DRAM and writes PF
@@ -1613,6 +1669,9 @@ mbox_platform_changes(struct mbox_platform *mbox_plat, uintptr_t bar2, uintptr_t
mbox_plat->mbox_region_base =
bar2 + (RVU_PFX_FUNC_PFAF_MBOX +
((uint64_t)RVU_BLOCK_ADDR_MBOX << RVU_FUNC_BLKADDR_SHIFT));
+ /* Mbox operations */
+ mbox_plat->ops->af_pf_mbox_irq = roc_af_pf_mbox_irq_cn20k;
+ mbox_plat->ops->pf_vf_mbox_irq = roc_pf_vf_mbox_irq_cn20k;
/* Interrupt vectors */
mbox_plat->pfaf_vec = RVU_MBOX_PF_INT_VEC_AFPF_MBOX;
mbox_plat->pfvf_mbox0_vec = RVU_MBOX_PF_INT_VEC_VFPF_MBOX0;
@@ -1630,6 +1689,9 @@ mbox_platform_changes(struct mbox_platform *mbox_plat, uintptr_t bar2, uintptr_t
} else {
mbox_plat->mbox_reg_base = bar2;
mbox_plat->mbox_region_base = bar4;
+ /* Mbox operations */
+ mbox_plat->ops->af_pf_mbox_irq = roc_af_pf_mbox_irq;
+ mbox_plat->ops->pf_vf_mbox_irq = roc_pf_vf_mbox_irq;
mbox_plat->pfaf_vec = RVU_PF_INT_VEC_AFPF_MBOX;
mbox_plat->pfvf_mbox0_vec = RVU_PF_INT_VEC_VFPF_MBOX0;
mbox_plat->pfvf_mbox1_vec = RVU_PF_INT_VEC_VFPF_MBOX1;
@@ -1647,6 +1709,8 @@ mbox_platform_changes(struct mbox_platform *mbox_plat, uintptr_t bar2, uintptr_t
if (roc_model_is_cn10k())
mbox_plat->mbox_region_base = bar2 + RVU_VF_MBOX_REGION;
}
+fail:
+ return rc;
}
int
@@ -1678,7 +1742,12 @@ dev_init(struct dev *dev, struct plt_pci_device *pci_dev)
rc = -ENOMEM;
goto fail;
}
- mbox_platform_changes(dev->mbox_plat, bar2, bar4, is_vf);
+
+ if (mbox_platform_changes(dev->mbox_plat, bar2, bar4, is_vf)) {
+ plt_err("Failed to populate platform specific changes");
+ rc = -ENOMEM;
+ goto mbox_plat_free;
+ }
mbox_reg_base = dev->mbox_plat->mbox_reg_base;
mbox_region_base = dev->mbox_plat->mbox_region_base;
@@ -1824,6 +1893,8 @@ dev_init(struct dev *dev, struct plt_pci_device *pci_dev)
mbox_fini(dev->mbox);
mbox_fini(&dev->mbox_up);
error:
+ plt_free(dev->mbox_plat->ops);
+mbox_plat_free:
plt_free(dev->mbox_plat);
fail:
return rc;
@@ -1883,6 +1954,7 @@ dev_fini(struct dev *dev, struct plt_pci_device *pci_dev)
mbox_fini(mbox);
dev->mbox_active = 0;
+ plt_free(dev->mbox_plat->ops);
plt_free(dev->mbox_plat);
/* Disable MSIX vectors */
dev_irqs_disable(intr_handle);
diff --git a/drivers/common/cnxk/roc_dev_priv.h b/drivers/common/cnxk/roc_dev_priv.h
index c766183196..a8e40fa9d5 100644
--- a/drivers/common/cnxk/roc_dev_priv.h
+++ b/drivers/common/cnxk/roc_dev_priv.h
@@ -106,6 +106,15 @@ struct mbox_sync {
pthread_mutex_t mutex;
};
+/* AF PF VF mbox interrupt callbacks */
+typedef void (*af_pf_mbox_irq_t)(void *param);
+typedef void (*pf_vf_mbox_irq_t)(void *param);
+
+struct mbox_ops {
+ af_pf_mbox_irq_t af_pf_mbox_irq;
+ pf_vf_mbox_irq_t pf_vf_mbox_irq;
+};
+
struct mbox_platform {
uint8_t pfaf_vec;
uint8_t pfvf_mbox0_vec;
@@ -120,6 +129,8 @@ struct mbox_platform {
uint64_t pfvf1_mbox_int_ena_w1c[MAX_VFPF_DWORD_BITS];
uintptr_t mbox_reg_base;
uintptr_t mbox_region_base;
+ /* Mbox operations */
+ struct mbox_ops *ops;
};
struct dev {
diff --git a/drivers/common/cnxk/roc_mbox.c b/drivers/common/cnxk/roc_mbox.c
index db77babfdb..eb5bd771fe 100644
--- a/drivers/common/cnxk/roc_mbox.c
+++ b/drivers/common/cnxk/roc_mbox.c
@@ -275,7 +275,7 @@ mbox_msg_send_data(struct mbox *mbox, int devid, uint8_t data)
tx_hdr->msg_size = mdev->msg_size;
mdev->msg_size = 0;
mdev->rsp_size = 0;
- mdev->msgs_acked = 0;
+ __atomic_store_n(&mdev->msgs_acked, 0, __ATOMIC_RELEASE);
/* num_msgs != 0 signals to the peer that the buffer has a number of
* messages. So this should be written after copying txmem
@@ -417,7 +417,7 @@ mbox_wait(struct mbox *mbox, int devid, uint32_t rst_timo)
* mdev->msgs_acked are incremented at process_msgs() in interrupt
* thread context.
*/
- while (mdev->num_msgs > mdev->msgs_acked) {
+ while (mdev->num_msgs > __atomic_load_n(&mdev->msgs_acked, __ATOMIC_ACQUIRE)) {
plt_delay_us(sleep);
timeout += sleep;
if (timeout >= rst_timo) {
@@ -433,13 +433,11 @@ mbox_wait(struct mbox *mbox, int devid, uint32_t rst_timo)
"(tx/rx num_msgs: %d/%d), msg_size: %d, "
"rsp_size: %d",
devid, timeout, mdev->num_msgs,
- mdev->msgs_acked, tx_hdr->num_msgs,
- rx_hdr->num_msgs, mdev->msg_size,
- mdev->rsp_size);
+ __atomic_load_n(&mdev->msgs_acked, __ATOMIC_ACQUIRE),
+ tx_hdr->num_msgs, rx_hdr->num_msgs, mdev->msg_size, mdev->rsp_size);
return -EIO;
}
- plt_rmb();
}
return 0;
}
--
2.46.0.469.g4590f2e941
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] common/cnxk: add mailbox debug trace
2024-11-04 13:07 [PATCH 1/2] common/cnxk: fix mailbox timeout issues Harman Kalra
@ 2024-11-04 13:08 ` Harman Kalra
2024-11-07 8:04 ` [EXTERNAL] " Jerin Jacob
2024-11-14 16:38 ` Stephen Hemminger
2024-11-14 9:31 ` [PATCH v2 1/2] common/cnxk: fix mailbox timeout issues Harman Kalra
1 sibling, 2 replies; 6+ messages in thread
From: Harman Kalra @ 2024-11-04 13:08 UTC (permalink / raw)
To: Nithin Dabilpuram, Kiran Kumar K, Sunil Kumar Kori, Satha Rao,
Harman Kalra
Cc: dev
Implementing mailbox debug trace points to trace function calls.
Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
drivers/common/cnxk/roc_dev.c | 89 ++++++++++++++++--------------
drivers/common/cnxk/roc_mbox.c | 34 ++++++++++++
drivers/common/cnxk/roc_mbox.h | 2 +
drivers/common/cnxk/roc_platform.c | 9 +++
drivers/common/cnxk/roc_platform.h | 87 +++++++++++++++++++++++++++--
5 files changed, 175 insertions(+), 46 deletions(-)
diff --git a/drivers/common/cnxk/roc_dev.c b/drivers/common/cnxk/roc_dev.c
index 1127d8185c..e9bf8c01b4 100644
--- a/drivers/common/cnxk/roc_dev.c
+++ b/drivers/common/cnxk/roc_dev.c
@@ -26,7 +26,6 @@
/* RVU PF interrupt status as received from AF*/
#define RVU_PF_INTR_STATUS 0x3
-
static void *
mbox_mem_map(off_t off, size_t size)
{
@@ -83,6 +82,7 @@ pf_af_sync_msg(struct dev *dev, struct mbox_msghdr **rsp)
timeout += sleep;
if (timeout >= mbox->rsp_tmo) {
plt_err("Message timeout: %dms", mbox->rsp_tmo);
+ roc_trace_mbox_error("AFPF_SYNC", "RSP_TMO", -EIO);
rc = -EIO;
break;
}
@@ -137,6 +137,7 @@ af_pf_wait_msg(struct dev *dev, uint16_t vf, int num_msg)
timeout++;
if (timeout >= mbox->rsp_tmo) {
plt_err("Routed messages %d timeout: %dms", num_msg, mbox->rsp_tmo);
+ roc_trace_mbox_error("AFPF_WAIT", "RTD_MSG_TMO", -EIO);
break;
}
int_status = plt_read64(dev->mbox_reg_base + RVU_PF_INT);
@@ -149,16 +150,18 @@ af_pf_wait_msg(struct dev *dev, uint16_t vf, int num_msg)
plt_write64(~0ull, dev->mbox_reg_base + RVU_PF_INT_ENA_W1S);
req_hdr = (struct mbox_hdr *)((uintptr_t)mdev->mbase + mbox->rx_start);
- if (req_hdr->num_msgs != num_msg)
- plt_err("Routed messages: %d received: %d", num_msg,
- req_hdr->num_msgs);
+ if (req_hdr->num_msgs != num_msg) {
+ plt_err("Routed messages: %d received: %d", num_msg, req_hdr->num_msgs);
+ roc_trace_mbox_error("AFPF_WAIT", "RTD_INV_CNT", num_msg);
+ }
/* Get messages from mbox */
- offset = mbox->rx_start +
- PLT_ALIGN(sizeof(struct mbox_hdr), MBOX_MSG_ALIGN);
+ offset = mbox->rx_start + PLT_ALIGN(sizeof(struct mbox_hdr), MBOX_MSG_ALIGN);
for (i = 0; i < req_hdr->num_msgs; i++) {
msg = (struct mbox_msghdr *)((uintptr_t)mdev->mbase + offset);
size = mbox->rx_start + msg->next_msgoff - offset;
+ roc_trace_mbox_process("AFPF_WAIT", mbox_id2name(msg->id), req_hdr->num_msgs,
+ msg->pcifunc);
/* Reserve PF/VF mbox message */
size = PLT_ALIGN(size, MBOX_MSG_ALIGN);
@@ -288,6 +291,8 @@ vf_pf_process_msgs(struct dev *dev, uint16_t vf)
/* RVU_PF_FUNC_S */
msg->pcifunc = dev_pf_func(dev->pf, vf);
+ roc_trace_mbox_process("VFPF_MSG", mbox_id2name(msg->id), req_hdr->num_msgs,
+ msg->pcifunc);
if (msg->id == MBOX_MSG_READY) {
struct ready_msg_rsp *rsp;
@@ -333,8 +338,7 @@ vf_pf_process_msgs(struct dev *dev, uint16_t vf)
}
if (routed > 0) {
- plt_base_dbg("pf:%d routed %d messages from vf:%d to AF",
- dev->pf, routed, vf);
+ roc_trace_mbox_vf_pf_handle("RTG_AF", dev_pf_func(dev->pf, vf), routed);
/* PF will send the messages to AF and wait for responses */
af_pf_wait_msg(dev, vf, routed);
mbox_reset(dev->mbox, 0);
@@ -343,8 +347,7 @@ vf_pf_process_msgs(struct dev *dev, uint16_t vf)
/* Send mbox responses to VF */
if (mdev->num_msgs) {
- plt_base_dbg("pf:%d reply %d messages to vf:%d", dev->pf,
- mdev->num_msgs, vf);
+ roc_trace_mbox_vf_pf_handle("RSP_VF", dev_pf_func(dev->pf, vf), mdev->num_msgs);
mbox_msg_send(mbox, vf);
}
@@ -378,25 +381,24 @@ vf_pf_process_up_msgs(struct dev *dev, uint16_t vf)
switch (msg->id) {
case MBOX_MSG_CGX_LINK_EVENT:
- plt_base_dbg("PF: Msg 0x%x (%s) fn:0x%x (pf:%d,vf:%d)",
- msg->id, mbox_id2name(msg->id),
- msg->pcifunc, dev_get_pf(msg->pcifunc),
- dev_get_vf(msg->pcifunc));
+ roc_trace_mbox_process("VFPF_UPMSG_CGX", mbox_id2name(msg->id),
+ req_hdr->num_msgs, msg->pcifunc);
break;
case MBOX_MSG_CGX_PTP_RX_INFO:
- plt_base_dbg("PF: Msg 0x%x (%s) fn:0x%x (pf:%d,vf:%d)",
- msg->id, mbox_id2name(msg->id),
- msg->pcifunc, dev_get_pf(msg->pcifunc),
- dev_get_vf(msg->pcifunc));
+ roc_trace_mbox_process("VFPF_UPMSG_PTP", mbox_id2name(msg->id),
+ req_hdr->num_msgs, msg->pcifunc);
break;
default:
if (roc_rvu_lf_msg_id_range_check(dev->roc_rvu_lf, msg->id))
plt_base_dbg("PF: Msg 0x%x fn:0x%x (pf:%d,vf:%d)",
msg->id, msg->pcifunc, dev_get_pf(msg->pcifunc),
dev_get_vf(msg->pcifunc));
- else
- plt_err("Not handled UP msg 0x%x (%s) func:0x%x",
- msg->id, mbox_id2name(msg->id), msg->pcifunc);
+ else {
+ roc_trace_mbox_process("UNHND_UPMSG", mbox_id2name(msg->id),
+ req_hdr->num_msgs, msg->pcifunc);
+ plt_err("Not handled UP msg 0x%x (%s) func:0x%x", msg->id,
+ mbox_id2name(msg->id), msg->pcifunc);
+ }
}
offset = mbox->rx_start + msg->next_msgoff;
}
@@ -446,8 +448,7 @@ roc_vf_pf_mbox_irq(void *param)
if (!intr)
continue;
- plt_base_dbg("vfpf: %d intr: 0x%" PRIx64 " (pf:%d, vf:%d)", vfpf, intr, dev->pf,
- dev->vf);
+ roc_trace_mbox_interrupt("VFPF_IRQ", dev_pf_func(dev->pf, dev->vf), intr, 0);
/* Save and clear intr bits */
intrb.bits[vfpf] |= intr;
@@ -489,9 +490,8 @@ process_msgs(struct dev *dev, struct mbox *mbox)
msg = (struct mbox_msghdr *)((uintptr_t)mdev->mbase + offset);
msgs_acked++;
- plt_base_dbg("Message 0x%x (%s) pf:%d/vf:%d", msg->id,
- mbox_id2name(msg->id), dev_get_pf(msg->pcifunc),
- dev_get_vf(msg->pcifunc));
+ roc_trace_mbox_process("PRC_MSG", mbox_id2name(msg->id), req_hdr->num_msgs,
+ msg->pcifunc);
switch (msg->id) {
/* Add message id's that are handled here */
@@ -514,6 +514,8 @@ process_msgs(struct dev *dev, struct mbox *mbox)
} else {
plt_err("Message (%s) response has err=%d",
mbox_id2name(msg->id), msg->rc);
+ roc_trace_mbox_error("PRC_MSG_ERR", mbox_id2name(msg->id),
+ msg->rc);
}
}
break;
@@ -526,14 +528,19 @@ process_msgs(struct dev *dev, struct mbox *mbox)
} else {
plt_err("Message (%s) response has err=%d",
mbox_id2name(msg->id), msg->rc);
+ roc_trace_mbox_error("PRC_MSG_ERR", mbox_id2name(msg->id),
+ msg->rc);
}
}
break;
default:
- if (msg->rc)
+ if (msg->rc) {
plt_err("Message (%s) response has err=%d (%s)",
mbox_id2name(msg->id), msg->rc, roc_error_msg_get(msg->rc));
+ roc_trace_mbox_error("PRC_MSG_INV_RSP", mbox_id2name(msg->id),
+ msg->rc);
+ }
break;
}
offset = mbox->rx_start + msg->next_msgoff;
@@ -564,8 +571,8 @@ pf_vf_mbox_send_up_msg(struct dev *dev, void *rec_msg)
if (!(dev->active_vfs[vf / max_bits] & (BIT_ULL(vf))))
continue;
- plt_base_dbg("(%s) size: %zx to VF: %d",
- mbox_id2name(msg->hdr.id), size, vf);
+ roc_trace_mbox_process("PFVF_SND_UPMSG", mbox_id2name(msg->hdr.id), 1,
+ (uint16_t)dev_pf_func(dev->pf, vf));
/* Reserve PF/VF mbox message */
vf_msg = mbox_alloc_msg(vf_mbox, vf, size);
@@ -895,9 +902,9 @@ process_msgs_up(struct dev *dev, struct mbox *mbox)
for (i = 0; i < req_hdr->num_msgs; i++) {
msg = (struct mbox_msghdr *)((uintptr_t)mdev->mbase + offset);
- plt_base_dbg("Message 0x%x (%s) pf:%d/vf:%d", msg->id,
- mbox_id2name(msg->id), dev_get_pf(msg->pcifunc),
- dev_get_vf(msg->pcifunc));
+ plt_base_dbg("Message 0x%x (%s) pf:%d/vf:%d", msg->id, mbox_id2name(msg->id),
+ dev_get_pf(msg->pcifunc), dev_get_vf(msg->pcifunc));
+ roc_trace_mbox_process("PRC_UPMSG", mbox_id2name(msg->id), req_hdr->num_msgs, msg->pcifunc);
if (roc_rvu_lf_msg_id_range_check(dev->roc_rvu_lf, msg->id)) {
size = mbox->rx_start + msg->next_msgoff - offset;
err = process_rvu_lf_msgs_up(dev, msg, size);
@@ -905,9 +912,10 @@ process_msgs_up(struct dev *dev, struct mbox *mbox)
plt_err("Error %d handling 0x%x RVU_LF up msg", err, msg->id);
} else {
err = mbox_process_msgs_up(dev, msg);
- if (err)
- plt_err("Error %d handling 0x%x (%s)", err, msg->id,
- mbox_id2name(msg->id));
+ if (err) {
+ roc_trace_mbox_error("PRC_UPMSG_ERR", mbox_id2name(msg->id), err);
+ plt_err("Error %d handling 0x%x (%s)", err, msg->id, mbox_id2name(msg->id));
+ }
}
offset = mbox->rx_start + msg->next_msgoff;
}
@@ -930,7 +938,7 @@ roc_pf_vf_mbox_irq_cn20k(void *param)
plt_base_dbg("Proceeding to check mbox UP messages if any");
plt_write64(intr, dev->mbox_reg_base + RVU_VF_INT);
- plt_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev->vf);
+ roc_trace_mbox_interrupt("PFVF_IRQ_20K", dev_pf_func(dev->pf, dev->vf), intr, 0);
/* If interrupt occurred for down message */
if (intr & BIT_ULL(1))
@@ -954,7 +962,7 @@ roc_af_pf_mbox_irq_cn20k(void *param)
plt_base_dbg("Proceeding to check mbox UP messages if any");
plt_write64(intr, dev->mbox_reg_base + RVU_PF_INT);
- plt_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev->vf);
+ roc_trace_mbox_interrupt("AFPF_IRQ_20K", dev_pf_func(dev->pf, dev->vf), intr, 0);
/* If interrupt occurred for down message */
if (intr & BIT_ULL(1))
@@ -978,12 +986,12 @@ roc_pf_vf_mbox_irq(void *param)
plt_base_dbg("Proceeding to check mbox UP messages if any");
plt_write64(intr, dev->mbox_reg_base + RVU_VF_INT);
- plt_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev->vf);
/* Reading for UP/DOWN message, next message sending will be delayed
* by 1ms until this region is zeroed mbox_wait_for_zero()
*/
mbox_data = plt_read64(dev->mbox_reg_base + RVU_VF_VFPF_MBOX0);
+ roc_trace_mbox_interrupt("PFVF_IRQ", dev_pf_func(dev->pf, dev->vf), intr, mbox_data);
/* If interrupt occurred for down message */
if (mbox_data & MBOX_DOWN_MSG) {
mbox_data &= ~MBOX_DOWN_MSG;
@@ -1015,12 +1023,12 @@ roc_af_pf_mbox_irq(void *param)
plt_base_dbg("Proceeding to check mbox UP messages if any");
plt_write64(intr, dev->mbox_reg_base + RVU_PF_INT);
- plt_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev->vf);
/* Reading for UP/DOWN message, next message sending will be delayed
* by 1ms until this region is zeroed mbox_wait_for_zero()
*/
mbox_data = plt_read64(dev->mbox_reg_base + RVU_PF_PFAF_MBOX0);
+ roc_trace_mbox_interrupt("AFPF_IRQ", dev_pf_func(dev->pf, dev->vf), intr, mbox_data);
/* If interrupt occurred for down message */
if (mbox_data & MBOX_DOWN_MSG) {
mbox_data &= ~MBOX_DOWN_MSG;
@@ -1327,8 +1335,7 @@ vf_flr_handle_msg(void *param, dev_intr_t *flr)
for (vf = 0; vf < max_vf; vf++) {
if (flr->bits[vf / max_bits] & BIT_ULL(vf % max_bits)) {
- plt_base_dbg("Process FLR vf:%d request (pf:%d, vf:%d)",
- vf, dev->pf, dev->vf);
+ roc_trace_mbox_vf_flr("PRC_VFFLR", vf, dev_pf_func(dev->pf, dev->vf));
/* Inform AF about VF reset */
vf_flr_send_msg(dev, vf);
flr->bits[vf / max_bits] &= ~(BIT_ULL(vf % max_bits));
diff --git a/drivers/common/cnxk/roc_mbox.c b/drivers/common/cnxk/roc_mbox.c
index eb5bd771fe..e1b0f2fa63 100644
--- a/drivers/common/cnxk/roc_mbox.c
+++ b/drivers/common/cnxk/roc_mbox.c
@@ -263,6 +263,34 @@ mbox_wait_for_zero(struct mbox *mbox, int devid)
return data == 0;
}
+static inline void
+mbox_debug_region(const char *ops, struct mbox *mbox, int devid, uint64_t intr_val, int rc)
+{
+ struct mbox_dev *mdev = &mbox->dev[devid];
+ struct mbox_msghdr *msg_rx, *msg_tx;
+ uint64_t offset_rx, offset_tx;
+ char buf[BUFSIZ];
+
+ if (!plt_trace_point_is_enabled(&__cnxk_trace_mbox_region))
+ return;
+
+ /* Get messages from mbox */
+ offset_tx = mbox->tx_start + PLT_ALIGN(sizeof(struct mbox_hdr), MBOX_MSG_ALIGN);
+
+ msg_tx = (struct mbox_msghdr *)((uintptr_t)mdev->mbase + offset_tx);
+ snprintf(buf, BUFSIZ, "%s_TX", ops);
+ /* Only while sending message choose a rand identifier for the msg */
+ if (intr_val)
+ msg_tx->cookie = rand();
+ roc_trace_mbox_region(buf, mbox_id2name(msg_tx->id), msg_tx->pcifunc, (int)intr_val,
+ msg_tx->cookie);
+
+ offset_rx = mbox->rx_start + PLT_ALIGN(sizeof(struct mbox_hdr), MBOX_MSG_ALIGN);
+ msg_rx = (struct mbox_msghdr *)((uintptr_t)mdev->mbase + offset_rx);
+ snprintf(buf, BUFSIZ, "%s_RX", ops);
+ roc_trace_mbox_region(buf, mbox_id2name(msg_rx->id), msg_rx->pcifunc, rc, msg_rx->cookie);
+}
+
static void
mbox_msg_send_data(struct mbox *mbox, int devid, uint8_t data)
{
@@ -291,6 +319,8 @@ mbox_msg_send_data(struct mbox *mbox, int devid, uint8_t data)
(volatile void *)(mbox->reg_base + (mbox->trigger | (devid << mbox->tr_shift))));
intr_val |= (uint64_t)data;
+
+ mbox_debug_region("SND_MSG", mbox, devid, intr_val, 0);
/* The interrupt should be fired after num_msgs is written
* to the shared memory
*/
@@ -342,6 +372,8 @@ mbox_get_rsp(struct mbox *mbox, int devid, void **msg)
if (msg != NULL)
*msg = msghdr;
+ mbox_debug_region("GET_RSP", mbox, devid, 0, msghdr->rc);
+
return msghdr->rc;
}
@@ -436,6 +468,8 @@ mbox_wait(struct mbox *mbox, int devid, uint32_t rst_timo)
__atomic_load_n(&mdev->msgs_acked, __ATOMIC_ACQUIRE),
tx_hdr->num_msgs, rx_hdr->num_msgs, mdev->msg_size, mdev->rsp_size);
+ mbox_debug_region("MBX_TMO", mbox, devid, 0, -EIO);
+
return -EIO;
}
}
diff --git a/drivers/common/cnxk/roc_mbox.h b/drivers/common/cnxk/roc_mbox.h
index e7a916d61c..3ca8442b55 100644
--- a/drivers/common/cnxk/roc_mbox.h
+++ b/drivers/common/cnxk/roc_mbox.h
@@ -32,6 +32,8 @@ struct mbox_msghdr {
uint16_t __io ver;
/* Offset of next msg within mailbox region */
uint16_t __io next_msgoff;
+ /* Unique ID for trace debug */
+ uint16_t __io cookie;
int __io rc; /* Msg processed response code */
};
diff --git a/drivers/common/cnxk/roc_platform.c b/drivers/common/cnxk/roc_platform.c
index f1e0a93d97..f7678280f5 100644
--- a/drivers/common/cnxk/roc_platform.c
+++ b/drivers/common/cnxk/roc_platform.c
@@ -3,6 +3,7 @@
*/
#include <rte_log.h>
+#include <rte_trace_point_register.h>
#include "roc_api.h"
@@ -85,6 +86,14 @@ roc_plt_init(void)
return 0;
}
+/* ROC trace points */
+RTE_TRACE_POINT_REGISTER(cnxk_trace_mbox_region, cnxk.mbox.region)
+RTE_TRACE_POINT_REGISTER(cnxk_trace_mbox_process, cnxk.mbox.process)
+RTE_TRACE_POINT_REGISTER(cnxk_trace_mbox_interrupt, cnxk.mbox.interrupt)
+RTE_TRACE_POINT_REGISTER(cnxk_trace_mbox_vf_flr, cnxk.mbox.vf.flr)
+RTE_TRACE_POINT_REGISTER(cnxk_trace_mbox_vf_pf_handle, cnxk.mbox.vf.pf.handle)
+RTE_TRACE_POINT_REGISTER(cnxk_trace_mbox_error, cnxk.mbox.error)
+
RTE_LOG_REGISTER_SUFFIX(cnxk_logtype_base, base, INFO);
RTE_LOG_REGISTER_SUFFIX(cnxk_logtype_mbox, mbox, NOTICE);
RTE_LOG_REGISTER_SUFFIX(cnxk_logtype_cpt, crypto, NOTICE);
diff --git a/drivers/common/cnxk/roc_platform.h b/drivers/common/cnxk/roc_platform.h
index df4f88f288..ef2aaa312f 100644
--- a/drivers/common/cnxk/roc_platform.h
+++ b/drivers/common/cnxk/roc_platform.h
@@ -5,12 +5,12 @@
#ifndef _ROC_PLATFORM_H_
#define _ROC_PLATFORM_H_
-#include <rte_compat.h>
+#include <bus_pci_driver.h>
#include <rte_alarm.h>
#include <rte_bitmap.h>
-#include <bus_pci_driver.h>
#include <rte_byteorder.h>
#include <rte_common.h>
+#include <rte_compat.h>
#include <rte_cycles.h>
#include <rte_ether.h>
#include <rte_interrupts.h>
@@ -25,6 +25,7 @@
#include <rte_string_fns.h>
#include <rte_tailq.h>
#include <rte_telemetry.h>
+#include <rte_trace_point.h>
#include "eal_filesystem.h"
@@ -260,6 +261,9 @@ plt_thread_is_valid(plt_thread_t thr)
#define plt_tel_data_add_dict_u64 rte_tel_data_add_dict_uint
#define plt_telemetry_register_cmd rte_telemetry_register_cmd
+/* Trace */
+#define plt_trace_point_is_enabled rte_trace_point_is_enabled
+
/* Log */
extern int cnxk_logtype_base;
#define RTE_LOGTYPE_base cnxk_logtype_base
@@ -374,9 +378,82 @@ plt_lmt_region_reserve_aligned(const char *name, size_t len, uint32_t align)
/* To ensure returned memory is physically contiguous, bounding
* the start and end address in 2M range.
*/
- return rte_memzone_reserve_bounded(name, len, SOCKET_ID_ANY,
- RTE_MEMZONE_IOVA_CONTIG,
- align, RTE_PGSIZE_2M);
+ return rte_memzone_reserve_bounded(name, len, SOCKET_ID_ANY, RTE_MEMZONE_IOVA_CONTIG, align,
+ RTE_PGSIZE_2M);
+}
+
+/* ROC trace points */
+RTE_TRACE_POINT(cnxk_trace_mbox_region,
+ RTE_TRACE_POINT_ARGS(const char *func, const char *msg, uint16_t pcifunc, int data,
+ uint16_t cookie),
+ rte_trace_point_emit_string(func);
+ rte_trace_point_emit_string(msg); rte_trace_point_emit_u16(pcifunc);
+ rte_trace_point_emit_int(data); rte_trace_point_emit_u16(cookie);)
+
+RTE_TRACE_POINT(cnxk_trace_mbox_process,
+ RTE_TRACE_POINT_ARGS(const char *func, const char *msg, uint16_t num_msgs,
+ uint16_t pcifunc),
+ rte_trace_point_emit_string(func);
+ rte_trace_point_emit_string(msg); rte_trace_point_emit_u16(num_msgs);
+ rte_trace_point_emit_u16(pcifunc);)
+
+RTE_TRACE_POINT(cnxk_trace_mbox_interrupt,
+ RTE_TRACE_POINT_ARGS(const char *func, int pcifunc, uint64_t intr,
+ uint64_t mbox_data),
+ rte_trace_point_emit_string(func);
+ rte_trace_point_emit_int(pcifunc); rte_trace_point_emit_u64(intr);
+ rte_trace_point_emit_u64(mbox_data);)
+
+RTE_TRACE_POINT(cnxk_trace_mbox_vf_flr,
+ RTE_TRACE_POINT_ARGS(const char *func, uint16_t from_vf, uint16_t pcifunc),
+ rte_trace_point_emit_string(func);
+ rte_trace_point_emit_u16(from_vf); rte_trace_point_emit_u16(pcifunc);)
+
+RTE_TRACE_POINT(cnxk_trace_mbox_vf_pf_handle,
+ RTE_TRACE_POINT_ARGS(const char *ops, uint16_t pcifunc, int data),
+ rte_trace_point_emit_string(ops);
+ rte_trace_point_emit_u16(pcifunc); rte_trace_point_emit_int(data);)
+
+RTE_TRACE_POINT(cnxk_trace_mbox_error,
+ RTE_TRACE_POINT_ARGS(const char *func, const char *msg, int data),
+ rte_trace_point_emit_string(func);
+ rte_trace_point_emit_string(msg); rte_trace_point_emit_int(data);)
+
+static inline void
+roc_trace_mbox_region(const char *func, const char *msg, uint16_t pcifunc, int data,
+ uint16_t cookie)
+{
+ cnxk_trace_mbox_region(func, msg, pcifunc, data, cookie);
+}
+
+static inline void
+roc_trace_mbox_process(const char *func, const char *msg, uint16_t num_msgs, uint16_t pcifunc)
+{
+ cnxk_trace_mbox_process(func, msg, num_msgs, pcifunc);
+}
+
+static inline void
+roc_trace_mbox_interrupt(const char *func, int pcifunc, uint64_t intr, uint64_t mbox_data)
+{
+ cnxk_trace_mbox_interrupt(func, pcifunc, intr, mbox_data);
+}
+
+static inline void
+roc_trace_mbox_vf_pf_handle(const char *func, uint16_t pcifunc, int data)
+{
+ cnxk_trace_mbox_vf_pf_handle(func, pcifunc, data);
+}
+
+static inline void
+roc_trace_mbox_error(const char *func, const char *msg, int data)
+{
+ cnxk_trace_mbox_error(func, msg, data);
+}
+
+static inline void
+roc_trace_mbox_vf_flr(const char *func, uint16_t from_vf, uint16_t pcifunc)
+{
+ cnxk_trace_mbox_vf_flr(func, from_vf, pcifunc);
}
#endif /* _ROC_PLATFORM_H_ */
--
2.46.0.469.g4590f2e941
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [EXTERNAL] [PATCH 2/2] common/cnxk: add mailbox debug trace
2024-11-04 13:08 ` [PATCH 2/2] common/cnxk: add mailbox debug trace Harman Kalra
@ 2024-11-07 8:04 ` Jerin Jacob
2024-11-14 16:38 ` Stephen Hemminger
1 sibling, 0 replies; 6+ messages in thread
From: Jerin Jacob @ 2024-11-07 8:04 UTC (permalink / raw)
To: Harman Kalra, Nithin Kumar Dabilpuram, Kiran Kumar Kokkilagadda,
Sunil Kumar Kori, Satha Koteswara Rao Kottidi, Harman Kalra
Cc: dev
[-- Attachment #1: Type: text/plain, Size: 2348 bytes --]
> -----Original Message-----
> From: Harman Kalra <hkalra@marvell.com>
> Sent: Monday, November 4, 2024 6:38 PM
> To: Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>; Kiran Kumar
> Kokkilagadda <kirankumark@marvell.com>; Sunil Kumar Kori
> <skori@marvell.com>; Satha Koteswara Rao Kottidi
> <skoteshwar@marvell.com>; Harman Kalra <hkalra@marvell.com>
> Cc: dev@dpdk.org
> Subject: [EXTERNAL] [PATCH 2/2] common/cnxk: add mailbox debug trace
>
> Implementing mailbox debug trace points to trace function calls. Signed-off-by:
> Harman Kalra <hkalra@ marvell. com> --- drivers/common/cnxk/roc_dev. c | 89
> ++++++++++++++++-------------- drivers/common/cnxk/roc_mbox. c | 34
> ++++++++++++ drivers/common/cnxk/roc_mbox. h
> Implementing mailbox debug trace points to trace function calls.
>
> Signed-off-by: Harman Kalra <hkalra@marvell.com>
> ---
> drivers/common/cnxk/roc_dev.c | 89 ++++++++++++++++--------------
> drivers/common/cnxk/roc_mbox.c | 34 ++++++++++++
> drivers/common/cnxk/roc_mbox.h | 2 +
> drivers/common/cnxk/roc_platform.c | 9 +++
> drivers/common/cnxk/roc_platform.h | 87 +++++++++++++++++++++++++++--
> 5 files changed, 175 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/common/cnxk/roc_dev.c b/drivers/common/cnxk/roc_dev.c
> index 1127d8185c..e9bf8c01b4 100644
> --- a/drivers/common/cnxk/roc_dev.c
> +++ b/drivers/common/cnxk/roc_dev.c
> @@ -26,7 +26,6 @@
>
> /* RVU PF interrupt status as received from AF*/ #define
> RVU_PF_INTR_STATUS 0x3
> -
This change may not be needed.
Also, Fix following issues
### [PATCH] common/cnxk: fix mailbox timeout issues
Warning in drivers/common/cnxk/roc_dev.c:
Warning in drivers/common/cnxk/roc_mbox.c:
Using __atomic_xxx/__ATOMIC_XXX built-ins, prefer rte_atomic_xxx/rte_memory_order_xxx
### [PATCH] common/cnxk: add mailbox debug trace
WARNING:LONG_LINE: line length of 108 exceeds 100 columns
#199: FILE: drivers/common/cnxk/roc_dev.c:907:
+ roc_trace_mbox_process("PRC_UPMSG", mbox_id2name(msg->id), req_hdr->num_msgs, msg->pcifunc);
WARNING:LONG_LINE: line length of 108 exceeds 100 columns
#212: FILE: drivers/common/cnxk/roc_dev.c:917:
+ plt_err("Error %d handling 0x%x (%s)", err, msg->id, mbox_id2name(msg->id));
);
[-- Attachment #2: winmail.dat --]
[-- Type: application/ms-tnef, Size: 15716 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] common/cnxk: fix mailbox timeout issues
2024-11-04 13:07 [PATCH 1/2] common/cnxk: fix mailbox timeout issues Harman Kalra
2024-11-04 13:08 ` [PATCH 2/2] common/cnxk: add mailbox debug trace Harman Kalra
@ 2024-11-14 9:31 ` Harman Kalra
2024-11-14 9:31 ` [PATCH v2 2/2] common/cnxk: add mailbox debug trace Harman Kalra
1 sibling, 1 reply; 6+ messages in thread
From: Harman Kalra @ 2024-11-14 9:31 UTC (permalink / raw)
To: Nithin Dabilpuram, Kiran Kumar K, Sunil Kumar Kori, Satha Rao,
Harman Kalra
Cc: dev
Found couple of reasons causing mailbox timeout:
* msgs_acked value by interrupt thread was not getting synced to thread
polling it.
* mbox_data value sent by AF was seen 0 causing neither down nor up msg
processing.
Fixes: 9bd368ca311a ("common/cnxk: enable PF VF mailbox")
Fixes: 61deac72abbf ("common/cnxk: add CN20KA mbox support")
Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
V2:
* Replaced gcc builtin __atomic_xxx intrinsics with corresponding
rte_atomic_xxx stdatomic API
drivers/common/cnxk/roc_dev.c | 106 ++++++++++++++++++++++++-----
drivers/common/cnxk/roc_dev_priv.h | 11 +++
drivers/common/cnxk/roc_mbox.c | 12 ++--
drivers/common/cnxk/roc_platform.h | 5 ++
4 files changed, 111 insertions(+), 23 deletions(-)
diff --git a/drivers/common/cnxk/roc_dev.c b/drivers/common/cnxk/roc_dev.c
index 32409f2ef3..0d1f9c4564 100644
--- a/drivers/common/cnxk/roc_dev.c
+++ b/drivers/common/cnxk/roc_dev.c
@@ -401,8 +401,7 @@ vf_pf_process_up_msgs(struct dev *dev, uint16_t vf)
offset = mbox->rx_start + msg->next_msgoff;
}
mbox_reset(mbox, vf);
- mdev->msgs_acked = msgs_acked;
- plt_wmb();
+ plt_atomic_store_explicit(&mdev->msgs_acked, msgs_acked, plt_memory_order_release);
return i;
}
@@ -542,8 +541,7 @@ process_msgs(struct dev *dev, struct mbox *mbox)
mbox_reset(mbox, 0);
/* Update acked if someone is waiting a message - mbox_wait is waiting */
- mdev->msgs_acked = msgs_acked;
- plt_wmb();
+ plt_atomic_store_explicit(&mdev->msgs_acked, msgs_acked, plt_memory_order_release);
}
/* Copies the message received from AF and sends it to VF */
@@ -920,12 +918,59 @@ process_msgs_up(struct dev *dev, struct mbox *mbox)
}
}
+/* IRQ to VF from PF - VF context (interrupt thread) */
+static void
+roc_pf_vf_mbox_irq_cn20k(void *param)
+{
+ struct dev *dev = param;
+ uint64_t intr;
+
+ intr = plt_read64(dev->mbox_reg_base + RVU_VF_INT);
+ if (intr == 0)
+ plt_base_dbg("Proceeding to check mbox UP messages if any");
+
+ plt_write64(intr, dev->mbox_reg_base + RVU_VF_INT);
+ plt_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev->vf);
+
+ /* If interrupt occurred for down message */
+ if (intr & BIT_ULL(1))
+ /* First process all configuration messages */
+ process_msgs(dev, dev->mbox);
+
+ /* If interrupt occurred for UP message */
+ if (intr & BIT_ULL(0))
+ process_msgs_up(dev, &dev->mbox_up);
+}
+
+/* IRQ to PF from AF - PF context (interrupt thread) */
+static void
+roc_af_pf_mbox_irq_cn20k(void *param)
+{
+ struct dev *dev = param;
+ uint64_t intr;
+
+ intr = plt_read64(dev->mbox_reg_base + RVU_PF_INT);
+ if (intr == 0)
+ plt_base_dbg("Proceeding to check mbox UP messages if any");
+
+ plt_write64(intr, dev->mbox_reg_base + RVU_PF_INT);
+ plt_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev->vf);
+
+ /* If interrupt occurred for down message */
+ if (intr & BIT_ULL(1))
+ process_msgs(dev, dev->mbox);
+
+ /* If interrupt occurred for up message */
+ if (intr & BIT_ULL(0))
+ process_msgs_up(dev, &dev->mbox_up);
+}
+
/* IRQ to VF from PF - VF context (interrupt thread) */
static void
roc_pf_vf_mbox_irq(void *param)
{
struct dev *dev = param;
- uint64_t mbox_data;
+ uint64_t mbox_data = 0;
uint64_t intr;
intr = plt_read64(dev->mbox_reg_base + RVU_VF_INT);
@@ -940,7 +985,7 @@ roc_pf_vf_mbox_irq(void *param)
*/
mbox_data = plt_read64(dev->mbox_reg_base + RVU_VF_VFPF_MBOX0);
/* If interrupt occurred for down message */
- if (mbox_data & MBOX_DOWN_MSG || intr & BIT_ULL(1)) {
+ if (mbox_data & MBOX_DOWN_MSG) {
mbox_data &= ~MBOX_DOWN_MSG;
plt_write64(mbox_data, dev->mbox_reg_base + RVU_VF_VFPF_MBOX0);
@@ -948,7 +993,7 @@ roc_pf_vf_mbox_irq(void *param)
process_msgs(dev, dev->mbox);
}
/* If interrupt occurred for UP message */
- if (mbox_data & MBOX_UP_MSG || intr & BIT_ULL(0)) {
+ if (mbox_data & MBOX_UP_MSG) {
mbox_data &= ~MBOX_UP_MSG;
plt_write64(mbox_data, dev->mbox_reg_base + RVU_VF_VFPF_MBOX0);
@@ -962,7 +1007,7 @@ static void
roc_af_pf_mbox_irq(void *param)
{
struct dev *dev = param;
- uint64_t mbox_data;
+ uint64_t mbox_data = 0;
uint64_t intr;
intr = plt_read64(dev->mbox_reg_base + RVU_PF_INT);
@@ -977,7 +1022,7 @@ roc_af_pf_mbox_irq(void *param)
*/
mbox_data = plt_read64(dev->mbox_reg_base + RVU_PF_PFAF_MBOX0);
/* If interrupt occurred for down message */
- if (mbox_data & MBOX_DOWN_MSG || intr & BIT_ULL(1)) {
+ if (mbox_data & MBOX_DOWN_MSG) {
mbox_data &= ~MBOX_DOWN_MSG;
plt_write64(mbox_data, dev->mbox_reg_base + RVU_PF_PFAF_MBOX0);
@@ -985,7 +1030,7 @@ roc_af_pf_mbox_irq(void *param)
process_msgs(dev, dev->mbox);
}
/* If interrupt occurred for up message */
- if (mbox_data & MBOX_UP_MSG || intr & BIT_ULL(0)) {
+ if (mbox_data & MBOX_UP_MSG) {
mbox_data &= ~MBOX_UP_MSG;
plt_write64(mbox_data, dev->mbox_reg_base + RVU_PF_PFAF_MBOX0);
@@ -1045,7 +1090,8 @@ mbox_register_pf_irq(struct plt_pci_device *pci_dev, struct dev *dev)
}
}
/* MBOX interrupt AF <-> PF */
- rc = dev_irq_register(intr_handle, roc_af_pf_mbox_irq, dev, dev->mbox_plat->pfaf_vec);
+ rc = dev_irq_register(intr_handle, dev->mbox_plat->ops->af_pf_mbox_irq, dev,
+ dev->mbox_plat->pfaf_vec);
if (rc) {
plt_err("Fail to register AF<->PF mbox irq");
return rc;
@@ -1073,7 +1119,8 @@ mbox_register_vf_irq(struct plt_pci_device *pci_dev, struct dev *dev)
plt_write64(~0ull, dev->mbox_reg_base + RVU_VF_INT_ENA_W1C);
/* MBOX interrupt PF <-> VF */
- rc = dev_irq_register(intr_handle, roc_pf_vf_mbox_irq, dev, RVU_VF_INT_VEC_MBOX);
+ rc = dev_irq_register(intr_handle, dev->mbox_plat->ops->pf_vf_mbox_irq, dev,
+ RVU_VF_INT_VEC_MBOX);
if (rc) {
plt_err("Fail to register PF<->VF mbox irq");
return rc;
@@ -1127,7 +1174,8 @@ mbox_unregister_pf_irq(struct plt_pci_device *pci_dev, struct dev *dev)
}
/* MBOX interrupt AF <-> PF */
- dev_irq_unregister(intr_handle, roc_af_pf_mbox_irq, dev, dev->mbox_plat->pfaf_vec);
+ dev_irq_unregister(intr_handle, dev->mbox_plat->ops->af_pf_mbox_irq, dev,
+ dev->mbox_plat->pfaf_vec);
}
static void
@@ -1139,7 +1187,8 @@ mbox_unregister_vf_irq(struct plt_pci_device *pci_dev, struct dev *dev)
plt_write64(~0ull, dev->mbox_reg_base + RVU_VF_INT_ENA_W1C);
/* Unregister the interrupt handler */
- dev_irq_unregister(intr_handle, roc_pf_vf_mbox_irq, dev, RVU_VF_INT_VEC_MBOX);
+ dev_irq_unregister(intr_handle, dev->mbox_plat->ops->pf_vf_mbox_irq, dev,
+ RVU_VF_INT_VEC_MBOX);
}
void
@@ -1599,10 +1648,17 @@ dev_cache_line_size_valid(void)
return true;
}
-static void
+static int
mbox_platform_changes(struct mbox_platform *mbox_plat, uintptr_t bar2, uintptr_t bar4, bool is_vf)
{
- int i;
+ int i, rc = 0;
+
+ /* Allocate memory for device ops */
+ mbox_plat->ops = plt_zmalloc(sizeof(struct mbox_ops), 0);
+ if (mbox_plat->ops == NULL) {
+ rc = -ENOMEM;
+ goto fail;
+ }
if (roc_model_is_cn20k()) {
/* For CN20K, AF allocates mbox memory in DRAM and writes PF
@@ -1613,6 +1669,9 @@ mbox_platform_changes(struct mbox_platform *mbox_plat, uintptr_t bar2, uintptr_t
mbox_plat->mbox_region_base =
bar2 + (RVU_PFX_FUNC_PFAF_MBOX +
((uint64_t)RVU_BLOCK_ADDR_MBOX << RVU_FUNC_BLKADDR_SHIFT));
+ /* Mbox operations */
+ mbox_plat->ops->af_pf_mbox_irq = roc_af_pf_mbox_irq_cn20k;
+ mbox_plat->ops->pf_vf_mbox_irq = roc_pf_vf_mbox_irq_cn20k;
/* Interrupt vectors */
mbox_plat->pfaf_vec = RVU_MBOX_PF_INT_VEC_AFPF_MBOX;
mbox_plat->pfvf_mbox0_vec = RVU_MBOX_PF_INT_VEC_VFPF_MBOX0;
@@ -1630,6 +1689,9 @@ mbox_platform_changes(struct mbox_platform *mbox_plat, uintptr_t bar2, uintptr_t
} else {
mbox_plat->mbox_reg_base = bar2;
mbox_plat->mbox_region_base = bar4;
+ /* Mbox operations */
+ mbox_plat->ops->af_pf_mbox_irq = roc_af_pf_mbox_irq;
+ mbox_plat->ops->pf_vf_mbox_irq = roc_pf_vf_mbox_irq;
mbox_plat->pfaf_vec = RVU_PF_INT_VEC_AFPF_MBOX;
mbox_plat->pfvf_mbox0_vec = RVU_PF_INT_VEC_VFPF_MBOX0;
mbox_plat->pfvf_mbox1_vec = RVU_PF_INT_VEC_VFPF_MBOX1;
@@ -1647,6 +1709,8 @@ mbox_platform_changes(struct mbox_platform *mbox_plat, uintptr_t bar2, uintptr_t
if (roc_model_is_cn10k())
mbox_plat->mbox_region_base = bar2 + RVU_VF_MBOX_REGION;
}
+fail:
+ return rc;
}
int
@@ -1678,7 +1742,12 @@ dev_init(struct dev *dev, struct plt_pci_device *pci_dev)
rc = -ENOMEM;
goto fail;
}
- mbox_platform_changes(dev->mbox_plat, bar2, bar4, is_vf);
+
+ if (mbox_platform_changes(dev->mbox_plat, bar2, bar4, is_vf)) {
+ plt_err("Failed to populate platform specific changes");
+ rc = -ENOMEM;
+ goto mbox_plat_free;
+ }
mbox_reg_base = dev->mbox_plat->mbox_reg_base;
mbox_region_base = dev->mbox_plat->mbox_region_base;
@@ -1824,6 +1893,8 @@ dev_init(struct dev *dev, struct plt_pci_device *pci_dev)
mbox_fini(dev->mbox);
mbox_fini(&dev->mbox_up);
error:
+ plt_free(dev->mbox_plat->ops);
+mbox_plat_free:
plt_free(dev->mbox_plat);
fail:
return rc;
@@ -1883,6 +1954,7 @@ dev_fini(struct dev *dev, struct plt_pci_device *pci_dev)
mbox_fini(mbox);
dev->mbox_active = 0;
+ plt_free(dev->mbox_plat->ops);
plt_free(dev->mbox_plat);
/* Disable MSIX vectors */
dev_irqs_disable(intr_handle);
diff --git a/drivers/common/cnxk/roc_dev_priv.h b/drivers/common/cnxk/roc_dev_priv.h
index c766183196..a8e40fa9d5 100644
--- a/drivers/common/cnxk/roc_dev_priv.h
+++ b/drivers/common/cnxk/roc_dev_priv.h
@@ -106,6 +106,15 @@ struct mbox_sync {
pthread_mutex_t mutex;
};
+/* AF PF VF mbox interrupt callbacks */
+typedef void (*af_pf_mbox_irq_t)(void *param);
+typedef void (*pf_vf_mbox_irq_t)(void *param);
+
+struct mbox_ops {
+ af_pf_mbox_irq_t af_pf_mbox_irq;
+ pf_vf_mbox_irq_t pf_vf_mbox_irq;
+};
+
struct mbox_platform {
uint8_t pfaf_vec;
uint8_t pfvf_mbox0_vec;
@@ -120,6 +129,8 @@ struct mbox_platform {
uint64_t pfvf1_mbox_int_ena_w1c[MAX_VFPF_DWORD_BITS];
uintptr_t mbox_reg_base;
uintptr_t mbox_region_base;
+ /* Mbox operations */
+ struct mbox_ops *ops;
};
struct dev {
diff --git a/drivers/common/cnxk/roc_mbox.c b/drivers/common/cnxk/roc_mbox.c
index db77babfdb..a3b96e6e2b 100644
--- a/drivers/common/cnxk/roc_mbox.c
+++ b/drivers/common/cnxk/roc_mbox.c
@@ -275,7 +275,7 @@ mbox_msg_send_data(struct mbox *mbox, int devid, uint8_t data)
tx_hdr->msg_size = mdev->msg_size;
mdev->msg_size = 0;
mdev->rsp_size = 0;
- mdev->msgs_acked = 0;
+ plt_atomic_store_explicit(&mdev->msgs_acked, 0, plt_memory_order_release);
/* num_msgs != 0 signals to the peer that the buffer has a number of
* messages. So this should be written after copying txmem
@@ -417,7 +417,8 @@ mbox_wait(struct mbox *mbox, int devid, uint32_t rst_timo)
* mdev->msgs_acked are incremented at process_msgs() in interrupt
* thread context.
*/
- while (mdev->num_msgs > mdev->msgs_acked) {
+ while (mdev->num_msgs > plt_atomic_load_explicit(&mdev->msgs_acked,
+ plt_memory_order_acquire)) {
plt_delay_us(sleep);
timeout += sleep;
if (timeout >= rst_timo) {
@@ -433,13 +434,12 @@ mbox_wait(struct mbox *mbox, int devid, uint32_t rst_timo)
"(tx/rx num_msgs: %d/%d), msg_size: %d, "
"rsp_size: %d",
devid, timeout, mdev->num_msgs,
- mdev->msgs_acked, tx_hdr->num_msgs,
- rx_hdr->num_msgs, mdev->msg_size,
- mdev->rsp_size);
+ plt_atomic_load_explicit(&mdev->msgs_acked,
+ plt_memory_order_acquire),
+ tx_hdr->num_msgs, rx_hdr->num_msgs, mdev->msg_size, mdev->rsp_size);
return -EIO;
}
- plt_rmb();
}
return 0;
}
diff --git a/drivers/common/cnxk/roc_platform.h b/drivers/common/cnxk/roc_platform.h
index df4f88f288..4076dbe94d 100644
--- a/drivers/common/cnxk/roc_platform.h
+++ b/drivers/common/cnxk/roc_platform.h
@@ -211,6 +211,11 @@ plt_thread_is_valid(plt_thread_t thr)
#define plt_io_rmb() rte_io_rmb()
#define plt_atomic_thread_fence rte_atomic_thread_fence
+#define plt_atomic_store_explicit rte_atomic_store_explicit
+#define plt_atomic_load_explicit rte_atomic_load_explicit
+#define plt_memory_order_release rte_memory_order_release
+#define plt_memory_order_acquire rte_memory_order_acquire
+
#define plt_bit_relaxed_get32 rte_bit_relaxed_get32
#define plt_bit_relaxed_set32 rte_bit_relaxed_set32
#define plt_bit_relaxed_clear32 rte_bit_relaxed_clear32
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] common/cnxk: add mailbox debug trace
2024-11-14 9:31 ` [PATCH v2 1/2] common/cnxk: fix mailbox timeout issues Harman Kalra
@ 2024-11-14 9:31 ` Harman Kalra
0 siblings, 0 replies; 6+ messages in thread
From: Harman Kalra @ 2024-11-14 9:31 UTC (permalink / raw)
To: Nithin Dabilpuram, Kiran Kumar K, Sunil Kumar Kori, Satha Rao,
Harman Kalra
Cc: dev
Implementing mailbox debug trace points to trace function calls.
Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
V2:
* Fixed line length exceeding 100 columns
drivers/common/cnxk/roc_dev.c | 87 ++++++++++++++++--------------
drivers/common/cnxk/roc_mbox.c | 34 ++++++++++++
drivers/common/cnxk/roc_mbox.h | 2 +
drivers/common/cnxk/roc_platform.c | 9 ++++
drivers/common/cnxk/roc_platform.h | 87 ++++++++++++++++++++++++++++--
5 files changed, 175 insertions(+), 44 deletions(-)
diff --git a/drivers/common/cnxk/roc_dev.c b/drivers/common/cnxk/roc_dev.c
index 0d1f9c4564..2dc437c491 100644
--- a/drivers/common/cnxk/roc_dev.c
+++ b/drivers/common/cnxk/roc_dev.c
@@ -26,7 +26,6 @@
/* RVU PF interrupt status as received from AF*/
#define RVU_PF_INTR_STATUS 0x3
-
static void *
mbox_mem_map(off_t off, size_t size)
{
@@ -83,6 +82,7 @@ pf_af_sync_msg(struct dev *dev, struct mbox_msghdr **rsp)
timeout += sleep;
if (timeout >= mbox->rsp_tmo) {
plt_err("Message timeout: %dms", mbox->rsp_tmo);
+ roc_trace_mbox_error("AFPF_SYNC", "RSP_TMO", -EIO);
rc = -EIO;
break;
}
@@ -137,6 +137,7 @@ af_pf_wait_msg(struct dev *dev, uint16_t vf, int num_msg)
timeout++;
if (timeout >= mbox->rsp_tmo) {
plt_err("Routed messages %d timeout: %dms", num_msg, mbox->rsp_tmo);
+ roc_trace_mbox_error("AFPF_WAIT", "RTD_MSG_TMO", -EIO);
break;
}
int_status = plt_read64(dev->mbox_reg_base + RVU_PF_INT);
@@ -149,16 +150,18 @@ af_pf_wait_msg(struct dev *dev, uint16_t vf, int num_msg)
plt_write64(~0ull, dev->mbox_reg_base + RVU_PF_INT_ENA_W1S);
req_hdr = (struct mbox_hdr *)((uintptr_t)mdev->mbase + mbox->rx_start);
- if (req_hdr->num_msgs != num_msg)
- plt_err("Routed messages: %d received: %d", num_msg,
- req_hdr->num_msgs);
+ if (req_hdr->num_msgs != num_msg) {
+ plt_err("Routed messages: %d received: %d", num_msg, req_hdr->num_msgs);
+ roc_trace_mbox_error("AFPF_WAIT", "RTD_INV_CNT", num_msg);
+ }
/* Get messages from mbox */
- offset = mbox->rx_start +
- PLT_ALIGN(sizeof(struct mbox_hdr), MBOX_MSG_ALIGN);
+ offset = mbox->rx_start + PLT_ALIGN(sizeof(struct mbox_hdr), MBOX_MSG_ALIGN);
for (i = 0; i < req_hdr->num_msgs; i++) {
msg = (struct mbox_msghdr *)((uintptr_t)mdev->mbase + offset);
size = mbox->rx_start + msg->next_msgoff - offset;
+ roc_trace_mbox_process("AFPF_WAIT", mbox_id2name(msg->id), req_hdr->num_msgs,
+ msg->pcifunc);
/* Reserve PF/VF mbox message */
size = PLT_ALIGN(size, MBOX_MSG_ALIGN);
@@ -288,6 +291,8 @@ vf_pf_process_msgs(struct dev *dev, uint16_t vf)
/* RVU_PF_FUNC_S */
msg->pcifunc = dev_pf_func(dev->pf, vf);
+ roc_trace_mbox_process("VFPF_MSG", mbox_id2name(msg->id), req_hdr->num_msgs,
+ msg->pcifunc);
if (msg->id == MBOX_MSG_READY) {
struct ready_msg_rsp *rsp;
@@ -333,8 +338,7 @@ vf_pf_process_msgs(struct dev *dev, uint16_t vf)
}
if (routed > 0) {
- plt_base_dbg("pf:%d routed %d messages from vf:%d to AF",
- dev->pf, routed, vf);
+ roc_trace_mbox_vf_pf_handle("RTG_AF", dev_pf_func(dev->pf, vf), routed);
/* PF will send the messages to AF and wait for responses */
af_pf_wait_msg(dev, vf, routed);
mbox_reset(dev->mbox, 0);
@@ -343,8 +347,7 @@ vf_pf_process_msgs(struct dev *dev, uint16_t vf)
/* Send mbox responses to VF */
if (mdev->num_msgs) {
- plt_base_dbg("pf:%d reply %d messages to vf:%d", dev->pf,
- mdev->num_msgs, vf);
+ roc_trace_mbox_vf_pf_handle("RSP_VF", dev_pf_func(dev->pf, vf), mdev->num_msgs);
mbox_msg_send(mbox, vf);
}
@@ -378,25 +381,24 @@ vf_pf_process_up_msgs(struct dev *dev, uint16_t vf)
switch (msg->id) {
case MBOX_MSG_CGX_LINK_EVENT:
- plt_base_dbg("PF: Msg 0x%x (%s) fn:0x%x (pf:%d,vf:%d)",
- msg->id, mbox_id2name(msg->id),
- msg->pcifunc, dev_get_pf(msg->pcifunc),
- dev_get_vf(msg->pcifunc));
+ roc_trace_mbox_process("VFPF_UPMSG_CGX", mbox_id2name(msg->id),
+ req_hdr->num_msgs, msg->pcifunc);
break;
case MBOX_MSG_CGX_PTP_RX_INFO:
- plt_base_dbg("PF: Msg 0x%x (%s) fn:0x%x (pf:%d,vf:%d)",
- msg->id, mbox_id2name(msg->id),
- msg->pcifunc, dev_get_pf(msg->pcifunc),
- dev_get_vf(msg->pcifunc));
+ roc_trace_mbox_process("VFPF_UPMSG_PTP", mbox_id2name(msg->id),
+ req_hdr->num_msgs, msg->pcifunc);
break;
default:
if (roc_rvu_lf_msg_id_range_check(dev->roc_rvu_lf, msg->id))
plt_base_dbg("PF: Msg 0x%x fn:0x%x (pf:%d,vf:%d)",
msg->id, msg->pcifunc, dev_get_pf(msg->pcifunc),
dev_get_vf(msg->pcifunc));
- else
- plt_err("Not handled UP msg 0x%x (%s) func:0x%x",
- msg->id, mbox_id2name(msg->id), msg->pcifunc);
+ else {
+ roc_trace_mbox_process("UNHND_UPMSG", mbox_id2name(msg->id),
+ req_hdr->num_msgs, msg->pcifunc);
+ plt_err("Not handled UP msg 0x%x (%s) func:0x%x", msg->id,
+ mbox_id2name(msg->id), msg->pcifunc);
+ }
}
offset = mbox->rx_start + msg->next_msgoff;
}
@@ -446,8 +448,7 @@ roc_vf_pf_mbox_irq(void *param)
if (!intr)
continue;
- plt_base_dbg("vfpf: %d intr: 0x%" PRIx64 " (pf:%d, vf:%d)", vfpf, intr, dev->pf,
- dev->vf);
+ roc_trace_mbox_interrupt("VFPF_IRQ", dev_pf_func(dev->pf, dev->vf), intr, 0);
/* Save and clear intr bits */
intrb.bits[vfpf] |= intr;
@@ -489,9 +490,8 @@ process_msgs(struct dev *dev, struct mbox *mbox)
msg = (struct mbox_msghdr *)((uintptr_t)mdev->mbase + offset);
msgs_acked++;
- plt_base_dbg("Message 0x%x (%s) pf:%d/vf:%d", msg->id,
- mbox_id2name(msg->id), dev_get_pf(msg->pcifunc),
- dev_get_vf(msg->pcifunc));
+ roc_trace_mbox_process("PRC_MSG", mbox_id2name(msg->id), req_hdr->num_msgs,
+ msg->pcifunc);
switch (msg->id) {
/* Add message id's that are handled here */
@@ -514,6 +514,8 @@ process_msgs(struct dev *dev, struct mbox *mbox)
} else {
plt_err("Message (%s) response has err=%d",
mbox_id2name(msg->id), msg->rc);
+ roc_trace_mbox_error("PRC_MSG_ERR", mbox_id2name(msg->id),
+ msg->rc);
}
}
break;
@@ -526,14 +528,19 @@ process_msgs(struct dev *dev, struct mbox *mbox)
} else {
plt_err("Message (%s) response has err=%d",
mbox_id2name(msg->id), msg->rc);
+ roc_trace_mbox_error("PRC_MSG_ERR", mbox_id2name(msg->id),
+ msg->rc);
}
}
break;
default:
- if (msg->rc)
+ if (msg->rc) {
plt_err("Message (%s) response has err=%d (%s)",
mbox_id2name(msg->id), msg->rc, roc_error_msg_get(msg->rc));
+ roc_trace_mbox_error("PRC_MSG_INV_RSP", mbox_id2name(msg->id),
+ msg->rc);
+ }
break;
}
offset = mbox->rx_start + msg->next_msgoff;
@@ -564,8 +571,8 @@ pf_vf_mbox_send_up_msg(struct dev *dev, void *rec_msg)
if (!(dev->active_vfs[vf / max_bits] & (BIT_ULL(vf))))
continue;
- plt_base_dbg("(%s) size: %zx to VF: %d",
- mbox_id2name(msg->hdr.id), size, vf);
+ roc_trace_mbox_process("PFVF_SND_UPMSG", mbox_id2name(msg->hdr.id), 1,
+ (uint16_t)dev_pf_func(dev->pf, vf));
/* Reserve PF/VF mbox message */
vf_msg = mbox_alloc_msg(vf_mbox, vf, size);
@@ -895,9 +902,10 @@ process_msgs_up(struct dev *dev, struct mbox *mbox)
for (i = 0; i < req_hdr->num_msgs; i++) {
msg = (struct mbox_msghdr *)((uintptr_t)mdev->mbase + offset);
- plt_base_dbg("Message 0x%x (%s) pf:%d/vf:%d", msg->id,
- mbox_id2name(msg->id), dev_get_pf(msg->pcifunc),
- dev_get_vf(msg->pcifunc));
+ plt_base_dbg("Message 0x%x (%s) pf:%d/vf:%d", msg->id, mbox_id2name(msg->id),
+ dev_get_pf(msg->pcifunc), dev_get_vf(msg->pcifunc));
+ roc_trace_mbox_process("PRC_UPMSG", mbox_id2name(msg->id), req_hdr->num_msgs,
+ msg->pcifunc);
if (roc_rvu_lf_msg_id_range_check(dev->roc_rvu_lf, msg->id)) {
size = mbox->rx_start + msg->next_msgoff - offset;
err = process_rvu_lf_msgs_up(dev, msg, size);
@@ -905,9 +913,11 @@ process_msgs_up(struct dev *dev, struct mbox *mbox)
plt_err("Error %d handling 0x%x RVU_LF up msg", err, msg->id);
} else {
err = mbox_process_msgs_up(dev, msg);
- if (err)
+ if (err) {
+ roc_trace_mbox_error("PRC_UPMSG_ERR", mbox_id2name(msg->id), err);
plt_err("Error %d handling 0x%x (%s)", err, msg->id,
mbox_id2name(msg->id));
+ }
}
offset = mbox->rx_start + msg->next_msgoff;
}
@@ -930,7 +940,7 @@ roc_pf_vf_mbox_irq_cn20k(void *param)
plt_base_dbg("Proceeding to check mbox UP messages if any");
plt_write64(intr, dev->mbox_reg_base + RVU_VF_INT);
- plt_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev->vf);
+ roc_trace_mbox_interrupt("PFVF_IRQ_20K", dev_pf_func(dev->pf, dev->vf), intr, 0);
/* If interrupt occurred for down message */
if (intr & BIT_ULL(1))
@@ -954,7 +964,7 @@ roc_af_pf_mbox_irq_cn20k(void *param)
plt_base_dbg("Proceeding to check mbox UP messages if any");
plt_write64(intr, dev->mbox_reg_base + RVU_PF_INT);
- plt_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev->vf);
+ roc_trace_mbox_interrupt("AFPF_IRQ_20K", dev_pf_func(dev->pf, dev->vf), intr, 0);
/* If interrupt occurred for down message */
if (intr & BIT_ULL(1))
@@ -978,12 +988,12 @@ roc_pf_vf_mbox_irq(void *param)
plt_base_dbg("Proceeding to check mbox UP messages if any");
plt_write64(intr, dev->mbox_reg_base + RVU_VF_INT);
- plt_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev->vf);
/* Reading for UP/DOWN message, next message sending will be delayed
* by 1ms until this region is zeroed mbox_wait_for_zero()
*/
mbox_data = plt_read64(dev->mbox_reg_base + RVU_VF_VFPF_MBOX0);
+ roc_trace_mbox_interrupt("PFVF_IRQ", dev_pf_func(dev->pf, dev->vf), intr, mbox_data);
/* If interrupt occurred for down message */
if (mbox_data & MBOX_DOWN_MSG) {
mbox_data &= ~MBOX_DOWN_MSG;
@@ -1015,12 +1025,12 @@ roc_af_pf_mbox_irq(void *param)
plt_base_dbg("Proceeding to check mbox UP messages if any");
plt_write64(intr, dev->mbox_reg_base + RVU_PF_INT);
- plt_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev->vf);
/* Reading for UP/DOWN message, next message sending will be delayed
* by 1ms until this region is zeroed mbox_wait_for_zero()
*/
mbox_data = plt_read64(dev->mbox_reg_base + RVU_PF_PFAF_MBOX0);
+ roc_trace_mbox_interrupt("AFPF_IRQ", dev_pf_func(dev->pf, dev->vf), intr, mbox_data);
/* If interrupt occurred for down message */
if (mbox_data & MBOX_DOWN_MSG) {
mbox_data &= ~MBOX_DOWN_MSG;
@@ -1327,8 +1337,7 @@ vf_flr_handle_msg(void *param, dev_intr_t *flr)
for (vf = 0; vf < max_vf; vf++) {
if (flr->bits[vf / max_bits] & BIT_ULL(vf % max_bits)) {
- plt_base_dbg("Process FLR vf:%d request (pf:%d, vf:%d)",
- vf, dev->pf, dev->vf);
+ roc_trace_mbox_vf_flr("PRC_VFFLR", vf, dev_pf_func(dev->pf, dev->vf));
/* Inform AF about VF reset */
vf_flr_send_msg(dev, vf);
flr->bits[vf / max_bits] &= ~(BIT_ULL(vf % max_bits));
diff --git a/drivers/common/cnxk/roc_mbox.c b/drivers/common/cnxk/roc_mbox.c
index a3b96e6e2b..c07202b964 100644
--- a/drivers/common/cnxk/roc_mbox.c
+++ b/drivers/common/cnxk/roc_mbox.c
@@ -263,6 +263,34 @@ mbox_wait_for_zero(struct mbox *mbox, int devid)
return data == 0;
}
+static inline void
+mbox_debug_region(const char *ops, struct mbox *mbox, int devid, uint64_t intr_val, int rc)
+{
+ struct mbox_dev *mdev = &mbox->dev[devid];
+ struct mbox_msghdr *msg_rx, *msg_tx;
+ uint64_t offset_rx, offset_tx;
+ char buf[BUFSIZ];
+
+ if (!plt_trace_point_is_enabled(&__cnxk_trace_mbox_region))
+ return;
+
+ /* Get messages from mbox */
+ offset_tx = mbox->tx_start + PLT_ALIGN(sizeof(struct mbox_hdr), MBOX_MSG_ALIGN);
+
+ msg_tx = (struct mbox_msghdr *)((uintptr_t)mdev->mbase + offset_tx);
+ snprintf(buf, BUFSIZ, "%s_TX", ops);
+ /* Only while sending message choose a rand identifier for the msg */
+ if (intr_val)
+ msg_tx->cookie = rand();
+ roc_trace_mbox_region(buf, mbox_id2name(msg_tx->id), msg_tx->pcifunc, (int)intr_val,
+ msg_tx->cookie);
+
+ offset_rx = mbox->rx_start + PLT_ALIGN(sizeof(struct mbox_hdr), MBOX_MSG_ALIGN);
+ msg_rx = (struct mbox_msghdr *)((uintptr_t)mdev->mbase + offset_rx);
+ snprintf(buf, BUFSIZ, "%s_RX", ops);
+ roc_trace_mbox_region(buf, mbox_id2name(msg_rx->id), msg_rx->pcifunc, rc, msg_rx->cookie);
+}
+
static void
mbox_msg_send_data(struct mbox *mbox, int devid, uint8_t data)
{
@@ -291,6 +319,8 @@ mbox_msg_send_data(struct mbox *mbox, int devid, uint8_t data)
(volatile void *)(mbox->reg_base + (mbox->trigger | (devid << mbox->tr_shift))));
intr_val |= (uint64_t)data;
+
+ mbox_debug_region("SND_MSG", mbox, devid, intr_val, 0);
/* The interrupt should be fired after num_msgs is written
* to the shared memory
*/
@@ -342,6 +372,8 @@ mbox_get_rsp(struct mbox *mbox, int devid, void **msg)
if (msg != NULL)
*msg = msghdr;
+ mbox_debug_region("GET_RSP", mbox, devid, 0, msghdr->rc);
+
return msghdr->rc;
}
@@ -438,6 +470,8 @@ mbox_wait(struct mbox *mbox, int devid, uint32_t rst_timo)
plt_memory_order_acquire),
tx_hdr->num_msgs, rx_hdr->num_msgs, mdev->msg_size, mdev->rsp_size);
+ mbox_debug_region("MBX_TMO", mbox, devid, 0, -EIO);
+
return -EIO;
}
}
diff --git a/drivers/common/cnxk/roc_mbox.h b/drivers/common/cnxk/roc_mbox.h
index e7a916d61c..3ca8442b55 100644
--- a/drivers/common/cnxk/roc_mbox.h
+++ b/drivers/common/cnxk/roc_mbox.h
@@ -32,6 +32,8 @@ struct mbox_msghdr {
uint16_t __io ver;
/* Offset of next msg within mailbox region */
uint16_t __io next_msgoff;
+ /* Unique ID for trace debug */
+ uint16_t __io cookie;
int __io rc; /* Msg processed response code */
};
diff --git a/drivers/common/cnxk/roc_platform.c b/drivers/common/cnxk/roc_platform.c
index f1e0a93d97..f7678280f5 100644
--- a/drivers/common/cnxk/roc_platform.c
+++ b/drivers/common/cnxk/roc_platform.c
@@ -3,6 +3,7 @@
*/
#include <rte_log.h>
+#include <rte_trace_point_register.h>
#include "roc_api.h"
@@ -85,6 +86,14 @@ roc_plt_init(void)
return 0;
}
+/* ROC trace points */
+RTE_TRACE_POINT_REGISTER(cnxk_trace_mbox_region, cnxk.mbox.region)
+RTE_TRACE_POINT_REGISTER(cnxk_trace_mbox_process, cnxk.mbox.process)
+RTE_TRACE_POINT_REGISTER(cnxk_trace_mbox_interrupt, cnxk.mbox.interrupt)
+RTE_TRACE_POINT_REGISTER(cnxk_trace_mbox_vf_flr, cnxk.mbox.vf.flr)
+RTE_TRACE_POINT_REGISTER(cnxk_trace_mbox_vf_pf_handle, cnxk.mbox.vf.pf.handle)
+RTE_TRACE_POINT_REGISTER(cnxk_trace_mbox_error, cnxk.mbox.error)
+
RTE_LOG_REGISTER_SUFFIX(cnxk_logtype_base, base, INFO);
RTE_LOG_REGISTER_SUFFIX(cnxk_logtype_mbox, mbox, NOTICE);
RTE_LOG_REGISTER_SUFFIX(cnxk_logtype_cpt, crypto, NOTICE);
diff --git a/drivers/common/cnxk/roc_platform.h b/drivers/common/cnxk/roc_platform.h
index 4076dbe94d..d62d1db8d1 100644
--- a/drivers/common/cnxk/roc_platform.h
+++ b/drivers/common/cnxk/roc_platform.h
@@ -5,12 +5,12 @@
#ifndef _ROC_PLATFORM_H_
#define _ROC_PLATFORM_H_
-#include <rte_compat.h>
+#include <bus_pci_driver.h>
#include <rte_alarm.h>
#include <rte_bitmap.h>
-#include <bus_pci_driver.h>
#include <rte_byteorder.h>
#include <rte_common.h>
+#include <rte_compat.h>
#include <rte_cycles.h>
#include <rte_ether.h>
#include <rte_interrupts.h>
@@ -25,6 +25,7 @@
#include <rte_string_fns.h>
#include <rte_tailq.h>
#include <rte_telemetry.h>
+#include <rte_trace_point.h>
#include "eal_filesystem.h"
@@ -265,6 +266,9 @@ plt_thread_is_valid(plt_thread_t thr)
#define plt_tel_data_add_dict_u64 rte_tel_data_add_dict_uint
#define plt_telemetry_register_cmd rte_telemetry_register_cmd
+/* Trace */
+#define plt_trace_point_is_enabled rte_trace_point_is_enabled
+
/* Log */
extern int cnxk_logtype_base;
#define RTE_LOGTYPE_base cnxk_logtype_base
@@ -379,9 +383,82 @@ plt_lmt_region_reserve_aligned(const char *name, size_t len, uint32_t align)
/* To ensure returned memory is physically contiguous, bounding
* the start and end address in 2M range.
*/
- return rte_memzone_reserve_bounded(name, len, SOCKET_ID_ANY,
- RTE_MEMZONE_IOVA_CONTIG,
- align, RTE_PGSIZE_2M);
+ return rte_memzone_reserve_bounded(name, len, SOCKET_ID_ANY, RTE_MEMZONE_IOVA_CONTIG, align,
+ RTE_PGSIZE_2M);
+}
+
+/* ROC trace points */
+RTE_TRACE_POINT(cnxk_trace_mbox_region,
+ RTE_TRACE_POINT_ARGS(const char *func, const char *msg, uint16_t pcifunc, int data,
+ uint16_t cookie),
+ rte_trace_point_emit_string(func);
+ rte_trace_point_emit_string(msg); rte_trace_point_emit_u16(pcifunc);
+ rte_trace_point_emit_int(data); rte_trace_point_emit_u16(cookie);)
+
+RTE_TRACE_POINT(cnxk_trace_mbox_process,
+ RTE_TRACE_POINT_ARGS(const char *func, const char *msg, uint16_t num_msgs,
+ uint16_t pcifunc),
+ rte_trace_point_emit_string(func);
+ rte_trace_point_emit_string(msg); rte_trace_point_emit_u16(num_msgs);
+ rte_trace_point_emit_u16(pcifunc);)
+
+RTE_TRACE_POINT(cnxk_trace_mbox_interrupt,
+ RTE_TRACE_POINT_ARGS(const char *func, int pcifunc, uint64_t intr,
+ uint64_t mbox_data),
+ rte_trace_point_emit_string(func);
+ rte_trace_point_emit_int(pcifunc); rte_trace_point_emit_u64(intr);
+ rte_trace_point_emit_u64(mbox_data);)
+
+RTE_TRACE_POINT(cnxk_trace_mbox_vf_flr,
+ RTE_TRACE_POINT_ARGS(const char *func, uint16_t from_vf, uint16_t pcifunc),
+ rte_trace_point_emit_string(func);
+ rte_trace_point_emit_u16(from_vf); rte_trace_point_emit_u16(pcifunc);)
+
+RTE_TRACE_POINT(cnxk_trace_mbox_vf_pf_handle,
+ RTE_TRACE_POINT_ARGS(const char *ops, uint16_t pcifunc, int data),
+ rte_trace_point_emit_string(ops);
+ rte_trace_point_emit_u16(pcifunc); rte_trace_point_emit_int(data);)
+
+RTE_TRACE_POINT(cnxk_trace_mbox_error,
+ RTE_TRACE_POINT_ARGS(const char *func, const char *msg, int data),
+ rte_trace_point_emit_string(func);
+ rte_trace_point_emit_string(msg); rte_trace_point_emit_int(data);)
+
+static inline void
+roc_trace_mbox_region(const char *func, const char *msg, uint16_t pcifunc, int data,
+ uint16_t cookie)
+{
+ cnxk_trace_mbox_region(func, msg, pcifunc, data, cookie);
+}
+
+static inline void
+roc_trace_mbox_process(const char *func, const char *msg, uint16_t num_msgs, uint16_t pcifunc)
+{
+ cnxk_trace_mbox_process(func, msg, num_msgs, pcifunc);
+}
+
+static inline void
+roc_trace_mbox_interrupt(const char *func, int pcifunc, uint64_t intr, uint64_t mbox_data)
+{
+ cnxk_trace_mbox_interrupt(func, pcifunc, intr, mbox_data);
+}
+
+static inline void
+roc_trace_mbox_vf_pf_handle(const char *func, uint16_t pcifunc, int data)
+{
+ cnxk_trace_mbox_vf_pf_handle(func, pcifunc, data);
+}
+
+static inline void
+roc_trace_mbox_error(const char *func, const char *msg, int data)
+{
+ cnxk_trace_mbox_error(func, msg, data);
+}
+
+static inline void
+roc_trace_mbox_vf_flr(const char *func, uint16_t from_vf, uint16_t pcifunc)
+{
+ cnxk_trace_mbox_vf_flr(func, from_vf, pcifunc);
}
#endif /* _ROC_PLATFORM_H_ */
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] common/cnxk: add mailbox debug trace
2024-11-04 13:08 ` [PATCH 2/2] common/cnxk: add mailbox debug trace Harman Kalra
2024-11-07 8:04 ` [EXTERNAL] " Jerin Jacob
@ 2024-11-14 16:38 ` Stephen Hemminger
1 sibling, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2024-11-14 16:38 UTC (permalink / raw)
To: Harman Kalra
Cc: Nithin Dabilpuram, Kiran Kumar K, Sunil Kumar Kori, Satha Rao, dev
On Mon, 4 Nov 2024 18:38:00 +0530
Harman Kalra <hkalra@marvell.com> wrote:
> Implementing mailbox debug trace points to trace function calls.
>
> Signed-off-by: Harman Kalra <hkalra@marvell.com>
Missing use of RTE_ATOMIC() to flag atomic variables.
-----------------------Summary of failed steps-----------------------
"ubuntu-22.04-clang-stdatomic" failed at step Build and test
----------------------End summary of failed steps--------------------
-------------------------------BEGIN LOGS----------------------------
####################################################################################
#### [Begin job log] "ubuntu-22.04-clang-stdatomic" at step Build and test
####################################################################################
atomic_store_explicit(ptr, val, memorder)
^ ~~~
/usr/lib/llvm-14/lib/clang/14.0.0/include/stdatomic.h:127:31: note: expanded from macro 'atomic_store_explicit'
#define atomic_store_explicit __c11_atomic_store
^
../drivers/common/cnxk/roc_dev.c:551:2: error: address argument to atomic operation must be a pointer to _Atomic type ('uint16_t *' (aka 'unsigned short *') invalid)
plt_atomic_store_explicit(&mdev->msgs_acked, msgs_acked, plt_memory_order_release);
^ ~~~~~~~~~~~~~~~~~
../drivers/common/cnxk/roc_platform.h:215:35: note: expanded from macro 'plt_atomic_store_explicit'
#define plt_atomic_store_explicit rte_atomic_store_explicit
^
../lib/eal/include/rte_stdatomic.h:72:2: note: expanded from macro 'rte_atomic_store_explicit'
atomic_store_explicit(ptr, val, memorder)
^ ~~~
/usr/lib/llvm-14/lib/clang/14.0.0/include/stdatomic.h:127:31: note: expanded from macro 'atomic_store_explicit'
#define atomic_store_explicit __c11_atomic_store
^
2 errors generated.
[749/3145] Compiling C object drivers/libtmp_rte_common_cnxk.a.p/common_cnxk_roc_cpt.c.o
[750/3145] Compiling C object drivers/libtmp_rte_common_cnxk.a.p/common_cnxk_roc_cpt_debug.c.o
[751/3145] Compiling C object drivers/libtmp_rte_common_cnxk.a.p/common_cnxk_roc_dpi.c.o
[752/3145] Compiling C object drivers/libtmp_rte_common_cnxk.a.p/common_cnxk_roc_eswitch.c.o
[753/3145] Compiling C object lib/librte_pipeline.a.p/pipeline_rte_table_action.c.o
ninja: build stopped: subcommand failed.
##[error]Process completed with exit code 1.
####################################################################################
#### [End job log] "ubuntu-22.04-clang-stdatomic" at step Build and test
##############################################################################
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-11-14 16:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-04 13:07 [PATCH 1/2] common/cnxk: fix mailbox timeout issues Harman Kalra
2024-11-04 13:08 ` [PATCH 2/2] common/cnxk: add mailbox debug trace Harman Kalra
2024-11-07 8:04 ` [EXTERNAL] " Jerin Jacob
2024-11-14 16:38 ` Stephen Hemminger
2024-11-14 9:31 ` [PATCH v2 1/2] common/cnxk: fix mailbox timeout issues Harman Kalra
2024-11-14 9:31 ` [PATCH v2 2/2] common/cnxk: add mailbox debug trace Harman Kalra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).