> -----Original Message----- > From: Brian Vazquez <[email protected]> > Sent: Friday, October 9, 2026 8:16 PM > To: Brian Vazquez <[email protected]>; Nguyen, Anthony L > <[email protected]>; Kitszel, Przemyslaw > <[email protected]>; Andrew Lunn <[email protected]>; > David S. Miller <[email protected]>; Eric Dumazet > <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni > <[email protected]>; [email protected]; > [email protected] > Cc: Chittim, Madhu <[email protected]>; Hay, Joshua A > <[email protected]>; Samudrala, Sridhar > <[email protected]>; [email protected]; David > Decotigny <[email protected]>; Li Li <[email protected]>; Tantilov, Emil > S <[email protected]>; Brian Vazquez <[email protected]>; Paul > Menzel <[email protected]> > 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 <[email protected]> > Reviewed-by: Paul Menzel <[email protected]> > Reviewed-by: Li Li <[email protected]> > Signed-off-by: Brian Vazquez <[email protected]> > --- > 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 <[email protected]>
