On 09/27/2016 07:41 PM, Keller, Jacob E wrote:
> On Tue, 2016-09-27 at 18:14 -0300, Guilherme G. Piccoli wrote:
>> Although rare, it's possible to hit PCI error early on device
>> probe, meaning possibly some structs are not entirely initialized,
>> and some might even be completely uninitialized, leading to NULL
>> pointer dereference.
>>
>> The i40e driver currently presents a "bad" behavior if device hits
>> such early PCI error: firstly, the struct i40e_pf might not be
>> attached to pci_dev yet, leading to a NULL pointer dereference on
>> access to pf->state.
>>
>
> Oops! Nice find!
>
>> Even checking if the struct is NULL and avoiding the access in that
>> case isn't enough, since the driver cannot recover from PCI error
>> that early; in our experiments we saw multiple failures on kernel
>> log, like:
>>
>> [549.664] i40e 0007:01:00.1: Initial pf_reset failed: -15
>> [549.664] i40e: probe of 0007:01:00.1 failed with error -15
>> [...]
>> [871.644] i40e 0007:01:00.1: The driver for the device stopped
>> because the
>> device firmware failed to init. Try updating your NVM image.
>> [871.644] i40e: probe of 0007:01:00.1 failed with error -32
>> [...]
>> [872.516] i40e 0007:01:00.0: ARQ: Unknown event 0x0000 ignored
>>
>> Between the first probe failure (error -15) and the second (error
>> -32)
>> another PCI error happened due to the first bad probe. Also, driver
>> started to flood console with those ARQ event messages.
>>
>> This patch will prevent these issues by allowing error recovery
>> mechanism to remove the failed device from the system instead of
>> trying to recover from early PCI errors during device probe.
>>
>
> This seems reasonable.
>
>> Signed-off-by: Guilherme G. Piccoli <gpicc...@linux.vnet.ibm.com>
>> ---
>> drivers/net/ethernet/intel/i40e/i40e_main.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> index d0b3a1b..dad15b6 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> @@ -11360,6 +11360,12 @@ static pci_ers_result_t
>> i40e_pci_error_detected(struct pci_dev *pdev,
>>
>> dev_info(&pdev->dev, "%s: error %d\n", __func__, error);
>>
>> + if (!pf) {
>> + dev_info(&pdev->dev,
>> + "Cannot recover - error happened during
>> device probe\n");
>> + return PCI_ERS_RESULT_DISCONNECT;
>> + }
>> +
>
> Looks good to me.
>
> Acked-by: Jacob Keller <jacob.e.kel...@intel.com>
>
> Thanks for the bug fix and detailed explanation!
Nice Jacob, thanks very much for the review and ack.
Cheers,
Guilherme
> Regards,
> Jake
>
>> /* shutdown all operations */
>> if (!test_bit(__I40E_SUSPENDED, &pf->state)) {
>> rtnl_lock();
