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: <20241103112234.1b057a75@kernel.org>
Date: Sun, 3 Nov 2024 11:22:34 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Vadim Fedorenko <vadfed@...a.com>
Cc: Vadim Fedorenko <vadim.fedorenko@...ux.dev>, Michael Chan
 <michael.chan@...adcom.com>, Pavan Chebbi <pavan.chebbi@...adcom.com>,
 Andrew Lunn <andrew+netdev@...n.ch>, Paolo Abeni <pabeni@...hat.com>,
 "David S. Miller" <davem@...emloft.net>, <netdev@...r.kernel.org>, "Richard
 Cochran" <richardcochran@...il.com>
Subject: Re: [PATCH net-next v4 1/2] bnxt_en: cache only 24 bits of hw
 counter

On Tue, 29 Oct 2024 13:54:52 -0700 Vadim Fedorenko wrote:
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> index fa514be87650..820c7e83e586 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c
> @@ -106,7 +106,7 @@ static void bnxt_ptp_get_current_time(struct bnxt *bp)
>  	if (!ptp)
>  		return;
>  	spin_lock_irqsave(&ptp->ptp_lock, flags);
> -	WRITE_ONCE(ptp->old_time, ptp->current_time);
> +	WRITE_ONCE(ptp->old_time, (u32)(ptp->current_time >> BNXT_HI_TIMER_SHIFT));

the casts to u32 seem unnecessary since write to u32 will truncate
the value, anyway, and they make the lines go over 80 columns

>  	bnxt_refclk_read(bp, NULL, &ptp->current_time);
>  	spin_unlock_irqrestore(&ptp->ptp_lock, flags);
>  }
> @@ -174,7 +174,7 @@ void bnxt_ptp_update_current_time(struct bnxt *bp)
>  	struct bnxt_ptp_cfg *ptp = bp->ptp_cfg;
>  
>  	bnxt_refclk_read(ptp->bp, NULL, &ptp->current_time);
> -	WRITE_ONCE(ptp->old_time, ptp->current_time);
> +	WRITE_ONCE(ptp->old_time, (u32)(ptp->current_time >> BNXT_HI_TIMER_SHIFT));
>  }
>  
>  static int bnxt_ptp_adjphc(struct bnxt_ptp_cfg *ptp, s64 delta)
> @@ -813,7 +813,7 @@ int bnxt_get_rx_ts_p5(struct bnxt *bp, u64 *ts, u32 pkt_ts)
>  	if (!ptp)
>  		return -ENODEV;
>  
> -	BNXT_READ_TIME64(ptp, time, ptp->old_time);
> +	time = (u64)(READ_ONCE(ptp->old_time) << BNXT_HI_TIMER_SHIFT);

And this cast looks misplaced, I presume you want the shift to operate
on 64b. The way this is written the shift will be truncated to 32b,
and then we will promote, with top 32b being all 0.
-- 
pw-bot: cr

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ