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
From: Xi Wang <[email protected]>
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 <[email protected]>
Signed-off-by: Peng Li <[email protected]>
Signed-off-by: Salil Mehta <[email protected]>
---
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
From: Xi Wang <[email protected]>
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 <[email protected]>
Signed-off-by: Peng Li <[email protected]>
Signed-off-by: Salil Mehta <[email protected]>
---
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
From: Xi Wang <[email protected]>
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 <[email protected]>
Signed-off-by: Peng Li <[email protected]>
Signed-off-by: Salil Mehta <[email protected]>
---
.../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
From: Salil Mehta <[email protected]>
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().
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
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:[email protected]]
> Sent: Tuesday, June 05, 2018 7:41 PM
> To: Salil Mehta <[email protected]>
> Cc: Zhuangyuzeng (Yisen) <[email protected]>; lipeng (Y)
> <[email protected]>; [email protected]; [email protected];
> [email protected]; Linuxarm <[email protected]>; wangxi
> (M) <[email protected]>
> Subject: Re: [PATCH net-next 2/3] net: hns3: Fix for VF mailbox
> receiving unknown message
>
> From: Salil Mehta <[email protected]>
> 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().