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: <0f315501-e8cb-4904-8c43-d9721fdef846@prolan.hu>
Date: Fri, 14 Jun 2024 09:59:16 +0200
From: Csókás Bence <csokas.bence@...lan.hu>
To: Jakub Kicinski <kuba@...nel.org>
CC: Frank Li <Frank.Li@...escale.com>, "David S. Miller"
	<davem@...emloft.net>, <imx@...ts.linux.dev>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, Richard Cochran <richardcochran@...il.com>,
	Wei Fang <wei.fang@....com>, Shenwei Wang <shenwei.wang@....com>, Clark Wang
	<xiaoning.wang@....com>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni
	<pabeni@...hat.com>
Subject: Re: [PATCH resubmit 2] net: fec: Fix FEC_ECR_EN1588 being cleared on
 link-down

On 6/13/24 17:12, Jakub Kicinski wrote:
> On Tue, 11 Jun 2024 10:04:05 +0200 Csókás, Bence wrote:
>> +	if (fep->bufdesc_ex) {
>> +		val = readl(fep->hwp + FEC_ECNTRL);
>> +		val |= FEC_ECR_EN1588;
>> +		writel(val, fep->hwp + FEC_ECNTRL);
> 
> FEC_ECNTRL gets written multiple times in this function,
> including with 0, and then you RMW it to add this flag.
> 
> Is this intentional? It really seems like you should be
> adding this flag more consistently or making sure its
> not cleared, rather than appending "add it back" at the
> end of the function...

It only writes 0 if WOL is disabled AND the device has the MULTI_QUEUES 
quirk. Otherwise, we either write FEC_ECR_RESET, which resets the device 
(and the HW changes ECNTRL to its reset value), OR we RMW set the WOL 
sleep bits. And then, if some more quirks are set, we set ETHEREN.

So I think RMW is the safest route here, instead of trying to keep track 
of all these different branches, re-read ECNTRL after reset etc.

Bence


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ