diff mbox series

[2/2] common/cnxk: fix race condition between up and down mbox

Message ID 20231005062513.29467-2-hkalra@marvell.com (mailing list archive)
State Accepted, archived
Delegated to: Jerin Jacob
Headers
Series [1/2] common/cnxk: fix handling up and down interrupts |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS

Commit Message

Harman Kalra Oct. 5, 2023, 6:25 a.m. UTC 
  Fixing a possible case for race condition where an up mbox
interrupt over writes the down mbox message.
Although mbox_wait_for_zero() makes sure no up/down message
is pending before raising an up mbox interrupt. But there is a
small window were a VF may send a down mbox request to PF after
mbox_wait_for_zero() and before PF attempts to send a up
message to same VF. In such scenario interrupt register which has
down message bit set will get overwritten by up message bit.

As a solution, read interrupt register and OR the status with
required up/down bit before writing to the interrupt register.

Fixes: fa4ee2d43188 ("common/cnxk: sync between mbox up and down messages")

Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
 drivers/common/cnxk/roc_mbox.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

Comments

Jerin Jacob Oct. 5, 2023, 2:16 p.m. UTC | #1 
On Thu, Oct 5, 2023 at 12:06 PM Harman Kalra <hkalra@marvell.com> wrote:
>
> Fixing a possible case for race condition where an up mbox
> interrupt over writes the down mbox message.
> Although mbox_wait_for_zero() makes sure no up/down message
> is pending before raising an up mbox interrupt. But there is a
> small window were a VF may send a down mbox request to PF after
> mbox_wait_for_zero() and before PF attempts to send a up
> message to same VF. In such scenario interrupt register which has
> down message bit set will get overwritten by up message bit.
>
> As a solution, read interrupt register and OR the status with
> required up/down bit before writing to the interrupt register.
>
> Fixes: fa4ee2d43188 ("common/cnxk: sync between mbox up and down messages")

Cc: stable@dpdk.org

>
> Signed-off-by: Harman Kalra <hkalra@marvell.com>

Series applied to dpdk-next-net-mrvl/for-next-net. Thanks
diff mbox series

Patch

diff --git a/drivers/common/cnxk/roc_mbox.c b/drivers/common/cnxk/roc_mbox.c
index c91fa63e83..7b734fcd24 100644
--- a/drivers/common/cnxk/roc_mbox.c
+++ b/drivers/common/cnxk/roc_mbox.c
@@ -209,10 +209,9 @@  static void
 mbox_msg_send_data(struct mbox *mbox, int devid, uint8_t data)
 {
 	struct mbox_dev *mdev = &mbox->dev[devid];
-	struct mbox_hdr *tx_hdr =
-		(struct mbox_hdr *)((uintptr_t)mdev->mbase + mbox->tx_start);
-	struct mbox_hdr *rx_hdr =
-		(struct mbox_hdr *)((uintptr_t)mdev->mbase + mbox->rx_start);
+	struct mbox_hdr *tx_hdr = (struct mbox_hdr *)((uintptr_t)mdev->mbase + mbox->tx_start);
+	struct mbox_hdr *rx_hdr = (struct mbox_hdr *)((uintptr_t)mdev->mbase + mbox->rx_start);
+	uint64_t intr_val;
 
 	/* Reset header for next messages */
 	tx_hdr->msg_size = mdev->msg_size;
@@ -229,11 +228,16 @@  mbox_msg_send_data(struct mbox *mbox, int devid, uint8_t data)
 	/* Sync mbox data into memory */
 	plt_wmb();
 
+	/* Check for any pending interrupt */
+	intr_val = plt_read64(
+		(volatile void *)(mbox->reg_base + (mbox->trigger | (devid << mbox->tr_shift))));
+
+	intr_val |= (uint64_t)data;
 	/* The interrupt should be fired after num_msgs is written
 	 * to the shared memory
 	 */
-	plt_write64(data, (volatile void *)(mbox->reg_base +
-				(mbox->trigger | (devid << mbox->tr_shift))));
+	plt_write64(intr_val, (volatile void *)(mbox->reg_base +
+						(mbox->trigger | (devid << mbox->tr_shift))));
 }
 
 /**