* [PATCH 1/2] common/cnxk: fix handling up and down interrupts
@ 2023-10-05 6:25 Harman Kalra
2023-10-05 6:25 ` [PATCH 2/2] common/cnxk: fix race condition between up and down mbox Harman Kalra
0 siblings, 1 reply; 3+ messages in thread
From: Harman Kalra @ 2023-10-05 6:25 UTC (permalink / raw)
To: jerinj, Nithin Dabilpuram, Kiran Kumar K, Sunil Kumar Kori, Satha Rao
Cc: dev, Harman Kalra
Down messages should be processed only when down interrupt is received
while UP messages should be processed when up interrupt is received.
A scenario has been observed where processing down message on UP
message interrupt causes processing of old message response while
response of latest message is not received.
Fixes: fa4ee2d43188 ("common/cnxk: sync between mbox up and down messages")
Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
drivers/common/cnxk/roc_dev.c | 40 ++++++++++++++++++++++++-----------
1 file changed, 28 insertions(+), 12 deletions(-)
diff --git a/drivers/common/cnxk/roc_dev.c b/drivers/common/cnxk/roc_dev.c
index e41235ea8a..c948caba63 100644
--- a/drivers/common/cnxk/roc_dev.c
+++ b/drivers/common/cnxk/roc_dev.c
@@ -770,14 +770,22 @@ roc_pf_vf_mbox_irq(void *param)
* by 1ms until this region is zeroed mbox_wait_for_zero()
*/
mbox_data = plt_read64(dev->bar2 + RVU_VF_VFPF_MBOX0);
- if (mbox_data)
- plt_write64(!mbox_data, dev->bar2 + RVU_VF_VFPF_MBOX0);
+ /* If interrupt occurred for down message */
+ if (mbox_data & MBOX_DOWN_MSG) {
+ mbox_data &= ~MBOX_DOWN_MSG;
+ plt_write64(mbox_data, dev->bar2 + RVU_VF_VFPF_MBOX0);
- /* First process all configuration messages */
- process_msgs(dev, dev->mbox);
+ /* First process all configuration messages */
+ process_msgs(dev, dev->mbox);
+ }
+ /* If interrupt occurred for UP message */
+ if (mbox_data & MBOX_UP_MSG) {
+ mbox_data &= ~MBOX_UP_MSG;
+ plt_write64(mbox_data, dev->bar2 + RVU_VF_VFPF_MBOX0);
- /* Process Uplink messages */
- process_msgs_up(dev, &dev->mbox_up);
+ /* Process Uplink messages */
+ process_msgs_up(dev, &dev->mbox_up);
+ }
}
/* IRQ to PF from AF - PF context (interrupt thread) */
@@ -799,14 +807,22 @@ roc_af_pf_mbox_irq(void *param)
* by 1ms until this region is zeroed mbox_wait_for_zero()
*/
mbox_data = plt_read64(dev->bar2 + RVU_PF_PFAF_MBOX0);
- if (mbox_data)
- plt_write64(!mbox_data, dev->bar2 + RVU_PF_PFAF_MBOX0);
+ /* If interrupt occurred for down message */
+ if (mbox_data & MBOX_DOWN_MSG) {
+ mbox_data &= ~MBOX_DOWN_MSG;
+ plt_write64(mbox_data, dev->bar2 + RVU_PF_PFAF_MBOX0);
- /* First process all configuration messages */
- process_msgs(dev, dev->mbox);
+ /* First process all configuration messages */
+ process_msgs(dev, dev->mbox);
+ }
+ /* If interrupt occurred for up message */
+ if (mbox_data & MBOX_UP_MSG) {
+ mbox_data &= ~MBOX_UP_MSG;
+ plt_write64(mbox_data, dev->bar2 + RVU_PF_PFAF_MBOX0);
- /* Process Uplink messages */
- process_msgs_up(dev, &dev->mbox_up);
+ /* Process Uplink messages */
+ process_msgs_up(dev, &dev->mbox_up);
+ }
}
static int
--
2.18.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 2/2] common/cnxk: fix race condition between up and down mbox
2023-10-05 6:25 [PATCH 1/2] common/cnxk: fix handling up and down interrupts Harman Kalra
@ 2023-10-05 6:25 ` Harman Kalra
2023-10-05 14:16 ` Jerin Jacob
0 siblings, 1 reply; 3+ messages in thread
From: Harman Kalra @ 2023-10-05 6:25 UTC (permalink / raw)
To: jerinj, Nithin Dabilpuram, Kiran Kumar K, Sunil Kumar Kori, Satha Rao
Cc: dev, Harman Kalra
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(-)
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))));
}
/**
--
2.18.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/2] common/cnxk: fix race condition between up and down mbox
2023-10-05 6:25 ` [PATCH 2/2] common/cnxk: fix race condition between up and down mbox Harman Kalra
@ 2023-10-05 14:16 ` Jerin Jacob
0 siblings, 0 replies; 3+ messages in thread
From: Jerin Jacob @ 2023-10-05 14:16 UTC (permalink / raw)
To: Harman Kalra
Cc: jerinj, Nithin Dabilpuram, Kiran Kumar K, Sunil Kumar Kori,
Satha Rao, dev
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-10-05 14:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-05 6:25 [PATCH 1/2] common/cnxk: fix handling up and down interrupts Harman Kalra
2023-10-05 6:25 ` [PATCH 2/2] common/cnxk: fix race condition between up and down mbox Harman Kalra
2023-10-05 14:16 ` 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).