Re: [dpdk-dev] [PATCH v7 2/2] common/octeontx2: add polling based response mbox message

[Sunil Kumar Kori <skori@marvell.com>]

Hello Thomas,

I have uploaded next version after handling Jerin's comments.
Can you please look into the series so that It can be applied if there is no more comments ? 
Thanks.

Regards
Sunil Kumar Kori

>-----Original Message-----
>From: Sunil Kumar Kori <skori@marvell.com>
>Sent: Tuesday, January 14, 2020 2:35 PM
>To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Nithin Kumar
>Dabilpuram <ndabilpuram@marvell.com>; Vamsi Krishna Attunuru
><vattunuru@marvell.com>
>Cc: dev@dpdk.org; Sunil Kumar Kori <skori@marvell.com>; Harman Kalra
><hkalra@marvell.com>
>Subject: [PATCH v7 2/2] common/octeontx2: add polling based response
>mbox message
>
>Currently otx2_mbox_get_rsp_xxx get response once AF driver interrupts
>after completion. But this function will get into deadlock if called in another
>interrupt context.
>
>To avoid it, implemented another version of this function which polls on
>dedicated memory for a given timeout.
>
>Also after clearing interrupt, there could UP messages available for processing.
>So irq handler must check mbox messages.
>
>Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
>Signed-off-by: Harman Kalra <hkalra@marvell.com>
>Acked-by: Jerin Jacob <jerinj@marvell.com>
>---
>v7:
> - Corrected spelling in commit message
>v6:
> - Removed unnecessary code.
>v5:
> - Fix shared library compilation error
>v4:
> - used rte_io_rmb instead of rte_rmb in mbox_poll.
>v3:
> - Remove experimental tag as API is defined static.
> - Merge all changes to single patch.
>v2:
> - Included Makefile and meson build changes.
> - Rebased patch on 19.11-rc4
>
> drivers/common/octeontx2/otx2_dev.c  | 41 +++++++++++-----------
>drivers/common/octeontx2/otx2_mbox.c | 51 ++++++++++++++++++++++++---
>-  drivers/common/octeontx2/otx2_mbox.h |  5 +--
> 3 files changed, 70 insertions(+), 27 deletions(-)
>
>diff --git a/drivers/common/octeontx2/otx2_dev.c
>b/drivers/common/octeontx2/otx2_dev.c
>index 0fc799e4a..d61c712fa 100644
>--- a/drivers/common/octeontx2/otx2_dev.c
>+++ b/drivers/common/octeontx2/otx2_dev.c
>@@ -577,17 +577,16 @@ otx2_pf_vf_mbox_irq(void *param)
>
> 	intr = otx2_read64(dev->bar2 + RVU_VF_INT);
> 	if (intr == 0)
>-		return;
>+		otx2_base_dbg("Proceeding to check mbox UP messages if
>any");
>
> 	otx2_write64(intr, dev->bar2 + RVU_VF_INT);
> 	otx2_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev-
>>vf);
>-	if (intr) {
>-		/* First process all configuration messages */
>-		otx2_process_msgs(dev, dev->mbox);
>
>-		/* Process Uplink messages */
>-		otx2_process_msgs_up(dev, &dev->mbox_up);
>-	}
>+	/* First process all configuration messages */
>+	otx2_process_msgs(dev, dev->mbox);
>+
>+	/* Process Uplink messages */
>+	otx2_process_msgs_up(dev, &dev->mbox_up);
> }
>
> static void
>@@ -598,18 +597,16 @@ otx2_af_pf_mbox_irq(void *param)
>
> 	intr = otx2_read64(dev->bar2 + RVU_PF_INT);
> 	if (intr == 0)
>-		return;
>+		otx2_base_dbg("Proceeding to check mbox UP messages if
>any");
>
> 	otx2_write64(intr, dev->bar2 + RVU_PF_INT);
>-
> 	otx2_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev-
>>vf);
>-	if (intr) {
>-		/* First process all configuration messages */
>-		otx2_process_msgs(dev, dev->mbox);
>
>-		/* Process Uplink messages */
>-		otx2_process_msgs_up(dev, &dev->mbox_up);
>-	}
>+	/* First process all configuration messages */
>+	otx2_process_msgs(dev, dev->mbox);
>+
>+	/* Process Uplink messages */
>+	otx2_process_msgs_up(dev, &dev->mbox_up);
> }
>
> static int
>@@ -900,6 +897,7 @@ otx2_dev_priv_init(struct rte_pci_device *pci_dev,
>void *otx2_dev)  {
> 	int up_direction = MBOX_DIR_PFAF_UP;
> 	int rc, direction = MBOX_DIR_PFAF;
>+	uint64_t intr_offset = RVU_PF_INT;
> 	struct otx2_dev *dev = otx2_dev;
> 	uintptr_t bar2, bar4;
> 	uint64_t bar4_addr;
>@@ -924,15 +922,18 @@ otx2_dev_priv_init(struct rte_pci_device *pci_dev,
>void *otx2_dev)
> 	if (otx2_dev_is_vf(dev)) {
> 		direction = MBOX_DIR_VFPF;
> 		up_direction = MBOX_DIR_VFPF_UP;
>+		intr_offset = RVU_VF_INT;
> 	}
>
> 	/* Initialize the local mbox */
>-	rc = otx2_mbox_init(&dev->mbox_local, bar4, bar2, direction, 1);
>+	rc = otx2_mbox_init(&dev->mbox_local, bar4, bar2, direction, 1,
>+			    intr_offset);
> 	if (rc)
> 		goto error;
> 	dev->mbox = &dev->mbox_local;
>
>-	rc = otx2_mbox_init(&dev->mbox_up, bar4, bar2, up_direction, 1);
>+	rc = otx2_mbox_init(&dev->mbox_up, bar4, bar2, up_direction, 1,
>+			    intr_offset);
> 	if (rc)
> 		goto error;
>
>@@ -967,13 +968,15 @@ otx2_dev_priv_init(struct rte_pci_device *pci_dev,
>void *otx2_dev)
> 		}
> 		/* Init mbox object */
> 		rc = otx2_mbox_init(&dev->mbox_vfpf, (uintptr_t)hwbase,
>-				    bar2, MBOX_DIR_PFVF, pci_dev->max_vfs);
>+				    bar2, MBOX_DIR_PFVF, pci_dev->max_vfs,
>+				    intr_offset);
> 		if (rc)
> 			goto iounmap;
>
> 		/* PF -> VF UP messages */
> 		rc = otx2_mbox_init(&dev->mbox_vfpf_up, (uintptr_t)hwbase,
>-				    bar2, MBOX_DIR_PFVF_UP, pci_dev-
>>max_vfs);
>+				    bar2, MBOX_DIR_PFVF_UP, pci_dev-
>>max_vfs,
>+				    intr_offset);
> 		if (rc)
> 			goto mbox_fini;
> 	}
>diff --git a/drivers/common/octeontx2/otx2_mbox.c
>b/drivers/common/octeontx2/otx2_mbox.c
>index c359bf42f..1ec0d6f69 100644
>--- a/drivers/common/octeontx2/otx2_mbox.c
>+++ b/drivers/common/octeontx2/otx2_mbox.c
>@@ -11,6 +11,7 @@
> #include <rte_cycles.h>
>
> #include "otx2_mbox.h"
>+#include "otx2_dev.h"
>
> #define RVU_AF_AFPF_MBOX0	(0x02000)
> #define RVU_AF_AFPF_MBOX1	(0x02008)
>@@ -59,12 +60,13 @@ otx2_mbox_reset(struct otx2_mbox *mbox, int devid)
>}
>
> int
>-otx2_mbox_init(struct otx2_mbox *mbox, uintptr_t hwbase,
>-	       uintptr_t reg_base, int direction, int ndevs)
>+otx2_mbox_init(struct otx2_mbox *mbox, uintptr_t hwbase, uintptr_t
>reg_base,
>+	       int direction, int ndevs, uint64_t intr_offset)
> {
> 	struct otx2_mbox_dev *mdev;
> 	int devid;
>
>+	mbox->intr_offset = intr_offset;
> 	mbox->reg_base = reg_base;
> 	mbox->hwbase = hwbase;
>
>@@ -244,6 +246,39 @@ otx2_mbox_get_rsp(struct otx2_mbox *mbox, int
>devid, void **msg)
> 	return msghdr->rc;
> }
>
>+/**
>+ * Polling for given wait time to get mailbox response  */ static int
>+mbox_poll(struct otx2_mbox *mbox, uint32_t wait) {
>+	uint32_t timeout = 0, sleep = 1;
>+	uint32_t wait_us = wait * 1000;
>+	uint64_t rsp_reg = 0;
>+	uintptr_t reg_addr;
>+
>+	reg_addr = mbox->reg_base + mbox->intr_offset;
>+	do {
>+		rsp_reg = otx2_read64(reg_addr);
>+
>+		if (timeout >= wait_us)
>+			return -ETIMEDOUT;
>+
>+		rte_delay_us(sleep);
>+		timeout += sleep;
>+	} while (!rsp_reg);
>+
>+	rte_smp_rmb();
>+
>+	/* Clear interrupt */
>+	otx2_write64(rsp_reg, reg_addr);
>+
>+	/* Reset mbox */
>+	otx2_mbox_reset(mbox, 0);
>+
>+	return 0;
>+}
>+
> /**
>  * @internal
>  * Wait and get mailbox response with timeout @@ -321,11 +356,15 @@
>otx2_mbox_wait_for_rsp_tmo(struct otx2_mbox *mbox, int devid, uint32_t
>tmo)
> 	}
>
> 	/* Wait message */
>-	rc = mbox_wait(mbox, devid, tmo);
>-	if (rc)
>-		return rc;
>+	if (rte_thread_is_intr())
>+		rc = mbox_poll(mbox, tmo);
>+	else
>+		rc = mbox_wait(mbox, devid, tmo);
>
>-	return mdev->msgs_acked;
>+	if (!rc)
>+		rc = mdev->num_msgs;
>+
>+	return rc;
> }
>
> /**
>diff --git a/drivers/common/octeontx2/otx2_mbox.h
>b/drivers/common/octeontx2/otx2_mbox.h
>index e0e4e2f63..0535cec36 100644
>--- a/drivers/common/octeontx2/otx2_mbox.h
>+++ b/drivers/common/octeontx2/otx2_mbox.h
>@@ -73,6 +73,7 @@ struct otx2_mbox {
> 	uint16_t tx_size;  /* Size of Tx region */
> 	uint16_t ndevs;    /* The number of peers */
> 	struct otx2_mbox_dev *dev;
>+	uint64_t intr_offset; /* Offset to interrupt register */
> };
>
> /* Header which precedes all mbox messages */ @@ -1562,8 +1563,8 @@
>struct tim_enable_rsp {  const char *otx2_mbox_id2name(uint16_t id);  int
>otx2_mbox_id2size(uint16_t id);  void otx2_mbox_reset(struct otx2_mbox
>*mbox, int devid); -int otx2_mbox_init(struct otx2_mbox *mbox, uintptr_t
>hwbase,
>-		   uintptr_t reg_base, int direction, int ndevs);
>+int otx2_mbox_init(struct otx2_mbox *mbox, uintptr_t hwbase, uintptr_t
>reg_base,
>+		   int direction, int ndevsi, uint64_t intr_offset);
> void otx2_mbox_fini(struct otx2_mbox *mbox);  void
>otx2_mbox_msg_send(struct otx2_mbox *mbox, int devid);  int
>otx2_mbox_wait_for_rsp(struct otx2_mbox *mbox, int devid);
>--
>2.17.1