Re: [PATCH iwl-next v2 2/2] idpf: implement pci error handlers

["Tantilov, Emil S" <emil.s.tantilov@intel.com>]

On 4/14/2026 4:09 AM, Loktionov, Aleksandr wrote:
> 
> 
>> -----Original Message-----
>> From: Tantilov, Emil S <emil.s.tantilov@intel.com>
>> Sent: Tuesday, April 14, 2026 5:17 AM
>> To: intel-wired-lan@lists.osuosl.org
>> Cc: netdev@vger.kernel.org; Kitszel, Przemyslaw
>> <przemyslaw.kitszel@intel.com>; Bhat, Jay <jay.bhat@intel.com>;
>> Barrera, Ivan D <ivan.d.barrera@intel.com>; Loktionov, Aleksandr
>> <aleksandr.loktionov@intel.com>; Zaremba, Larysa
>> <larysa.zaremba@intel.com>; Nguyen, Anthony L
>> <anthony.l.nguyen@intel.com>; andrew+netdev@lunn.ch;
>> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
>> pabeni@redhat.com; Lobakin, Aleksander <aleksander.lobakin@intel.com>;
>> linux-pci@vger.kernel.org; Chittim, Madhu <madhu.chittim@intel.com>;
>> decot@google.com; willemb@google.com; sheenamo@google.com;
>> lukas@wunner.de
>> Subject: [PATCH iwl-next v2 2/2] idpf: implement pci error handlers
>>
>> Add callbacks to handle PCI errors and FLR reset. When preparing to
>> handle reset on the bus, the driver must stop all operations that can
>> lead to MMIO access in order to prevent HW errors. To accomplish this
>> introduce helper
>> idpf_reset_prepare() that gets called prior to FLR or when PCI error
>> is detected. Upon resume the recovery is done through the existing
>> reset path by starting the event task.
>>
>> The following callbacks are implemented:
>> .reset_prepare runs the first portion of the generic reset path
>> leading up to the part where we wait for the reset to complete.
>> .reset_done/resume runs the recovery part of the reset handling.
>> .error_detected is the callback dealing with PCI errors, similar to
>> the prepare call, we stop all operations, prior to attempting a
>> recovery.
>> .slot_reset is the callback attempting to restore the device, provided
>> a PCI reset was initiated by the AER driver.
>>
>> Whereas previously the init logic guaranteed netdevs during reset, the
>> addition of idpf_detach_and_close() to the PCI callbacks flow makes it
>> possible for the function to be called without netdevs. Add check to
>> avoid NULL pointer dereference in that case.
>>
>> Co-developed-by: Alan Brady <alan.brady@intel.com>
>> Signed-off-by: Alan Brady <alan.brady@intel.com>
>> Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
>> Reviewed-by: Jay Bhat <jay.bhat@intel.com>
>> Reviewed-by: Madhu Chittim <madhu.chittim@intel.com>
>> ---
>>   drivers/net/ethernet/intel/idpf/idpf.h      |   3 +
>>   drivers/net/ethernet/intel/idpf/idpf_lib.c  |  13 ++-
>> drivers/net/ethernet/intel/idpf/idpf_main.c | 112 ++++++++++++++++++++
>>   3 files changed, 126 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/idpf/idpf.h
>> b/drivers/net/ethernet/intel/idpf/idpf.h
>> index 1d0e32e47e87..164d2f3e233a 100644
>> --- a/drivers/net/ethernet/intel/idpf/idpf.h
>> +++ b/drivers/net/ethernet/intel/idpf/idpf.h
>> @@ -88,6 +88,7 @@ enum idpf_state {
>>    * @IDPF_REMOVE_IN_PROG: Driver remove in progress
>>    * @IDPF_MB_INTR_MODE: Mailbox in interrupt mode
>>    * @IDPF_VC_CORE_INIT: virtchnl core has been init
>> + * @IDPF_PCI_CB_RESET: Reset via the PCI callbacks
>>    * @IDPF_FLAGS_NBITS: Must be last
>>    */
>>   enum idpf_flags {
>> @@ -97,6 +98,7 @@ enum idpf_flags {
>>   	IDPF_REMOVE_IN_PROG,
>>   	IDPF_MB_INTR_MODE,
>>   	IDPF_VC_CORE_INIT,
> 
> ...
> 
>> +/**
>> + * idpf_pci_err_resume - Resume operations after PCI error recovery
>> + * @pdev: PCI device struct
>> + */
>> +static void idpf_pci_err_resume(struct pci_dev *pdev) {
>> +	struct idpf_adapter *adapter = pci_get_drvdata(pdev);
>> +
>> +	/* Force a PFR when resuming from PCI error. */
>> +	if (test_and_set_bit(IDPF_PCI_CB_RESET, adapter->flags))
>> +		adapter->dev_ops.reg_ops.trigger_reset(adapter,
>> IDPF_HR_FUNC_RESET);
> You say "Force a PFR", but PFR is only triggered on the AER path, not on the FLR path.

Hence the "force" - the call to `trigger_reset` results in a PFR and is
only needed in the case of a PCI error. If this function was called
because a user issued an FLR, the kernel will trigger it for us. This
way we can reuse the reset handling path to restore the operation of the
netdevs.

Though I may be misunderstanding - are you referring to the wording or
the logic?

Thanks,
Emil

> 
> Everything else looks fine
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> 
>> +
>> +	queue_delayed_work(adapter->vc_event_wq,
>> +			   &adapter->vc_event_task,
>> +			   msecs_to_jiffies(300));
>> +}
> 
> ...
> 
>>   };
>>   module_pci_driver(idpf_driver);
>> --
>> 2.37.3
>