Patchwork [dpdk-dev,v2] net/i40e: firmware status check

login
register
mail settings
Submitter Xiaoyun Li
Date Dec. 3, 2018, 7:07 a.m.
Message ID <1543820866-3644-1-git-send-email-xiaoyun.li@intel.com>
Download mbox | patch
Permalink /patch/670231/
State New
Headers show

Comments

Xiaoyun Li - Dec. 3, 2018, 7:07 a.m.
Check the firmware status at init time. If the firmware is in
recovery mode, alert the user to check it.

Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
---
v2:
 * Rebase to the newest codes.
---
 drivers/net/i40e/i40e_ethdev.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
Zhang Qi - Dec. 3, 2018, 8:07 a.m.
> -----Original Message-----
> From: Li, Xiaoyun
> Sent: Monday, December 3, 2018 3:08 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>
> Subject: [PATCH v2] net/i40e: firmware status check
> 
> Check the firmware status at init time. If the firmware is in recovery mode, alert
> the user to check it.
> 
> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi
Kevin Traynor - Dec. 14, 2018, 4:59 p.m.
On 12/03/2018 08:07 AM, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: Li, Xiaoyun
>> Sent: Monday, December 3, 2018 3:08 PM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>
>> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>
>> Subject: [PATCH v2] net/i40e: firmware status check
>>
>> Check the firmware status at init time. If the firmware is in recovery mode, alert
>> the user to check it.
>>
>> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
> 
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> Applied to dpdk-next-net-intel.
> 

This was applied with a 'Cc:stable' tag, but no 'Fixes' tag.
What stable branches is it relevant for?

> Thanks
> Qi
> 
>
Ferruh Yigit - Dec. 17, 2018, 10:57 a.m.
On 12/14/2018 4:59 PM, Kevin Traynor wrote:
> On 12/03/2018 08:07 AM, Zhang, Qi Z wrote:
>>
>>
>>> -----Original Message-----
>>> From: Li, Xiaoyun
>>> Sent: Monday, December 3, 2018 3:08 PM
>>> To: Zhang, Qi Z <qi.z.zhang@intel.com>
>>> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>
>>> Subject: [PATCH v2] net/i40e: firmware status check
>>>
>>> Check the firmware status at init time. If the firmware is in recovery mode, alert
>>> the user to check it.
>>>
>>> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
>>
>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
>>
>> Applied to dpdk-next-net-intel.
>>
> 
> This was applied with a 'Cc:stable' tag, but no 'Fixes' tag.

This fixes a behavior in the driver, but not a specific code/commit, and author
request this behavior change to backport. This request makes sense to me but
what do you think from stable tree point of view? Are you OK with this kind of
request?

> What stable branches is it relevant for?

I agree it is hard to define the scope of the fix without having the code that
is fixed. Do you have any suggestion how to formalize the request for these kind
of issues?

> 
>> Thanks
>> Qi
>>
>>
>
Kevin Traynor - Dec. 17, 2018, 4:39 p.m.
On 12/17/2018 10:57 AM, Ferruh Yigit wrote:
> On 12/14/2018 4:59 PM, Kevin Traynor wrote:
>> On 12/03/2018 08:07 AM, Zhang, Qi Z wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Li, Xiaoyun
>>>> Sent: Monday, December 3, 2018 3:08 PM
>>>> To: Zhang, Qi Z <qi.z.zhang@intel.com>
>>>> Cc: dev@dpdk.org; Li, Xiaoyun <xiaoyun.li@intel.com>
>>>> Subject: [PATCH v2] net/i40e: firmware status check
>>>>
>>>> Check the firmware status at init time. If the firmware is in recovery mode, alert
>>>> the user to check it.
>>>>
>>>> Signed-off-by: Xiaoyun Li <xiaoyun.li@intel.com>
>>>
>>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
>>>
>>> Applied to dpdk-next-net-intel.
>>>
>>
>> This was applied with a 'Cc:stable' tag, but no 'Fixes' tag.
> 
> This fixes a behavior in the driver, but not a specific code/commit, and author
> request this behavior change to backport. This request makes sense to me but
> what do you think from stable tree point of view? Are you OK with this kind of
> request?
> 

It makes sense to me also to backport, I guess it can be seen as a fix
for the original pmd which was missing this code:

Fixes: 4861cde46116 ("i40e: new poll mode driver")

or if only relevant since some base driver/firmware change, then Fixes:
from that update.

>> What stable branches is it relevant for?
> 
> I agree it is hard to define the scope of the fix without having the code that
> is fixed. Do you have any suggestion how to formalize the request for these kind
> of issues?
> 

I tend to think if it's a *fix*, then some code was previously added
that was incorrect, or had a missing piece, or became incorrect at some
point due to another other change, so 'Fixes:' should almost always be
possible.

However, if for some reason it's not clear and there's not too many,
then a simple solution is to reply to thread (cc'ing stable) saying
which which stable branches it is relevant for. I will check the thread
for info when I see a patch like that. 'Fixes:' is much preferred
though, so not to have manual checking of email threads.

Kevin.

>>
>>> Thanks
>>> Qi
>>>
>>>
>>
>

Patch

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index fea42b0..93cc17b 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1273,7 +1273,7 @@  eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct i40e_vsi *vsi;
 	int ret;
-	uint32_t len;
+	uint32_t len, val;
 	uint8_t aq_fail = 0;
 
 	PMD_INIT_FUNC_TRACE();
@@ -1324,6 +1324,15 @@  eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 	 */
 	hw->switch_tag = 0xffff;
 
+	val = I40E_READ_REG(hw, I40E_GL_FWSTS);
+	if (val & I40E_GL_FWSTS_FWS1B_MASK) {
+		PMD_INIT_LOG(ERR, "\nERROR: "
+			"Firmware recovery mode detected. Limiting functionality.\n"
+			"Refer to the Intel(R) Ethernet Adapters and Devices "
+			"User Guide for details on firmware recovery mode.");
+		return -EIO;
+	}
+
 	/* Check if need to support multi-driver */
 	i40e_support_multi_driver(dev);
 	/* Check if users want the latest supported vec path */