* Re: [iwl-net PATCH] idpf: wait for the reset completed state in idpf_check_reset_complete()
2026-10-08 17:33 [iwl-net PATCH] idpf: wait for the reset completed state in idpf_check_reset_complete() Brian Vazquez
@ 2026-10-08 17:39 ` netdev-bot+sinfo
2026-10-08 17:59 ` Tantilov, Emil S
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: netdev-bot+sinfo @ 2026-10-08 17:39 UTC (permalink / raw)
To: Brian Vazquez
Cc: Brian Vazquez, Tony Nguyen, Przemek Kitszel, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
intel-wired-lan, netdev, Madhu Chittim, Pavan Kumar Linga,
Joshua Hay, Shailendra Bhatnagar, Sridhar Samudrala,
linux-kernel, David Decotigny, Li Li, Emil Tantilov
Hi!
This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:
- What hardware the change was tested on. For driver fixes please
mention the device (and if relevant firmware version) used for
testing, or say that the change was not tested on real hardware.
Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.
The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [iwl-net PATCH] idpf: wait for the reset completed state in idpf_check_reset_complete()
2026-10-08 17:33 [iwl-net PATCH] idpf: wait for the reset completed state in idpf_check_reset_complete() Brian Vazquez
2026-10-08 17:39 ` netdev-bot+sinfo
@ 2026-10-08 17:59 ` Tantilov, Emil S
2026-10-08 18:31 ` Paul Menzel
2026-10-08 19:15 ` Li Li
3 siblings, 0 replies; 6+ messages in thread
From: Tantilov, Emil S @ 2026-10-08 17:59 UTC (permalink / raw)
To: Brian Vazquez, Brian Vazquez, Tony Nguyen, Przemek Kitszel,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, intel-wired-lan, netdev
Cc: Madhu Chittim, Pavan Kumar Linga, Joshua Hay,
Shailendra Bhatnagar, Sridhar Samudrala, linux-kernel,
David Decotigny, Li Li
On 10/8/2026 10:33 AM, Brian Vazquez 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@google.com>
> ---
> 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)
Reviewed-by: Emil Tantilov <emil.s.tantilov@intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [iwl-net PATCH] idpf: wait for the reset completed state in idpf_check_reset_complete()
2026-10-08 17:33 [iwl-net PATCH] idpf: wait for the reset completed state in idpf_check_reset_complete() Brian Vazquez
2026-10-08 17:39 ` netdev-bot+sinfo
2026-10-08 17:59 ` Tantilov, Emil S
@ 2026-10-08 18:31 ` Paul Menzel
2026-10-08 20:43 ` Brian Vazquez
2026-10-08 19:15 ` Li Li
3 siblings, 1 reply; 6+ messages in thread
From: Paul Menzel @ 2026-10-08 18:31 UTC (permalink / raw)
To: Brian Vazquez, Brian Vazquez
Cc: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, intel-wired-lan,
netdev, Madhu Chittim, Pavan Kumar Linga, Joshua Hay,
Shailendra Bhatnagar, Sridhar Samudrala, linux-kernel,
David Decotigny, Li Li, Emil Tantilov
Dear Brian,
Thank you for your patch.
Am 08.10.26 um 19:33 schrieb Brian Vazquez:
> 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
For me it’d be helpful if you mentioned the datasheet.
> 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.
Please mention the device, so it’s easier to reproduce. Also, the
command to trigger the reset.
> 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@google.com>
> ---
> 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);
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [iwl-net PATCH] idpf: wait for the reset completed state in idpf_check_reset_complete()
2026-10-08 18:31 ` Paul Menzel
@ 2026-10-08 20:43 ` Brian Vazquez
0 siblings, 0 replies; 6+ messages in thread
From: Brian Vazquez @ 2026-10-08 20:43 UTC (permalink / raw)
To: Paul Menzel
Cc: Brian Vazquez, Tony Nguyen, Przemek Kitszel, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
intel-wired-lan, netdev, Madhu Chittim, Joshua Hay,
Sridhar Samudrala, linux-kernel, David Decotigny, Li Li,
Emil Tantilov
Thanks for the review, Paul. I answered inline.
I will fold the spec reference, device and reproducer into v2 and
carry them forward.
the Reviewed-by tags.
On Thu, Oct 8, 2026 at 2:31 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Dear Brian,
>
>
> Thank you for your patch.
>
> Am 08.10.26 um 19:33 schrieb Brian Vazquez:
> > 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
>
> For me it’d be helpful if you mentioned the datasheet.
The state encoding is in the public IDPF specification, "PF Reset
Status - PFGEN_RSTAT" [1]:
[1] https://github.com/oasis-tcs/idpf-specification/blob/main/idpf_specification.md
>
> > 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.
>
> Please mention the device, so it’s easier to reproduce. Also, the
> command to trigger the reset.
Intel IPU E2100, 8086:1452. An FLR on an active PF is enough:
# 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 s; the VERSION timeout is silent on this path.
>
> > 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@google.com>
> > ---
> > 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);
>
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
>
>
> Kind regards,
>
> Paul
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [iwl-net PATCH] idpf: wait for the reset completed state in idpf_check_reset_complete()
2026-10-08 17:33 [iwl-net PATCH] idpf: wait for the reset completed state in idpf_check_reset_complete() Brian Vazquez
` (2 preceding siblings ...)
2026-10-08 18:31 ` Paul Menzel
@ 2026-10-08 19:15 ` Li Li
3 siblings, 0 replies; 6+ messages in thread
From: Li Li @ 2026-10-08 19:15 UTC (permalink / raw)
To: Brian Vazquez
Cc: Brian Vazquez, Tony Nguyen, Przemek Kitszel, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
intel-wired-lan, netdev, Madhu Chittim, Pavan Kumar Linga,
Joshua Hay, Shailendra Bhatnagar, Sridhar Samudrala,
linux-kernel, David Decotigny, Emil Tantilov
Reviewed-by: Li Li <boolli@google.com>
On Thu, Oct 8, 2026 at 10:33 AM Brian Vazquez <brianvv@google.com> 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@google.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread