linux-kernel - Re: [net-next PATCH v2] octeontx2: Add PTP clock driver for Octeon PTM clock.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 28 Feb 2024 10:08:57 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Sai Krishna Gajula <saikrishnag@...vell.com>
Cc: "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"richardcochran@...il.com" <richardcochran@...il.com>,
	"horms@...nel.org" <horms@...nel.org>,
	"vinicius.gomes@...el.com" <vinicius.gomes@...el.com>,
	"vadim.fedorenko@...ux.dev" <vadim.fedorenko@...ux.dev>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"kuba@...nel.org" <kuba@...nel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Sunil Kovvuri Goutham <sgoutham@...vell.com>,
	Geethasowjanya Akula <gakula@...vell.com>,
	Linu Cherian <lcherian@...vell.com>,
	Hariprasad Kelam <hkelam@...vell.com>,
	Subbaraya Sundeep Bhatta <sbhatta@...vell.com>,
	Naveen Mamindlapalli <naveenm@...vell.com>
Subject: Re: [net-next PATCH v2] octeontx2: Add PTP clock driver for Octeon
 PTM clock.

On Wed, Feb 28, 2024 at 12:37:02PM +0000, Sai Krishna Gajula wrote:
> > -----Original Message-----
> > From: Bjorn Helgaas <helgaas@...nel.org>
> > Sent: Monday, February 26, 2024 10:31 PM
> ...
> > On Mon, Feb 26, 2024 at 03:40:25PM +0000, Sai Krishna Gajula wrote:
> > > > -----Original Message-----
> > > > From: Bjorn Helgaas <helgaas@...nel.org>
> > > > Sent: Wednesday, February 14, 2024 10:59 PM ...
> > > > On Wed, Feb 14, 2024 at 06:38:53PM +0530, Sai Krishna wrote:
> > > > > The PCIe PTM(Precision time measurement) protocol provides precise
> > > > > coordination of events across multiple components like PCIe host
> > > > > clock, PCIe EP PHC local clocks of PCIe devices. This patch adds
> > > > > support for ptp clock based PTM clock. We can use this PTP device
> > > > > to sync the PTM time with CLOCK_REALTIME or other PTP PHC devices
> > > > > using phc2sys.

> > > > > +static int __init ptp_oct_ptm_init(void) {
> > > > > +	struct pci_dev *pdev = NULL;
> > > > > +
> > > > > +	pdev = pci_get_device(PCI_VENDOR_ID_CAVIUM,
> > > > > +			      PCI_DEVID_OCTEONTX2_PTP, pdev);
> > > >
> > > > pci_get_device() is a sub-optimal method for a driver to claim a device.
> > > > pci_register_driver() is the preferred method.  If you can't use
> > > > that, a comment here explaining why not would be helpful.
> > >
> > > We just want to check the PTP device availability in the system as one
> > > of the use case is to sync PTM time to PTP.
> > 
> > This doesn't explain why you can't use pci_register_driver().  Can
> > you clarify that?
> 
> This is not a PCI endpoint driver.  This piece of code is used to
> identify the silicon version.  We will update the code by reading
> the silicon version from Endpoint internal BAR register offsets.

> > I assume the PCI_DEVID_OCTEONTX2_PTP device is a PCIe Endpoint,
> > and this driver runs on the host?  I.e., this driver does not run
> > as firmware on the Endpoint itself?  So if you run lspci on the
> > host, you would see this device as one of the PCI devices?
> > 
> > If that's the case, a driver would normally operate the device via
> > MMIO accesses to regions described by PCI BARs.  "lspci -v" would
> > show those addresses.
> 
> This driver don't run on Host but runs on the EP firmware itself.

The "endpoint driver" terminology is a bit confusing here.  See
Documentation/PCI/endpoint/pci-endpoint.rst for details.

If this driver actually runs as part of the Endpoint firmware, it
would not normally see a hierarchy of pci_devs, and I don't think
pci_get_device() would work.

So I suspect this driver actually runs on the host, and it looks like
it wants to use the same device (0x177d:0xa00c) as these two drivers:

  drivers/net/ethernet/cavium/common/cavium_ptp.c:#define PCI_DEVICE_ID_CAVIUM_PTP        0xA00C
  drivers/net/ethernet/marvell/octeontx2/af/ptp.c:#define PCI_DEVID_OCTEONTX2_PTP                 0xA00C

It seems like maybe it should be integrated into them?  Otherwise you
have multiple drivers thinking they are controlling a single device.

Bjorn