netdev - Re: [PATCH v2 net 2/4] net: aquantia: when cleaning hw cache it should be toggled

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] [day] [month] [year] [list]

Message-ID: <20191016125928.24d72dce@cakuba.netronome.com>
Date:   Wed, 16 Oct 2019 12:59:28 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Igor Russkikh <Igor.Russkikh@...antia.com>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "David S . Miller" <davem@...emloft.net>
Subject: Re: [PATCH v2 net 2/4] net: aquantia: when cleaning hw cache it
 should be toggled

On Wed, 16 Oct 2019 13:19:30 +0000, Igor Russkikh wrote:
> >> It was previosly always just set. Due to the way driver stops HW this
> >> never actually caused any issues, but it still may, so cleaning this up.  
> > 
> > Hm. So is it a cleanup of fix? Does the way code is written guarantee
> > it will never cause issues?  
> 
> Yes, thats a cleanup. We just had other products where this cache reset had to
> be done multiple times. Obviously doing that second time was just no-op for
> hardware ;)
> On linux this always gets called on deinit only once - thus it was safe.
> We just aligning here the linux driver with actual HW specification.

I see, in light of that explanation I think it'd be appropriate to take
the Fixes tag away and move this patch to the net-next series.

> >> +	if (err)
> >> +		goto err_exit;
> >> +
> >> +	readx_poll_timeout_atomic(hw_atl_rdm_rx_dma_desc_cache_init_done_get,
> >> +				  self, val, val == 1, 1000U, 10000U);  
> > 
> > It's a little strange to toggle, yet wait for it to be of a specific
> > value..  
> 
> Notice thats a different value - 'cache_init_done' bit.
> This is used by HW to indicate completion of cache reset operation.

Ah, ack, it's an "eyeful".. :)