[PATCH v2 1/2] common/cnxk: fix mailbox timeout issues

[Harman Kalra <hkalra@marvell.com>]

Found couple of reasons causing mailbox timeout:
* msgs_acked value by interrupt thread was not getting synced to thread
polling it.
* mbox_data value sent by AF was seen 0 causing neither down nor up msg
processing.

Fixes: 9bd368ca311a ("common/cnxk: enable PF VF mailbox")
Fixes: 61deac72abbf ("common/cnxk: add CN20KA mbox support")

Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
 V2:
 * Replaced gcc builtin __atomic_xxx intrinsics with corresponding
 rte_atomic_xxx stdatomic API

 drivers/common/cnxk/roc_dev.c      | 106 ++++++++++++++++++++++++-----
 drivers/common/cnxk/roc_dev_priv.h |  11 +++
 drivers/common/cnxk/roc_mbox.c     |  12 ++--
 drivers/common/cnxk/roc_platform.h |   5 ++
 4 files changed, 111 insertions(+), 23 deletions(-)

diff --git a/drivers/common/cnxk/roc_dev.c b/drivers/common/cnxk/roc_dev.c
index 32409f2ef3..0d1f9c4564 100644
--- a/drivers/common/cnxk/roc_dev.c
+++ b/drivers/common/cnxk/roc_dev.c
@@ -401,8 +401,7 @@ vf_pf_process_up_msgs(struct dev *dev, uint16_t vf)
 		offset = mbox->rx_start + msg->next_msgoff;
 	}
 	mbox_reset(mbox, vf);
-	mdev->msgs_acked = msgs_acked;
-	plt_wmb();
+	plt_atomic_store_explicit(&mdev->msgs_acked, msgs_acked, plt_memory_order_release);
 
 	return i;
 }
@@ -542,8 +541,7 @@ process_msgs(struct dev *dev, struct mbox *mbox)
 
 	mbox_reset(mbox, 0);
 	/* Update acked if someone is waiting a message - mbox_wait is waiting */
-	mdev->msgs_acked = msgs_acked;
-	plt_wmb();
+	plt_atomic_store_explicit(&mdev->msgs_acked, msgs_acked, plt_memory_order_release);
 }
 
 /* Copies the message received from AF and sends it to VF */
@@ -920,12 +918,59 @@ process_msgs_up(struct dev *dev, struct mbox *mbox)
 	}
 }
 
+/* IRQ to VF from PF - VF context (interrupt thread) */
+static void
+roc_pf_vf_mbox_irq_cn20k(void *param)
+{
+	struct dev *dev = param;
+	uint64_t intr;
+
+	intr = plt_read64(dev->mbox_reg_base + RVU_VF_INT);
+	if (intr == 0)
+		plt_base_dbg("Proceeding to check mbox UP messages if any");
+
+	plt_write64(intr, dev->mbox_reg_base + RVU_VF_INT);
+	plt_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev->vf);
+
+	/* If interrupt occurred for down message */
+	if (intr & BIT_ULL(1))
+		/* First process all configuration messages */
+		process_msgs(dev, dev->mbox);
+
+	/* If interrupt occurred for UP message */
+	if (intr & BIT_ULL(0))
+		process_msgs_up(dev, &dev->mbox_up);
+}
+
+/* IRQ to PF from AF - PF context (interrupt thread) */
+static void
+roc_af_pf_mbox_irq_cn20k(void *param)
+{
+	struct dev *dev = param;
+	uint64_t intr;
+
+	intr = plt_read64(dev->mbox_reg_base + RVU_PF_INT);
+	if (intr == 0)
+		plt_base_dbg("Proceeding to check mbox UP messages if any");
+
+	plt_write64(intr, dev->mbox_reg_base + RVU_PF_INT);
+	plt_base_dbg("Irq 0x%" PRIx64 "(pf:%d,vf:%d)", intr, dev->pf, dev->vf);
+
+	/* If interrupt occurred for down message */
+	if (intr & BIT_ULL(1))
+		process_msgs(dev, dev->mbox);
+
+	/* If interrupt occurred for up message */
+	if (intr & BIT_ULL(0))
+		process_msgs_up(dev, &dev->mbox_up);
+}
+
 /* IRQ to VF from PF - VF context (interrupt thread) */
 static void
 roc_pf_vf_mbox_irq(void *param)
 {
 	struct dev *dev = param;
-	uint64_t mbox_data;
+	uint64_t mbox_data = 0;
 	uint64_t intr;
 
 	intr = plt_read64(dev->mbox_reg_base + RVU_VF_INT);
@@ -940,7 +985,7 @@ roc_pf_vf_mbox_irq(void *param)
 	 */
 	mbox_data = plt_read64(dev->mbox_reg_base + RVU_VF_VFPF_MBOX0);
 	/* If interrupt occurred for down message */
-	if (mbox_data & MBOX_DOWN_MSG || intr & BIT_ULL(1)) {
+	if (mbox_data & MBOX_DOWN_MSG) {
 		mbox_data &= ~MBOX_DOWN_MSG;
 		plt_write64(mbox_data, dev->mbox_reg_base + RVU_VF_VFPF_MBOX0);
 
@@ -948,7 +993,7 @@ roc_pf_vf_mbox_irq(void *param)
 		process_msgs(dev, dev->mbox);
 	}
 	/* If interrupt occurred for UP message */
-	if (mbox_data & MBOX_UP_MSG || intr & BIT_ULL(0)) {
+	if (mbox_data & MBOX_UP_MSG) {
 		mbox_data &= ~MBOX_UP_MSG;
 		plt_write64(mbox_data, dev->mbox_reg_base + RVU_VF_VFPF_MBOX0);
 
@@ -962,7 +1007,7 @@ static void
 roc_af_pf_mbox_irq(void *param)
 {
 	struct dev *dev = param;
-	uint64_t mbox_data;
+	uint64_t mbox_data = 0;
 	uint64_t intr;
 
 	intr = plt_read64(dev->mbox_reg_base + RVU_PF_INT);
@@ -977,7 +1022,7 @@ roc_af_pf_mbox_irq(void *param)
 	 */
 	mbox_data = plt_read64(dev->mbox_reg_base + RVU_PF_PFAF_MBOX0);
 	/* If interrupt occurred for down message */
-	if (mbox_data & MBOX_DOWN_MSG || intr & BIT_ULL(1)) {
+	if (mbox_data & MBOX_DOWN_MSG) {
 		mbox_data &= ~MBOX_DOWN_MSG;
 		plt_write64(mbox_data, dev->mbox_reg_base + RVU_PF_PFAF_MBOX0);
 
@@ -985,7 +1030,7 @@ roc_af_pf_mbox_irq(void *param)
 		process_msgs(dev, dev->mbox);
 	}
 	/* If interrupt occurred for up message */
-	if (mbox_data & MBOX_UP_MSG || intr & BIT_ULL(0)) {
+	if (mbox_data & MBOX_UP_MSG) {
 		mbox_data &= ~MBOX_UP_MSG;
 		plt_write64(mbox_data, dev->mbox_reg_base + RVU_PF_PFAF_MBOX0);
 
@@ -1045,7 +1090,8 @@ mbox_register_pf_irq(struct plt_pci_device *pci_dev, struct dev *dev)
 		}
 	}
 	/* MBOX interrupt AF <-> PF */
-	rc = dev_irq_register(intr_handle, roc_af_pf_mbox_irq, dev, dev->mbox_plat->pfaf_vec);
+	rc = dev_irq_register(intr_handle, dev->mbox_plat->ops->af_pf_mbox_irq, dev,
+			      dev->mbox_plat->pfaf_vec);
 	if (rc) {
 		plt_err("Fail to register AF<->PF mbox irq");
 		return rc;
@@ -1073,7 +1119,8 @@ mbox_register_vf_irq(struct plt_pci_device *pci_dev, struct dev *dev)
 	plt_write64(~0ull, dev->mbox_reg_base + RVU_VF_INT_ENA_W1C);
 
 	/* MBOX interrupt PF <-> VF */
-	rc = dev_irq_register(intr_handle, roc_pf_vf_mbox_irq, dev, RVU_VF_INT_VEC_MBOX);
+	rc = dev_irq_register(intr_handle, dev->mbox_plat->ops->pf_vf_mbox_irq, dev,
+			      RVU_VF_INT_VEC_MBOX);
 	if (rc) {
 		plt_err("Fail to register PF<->VF mbox irq");
 		return rc;
@@ -1127,7 +1174,8 @@ mbox_unregister_pf_irq(struct plt_pci_device *pci_dev, struct dev *dev)
 	}
 
 	/* MBOX interrupt AF <-> PF */
-	dev_irq_unregister(intr_handle, roc_af_pf_mbox_irq, dev, dev->mbox_plat->pfaf_vec);
+	dev_irq_unregister(intr_handle, dev->mbox_plat->ops->af_pf_mbox_irq, dev,
+			   dev->mbox_plat->pfaf_vec);
 }
 
 static void
@@ -1139,7 +1187,8 @@ mbox_unregister_vf_irq(struct plt_pci_device *pci_dev, struct dev *dev)
 	plt_write64(~0ull, dev->mbox_reg_base + RVU_VF_INT_ENA_W1C);
 
 	/* Unregister the interrupt handler */
-	dev_irq_unregister(intr_handle, roc_pf_vf_mbox_irq, dev, RVU_VF_INT_VEC_MBOX);
+	dev_irq_unregister(intr_handle, dev->mbox_plat->ops->pf_vf_mbox_irq, dev,
+			   RVU_VF_INT_VEC_MBOX);
 }
 
 void
@@ -1599,10 +1648,17 @@ dev_cache_line_size_valid(void)
 	return true;
 }
 
-static void
+static int
 mbox_platform_changes(struct mbox_platform *mbox_plat, uintptr_t bar2, uintptr_t bar4, bool is_vf)
 {
-	int i;
+	int i, rc = 0;
+
+	/* Allocate memory for device ops */
+	mbox_plat->ops = plt_zmalloc(sizeof(struct mbox_ops), 0);
+	if (mbox_plat->ops == NULL) {
+		rc = -ENOMEM;
+		goto fail;
+	}
 
 	if (roc_model_is_cn20k()) {
 		/* For CN20K, AF allocates mbox memory in DRAM and writes PF
@@ -1613,6 +1669,9 @@ mbox_platform_changes(struct mbox_platform *mbox_plat, uintptr_t bar2, uintptr_t
 		mbox_plat->mbox_region_base =
 			bar2 + (RVU_PFX_FUNC_PFAF_MBOX +
 				((uint64_t)RVU_BLOCK_ADDR_MBOX << RVU_FUNC_BLKADDR_SHIFT));
+		/* Mbox operations */
+		mbox_plat->ops->af_pf_mbox_irq = roc_af_pf_mbox_irq_cn20k;
+		mbox_plat->ops->pf_vf_mbox_irq = roc_pf_vf_mbox_irq_cn20k;
 		/* Interrupt vectors */
 		mbox_plat->pfaf_vec = RVU_MBOX_PF_INT_VEC_AFPF_MBOX;
 		mbox_plat->pfvf_mbox0_vec = RVU_MBOX_PF_INT_VEC_VFPF_MBOX0;
@@ -1630,6 +1689,9 @@ mbox_platform_changes(struct mbox_platform *mbox_plat, uintptr_t bar2, uintptr_t
 	} else {
 		mbox_plat->mbox_reg_base = bar2;
 		mbox_plat->mbox_region_base = bar4;
+		/* Mbox operations */
+		mbox_plat->ops->af_pf_mbox_irq = roc_af_pf_mbox_irq;
+		mbox_plat->ops->pf_vf_mbox_irq = roc_pf_vf_mbox_irq;
 		mbox_plat->pfaf_vec = RVU_PF_INT_VEC_AFPF_MBOX;
 		mbox_plat->pfvf_mbox0_vec = RVU_PF_INT_VEC_VFPF_MBOX0;
 		mbox_plat->pfvf_mbox1_vec = RVU_PF_INT_VEC_VFPF_MBOX1;
@@ -1647,6 +1709,8 @@ mbox_platform_changes(struct mbox_platform *mbox_plat, uintptr_t bar2, uintptr_t
 		if (roc_model_is_cn10k())
 			mbox_plat->mbox_region_base = bar2 + RVU_VF_MBOX_REGION;
 	}
+fail:
+	return rc;
 }
 
 int
@@ -1678,7 +1742,12 @@ dev_init(struct dev *dev, struct plt_pci_device *pci_dev)
 		rc = -ENOMEM;
 		goto fail;
 	}
-	mbox_platform_changes(dev->mbox_plat, bar2, bar4, is_vf);
+
+	if (mbox_platform_changes(dev->mbox_plat, bar2, bar4, is_vf)) {
+		plt_err("Failed to populate platform specific changes");
+		rc = -ENOMEM;
+		goto mbox_plat_free;
+	}
 
 	mbox_reg_base = dev->mbox_plat->mbox_reg_base;
 	mbox_region_base = dev->mbox_plat->mbox_region_base;
@@ -1824,6 +1893,8 @@ dev_init(struct dev *dev, struct plt_pci_device *pci_dev)
 	mbox_fini(dev->mbox);
 	mbox_fini(&dev->mbox_up);
 error:
+	plt_free(dev->mbox_plat->ops);
+mbox_plat_free:
 	plt_free(dev->mbox_plat);
 fail:
 	return rc;
@@ -1883,6 +1954,7 @@ dev_fini(struct dev *dev, struct plt_pci_device *pci_dev)
 	mbox_fini(mbox);
 	dev->mbox_active = 0;
 
+	plt_free(dev->mbox_plat->ops);
 	plt_free(dev->mbox_plat);
 	/* Disable MSIX vectors */
 	dev_irqs_disable(intr_handle);
diff --git a/drivers/common/cnxk/roc_dev_priv.h b/drivers/common/cnxk/roc_dev_priv.h
index c766183196..a8e40fa9d5 100644
--- a/drivers/common/cnxk/roc_dev_priv.h
+++ b/drivers/common/cnxk/roc_dev_priv.h
@@ -106,6 +106,15 @@ struct mbox_sync {
 	pthread_mutex_t mutex;
 };
 
+/* AF PF VF mbox interrupt callbacks */
+typedef void (*af_pf_mbox_irq_t)(void *param);
+typedef void (*pf_vf_mbox_irq_t)(void *param);
+
+struct mbox_ops {
+	af_pf_mbox_irq_t af_pf_mbox_irq;
+	pf_vf_mbox_irq_t pf_vf_mbox_irq;
+};
+
 struct mbox_platform {
 	uint8_t pfaf_vec;
 	uint8_t pfvf_mbox0_vec;
@@ -120,6 +129,8 @@ struct mbox_platform {
 	uint64_t pfvf1_mbox_int_ena_w1c[MAX_VFPF_DWORD_BITS];
 	uintptr_t mbox_reg_base;
 	uintptr_t mbox_region_base;
+	/* Mbox operations */
+	struct mbox_ops *ops;
 };
 
 struct dev {
diff --git a/drivers/common/cnxk/roc_mbox.c b/drivers/common/cnxk/roc_mbox.c
index db77babfdb..a3b96e6e2b 100644
--- a/drivers/common/cnxk/roc_mbox.c
+++ b/drivers/common/cnxk/roc_mbox.c
@@ -275,7 +275,7 @@ mbox_msg_send_data(struct mbox *mbox, int devid, uint8_t data)
 	tx_hdr->msg_size = mdev->msg_size;
 	mdev->msg_size = 0;
 	mdev->rsp_size = 0;
-	mdev->msgs_acked = 0;
+	plt_atomic_store_explicit(&mdev->msgs_acked, 0, plt_memory_order_release);
 
 	/* num_msgs != 0 signals to the peer that the buffer has a number of
 	 * messages. So this should be written after copying txmem
@@ -417,7 +417,8 @@ mbox_wait(struct mbox *mbox, int devid, uint32_t rst_timo)
 	 * mdev->msgs_acked are incremented at process_msgs() in interrupt
 	 * thread context.
 	 */
-	while (mdev->num_msgs > mdev->msgs_acked) {
+	while (mdev->num_msgs > plt_atomic_load_explicit(&mdev->msgs_acked,
+							 plt_memory_order_acquire)) {
 		plt_delay_us(sleep);
 		timeout += sleep;
 		if (timeout >= rst_timo) {
@@ -433,13 +434,12 @@ mbox_wait(struct mbox *mbox, int devid, uint32_t rst_timo)
 				"(tx/rx num_msgs: %d/%d), msg_size: %d, "
 				"rsp_size: %d",
 				devid, timeout, mdev->num_msgs,
-				mdev->msgs_acked, tx_hdr->num_msgs,
-				rx_hdr->num_msgs, mdev->msg_size,
-				mdev->rsp_size);
+				plt_atomic_load_explicit(&mdev->msgs_acked,
+							 plt_memory_order_acquire),
+				tx_hdr->num_msgs, rx_hdr->num_msgs, mdev->msg_size, mdev->rsp_size);
 
 			return -EIO;
 		}
-		plt_rmb();
 	}
 	return 0;
 }
diff --git a/drivers/common/cnxk/roc_platform.h b/drivers/common/cnxk/roc_platform.h
index df4f88f288..4076dbe94d 100644
--- a/drivers/common/cnxk/roc_platform.h
+++ b/drivers/common/cnxk/roc_platform.h
@@ -211,6 +211,11 @@ plt_thread_is_valid(plt_thread_t thr)
 #define plt_io_rmb()		rte_io_rmb()
 #define plt_atomic_thread_fence rte_atomic_thread_fence
 
+#define plt_atomic_store_explicit rte_atomic_store_explicit
+#define plt_atomic_load_explicit  rte_atomic_load_explicit
+#define plt_memory_order_release  rte_memory_order_release
+#define plt_memory_order_acquire  rte_memory_order_acquire
+
 #define plt_bit_relaxed_get32   rte_bit_relaxed_get32
 #define plt_bit_relaxed_set32   rte_bit_relaxed_set32
 #define plt_bit_relaxed_clear32 rte_bit_relaxed_clear32
-- 
2.25.1