RE: [PATCH iwl-net v2] idpf: wait for the reset completed state in idpf_check_reset_complete()

[Salin, Samuel]

> -----Original Message-----
> From: Loktionov, Aleksandr <aleksandr.loktionov@xxxxxxxxx>
> Sent: Wednesday, October 14, 2026 3:47 AM
> To: Brian Vazquez <brianvv@xxxxxxxxxx>; Brian Vazquez
> <brianvv.kernel@xxxxxxxxx>; Nguyen, Anthony L
> <anthony.l.nguyen@xxxxxxxxx>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@xxxxxxxxx>; Andrew Lunn <andrew+netdev@xxxxxxx>;
> David S. Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet
> <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni
> <pabeni@xxxxxxxxxx>; intel-wired-lan@xxxxxxxxxxxxxxxx;
> netdev@xxxxxxxxxxxxxxx
> Cc: Chittim, Madhu <madhu.chittim@xxxxxxxxx>; Hay, Joshua A
> <joshua.a.hay@xxxxxxxxx>; Samudrala, Sridhar
> <sridhar.samudrala@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; David
> Decotigny <decot@xxxxxxxxxx>; Li Li <boolli@xxxxxxxxxx>; Tantilov, Emil S
> <emil.s.tantilov@xxxxxxxxx>; Paul Menzel <pmenzel@xxxxxxxxxxxxx>
> Subject: RE: [PATCH iwl-net v2] idpf: wait for the reset completed state in
> idpf_check_reset_complete()
> 
> 
> 
> > -----Original Message-----
> > From: Brian Vazquez <brianvv@xxxxxxxxxx>
> > Sent: Friday, October 9, 2026 8:16 PM
> > To: Brian Vazquez <brianvv.kernel@xxxxxxxxx>; Nguyen, Anthony L
> > <anthony.l.nguyen@xxxxxxxxx>; Kitszel, Przemyslaw
> > <przemyslaw.kitszel@xxxxxxxxx>; Andrew Lunn <andrew+netdev@xxxxxxx>;
> > David S. Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet
> > <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni
> > <pabeni@xxxxxxxxxx>; intel-wired-lan@xxxxxxxxxxxxxxxx;
> > netdev@xxxxxxxxxxxxxxx
> > Cc: Chittim, Madhu <madhu.chittim@xxxxxxxxx>; Hay, Joshua A
> > <joshua.a.hay@xxxxxxxxx>; Samudrala, Sridhar
> > <sridhar.samudrala@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; David
> > Decotigny <decot@xxxxxxxxxx>; Li Li <boolli@xxxxxxxxxx>; Tantilov,
> > Emil S <emil.s.tantilov@xxxxxxxxx>; Brian Vazquez
> > <brianvv@xxxxxxxxxx>; Paul Menzel <pmenzel@xxxxxxxxxxxxx>
> > Subject: [PATCH iwl-net v2] idpf: wait for the reset completed state
> > in
> > idpf_check_reset_complete()
> >
> > PFGEN_RSTAT[1:0] / VFGEN_RSTAT[1:0] (PFR_STATE / VFR_STATE) encode
> the
> > function reset progress as a 2-bit state, not a flag. From the IDPF
> > specification [1], "PF Reset Status - PFGEN_RSTAT":
> >
> >   00b - reset in progress
> >   01b - reset completed, written by the control plane
> >   10b - function active, written by the control plane once the function
> >         has completed VIRTCHNL2_OP_VERSION
> >   11b - reserved
> >
> > The "Function Level Reset" chapter of the same document has the driver
> > poll the register until it reads 01b before re-initializing.
> >
> > idpf_check_reset_complete() tests (reg_val & rstat_m) as a boolean,
> > with rstat_m = GENMASK(1, 0), so any non-zero state is taken as
> > "completed", although only 01b carries that meaning.
> >
> > That only works if hardware moves the field to 00b as soon as the
> > reset is triggered. On the Intel IPU E2100 (8086:1452) it does not:
> > PFSWR transitions PFR_STATE to 00b only when the field currently reads
> 01b.
> > When the function is active (10b) the field keeps its value until the
> > control plane finishes the reset and writes 01b, which takes a few
> > hundred milliseconds.
> >
> > Consequently, on a hard reset of an active function
> > (IDPF_HR_FUNC_RESET) the first poll sees 10b, returns immediately, the
> > driver re-creates the mailbox and sends VIRTCHNL2_OP_VERSION while the
> > reset is still being processed. The control plane tears the mailbox
> > down underneath it, VERSION is never answered, and the driver sits in
> > a
> > 60 second virtchnl timeout (-ETIME) before going through a second hard
> > reset which then succeeds.
> >
> > An FLR on an active PF is enough to reproduce it:
> >
> >   # echo 1 > /sys/bus/pci/devices/0000:01:00.0/reset
> >
> >   [  299.685030] idpf 0000:01:00.0: HW reset detected
> >   [  299.715710] idpf 0000:01:00.0: Device HW Reset initiated
> >   [  362.916087] idpf 0000:01:00.0: Device HW Reset initiated
> >   [  364.318246] eth0: (slave eth1): link status definitely up, ...
> >
> > Nothing is logged during the 63 seconds; the timeout is silent on this
> > path.
> >
> > Make idpf_check_reset_complete() wait for the state to be exactly 01b.
> > This is the stricter reading of the register definition and it does
> > not depend on how the field got to its current value: functions that
> > are still in progress (00b) or active (10b) keep polling until the
> > control plane reports completion.
> >
> > With the change the same FLR results in a single reset and the link is
> > back after 1.5 seconds:
> >
> >   [  206.856635] idpf 0000:01:00.0: HW reset detected
> >   [  206.887106] idpf 0000:01:00.0: Device HW Reset initiated
> >   [  208.368721] eth0: (slave eth1): link status definitely up, ...
> >
> > [1]
> > https://github.com/oasis-tcs/idpf-
> > specification/blob/main/idpf_specification.md
> >
> > Fixes: 8077c727561a ("idpf: add controlq init and reset checks")
> > Reviewed-by: Emil Tantilov <emil.s.tantilov@xxxxxxxxx>
> > Reviewed-by: Paul Menzel <pmenzel@xxxxxxxxxxxxx>
> > Reviewed-by: Li Li <boolli@xxxxxxxxxx>
> > Signed-off-by: Brian Vazquez <brianvv@xxxxxxxxxx>
> > ---
> > v2: reference the IDPF spec, name the device and add an FLR reproducer
> >     with before/after (Paul); collect Reviewed-by tags. No code change.
> >
> >  drivers/net/ethernet/intel/idpf/idpf.h     | 2 ++
> >  drivers/net/ethernet/intel/idpf/idpf_lib.c | 3 ++-
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf.h
> > b/drivers/net/ethernet/intel/idpf/idpf.h
> > index 470bc23c844c..ce8a1bd0ec8b 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf.h
> > +++ b/drivers/net/ethernet/intel/idpf/idpf.h
> > @@ -166,6 +166,8 @@ struct idpf_netdev_priv {
> >  	spinlock_t stats_lock;
> >  };
> >
> > +#define IDPF_RSTAT_COMPLETE		0x01
> > +
> >  /**
> >   * struct idpf_reset_reg - Reset register offsets/masks
> >   * @rstat: Reset status register
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c
> > b/drivers/net/ethernet/intel/idpf/idpf_lib.c
> > index 827c795afcb6..92eb3e9de77c 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
> > @@ -1874,7 +1874,8 @@ static int idpf_check_reset_complete(struct
> > idpf_adapter *adapter,
> >  		 * register for us yet and 0xFFFFFFFF is not a valid value for
> >  		 * the register, so treat that as invalid.
> >  		 */
> > -		if (reg_val != 0xFFFFFFFF && (reg_val & reset_reg-
> > >rstat_m))
> > +		if (reg_val != 0xFFFFFFFF &&
> > +		    (reg_val & reset_reg->rstat_m) == IDPF_RSTAT_COMPLETE)
> >  			return 0;
> >
> >  		usleep_range(5000, 10000);
> > --
> > 2.56.0.385.gd3acb90ef8-goog
> 
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@xxxxxxxxx>

Tested-by: Samuel Salin <Samuel.salin@xxxxxxxxx>