DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/5] drivers/octeontx2: allow experimental symbols
@ 2019-11-25 11:35 Sunil Kumar Kori
  2019-11-25 11:35 ` [dpdk-dev] [PATCH v3 2/5] eal: add API to check if its interrupt context Sunil Kumar Kori
                   ` (4 more replies)
  0 siblings, 5 replies; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-11-25 11:35 UTC (permalink / raw)
  To: Jerin Jacob, Nithin Dabilpuram, Vamsi Attunuru, Kiran Kumar K, Satha Rao
  Cc: dev, Sunil Kumar Kori

Multiple experimental symbols are used. They must be allowed
to avoid compilation error.

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
 drivers/common/octeontx2/Makefile  | 2 ++
 drivers/net/octeontx2/Makefile     | 2 ++
 drivers/raw/octeontx2_dma/Makefile | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/drivers/common/octeontx2/Makefile b/drivers/common/octeontx2/Makefile
index eaff29433..b1c1792fb 100644
--- a/drivers/common/octeontx2/Makefile
+++ b/drivers/common/octeontx2/Makefile
@@ -22,6 +22,8 @@ CFLAGS += -diag-disable 2259
 endif
 endif
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
 EXPORT_MAP := rte_common_octeontx2_version.map
 
 #
diff --git a/drivers/net/octeontx2/Makefile b/drivers/net/octeontx2/Makefile
index 68f5765db..3da4d8cc1 100644
--- a/drivers/net/octeontx2/Makefile
+++ b/drivers/net/octeontx2/Makefile
@@ -26,6 +26,8 @@ CFLAGS += -diag-disable 2259
 endif
 endif
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
 EXPORT_MAP := rte_pmd_octeontx2_version.map
 
 #
diff --git a/drivers/raw/octeontx2_dma/Makefile b/drivers/raw/octeontx2_dma/Makefile
index c64ca3497..0d0c530fe 100644
--- a/drivers/raw/octeontx2_dma/Makefile
+++ b/drivers/raw/octeontx2_dma/Makefile
@@ -22,6 +22,8 @@ CFLAGS += -diag-disable 2259
 endif
 endif
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
 EXPORT_MAP := rte_rawdev_octeontx2_dma_version.map
 
 #
-- 
2.17.1


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

* [dpdk-dev] [PATCH v3 2/5] eal: add API to check if its interrupt context
  2019-11-25 11:35 [dpdk-dev] [PATCH 1/5] drivers/octeontx2: allow experimental symbols Sunil Kumar Kori
@ 2019-11-25 11:35 ` Sunil Kumar Kori
  2019-11-25 11:35 ` [dpdk-dev] [PATCH 3/5] common/octeontx2: add interrupt offset to mbox structure Sunil Kumar Kori
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-11-25 11:35 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, Sunil Kumar Kori, Harman Kalra

Added an API to check if current execution is in interrupt
context. This will be helpful to handle nested interrupt cases.

Signed-off-by: Harman Kalra <hkalra@marvell.com>
Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
V3:
 * Rebased patch on latest commit
V2:
 * Removed unnecessary if statement
 * Reworded subject line
 * Updated return values of API

 lib/librte_eal/common/include/rte_interrupts.h | 15 +++++++++++++++
 lib/librte_eal/freebsd/eal/eal_interrupts.c    |  5 +++++
 lib/librte_eal/linux/eal/eal_interrupts.c      |  5 +++++
 lib/librte_eal/rte_eal_version.map             |  1 +
 4 files changed, 26 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_interrupts.h b/lib/librte_eal/common/include/rte_interrupts.h
index e3b406abc..0a82e28a8 100644
--- a/lib/librte_eal/common/include/rte_interrupts.h
+++ b/lib/librte_eal/common/include/rte_interrupts.h
@@ -138,6 +138,21 @@ int rte_intr_disable(const struct rte_intr_handle *intr_handle);
 __rte_experimental
 int rte_intr_ack(const struct rte_intr_handle *intr_handle);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Check if currently executing in interrupt context
+ *
+ * @return
+ *  - positive in case of interrupt context
+ *  - zero in case of process context
+ *  - negative if unsuccessful
+ */
+__rte_experimental
+int
+rte_thread_is_intr(void);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_eal/freebsd/eal/eal_interrupts.c b/lib/librte_eal/freebsd/eal/eal_interrupts.c
index f6831b790..ce2a27b4a 100644
--- a/lib/librte_eal/freebsd/eal/eal_interrupts.c
+++ b/lib/librte_eal/freebsd/eal/eal_interrupts.c
@@ -671,3 +671,8 @@ rte_intr_free_epoll_fd(struct rte_intr_handle *intr_handle)
 {
 	RTE_SET_USED(intr_handle);
 }
+
+int rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c b/lib/librte_eal/linux/eal/eal_interrupts.c
index 1955324d3..c00f5a575 100644
--- a/lib/librte_eal/linux/eal/eal_interrupts.c
+++ b/lib/librte_eal/linux/eal/eal_interrupts.c
@@ -1487,3 +1487,8 @@ rte_intr_cap_multiple(struct rte_intr_handle *intr_handle)
 
 	return 0;
 }
+
+int rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index e38d02530..397e787cf 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -332,4 +332,5 @@ EXPERIMENTAL {
 	# added in 19.11
 	rte_log_get_stream;
 	rte_mcfg_get_single_file_segments;
+	rte_thread_is_intr;
 };
-- 
2.17.1


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

* [dpdk-dev] [PATCH 3/5] common/octeontx2: add interrupt offset to mbox structure
  2019-11-25 11:35 [dpdk-dev] [PATCH 1/5] drivers/octeontx2: allow experimental symbols Sunil Kumar Kori
  2019-11-25 11:35 ` [dpdk-dev] [PATCH v3 2/5] eal: add API to check if its interrupt context Sunil Kumar Kori
@ 2019-11-25 11:35 ` Sunil Kumar Kori
  2019-11-25 11:35 ` [dpdk-dev] [PATCH 4/5] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-11-25 11:35 UTC (permalink / raw)
  To: Jerin Jacob, Nithin Dabilpuram, Vamsi Attunuru; +Cc: dev, Sunil Kumar Kori

mbox response are triggered at bar2 + RVU_PF_INT or bar2 + RVU_VF_INT,
depending upon the device. To process these interrupts different irq
handlers are installed. To unify it, added offset field into mbox
structure which is initialized with RVU_PF_INT or RVU_VF_INT at the time
of otx2_mbox_init.

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
 drivers/common/octeontx2/otx2_dev.c  | 14 ++++++++++----
 drivers/common/octeontx2/otx2_mbox.c |  5 +++--
 drivers/common/octeontx2/otx2_mbox.h |  5 +++--
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/common/octeontx2/otx2_dev.c b/drivers/common/octeontx2/otx2_dev.c
index 0fc799e4a..6f29d6108 100644
--- a/drivers/common/octeontx2/otx2_dev.c
+++ b/drivers/common/octeontx2/otx2_dev.c
@@ -900,6 +900,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 +925,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 +971,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..ad8e0c3aa 100644
--- a/drivers/common/octeontx2/otx2_mbox.c
+++ b/drivers/common/octeontx2/otx2_mbox.c
@@ -59,12 +59,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;
 
diff --git a/drivers/common/octeontx2/otx2_mbox.h b/drivers/common/octeontx2/otx2_mbox.h
index e0e4e2f63..162d12468 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


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

* [dpdk-dev] [PATCH 4/5] common/octeontx2: add polling based response mbox message
  2019-11-25 11:35 [dpdk-dev] [PATCH 1/5] drivers/octeontx2: allow experimental symbols Sunil Kumar Kori
  2019-11-25 11:35 ` [dpdk-dev] [PATCH v3 2/5] eal: add API to check if its interrupt context Sunil Kumar Kori
  2019-11-25 11:35 ` [dpdk-dev] [PATCH 3/5] common/octeontx2: add interrupt offset to mbox structure Sunil Kumar Kori
@ 2019-11-25 11:35 ` Sunil Kumar Kori
  2019-11-25 11:35 ` [dpdk-dev] [PATCH 5/5] common/octeontx2: enhancing mbox APIs to get response Sunil Kumar Kori
  2019-11-26  6:15 ` [dpdk-dev] [PATCH v2 1/5] drivers/octeontx2: allow experimental symbols Sunil Kumar Kori
  4 siblings, 0 replies; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-11-25 11:35 UTC (permalink / raw)
  To: Jerin Jacob, Nithin Dabilpuram, Vamsi Attunuru; +Cc: dev, Sunil Kumar Kori

Currently otx2_mbox_get_rsp get response once AF driver
interrupts after completion. But this funciton 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>
---
 drivers/common/octeontx2/otx2_mbox.c          | 58 +++++++++++++++++++
 drivers/common/octeontx2/otx2_mbox.h          |  7 +++
 .../rte_common_octeontx2_version.map          |  7 +++
 3 files changed, 72 insertions(+)

diff --git a/drivers/common/octeontx2/otx2_mbox.c b/drivers/common/octeontx2/otx2_mbox.c
index ad8e0c3aa..af34fd19d 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)
@@ -245,6 +246,63 @@ otx2_mbox_get_rsp(struct otx2_mbox *mbox, int devid, void **msg)
 	return msghdr->rc;
 }
 
+/**
+ * @internal
+ * Polling for given wait time to get mailbox response
+ */
+int
+otx2_mbox_get_rsp_poll_tmo(struct otx2_mbox *mbox, int devid, void **msg,
+			   uint32_t wait)
+{
+	struct otx2_mbox_dev *mdev = &mbox->dev[devid];
+	uint32_t timeout = 0, sleep = 1;
+	struct mbox_msghdr *msghdr;
+	uint64_t rsp_reg = 0;
+	uintptr_t reg_addr;
+	uint64_t offset;
+
+	rte_rmb();
+
+	offset = mbox->rx_start +
+		RTE_ALIGN(sizeof(struct mbox_hdr), MBOX_MSG_ALIGN);
+	msghdr = (struct mbox_msghdr *)((uintptr_t)mdev->mbase + offset);
+
+	reg_addr = mbox->reg_base + mbox->intr_offset;
+	while (!rsp_reg) {
+		rte_rmb();
+		rsp_reg = otx2_read64(reg_addr);
+
+		if (timeout >= wait)
+			return -ETIMEDOUT;
+
+		rte_delay_ms(sleep);
+		timeout += sleep;
+	}
+
+	if (msg != NULL)
+		*msg = msghdr;
+
+	/* Clear interrupt */
+	otx2_write64(rsp_reg, reg_addr);
+
+	/* Reset mbox */
+	otx2_mbox_reset(mbox, 0);
+
+	return msghdr->rc;
+}
+
+/**
+ * @internal
+ * Polling for 5 seconds to get mailbox response
+ */
+int
+otx2_mbox_get_rsp_poll(struct otx2_mbox *mbox, int devid, void **msg)
+{
+	uint32_t wait = 5 * MS_PER_S; /* 5 Seconds */
+
+	return otx2_mbox_get_rsp_poll_tmo(mbox, devid, msg, wait);
+}
+
 /**
  * @internal
  * Wait and get mailbox response with timeout
diff --git a/drivers/common/octeontx2/otx2_mbox.h b/drivers/common/octeontx2/otx2_mbox.h
index 162d12468..237d4cf45 100644
--- a/drivers/common/octeontx2/otx2_mbox.h
+++ b/drivers/common/octeontx2/otx2_mbox.h
@@ -1570,6 +1570,13 @@ void otx2_mbox_msg_send(struct otx2_mbox *mbox, int devid);
 int otx2_mbox_wait_for_rsp(struct otx2_mbox *mbox, int devid);
 int otx2_mbox_wait_for_rsp_tmo(struct otx2_mbox *mbox, int devid, uint32_t tmo);
 int otx2_mbox_get_rsp(struct otx2_mbox *mbox, int devid, void **msg);
+
+__rte_experimental
+int otx2_mbox_get_rsp_poll(struct otx2_mbox *mbox, int devid, void **msg);
+__rte_experimental
+int otx2_mbox_get_rsp_poll_tmo(struct otx2_mbox *mbox, int devid, void **msg,
+			       uint32_t tmo);
+
 int otx2_mbox_get_rsp_tmo(struct otx2_mbox *mbox, int devid, void **msg,
 			  uint32_t tmo);
 int otx2_mbox_get_availmem(struct otx2_mbox *mbox, int devid);
diff --git a/drivers/common/octeontx2/rte_common_octeontx2_version.map b/drivers/common/octeontx2/rte_common_octeontx2_version.map
index adad21a2d..dcbca2444 100644
--- a/drivers/common/octeontx2/rte_common_octeontx2_version.map
+++ b/drivers/common/octeontx2/rte_common_octeontx2_version.map
@@ -33,3 +33,10 @@ DPDK_20.0 {
 
 	local: *;
 };
+
+EXPERIMENTAL {
+	global:
+
+	otx2_mbox_get_rsp_poll;
+	otx2_mbox_get_rsp_poll_tmo;
+};
-- 
2.17.1


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

* [dpdk-dev] [PATCH 5/5] common/octeontx2: enhancing mbox APIs to get response
  2019-11-25 11:35 [dpdk-dev] [PATCH 1/5] drivers/octeontx2: allow experimental symbols Sunil Kumar Kori
                   ` (2 preceding siblings ...)
  2019-11-25 11:35 ` [dpdk-dev] [PATCH 4/5] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
@ 2019-11-25 11:35 ` Sunil Kumar Kori
  2019-11-26  6:15 ` [dpdk-dev] [PATCH v2 1/5] drivers/octeontx2: allow experimental symbols Sunil Kumar Kori
  4 siblings, 0 replies; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-11-25 11:35 UTC (permalink / raw)
  To: Jerin Jacob, Nithin Dabilpuram, Vamsi Attunuru; +Cc: dev, Sunil Kumar Kori

Based on thread context, mbox API will get response for submitted
request. If thread runs in interrupt context then it uses polling
based get response API otherwise interrupt based.

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
 drivers/common/octeontx2/otx2_dev.c  | 27 ++++++++++++---------------
 drivers/common/octeontx2/otx2_mbox.h | 21 +++++++++++++++++----
 2 files changed, 29 insertions(+), 19 deletions(-)

diff --git a/drivers/common/octeontx2/otx2_dev.c b/drivers/common/octeontx2/otx2_dev.c
index 6f29d6108..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
diff --git a/drivers/common/octeontx2/otx2_mbox.h b/drivers/common/octeontx2/otx2_mbox.h
index 237d4cf45..e82dfe530 100644
--- a/drivers/common/octeontx2/otx2_mbox.h
+++ b/drivers/common/octeontx2/otx2_mbox.h
@@ -9,6 +9,7 @@
 #include <stdbool.h>
 
 #include <rte_ether.h>
+#include <rte_interrupts.h>
 #include <rte_spinlock.h>
 
 #include <otx2_common.h>
@@ -1627,28 +1628,40 @@ static inline int
 otx2_mbox_process(struct otx2_mbox *mbox)
 {
 	otx2_mbox_msg_send(mbox, 0);
-	return otx2_mbox_get_rsp(mbox, 0, NULL);
+	if (rte_thread_is_intr())
+		return otx2_mbox_get_rsp_poll(mbox, 0, NULL);
+	else
+		return otx2_mbox_get_rsp(mbox, 0, NULL);
 }
 
 static inline int
 otx2_mbox_process_msg(struct otx2_mbox *mbox, void **msg)
 {
 	otx2_mbox_msg_send(mbox, 0);
-	return otx2_mbox_get_rsp(mbox, 0, msg);
+	if (rte_thread_is_intr())
+		return otx2_mbox_get_rsp_poll(mbox, 0, msg);
+	else
+		return otx2_mbox_get_rsp(mbox, 0, msg);
 }
 
 static inline int
 otx2_mbox_process_tmo(struct otx2_mbox *mbox, uint32_t tmo)
 {
 	otx2_mbox_msg_send(mbox, 0);
-	return otx2_mbox_get_rsp_tmo(mbox, 0, NULL, tmo);
+	if (rte_thread_is_intr())
+		return otx2_mbox_get_rsp_poll_tmo(mbox, 0, NULL, tmo);
+	else
+		return otx2_mbox_get_rsp_tmo(mbox, 0, NULL, tmo);
 }
 
 static inline int
 otx2_mbox_process_msg_tmo(struct otx2_mbox *mbox, void **msg, uint32_t tmo)
 {
 	otx2_mbox_msg_send(mbox, 0);
-	return otx2_mbox_get_rsp_tmo(mbox, 0, msg, tmo);
+	if (rte_thread_is_intr())
+		return otx2_mbox_get_rsp_poll_tmo(mbox, 0, msg, tmo);
+	else
+		return otx2_mbox_get_rsp_tmo(mbox, 0, msg, tmo);
 }
 
 int otx2_send_ready_msg(struct otx2_mbox *mbox, uint16_t *pf_func /* out */);
-- 
2.17.1


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

* [dpdk-dev] [PATCH v2 1/5] drivers/octeontx2: allow experimental symbols
  2019-11-25 11:35 [dpdk-dev] [PATCH 1/5] drivers/octeontx2: allow experimental symbols Sunil Kumar Kori
                   ` (3 preceding siblings ...)
  2019-11-25 11:35 ` [dpdk-dev] [PATCH 5/5] common/octeontx2: enhancing mbox APIs to get response Sunil Kumar Kori
@ 2019-11-26  6:15 ` Sunil Kumar Kori
  2019-11-26 16:21   ` Thomas Monjalon
  2019-11-27 10:22   ` [dpdk-dev] [PATCH v2 1/4] eal: add API to check if its interrupt context Sunil Kumar Kori
  4 siblings, 2 replies; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-11-26  6:15 UTC (permalink / raw)
  To: Jerin Jacob, Nithin Dabilpuram, Vamsi Attunuru, Kiran Kumar K, Satha Rao
  Cc: dev, Sunil Kumar Kori

Multiple experimental symbols are used. They must be allowed
to avoid compilation error.

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
 drivers/common/octeontx2/Makefile     | 2 ++
 drivers/common/octeontx2/meson.build  | 2 ++
 drivers/net/octeontx2/Makefile        | 2 ++
 drivers/net/octeontx2/meson.build     | 2 ++
 drivers/raw/octeontx2_dma/Makefile    | 2 ++
 drivers/raw/octeontx2_dma/meson.build | 2 ++
 6 files changed, 12 insertions(+)

diff --git a/drivers/common/octeontx2/Makefile b/drivers/common/octeontx2/Makefile
index eaff29433..b1c1792fb 100644
--- a/drivers/common/octeontx2/Makefile
+++ b/drivers/common/octeontx2/Makefile
@@ -22,6 +22,8 @@ CFLAGS += -diag-disable 2259
 endif
 endif
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
 EXPORT_MAP := rte_common_octeontx2_version.map
 
 #
diff --git a/drivers/common/octeontx2/meson.build b/drivers/common/octeontx2/meson.build
index b79145788..a03c6a39b 100644
--- a/drivers/common/octeontx2/meson.build
+++ b/drivers/common/octeontx2/meson.build
@@ -8,6 +8,8 @@ sources= files('otx2_dev.c',
 		'otx2_common.c',
 	       )
 
+allow_experimental_apis = true
+
 extra_flags = []
 # This integrated controller runs only on a arm64 machine, remove 32bit warnings
 if not dpdk_conf.get('RTE_ARCH_64')
diff --git a/drivers/net/octeontx2/Makefile b/drivers/net/octeontx2/Makefile
index 68f5765db..3da4d8cc1 100644
--- a/drivers/net/octeontx2/Makefile
+++ b/drivers/net/octeontx2/Makefile
@@ -26,6 +26,8 @@ CFLAGS += -diag-disable 2259
 endif
 endif
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
 EXPORT_MAP := rte_pmd_octeontx2_version.map
 
 #
diff --git a/drivers/net/octeontx2/meson.build b/drivers/net/octeontx2/meson.build
index fad3076a3..a976a2c19 100644
--- a/drivers/net/octeontx2/meson.build
+++ b/drivers/net/octeontx2/meson.build
@@ -24,6 +24,8 @@ sources = files('otx2_rx.c',
 		'otx2_ethdev_devargs.c'
 		)
 
+allow_experimental_apis = true
+
 deps += ['bus_pci', 'common_octeontx2', 'mempool_octeontx2']
 
 cflags += ['-flax-vector-conversions']
diff --git a/drivers/raw/octeontx2_dma/Makefile b/drivers/raw/octeontx2_dma/Makefile
index c64ca3497..0d0c530fe 100644
--- a/drivers/raw/octeontx2_dma/Makefile
+++ b/drivers/raw/octeontx2_dma/Makefile
@@ -22,6 +22,8 @@ CFLAGS += -diag-disable 2259
 endif
 endif
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
 EXPORT_MAP := rte_rawdev_octeontx2_dma_version.map
 
 #
diff --git a/drivers/raw/octeontx2_dma/meson.build b/drivers/raw/octeontx2_dma/meson.build
index 11f74680a..f8f88aa7e 100644
--- a/drivers/raw/octeontx2_dma/meson.build
+++ b/drivers/raw/octeontx2_dma/meson.build
@@ -5,6 +5,8 @@
 deps += ['bus_pci', 'common_octeontx2', 'rawdev']
 sources = files('otx2_dpi_rawdev.c', 'otx2_dpi_msg.c', 'otx2_dpi_test.c')
 
+allow_experimental_apis = true
+
 extra_flags = []
 # This integrated controller runs only on a arm64 machine, remove 32bit warnings
 if not dpdk_conf.get('RTE_ARCH_64')
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH v2 1/5] drivers/octeontx2: allow experimental symbols
  2019-11-26  6:15 ` [dpdk-dev] [PATCH v2 1/5] drivers/octeontx2: allow experimental symbols Sunil Kumar Kori
@ 2019-11-26 16:21   ` Thomas Monjalon
  2019-11-27  8:34     ` [dpdk-dev] [EXT] " Sunil Kumar Kori
  2019-11-27 10:22   ` [dpdk-dev] [PATCH v2 1/4] eal: add API to check if its interrupt context Sunil Kumar Kori
  1 sibling, 1 reply; 38+ messages in thread
From: Thomas Monjalon @ 2019-11-26 16:21 UTC (permalink / raw)
  To: dev
  Cc: Sunil Kumar Kori, Jerin Jacob, Nithin Dabilpuram, Vamsi Attunuru,
	Kiran Kumar K, Satha Rao

26/11/2019 07:15, Sunil Kumar Kori:
> Multiple experimental symbols are used. They must be allowed
> to avoid compilation error.

If there was an error,
1/ I would see it in my compilation test
2/ There would be some "Fixes:" tag to point the root cause

I guess this patch is not critical.
Where are patches 2, 3, 4, 5?

> 
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> ---
>  drivers/common/octeontx2/Makefile     | 2 ++
>  drivers/common/octeontx2/meson.build  | 2 ++
>  drivers/net/octeontx2/Makefile        | 2 ++
>  drivers/net/octeontx2/meson.build     | 2 ++
>  drivers/raw/octeontx2_dma/Makefile    | 2 ++
>  drivers/raw/octeontx2_dma/meson.build | 2 ++
>  6 files changed, 12 insertions(+)




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

* Re: [dpdk-dev] [EXT] Re: [PATCH v2 1/5] drivers/octeontx2: allow experimental symbols
  2019-11-26 16:21   ` Thomas Monjalon
@ 2019-11-27  8:34     ` Sunil Kumar Kori
  2019-11-27  9:42       ` Thomas Monjalon
  0 siblings, 1 reply; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-11-27  8:34 UTC (permalink / raw)
  To: Thomas Monjalon, dev
  Cc: Jerin Jacob Kollanukkaran, Nithin Kumar Dabilpuram,
	Vamsi Krishna Attunuru, Kiran Kumar Kokkilagadda,
	Satha Koteswara Rao Kottidi



Regards
Sunil Kumar Kori

>-----Original Message-----
>From: Thomas Monjalon <thomas@monjalon.net>
>Sent: Tuesday, November 26, 2019 9:51 PM
>To: dev@dpdk.org
>Cc: Sunil Kumar Kori <skori@marvell.com>; Jerin Jacob Kollanukkaran
><jerinj@marvell.com>; Nithin Kumar Dabilpuram
><ndabilpuram@marvell.com>; Vamsi Krishna Attunuru
><vattunuru@marvell.com>; Kiran Kumar Kokkilagadda
><kirankumark@marvell.com>; Satha Koteswara Rao Kottidi
><skoteshwar@marvell.com>
>Subject: [EXT] Re: [dpdk-dev] [PATCH v2 1/5] drivers/octeontx2: allow
>experimental symbols
>
>External Email
>
>----------------------------------------------------------------------
>26/11/2019 07:15, Sunil Kumar Kori:
>> Multiple experimental symbols are used. They must be allowed to avoid
>> compilation error.
>
>If there was an error,
>1/ I would see it in my compilation test 2/ There would be some "Fixes:" tag to
>point the root cause

Actually change is part of below series which is started using experimental symbols in mentioned driver.
Apart of these drivers, FLAGs are already present in other drivers so that there is no error found. 
Error occurs when applying below series. 

So I had two options to get it done. Either I can do Makefile changes along with the corresponding changes or
make them as a separate patch. I have chosen second option.
And because of this is new change, it does not contain any "Fixes" tag.
>
>I guess this patch is not critical.
>Where are patches 2, 3, 4, 5?
Following are the series of patches:
http://patches.dpdk.org/patch/63299/
http://patches.dpdk.org/patch/63271/
http://patches.dpdk.org/patch/63272/
http://patches.dpdk.org/patch/63273/
http://patches.dpdk.org/patch/63274/
Not all the patches are reworked for next versions that's why same is not reflected in subject line. 
>
>>
>> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
>> ---
>>  drivers/common/octeontx2/Makefile     | 2 ++
>>  drivers/common/octeontx2/meson.build  | 2 ++
>>  drivers/net/octeontx2/Makefile        | 2 ++
>>  drivers/net/octeontx2/meson.build     | 2 ++
>>  drivers/raw/octeontx2_dma/Makefile    | 2 ++
>>  drivers/raw/octeontx2_dma/meson.build | 2 ++
>>  6 files changed, 12 insertions(+)
>
>


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

* Re: [dpdk-dev] [EXT] Re: [PATCH v2 1/5] drivers/octeontx2: allow experimental symbols
  2019-11-27  8:34     ` [dpdk-dev] [EXT] " Sunil Kumar Kori
@ 2019-11-27  9:42       ` Thomas Monjalon
  0 siblings, 0 replies; 38+ messages in thread
From: Thomas Monjalon @ 2019-11-27  9:42 UTC (permalink / raw)
  To: Sunil Kumar Kori
  Cc: dev, Jerin Jacob Kollanukkaran, Nithin Kumar Dabilpuram,
	Vamsi Krishna Attunuru, Kiran Kumar Kokkilagadda,
	Satha Koteswara Rao Kottidi

27/11/2019 09:34, Sunil Kumar Kori:
> From: Thomas Monjalon <thomas@monjalon.net>
> >26/11/2019 07:15, Sunil Kumar Kori:
> >> Multiple experimental symbols are used. They must be allowed to avoid
> >> compilation error.
> >
> >If there was an error,
> >1/ I would see it in my compilation test 2/ There would be some "Fixes:" tag to
> >point the root cause
> 
> Actually change is part of below series which is started using experimental symbols in mentioned driver.
> Apart of these drivers, FLAGs are already present in other drivers so that there is no error found. 
> Error occurs when applying below series. 
> 
> So I had two options to get it done. Either I can do Makefile changes along with the corresponding changes or
> make them as a separate patch. I have chosen second option.

This is the wrong choice :)
When you do a change, it should be atomic, i.e. includes changes which are dependent.

> And because of this is new change, it does not contain any "Fixes" tag.
> >
> >I guess this patch is not critical.
> >Where are patches 2, 3, 4, 5?
> Following are the series of patches:
> http://patches.dpdk.org/patch/63299/
> http://patches.dpdk.org/patch/63271/
> http://patches.dpdk.org/patch/63272/
> http://patches.dpdk.org/patch/63273/
> http://patches.dpdk.org/patch/63274/
> Not all the patches are reworked for next versions that's why same is not reflected in subject line. 

When you send a new version, you should send the whole series again.
And please don't forget changelogs and --in-reply-to.

Thanks



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

* [dpdk-dev] [PATCH v2 1/4] eal: add API to check if its interrupt context
  2019-11-26  6:15 ` [dpdk-dev] [PATCH v2 1/5] drivers/octeontx2: allow experimental symbols Sunil Kumar Kori
  2019-11-26 16:21   ` Thomas Monjalon
@ 2019-11-27 10:22   ` Sunil Kumar Kori
  2019-11-27 10:22     ` [dpdk-dev] [PATCH v2 2/4] common/octeontx2: add interrupt offset to mbox structure Sunil Kumar Kori
                       ` (4 more replies)
  1 sibling, 5 replies; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-11-27 10:22 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, Sunil Kumar Kori, Harman Kalra

Added an API to check if current execution is in interrupt
context. This will be helpful to handle nested interrupt cases.

Signed-off-by: Harman Kalra <hkalra@marvell.com>
Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
v2:
 - Rebased patch on 19.11-rc4

 lib/librte_eal/common/include/rte_interrupts.h | 15 +++++++++++++++
 lib/librte_eal/freebsd/eal/eal_interrupts.c    |  5 +++++
 lib/librte_eal/linux/eal/eal_interrupts.c      |  5 +++++
 lib/librte_eal/rte_eal_version.map             |  1 +
 4 files changed, 26 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_interrupts.h b/lib/librte_eal/common/include/rte_interrupts.h
index e3b406abc..0a82e28a8 100644
--- a/lib/librte_eal/common/include/rte_interrupts.h
+++ b/lib/librte_eal/common/include/rte_interrupts.h
@@ -138,6 +138,21 @@ int rte_intr_disable(const struct rte_intr_handle *intr_handle);
 __rte_experimental
 int rte_intr_ack(const struct rte_intr_handle *intr_handle);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Check if currently executing in interrupt context
+ *
+ * @return
+ *  - positive in case of interrupt context
+ *  - zero in case of process context
+ *  - negative if unsuccessful
+ */
+__rte_experimental
+int
+rte_thread_is_intr(void);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_eal/freebsd/eal/eal_interrupts.c b/lib/librte_eal/freebsd/eal/eal_interrupts.c
index f6831b790..ce2a27b4a 100644
--- a/lib/librte_eal/freebsd/eal/eal_interrupts.c
+++ b/lib/librte_eal/freebsd/eal/eal_interrupts.c
@@ -671,3 +671,8 @@ rte_intr_free_epoll_fd(struct rte_intr_handle *intr_handle)
 {
 	RTE_SET_USED(intr_handle);
 }
+
+int rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c b/lib/librte_eal/linux/eal/eal_interrupts.c
index 1955324d3..c00f5a575 100644
--- a/lib/librte_eal/linux/eal/eal_interrupts.c
+++ b/lib/librte_eal/linux/eal/eal_interrupts.c
@@ -1487,3 +1487,8 @@ rte_intr_cap_multiple(struct rte_intr_handle *intr_handle)
 
 	return 0;
 }
+
+int rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index e38d02530..397e787cf 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -332,4 +332,5 @@ EXPERIMENTAL {
 	# added in 19.11
 	rte_log_get_stream;
 	rte_mcfg_get_single_file_segments;
+	rte_thread_is_intr;
 };
-- 
2.17.1


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

* [dpdk-dev] [PATCH v2 2/4] common/octeontx2: add interrupt offset to mbox structure
  2019-11-27 10:22   ` [dpdk-dev] [PATCH v2 1/4] eal: add API to check if its interrupt context Sunil Kumar Kori
@ 2019-11-27 10:22     ` Sunil Kumar Kori
  2019-11-27 10:22     ` [dpdk-dev] [PATCH v2 3/4] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
                       ` (3 subsequent siblings)
  4 siblings, 0 replies; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-11-27 10:22 UTC (permalink / raw)
  To: Jerin Jacob, Nithin Dabilpuram, Vamsi Attunuru; +Cc: dev, Sunil Kumar Kori

mbox response are triggered at bar2 + RVU_PF_INT or bar2 + RVU_VF_INT,
depending upon the device. To process these interrupts different irq
handlers are installed. To unify it, added offset field into mbox
structure which is initialized with RVU_PF_INT or RVU_VF_INT at the time
of otx2_mbox_init.

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
v2:
 * Rebased patch on 19.11-rc4

 drivers/common/octeontx2/otx2_dev.c  | 14 ++++++++++----
 drivers/common/octeontx2/otx2_mbox.c |  5 +++--
 drivers/common/octeontx2/otx2_mbox.h |  5 +++--
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/common/octeontx2/otx2_dev.c b/drivers/common/octeontx2/otx2_dev.c
index 0fc799e4a..6f29d6108 100644
--- a/drivers/common/octeontx2/otx2_dev.c
+++ b/drivers/common/octeontx2/otx2_dev.c
@@ -900,6 +900,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 +925,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 +971,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..ad8e0c3aa 100644
--- a/drivers/common/octeontx2/otx2_mbox.c
+++ b/drivers/common/octeontx2/otx2_mbox.c
@@ -59,12 +59,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;
 
diff --git a/drivers/common/octeontx2/otx2_mbox.h b/drivers/common/octeontx2/otx2_mbox.h
index e0e4e2f63..162d12468 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


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

* [dpdk-dev] [PATCH v2 3/4] common/octeontx2: add polling based response mbox message
  2019-11-27 10:22   ` [dpdk-dev] [PATCH v2 1/4] eal: add API to check if its interrupt context Sunil Kumar Kori
  2019-11-27 10:22     ` [dpdk-dev] [PATCH v2 2/4] common/octeontx2: add interrupt offset to mbox structure Sunil Kumar Kori
@ 2019-11-27 10:22     ` Sunil Kumar Kori
  2019-11-27 10:22     ` [dpdk-dev] [PATCH v2 4/4] drivers/octeontx2: enhancing mbox APIs to get response Sunil Kumar Kori
                       ` (2 subsequent siblings)
  4 siblings, 0 replies; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-11-27 10:22 UTC (permalink / raw)
  To: Jerin Jacob, Nithin Dabilpuram, Vamsi Attunuru; +Cc: dev, Sunil Kumar Kori

Currently otx2_mbox_get_rsp get response once AF driver
interrupts after completion. But this funciton 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>
---
v2:
 - Included Makefile and meson build changes.
 - Rebased patch on 19.11-rc4

 drivers/common/octeontx2/Makefile             |  2 +
 drivers/common/octeontx2/meson.build          |  2 +
 drivers/common/octeontx2/otx2_mbox.c          | 58 +++++++++++++++++++
 drivers/common/octeontx2/otx2_mbox.h          |  7 +++
 .../rte_common_octeontx2_version.map          |  7 +++
 5 files changed, 76 insertions(+)

diff --git a/drivers/common/octeontx2/Makefile b/drivers/common/octeontx2/Makefile
index eaff29433..b1c1792fb 100644
--- a/drivers/common/octeontx2/Makefile
+++ b/drivers/common/octeontx2/Makefile
@@ -22,6 +22,8 @@ CFLAGS += -diag-disable 2259
 endif
 endif
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
 EXPORT_MAP := rte_common_octeontx2_version.map
 
 #
diff --git a/drivers/common/octeontx2/meson.build b/drivers/common/octeontx2/meson.build
index b79145788..a03c6a39b 100644
--- a/drivers/common/octeontx2/meson.build
+++ b/drivers/common/octeontx2/meson.build
@@ -8,6 +8,8 @@ sources= files('otx2_dev.c',
 		'otx2_common.c',
 	       )
 
+allow_experimental_apis = true
+
 extra_flags = []
 # This integrated controller runs only on a arm64 machine, remove 32bit warnings
 if not dpdk_conf.get('RTE_ARCH_64')
diff --git a/drivers/common/octeontx2/otx2_mbox.c b/drivers/common/octeontx2/otx2_mbox.c
index ad8e0c3aa..af34fd19d 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)
@@ -245,6 +246,63 @@ otx2_mbox_get_rsp(struct otx2_mbox *mbox, int devid, void **msg)
 	return msghdr->rc;
 }
 
+/**
+ * @internal
+ * Polling for given wait time to get mailbox response
+ */
+int
+otx2_mbox_get_rsp_poll_tmo(struct otx2_mbox *mbox, int devid, void **msg,
+			   uint32_t wait)
+{
+	struct otx2_mbox_dev *mdev = &mbox->dev[devid];
+	uint32_t timeout = 0, sleep = 1;
+	struct mbox_msghdr *msghdr;
+	uint64_t rsp_reg = 0;
+	uintptr_t reg_addr;
+	uint64_t offset;
+
+	rte_rmb();
+
+	offset = mbox->rx_start +
+		RTE_ALIGN(sizeof(struct mbox_hdr), MBOX_MSG_ALIGN);
+	msghdr = (struct mbox_msghdr *)((uintptr_t)mdev->mbase + offset);
+
+	reg_addr = mbox->reg_base + mbox->intr_offset;
+	while (!rsp_reg) {
+		rte_rmb();
+		rsp_reg = otx2_read64(reg_addr);
+
+		if (timeout >= wait)
+			return -ETIMEDOUT;
+
+		rte_delay_ms(sleep);
+		timeout += sleep;
+	}
+
+	if (msg != NULL)
+		*msg = msghdr;
+
+	/* Clear interrupt */
+	otx2_write64(rsp_reg, reg_addr);
+
+	/* Reset mbox */
+	otx2_mbox_reset(mbox, 0);
+
+	return msghdr->rc;
+}
+
+/**
+ * @internal
+ * Polling for 5 seconds to get mailbox response
+ */
+int
+otx2_mbox_get_rsp_poll(struct otx2_mbox *mbox, int devid, void **msg)
+{
+	uint32_t wait = 5 * MS_PER_S; /* 5 Seconds */
+
+	return otx2_mbox_get_rsp_poll_tmo(mbox, devid, msg, wait);
+}
+
 /**
  * @internal
  * Wait and get mailbox response with timeout
diff --git a/drivers/common/octeontx2/otx2_mbox.h b/drivers/common/octeontx2/otx2_mbox.h
index 162d12468..237d4cf45 100644
--- a/drivers/common/octeontx2/otx2_mbox.h
+++ b/drivers/common/octeontx2/otx2_mbox.h
@@ -1570,6 +1570,13 @@ void otx2_mbox_msg_send(struct otx2_mbox *mbox, int devid);
 int otx2_mbox_wait_for_rsp(struct otx2_mbox *mbox, int devid);
 int otx2_mbox_wait_for_rsp_tmo(struct otx2_mbox *mbox, int devid, uint32_t tmo);
 int otx2_mbox_get_rsp(struct otx2_mbox *mbox, int devid, void **msg);
+
+__rte_experimental
+int otx2_mbox_get_rsp_poll(struct otx2_mbox *mbox, int devid, void **msg);
+__rte_experimental
+int otx2_mbox_get_rsp_poll_tmo(struct otx2_mbox *mbox, int devid, void **msg,
+			       uint32_t tmo);
+
 int otx2_mbox_get_rsp_tmo(struct otx2_mbox *mbox, int devid, void **msg,
 			  uint32_t tmo);
 int otx2_mbox_get_availmem(struct otx2_mbox *mbox, int devid);
diff --git a/drivers/common/octeontx2/rte_common_octeontx2_version.map b/drivers/common/octeontx2/rte_common_octeontx2_version.map
index adad21a2d..dcbca2444 100644
--- a/drivers/common/octeontx2/rte_common_octeontx2_version.map
+++ b/drivers/common/octeontx2/rte_common_octeontx2_version.map
@@ -33,3 +33,10 @@ DPDK_20.0 {
 
 	local: *;
 };
+
+EXPERIMENTAL {
+	global:
+
+	otx2_mbox_get_rsp_poll;
+	otx2_mbox_get_rsp_poll_tmo;
+};
-- 
2.17.1


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

* [dpdk-dev] [PATCH v2 4/4] drivers/octeontx2: enhancing mbox APIs to get response
  2019-11-27 10:22   ` [dpdk-dev] [PATCH v2 1/4] eal: add API to check if its interrupt context Sunil Kumar Kori
  2019-11-27 10:22     ` [dpdk-dev] [PATCH v2 2/4] common/octeontx2: add interrupt offset to mbox structure Sunil Kumar Kori
  2019-11-27 10:22     ` [dpdk-dev] [PATCH v2 3/4] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
@ 2019-11-27 10:22     ` Sunil Kumar Kori
  2019-11-28  0:03     ` [dpdk-dev] [PATCH v2 1/4] eal: add API to check if its interrupt context Stephen Hemminger
  2019-12-16 10:39     ` [dpdk-dev] [PATCH v3 1/2] " Sunil Kumar Kori
  4 siblings, 0 replies; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-11-27 10:22 UTC (permalink / raw)
  To: Jerin Jacob, Nithin Dabilpuram, Vamsi Attunuru, Kiran Kumar K, Satha Rao
  Cc: dev, Sunil Kumar Kori

Based on thread context, mbox API will get response for submitted
request. If thread runs in interrupt context then it uses polling
based get response API otherwise interrupt based.

Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
v2:
 - Included Makefile and meson build changes.
 - Rebased patch on 19.11-rc4

 drivers/common/octeontx2/otx2_dev.c   | 27 ++++++++++++---------------
 drivers/common/octeontx2/otx2_mbox.h  | 21 +++++++++++++++++----
 drivers/net/octeontx2/Makefile        |  2 ++
 drivers/net/octeontx2/meson.build     |  2 ++
 drivers/raw/octeontx2_dma/Makefile    |  2 ++
 drivers/raw/octeontx2_dma/meson.build |  2 ++
 6 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/drivers/common/octeontx2/otx2_dev.c b/drivers/common/octeontx2/otx2_dev.c
index 6f29d6108..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
diff --git a/drivers/common/octeontx2/otx2_mbox.h b/drivers/common/octeontx2/otx2_mbox.h
index 237d4cf45..e82dfe530 100644
--- a/drivers/common/octeontx2/otx2_mbox.h
+++ b/drivers/common/octeontx2/otx2_mbox.h
@@ -9,6 +9,7 @@
 #include <stdbool.h>
 
 #include <rte_ether.h>
+#include <rte_interrupts.h>
 #include <rte_spinlock.h>
 
 #include <otx2_common.h>
@@ -1627,28 +1628,40 @@ static inline int
 otx2_mbox_process(struct otx2_mbox *mbox)
 {
 	otx2_mbox_msg_send(mbox, 0);
-	return otx2_mbox_get_rsp(mbox, 0, NULL);
+	if (rte_thread_is_intr())
+		return otx2_mbox_get_rsp_poll(mbox, 0, NULL);
+	else
+		return otx2_mbox_get_rsp(mbox, 0, NULL);
 }
 
 static inline int
 otx2_mbox_process_msg(struct otx2_mbox *mbox, void **msg)
 {
 	otx2_mbox_msg_send(mbox, 0);
-	return otx2_mbox_get_rsp(mbox, 0, msg);
+	if (rte_thread_is_intr())
+		return otx2_mbox_get_rsp_poll(mbox, 0, msg);
+	else
+		return otx2_mbox_get_rsp(mbox, 0, msg);
 }
 
 static inline int
 otx2_mbox_process_tmo(struct otx2_mbox *mbox, uint32_t tmo)
 {
 	otx2_mbox_msg_send(mbox, 0);
-	return otx2_mbox_get_rsp_tmo(mbox, 0, NULL, tmo);
+	if (rte_thread_is_intr())
+		return otx2_mbox_get_rsp_poll_tmo(mbox, 0, NULL, tmo);
+	else
+		return otx2_mbox_get_rsp_tmo(mbox, 0, NULL, tmo);
 }
 
 static inline int
 otx2_mbox_process_msg_tmo(struct otx2_mbox *mbox, void **msg, uint32_t tmo)
 {
 	otx2_mbox_msg_send(mbox, 0);
-	return otx2_mbox_get_rsp_tmo(mbox, 0, msg, tmo);
+	if (rte_thread_is_intr())
+		return otx2_mbox_get_rsp_poll_tmo(mbox, 0, msg, tmo);
+	else
+		return otx2_mbox_get_rsp_tmo(mbox, 0, msg, tmo);
 }
 
 int otx2_send_ready_msg(struct otx2_mbox *mbox, uint16_t *pf_func /* out */);
diff --git a/drivers/net/octeontx2/Makefile b/drivers/net/octeontx2/Makefile
index 68f5765db..3da4d8cc1 100644
--- a/drivers/net/octeontx2/Makefile
+++ b/drivers/net/octeontx2/Makefile
@@ -26,6 +26,8 @@ CFLAGS += -diag-disable 2259
 endif
 endif
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
 EXPORT_MAP := rte_pmd_octeontx2_version.map
 
 #
diff --git a/drivers/net/octeontx2/meson.build b/drivers/net/octeontx2/meson.build
index fad3076a3..a976a2c19 100644
--- a/drivers/net/octeontx2/meson.build
+++ b/drivers/net/octeontx2/meson.build
@@ -24,6 +24,8 @@ sources = files('otx2_rx.c',
 		'otx2_ethdev_devargs.c'
 		)
 
+allow_experimental_apis = true
+
 deps += ['bus_pci', 'common_octeontx2', 'mempool_octeontx2']
 
 cflags += ['-flax-vector-conversions']
diff --git a/drivers/raw/octeontx2_dma/Makefile b/drivers/raw/octeontx2_dma/Makefile
index c64ca3497..0d0c530fe 100644
--- a/drivers/raw/octeontx2_dma/Makefile
+++ b/drivers/raw/octeontx2_dma/Makefile
@@ -22,6 +22,8 @@ CFLAGS += -diag-disable 2259
 endif
 endif
 
+CFLAGS += -DALLOW_EXPERIMENTAL_API
+
 EXPORT_MAP := rte_rawdev_octeontx2_dma_version.map
 
 #
diff --git a/drivers/raw/octeontx2_dma/meson.build b/drivers/raw/octeontx2_dma/meson.build
index 11f74680a..f8f88aa7e 100644
--- a/drivers/raw/octeontx2_dma/meson.build
+++ b/drivers/raw/octeontx2_dma/meson.build
@@ -5,6 +5,8 @@
 deps += ['bus_pci', 'common_octeontx2', 'rawdev']
 sources = files('otx2_dpi_rawdev.c', 'otx2_dpi_msg.c', 'otx2_dpi_test.c')
 
+allow_experimental_apis = true
+
 extra_flags = []
 # This integrated controller runs only on a arm64 machine, remove 32bit warnings
 if not dpdk_conf.get('RTE_ARCH_64')
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH v2 1/4] eal: add API to check if its interrupt context
  2019-11-27 10:22   ` [dpdk-dev] [PATCH v2 1/4] eal: add API to check if its interrupt context Sunil Kumar Kori
                       ` (2 preceding siblings ...)
  2019-11-27 10:22     ` [dpdk-dev] [PATCH v2 4/4] drivers/octeontx2: enhancing mbox APIs to get response Sunil Kumar Kori
@ 2019-11-28  0:03     ` Stephen Hemminger
  2019-11-28  8:10       ` [dpdk-dev] [EXT] " Harman Kalra
  2019-12-16 10:39     ` [dpdk-dev] [PATCH v3 1/2] " Sunil Kumar Kori
  4 siblings, 1 reply; 38+ messages in thread
From: Stephen Hemminger @ 2019-11-28  0:03 UTC (permalink / raw)
  To: Sunil Kumar Kori; +Cc: Bruce Richardson, dev, Harman Kalra

On Wed, 27 Nov 2019 15:52:19 +0530
Sunil Kumar Kori <skori@marvell.com> wrote:

>  
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Check if currently executing in interrupt context
> + *
> + * @return
> + *  - positive in case of interrupt context
> + *  - zero in case of process context
> + *  - negative if unsuccessful
> + */
> +__rte_experimental
> +int
> +rte_thread_is_intr(void);

If you only need this in drivers, it should be internal not exposed
as part of API

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

* Re: [dpdk-dev] [EXT] Re: [PATCH v2 1/4] eal: add API to check if its interrupt context
  2019-11-28  0:03     ` [dpdk-dev] [PATCH v2 1/4] eal: add API to check if its interrupt context Stephen Hemminger
@ 2019-11-28  8:10       ` Harman Kalra
  2019-11-28 16:45         ` Stephen Hemminger
  0 siblings, 1 reply; 38+ messages in thread
From: Harman Kalra @ 2019-11-28  8:10 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Sunil Kumar Kori, Bruce Richardson, dev

On Wed, Nov 27, 2019 at 04:03:19PM -0800, Stephen Hemminger wrote:
> External Email
> 
> ----------------------------------------------------------------------
> On Wed, 27 Nov 2019 15:52:19 +0530
> Sunil Kumar Kori <skori@marvell.com> wrote:
> 
> >  
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice
> > + *
> > + * Check if currently executing in interrupt context
> > + *
> > + * @return
> > + *  - positive in case of interrupt context
> > + *  - zero in case of process context
> > + *  - negative if unsuccessful
> > + */
> > +__rte_experimental
> > +int
> > +rte_thread_is_intr(void);
> 
> If you only need this in drivers, it should be internal not exposed
> as part of API

Sorry, but can you please help me understand the query. Do you mean:
* Since "rte_thread_is_intr"  would be used only used by
libraries/drivers and not by any external application, rather having
it in "rte_interrupt.h", we should have it in some internal header
file like "eal_private.h" ??

ANS - Yes we can do that but since all other related APIs like
"rte_intr_ack", "rte_intr_enable/disable" which are also used by
drivers/lib and not application, are prototyped in "rte_interupt.h".

OR do u mean
* Since only octeontx2 driver is using this, why it is exposed as an API
rather it should be defined as some driver internal function ??

ANS - "rte_thread_is_intr" is an counter part of "in_interrupt()" in
DPDK, which will return whether the current execution is in interrupt
context. This helps in dealing with nested interrupts case. We faced a
similar case while handling hotplug probing initiated via secondary
process.
We believe this API could be very useful for many drivers which might
end up in handling nested interrupts case.

Thanks
Harman

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

* Re: [dpdk-dev] [EXT] Re: [PATCH v2 1/4] eal: add API to check if its interrupt context
  2019-11-28  8:10       ` [dpdk-dev] [EXT] " Harman Kalra
@ 2019-11-28 16:45         ` Stephen Hemminger
  2019-12-04 16:23           ` Harman Kalra
  0 siblings, 1 reply; 38+ messages in thread
From: Stephen Hemminger @ 2019-11-28 16:45 UTC (permalink / raw)
  To: Harman Kalra; +Cc: Sunil Kumar Kori, Bruce Richardson, dev

On Thu, 28 Nov 2019 08:10:23 +0000
Harman Kalra <hkalra@marvell.com> wrote:

> On Wed, Nov 27, 2019 at 04:03:19PM -0800, Stephen Hemminger wrote:
> > External Email
> > 
> > ----------------------------------------------------------------------
> > On Wed, 27 Nov 2019 15:52:19 +0530
> > Sunil Kumar Kori <skori@marvell.com> wrote:
> >   
> > >  
> > > +/**
> > > + * @warning
> > > + * @b EXPERIMENTAL: this API may change without prior notice
> > > + *
> > > + * Check if currently executing in interrupt context
> > > + *
> > > + * @return
> > > + *  - positive in case of interrupt context
> > > + *  - zero in case of process context
> > > + *  - negative if unsuccessful
> > > + */
> > > +__rte_experimental
> > > +int
> > > +rte_thread_is_intr(void);  
> > 
> > If you only need this in drivers, it should be internal not exposed
> > as part of API  
> 
> Sorry, but can you please help me understand the query. Do you mean:
> * Since "rte_thread_is_intr"  would be used only used by
> libraries/drivers and not by any external application, rather having
> it in "rte_interrupt.h", we should have it in some internal header
> file like "eal_private.h" ??
> 
> ANS - Yes we can do that but since all other related APIs like
> "rte_intr_ack", "rte_intr_enable/disable" which are also used by
> drivers/lib and not application, are prototyped in "rte_interupt.h".
> 
> OR do u mean
> * Since only octeontx2 driver is using this, why it is exposed as an API
> rather it should be defined as some driver internal function ??
> 
> ANS - "rte_thread_is_intr" is an counter part of "in_interrupt()" in
> DPDK, which will return whether the current execution is in interrupt
> context. This helps in dealing with nested interrupts case. We faced a
> similar case while handling hotplug probing initiated via secondary
> process.
> We believe this API could be very useful for many drivers which might
> end up in handling nested interrupts case.
> 
> Thanks
> Harman

What I meant was that some functions in EAL are documented as
being internal (flagged with @internal) and don't show up in
the documented API.

My concern is that if we make this part of the public API, it makes
it harder to change structural things like the interrupt thread
later.  The interrupt thread is already problematic for several
other usages.


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

* Re: [dpdk-dev] [EXT] Re: [PATCH v2 1/4] eal: add API to check if its interrupt context
  2019-11-28 16:45         ` Stephen Hemminger
@ 2019-12-04 16:23           ` Harman Kalra
  0 siblings, 0 replies; 38+ messages in thread
From: Harman Kalra @ 2019-12-04 16:23 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Sunil Kumar Kori, Bruce Richardson, dev

On Thu, Nov 28, 2019 at 08:45:42AM -0800, Stephen Hemminger wrote:
> On Thu, 28 Nov 2019 08:10:23 +0000
> Harman Kalra <hkalra@marvell.com> wrote:
> 
> > On Wed, Nov 27, 2019 at 04:03:19PM -0800, Stephen Hemminger wrote:
> > > External Email
> > > 
> > > ----------------------------------------------------------------------
> > > On Wed, 27 Nov 2019 15:52:19 +0530
> > > Sunil Kumar Kori <skori@marvell.com> wrote:
> > >   
> > > >  
> > > > +/**
> > > > + * @warning
> > > > + * @b EXPERIMENTAL: this API may change without prior notice
> > > > + *
> > > > + * Check if currently executing in interrupt context
> > > > + *
> > > > + * @return
> > > > + *  - positive in case of interrupt context
> > > > + *  - zero in case of process context
> > > > + *  - negative if unsuccessful
> > > > + */
> > > > +__rte_experimental
> > > > +int
> > > > +rte_thread_is_intr(void);  
> > > 
> > > If you only need this in drivers, it should be internal not exposed
> > > as part of API  
> > 
> > Sorry, but can you please help me understand the query. Do you mean:
> > * Since "rte_thread_is_intr"  would be used only used by
> > libraries/drivers and not by any external application, rather having
> > it in "rte_interrupt.h", we should have it in some internal header
> > file like "eal_private.h" ??
> > 
> > ANS - Yes we can do that but since all other related APIs like
> > "rte_intr_ack", "rte_intr_enable/disable" which are also used by
> > drivers/lib and not application, are prototyped in "rte_interupt.h".
> > 
> > OR do u mean
> > * Since only octeontx2 driver is using this, why it is exposed as an API
> > rather it should be defined as some driver internal function ??
> > 
> > ANS - "rte_thread_is_intr" is an counter part of "in_interrupt()" in
> > DPDK, which will return whether the current execution is in interrupt
> > context. This helps in dealing with nested interrupts case. We faced a
> > similar case while handling hotplug probing initiated via secondary
> > process.
> > We believe this API could be very useful for many drivers which might
> > end up in handling nested interrupts case.
> > 
> > Thanks
> > Harman
> 
> What I meant was that some functions in EAL are documented as
> being internal (flagged with @internal) and don't show up in
> the documented API.
> 
> My concern is that if we make this part of the public API, it makes
> it harder to change structural things like the interrupt thread
> later.  The interrupt thread is already problematic for several
> other usages.
> 

I think now I understood your point, I will move prototype of this API to
"common/include/rte_eal_interrupts.h", flag it as @internal
and resend next version. I think this file was created for such
APIs only as its description says.

/**
 * @file rte_eal_interrupts.h
 * @internal
 *
 * Contains function prototypes exposed by the EAL for interrupt handling by
 * drivers and other DPDK internal consumers.
 */

Thanks
Harman

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

* [dpdk-dev] [PATCH v3 1/2] eal: add API to check if its interrupt context
  2019-11-27 10:22   ` [dpdk-dev] [PATCH v2 1/4] eal: add API to check if its interrupt context Sunil Kumar Kori
                       ` (3 preceding siblings ...)
  2019-11-28  0:03     ` [dpdk-dev] [PATCH v2 1/4] eal: add API to check if its interrupt context Stephen Hemminger
@ 2019-12-16 10:39     ` Sunil Kumar Kori
  2019-12-16 10:39       ` [dpdk-dev] [PATCH v3 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
  4 siblings, 1 reply; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-12-16 10:39 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, Harman Kalra, Sunil Kumar Kori

From: Harman Kalra <hkalra@marvell.com>

Added an API to check if current execution is in interrupt
context. This will be helpful to handle nested interrupt cases.

Signed-off-by: Harman Kalra <hkalra@marvell.com>
Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
v3:
 - API Comment is updated as per man page.
 - Scope updated within the library/driver only.
 - Remove experimental tag
v2:
 - Rebased patch on 19.11-rc4

 lib/librte_eal/common/include/rte_eal_interrupts.h | 11 +++++++++++
 lib/librte_eal/freebsd/eal/eal_interrupts.c        |  5 +++++
 lib/librte_eal/linux/eal/eal_interrupts.c          |  5 +++++
 3 files changed, 21 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_eal_interrupts.h b/lib/librte_eal/common/include/rte_eal_interrupts.h
index b370c0d26..19d1d45ab 100644
--- a/lib/librte_eal/common/include/rte_eal_interrupts.h
+++ b/lib/librte_eal/common/include/rte_eal_interrupts.h
@@ -220,4 +220,15 @@ rte_intr_allow_others(struct rte_intr_handle *intr_handle);
 int
 rte_intr_cap_multiple(struct rte_intr_handle *intr_handle);
 
+/**
+ * @internal
+ * Check if currently executing in interrupt context
+ *
+ * @return
+ *  - non zero in case of interrupt context
+ *  - zero in case of process context
+ */
+int
+rte_thread_is_intr(void);
+
 #endif /* _RTE_EAL_INTERRUPTS_H_ */
diff --git a/lib/librte_eal/freebsd/eal/eal_interrupts.c b/lib/librte_eal/freebsd/eal/eal_interrupts.c
index f6831b790..ce2a27b4a 100644
--- a/lib/librte_eal/freebsd/eal/eal_interrupts.c
+++ b/lib/librte_eal/freebsd/eal/eal_interrupts.c
@@ -671,3 +671,8 @@ rte_intr_free_epoll_fd(struct rte_intr_handle *intr_handle)
 {
 	RTE_SET_USED(intr_handle);
 }
+
+int rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c b/lib/librte_eal/linux/eal/eal_interrupts.c
index 14ebb108c..cb8e10709 100644
--- a/lib/librte_eal/linux/eal/eal_interrupts.c
+++ b/lib/librte_eal/linux/eal/eal_interrupts.c
@@ -1488,3 +1488,8 @@ rte_intr_cap_multiple(struct rte_intr_handle *intr_handle)
 
 	return 0;
 }
+
+int rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
-- 
2.17.1


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

* [dpdk-dev] [PATCH v3 2/2] common/octeontx2: add polling based response mbox message
  2019-12-16 10:39     ` [dpdk-dev] [PATCH v3 1/2] " Sunil Kumar Kori
@ 2019-12-16 10:39       ` Sunil Kumar Kori
  2019-12-17  8:02         ` Gavin Hu (Arm Technology China)
                           ` (2 more replies)
  0 siblings, 3 replies; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-12-16 10:39 UTC (permalink / raw)
  To: Jerin Jacob, Nithin Dabilpuram, Vamsi Attunuru
  Cc: dev, Sunil Kumar Kori, Harman Kalra

Currently otx2_mbox_get_rsp_xxx get response once AF driver
interrupts after completion. But this funciton 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>
---
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 | 57 +++++++++++++++++++++++-----
 drivers/common/octeontx2/otx2_mbox.h |  5 ++-
 3 files changed, 72 insertions(+), 31 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..f07f07918 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;
 
@@ -230,8 +232,8 @@ otx2_mbox_get_rsp(struct otx2_mbox *mbox, int devid, void **msg)
 	int rc;
 
 	rc = otx2_mbox_wait_for_rsp(mbox, devid);
-	if (rc != 1)
-		return -EIO;
+	if (rc < 0)
+		return rc;
 
 	rte_rmb();
 
@@ -244,6 +246,37 @@ 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;
+	uint64_t rsp_reg = 0;
+	uintptr_t reg_addr;
+
+	reg_addr = mbox->reg_base + mbox->intr_offset;
+	while (!rsp_reg) {
+		rte_rmb();
+		rsp_reg = otx2_read64(reg_addr);
+
+		if (timeout >= wait)
+			return -ETIMEDOUT;
+
+		rte_delay_us(sleep);
+		timeout += sleep;
+	}
+
+	/* 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
@@ -258,8 +291,8 @@ otx2_mbox_get_rsp_tmo(struct otx2_mbox *mbox, int devid, void **msg,
 	int rc;
 
 	rc = otx2_mbox_wait_for_rsp_tmo(mbox, devid, tmo);
-	if (rc != 1)
-		return -EIO;
+	if (rc < 0)
+		return rc;
 
 	rte_rmb();
 
@@ -321,11 +354,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);
+		if (!rc)
+			rc = mdev->msgs_acked;
+	}
 
-	return mdev->msgs_acked;
+	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


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

* Re: [dpdk-dev] [PATCH v3 2/2] common/octeontx2: add polling based response mbox message
  2019-12-16 10:39       ` [dpdk-dev] [PATCH v3 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
@ 2019-12-17  8:02         ` Gavin Hu (Arm Technology China)
  2019-12-17 11:14           ` Jerin Jacob
  2019-12-17 10:42         ` [dpdk-dev] [PATCH v4 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
  2019-12-17 16:53         ` [dpdk-dev] [PATCH v4 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
  2 siblings, 1 reply; 38+ messages in thread
From: Gavin Hu (Arm Technology China) @ 2019-12-17  8:02 UTC (permalink / raw)
  To: Sunil Kumar Kori, jerinj, Nithin Dabilpuram, Vamsi Attunuru
  Cc: dev, Harman Kalra, Honnappa Nagarahalli, nd

Hi Sunil,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Sunil Kumar Kori
> Sent: Monday, December 16, 2019 6:40 PM
> To: jerinj@marvell.com; Nithin Dabilpuram <ndabilpuram@marvell.com>;
> Vamsi Attunuru <vattunuru@marvell.com>
> Cc: dev@dpdk.org; Sunil Kumar Kori <skori@marvell.com>; Harman Kalra
> <hkalra@marvell.com>
> Subject: [dpdk-dev] [PATCH v3 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 funciton 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>
> ---
> 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 | 57 +++++++++++++++++++++++---
> --
>  drivers/common/octeontx2/otx2_mbox.h |  5 ++-
>  3 files changed, 72 insertions(+), 31 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..f07f07918 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;
> 
> @@ -230,8 +232,8 @@ otx2_mbox_get_rsp(struct otx2_mbox *mbox, int
> devid, void **msg)
>  	int rc;
> 
>  	rc = otx2_mbox_wait_for_rsp(mbox, devid);
> -	if (rc != 1)
> -		return -EIO;
> +	if (rc < 0)
> +		return rc;
> 
>  	rte_rmb();
> 
> @@ -244,6 +246,37 @@ 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;
> +	uint64_t rsp_reg = 0;
> +	uintptr_t reg_addr;
> +
> +	reg_addr = mbox->reg_base + mbox->intr_offset;
> +	while (!rsp_reg) {
The first iteration of (!rsp_reg) always evaluate to 'true'.
Why not use do .. while to save one iteration?
> +		rte_rmb();
Rte_rmb is overkill, how is reg_addr mapped(what is the memory attribute? WC? cacheable? )
If it is a PCI mmaped region, then rte_io_barrier is okay, I am proposing to relax the io barrier for aarch64 as a compiler barrier. 
 http://patches.dpdk.org/patch/61662/  

> +		rsp_reg = otx2_read64(reg_addr);
> +
> +		if (timeout >= wait)
> +			return -ETIMEDOUT;
> +
> +		rte_delay_us(sleep);
> +		timeout += sleep;
> +	}
> +
> +	/* 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
> @@ -258,8 +291,8 @@ otx2_mbox_get_rsp_tmo(struct otx2_mbox *mbox,
> int devid, void **msg,
>  	int rc;
> 
>  	rc = otx2_mbox_wait_for_rsp_tmo(mbox, devid, tmo);
> -	if (rc != 1)
> -		return -EIO;
> +	if (rc < 0)
> +		return rc;
> 
>  	rte_rmb();
> 
> @@ -321,11 +354,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);
> +		if (!rc)
> +			rc = mdev->msgs_acked;
> +	}
> 
> -	return mdev->msgs_acked;
> +	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


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

* [dpdk-dev] [PATCH v4 1/2] eal: add API to check if its interrupt context
  2019-12-16 10:39       ` [dpdk-dev] [PATCH v3 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
  2019-12-17  8:02         ` Gavin Hu (Arm Technology China)
@ 2019-12-17 10:42         ` Sunil Kumar Kori
  2019-12-17 10:42           ` [dpdk-dev] [PATCH v4 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
  2019-12-17 16:53         ` [dpdk-dev] [PATCH v4 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
  2 siblings, 1 reply; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-12-17 10:42 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, Harman Kalra, Sunil Kumar Kori

From: Harman Kalra <hkalra@marvell.com>

Added an API to check if current execution is in interrupt
context. This will be helpful to handle nested interrupt cases.

Signed-off-by: Harman Kalra <hkalra@marvell.com>
Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
v3:
 - API Comment is updated as per man page.
 - Scope updated within the library/driver only.
 - Remove experimental tag
v2:
 - Rebased patch on 19.11-rc4

 lib/librte_eal/common/include/rte_eal_interrupts.h | 11 +++++++++++
 lib/librte_eal/freebsd/eal/eal_interrupts.c        |  5 +++++
 lib/librte_eal/linux/eal/eal_interrupts.c          |  5 +++++
 3 files changed, 21 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_eal_interrupts.h b/lib/librte_eal/common/include/rte_eal_interrupts.h
index b370c0d26..19d1d45ab 100644
--- a/lib/librte_eal/common/include/rte_eal_interrupts.h
+++ b/lib/librte_eal/common/include/rte_eal_interrupts.h
@@ -220,4 +220,15 @@ rte_intr_allow_others(struct rte_intr_handle *intr_handle);
 int
 rte_intr_cap_multiple(struct rte_intr_handle *intr_handle);
 
+/**
+ * @internal
+ * Check if currently executing in interrupt context
+ *
+ * @return
+ *  - non zero in case of interrupt context
+ *  - zero in case of process context
+ */
+int
+rte_thread_is_intr(void);
+
 #endif /* _RTE_EAL_INTERRUPTS_H_ */
diff --git a/lib/librte_eal/freebsd/eal/eal_interrupts.c b/lib/librte_eal/freebsd/eal/eal_interrupts.c
index f6831b790..ce2a27b4a 100644
--- a/lib/librte_eal/freebsd/eal/eal_interrupts.c
+++ b/lib/librte_eal/freebsd/eal/eal_interrupts.c
@@ -671,3 +671,8 @@ rte_intr_free_epoll_fd(struct rte_intr_handle *intr_handle)
 {
 	RTE_SET_USED(intr_handle);
 }
+
+int rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c b/lib/librte_eal/linux/eal/eal_interrupts.c
index 14ebb108c..cb8e10709 100644
--- a/lib/librte_eal/linux/eal/eal_interrupts.c
+++ b/lib/librte_eal/linux/eal/eal_interrupts.c
@@ -1488,3 +1488,8 @@ rte_intr_cap_multiple(struct rte_intr_handle *intr_handle)
 
 	return 0;
 }
+
+int rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
-- 
2.17.1


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

* [dpdk-dev] [PATCH v4 2/2] common/octeontx2: add polling based response mbox message
  2019-12-17 10:42         ` [dpdk-dev] [PATCH v4 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
@ 2019-12-17 10:42           ` Sunil Kumar Kori
  0 siblings, 0 replies; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-12-17 10:42 UTC (permalink / raw)
  To: Jerin Jacob, Nithin Dabilpuram, Vamsi Attunuru
  Cc: dev, Sunil Kumar Kori, Harman Kalra

Currently otx2_mbox_get_rsp_xxx get response once AF driver
interrupts after completion. But this funciton 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>
---
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 | 58 +++++++++++++++++++++++-----
 drivers/common/octeontx2/otx2_mbox.h |  5 ++-
 3 files changed, 73 insertions(+), 31 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..64f5b9afa 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;
 
@@ -230,8 +232,8 @@ otx2_mbox_get_rsp(struct otx2_mbox *mbox, int devid, void **msg)
 	int rc;
 
 	rc = otx2_mbox_wait_for_rsp(mbox, devid);
-	if (rc != 1)
-		return -EIO;
+	if (rc < 0)
+		return rc;
 
 	rte_rmb();
 
@@ -244,6 +246,38 @@ 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 {
+		rte_io_rmb();
+		rsp_reg = otx2_read64(reg_addr);
+
+		if (timeout >= wait_us)
+			return -ETIMEDOUT;
+
+		rte_delay_us(sleep);
+		timeout += sleep;
+	} while (!rsp_reg);
+
+	/* 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
@@ -258,8 +292,8 @@ otx2_mbox_get_rsp_tmo(struct otx2_mbox *mbox, int devid, void **msg,
 	int rc;
 
 	rc = otx2_mbox_wait_for_rsp_tmo(mbox, devid, tmo);
-	if (rc != 1)
-		return -EIO;
+	if (rc < 0)
+		return rc;
 
 	rte_rmb();
 
@@ -321,11 +355,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


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

* Re: [dpdk-dev] [PATCH v3 2/2] common/octeontx2: add polling based response mbox message
  2019-12-17  8:02         ` Gavin Hu (Arm Technology China)
@ 2019-12-17 11:14           ` Jerin Jacob
  2019-12-18  2:45             ` Gavin Hu
  0 siblings, 1 reply; 38+ messages in thread
From: Jerin Jacob @ 2019-12-17 11:14 UTC (permalink / raw)
  To: Gavin Hu (Arm Technology China)
  Cc: Sunil Kumar Kori, jerinj, Nithin Dabilpuram, Vamsi Attunuru, dev,
	Harman Kalra, Honnappa Nagarahalli, nd

> > +mbox_poll(struct otx2_mbox *mbox, uint32_t wait)
> > +{
> > +     uint32_t timeout = 0, sleep = 1;
> > +     uint64_t rsp_reg = 0;
> > +     uintptr_t reg_addr;
> > +
> > +     reg_addr = mbox->reg_base + mbox->intr_offset;
> > +     while (!rsp_reg) {
> The first iteration of (!rsp_reg) always evaluate to 'true'.
> Why not use do .. while to save one iteration?
> > +             rte_rmb();
> Rte_rmb is overkill, how is reg_addr mapped(what is the memory attribute? WC? cacheable? )
> If it is a PCI mmaped region, then rte_io_barrier is okay, I am proposing to relax the io barrier for aarch64 as a compiler barrier.
>  http://patches.dpdk.org/patch/61662/

It is the device memory and Linux kernel(_CPU_ not IO) is updating the
memory on the other side. I think,
we would need rte_smp_rmb() here. Since it was slow-path code, added
the full barrier here.
I think it is OK to change rte_smp_rmb() not to rte_io_* version. Correct?

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

* [dpdk-dev] [PATCH v4 1/2] eal: add API to check if its interrupt context
  2019-12-16 10:39       ` [dpdk-dev] [PATCH v3 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
  2019-12-17  8:02         ` Gavin Hu (Arm Technology China)
  2019-12-17 10:42         ` [dpdk-dev] [PATCH v4 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
@ 2019-12-17 16:53         ` Sunil Kumar Kori
  2019-12-17 16:53           ` [dpdk-dev] [PATCH v4 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
  2019-12-18  7:07           ` [dpdk-dev] [PATCH v5 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
  2 siblings, 2 replies; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-12-17 16:53 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, Harman Kalra, Sunil Kumar Kori

From: Harman Kalra <hkalra@marvell.com>

Added an API to check if current execution is in interrupt
context. This will be helpful to handle nested interrupt cases.

Signed-off-by: Harman Kalra <hkalra@marvell.com>
Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
v3:
 - API Comment is updated as per man page.
 - Scope updated within the library/driver only.
 - Remove experimental tag
v2:
 - Rebased patch on 19.11-rc4

 lib/librte_eal/common/include/rte_eal_interrupts.h | 11 +++++++++++
 lib/librte_eal/freebsd/eal/eal_interrupts.c        |  5 +++++
 lib/librte_eal/linux/eal/eal_interrupts.c          |  5 +++++
 3 files changed, 21 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_eal_interrupts.h b/lib/librte_eal/common/include/rte_eal_interrupts.h
index b370c0d26..19d1d45ab 100644
--- a/lib/librte_eal/common/include/rte_eal_interrupts.h
+++ b/lib/librte_eal/common/include/rte_eal_interrupts.h
@@ -220,4 +220,15 @@ rte_intr_allow_others(struct rte_intr_handle *intr_handle);
 int
 rte_intr_cap_multiple(struct rte_intr_handle *intr_handle);
 
+/**
+ * @internal
+ * Check if currently executing in interrupt context
+ *
+ * @return
+ *  - non zero in case of interrupt context
+ *  - zero in case of process context
+ */
+int
+rte_thread_is_intr(void);
+
 #endif /* _RTE_EAL_INTERRUPTS_H_ */
diff --git a/lib/librte_eal/freebsd/eal/eal_interrupts.c b/lib/librte_eal/freebsd/eal/eal_interrupts.c
index f6831b790..ce2a27b4a 100644
--- a/lib/librte_eal/freebsd/eal/eal_interrupts.c
+++ b/lib/librte_eal/freebsd/eal/eal_interrupts.c
@@ -671,3 +671,8 @@ rte_intr_free_epoll_fd(struct rte_intr_handle *intr_handle)
 {
 	RTE_SET_USED(intr_handle);
 }
+
+int rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c b/lib/librte_eal/linux/eal/eal_interrupts.c
index 14ebb108c..cb8e10709 100644
--- a/lib/librte_eal/linux/eal/eal_interrupts.c
+++ b/lib/librte_eal/linux/eal/eal_interrupts.c
@@ -1488,3 +1488,8 @@ rte_intr_cap_multiple(struct rte_intr_handle *intr_handle)
 
 	return 0;
 }
+
+int rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
-- 
2.17.1


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

* [dpdk-dev] [PATCH v4 2/2] common/octeontx2: add polling based response mbox message
  2019-12-17 16:53         ` [dpdk-dev] [PATCH v4 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
@ 2019-12-17 16:53           ` Sunil Kumar Kori
  2019-12-18  2:54             ` Gavin Hu
  2019-12-18  7:07           ` [dpdk-dev] [PATCH v5 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
  1 sibling, 1 reply; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-12-17 16:53 UTC (permalink / raw)
  To: Jerin Jacob, Nithin Dabilpuram, Vamsi Attunuru
  Cc: dev, Sunil Kumar Kori, Harman Kalra

Currently otx2_mbox_get_rsp_xxx get response once AF driver
interrupts after completion. But this funciton 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>
---
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 | 58 +++++++++++++++++++++++-----
 drivers/common/octeontx2/otx2_mbox.h |  5 ++-
 3 files changed, 73 insertions(+), 31 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..358e4b3bd 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;
 
@@ -230,8 +232,8 @@ otx2_mbox_get_rsp(struct otx2_mbox *mbox, int devid, void **msg)
 	int rc;
 
 	rc = otx2_mbox_wait_for_rsp(mbox, devid);
-	if (rc != 1)
-		return -EIO;
+	if (rc < 0)
+		return rc;
 
 	rte_rmb();
 
@@ -244,6 +246,38 @@ 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 {
+		rte_smp_rmb();
+		rsp_reg = otx2_read64(reg_addr);
+
+		if (timeout >= wait_us)
+			return -ETIMEDOUT;
+
+		rte_delay_us(sleep);
+		timeout += sleep;
+	} while (!rsp_reg);
+
+	/* 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
@@ -258,8 +292,8 @@ otx2_mbox_get_rsp_tmo(struct otx2_mbox *mbox, int devid, void **msg,
 	int rc;
 
 	rc = otx2_mbox_wait_for_rsp_tmo(mbox, devid, tmo);
-	if (rc != 1)
-		return -EIO;
+	if (rc < 0)
+		return rc;
 
 	rte_rmb();
 
@@ -321,11 +355,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


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

* Re: [dpdk-dev] [PATCH v3 2/2] common/octeontx2: add polling based response mbox message
  2019-12-17 11:14           ` Jerin Jacob
@ 2019-12-18  2:45             ` Gavin Hu
  0 siblings, 0 replies; 38+ messages in thread
From: Gavin Hu @ 2019-12-18  2:45 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: Sunil Kumar Kori, jerinj, Nithin Dabilpuram, Vamsi Attunuru, dev,
	Harman Kalra, Honnappa Nagarahalli, nd, nd

Hi Jerin,

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Tuesday, December 17, 2019 7:14 PM
> To: Gavin Hu <Gavin.Hu@arm.com>
> Cc: Sunil Kumar Kori <skori@marvell.com>; jerinj@marvell.com; Nithin
> Dabilpuram <ndabilpuram@marvell.com>; Vamsi Attunuru
> <vattunuru@marvell.com>; dev@dpdk.org; Harman Kalra
> <hkalra@marvell.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v3 2/2] common/octeontx2: add polling
> based response mbox message
> 
> > > +mbox_poll(struct otx2_mbox *mbox, uint32_t wait)
> > > +{
> > > +     uint32_t timeout = 0, sleep = 1;
> > > +     uint64_t rsp_reg = 0;
> > > +     uintptr_t reg_addr;
> > > +
> > > +     reg_addr = mbox->reg_base + mbox->intr_offset;
> > > +     while (!rsp_reg) {
> > The first iteration of (!rsp_reg) always evaluate to 'true'.
> > Why not use do .. while to save one iteration?
> > > +             rte_rmb();
> > Rte_rmb is overkill, how is reg_addr mapped(what is the memory attribute?
> WC? cacheable? )
> > If it is a PCI mmaped region, then rte_io_barrier is okay, I am proposing to
> relax the io barrier for aarch64 as a compiler barrier.
> >  http://patches.dpdk.org/patch/61662/
> 
> It is the device memory and Linux kernel(_CPU_ not IO) is updating the
> memory on the other side. I think,
> we would need rte_smp_rmb() here. Since it was slow-path code, added
> the full barrier here.
> I think it is OK to change rte_smp_rmb() not to rte_io_* version. Correct?
I want to summarize as follows:
rte_io_* should be used in this case as it is the device memory.
rte_smp_* is for normal memory, including DMA buffer resides in the normal memory and updated by devices.
Rte_cio_* should be used for ordering between normal memory and device memory, for example, updates to descriptors in the normal memory and a doorbell write. 

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

* Re: [dpdk-dev] [PATCH v4 2/2] common/octeontx2: add polling based response mbox message
  2019-12-17 16:53           ` [dpdk-dev] [PATCH v4 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
@ 2019-12-18  2:54             ` Gavin Hu
  0 siblings, 0 replies; 38+ messages in thread
From: Gavin Hu @ 2019-12-18  2:54 UTC (permalink / raw)
  To: Sunil Kumar Kori, jerinj, Nithin Dabilpuram, Vamsi Attunuru
  Cc: dev, Harman Kalra, nd

Hi Sunil,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Sunil Kumar Kori
> Sent: Wednesday, December 18, 2019 12:53 AM
> To: jerinj@marvell.com; Nithin Dabilpuram <ndabilpuram@marvell.com>;
> Vamsi Attunuru <vattunuru@marvell.com>
> Cc: dev@dpdk.org; Sunil Kumar Kori <skori@marvell.com>; Harman Kalra
> <hkalra@marvell.com>
> Subject: [dpdk-dev] [PATCH v4 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 funciton 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>
> ---
> 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 | 58 +++++++++++++++++++++++---
> --
>  drivers/common/octeontx2/otx2_mbox.h |  5 ++-
>  3 files changed, 73 insertions(+), 31 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..358e4b3bd 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;
> 
> @@ -230,8 +232,8 @@ otx2_mbox_get_rsp(struct otx2_mbox *mbox, int
> devid, void **msg)
>  	int rc;
> 
>  	rc = otx2_mbox_wait_for_rsp(mbox, devid);
> -	if (rc != 1)
> -		return -EIO;
> +	if (rc < 0)
> +		return rc;
> 
>  	rte_rmb();
> 
> @@ -244,6 +246,38 @@ 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 {
> +		rte_smp_rmb();
Device memory is not within the inner sharable domain, here rte_io_rmb should be used.
More details are explained in the v3 thread.
> +		rsp_reg = otx2_read64(reg_addr);
> +
> +		if (timeout >= wait_us)
> +			return -ETIMEDOUT;
> +
> +		rte_delay_us(sleep);
> +		timeout += sleep;
> +	} while (!rsp_reg);
> +
> +	/* 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
> @@ -258,8 +292,8 @@ otx2_mbox_get_rsp_tmo(struct otx2_mbox *mbox,
> int devid, void **msg,
>  	int rc;
> 
>  	rc = otx2_mbox_wait_for_rsp_tmo(mbox, devid, tmo);
> -	if (rc != 1)
> -		return -EIO;
> +	if (rc < 0)
> +		return rc;
> 
>  	rte_rmb();
> 
> @@ -321,11 +355,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


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

* [dpdk-dev] [PATCH v5 1/2] eal: add API to check if its interrupt context
  2019-12-17 16:53         ` [dpdk-dev] [PATCH v4 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
  2019-12-17 16:53           ` [dpdk-dev] [PATCH v4 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
@ 2019-12-18  7:07           ` Sunil Kumar Kori
  2019-12-18  7:07             ` [dpdk-dev] [PATCH v5 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
  1 sibling, 1 reply; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-12-18  7:07 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, Harman Kalra, Sunil Kumar Kori

From: Harman Kalra <hkalra@marvell.com>

Added an API to check if current execution is in interrupt
context. This will be helpful to handle nested interrupt cases.

Signed-off-by: Harman Kalra <hkalra@marvell.com>
Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
v5:
 - Fix shared library compilation error
v4:
 - No changes.
v3:
 - API Comment is updated as per man page.
 - Scope updated within the library/driver only.
 - Remove experimental tag
v2:
 - Rebased patch on 19.11-rc4

 lib/librte_eal/common/include/rte_eal_interrupts.h | 11 +++++++++++
 lib/librte_eal/freebsd/eal/eal_interrupts.c        |  5 +++++
 lib/librte_eal/linux/eal/eal_interrupts.c          |  5 +++++
 lib/librte_eal/rte_eal_version.map                 |  1 +
 4 files changed, 22 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_eal_interrupts.h b/lib/librte_eal/common/include/rte_eal_interrupts.h
index b370c0d26..19d1d45ab 100644
--- a/lib/librte_eal/common/include/rte_eal_interrupts.h
+++ b/lib/librte_eal/common/include/rte_eal_interrupts.h
@@ -220,4 +220,15 @@ rte_intr_allow_others(struct rte_intr_handle *intr_handle);
 int
 rte_intr_cap_multiple(struct rte_intr_handle *intr_handle);
 
+/**
+ * @internal
+ * Check if currently executing in interrupt context
+ *
+ * @return
+ *  - non zero in case of interrupt context
+ *  - zero in case of process context
+ */
+int
+rte_thread_is_intr(void);
+
 #endif /* _RTE_EAL_INTERRUPTS_H_ */
diff --git a/lib/librte_eal/freebsd/eal/eal_interrupts.c b/lib/librte_eal/freebsd/eal/eal_interrupts.c
index f6831b790..ce2a27b4a 100644
--- a/lib/librte_eal/freebsd/eal/eal_interrupts.c
+++ b/lib/librte_eal/freebsd/eal/eal_interrupts.c
@@ -671,3 +671,8 @@ rte_intr_free_epoll_fd(struct rte_intr_handle *intr_handle)
 {
 	RTE_SET_USED(intr_handle);
 }
+
+int rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c b/lib/librte_eal/linux/eal/eal_interrupts.c
index 14ebb108c..cb8e10709 100644
--- a/lib/librte_eal/linux/eal/eal_interrupts.c
+++ b/lib/librte_eal/linux/eal/eal_interrupts.c
@@ -1488,3 +1488,8 @@ rte_intr_cap_multiple(struct rte_intr_handle *intr_handle)
 
 	return 0;
 }
+
+int rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index e38d02530..06ed2e9dc 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -91,6 +91,7 @@ DPDK_20.0 {
 	rte_intr_free_epoll_fd;
 	rte_intr_rx_ctl;
 	rte_intr_tls_epfd;
+	rte_thread_is_intr;
 	rte_keepalive_create;
 	rte_keepalive_dispatch_pings;
 	rte_keepalive_mark_alive;
-- 
2.17.1


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

* [dpdk-dev] [PATCH v5 2/2] common/octeontx2: add polling based response mbox message
  2019-12-18  7:07           ` [dpdk-dev] [PATCH v5 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
@ 2019-12-18  7:07             ` Sunil Kumar Kori
  2019-12-20  6:56               ` [dpdk-dev] [PATCH v6 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
  0 siblings, 1 reply; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-12-18  7:07 UTC (permalink / raw)
  To: Jerin Jacob, Nithin Dabilpuram, Vamsi Attunuru
  Cc: dev, Sunil Kumar Kori, Harman Kalra

Currently otx2_mbox_get_rsp_xxx get response once AF driver
interrupts after completion. But this funciton 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>
---
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 | 58 +++++++++++++++++++++++-----
 drivers/common/octeontx2/otx2_mbox.h |  5 ++-
 3 files changed, 73 insertions(+), 31 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..358e4b3bd 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;
 
@@ -230,8 +232,8 @@ otx2_mbox_get_rsp(struct otx2_mbox *mbox, int devid, void **msg)
 	int rc;
 
 	rc = otx2_mbox_wait_for_rsp(mbox, devid);
-	if (rc != 1)
-		return -EIO;
+	if (rc < 0)
+		return rc;
 
 	rte_rmb();
 
@@ -244,6 +246,38 @@ 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 {
+		rte_smp_rmb();
+		rsp_reg = otx2_read64(reg_addr);
+
+		if (timeout >= wait_us)
+			return -ETIMEDOUT;
+
+		rte_delay_us(sleep);
+		timeout += sleep;
+	} while (!rsp_reg);
+
+	/* 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
@@ -258,8 +292,8 @@ otx2_mbox_get_rsp_tmo(struct otx2_mbox *mbox, int devid, void **msg,
 	int rc;
 
 	rc = otx2_mbox_wait_for_rsp_tmo(mbox, devid, tmo);
-	if (rc != 1)
-		return -EIO;
+	if (rc < 0)
+		return rc;
 
 	rte_rmb();
 
@@ -321,11 +355,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


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

* [dpdk-dev] [PATCH v6 1/2] eal: add API to check if its interrupt context
  2019-12-18  7:07             ` [dpdk-dev] [PATCH v5 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
@ 2019-12-20  6:56               ` Sunil Kumar Kori
  2019-12-20  6:56                 ` [dpdk-dev] [PATCH v6 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
  2020-01-14  8:37                 ` [dpdk-dev] [PATCH v6 " Jerin Jacob
  0 siblings, 2 replies; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-12-20  6:56 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, Harman Kalra, Sunil Kumar Kori

From: Harman Kalra <hkalra@marvell.com>

Added an API to check if current execution is in interrupt
context. This will be helpful to handle nested interrupt cases.

Signed-off-by: Harman Kalra <hkalra@marvell.com>
Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
---
v6:
 - No changes.
v5:
 - Fix shared library compilation error
v4:
 - No changes.
v3:
 - API Comment is updated as per man page.
 - Scope updated within the library/driver only.
 - Remove experimental tag
v2:
 - Rebased patch on 19.11-rc4

 lib/librte_eal/common/include/rte_eal_interrupts.h | 11 +++++++++++
 lib/librte_eal/freebsd/eal/eal_interrupts.c        |  5 +++++
 lib/librte_eal/linux/eal/eal_interrupts.c          |  5 +++++
 lib/librte_eal/rte_eal_version.map                 |  1 +
 4 files changed, 22 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_eal_interrupts.h b/lib/librte_eal/common/include/rte_eal_interrupts.h
index b370c0d26..19d1d45ab 100644
--- a/lib/librte_eal/common/include/rte_eal_interrupts.h
+++ b/lib/librte_eal/common/include/rte_eal_interrupts.h
@@ -220,4 +220,15 @@ rte_intr_allow_others(struct rte_intr_handle *intr_handle);
 int
 rte_intr_cap_multiple(struct rte_intr_handle *intr_handle);
 
+/**
+ * @internal
+ * Check if currently executing in interrupt context
+ *
+ * @return
+ *  - non zero in case of interrupt context
+ *  - zero in case of process context
+ */
+int
+rte_thread_is_intr(void);
+
 #endif /* _RTE_EAL_INTERRUPTS_H_ */
diff --git a/lib/librte_eal/freebsd/eal/eal_interrupts.c b/lib/librte_eal/freebsd/eal/eal_interrupts.c
index f6831b790..ce2a27b4a 100644
--- a/lib/librte_eal/freebsd/eal/eal_interrupts.c
+++ b/lib/librte_eal/freebsd/eal/eal_interrupts.c
@@ -671,3 +671,8 @@ rte_intr_free_epoll_fd(struct rte_intr_handle *intr_handle)
 {
 	RTE_SET_USED(intr_handle);
 }
+
+int rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c b/lib/librte_eal/linux/eal/eal_interrupts.c
index 14ebb108c..cb8e10709 100644
--- a/lib/librte_eal/linux/eal/eal_interrupts.c
+++ b/lib/librte_eal/linux/eal/eal_interrupts.c
@@ -1488,3 +1488,8 @@ rte_intr_cap_multiple(struct rte_intr_handle *intr_handle)
 
 	return 0;
 }
+
+int rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index e38d02530..06ed2e9dc 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -91,6 +91,7 @@ DPDK_20.0 {
 	rte_intr_free_epoll_fd;
 	rte_intr_rx_ctl;
 	rte_intr_tls_epfd;
+	rte_thread_is_intr;
 	rte_keepalive_create;
 	rte_keepalive_dispatch_pings;
 	rte_keepalive_mark_alive;
-- 
2.17.1


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

* [dpdk-dev] [PATCH v6 2/2] common/octeontx2: add polling based response mbox message
  2019-12-20  6:56               ` [dpdk-dev] [PATCH v6 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
@ 2019-12-20  6:56                 ` Sunil Kumar Kori
  2020-01-14  8:41                   ` Jerin Jacob
  2020-01-14  9:04                   ` [dpdk-dev] [PATCH v7 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
  2020-01-14  8:37                 ` [dpdk-dev] [PATCH v6 " Jerin Jacob
  1 sibling, 2 replies; 38+ messages in thread
From: Sunil Kumar Kori @ 2019-12-20  6:56 UTC (permalink / raw)
  To: Jerin Jacob, Nithin Dabilpuram, Vamsi Attunuru
  Cc: dev, Sunil Kumar Kori, Harman Kalra

Currently otx2_mbox_get_rsp_xxx get response once AF driver
interrupts after completion. But this funciton 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>
---
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


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

* Re: [dpdk-dev] [PATCH v6 1/2] eal: add API to check if its interrupt context
  2019-12-20  6:56               ` [dpdk-dev] [PATCH v6 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
  2019-12-20  6:56                 ` [dpdk-dev] [PATCH v6 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
@ 2020-01-14  8:37                 ` Jerin Jacob
  1 sibling, 0 replies; 38+ messages in thread
From: Jerin Jacob @ 2020-01-14  8:37 UTC (permalink / raw)
  To: Sunil Kumar Kori, Thomas Monjalon, David Marchand
  Cc: Bruce Richardson, dpdk-dev, Harman Kalra

On Fri, Dec 20, 2019 at 12:26 PM Sunil Kumar Kori <skori@marvell.com> wrote:
>
> From: Harman Kalra <hkalra@marvell.com>
>
> Added an API to check if current execution is in interrupt
> context. This will be helpful to handle nested interrupt cases.
>
> Signed-off-by: Harman Kalra <hkalra@marvell.com>
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>

Reviewed-by: Jerin Jacob <jerinj@marvell.com>



> ---
> v6:
>  - No changes.
> v5:
>  - Fix shared library compilation error
> v4:
>  - No changes.
> v3:
>  - API Comment is updated as per man page.
>  - Scope updated within the library/driver only.
>  - Remove experimental tag
> v2:
>  - Rebased patch on 19.11-rc4
>
>  lib/librte_eal/common/include/rte_eal_interrupts.h | 11 +++++++++++
>  lib/librte_eal/freebsd/eal/eal_interrupts.c        |  5 +++++
>  lib/librte_eal/linux/eal/eal_interrupts.c          |  5 +++++
>  lib/librte_eal/rte_eal_version.map                 |  1 +
>  4 files changed, 22 insertions(+)
>
> diff --git a/lib/librte_eal/common/include/rte_eal_interrupts.h b/lib/librte_eal/common/include/rte_eal_interrupts.h
> index b370c0d26..19d1d45ab 100644
> --- a/lib/librte_eal/common/include/rte_eal_interrupts.h
> +++ b/lib/librte_eal/common/include/rte_eal_interrupts.h
> @@ -220,4 +220,15 @@ rte_intr_allow_others(struct rte_intr_handle *intr_handle);
>  int
>  rte_intr_cap_multiple(struct rte_intr_handle *intr_handle);
>
> +/**
> + * @internal
> + * Check if currently executing in interrupt context
> + *
> + * @return
> + *  - non zero in case of interrupt context
> + *  - zero in case of process context
> + */
> +int
> +rte_thread_is_intr(void);
> +
>  #endif /* _RTE_EAL_INTERRUPTS_H_ */
> diff --git a/lib/librte_eal/freebsd/eal/eal_interrupts.c b/lib/librte_eal/freebsd/eal/eal_interrupts.c
> index f6831b790..ce2a27b4a 100644
> --- a/lib/librte_eal/freebsd/eal/eal_interrupts.c
> +++ b/lib/librte_eal/freebsd/eal/eal_interrupts.c
> @@ -671,3 +671,8 @@ rte_intr_free_epoll_fd(struct rte_intr_handle *intr_handle)
>  {
>         RTE_SET_USED(intr_handle);
>  }
> +
> +int rte_thread_is_intr(void)
> +{
> +       return pthread_equal(intr_thread, pthread_self());
> +}
> diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c b/lib/librte_eal/linux/eal/eal_interrupts.c
> index 14ebb108c..cb8e10709 100644
> --- a/lib/librte_eal/linux/eal/eal_interrupts.c
> +++ b/lib/librte_eal/linux/eal/eal_interrupts.c
> @@ -1488,3 +1488,8 @@ rte_intr_cap_multiple(struct rte_intr_handle *intr_handle)
>
>         return 0;
>  }
> +
> +int rte_thread_is_intr(void)
> +{
> +       return pthread_equal(intr_thread, pthread_self());
> +}
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index e38d02530..06ed2e9dc 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -91,6 +91,7 @@ DPDK_20.0 {
>         rte_intr_free_epoll_fd;
>         rte_intr_rx_ctl;
>         rte_intr_tls_epfd;
> +       rte_thread_is_intr;
>         rte_keepalive_create;
>         rte_keepalive_dispatch_pings;
>         rte_keepalive_mark_alive;
> --
> 2.17.1
>

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

* Re: [dpdk-dev] [PATCH v6 2/2] common/octeontx2: add polling based response mbox message
  2019-12-20  6:56                 ` [dpdk-dev] [PATCH v6 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
@ 2020-01-14  8:41                   ` Jerin Jacob
  2020-01-14 10:17                     ` Thomas Monjalon
  2020-01-14  9:04                   ` [dpdk-dev] [PATCH v7 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
  1 sibling, 1 reply; 38+ messages in thread
From: Jerin Jacob @ 2020-01-14  8:41 UTC (permalink / raw)
  To: Sunil Kumar Kori, Thomas Monjalon, David Marchand
  Cc: Jerin Jacob, Nithin Dabilpuram, Vamsi Attunuru, dpdk-dev, Harman Kalra

On Fri, Dec 20, 2019 at 12:27 PM Sunil Kumar Kori <skori@marvell.com> wrote:
>
> Currently otx2_mbox_get_rsp_xxx get response once AF driver
> interrupts after completion. But this funciton will get into

s/funciton/function

> 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>

With the above change:
Acked-by: Jerin Jacob <jerinj@marvell.com>

@Thomas Monjalon  Since this patch has a dependency on an eal
patch(1/2  eal: add API to check if its interrupt context), I am
delegating this patch to EAL maintainer.



> ---
> 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
>

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

* [dpdk-dev] [PATCH v7 1/2] eal: add API to check if its interrupt context
  2019-12-20  6:56                 ` [dpdk-dev] [PATCH v6 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
  2020-01-14  8:41                   ` Jerin Jacob
@ 2020-01-14  9:04                   ` Sunil Kumar Kori
  2020-01-14  9:04                     ` [dpdk-dev] [PATCH v7 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
  2020-02-06 15:35                     ` [dpdk-dev] [PATCH v7 1/2] eal: add API to check if its interrupt context David Marchand
  1 sibling, 2 replies; 38+ messages in thread
From: Sunil Kumar Kori @ 2020-01-14  9:04 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev, Harman Kalra, Sunil Kumar Kori

From: Harman Kalra <hkalra@marvell.com>

Added an API to check if current execution is in interrupt
context. This will be helpful to handle nested interrupt cases.

Signed-off-by: Harman Kalra <hkalra@marvell.com>
Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
Reviewed-by: Jerin Jacob <jerinj@marvell.com>
---
v7:
 - No changes.
v6:
 - No changes.
v5:
 - Fix shared library compilation error
v4:
 - No changes.
v3:
 - API Comment is updated as per man page.
 - Scope updated within the library/driver only.
 - Remove experimental tag
v2:
 - Rebased patch on 19.11-rc4

 lib/librte_eal/common/include/rte_eal_interrupts.h | 11 +++++++++++
 lib/librte_eal/freebsd/eal/eal_interrupts.c        |  5 +++++
 lib/librte_eal/linux/eal/eal_interrupts.c          |  5 +++++
 lib/librte_eal/rte_eal_version.map                 |  1 +
 4 files changed, 22 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_eal_interrupts.h b/lib/librte_eal/common/include/rte_eal_interrupts.h
index b370c0d26..19d1d45ab 100644
--- a/lib/librte_eal/common/include/rte_eal_interrupts.h
+++ b/lib/librte_eal/common/include/rte_eal_interrupts.h
@@ -220,4 +220,15 @@ rte_intr_allow_others(struct rte_intr_handle *intr_handle);
 int
 rte_intr_cap_multiple(struct rte_intr_handle *intr_handle);
 
+/**
+ * @internal
+ * Check if currently executing in interrupt context
+ *
+ * @return
+ *  - non zero in case of interrupt context
+ *  - zero in case of process context
+ */
+int
+rte_thread_is_intr(void);
+
 #endif /* _RTE_EAL_INTERRUPTS_H_ */
diff --git a/lib/librte_eal/freebsd/eal/eal_interrupts.c b/lib/librte_eal/freebsd/eal/eal_interrupts.c
index f6831b790..ce2a27b4a 100644
--- a/lib/librte_eal/freebsd/eal/eal_interrupts.c
+++ b/lib/librte_eal/freebsd/eal/eal_interrupts.c
@@ -671,3 +671,8 @@ rte_intr_free_epoll_fd(struct rte_intr_handle *intr_handle)
 {
 	RTE_SET_USED(intr_handle);
 }
+
+int rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c b/lib/librte_eal/linux/eal/eal_interrupts.c
index 14ebb108c..cb8e10709 100644
--- a/lib/librte_eal/linux/eal/eal_interrupts.c
+++ b/lib/librte_eal/linux/eal/eal_interrupts.c
@@ -1488,3 +1488,8 @@ rte_intr_cap_multiple(struct rte_intr_handle *intr_handle)
 
 	return 0;
 }
+
+int rte_thread_is_intr(void)
+{
+	return pthread_equal(intr_thread, pthread_self());
+}
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index e38d02530..06ed2e9dc 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -91,6 +91,7 @@ DPDK_20.0 {
 	rte_intr_free_epoll_fd;
 	rte_intr_rx_ctl;
 	rte_intr_tls_epfd;
+	rte_thread_is_intr;
 	rte_keepalive_create;
 	rte_keepalive_dispatch_pings;
 	rte_keepalive_mark_alive;
-- 
2.17.1


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

* [dpdk-dev] [PATCH v7 2/2] common/octeontx2: add polling based response mbox message
  2020-01-14  9:04                   ` [dpdk-dev] [PATCH v7 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
@ 2020-01-14  9:04                     ` Sunil Kumar Kori
  2020-01-21  8:37                       ` Sunil Kumar Kori
  2020-02-06 15:35                     ` [dpdk-dev] [PATCH v7 1/2] eal: add API to check if its interrupt context David Marchand
  1 sibling, 1 reply; 38+ messages in thread
From: Sunil Kumar Kori @ 2020-01-14  9:04 UTC (permalink / raw)
  To: Jerin Jacob, Nithin Dabilpuram, Vamsi Attunuru
  Cc: dev, Sunil Kumar Kori, Harman Kalra

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


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

* Re: [dpdk-dev] [PATCH v6 2/2] common/octeontx2: add polling based response mbox message
  2020-01-14  8:41                   ` Jerin Jacob
@ 2020-01-14 10:17                     ` Thomas Monjalon
  0 siblings, 0 replies; 38+ messages in thread
From: Thomas Monjalon @ 2020-01-14 10:17 UTC (permalink / raw)
  To: Jerin Jacob
  Cc: Sunil Kumar Kori, David Marchand, Jerin Jacob, Nithin Dabilpuram,
	Vamsi Attunuru, dpdk-dev, Harman Kalra

14/01/2020 09:41, Jerin Jacob:
> On Fri, Dec 20, 2019 at 12:27 PM Sunil Kumar Kori <skori@marvell.com> wrote:
> >
> > Currently otx2_mbox_get_rsp_xxx get response once AF driver
> > interrupts after completion. But this funciton will get into
> 
> s/funciton/function
> 
> > 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>
> 
> With the above change:
> Acked-by: Jerin Jacob <jerinj@marvell.com>
> 
> @Thomas Monjalon  Since this patch has a dependency on an eal
> patch(1/2  eal: add API to check if its interrupt context), I am
> delegating this patch to EAL maintainer.

Yes, we don't split series.
So if there is an EAL change in a series, the whole series
must be merged in master.





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

* Re: [dpdk-dev] [PATCH v7 2/2] common/octeontx2: add polling based response mbox message
  2020-01-14  9:04                     ` [dpdk-dev] [PATCH v7 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
@ 2020-01-21  8:37                       ` Sunil Kumar Kori
  0 siblings, 0 replies; 38+ messages in thread
From: Sunil Kumar Kori @ 2020-01-21  8:37 UTC (permalink / raw)
  To: Sunil Kumar Kori, Jerin Jacob Kollanukkaran,
	Nithin Kumar Dabilpuram, Vamsi Krishna Attunuru
  Cc: dev, Harman Kalra

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


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

* Re: [dpdk-dev] [PATCH v7 1/2] eal: add API to check if its interrupt context
  2020-01-14  9:04                   ` [dpdk-dev] [PATCH v7 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
  2020-01-14  9:04                     ` [dpdk-dev] [PATCH v7 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
@ 2020-02-06 15:35                     ` David Marchand
  1 sibling, 0 replies; 38+ messages in thread
From: David Marchand @ 2020-02-06 15:35 UTC (permalink / raw)
  To: Sunil Kumar Kori; +Cc: Bruce Richardson, dev, Harman Kalra

On Tue, Jan 14, 2020 at 10:05 AM Sunil Kumar Kori <skori@marvell.com> wrote:
>
> From: Harman Kalra <hkalra@marvell.com>
>
> Added an API to check if current execution is in interrupt
> context. This will be helpful to handle nested interrupt cases.
>
> Signed-off-by: Harman Kalra <hkalra@marvell.com>
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> Reviewed-by: Jerin Jacob <jerinj@marvell.com>
> ---
> v7:
>  - No changes.
> v6:
>  - No changes.
> v5:
>  - Fix shared library compilation error
> v4:
>  - No changes.
> v3:
>  - API Comment is updated as per man page.
>  - Scope updated within the library/driver only.
>  - Remove experimental tag
> v2:
>  - Rebased patch on 19.11-rc4
>
>  lib/librte_eal/common/include/rte_eal_interrupts.h | 11 +++++++++++
>  lib/librte_eal/freebsd/eal/eal_interrupts.c        |  5 +++++
>  lib/librte_eal/linux/eal/eal_interrupts.c          |  5 +++++
>  lib/librte_eal/rte_eal_version.map                 |  1 +
>  4 files changed, 22 insertions(+)
>
> diff --git a/lib/librte_eal/common/include/rte_eal_interrupts.h b/lib/librte_eal/common/include/rte_eal_interrupts.h
> index b370c0d26..19d1d45ab 100644
> --- a/lib/librte_eal/common/include/rte_eal_interrupts.h
> +++ b/lib/librte_eal/common/include/rte_eal_interrupts.h
> @@ -220,4 +220,15 @@ rte_intr_allow_others(struct rte_intr_handle *intr_handle);
>  int
>  rte_intr_cap_multiple(struct rte_intr_handle *intr_handle);
>
> +/**
> + * @internal
> + * Check if currently executing in interrupt context
> + *
> + * @return
> + *  - non zero in case of interrupt context
> + *  - zero in case of process context
> + */
> +int
> +rte_thread_is_intr(void);
> +
>  #endif /* _RTE_EAL_INTERRUPTS_H_ */
> diff --git a/lib/librte_eal/freebsd/eal/eal_interrupts.c b/lib/librte_eal/freebsd/eal/eal_interrupts.c
> index f6831b790..ce2a27b4a 100644
> --- a/lib/librte_eal/freebsd/eal/eal_interrupts.c
> +++ b/lib/librte_eal/freebsd/eal/eal_interrupts.c
> @@ -671,3 +671,8 @@ rte_intr_free_epoll_fd(struct rte_intr_handle *intr_handle)
>  {
>         RTE_SET_USED(intr_handle);
>  }
> +
> +int rte_thread_is_intr(void)
> +{
> +       return pthread_equal(intr_thread, pthread_self());
> +}
> diff --git a/lib/librte_eal/linux/eal/eal_interrupts.c b/lib/librte_eal/linux/eal/eal_interrupts.c
> index 14ebb108c..cb8e10709 100644
> --- a/lib/librte_eal/linux/eal/eal_interrupts.c
> +++ b/lib/librte_eal/linux/eal/eal_interrupts.c
> @@ -1488,3 +1488,8 @@ rte_intr_cap_multiple(struct rte_intr_handle *intr_handle)
>
>         return 0;
>  }
> +
> +int rte_thread_is_intr(void)
> +{
> +       return pthread_equal(intr_thread, pthread_self());
> +}
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index e38d02530..06ed2e9dc 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -91,6 +91,7 @@ DPDK_20.0 {
>         rte_intr_free_epoll_fd;
>         rte_intr_rx_ctl;
>         rte_intr_tls_epfd;
> +       rte_thread_is_intr;
>         rte_keepalive_create;
>         rte_keepalive_dispatch_pings;
>         rte_keepalive_mark_alive;
> --
> 2.17.1


We can't put this symbol in the stable ABI and just flag it as
internal in the function description.
Users won't notice this warning.

I moved this as an experimental API and enabled the use of
experimental api in octeontx2 common driver.
We can revisit once we have the rte_internal tag Neil had proposed.


Series applied, thanks.



--
David Marchand


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

end of thread, other threads:[~2020-02-06 15:35 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-25 11:35 [dpdk-dev] [PATCH 1/5] drivers/octeontx2: allow experimental symbols Sunil Kumar Kori
2019-11-25 11:35 ` [dpdk-dev] [PATCH v3 2/5] eal: add API to check if its interrupt context Sunil Kumar Kori
2019-11-25 11:35 ` [dpdk-dev] [PATCH 3/5] common/octeontx2: add interrupt offset to mbox structure Sunil Kumar Kori
2019-11-25 11:35 ` [dpdk-dev] [PATCH 4/5] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
2019-11-25 11:35 ` [dpdk-dev] [PATCH 5/5] common/octeontx2: enhancing mbox APIs to get response Sunil Kumar Kori
2019-11-26  6:15 ` [dpdk-dev] [PATCH v2 1/5] drivers/octeontx2: allow experimental symbols Sunil Kumar Kori
2019-11-26 16:21   ` Thomas Monjalon
2019-11-27  8:34     ` [dpdk-dev] [EXT] " Sunil Kumar Kori
2019-11-27  9:42       ` Thomas Monjalon
2019-11-27 10:22   ` [dpdk-dev] [PATCH v2 1/4] eal: add API to check if its interrupt context Sunil Kumar Kori
2019-11-27 10:22     ` [dpdk-dev] [PATCH v2 2/4] common/octeontx2: add interrupt offset to mbox structure Sunil Kumar Kori
2019-11-27 10:22     ` [dpdk-dev] [PATCH v2 3/4] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
2019-11-27 10:22     ` [dpdk-dev] [PATCH v2 4/4] drivers/octeontx2: enhancing mbox APIs to get response Sunil Kumar Kori
2019-11-28  0:03     ` [dpdk-dev] [PATCH v2 1/4] eal: add API to check if its interrupt context Stephen Hemminger
2019-11-28  8:10       ` [dpdk-dev] [EXT] " Harman Kalra
2019-11-28 16:45         ` Stephen Hemminger
2019-12-04 16:23           ` Harman Kalra
2019-12-16 10:39     ` [dpdk-dev] [PATCH v3 1/2] " Sunil Kumar Kori
2019-12-16 10:39       ` [dpdk-dev] [PATCH v3 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
2019-12-17  8:02         ` Gavin Hu (Arm Technology China)
2019-12-17 11:14           ` Jerin Jacob
2019-12-18  2:45             ` Gavin Hu
2019-12-17 10:42         ` [dpdk-dev] [PATCH v4 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
2019-12-17 10:42           ` [dpdk-dev] [PATCH v4 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
2019-12-17 16:53         ` [dpdk-dev] [PATCH v4 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
2019-12-17 16:53           ` [dpdk-dev] [PATCH v4 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
2019-12-18  2:54             ` Gavin Hu
2019-12-18  7:07           ` [dpdk-dev] [PATCH v5 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
2019-12-18  7:07             ` [dpdk-dev] [PATCH v5 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
2019-12-20  6:56               ` [dpdk-dev] [PATCH v6 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
2019-12-20  6:56                 ` [dpdk-dev] [PATCH v6 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
2020-01-14  8:41                   ` Jerin Jacob
2020-01-14 10:17                     ` Thomas Monjalon
2020-01-14  9:04                   ` [dpdk-dev] [PATCH v7 1/2] eal: add API to check if its interrupt context Sunil Kumar Kori
2020-01-14  9:04                     ` [dpdk-dev] [PATCH v7 2/2] common/octeontx2: add polling based response mbox message Sunil Kumar Kori
2020-01-21  8:37                       ` Sunil Kumar Kori
2020-02-06 15:35                     ` [dpdk-dev] [PATCH v7 1/2] eal: add API to check if its interrupt context David Marchand
2020-01-14  8:37                 ` [dpdk-dev] [PATCH v6 " Jerin Jacob

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).