diff mbox series

[v2,3/8] net: thunderx: make CFG_DONE message to run through generic send-ack sequence

Message ID 20190218095159.3847-4-vlomovtsev@marvell.com (mailing list archive)
State New, archived
Headers show
Series nic: thunderx: fix communication races between VF & PF | expand

Commit Message

Vadim Lomovtsev Feb. 18, 2019, 9:52 a.m. UTC
At the end of NIC VF initialization VF sends CFG_DONE message to PF without
using nicvf_msg_send_to_pf routine. This potentially could re-write data in
mailbox. This commit is to implement common way of sending CFG_DONE message
by the same way with other configuration messages by using
nicvf_send_msg_to_pf() routine.

Signed-off-by: Vadim Lomovtsev <vlomovtsev@marvell.com>
---
 drivers/net/ethernet/cavium/thunder/nic_main.c |  2 +-
 .../net/ethernet/cavium/thunder/nicvf_main.c   | 18 +++++++++++++++---
 2 files changed, 16 insertions(+), 4 deletions(-)

Comments

David Miller Feb. 18, 2019, 11:33 p.m. UTC | #1
From: Vadim Lomovtsev <vlomovtsev@marvell.com>
Date: Mon, 18 Feb 2019 09:52:14 +0000

> @@ -169,6 +169,20 @@ static int nicvf_check_pf_ready(struct nicvf *nic)
>  	return 1;
>  }
>  
> +static int nicvf_send_cfg_done(struct nicvf *nic)
> +{
> +	union nic_mbx mbx = {};
> +
> +	mbx.msg.msg = NIC_MBOX_MSG_CFG_DONE;
> +	if (nicvf_send_msg_to_pf(nic, &mbx)) {
> +		netdev_err(nic->netdev,
> +			   "PF didn't respond to CFG DONE msg\n");
> +		return 0;
> +	}
> +
> +	return 1;
> +}
 ...
> @@ -1515,8 +1528,7 @@ int nicvf_open(struct net_device *netdev)
>  		nicvf_enable_intr(nic, NICVF_INTR_RBDR, qidx);
>  
>  	/* Send VF config done msg to PF */
> -	mbx.msg.msg = NIC_MBOX_MSG_CFG_DONE;
> -	nicvf_write_to_mbx(nic, &mbx);
> +	nicvf_send_cfg_done(nic);

If the one and only call site doesn't even bother to check the return
value, just make it return void.

Thanks.
Vadim Lomovtsev Feb. 19, 2019, 11:21 a.m. UTC | #2
Hi David,

On Mon, Feb 18, 2019 at 03:33:10PM -0800, David Miller wrote:
> From: Vadim Lomovtsev <vlomovtsev@marvell.com>
> Date: Mon, 18 Feb 2019 09:52:14 +0000
> 
> > @@ -169,6 +169,20 @@ static int nicvf_check_pf_ready(struct nicvf *nic)
> >  	return 1;
> >  }
> >  
> > +static int nicvf_send_cfg_done(struct nicvf *nic)
> > +{
> > +	union nic_mbx mbx = {};
> > +
> > +	mbx.msg.msg = NIC_MBOX_MSG_CFG_DONE;
> > +	if (nicvf_send_msg_to_pf(nic, &mbx)) {
> > +		netdev_err(nic->netdev,
> > +			   "PF didn't respond to CFG DONE msg\n");
> > +		return 0;
> > +	}
> > +
> > +	return 1;
> > +}
>  ...
> > @@ -1515,8 +1528,7 @@ int nicvf_open(struct net_device *netdev)
> >  		nicvf_enable_intr(nic, NICVF_INTR_RBDR, qidx);
> >  
> >  	/* Send VF config done msg to PF */
> > -	mbx.msg.msg = NIC_MBOX_MSG_CFG_DONE;
> > -	nicvf_write_to_mbx(nic, &mbx);
> > +	nicvf_send_cfg_done(nic);
> 
> If the one and only call site doesn't even bother to check the return
> value, just make it return void.
> 
> Thanks.

Thank you for your time and comments.
I'll update patch and re-submit.

WBR,
Vadim
diff mbox series

Patch

diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c
index 6c8dcb65ff03..90497a27df18 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -1039,7 +1039,7 @@  static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
 	case NIC_MBOX_MSG_CFG_DONE:
 		/* Last message of VF config msg sequence */
 		nic_enable_vf(nic, vf, true);
-		goto unlock;
+		break;
 	case NIC_MBOX_MSG_SHUTDOWN:
 		/* First msg in VF teardown sequence */
 		if (vf >= nic->num_vf_en)
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index abf24e7dff2d..b0e8a04e0f1e 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -169,6 +169,20 @@  static int nicvf_check_pf_ready(struct nicvf *nic)
 	return 1;
 }
 
+static int nicvf_send_cfg_done(struct nicvf *nic)
+{
+	union nic_mbx mbx = {};
+
+	mbx.msg.msg = NIC_MBOX_MSG_CFG_DONE;
+	if (nicvf_send_msg_to_pf(nic, &mbx)) {
+		netdev_err(nic->netdev,
+			   "PF didn't respond to CFG DONE msg\n");
+		return 0;
+	}
+
+	return 1;
+}
+
 static void nicvf_read_bgx_stats(struct nicvf *nic, struct bgx_stats_msg *bgx)
 {
 	if (bgx->rx)
@@ -1416,7 +1430,6 @@  int nicvf_open(struct net_device *netdev)
 	struct nicvf *nic = netdev_priv(netdev);
 	struct queue_set *qs = nic->qs;
 	struct nicvf_cq_poll *cq_poll = NULL;
-	union nic_mbx mbx = {};
 
 	/* wait till all queued set_rx_mode tasks completes if any */
 	drain_workqueue(nic->nicvf_rx_mode_wq);
@@ -1515,8 +1528,7 @@  int nicvf_open(struct net_device *netdev)
 		nicvf_enable_intr(nic, NICVF_INTR_RBDR, qidx);
 
 	/* Send VF config done msg to PF */
-	mbx.msg.msg = NIC_MBOX_MSG_CFG_DONE;
-	nicvf_write_to_mbx(nic, &mbx);
+	nicvf_send_cfg_done(nic);
 
 	return 0;
 cleanup: