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 phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 11767C433EF for ; Fri, 11 Feb 2022 15:50:53 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id CD88583A9D; Fri, 11 Feb 2022 16:50:50 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="CcreGorH"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id D376C83A9E; Fri, 11 Feb 2022 16:50:48 +0100 (CET) Received: from mail-ej1-x633.google.com (mail-ej1-x633.google.com [IPv6:2a00:1450:4864:20::633]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 2318E83A98 for ; Fri, 11 Feb 2022 16:50:45 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=aford173@gmail.com Received: by mail-ej1-x633.google.com with SMTP id qk11so3443007ejb.2 for ; Fri, 11 Feb 2022 07:50:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=TmE4x+gME72cXzp75dS3KeLTtqxTRYwBacN4JdhIwwc=; b=CcreGorH8RLn+pSDgqdD0MMjmbZXaTVfgY2sltapDO+vZCZp2bIiOmMoHH3p9EUfwO nva8k1GaeVjyrnInEFrEUz9tV11R3Z21gCOWxnfyjt5TUvHzw3WmCbAzhpTxUiftN6mR s+XfzlQ7JpvsJsNp4n2AOoaD0i4vvodhYS9iTyYLufkH3jDboyktwLCmLdVXH5HbPMj4 H34fjU7U7+CoxIyALxpf5Lk9ToTcSTQ/LFpQZsZKCq1+EDjzATObJRnsp8D6M5Hw1Hge GchkWAkJhFCqndaVUorVCwyKxe4/P2uR0OWCFUfrmTfXupfZ1fud1xLMTvvA3t6BIHbF cw7w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=TmE4x+gME72cXzp75dS3KeLTtqxTRYwBacN4JdhIwwc=; b=RMdERBLMEeLF393U9GzJzTKf106S1AdhDSxFZxPQgh8GrOsCXqOVpVQEYiH2YKKxAb vAYu/SSud9ffgMrIby6bprXLiV9KhdT2V1TqCImGfUW+Hc5IgJuPOKtL5FqGrE1ttCtB Pgys9+YXzkazBY9pYUT3IRF1y1P9Dr26bJ7gK1JtcLMeZLZWHryku8PQgJ/Eb5m3EAbs O9Qmj4z6hCrOGOk59pEreWBRzuEPuWixQnPbmbWcK+5uYYUXfcSt3e7N74li+yf7ozkA vVXKf0dY+pt1xwj09YO2+COqZz1rgP0Qncaywgm2tXiuSmcf+fVHX4m37pKi6Qp8ITGS M25w== X-Gm-Message-State: AOAM533EfIbdan2jOZukOactTbE5Y1BqsQFPYKmHt4k1uQvHB8qmkvCa YBwXhRmDZxAMQ4cjcNgeOqjkQS1Cntjcz0NTelI= X-Google-Smtp-Source: ABdhPJxHpNvOGRNho8MOrc423+rBJhHmV6d2gyTHRY5pZHIP1MUNtTrSsNLggUGN+ANDwwBg2u/Gx4so6jnkml3PMjw= X-Received: by 2002:a17:906:4787:: with SMTP id cw7mr1977246ejc.504.1644594644209; Fri, 11 Feb 2022 07:50:44 -0800 (PST) MIME-Version: 1.0 References: <20211014184811.482560-1-sjg@chromium.org> <20211014124803.v3.7.Id5595981cd99201c6a2d8b714254d775436a3483@changeid> <20220209123219.GZ7515@bill-the-cat> <20220210145750.GH2697206@bill-the-cat> In-Reply-To: <20220210145750.GH2697206@bill-the-cat> From: Adam Ford Date: Fri, 11 Feb 2022 09:50:32 -0600 Message-ID: Subject: Re: [PATCH v3 07/18] pxe: Move pxe_utils files To: Tom Rini Cc: Simon Glass , U-Boot Mailing List , Patrice Chotard , Artem Lapkin , Joe Hershberger , Heinrich Schuchardt , Peter Hoyes Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean On Thu, Feb 10, 2022 at 8:57 AM Tom Rini wrote: > > On Thu, Feb 10, 2022 at 07:56:52AM -0600, Adam Ford wrote: > > On Wed, Feb 9, 2022 at 11:16 AM Simon Glass wrote: > > > > > > Hi, > > > > > > On Wed, 9 Feb 2022 at 05:32, Tom Rini wrote: > > > > > > > > On Wed, Feb 09, 2022 at 05:40:03AM -0600, Adam Ford wrote: > > > > > On Thu, Oct 14, 2021 at 1:50 PM Simon Glass wr= ote: > > > > > > > > > > > > Move the header file into the main include/ directory so we can= use it > > > > > > from the bootmethod code. Move the C file into boot/ since it r= elates to > > > > > > booting. > > > > > > > > > > > +cc lokeshvutla@ti.com > > > > > > > > > > Simon, > > > > > > > > > > I can't explain why, but with git bisect, it appears this patch b= reaks > > > > > my omap3_logic board (DM3730) by making it wrongly think there is= 4GB > > > > > of RAM, when in reality there is only 256MB. We have both 256MB = and > > > > > 512MB parts, and the automatic memory detection has always 'just > > > > > worked' in the past. > > > > > > > > > > With this patch now, I see: > > > > > U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:23:42 -060= 0) > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > DRAM: 4 GiB > > > > > > > > > > > > > > > With the previous commit, 8018b9af57b5 ("pxe: Tidy up the is_pxe > > > > > global"), it properly detects the RAM and fully boots. > > > > > > > > > > U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:21:39 -060= 0) > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 1 GHz > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Development Kit > > > > > DRAM: 256 MiB > > > > > NAND: 512 MiB > > > > > MMC: OMAP SD/MMC: 0 > > > > > Loading Environment from NAND... OK > > > > > OMAP die ID: 619e00029ff800000168300f1502501f > > > > > Net: eth0: ethernet@08000000 > > > > > Hit any key to stop autoboot: 0 > > > > > OMAP Logic # > > > > > > > > > > I have CONFIG_CMD_BOOTM, CONFIG_CMD_PXE and CONFIG_CMD_SYSBOOT a= ll > > > > > defined, so I am having a hard time understanding why this would > > > > > change behavior or stomp on the the structure that knows the memo= ry > > > > > size. > > > > > > > > > > If I jump ahead to the current 'master' 531c0089457:("Merge branc= h > > > > > '2022-02-08-TI-platform-updates') and revert this patch, my boar= d > > > > > boots correctly again, but I am struggling to understand why. > > + Marek Beh=C3=BAn > > > > > > > > > > > > Do you have any suggestions for me to try? > > > > > > > > I would suggest objdump disassemble U-Boot before/after and see wha= t > > > > functions have changed. > > > > > > Keep an eye out for a BSS variable that is used before relocation, pe= rhaps? > > > > I am still investigating, but disabling LTO appears to fix the issue > > for me. I'd like to keep LTO, so I'm going to attempt to focus on the > > differences in the affected functions and how this patch makes LTO > > behave differently. > > > > The disassembly of U-Boot is large, so it's going to take me a bit of > > time to investigate. If someone has any LTO-related suggestions that > > I could try, I'd be open to try them too. > > Wait, the disassembly is large, or the differences between the > disassembly, before/after this change alone, are large? It's feeling I will be the first to admit thatI am not very good with the assembly side of things, but this is what I did: git checkout master make CROSS_COMPILE=3Darm-linux-gnueabihf- -j8 arm-linux-gnueabihf-objdump -S u-boot > broken.dump git revert 262cfb5b15420a1aea465745a821e684b3dfa153 make CROSS_COMPILE=3Darm-linux-gnueabihf- -j8 arm-linux-gnueabihf-objdump -S u-boot > working.dump diff --side-by-side --suppress-common-lines broken.dump working.dump > broken-working.diff cat -n broken-working.diff The broken-working.diff file with common lines suppressed is 236256 lines l= ong. When I disable LTO for just pxe_utils.o and redo the same exercise, the diff file with common-lines removed is 266573 lines long. Maybe I am not using objdump correctly. I am not all that familiar with this code either, so I am not sure which variables should be in BSS. I did a search in both working and non-working dumps to look for keyworks like BSS, but from what I can tell, both have similar functions: gd->mon_len =3D (ulong)&__bss_end - (ulong)_start; /* TODO: use (ulong)&__bss_end - (ulong)&__text_start; ? */ gd->mon_len =3D (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; gd->mon_len =3D (ulong)&__bss_end - (ulong)_start; * reserve memory for U-Boot code, data & bss 8011051a : #if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_EARLY_BSS) CLEAR_BSS #if !defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SPL_EARLY_BSS) CLEAR_BSS CLEAR_BSS When I grepped for mon_len, both sets of dumps looked nearly identical: aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len working.dump lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); 80112724 : static int setup_mon_len(void) gd->mon_len =3D (ulong)&__bss_end - (ulong)_start; 80112726: 4903 ldr r1, [pc, #12] ; (80112734 ) 80112728: 4b03 ldr r3, [pc, #12] ; (80112738 ) gd->mon_len =3D (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; gd->mon_len =3D (ulong)&__bss_end - (ulong)_start; gd->ram_top =3D board_get_usable_ram_top(gd->mon_len); gd->relocaddr -=3D gd->mon_len; gd->mon_len >> 10, gd->relocaddr); ip =3D mon_lengths[yleap]; aford@aford-OptiPlex-7050:~/src/u-boot$ grep mon_len broken.dump lmb_reserve(lmb, (phys_addr_t)(uintptr_t)_start, gd->mon_len); 80110398 : static int setup_mon_len(void) gd->mon_len =3D (ulong)&__bss_end - (ulong)_start; 8011039a: 4903 ldr r1, [pc, #12] ; (801103a8 ) 8011039c: 4b03 ldr r3, [pc, #12] ; (801103ac ) gd->mon_len =3D (ulong)&__bss_end - CONFIG_SYS_MONITOR_BASE; gd->mon_len =3D (ulong)&__bss_end - (ulong)_start; gd->ram_top =3D board_get_usable_ram_top(gd->mon_len); gd->relocaddr -=3D gd->mon_len; gd->mon_len >> 10, gd->relocaddr); ip =3D mon_lengths[yleap]; aford@aford-OptiPlex-7050:~/src/u-boot$ Since I think I narrowed it down to the pxe_utils.o file, I thought I'd do an objdump of both the working and non-working versions of pxe_utils.o and this is where it got interesting. With LTO building pxe_utils.o, the dump looks empty: arm-linux-gnueabihf-objdump -S boot/pxe_utils.o > pxe-notworking.dump cat pxe-notworking.dump boot/pxe_utils.o: file format elf32-littlearm ^-- no actual code dump If I take the working version of this same file without LTO enabled and do a dump, and it's 2291 lines long and full of functions. I tried adding some __used to the non-static function names, but that didn't appear to make any difference to the objdump of pxe_utils.o adam > like just how we remove the LTO flags from > arch/arm/mach-omap2/omap3/board.o something else might also need that, > OR digging more to find out issue LTO is exposing in terms of variables > being in the data and not BSS section and needing initialization or > similar. > > -- > Tom