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

"Salin, Samuel" <[email protected]>
Newsgroups org.osuosl.intel-wired-lan,org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <SJ1PR11MB629769E42A2223130F20915A9BDA2@SJ1PR11MB6297.namprd11.prod.outlook.com>
> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of
> Myeonghun Pak
> Sent: Saturday, July 25, 2026 6:55 AM
> To: Loktionov, Aleksandr <[email protected]>
> Cc: Nguyen, Anthony L <[email protected]>; Kitszel, Przemyslaw
> <[email protected]>; [email protected]; 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]; Ijae Kim
> <[email protected]>
> Subject: Re: [Intel-wired-lan] [PATCH v3 net] idpf: disable PTM on probe
> failure and on remove
> 
> Understood. I won’t repost it unless I receive concrete feedback.
> 
> Thanks for the clarification.
> 
> Best regards,
> Myeonghun Park
> 
> 2026년 7월 23일 (목) 오후 6:17, Loktionov, Aleksandr
> <[email protected]>님이 작성:
> >
> >
> >
> > > -----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 = -ENOMEM;
> > > -             goto err_free;
> > > +             goto err_disable_ptm;
> > >       }
> > >
> > >       adapter->serv_wq = 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]>

Tested-by: Samuel Salin <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.