Re: [Intel-wired-lan] [PATCH v3 net] idpf: disable PTM on probe failure and on remove

Myeonghun Pak <[email protected]> Sat, 25 Jul 2026 22:55:22 +0900
Newsgroups org.osuosl.intel-wired-lan,org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <CAGEsz8HNGUXsiZ_4Eg6atPjzWnTbWoxDwGVybgks3DmGhyH-=w@mail.gmail.com>
Understood. I won=E2=80=99t repost it unless I receive concrete feedback.

Thanks for the clarification.

Best regards,
Myeonghun Park

2026=EB=85=84 7=EC=9B=94 23=EC=9D=BC (=EB=AA=A9) =EC=98=A4=ED=9B=84 6:17, L=
oktionov, Aleksandr
<[email protected]>=EB=8B=98=EC=9D=B4 =EC=9E=91=EC=84=B1:
>
>
>
> > -----Original Message-----
> > From: Intel-wired-lan <[email protected]> On Behalf
> > Of Myeonghun Pak
> > Sent: Monday, July 20, 2026 4:35 PM
> > To: Nguyen, Anthony L <[email protected]>; Kitszel,
> > Przemyslaw <[email protected]>; intel-wired-
> > [email protected]
> > Cc: Olech, Milena <[email protected]>; Tantilov, Emil S
> > <[email protected]>; Mina Almasry <[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]; Myeonghun Pak
> > <[email protected]>; Ijae Kim <[email protected]>
> > Subject: [Intel-wired-lan] [PATCH v3 net] idpf: disable PTM on probe
> > failure and on remove
> >
> > idpf_probe() enables PCIe Precision Time Measurement with
> > pci_enable_ptm(), which takes a reference on the device and on every
> > PTM-capable device up the path to the PTM Root.
> >
> > Neither the probe error path nor idpf_remove() drops that reference,
> > so the PTM enable counts of this device and of its upstream path stay
> > elevated with no bound driver, and the device's PTM control bits
> > remain set.  pcim_enable_device() only arranges for
> > pci_disable_device() and does not undo the PTM enable.
> >
> > Add the matching pci_disable_ptm() to the common probe unwind and to
> > idpf_remove().  pci_enable_ptm() failure is not fatal here, so guard
> > both calls with pcie_ptm_enabled(): pci_disable_ptm() decrements
> > dev->ptm_enable_cnt unconditionally and then recurses upstream, so
> > calling it after a failed enable would drive this device's count
> > negative and wrongly decrement parents shared with other endpoints.
> >
> > This issue was identified during our ongoing static-analysis research
> > while reviewing kernel code.
> >
> > Fixes: 8d5e12c5921c ("idpf: add initial PTP support")
> > Co-developed-by: Ijae Kim <[email protected]>
> > Signed-off-by: Ijae Kim <[email protected]>
> > Signed-off-by: Myeonghun Pak <[email protected]>
> > ---
> > Changes in v3:
> > - Rebased; aa8671af0c38 ("PCI/PTM: Drop pci_enable_ptm() granularity
> >   parameter") changed the call signature, so v2 no longer applied.
> > - Guard both pci_disable_ptm() calls with pcie_ptm_enabled(), as
> >   pci_disable_ptm() is refcounted and recurses upstream since
> >   e1092d5e15e6 ("PCI/PTM: Do not enable PTM automatically for Root and
> >   Switch Upstream Ports").  Raised by Tony Nguyen.
> > - Dropped the v2 claim that pci_disable_ptm() is a no-op when PTM was
> > not
> >   enabled; that is no longer true.
> >
> > Changes in v2:
> > - Disable PTM in the probe error path, as requested by Emil Tantilov.
> >
> >  drivers/net/ethernet/intel/idpf/idpf_main.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c
> > b/drivers/net/ethernet/intel/idpf/idpf_main.c
> > index ab3c409..97bafeb 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_main.c
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
> > @@ -159,6 +159,8 @@ destroy_wqs:
> >       mutex_destroy(&adapter->queue_lock);
> >       mutex_destroy(&adapter->vc_buf_lock);
> >
> > +     if (pcie_ptm_enabled(pdev))
> > +             pci_disable_ptm(pdev);
> >       pci_set_drvdata(pdev, NULL);
> >       kfree(adapter);
> >  }
> > @@ -266,7 +268,7 @@ static int idpf_probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> >       if (err) {
> >               pci_err(pdev, "DMA configuration failed: %pe\n",
> > ERR_PTR(err));
> >
> > -             goto err_free;
> > +             goto err_disable_ptm;
> >       }
> >
> >       pci_set_master(pdev);
> > @@ -279,7 +281,7 @@ static int idpf_probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> >       if (!adapter->init_wq) {
> >               dev_err(dev, "Failed to allocate init workqueue\n");
> >               err =3D -ENOMEM;
> > -             goto err_free;
> > +             goto err_disable_ptm;
> >       }
> >
> >       adapter->serv_wq =3D alloc_workqueue("%s-%s-service", @@ -366,6
> > +368,9 @@ err_mbx_wq_alloc:
> >       destroy_workqueue(adapter->serv_wq);
> >  err_serv_wq_alloc:
> >       destroy_workqueue(adapter->init_wq);
> > +err_disable_ptm:
> > +     if (pcie_ptm_enabled(pdev))
> > +             pci_disable_ptm(pdev);
> >  err_free:
> >       kfree(adapter);
> >       return err;
> > --
> > 2.47.1
>
> Reviewed-by: Aleksandr Loktionov <[email protected]>