[PATCH net-next 0/3] Bug fixes & optimization for HNS3 Driver

[PATCH net-next 0/3] Bug fixes & optimization for HNS3 Driver

[Salil Mehta]

This patch-set presents 2 priority bug fixes and an optimization for
HNS3 driver.

Xi Wang (3):
  net: hns3: Fix for VF mailbox cannot receiving PF response
  net: hns3: Fix for VF mailbox receiving unknown message
  net: hns3: Optimize PF CMDQ interrupt switching process

 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c    | 13 ++++++++++++
 .../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c  |  3 +++
 .../ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c   | 23 +++++++++++++++++++---
 3 files changed, 36 insertions(+), 3 deletions(-)

-- 
2.7.4

[PATCH net-next 1/3] net: hns3: Fix for VF mailbox cannot receiving PF response

[Salil Mehta]

From: Xi Wang <wangxi11@huawei.com>

When the VF frequently switches the CMDQ interrupt, if the CMDQ_SRC is not
cleared, the VF will not receive the new PF response after the interrupt
is re-enabled, the corresponding log is as follows:

[  317.482222] hns3 0000:00:03.0: VF could not get mbx resp(=0) from PF
in 500 tries
[  317.483137] hns3 0000:00:03.0: VF request to get tqp info from PF
failed -5

This patch fixes this problem by clearing CMDQ_SRC before enabling
interrupt and syncing pending IRQ handlers after disabling interrupt.

Fixes: e2cb1dec9779 ("net: hns3: Add HNS3 VF HCL(Hardware Compatibility Layer) Support")
Signed-off-by: Xi Wang <wangxi11@huawei.com>
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index dd8e8e6..d55ee9c 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -1576,6 +1576,8 @@ static int hclgevf_misc_irq_init(struct hclgevf_dev *hdev)
 		return ret;
 	}
 
+	hclgevf_clear_event_cause(hdev, 0);
+
 	/* enable misc. vector(vector 0) */
 	hclgevf_enable_vector(&hdev->misc_vector, true);
 
@@ -1586,6 +1588,7 @@ static void hclgevf_misc_irq_uninit(struct hclgevf_dev *hdev)
 {
 	/* disable misc vector(vector 0) */
 	hclgevf_enable_vector(&hdev->misc_vector, false);
+	synchronize_irq(hdev->misc_vector.vector_irq);
 	free_irq(hdev->misc_vector.vector_irq, hdev);
 	hclgevf_free_vector(hdev, 0);
 }
-- 
2.7.4

[PATCH net-next 2/3] net: hns3: Fix for VF mailbox receiving unknown message

[Salil Mehta]

From: Xi Wang <wangxi11@huawei.com>

Before the firmware updates the crq's tail pointer, if the VF driver
reads the data in the crq, the data may be incomplete at this time,
which will lead to the driver read an unknown message.

This patch fixes it by checking if crq is empty before reading the
message.

Fixes: b11a0bb231f3 ("net: hns3: Add mailbox support to VF driver")
Signed-off-by: Xi Wang <wangxi11@huawei.com>
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 .../ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c   | 23 +++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c
index a286184..173ca27 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_mbx.c
@@ -126,6 +126,13 @@ int hclgevf_send_mbx_msg(struct hclgevf_dev *hdev, u16 code, u16 subcode,
 	return status;
 }
 
+static bool hclgevf_cmd_crq_empty(struct hclgevf_hw *hw)
+{
+	u32 tail = hclgevf_read_dev(hw, HCLGEVF_NIC_CRQ_TAIL_REG);
+
+	return tail == hw->cmq.crq.next_to_use;
+}
+
 void hclgevf_mbx_handler(struct hclgevf_dev *hdev)
 {
 	struct hclgevf_mbx_resp_status *resp;
@@ -140,11 +147,22 @@ void hclgevf_mbx_handler(struct hclgevf_dev *hdev)
 	resp = &hdev->mbx_resp;
 	crq = &hdev->hw.cmq.crq;
 
-	flag = le16_to_cpu(crq->desc[crq->next_to_use].flag);
-	while (hnae_get_bit(flag, HCLGEVF_CMDQ_RX_OUTVLD_B)) {
+	while (!hclgevf_cmd_crq_empty(&hdev->hw)) {
 		desc = &crq->desc[crq->next_to_use];
 		req = (struct hclge_mbx_pf_to_vf_cmd *)desc->data;
 
+		flag = le16_to_cpu(crq->desc[crq->next_to_use].flag);
+		if (unlikely(!hnae3_get_bit(flag, HCLGEVF_CMDQ_RX_OUTVLD_B))) {
+			dev_warn(&hdev->pdev->dev,
+				 "dropped invalid mailbox message, code = %d\n",
+				 req->msg[0]);
+
+			/* dropping/not processing this invalid message */
+			crq->desc[crq->next_to_use].flag = 0;
+			hclge_mbx_ring_ptr_move_crq(crq);
+			continue;
+		}
+
 		/* synchronous messages are time critical and need preferential
 		 * treatment. Therefore, we need to acknowledge all the sync
 		 * responses as quickly as possible so that waiting tasks do not
@@ -205,7 +223,6 @@ void hclgevf_mbx_handler(struct hclgevf_dev *hdev)
 		}
 		crq->desc[crq->next_to_use].flag = 0;
 		hclge_mbx_ring_ptr_move_crq(crq);
-		flag = le16_to_cpu(crq->desc[crq->next_to_use].flag);
 	}
 
 	/* Write back CMDQ_RQ header pointer, M7 need this pointer */
-- 
2.7.4

[PATCH net-next 3/3] net: hns3: Optimize PF CMDQ interrupt switching process

[Salil Mehta]

From: Xi Wang <wangxi11@huawei.com>

When the PF frequently switches the CMDQ interrupt, if the CMDQ_SRC is
not cleared before the hardware interrupt is generated, the new interrupt
will not be reported.

This patch optimizes this problem by clearing CMDQ_SRC and RESET_STS
before enabling interrupt and syncing pending IRQ handlers after disabling
interrupt.

Fixes: 466b0c00391b ("net: hns3: Add support for misc interrupt")
Signed-off-by: Xi Wang <wangxi11@huawei.com>
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 2a80134..d318d35 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -2557,6 +2557,15 @@ static void hclge_clear_event_cause(struct hclge_dev *hdev, u32 event_type,
 	}
 }
 
+static void hclge_clear_all_event_cause(struct hclge_dev *hdev)
+{
+	hclge_clear_event_cause(hdev, HCLGE_VECTOR0_EVENT_RST,
+				BIT(HCLGE_VECTOR0_GLOBALRESET_INT_B) |
+				BIT(HCLGE_VECTOR0_CORERESET_INT_B) |
+				BIT(HCLGE_VECTOR0_IMPRESET_INT_B));
+	hclge_clear_event_cause(hdev, HCLGE_VECTOR0_EVENT_MBX, 0);
+}
+
 static void hclge_enable_vector(struct hclge_misc_vector *vector, bool enable)
 {
 	writel(enable ? 1 : 0, vector->addr);
@@ -5688,6 +5697,8 @@ static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev)
 	INIT_WORK(&hdev->rst_service_task, hclge_reset_service_task);
 	INIT_WORK(&hdev->mbx_service_task, hclge_mailbox_service_task);
 
+	hclge_clear_all_event_cause(hdev);
+
 	/* Enable MISC vector(vector0) */
 	hclge_enable_vector(&hdev->misc_vector, true);
 
@@ -5817,6 +5828,8 @@ static void hclge_uninit_ae_dev(struct hnae3_ae_dev *ae_dev)
 
 	/* Disable MISC vector(vector0) */
 	hclge_enable_vector(&hdev->misc_vector, false);
+	synchronize_irq(hdev->misc_vector.vector_irq);
+
 	hclge_destroy_cmd_queue(&hdev->hw);
 	hclge_misc_irq_uninit(hdev);
 	hclge_pci_uninit(hdev);
-- 
2.7.4

Re: [PATCH net-next 2/3] net: hns3: Fix for VF mailbox receiving unknown message

[David Miller]

From: Salil Mehta <salil.mehta@huawei.com>
Date: Tue, 5 Jun 2018 12:42:00 +0100

> +		if (unlikely(!hnae3_get_bit(flag, HCLGEVF_CMDQ_RX_OUTVLD_B))) {

This breaks the build, there is no such symbol named hnae3_get_bit().

Re: [PATCH net-next 2/3] net: hns3: Fix for VF mailbox receiving unknown message

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 9646 bytes --]

Hi Xi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Salil-Mehta/Bug-fixes-optimization-for-HNS3-Driver/20180606-155040
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=xtensa 

All errors (new ones prefixed by >>):

   In file included from include/linux/init.h:5,
                    from drivers/net/ethernet/hisilicon/hns3/hclge_mbx.h:6,
                    from drivers/net/ethernet//hisilicon/hns3/hns3vf/hclgevf_mbx.c:4:
   drivers/net/ethernet//hisilicon/hns3/hns3vf/hclgevf_mbx.c: In function 'hclgevf_mbx_handler':
>> drivers/net/ethernet//hisilicon/hns3/hns3vf/hclgevf_mbx.c:155:17: error: implicit declaration of function 'hnae3_get_bit'; did you mean 'hnae_get_bit'? [-Werror=implicit-function-declaration]
      if (unlikely(!hnae3_get_bit(flag, HCLGEVF_CMDQ_RX_OUTVLD_B))) {
                    ^~~~~~~~~~~~~
   include/linux/compiler.h:77:42: note: in definition of macro 'unlikely'
    # define unlikely(x) __builtin_expect(!!(x), 0)
                                             ^
   cc1: some warnings being treated as errors

vim +155 drivers/net/ethernet//hisilicon/hns3/hns3vf/hclgevf_mbx.c

     3	
   > 4	#include "hclge_mbx.h"
     5	#include "hclgevf_main.h"
     6	#include "hnae3.h"
     7	
     8	static void hclgevf_reset_mbx_resp_status(struct hclgevf_dev *hdev)
     9	{
    10		/* this function should be called with mbx_resp.mbx_mutex held
    11		 * to prtect the received_response from race condition
    12		 */
    13		hdev->mbx_resp.received_resp  = false;
    14		hdev->mbx_resp.origin_mbx_msg = 0;
    15		hdev->mbx_resp.resp_status    = 0;
    16		memset(hdev->mbx_resp.additional_info, 0, HCLGE_MBX_MAX_RESP_DATA_SIZE);
    17	}
    18	
    19	/* hclgevf_get_mbx_resp: used to get a response from PF after VF sends a mailbox
    20	 * message to PF.
    21	 * @hdev: pointer to struct hclgevf_dev
    22	 * @resp_msg: pointer to store the original message type and response status
    23	 * @len: the resp_msg data array length.
    24	 */
    25	static int hclgevf_get_mbx_resp(struct hclgevf_dev *hdev, u16 code0, u16 code1,
    26					u8 *resp_data, u16 resp_len)
    27	{
    28	#define HCLGEVF_MAX_TRY_TIMES	500
    29	#define HCLGEVF_SLEEP_USCOEND	1000
    30		struct hclgevf_mbx_resp_status *mbx_resp;
    31		u16 r_code0, r_code1;
    32		int i = 0;
    33	
    34		if (resp_len > HCLGE_MBX_MAX_RESP_DATA_SIZE) {
    35			dev_err(&hdev->pdev->dev,
    36				"VF mbx response len(=%d) exceeds maximum(=%d)\n",
    37				resp_len,
    38				HCLGE_MBX_MAX_RESP_DATA_SIZE);
    39			return -EINVAL;
    40		}
    41	
    42		while ((!hdev->mbx_resp.received_resp) && (i < HCLGEVF_MAX_TRY_TIMES)) {
    43			udelay(HCLGEVF_SLEEP_USCOEND);
    44			i++;
    45		}
    46	
    47		if (i >= HCLGEVF_MAX_TRY_TIMES) {
    48			dev_err(&hdev->pdev->dev,
    49				"VF could not get mbx resp(=%d) from PF in %d tries\n",
    50				hdev->mbx_resp.received_resp, i);
    51			return -EIO;
    52		}
    53	
    54		mbx_resp = &hdev->mbx_resp;
    55		r_code0 = (u16)(mbx_resp->origin_mbx_msg >> 16);
    56		r_code1 = (u16)(mbx_resp->origin_mbx_msg & 0xff);
    57	
    58		if (mbx_resp->resp_status)
    59			return mbx_resp->resp_status;
    60	
    61		if (resp_data)
    62			memcpy(resp_data, &mbx_resp->additional_info[0], resp_len);
    63	
    64		hclgevf_reset_mbx_resp_status(hdev);
    65	
    66		if (!(r_code0 == code0 && r_code1 == code1 && !mbx_resp->resp_status)) {
    67			dev_err(&hdev->pdev->dev,
    68				"VF could not match resp code(code0=%d,code1=%d), %d",
    69				code0, code1, mbx_resp->resp_status);
    70			return -EIO;
    71		}
    72	
    73		return 0;
    74	}
    75	
    76	int hclgevf_send_mbx_msg(struct hclgevf_dev *hdev, u16 code, u16 subcode,
    77				 const u8 *msg_data, u8 msg_len, bool need_resp,
    78				 u8 *resp_data, u16 resp_len)
    79	{
    80		struct hclge_mbx_vf_to_pf_cmd *req;
    81		struct hclgevf_desc desc;
    82		int status;
    83	
    84		req = (struct hclge_mbx_vf_to_pf_cmd *)desc.data;
    85	
    86		/* first two bytes are reserved for code & subcode */
    87		if (msg_len > (HCLGE_MBX_MAX_MSG_SIZE - 2)) {
    88			dev_err(&hdev->pdev->dev,
    89				"VF send mbx msg fail, msg len %d exceeds max len %d\n",
    90				msg_len, HCLGE_MBX_MAX_MSG_SIZE);
    91			return -EINVAL;
    92		}
    93	
    94		hclgevf_cmd_setup_basic_desc(&desc, HCLGEVF_OPC_MBX_VF_TO_PF, false);
    95		req->msg[0] = code;
    96		req->msg[1] = subcode;
    97		memcpy(&req->msg[2], msg_data, msg_len);
    98	
    99		/* synchronous send */
   100		if (need_resp) {
   101			mutex_lock(&hdev->mbx_resp.mbx_mutex);
   102			hclgevf_reset_mbx_resp_status(hdev);
   103			status = hclgevf_cmd_send(&hdev->hw, &desc, 1);
   104			if (status) {
   105				dev_err(&hdev->pdev->dev,
   106					"VF failed(=%d) to send mbx message to PF\n",
   107					status);
   108				mutex_unlock(&hdev->mbx_resp.mbx_mutex);
   109				return status;
   110			}
   111	
   112			status = hclgevf_get_mbx_resp(hdev, code, subcode, resp_data,
   113						      resp_len);
   114			mutex_unlock(&hdev->mbx_resp.mbx_mutex);
   115		} else {
   116			/* asynchronous send */
   117			status = hclgevf_cmd_send(&hdev->hw, &desc, 1);
   118			if (status) {
   119				dev_err(&hdev->pdev->dev,
   120					"VF failed(=%d) to send mbx message to PF\n",
   121					status);
   122				return status;
   123			}
   124		}
   125	
   126		return status;
   127	}
   128	
   129	static bool hclgevf_cmd_crq_empty(struct hclgevf_hw *hw)
   130	{
   131		u32 tail = hclgevf_read_dev(hw, HCLGEVF_NIC_CRQ_TAIL_REG);
   132	
   133		return tail == hw->cmq.crq.next_to_use;
   134	}
   135	
   136	void hclgevf_mbx_handler(struct hclgevf_dev *hdev)
   137	{
   138		struct hclgevf_mbx_resp_status *resp;
   139		struct hclge_mbx_pf_to_vf_cmd *req;
   140		struct hclgevf_cmq_ring *crq;
   141		struct hclgevf_desc *desc;
   142		u16 *msg_q;
   143		u16 flag;
   144		u8 *temp;
   145		int i;
   146	
   147		resp = &hdev->mbx_resp;
   148		crq = &hdev->hw.cmq.crq;
   149	
   150		while (!hclgevf_cmd_crq_empty(&hdev->hw)) {
   151			desc = &crq->desc[crq->next_to_use];
   152			req = (struct hclge_mbx_pf_to_vf_cmd *)desc->data;
   153	
   154			flag = le16_to_cpu(crq->desc[crq->next_to_use].flag);
 > 155			if (unlikely(!hnae3_get_bit(flag, HCLGEVF_CMDQ_RX_OUTVLD_B))) {
   156				dev_warn(&hdev->pdev->dev,
   157					 "dropped invalid mailbox message, code = %d\n",
   158					 req->msg[0]);
   159	
   160				/* dropping/not processing this invalid message */
   161				crq->desc[crq->next_to_use].flag = 0;
   162				hclge_mbx_ring_ptr_move_crq(crq);
   163				continue;
   164			}
   165	
   166			/* synchronous messages are time critical and need preferential
   167			 * treatment. Therefore, we need to acknowledge all the sync
   168			 * responses as quickly as possible so that waiting tasks do not
   169			 * timeout and simultaneously queue the async messages for later
   170			 * prcessing in context of mailbox task i.e. the slow path.
   171			 */
   172			switch (req->msg[0]) {
   173			case HCLGE_MBX_PF_VF_RESP:
   174				if (resp->received_resp)
   175					dev_warn(&hdev->pdev->dev,
   176						 "VF mbx resp flag not clear(%d)\n",
   177						 req->msg[1]);
   178				resp->received_resp = true;
   179	
   180				resp->origin_mbx_msg = (req->msg[1] << 16);
   181				resp->origin_mbx_msg |= req->msg[2];
   182				resp->resp_status = req->msg[3];
   183	
   184				temp = (u8 *)&req->msg[4];
   185				for (i = 0; i < HCLGE_MBX_MAX_RESP_DATA_SIZE; i++) {
   186					resp->additional_info[i] = *temp;
   187					temp++;
   188				}
   189				break;
   190			case HCLGE_MBX_LINK_STAT_CHANGE:
   191			case HCLGE_MBX_ASSERTING_RESET:
   192				/* set this mbx event as pending. This is required as we
   193				 * might loose interrupt event when mbx task is busy
   194				 * handling. This shall be cleared when mbx task just
   195				 * enters handling state.
   196				 */
   197				hdev->mbx_event_pending = true;
   198	
   199				/* we will drop the async msg if we find ARQ as full
   200				 * and continue with next message
   201				 */
   202				if (hdev->arq.count >= HCLGE_MBX_MAX_ARQ_MSG_NUM) {
   203					dev_warn(&hdev->pdev->dev,
   204						 "Async Q full, dropping msg(%d)\n",
   205						 req->msg[1]);
   206					break;
   207				}
   208	
   209				/* tail the async message in arq */
   210				msg_q = hdev->arq.msg_q[hdev->arq.tail];
   211				memcpy(&msg_q[0], req->msg, HCLGE_MBX_MAX_ARQ_MSG_SIZE);
   212				hclge_mbx_tail_ptr_move_arq(hdev->arq);
   213				hdev->arq.count++;
   214	
   215				hclgevf_mbx_task_schedule(hdev);
   216	
   217				break;
   218			default:
   219				dev_err(&hdev->pdev->dev,
   220					"VF received unsupported(%d) mbx msg from PF\n",
   221					req->msg[0]);
   222				break;
   223			}
   224			crq->desc[crq->next_to_use].flag = 0;
   225			hclge_mbx_ring_ptr_move_crq(crq);
   226		}
   227	
   228		/* Write back CMDQ_RQ header pointer, M7 need this pointer */
   229		hclgevf_write_dev(&hdev->hw, HCLGEVF_NIC_CRQ_HEAD_REG,
   230				  crq->next_to_use);
   231	}
   232	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 53124 bytes --]

RE: [PATCH net-next 2/3] net: hns3: Fix for VF mailbox receiving unknown message

[Salil Mehta]

Hi Dave,
Sorry about this. I have fixed this in the V2 patch floated just now.

Best regards
Salil.
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Tuesday, June 05, 2018 7:41 PM
> To: Salil Mehta <salil.mehta@huawei.com>
> Cc: Zhuangyuzeng (Yisen) <yisen.zhuang@huawei.com>; lipeng (Y)
> <lipeng321@huawei.com>; mehta.salil@opnsrc.net; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org; Linuxarm <linuxarm@huawei.com>; wangxi
> (M) <wangxi11@huawei.com>
> Subject: Re: [PATCH net-next 2/3] net: hns3: Fix for VF mailbox
> receiving unknown message
> 
> From: Salil Mehta <salil.mehta@huawei.com>
> Date: Tue, 5 Jun 2018 12:42:00 +0100
> 
> > +		if (unlikely(!hnae3_get_bit(flag,
> HCLGEVF_CMDQ_RX_OUTVLD_B))) {
> 
> This breaks the build, there is no such symbol named hnae3_get_bit().