linux-kernel - Re: [net-next PATCH v2] octeontx2-pf: Use PTP HW timestamp counter atomic update feature

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]

Message-ID: <BY3PR18MB470745294C1B03C0DA5364E6A010A@BY3PR18MB4707.namprd18.prod.outlook.com>
Date:   Fri, 11 Aug 2023 05:48:38 +0000
From:   Sai Krishna Gajula <saikrishnag@...vell.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "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>,
        Subbaraya Sundeep Bhatta <sbhatta@...vell.com>,
        Hariprasad Kelam <hkelam@...vell.com>,
        "richardcochran@...il.com" <richardcochran@...il.com>,
        Naveen Mamindlapalli <naveenm@...vell.com>
Subject: Re: [net-next PATCH v2] octeontx2-pf: Use PTP HW timestamp counter
 atomic update feature


> -----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Thursday, August 10, 2023 4:20 AM
> To: Sai Krishna Gajula <saikrishnag@...vell.com>
> Cc: davem@...emloft.net; edumazet@...gle.com; pabeni@...hat.com;
> netdev@...r.kernel.org; linux-kernel@...r.kernel.org; Sunil Kovvuri
> Goutham <sgoutham@...vell.com>; Geethasowjanya Akula
> <gakula@...vell.com>; Subbaraya Sundeep Bhatta
> <sbhatta@...vell.com>; Hariprasad Kelam <hkelam@...vell.com>;
> richardcochran@...il.com; Naveen Mamindlapalli
> <naveenm@...vell.com>
> Subject: Re: [net-next PATCH v2] octeontx2-pf: Use PTP HW timestamp
> counter atomic update feature
> 
> On Mon, 7 Aug 2023 19:35:35 +0530 Sai Krishna wrote:
> > Some of the newer silicon versions in CN10K series supports a feature
> > where in the current PTP timestamp in HW can be updated atomically
> > without losing any cpu cycles unlike read/modify/write register.
> > This patch uses this feature so that PTP accuracy can be improved
> > while adjusting the master offset in HW. There is no need for SW
> > timecounter when using this feature. So removed references to SW
> > timecounter wherever appropriate.
> 
> > -#include "ptp.h"
> >  #include "mbox.h"
> >  #include "rvu.h"
> > +#include "ptp.h"
> 
> If you reorder the includes - maybe put them in alphabetical order?

There are some structure definitions in rvu.h which are required in ptp.h. So, re-ordering in alphabetical order will give compilation issue. 

> >  static bool cn10k_ptp_errata(struct ptp *ptp)  {
> > -	if (ptp->pdev->subsystem_device ==
> PCI_SUBSYS_DEVID_CN10K_A_PTP ||
> > -	    ptp->pdev->subsystem_device ==
> PCI_SUBSYS_DEVID_CNF10K_A_PTP)
> > +	if ((is_ptp_dev_cn10ka(ptp) &&
> > +	     ((ptp->pdev->revision & 0x0F) == 0x0 || (ptp->pdev->revision &
> 0x0F) == 0x1)) ||
> > +	    (is_ptp_dev_cnf10ka(ptp) &&
> > +	     ((ptp->pdev->revision & 0x0F) == 0x0 || (ptp->pdev->revision &
> > +0x0F) == 0x1)))
> 
> Please refactor the revision check to avoid these long lines repeating the
> same logic

Sai:  Ack, will submit refactor changes in V3 patch.

> 
> >  		return true;
> > +
> >  	return false;
> >  }
> >
> > -static bool is_ptp_tsfmt_sec_nsec(struct ptp *ptp)
> > +static bool is_tstmp_atomic_update_supported(struct rvu *rvu)
> >  {
> > -	if (ptp->pdev->subsystem_device ==
> PCI_SUBSYS_DEVID_CN10K_A_PTP ||
> > -	    ptp->pdev->subsystem_device ==
> PCI_SUBSYS_DEVID_CNF10K_A_PTP)
> > -		return true;
> > -	return false;
> > +	struct ptp *ptp = rvu->ptp;
> > +	struct pci_dev *pdev;
> > +
> > +	if (is_rvu_otx2(rvu))
> > +		return false;
> > +
> > +	pdev = ptp->pdev;
> > +
> > +	/* On older silicon variants of CN10K, atomic update feature
> > +	 * is not available.
> > +	 */
> > +	if ((pdev->subsystem_device == PCI_SUBSYS_DEVID_CN10K_A_PTP
> &&
> > +	     (pdev->revision & 0x0F) == 0x0) ||
> > +	     (pdev->subsystem_device == PCI_SUBSYS_DEVID_CN10K_A_PTP
> &&
> > +	     (pdev->revision & 0x0F) == 0x1) ||
> > +	     (pdev->subsystem_device ==
> PCI_SUBSYS_DEVID_CNF10K_A_PTP &&
> > +	     (pdev->revision & 0x0F) == 0x0) ||
> > +	     (pdev->subsystem_device ==
> PCI_SUBSYS_DEVID_CNF10K_A_PTP &&
> > +	     (pdev->revision & 0x0F) == 0x1))
> 
> why are you not using cn10k_ptp_errata() here?

Ack, will submit the changes in V3 patch.

> 
> > +		return false;
> > +
> > +	return true;
> >  }
> 
> > -static int otx2_ptp_adjtime(struct ptp_clock_info *ptp_info, s64
> > delta)
> > +static int otx2_ptp_tc_adjtime(struct ptp_clock_info *ptp_info, s64
> > +delta)
> >  {
> >  	struct otx2_ptp *ptp = container_of(ptp_info, struct otx2_ptp,
> >  					    ptp_info);
> >  	struct otx2_nic *pfvf = ptp->nic;
> >
> > +	if (!ptp->nic)
> > +		return -ENODEV;
> 
> Is this check related to the rest of the patch?

I will remove this check and submit V3 patch. Will submit this check as a separate patch, if required.

Thanks,
Sai
> 
> >  	mutex_lock(&pfvf->mbox.lock);
> >  	timecounter_adjtime(&ptp->time_counter, delta);
> >  	mutex_unlock(&pfvf->mbox.lock);
> --
> pw-bot: cr