From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E0271C3DA7F for ; Mon, 12 Aug 2024 14:18:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type:MIME-Version: References:In-Reply-To:Subject:Cc:To:From:Message-ID:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=kgZkXetmB9JD9jqWSjB+Bradc0aVMUzCKPFH5ldDdOA=; b=A2Jkx3ycf23/dqH1s8vnvZymz4 auqgop3T2UPnKHHNSNOFTxJ2xyYiDBl4ZhtayYCjoR7Fwdb0QdTl8iT6oHoy1SAwYeErFl1CyqMrg H9AUD+ImZIgl/VocHdZE9Qqabqbx/lIxAn8I6iaLoXx/zODvWK6+yHvD21pCd5T+iFxxJ5kIgHtzG EZ6pHCRWUNhwmJICsDYyVkRyMsAVxmJlRPWHXsdKKch/I6B8TVJOAdS4B+E4SQSxgLvNkejGBOckH GrSmdxnW7AU06vws8bY56Rhbdui50JTK8v8Cr2IoD/NKILvPAdimxt/q2dNJgj5mUXY2wtEuIYxk9 Z/fqPItw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sdVsM-00000000YdQ-0wZI; Mon, 12 Aug 2024 14:18:22 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sdVpp-00000000YEo-0b7s for linux-arm-kernel@lists.infradead.org; Mon, 12 Aug 2024 14:16:37 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 41D1361209; Mon, 12 Aug 2024 14:15:44 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E281FC32782; Mon, 12 Aug 2024 14:15:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1723472143; bh=BGRlCIKuiTa0XsHIzgvyQx8GF3ZnAv0GUuP0rAeXyCU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=h92mLzSGxYNshbdDjnHt4dZzxeI4oQDUywM7ziZFFk66puOjtqnEM+7k/VomXLjWW cfMLqMBvuZdWU12nWRhbrUJzXv+uPPuIwHZOP9rZYzB3+xI5yLf2vNgUbfzqXq7xNR Xb9V7k6bZGYZNUdxkShOGLhhVVwYxFkjcOjeFNwxhQcClrf00dkHUK/IxeiKNUUIp2 BuIxmopuQDjVsgk+zHAbz54nqRscAWpvCZjCYyk7IV76H06RgfIzjCwg289/7mgGRs F/LIYoDOL5i9Q683+C0OLNP3rFM1xpGOM6TSdyhCi7aOlZsekQguaEEOcFJCxFRYGN rNOYryGQ5hpPg== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1sdVpl-0032oI-Rk; Mon, 12 Aug 2024 15:15:42 +0100 Date: Mon, 12 Aug 2024 15:15:41 +0100 Message-ID: <864j7p25f6.wl-maz@kernel.org> From: Marc Zyngier To: Breno Leitao Cc: kuba@kernel.org, Sunil Goutham , "David S. Miller" , Eric Dumazet , Paolo Abeni , horms@kernel.org, linux-arm-kernel@lists.infradead.org (moderated list:ARM/CAVIUM THUNDER NETWORK DRIVER), netdev@vger.kernel.org (open list:NETWORKING DRIVERS), linux-kernel@vger.kernel.org (open list) Subject: Re: [PATCH net-next v2] net: thunderx: Unembed netdev structure In-Reply-To: <20240626173503.87636-1-leitao@debian.org> References: <20240626173503.87636-1-leitao@debian.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/29.4 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: leitao@debian.org, kuba@kernel.org, sgoutham@marvell.com, davem@davemloft.net, edumazet@google.com, pabeni@redhat.com, horms@kernel.org, linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240812_071545_381353_C14B51A5 X-CRM114-Status: GOOD ( 21.05 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, 26 Jun 2024 18:35:02 +0100, Breno Leitao wrote: > > Embedding net_device into structures prohibits the usage of flexible > arrays in the net_device structure. For more details, see the discussion > at [1]. > > Un-embed the net_devices from struct lmac by converting them > into pointers, and allocating them dynamically. Use the leverage > alloc_netdev() to allocate the net_device object at > bgx_lmac_enable(). > > The free of the device occurs at bgx_lmac_disable(). > > Do not free_netdevice() if bgx_lmac_enable() fails after lmac->netdev > is allocated, since bgx_lmac_disable() is called if bgx_lmac_enable() > fails, and lmac->netdev will be freed there (similarly to lmac->dmacs). > > Link: https://lore.kernel.org/all/20240229225910.79e224cf@kernel.org/ [1] > Signed-off-by: Breno Leitao > --- > Changelog: > > v2: > * Fixed a wrong dereference in netdev_priv (Jakub) > > .../net/ethernet/cavium/thunder/thunder_bgx.c | 21 +++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) This patch causes my ThunderX box to explode badly: [ 10.022118] thunder_bgx, ver 1.0 [ 10.022594] libata version 3.00 loaded. [ 10.023226] mdio_thunder 0000:01:01.3: Added bus at 87e005003800 [ 10.023757] mdio_thunder 0000:01:01.3: Added bus at 87e005003880 [ 10.035431] thunder_bgx 0000:01:10.0: BGX0 QLM mode: XFI [ 10.045225] Unable to handle kernel NULL pointer dereference at virtual address 00000000000005e8 [ 10.069901] Mem abort info: [ 10.085236] ESR = 0x0000000096000044 [ 10.109767] EC = 0x25: DABT (current EL), IL = 32 bits [ 10.145191] SET = 0, FnV = 0 [ 10.148272] EA = 0, S1PTW = 0 [ 10.151422] FSC = 0x04: level 0 translation fault [ 10.156309] Data abort info: [ 10.159196] ISV = 0, ISS = 0x00000044, ISS2 = 0x00000000 [ 10.164689] CM = 0, WnR = 1, TnD = 0, TagAccess = 0 [ 10.169752] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 [ 10.175076] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000111b43000 [ 10.181533] [00000000000005e8] pgd=0000000000000000, p4d=0000000000000000 [ 10.188328] Internal error: Oops: 0000000096000044 [#1] PREEMPT SMP [ 10.194585] Modules linked in: libahci(E) nvme(E) nvme_core(E) t10_pi(E) mdio_thunder(E) thunder_bgx(E+) libata(E) mdio_devres(E) crc64_rocksoft(E) scsi_mod(E) igb(E+) thunder_xcv(E) mdio_cavium(E) crc64(E) i2c_algo_bit(E) gpio_keys(E) usbhid(E) scsi_common(E) of_mdio(E) fixed_phy(E) fwnode_mdio(E) i2c_thunderx(E) libphy(E) [ 10.223291] CPU: 0 PID: 341 Comm: kworker/0:4 Tainted: G E 6.10.0-rc5-01073-g94833addfaba #3309 [ 10.233368] Hardware name: GIGABYTE MT30-GS0/MT30-GS0, BIOS F02 08/06/2019 [ 10.240231] Workqueue: events work_for_cpu_fn [ 10.244588] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 10.251540] pc : bgx_probe+0x44c/0x640 [thunder_bgx] [ 10.256502] lr : bgx_probe+0x410/0x640 [thunder_bgx] [ 10.261460] sp : ffff800084dd3c80 [ 10.264876] x29: ffff800084dd3c80 x28: 0000000000000000 x27: ffff000ff6772a70 [ 10.272006] x26: ffff00010cb02480 x25: 0000000000000000 x24: ffff80007a325700 [ 10.279136] x23: ffff00010c0e60c8 x22: ffff80008100a3d8 x21: ffff000ff6772a88 [ 10.286266] x20: ffff00010c0e6000 x19: ffff00010cb02480 x18: ffffffffffffffff [ 10.293396] x17: 000000004b2d2331 x16: 00000000b606f3da x15: 0000000000000006 [ 10.300526] x14: 0000000000000000 x13: 3030383330303530 x12: 3065373820746120 [ 10.307656] x11: 7375622064656464 x10: ffff800081e158e8 x9 : ffff800080aa9a30 [ 10.314786] x8 : 0101010101010101 x7 : 0000000000000000 x6 : ffff00010c0e60c8 [ 10.321916] x5 : ffff800084dd3cf8 x4 : 0000000000000000 x3 : 0000000000000000 [ 10.329046] x2 : 0000000000000000 x1 : ffff80007a3296d0 x0 : ffff000ff6772a70 [ 10.336176] Call trace: [ 10.338613] bgx_probe+0x44c/0x640 [thunder_bgx] [ 10.343225] local_pci_probe+0x48/0xb8 [ 10.346966] work_for_cpu_fn+0x24/0x40 [ 10.350706] process_one_work+0x170/0x400 [ 10.354707] worker_thread+0x26c/0x388 [ 10.358446] kthread+0xfc/0x110 [ 10.361580] ret_from_fork+0x10/0x20 [ 10.365150] Code: 52800004 52800003 d2800002 f9401f47 (f902f4e6) [ 10.371232] ---[ end trace 0000000000000000 ]--- and I've confirmed that reverting this patch on top of -rc3 restores normal behaviour. There are two issues with this change: - bgx_lmac_enable() is called *after* bgx_acpi_register_phy() and bgx_init_of_phy(), both expecting netdev to be a valid pointer. - bgx_init_of_phy() populates the MAC addresses for *all* LMACs attached to a given BGX instance, and thus needs netdev for each of them to have been allocated. I have posted a potential fix at [1]. Thanks, M. [1] https://lore.kernel.org/r/20240812141322.1742918-1-maz@kernel.org -- Without deviation from the norm, progress is not possible.