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

Loktionov, Aleksandr
2026-04-15T08:53:22Z

> -----Original Message-----
> From: Tantilov, Emil S <emil.s.tantilov@intel.com>
> Sent: Tuesday, April 14, 2026 5:01 PM
> To: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>; 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>; 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: Re: [PATCH iwl-next v2 2/2] idpf: implement pci error
> handlers
>
>
>
> 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?
From the first glance the comment looks misleading from my point of view.
Please consider rewording.

>
> 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
> >