[PATCH iwl-net] igc: Disable PTM after the last device reset on remove

[PATCH iwl-net] igc: Disable PTM after the last device reset on remove

[Li Yifan]

I225/I226 have a hardware issue: after a PTM dialog, the next device
reset through the RST bit of the Device Control register (CTRL.RST)
can make the device send a PTM Request. The device sends it even if
PTM is disabled in its PTM Control register (PCI config space), the EN
bit of its PTM_CTRL register (MMIO) is clear and Bus Master Enable is
clear, so the driver cannot stop it. Commit c080fe262f9e ("igc: Fix
possible system crash when loading module") disables PTM before bus
mastering because of a related issue (PTM Requests with bus mastering
disabled), but that does not stop the request at CTRL.RST.

The request is harmless while the upstream port has PTM enabled. Since
commit e1092d5e15e6 ("PCI/PTM: Do not enable PTM automatically for Root
and Switch Upstream Ports"), pci_disable_ptm() also disables PTM on the
upstream port when the last user goes away. igc_remove() calls
pci_disable_ptm() before unregister_netdev(). If the interface is up,
unregister_netdev() then resets the device through igc_close() ->
igc_down() -> igc_reset(). The upstream port now handles the PTM
Request as an Unsupported Request. On an Intel Nova Lake-S system with
an I226-LM, this triggers DPC on "modprobe -r igc". Recovery fails
because the driver is unbinding, and the next probe fails:

  pcieport 0000:80:1d.0: DPC: containment event, status:0x1f01: unmasked uncorrectable error detected
  pcieport 0000:80:1d.0:    [20] UnsupReq               (First)
  pcieport 0000:80:1d.0: AER:   TLP Header: 0x34000000 0x81000052 0x00000000 0x00000000

Message code 0x52 in the TLP header is a PTM Request from 81:00.0.

Reset the device while the upstream port still has PTM enabled, so
that the PTM Request gets a response. Then disable PTM and bus
mastering. With this order, a later reset sends no PTM Request (tested
on I226-LM). igc_ptp_stop() has already cleared IGC_PTP_ENABLED, so no
new PTM dialog starts after this reset. If the interface is down,
unregister_netdev() does not reset the device, so reset it explicitly.
PTM is still disabled before bus mastering, as the related fix above
requires.

Skip the reset if the device cannot respond: error recovery is in
progress or has failed (pci_channel_offline()), the device is gone
(pci_device_is_present()), or igc_rd32() has already reported "PCIe
link lost" (IGC_REMOVED()). Otherwise the reset accesses a device in
error recovery, or polls all-ones register values for about two
seconds until igc_disable_pcie_master() times out.

Call pci_disable_ptm() only if PTM is enabled. pci_enable_ptm() can
fail in probe, for example if the upstream port does not support
PTM, and probe continues without PTM. The PCI/PTM change above
also made pci_disable_ptm() use a reference count: it decrements
dev->ptm_enable_cnt without a check, and then does the same for the
upstream port. Before, it did nothing if PTM was not enabled. After
a failed pci_enable_ptm(), the count of the device becomes negative
and stays negative after the driver unbinds. pcie_ptm_enabled()
returns true for a negative count, so after the next probe
igc_is_crosststamp_supported() reports PTM support and igc_ptp_reset()
starts PTM cycles on a device that does not have PTM enabled. Commit
650f197d8ea6 ("idpf: disable PTM on probe failure and on remove")
adds the same check to idpf.

Fixes: e1092d5e15e6 ("PCI/PTM: Do not enable PTM automatically for Root and Switch Upstream Ports")
Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Co-developed-by: Li Lili <lili.li@intel.com>
Signed-off-by: Li Lili <lili.li@intel.com>
Signed-off-by: Li Yifan <yifan2.li@intel.com>
Tested-by: Wang Jie <jie.wang@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index b8ac2f4fea5a..3ee8b05f0515 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -7406,9 +7406,6 @@ static void igc_remove(struct pci_dev *pdev)
 
 	igc_ptp_stop(adapter);
 
-	pci_disable_ptm(pdev);
-	pci_clear_master(pdev);
-
 	set_bit(__IGC_DOWN, &adapter->state);
 
 	timer_delete_sync(&adapter->watchdog_timer);
@@ -7427,6 +7424,22 @@ static void igc_remove(struct pci_dev *pdev)
 	igc_release_hw_control(adapter);
 	unregister_netdev(netdev);
 
+	/* After a PTM dialog, I225/I226 can send a PTM Request at the next
+	 * CTRL.RST, even with PTM and bus mastering disabled on the device.
+	 * Reset the device while PTM is still enabled on the upstream port,
+	 * so that the request gets a response. unregister_netdev() resets the
+	 * device only if the interface was up. Skip the reset if the device
+	 * cannot respond (error recovery, surprise removal or an earlier
+	 * "PCIe link lost"). Then disable PTM before bus mastering.
+	 */
+	if (!pci_channel_offline(pdev) && pci_device_is_present(pdev) &&
+	    !IGC_REMOVED(adapter->hw.hw_addr))
+		adapter->hw.mac.ops.reset_hw(&adapter->hw);
+
+	if (pcie_ptm_enabled(pdev))
+		pci_disable_ptm(pdev);
+	pci_clear_master(pdev);
+
 	igc_clear_interrupt_scheme(adapter);
 	pci_iounmap(pdev, adapter->io_addr);
 	pci_release_mem_regions(pdev);

base-commit: c550c1c9727726dcae121221c252013153fa5945
-- 
2.43.0

Re: [PATCH iwl-net] igc: Disable PTM after the last device reset on remove

[Bjorn Helgaas]

On Sat, Oct 31, 2026 at 10:19:28AM +0800, Li Yifan wrote:
> I225/I226 have a hardware issue: after a PTM dialog, the next device
> reset through the RST bit of the Device Control register (CTRL.RST)

I assume this Device Control is a device-specific register, not the
Device Control in the PCIe Capability, right?

> can make the device send a PTM Request. The device sends it even if
> PTM is disabled in its PTM Control register (PCI config space), the EN
> bit of its PTM_CTRL register (MMIO) is clear

And I guess PTM_CTRL is another device-specific register.  The
architected PTM Enable bit in the PTM Capability in config space is
supposed to be enough to prevent the device from issuing PTM Requests,
so I guess the device-specific control is only relevant because it's
not enough to work around this hardware defect.

> and Bus Master Enable is
> clear, so the driver cannot stop it. Commit c080fe262f9e ("igc: Fix
> possible system crash when loading module") disables PTM before bus
> mastering because of a related issue (PTM Requests with bus mastering
> disabled), but that does not stop the request at CTRL.RST.

c080fe262f9e and this both mention Bus Master Enable, but I don't
think it's relevant to either change.  Bus Master Enable controls
ability to issue Memory and I/O Read/Write Requests.  PTM Requests are
PCIe Messages (not Read/Write Requests) so Bus Master Enable should
not affect them.

If this is a documented erratum, can we include an identifier or a
pointer to it?

> The request is harmless while the upstream port has PTM enabled. Since
> commit e1092d5e15e6 ("PCI/PTM: Do not enable PTM automatically for Root
> and Switch Upstream Ports"), pci_disable_ptm() also disables PTM on the
> upstream port when the last user goes away. igc_remove() calls
> pci_disable_ptm() before unregister_netdev(). If the interface is up,
> unregister_netdev() then resets the device through igc_close() ->
> igc_down() -> igc_reset(). The upstream port now handles the PTM
> Request as an Unsupported Request. On an Intel Nova Lake-S system with
> an I226-LM, this triggers DPC on "modprobe -r igc". Recovery fails
> because the driver is unbinding, and the next probe fails:
> 
>   pcieport 0000:80:1d.0: DPC: containment event, status:0x1f01: unmasked uncorrectable error detected
>   pcieport 0000:80:1d.0:    [20] UnsupReq               (First)
>   pcieport 0000:80:1d.0: AER:   TLP Header: 0x34000000 0x81000052 0x00000000 0x00000000
> 
> Message code 0x52 in the TLP header is a PTM Request from 81:00.0.
> 
> Reset the device while the upstream port still has PTM enabled, so
> that the PTM Request gets a response. Then disable PTM and bus
> mastering. With this order, a later reset sends no PTM Request (tested
> on I226-LM). igc_ptp_stop() has already cleared IGC_PTP_ENABLED, so no
> new PTM dialog starts after this reset.

IGC_PTP_ENABLED is just a software flag, and it appears to be related
to PTP, not directly related to PTM, so there are some dots missing
between it and hardware PTM messages.

> If the interface is down,
> unregister_netdev() does not reset the device, so reset it explicitly.
> PTM is still disabled before bus mastering, as the related fix above
> requires.

Which fix requires PTM disable before bus mastering disable?  And why
is that required?

> Skip the reset if the device cannot respond: error recovery is in
> progress or has failed (pci_channel_offline()), the device is gone
> (pci_device_is_present()), or igc_rd32() has already reported "PCIe
> link lost" (IGC_REMOVED()). Otherwise the reset accesses a device in
> error recovery, or polls all-ones register values for about two
> seconds until igc_disable_pcie_master() times out.
> 
> Call pci_disable_ptm() only if PTM is enabled. pci_enable_ptm() can
> fail in probe, for example if the upstream port does not support
> PTM, and probe continues without PTM. The PCI/PTM change above
> also made pci_disable_ptm() use a reference count: it decrements
> dev->ptm_enable_cnt without a check, and then does the same for the
> upstream port. Before, it did nothing if PTM was not enabled. After
> a failed pci_enable_ptm(), the count of the device becomes negative
> and stays negative after the driver unbinds. pcie_ptm_enabled()
> returns true for a negative count, so after the next probe
> igc_is_crosststamp_supported() reports PTM support and igc_ptp_reset()
> starts PTM cycles on a device that does not have PTM enabled. Commit
> 650f197d8ea6 ("idpf: disable PTM on probe failure and on remove")
> adds the same check to idpf.

This sounds like a bug in the PCI core PTM support, i.e., maybe
pci_disable_ptm() should be checking for PTM being enabled?
Otherwise I'm afraid lots of drivers might trip over this.

> Fixes: e1092d5e15e6 ("PCI/PTM: Do not enable PTM automatically for Root and Switch Upstream Ports")
> Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Co-developed-by: Li Lili <lili.li@intel.com>
> Signed-off-by: Li Lili <lili.li@intel.com>
> Signed-off-by: Li Yifan <yifan2.li@intel.com>
> Tested-by: Wang Jie <jie.wang@intel.com>
> ---
>  drivers/net/ethernet/intel/igc/igc_main.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index b8ac2f4fea5a..3ee8b05f0515 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -7406,9 +7406,6 @@ static void igc_remove(struct pci_dev *pdev)
>  
>  	igc_ptp_stop(adapter);
>  
> -	pci_disable_ptm(pdev);
> -	pci_clear_master(pdev);
> -
>  	set_bit(__IGC_DOWN, &adapter->state);
>  
>  	timer_delete_sync(&adapter->watchdog_timer);
> @@ -7427,6 +7424,22 @@ static void igc_remove(struct pci_dev *pdev)
>  	igc_release_hw_control(adapter);
>  	unregister_netdev(netdev);
>  
> +	/* After a PTM dialog, I225/I226 can send a PTM Request at the next
> +	 * CTRL.RST, even with PTM and bus mastering disabled on the device.
> +	 * Reset the device while PTM is still enabled on the upstream port,
> +	 * so that the request gets a response. unregister_netdev() resets the
> +	 * device only if the interface was up. Skip the reset if the device
> +	 * cannot respond (error recovery, surprise removal or an earlier
> +	 * "PCIe link lost"). Then disable PTM before bus mastering.
> +	 */
> +	if (!pci_channel_offline(pdev) && pci_device_is_present(pdev) &&
> +	    !IGC_REMOVED(adapter->hw.hw_addr))

All these tests look racy.  .reset_hw() really should be able to
detect and deal with any errors that happen if it tries to access a
device that is no longer responding.

> +		adapter->hw.mac.ops.reset_hw(&adapter->hw);
> +
> +	if (pcie_ptm_enabled(pdev))
> +		pci_disable_ptm(pdev);
> +	pci_clear_master(pdev);

I don't think bus mastering has any connection to PTM, so why are we
moving pci_clear_master()?

>  	igc_clear_interrupt_scheme(adapter);
>  	pci_iounmap(pdev, adapter->io_addr);
>  	pci_release_mem_regions(pdev);
> 
> base-commit: c550c1c9727726dcae121221c252013153fa5945
> -- 
> 2.43.0
> 

RE: [PATCH iwl-net] igc: Disable PTM after the last device reset on remove

[Li, Yifan2]

Hi Bjorn,

Thanks for the review.

On Mon, Nov 02, 2026, Bjorn Helgaas wrote:
> 
> On Sat, Oct 31, 2026 at 10:19:28AM +0800, Li Yifan wrote:
> > I225/I226 have a hardware issue: after a PTM dialog, the next device
> > reset through the RST bit of the Device Control register (CTRL.RST)
> 
> I assume this Device Control is a device-specific register, not the Device Control
> in the PCIe Capability, right?

Yes. It is the device-specific CTRL register in MMIO space (IGC_CTRL,
offset 0x0, RST is bit 26). I took the name "Device Control" from the
comment for IGC_CTRL in igc_regs.h. v2 will call it "the
device-specific CTRL register".

> 
> > can make the device send a PTM Request. The device sends it even if
> > PTM is disabled in its PTM Control register (PCI config space), the EN
> > bit of its PTM_CTRL register (MMIO) is clear
> 
> And I guess PTM_CTRL is another device-specific register.  The architected PTM
> Enable bit in the PTM Capability in config space is supposed to be enough to
> prevent the device from issuing PTM Requests, so I guess the device-specific
> control is only relevant because it's not enough to work around this hardware
> defect.

Yes. PTM_CTRL is also in MMIO space (offset 0x12540, EN is bit 30).
The defect is that the device sends a PTM Request while the PTM Enable
bit in its PTM Capability is clear. When the request comes,
pci_disable_ptm() has already cleared that bit. I mentioned
PTM_CTRL.EN only to show that the driver has no other control that
stops the request. v2 will say this directly.

> 
> > and Bus Master Enable is
> > clear, so the driver cannot stop it. Commit c080fe262f9e ("igc: Fix
> > possible system crash when loading module") disables PTM before bus
> > mastering because of a related issue (PTM Requests with bus mastering
> > disabled), but that does not stop the request at CTRL.RST.
> 
> c080fe262f9e and this both mention Bus Master Enable, but I don't think it's
> relevant to either change.  Bus Master Enable controls ability to issue Memory
> and I/O Read/Write Requests.  PTM Requests are PCIe Messages (not
> Read/Write Requests) so Bus Master Enable should not affect them.

Agreed. Our data confirms that the I226 sent the request with Bus Master
Enable clear. v2 will remove Bus Master Enable from the description.

> 
> If this is a documented erratum, can we include an identifier or a pointer to it?

I am working with the I225/I226 hardware team to find out if there is an
erratum for this. If there is a public one, I will cite it in v2.

> 
> > The request is harmless while the upstream port has PTM enabled. Since
> > commit e1092d5e15e6 ("PCI/PTM: Do not enable PTM automatically for
> > Root and Switch Upstream Ports"), pci_disable_ptm() also disables PTM
> > on the upstream port when the last user goes away. igc_remove() calls
> > pci_disable_ptm() before unregister_netdev(). If the interface is up,
> > unregister_netdev() then resets the device through igc_close() ->
> > igc_down() -> igc_reset(). The upstream port now handles the PTM
> > Request as an Unsupported Request. On an Intel Nova Lake-S system with
> > an I226-LM, this triggers DPC on "modprobe -r igc". Recovery fails
> > because the driver is unbinding, and the next probe fails:
> >
> >   pcieport 0000:80:1d.0: DPC: containment event, status:0x1f01: unmasked
> uncorrectable error detected
> >   pcieport 0000:80:1d.0:    [20] UnsupReq               (First)
> >   pcieport 0000:80:1d.0: AER:   TLP Header: 0x34000000 0x81000052
> 0x00000000 0x00000000
> >
> > Message code 0x52 in the TLP header is a PTM Request from 81:00.0.
> >
> > Reset the device while the upstream port still has PTM enabled, so
> > that the PTM Request gets a response. Then disable PTM and bus
> > mastering. With this order, a later reset sends no PTM Request (tested
> > on I226-LM). igc_ptp_stop() has already cleared IGC_PTP_ENABLED, so no
> > new PTM dialog starts after this reset.
> 
> IGC_PTP_ENABLED is just a software flag, and it appears to be related to PTP,
> not directly related to PTM, so there are some dots missing between it and
> hardware PTM messages.

Sorry, I skipped steps. In igc, PTM is used only for PTP cross
timestamps. The driver starts PTM dialogs only from the PTP code, by
setting the TRIG bit in PTM_CTRL:
- igc_ptp_reset() sets PTM_CTRL.EN and triggers the first dialog.
- igc_ptp_getcrosststamp() triggers dialogs for each cross timestamp
  request.

igc_ptp_stop() runs before the reset and stops both:
1. igc_ptp_suspend() -> igc_ptm_stop() clears PTM_CTRL.EN.
2. ptp_clock_unregister() removes the PHC and waits for the
   getcrosststamp() calls in progress, so no more calls come.
3. It clears IGC_PTP_ENABLED. igc_reset() calls igc_ptp_reset(),
   which returns early when this flag is clear, so the reset path does
   not set PTM_CTRL.EN again.

So IGC_PTP_ENABLED does not control the hardware directly. It only
stops the driver from setting PTM_CTRL.EN again. v2 will describe
these steps.

> 
> > If the interface is down,
> > unregister_netdev() does not reset the device, so reset it explicitly.
> > PTM is still disabled before bus mastering, as the related fix above
> > requires.
> 
> Which fix requires PTM disable before bus mastering disable?  And why is that
> required?

c080fe262f9e. Its commit message says: "Avoid an i225/i226 hardware
issue that PTM requests can be made even though PCI bus mastering is
not enabled. These unexpected PTM requests can crash some systems."
The order is a workaround for that I225/I226 hardware issue, not a
PCIe spec requirement. The commit message does not say more about
why the order matters, but when I checked with the hardware team, I
found an internal erratum for that issue.

I found c080fe262f9e when I searched the igc history for this PTM
problem. Our issue is different: after the device has done a PTM
dialog, the next CTRL.RST can make it send a PTM Request, even when
PTM Enable is clear.

> 
> > Skip the reset if the device cannot respond: error recovery is in
> > progress or has failed (pci_channel_offline()), the device is gone
> > (pci_device_is_present()), or igc_rd32() has already reported "PCIe
> > link lost" (IGC_REMOVED()). Otherwise the reset accesses a device in
> > error recovery, or polls all-ones register values for about two
> > seconds until igc_disable_pcie_master() times out.
> >
> > Call pci_disable_ptm() only if PTM is enabled. pci_enable_ptm() can
> > fail in probe, for example if the upstream port does not support PTM,
> > and probe continues without PTM. The PCI/PTM change above also made
> > pci_disable_ptm() use a reference count: it decrements
> > dev->ptm_enable_cnt without a check, and then does the same for the
> > upstream port. Before, it did nothing if PTM was not enabled. After a
> > failed pci_enable_ptm(), the count of the device becomes negative and
> > stays negative after the driver unbinds. pcie_ptm_enabled() returns
> > true for a negative count, so after the next probe
> > igc_is_crosststamp_supported() reports PTM support and igc_ptp_reset()
> > starts PTM cycles on a device that does not have PTM enabled. Commit
> > 650f197d8ea6 ("idpf: disable PTM on probe failure and on remove") adds
> > the same check to idpf.
> 
> This sounds like a bug in the PCI core PTM support, i.e., maybe
> pci_disable_ptm() should be checking for PTM being enabled?
> Otherwise I'm afraid lots of drivers might trip over this.

Agree and thanks! I sent a PCI core fix:
https://lore.kernel.org/r/20261104121457.2108515-1-yifan2.li@intel.com

v2 will replace this paragraph with a short link to the core fix, and will drop
the pcie_ptm_enabled() check.

> 
> > Fixes: e1092d5e15e6 ("PCI/PTM: Do not enable PTM automatically for
> > Root and Switch Upstream Ports")
> > Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Co-developed-by: Li Lili <lili.li@intel.com>
> > Signed-off-by: Li Lili <lili.li@intel.com>
> > Signed-off-by: Li Yifan <yifan2.li@intel.com>
> > Tested-by: Wang Jie <jie.wang@intel.com>
> > ---
> >  drivers/net/ethernet/intel/igc/igc_main.c | 19 ++++++++++++++++---
> >  1 file changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> > b/drivers/net/ethernet/intel/igc/igc_main.c
> > index b8ac2f4fea5a..3ee8b05f0515 100644
> > --- a/drivers/net/ethernet/intel/igc/igc_main.c
> > +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> > @@ -7406,9 +7406,6 @@ static void igc_remove(struct pci_dev *pdev)
> >
> >  	igc_ptp_stop(adapter);
> >
> > -	pci_disable_ptm(pdev);
> > -	pci_clear_master(pdev);
> > -
> >  	set_bit(__IGC_DOWN, &adapter->state);
> >
> >  	timer_delete_sync(&adapter->watchdog_timer);
> > @@ -7427,6 +7424,22 @@ static void igc_remove(struct pci_dev *pdev)
> >  	igc_release_hw_control(adapter);
> >  	unregister_netdev(netdev);
> >
> > +	/* After a PTM dialog, I225/I226 can send a PTM Request at the next
> > +	 * CTRL.RST, even with PTM and bus mastering disabled on the device.
> > +	 * Reset the device while PTM is still enabled on the upstream port,
> > +	 * so that the request gets a response. unregister_netdev() resets the
> > +	 * device only if the interface was up. Skip the reset if the device
> > +	 * cannot respond (error recovery, surprise removal or an earlier
> > +	 * "PCIe link lost"). Then disable PTM before bus mastering.
> > +	 */
> > +	if (!pci_channel_offline(pdev) && pci_device_is_present(pdev) &&
> > +	    !IGC_REMOVED(adapter->hw.hw_addr))
> 
> All these tests look racy.  .reset_hw() really should be able to detect and deal
> with any errors that happen if it tries to access a device that is no longer
> responding.
 
Yes, the current checks are racy and too complicated. v2 removes
pci_device_is_present() and IGC_REMOVED() and keeps only
!pci_channel_offline(), the same check that igc_down() does before
igc_reset(). It only skips a reset that cannot work.

> 
> > +		adapter->hw.mac.ops.reset_hw(&adapter->hw);
> > +
> > +	if (pcie_ptm_enabled(pdev))
> > +		pci_disable_ptm(pdev);
> > +	pci_clear_master(pdev);
> 
> I don't think bus mastering has any connection to PTM, so why are we moving
> pci_clear_master()?
 
You are right. v2 leaves pci_clear_master() where it is and moves only
pci_disable_ptm(). I will test this order on the I226 before I send v2.

> 
> >  	igc_clear_interrupt_scheme(adapter);
> >  	pci_iounmap(pdev, adapter->io_addr);
> >  	pci_release_mem_regions(pdev);
> >
> > base-commit: c550c1c9727726dcae121221c252013153fa5945
> > --
> > 2.43.0
> >

Re: [PATCH iwl-net] igc: Disable PTM after the last device reset on remove

[Bjorn Helgaas]

On Wed, Nov 04, 2026 at 02:22:40PM +0000, Li, Yifan2 wrote:
> On Mon, Nov 02, 2026, Bjorn Helgaas wrote:
> > On Sat, Oct 31, 2026 at 10:19:28AM +0800, Li Yifan wrote:
> ...

> > > Reset the device while the upstream port still has PTM enabled,
> > > so that the PTM Request gets a response. Then disable PTM and
> > > bus mastering. With this order, a later reset sends no PTM
> > > Request (tested on I226-LM). igc_ptp_stop() has already cleared
> > > IGC_PTP_ENABLED, so no new PTM dialog starts after this reset.
> > 
> > IGC_PTP_ENABLED is just a software flag, and it appears to be
> > related to PTP, not directly related to PTM, so there are some
> > dots missing between it and hardware PTM messages.
> 
> Sorry, I skipped steps. In igc, PTM is used only for PTP cross
> timestamps. The driver starts PTM dialogs only from the PTP code, by
> setting the TRIG bit in PTM_CTRL:
> - igc_ptp_reset() sets PTM_CTRL.EN and triggers the first dialog.
> - igc_ptp_getcrosststamp() triggers dialogs for each cross timestamp
>   request.
> 
> igc_ptp_stop() runs before the reset and stops both:
> 1. igc_ptp_suspend() -> igc_ptm_stop() clears PTM_CTRL.EN.
> 2. ptp_clock_unregister() removes the PHC and waits for the
>    getcrosststamp() calls in progress, so no more calls come.
> 3. It clears IGC_PTP_ENABLED. igc_reset() calls igc_ptp_reset(),
>    which returns early when this flag is clear, so the reset path does
>    not set PTM_CTRL.EN again.
> 
> So IGC_PTP_ENABLED does not control the hardware directly. It only
> stops the driver from setting PTM_CTRL.EN again. v2 will describe
> these steps.

I don't think the PCIe spec includes any generic way to trigger PTM
dialogs, and in general, I think devices are permitted to initiate
them at any time, independent of driver or other events.  So I think
igc has device-specific knowledge that the NIC *only* initiates PTM
dialogs when a driver sets the TRIG bit.  If so, I think that's worth
a mention.

I haven't worked through the steps you outlined, but maybe they
explain why there's no PTM Request after the later reset.

> > > If the interface is down,
> > > unregister_netdev() does not reset the device, so reset it explicitly.
> > > PTM is still disabled before bus mastering, as the related fix above
> > > requires.
> > 
> > Which fix requires PTM disable before bus mastering disable?  And
> > why is that required?
> 
> c080fe262f9e. Its commit message says: "Avoid an i225/i226 hardware
> issue that PTM requests can be made even though PCI bus mastering is
> not enabled. These unexpected PTM requests can crash some systems."
> The order is a workaround for that I225/I226 hardware issue, not a
> PCIe spec requirement. The commit message does not say more about
> why the order matters, but when I checked with the hardware team, I
> found an internal erratum for that issue.
> 
> I found c080fe262f9e when I searched the igc history for this PTM
> problem. Our issue is different: after the device has done a PTM
> dialog, the next CTRL.RST can make it send a PTM Request, even when
> PTM Enable is clear.

I think that part of the c080fe262f9e commit log is just wrong.  The
fact that i225/i226 can make PTM requests when bus mastering is
disabled is perfectly fine; it's not a hardware issue.  I *suspect*
that only the pci_disable_ptm() in that commit is actually required.

> > > Skip the reset if the device cannot respond: error recovery is in
> > > progress or has failed (pci_channel_offline()), the device is gone
> > > (pci_device_is_present()), or igc_rd32() has already reported "PCIe
> > > link lost" (IGC_REMOVED()). Otherwise the reset accesses a device in
> > > error recovery, or polls all-ones register values for about two
> > > seconds until igc_disable_pcie_master() times out.
> > >
> > > Call pci_disable_ptm() only if PTM is enabled. pci_enable_ptm() can
> > > fail in probe, for example if the upstream port does not support PTM,
> > > and probe continues without PTM. The PCI/PTM change above also made
> > > pci_disable_ptm() use a reference count: it decrements
> > > dev->ptm_enable_cnt without a check, and then does the same for the
> > > upstream port. Before, it did nothing if PTM was not enabled. After a
> > > failed pci_enable_ptm(), the count of the device becomes negative and
> > > stays negative after the driver unbinds. pcie_ptm_enabled() returns
> > > true for a negative count, so after the next probe
> > > igc_is_crosststamp_supported() reports PTM support and igc_ptp_reset()
> > > starts PTM cycles on a device that does not have PTM enabled. Commit
> > > 650f197d8ea6 ("idpf: disable PTM on probe failure and on remove") adds
> > > the same check to idpf.
> > 
> > This sounds like a bug in the PCI core PTM support, i.e., maybe
> > pci_disable_ptm() should be checking for PTM being enabled?
> > Otherwise I'm afraid lots of drivers might trip over this.
> 
> Agree and thanks! I sent a PCI core fix:
> https://lore.kernel.org/r/20261104121457.2108515-1-yifan2.li@intel.com

Thanks for that!

Bjorn