netdev - RE: [PATCH net-next 7/8] ixgbe: extend PTP gettime function to read system 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:   Fri, 9 Nov 2018 18:14:06 +0000
From:   "Keller, Jacob E" <jacob.e.keller@...el.com>
To:     Miroslav Lichvar <mlichvar@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     Richard Cochran <richardcochran@...il.com>,
        "Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
Subject: RE: [PATCH net-next 7/8] ixgbe: extend PTP gettime function to read
 system clock

> -----Original Message-----
> From: Miroslav Lichvar [mailto:mlichvar@...hat.com]
> Sent: Friday, November 09, 2018 2:15 AM
> To: netdev@...r.kernel.org
> Cc: Richard Cochran <richardcochran@...il.com>; Keller, Jacob E
> <jacob.e.keller@...el.com>; Miroslav Lichvar <mlichvar@...hat.com>; Kirsher,
> Jeffrey T <jeffrey.t.kirsher@...el.com>
> Subject: [PATCH net-next 7/8] ixgbe: extend PTP gettime function to read system
> clock
> 
> -static int ixgbe_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
> +static int ixgbe_ptp_gettimex(struct ptp_clock_info *ptp,
> +			      struct timespec64 *ts,
> +			      struct ptp_system_timestamp *sts)
>  {
>  	struct ixgbe_adapter *adapter =
>  		container_of(ptp, struct ixgbe_adapter, ptp_caps);
> +	struct ixgbe_hw *hw = &adapter->hw;
>  	unsigned long flags;
> -	u64 ns;
> +	u64 ns, stamp;
> 
>  	spin_lock_irqsave(&adapter->tmreg_lock, flags);
> -	ns = timecounter_read(&adapter->hw_tc);
> +
> +	switch (adapter->hw.mac.type) {
> +	case ixgbe_mac_X550:
> +	case ixgbe_mac_X550EM_x:
> +	case ixgbe_mac_x550em_a:
> +		/* Upper 32 bits represent billions of cycles, lower 32 bits
> +		 * represent cycles. However, we use timespec64_to_ns for the
> +		 * correct math even though the units haven't been corrected
> +		 * yet.
> +		 */
> +		ptp_read_system_prets(sts);
> +		IXGBE_READ_REG(hw, IXGBE_SYSTIMR);
> +		ptp_read_system_postts(sts);
> +		ts->tv_nsec = IXGBE_READ_REG(hw, IXGBE_SYSTIML);
> +		ts->tv_sec = IXGBE_READ_REG(hw, IXGBE_SYSTIMH);
> +		stamp = timespec64_to_ns(ts);
> +		break;
> +	default:
> +		ptp_read_system_prets(sts);
> +		stamp = IXGBE_READ_REG(hw, IXGBE_SYSTIML);
> +		ptp_read_system_postts(sts);
> +		stamp |= (u64)IXGBE_READ_REG(hw, IXGBE_SYSTIMH) << 32;
> +		break;
> +	}
> +
> +	ns = timecounter_cyc2time(&adapter->hw_tc, stamp);
> +

At first, I was confused by this entire block of code, but then realized that we can't update the timecounter_read method, so we instead have to break this out so that our calls to ptp_read_system_prets() and ptp_read_system_postts() can be added between the register reads.

Ok, that makes sense.

>  	spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
> 
>  	*ts = ns_to_timespec64(ns);
> @@ -567,10 +597,14 @@ void ixgbe_ptp_overflow_check(struct ixgbe_adapter
> *adapter)
>  {
>  	bool timeout = time_is_before_jiffies(adapter->last_overflow_check +
>  					     IXGBE_OVERFLOW_PERIOD);
> -	struct timespec64 ts;
> +	unsigned long flags;
> 
>  	if (timeout) {
> -		ixgbe_ptp_gettime(&adapter->ptp_caps, &ts);
> +		/* Update the timecounter */
> +		spin_lock_irqsave(&adapter->tmreg_lock, flags);
> +		timecounter_read(&adapter->hw_tc);
> +		spin_unlock_irqrestore(&adapter->tmreg_lock, flags);
> +

This also explains this change where we now have to update the timecounter during the overflow check.

Ok, this makes sense to me.

Thanks,
Jake