Re: [dpdk-dev] [PATCH v3 1/2] net/hinic/base: fix FW hotactive problem

[Kevin Traynor <ktraynor@redhat.com>]

On 01/04/2020 10:30, Xiaoyun wang wrote:
> When PF detects FW is hotactive, up returns HINIC_DEV_BUSY_ACTIVE_FW for
> pf driver, when cmdq reinit at txrx flush, cmdq will fail, driver should
> reinit the cmdq when port start.
> 
> Fixes: d9ce1917941c ("net/hinic/base: add hardware operation")

Hi. Please add tag below if you are requesting this for backport.
(http://doc.dpdk.org/guides/contributing/patches.html#patch-for-stable-releases)

Cc: stable@dpdk.org

> Signed-off-by: Xiaoyun wang <cloud.wangxiaoyun@huawei.com>
> ---
>  drivers/net/hinic/base/hinic_pmd_cmdq.c  |  7 ++++--
>  drivers/net/hinic/base/hinic_pmd_hwdev.c |  4 ++--
>  drivers/net/hinic/base/hinic_pmd_mgmt.c  | 40 ++++++++++++++++++--------------
>  drivers/net/hinic/base/hinic_pmd_mgmt.h  |  2 ++
>  drivers/net/hinic/base/hinic_pmd_nicio.c |  2 +-
>  drivers/net/hinic/hinic_pmd_ethdev.c     |  7 +-----
>  6 files changed, 33 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/net/hinic/base/hinic_pmd_cmdq.c b/drivers/net/hinic/base/hinic_pmd_cmdq.c
> index 685498e..2e98b9c 100644
> --- a/drivers/net/hinic/base/hinic_pmd_cmdq.c
> +++ b/drivers/net/hinic/base/hinic_pmd_cmdq.c
> @@ -440,9 +440,12 @@ static int hinic_set_cmdq_ctxts(struct hinic_hwdev *hwdev)
>  					     cmdq_ctxt, in_size, NULL,
>  					     NULL, 0);
>  		if (err) {
> -			if (err == HINIC_MBOX_PF_BUSY_ACTIVE_FW)
> +			if (err == HINIC_MBOX_PF_BUSY_ACTIVE_FW ||
> +				err == HINIC_DEV_BUSY_ACTIVE_FW) {
>  				cmdqs->status |= HINIC_CMDQ_SET_FAIL;
> -			PMD_DRV_LOG(ERR, "Set cmdq ctxt failed");
> +				PMD_DRV_LOG(ERR, "PF or VF fw is hot active");
> +			}
> +			PMD_DRV_LOG(ERR, "Set cmdq ctxt failed, err: %d", err);
>  			return -EFAULT;
>  		}
>  	}
> diff --git a/drivers/net/hinic/base/hinic_pmd_hwdev.c b/drivers/net/hinic/base/hinic_pmd_hwdev.c
> index cf2a970..fc11ecd 100644
> --- a/drivers/net/hinic/base/hinic_pmd_hwdev.c
> +++ b/drivers/net/hinic/base/hinic_pmd_hwdev.c
> @@ -529,7 +529,7 @@ static int hinic_vf_rx_tx_flush(struct hinic_hwdev *hwdev)
>  
>  	err = hinic_reinit_cmdq_ctxts(hwdev);
>  	if (err)
> -		PMD_DRV_LOG(WARNING, "Reinit cmdq failed");
> +		PMD_DRV_LOG(WARNING, "Reinit cmdq failed when vf flush");
>  
>  	return err;
>  }
> @@ -587,7 +587,7 @@ static int hinic_pf_rx_tx_flush(struct hinic_hwdev *hwdev)
>  
>  	err = hinic_reinit_cmdq_ctxts(hwdev);
>  	if (err)
> -		PMD_DRV_LOG(WARNING, "Reinit cmdq failed");
> +		PMD_DRV_LOG(WARNING, "Reinit cmdq failed when pf flush");
>  
>  	return 0;
>  }
> diff --git a/drivers/net/hinic/base/hinic_pmd_mgmt.c b/drivers/net/hinic/base/hinic_pmd_mgmt.c
> index addc9d2..06c9b68 100644
> --- a/drivers/net/hinic/base/hinic_pmd_mgmt.c
> +++ b/drivers/net/hinic/base/hinic_pmd_mgmt.c
> @@ -248,6 +248,19 @@ static void free_msg_buf(struct hinic_msg_pf_to_mgmt *pf_to_mgmt)
>  	free_recv_msg(&pf_to_mgmt->recv_msg_from_mgmt);
>  }
>  
> +static int hinic_get_mgmt_channel_status(void *hwdev)
> +{
> +	struct hinic_hwif *hwif = ((struct hinic_hwdev *)hwdev)->hwif;
> +	u32 val;
> +
> +	if (hinic_func_type((struct hinic_hwdev *)hwdev) == TYPE_VF)
> +		return false;
> +
> +	val = hinic_hwif_read_reg(hwif, HINIC_ICPL_RESERVD_ADDR);
> +
> +	return HINIC_GET_MGMT_CHANNEL_STATUS(val, MGMT_CHANNEL_STATUS);
> +}
> +
>  /**
>   * send_msg_to_mgmt_async - send async message
>   * @pf_to_mgmt: PF to MGMT channel
> @@ -309,6 +322,14 @@ static int send_msg_to_mgmt_sync(struct hinic_msg_pf_to_mgmt *pf_to_mgmt,
>  	u64 header;
>  	u16 cmd_size = mgmt_msg_len(msg_len);
>  
> +	/* If fw is hot active, return failed */
> +	if (hinic_get_mgmt_channel_status(pf_to_mgmt->hwdev)) {
> +		if (mod == HINIC_MOD_COMM || mod == HINIC_MOD_L2NIC)
> +			return HINIC_DEV_BUSY_ACTIVE_FW;
> +		else
> +			return -EBUSY;
> +	}
> +
>  	if (direction == HINIC_MSG_RESPONSE)
>  		prepare_header(pf_to_mgmt, &header, msg_len, mod, ack_type,
>  			       direction, cmd, resp_msg_id);
> @@ -449,7 +470,7 @@ static void hinic_pf_to_mgmt_free(struct hinic_hwdev *hwdev)
>  			       recv_msg->msg_len);
>  			*out_size = recv_msg->msg_len;
>  		} else {
> -			PMD_DRV_LOG(ERR, "Mgmt rsp's msg len: %u overflow.",
> +			PMD_DRV_LOG(ERR, "Mgmt rsp's msg len:%u overflow.",
>  				recv_msg->msg_len);
>  			err = -ERANGE;
>  		}
> @@ -462,19 +483,6 @@ static void hinic_pf_to_mgmt_free(struct hinic_hwdev *hwdev)
>  	return err;
>  }
>  
> -static int hinic_get_mgmt_channel_status(void *hwdev)
> -{
> -	struct hinic_hwif *hwif = ((struct hinic_hwdev *)hwdev)->hwif;
> -	u32 val;
> -
> -	if (hinic_func_type((struct hinic_hwdev *)hwdev) == TYPE_VF)
> -		return false;
> -
> -	val = hinic_hwif_read_reg(hwif, HINIC_ICPL_RESERVD_ADDR);
> -
> -	return HINIC_GET_MGMT_CHANNEL_STATUS(val, MGMT_CHANNEL_STATUS);
> -}
> -
>  int hinic_msg_to_mgmt_sync(void *hwdev, enum hinic_mod_type mod, u8 cmd,
>  			   void *buf_in, u16 in_size,
>  			   void *buf_out, u16 *out_size, u32 timeout)
> @@ -484,10 +492,6 @@ int hinic_msg_to_mgmt_sync(void *hwdev, enum hinic_mod_type mod, u8 cmd,
>  	if (!hwdev || in_size > HINIC_MSG_TO_MGMT_MAX_LEN)
>  		return -EINVAL;
>  
> -	/* If status is hot upgrading, don't send message to mgmt */
> -	if (hinic_get_mgmt_channel_status(hwdev))
> -		return -EPERM;
> -
>  	if (hinic_func_type(hwdev) == TYPE_VF) {
>  		rc = hinic_mbox_to_pf(hwdev, mod, cmd, buf_in, in_size,
>  					buf_out, out_size, timeout);
> diff --git a/drivers/net/hinic/base/hinic_pmd_mgmt.h b/drivers/net/hinic/base/hinic_pmd_mgmt.h
> index cc18843..52b319e 100644
> --- a/drivers/net/hinic/base/hinic_pmd_mgmt.h
> +++ b/drivers/net/hinic/base/hinic_pmd_mgmt.h
> @@ -34,6 +34,8 @@
>  #define HINIC_MSG_HEADER_P2P_IDX_MASK				0xF
>  #define HINIC_MSG_HEADER_MSG_ID_MASK				0x3FF
>  
> +#define HINIC_DEV_BUSY_ACTIVE_FW				0xFE
> +
>  #define HINIC_MSG_HEADER_GET(val, member)			\
>  		(((val) >> HINIC_MSG_HEADER_##member##_SHIFT) & \
>  		HINIC_MSG_HEADER_##member##_MASK)
> diff --git a/drivers/net/hinic/base/hinic_pmd_nicio.c b/drivers/net/hinic/base/hinic_pmd_nicio.c
> index fd34b03..60c4e14 100644
> --- a/drivers/net/hinic/base/hinic_pmd_nicio.c
> +++ b/drivers/net/hinic/base/hinic_pmd_nicio.c
> @@ -537,7 +537,7 @@ int hinic_init_qp_ctxts(struct hinic_hwdev *hwdev)
>  	if (hwdev->cmdqs->status & HINIC_CMDQ_SET_FAIL) {
>  		err = hinic_reinit_cmdq_ctxts(hwdev);
>  		if (err) {
> -			PMD_DRV_LOG(ERR, "Reinit cmdq context failed, rc: %d",
> +			PMD_DRV_LOG(ERR, "Reinit cmdq context failed when dev start, err: %d",
>  				err);
>  			return err;
>  		}
> diff --git a/drivers/net/hinic/hinic_pmd_ethdev.c b/drivers/net/hinic/hinic_pmd_ethdev.c
> index 23724a0..239a78c 100644
> --- a/drivers/net/hinic/hinic_pmd_ethdev.c
> +++ b/drivers/net/hinic/hinic_pmd_ethdev.c
> @@ -1862,11 +1862,6 @@ static int hinic_flow_ctrl_get(struct rte_eth_dev *dev,
>  	else
>  		fc_conf->mode = RTE_FC_NONE;
>  
> -	PMD_DRV_LOG(INFO, "Get pause options, tx: %s, rx: %s, auto: %s\n",
> -		nic_pause.tx_pause ? "on" : "off",
> -		nic_pause.rx_pause ? "on" : "off",
> -		nic_pause.auto_neg ? "on" : "off");
> -
>  	return 0;
>  }
>  
> @@ -1900,7 +1895,7 @@ static int hinic_flow_ctrl_set(struct rte_eth_dev *dev,
>  	nic_dev->nic_pause.rx_pause = nic_pause.rx_pause;
>  	nic_dev->nic_pause.tx_pause = nic_pause.tx_pause;
>  
> -	PMD_DRV_LOG(INFO, "Get pause options, tx: %s, rx: %s, auto: %s\n",
> +	PMD_DRV_LOG(INFO, "Set pause options, tx: %s, rx: %s, auto: %s\n",
>  		nic_pause.tx_pause ? "on" : "off",
>  		nic_pause.rx_pause ? "on" : "off",
>  		nic_pause.auto_neg ? "on" : "off");
>