netdev - Re: [Intel-wired-lan] [PATCH v2 net-next] e1000e: Remove duplicated writel() in e1000_configure

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: <c3792285-1a36-4c48-8db8-69c28cfa8646@intel.com>
Date: Wed, 25 Sep 2024 10:28:29 +0300
From: Mor Bar-Gabay <morx.bar.gabay@...el.com>
To: Takamitsu Iwai <takamitz@...zon.co.jp>, "David S. Miller"
	<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
	<kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Tony Nguyen
	<anthony.l.nguyen@...el.com>, Przemek Kitszel <przemyslaw.kitszel@...el.com>
CC: <netdev@...r.kernel.org>, Kohei Enju <enjuk@...zon.com>,
	<intel-wired-lan@...ts.osuosl.org>
Subject: Re: [Intel-wired-lan] [PATCH v2 net-next] e1000e: Remove duplicated
 writel() in e1000_configure_tx/rx()

On 06/09/2024 5:17, Takamitsu Iwai wrote:
> Duplicated register initialization codes exist in e1000_configure_tx()
> and e1000_configure_rx().
> 
> For example, writel(0, tx_ring->head) writes 0 to tx_ring->head, which
> is adapter->hw.hw_addr + E1000_TDH(0).
> 
> This initialization is already done in ew32(TDH(0), 0).
> 
> ew32(TDH(0), 0) is equivalent to __ew32(hw, E1000_TDH(0), 0). It
> executes writel(0, hw->hw_addr + E1000_TDH(0)). Since variable hw is
> set to &adapter->hw, it is equal to writel(0, tx_ring->head).
> 
> We can remove similar four writel() in e1000_configure_tx() and
> e1000_configure_rx().
> 
> commit 0845d45e900c ("e1000e: Modify Tx/Rx configurations to avoid
> null pointer dereferences in e1000_open") has introduced these
> writel(). This commit moved register writing to
> e1000_configure_tx/rx(), and as result, it caused duplication in
> e1000_configure_tx/rx().
> 
> This patch modifies the sequence of register writing, but removing
> these writes is safe because the same writes were already there before
> the commit.
> 
> I also have checked the datasheets [0] [1] and have not found any
> description that we need to write RDH, RDT, TDH and TDT registers
> twice at initialization. Furthermore, we have tested this patch on an
> I219-V device physically.
> 
> Link: https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/82577-gbe-phy-datasheet.pdf [0]
> Link: https://www.intel.com/content/www/us/en/content-details/613460/intel-82583v-gbe-controller-datasheet.html [1]
> Tested-by: Kohei Enju <enjuk@...zon.com>
> Signed-off-by: Takamitsu Iwai <takamitz@...zon.co.jp>
> ---
> 
> v1->v2
> modify commit message to explain the reason why we can remove these writes safely.
> 
> v1 link
> https://lore.kernel.org/netdev/20240902061454.85744-1-takamitz@amazon.co.jp/
> 
>   drivers/net/ethernet/intel/e1000e/netdev.c | 6 ------
>   1 file changed, 6 deletions(-)
> 
Tested-by: Mor Bar-Gabay <morx.bar.gabay@...el.com>