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

[Loktionov, Aleksandr]

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