netdev - RZ/N1 gmac support

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

Date:   Tue, 5 Apr 2022 10:40:28 +0200
From:   Clément Léger <clement.leger@...tlin.com>
To:     Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Jose Abreu <joabreu@...opsys.com>
Cc:     netdev@...r.kernel.org,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Herve Codina <herve.codina@...tlin.com>,
        Milan STEVANOVIC <milan.stevanovic@...com>
Subject: RZ/N1  gmac support

Hello STMMAC maintainers,

I'm currently working on adding RZ/N1 Ethernet support [1] in the
kernel. As a part of this, the GMAC is a dwmac IP and is supported by
the stmmac driver. Unfortunately, some revisions of this SoC suffer from
a hardware bug [2] that requires that only one CPU core writes registers
at a time for all the following peripherals:
- GMAC
- Switch
- R-In Engine (Accessory registers)

In practice, this translates to using a function that does a writel() in
a mutual exlusion section using a spinlock (When only the 2 Cortex-A7
CPUs are running Linux) or a hardware semaphore (if the M3 core also
accesses these registers).

Since the stmmac driver uses writel() functions at different places,
this would requires to modify all theses calls. I can see multiple
solutions (which might not be ideal though):

1) Adding a write_reg() callback in plat_stmmacenet_data and call it
instead of writel. This would also require to pass the stmmac_priv
struct to all callbacks that uses writel() in order to call the
write_reg callback.

Pros:
 - Per platform
Cons:
 - Large modifications needed
 - Could slow down driver by adding writel indirection

2) Use a global gmac_writel function that would either use writel() or a
specific global custom_writel() override function depending on a static
key usage. For instance:

static inline void stmmac_writel(u32 value, volatile void __iomem *addr)
{
	if (static_key_true(use_custom_writel))
		stmmac_custom_writel(value, addr);
	else
		writel(value, addr);
}

Pros:
 - Really small overhead
 - Few modifications in the driver
Cons:
 - Global

I think the first solution is cleaner but requires a lot of
changes to modify all the writel calls for a single . Moreover it would
add an indirection to call writel which might degrade performances. One
solution to mitigate this would be to use a static key and thus the
second solution could also be considered (since the static key would be
global).

Any advice or other solutions is welcomed !

Thanks for your help,

[1]
https://www.renesas.com/us/en/document/mah/rzn1d-group-rzn1s-group-rzn1l-group-users-manual-r-engine-and-ethernet-peripherals
[2]
https://www.renesas.com/us/en/document/tcu/advanced-5port-switch-a5psw-function-issues-and-usage-notice

-- 
Clément Léger,
Embedded Linux and Kernel engineer at Bootlin
https://bootlin.com