kernel.git/drivers/net/ethernet/stmicro, branch linux-6.7.y

stmmac: Clear variable when destroying workqueue

2024-03-06T14:53:50Z

[ Upstream commit 8af411bbba1f457c33734795f024d0ef26d0963f ] Currently when suspending driver and stopping workqueue it is checked whether workqueue is not NULL and if so, it is destroyed. Function destroy_workqueue() does drain queue and does clear variable, but it does not set workqueue variable to NULL. This can cause kernel/module panic if code attempts to clear workqueue that was not initialized. This scenario is possible when resuming suspended driver in stmmac_resume(), because there is no handling for failed stmmac_hw_setup(), which can fail and return if DMA engine has failed to initialize, and workqueue is initialized after DMA engine. Should DMA engine fail to initialize, resume will proceed normally, but interface won't work and TX queue will eventually timeout, causing 'Reset adapter' error. This then does destroy workqueue during reset process. And since workqueue is initialized after DMA engine and can be skipped, it will cause kernel/module panic. To secure against this possible crash, set workqueue variable to NULL when destroying workqueue. Log/backtrace from crash goes as follows: [88.031977]------------[ cut here ]------------ [88.031985]NETDEV WATCHDOG: eth0 (sxgmac): transmit queue 1 timed out [88.032017]WARNING: CPU: 0 PID: 0 at net/sched/sch_generic.c:477 dev_watchdog+0x390/0x398 [88.032251]---[ end trace e70de432e4d5c2c0 ]--- [88.032282]sxgmac 16d88000.ethernet eth0: Reset adapter. [88.036359]------------[ cut here ]------------ [88.036519]Call trace: [88.036523] flush_workqueue+0x3e4/0x430 [88.036528] drain_workqueue+0xc4/0x160 [88.036533] destroy_workqueue+0x40/0x270 [88.036537] stmmac_fpe_stop_wq+0x4c/0x70 [88.036541] stmmac_release+0x278/0x280 [88.036546] __dev_close_many+0xcc/0x158 [88.036551] dev_close_many+0xbc/0x190 [88.036555] dev_close.part.0+0x70/0xc0 [88.036560] dev_close+0x24/0x30 [88.036564] stmmac_service_task+0x110/0x140 [88.036569] process_one_work+0x1d8/0x4a0 [88.036573] worker_thread+0x54/0x408 [88.036578] kthread+0x164/0x170 [88.036583] ret_from_fork+0x10/0x20 [88.036588]---[ end trace e70de432e4d5c2c1 ]--- [88.036597]Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004 Fixes: 5a5586112b929 ("net: stmmac: support FPE link partner hand-shaking procedure") Signed-off-by: Jakub Raczynski Reviewed-by: Jiri Pirko Signed-off-by: David S. Miller Signed-off-by: Sasha Levin

net: stmmac: Fix incorrect dereference in interrupt handlers

2024-03-01T12:41:54Z

[ Upstream commit 97dde84026339e4b4af9a6301f825d1828d7874b ] If 'dev' or 'data' is NULL, the 'priv' variable has an incorrect address when dereferencing calling netdev_err(). Since we get as 'dev_id' or 'data' what was passed as the 'dev' argument to request_irq() during interrupt initialization (that is, the net_device and rx/tx queue pointers initialized at the time of the call) and since there are usually no checks for the 'dev_id' argument in such handlers in other drivers, remove these checks from the handlers in stmmac driver. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 8532f613bc78 ("net: stmmac: introduce MSI Interrupt routines for mac, safety, RX & TX") Signed-off-by: Pavel Sakharov Reviewed-by: Serge Semin Signed-off-by: David S. Miller Signed-off-by: Sasha Levin

net: stmmac: protect updates of 64-bit statistics counters

2024-02-23T08:51:56Z

commit 38cc3c6dcc09dc3a1800b5ec22aef643ca11eab8 upstream. As explained by a comment in , write side of struct u64_stats_sync must ensure mutual exclusion, or one seqcount update could be lost on 32-bit platforms, thus blocking readers forever. Such lockups have been observed in real world after stmmac_xmit() on one CPU raced with stmmac_napi_poll_tx() on another CPU. To fix the issue without introducing a new lock, split the statics into three parts: 1. fields updated only under the tx queue lock, 2. fields updated only during NAPI poll, 3. fields updated only from interrupt context, Updates to fields in the first two groups are already serialized through other locks. It is sufficient to split the existing struct u64_stats_sync so that each group has its own. Note that tx_set_ic_bit is updated from both contexts. Split this counter so that each context gets its own, and calculate their sum to get the total value in stmmac_get_ethtool_stats(). For the third group, multiple interrupts may be processed by different CPUs at the same time, but interrupts on the same CPU will not nest. Move fields from this group to a newly created per-cpu struct stmmac_pcpu_stats. Fixes: 133466c3bbe1 ("net: stmmac: use per-queue 64 bit statistics where necessary") Link: https://lore.kernel.org/netdev/Za173PhviYg-1qIn@torres.zugschlus.de/t/ Cc: stable@vger.kernel.org Signed-off-by: Petr Tesarik Reviewed-by: Jisheng Zhang Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman

net: stmmac: do not clear TBS enable bit on link up/down

2024-02-23T08:51:38Z

commit 4896bb7c0b31a0a3379b290ea7729900c59e0c69 upstream. With the dma conf being reallocated on each call to stmmac_open(), any information in there is lost, unless we specifically handle it. The STMMAC_TBS_EN bit is set when adding an etf qdisc, and the etf qdisc therefore would stop working when link was set down and then back up. Fixes: ba39b344e924 ("net: ethernet: stmicro: stmmac: generate stmmac dma conf before open") Cc: stable@vger.kernel.org Signed-off-by: Esben Haabendal Signed-off-by: Paolo Abeni Signed-off-by: Greg Kroah-Hartman

net: stmmac: xgmac: fix a typo of register name in DPP safety handling

2024-02-16T18:14:32Z

commit 1ce2654d87e2fb91fea83b288bd9b2641045e42a upstream. DDPP is copied from Synopsys Data book: DDPP: Disable Data path Parity Protection. When it is 0x0, Data path Parity Protection is enabled. When it is 0x1, Data path Parity Protection is disabled. The macro name should be XGMAC_DPP_DISABLE. Fixes: 46eba193d04f ("net: stmmac: xgmac: fix handling of DPP safety error for DMA channels") Signed-off-by: Furong Xu <0x1207@gmail.com> Reviewed-by: Serge Semin Link: https://lore.kernel.org/r/20240203053133.1129236-1-0x1207@gmail.com Signed-off-by: Paolo Abeni Signed-off-by: Greg Kroah-Hartman

net: stmmac: xgmac: use #define for string constants

2024-02-16T18:14:32Z

commit 1692b9775e745f84b69dc8ad0075b0855a43db4e upstream. The cited commit introduces and uses the string constants dpp_tx_err and dpp_rx_err. These are assigned to constant fields of the array dwxgmac3_error_desc. It has been reported that on GCC 6 and 7.5.0 this results in warnings such as: .../dwxgmac2_core.c:836:20: error: initialiser element is not constant { true, "TDPES0", dpp_tx_err }, I have been able to reproduce this using: GCC 7.5.0, 8.4.0, 9.4.0 and 10.5.0. But not GCC 13.2.0. So it seems this effects older compilers but not newer ones. As Jon points out in his report, the minimum compiler supported by the kernel is GCC 5.1, so it does seem that this ought to be fixed. It is not clear to me what combination of 'const', if any, would address this problem. So this patch takes of using #defines for the string constants Compile tested only. Fixes: 46eba193d04f ("net: stmmac: xgmac: fix handling of DPP safety error for DMA channels") Reported-by: Jon Hunter Closes: https://lore.kernel.org/netdev/c25eb595-8d91-40ea-9f52-efa15ebafdbc@nvidia.com/ Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202402081135.lAxxBXHk-lkp@intel.com/ Signed-off-by: Simon Horman Link: https://lore.kernel.org/r/20240208-xgmac-const-v1-1-e69a1eeabfc8@kernel.org Signed-off-by: Jakub Kicinski Signed-off-by: Greg Kroah-Hartman

net: stmmac: xgmac: fix handling of DPP safety error for DMA channels

2024-02-16T18:14:19Z

[ Upstream commit 46eba193d04f8bd717e525eb4110f3c46c12aec3 ] Commit 56e58d6c8a56 ("net: stmmac: Implement Safety Features in XGMAC core") checks and reports safety errors, but leaves the Data Path Parity Errors for each channel in DMA unhandled at all, lead to a storm of interrupt. Fix it by checking and clearing the DMA_DPP_Interrupt_Status register. Fixes: 56e58d6c8a56 ("net: stmmac: Implement Safety Features in XGMAC core") Signed-off-by: Furong Xu <0x1207@gmail.com> Reviewed-by: Simon Horman Signed-off-by: David S. Miller Signed-off-by: Sasha Levin

net: stmmac: Wait a bit for the reset to take effect

2024-02-01T00:21:09Z

[ Upstream commit a5f5eee282a0aae80227697e1d9c811b1726d31d ] otherwise the synopsys_id value may be read out wrong, because the GMAC_VERSION register might still be in reset state, for at least 1 us after the reset is de-asserted. Add a wait for 10 us before continuing to be on the safe side. > From what have you got that delay value? Just try and error, with very old linux versions and old gcc versions the synopsys_id was read out correctly most of the time (but not always), with recent linux versions and recnet gcc versions it was read out wrongly most of the time, but again not always. I don't have access to the VHDL code in question, so I cannot tell why it takes so long to get the correct values, I also do not have more than a few hardware samples, so I cannot tell how long this timeout must be in worst case. Experimentally I can tell that the register is read several times as zero immediately after the reset is de-asserted, also adding several no-ops is not enough, adding a printk is enough, also udelay(1) seems to be enough but I tried that not very often, and I have not access to many hardware samples to be 100% sure about the necessary delay. And since the udelay here is only executed once per device instance, it seems acceptable to delay the boot for 10 us. BTW: my hardware's synopsys id is 0x37. Fixes: c5e4ddbdfa11 ("net: stmmac: Add support for optional reset control") Signed-off-by: Bernd Edlinger Reviewed-by: Jiri Pirko Reviewed-by: Serge Semin Link: https://lore.kernel.org/r/AS8P193MB1285A810BD78C111E7F6AA34E4752@AS8P193MB1285.EURP193.PROD.OUTLOOK.COM Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin

net: stmmac: ethtool: Fixed calltrace caused by unbalanced disable_irq_wake calls

2024-01-25T23:45:29Z

[ Upstream commit a23aa04042187cbde16f470b49d4ad60d32e9206 ] We found the following dmesg calltrace when testing the GMAC NIC notebook: [9.448656] ------------[ cut here ]------------ [9.448658] Unbalanced IRQ 43 wake disable [9.448673] WARNING: CPU: 3 PID: 1083 at kernel/irq/manage.c:688 irq_set_irq_wake+0xe0/0x128 [9.448717] CPU: 3 PID: 1083 Comm: ethtool Tainted: G O 4.19 #1 [9.448773] ... [9.448774] Call Trace: [9.448781] [<9000000000209b5c>] show_stack+0x34/0x140 [9.448788] [<9000000000d52700>] dump_stack+0x98/0xd0 [9.448794] [<9000000000228610>] __warn+0xa8/0x120 [9.448797] [<9000000000d2fb60>] report_bug+0x98/0x130 [9.448800] [<900000000020a418>] do_bp+0x248/0x2f0 [9.448805] [<90000000002035f4>] handle_bp_int+0x4c/0x78 [9.448808] [<900000000029ea40>] irq_set_irq_wake+0xe0/0x128 [9.448813] [<9000000000a96a7c>] stmmac_set_wol+0x134/0x150 [9.448819] [<9000000000be6ed0>] dev_ethtool+0x1368/0x2440 [9.448824] [<9000000000c08350>] dev_ioctl+0x1f8/0x3e0 [9.448827] [<9000000000bb2a34>] sock_ioctl+0x2a4/0x450 [9.448832] [<900000000046f044>] do_vfs_ioctl+0xa4/0x738 [9.448834] [<900000000046f778>] ksys_ioctl+0xa0/0xe8 [9.448837] [<900000000046f7d8>] sys_ioctl+0x18/0x28 [9.448840] [<9000000000211ab4>] syscall_common+0x20/0x34 [9.448842] ---[ end trace 40c18d9aec863c3e ]--- Multiple disable_irq_wake() calls will keep decreasing the IRQ wake_depth, When wake_depth is 0, calling disable_irq_wake() again, will report the above calltrace. Due to the need to appear in pairs, we cannot call disable_irq_wake() without calling enable_irq_wake(). Fix this by making sure there are no unbalanced disable_irq_wake() calls. Fixes: 3172d3afa998 ("stmmac: support wake up irq from external sources (v3)") Signed-off-by: Qiang Ma Reviewed-by: Simon Horman Link: https://lore.kernel.org/r/20240112021249.24598-1-maqianga@uniontech.com Signed-off-by: Paolo Abeni Signed-off-by: Sasha Levin

net: stmmac: Fix ethool link settings ops for integrated PCS

2024-01-25T23:45:27Z

[ Upstream commit 08300adac3b8dab9e2fd3be0155c7d3093c755f4 ] Currently get/set_link_ksettings ethtool ops are dependent on PCS. When PCS is integrated, it will not have separate link config. Bypass configuring and checking PCS for integrated PCS. Fixes: aa571b6275fb ("net: stmmac: add new switch to struct plat_stmmacenet_data") Tested-by: Andrew Halaney # sa8775p-ride Signed-off-by: Sneh Shah Reviewed-by: Simon Horman Signed-off-by: David S. Miller Signed-off-by: Sasha Levin