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

[Li Li]

Reviewed-by: Li Li <boolli@xxxxxxxxxx>


On Thu, Oct 8, 2026 at 10:33 AM Brian Vazquez <brianvv@xxxxxxxxxx> wrote:
>
> 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:
>
>   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
>
> 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 devices tested 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.
>
> 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.
>
> Fixes: 8077c727561a ("idpf: add controlq init and reset checks")
> Signed-off-by: Brian Vazquez <brianvv@xxxxxxxxxx>
> ---
>  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 2c148377540c..a827e0fa67c8 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
> @@ -1870,7 +1870,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
>