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 378A9C4332F for ; Fri, 11 Feb 2022 17:13:22 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 5662483B4B; Fri, 11 Feb 2022 18:13:20 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=konsulko.com header.i=@konsulko.com header.b="fj7aRfPR"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 16C4E83A37; Fri, 11 Feb 2022 18:13:18 +0100 (CET) Received: from mail-qt1-x832.google.com (mail-qt1-x832.google.com [IPv6:2607:f8b0:4864:20::832]) (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 2EC6B83504 for ; Fri, 11 Feb 2022 18:13:14 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=trini@konsulko.com Received: by mail-qt1-x832.google.com with SMTP id b5so9591326qtq.11 for ; Fri, 11 Feb 2022 09:13:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=B4TWFrPYgsOzE6+L9oH9wQg0/BWvuUpJCdyK/fobZEs=; b=fj7aRfPRtBsiVNfFnGwT+gnC5066aLwEHG7iJpd9STBj6AimJCQeIJiTfODVdaeIRS /8K2uWBq3zRcp6F544mgdT3aAx77yN/pmSaYICUA01NUD80F34DWKMpxc72hMgfLqw0J XWFP93ciywZH5WgtYcm62gqxJFRd1o0R+4D5E= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=B4TWFrPYgsOzE6+L9oH9wQg0/BWvuUpJCdyK/fobZEs=; b=QspOms4O2cnx3DIus8LC9NLVaPUV0AzPMg0yQEFcTbyuukrr/byvO/EjF8+r3DqJhx JnJBW1cFoCk4jsqIFCTyM4ZdpHAO+n7YjUilj/CDmbK91IZRnEFgCrsWM2xEY9jOlNgO Dm4HjCVZ/DWft8ix3Y9ppMLPMb4KMfzmg5TuX1rnH0QGmnqOnBF/dXuJFZWW39TPRlAq Crivrf5Je/8aCsgX+hIoVSQH22EZhjf1oj4a/iCq9jKALLkhhxhUnOpLSh89IpXrckzT z/Mm2ImUWeeLtJDGD5EBXuLg+A1o6LCAgt1Pn/qsu3tlNj7+tToHW1+DFpDA1Y4QC/O/ Y1Fw== X-Gm-Message-State: AOAM532eO0naeWoW/28X4Yhj4HSzR2k0aDaCVgSerTapY+xqooupmcdY XNBZroxojcXKMabQ80eNEQ1Uzw== X-Google-Smtp-Source: ABdhPJwjow22AJHeAYSiiPPIHrT0jI4y3sU4n5nOPvYuJ30s9B7uk1+hES9y4WIKKL5j0SQWBiazJw== X-Received: by 2002:ac8:4e94:: with SMTP id 20mr1841275qtp.529.1644599592959; Fri, 11 Feb 2022 09:13:12 -0800 (PST) Received: from bill-the-cat (2603-6081-7b01-cbda-2ef0-5dff-fedb-a8ba.res6.spectrum.com. [2603:6081:7b01:cbda:2ef0:5dff:fedb:a8ba]) by smtp.gmail.com with ESMTPSA id v73sm11404676qkb.51.2022.02.11.09.13.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 11 Feb 2022 09:13:12 -0800 (PST) Date: Fri, 11 Feb 2022 12:13:10 -0500 From: Tom Rini To: Adam Ford Cc: Simon Glass , U-Boot Mailing List , Patrice Chotard , Artem Lapkin , Joe Hershberger , Heinrich Schuchardt , Peter Hoyes Subject: Re: [PATCH v3 07/18] pxe: Move pxe_utils files Message-ID: <20220211171310.GA2697206@bill-the-cat> References: <20220209123219.GZ7515@bill-the-cat> <20220210145750.GH2697206@bill-the-cat> <20220211161208.GF2697206@bill-the-cat> <20220211164453.GG2697206@bill-the-cat> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="Lv0dYwq55yjFwMRW" Content-Disposition: inline In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett 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 --Lv0dYwq55yjFwMRW Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Feb 11, 2022 at 11:10:43AM -0600, Adam Ford wrote: > On Fri, Feb 11, 2022 at 10:44 AM Tom Rini wrote: > > > > On Fri, Feb 11, 2022 at 10:39:30AM -0600, Adam Ford wrote: > > > On Fri, Feb 11, 2022 at 10:12 AM Tom Rini wrote: > > > > > > > > On Fri, Feb 11, 2022 at 09:50:32AM -0600, Adam Ford wrote: > > > > > On Thu, Feb 10, 2022 at 8:57 AM Tom Rini wro= te: > > > > > > > > > > > > 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 wrote: > > > > > > > > > > > > > > > > > > > > > > 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 relates to > > > > > > > > > > > booting. > > > > > > > > > > > > > > > > > > > > > +cc lokeshvutla@ti.com > > > > > > > > > > > > > > > > > > > > Simon, > > > > > > > > > > > > > > > > > > > > I can't explain why, but with git bisect, it appears th= is patch breaks > > > > > > > > > > my omap3_logic board (DM3730) by making it wrongly thin= k there is 4GB > > > > > > > > > > of RAM, when in reality there is only 256MB. We have b= oth 256MB and > > > > > > > > > > 512MB parts, and the automatic memory detection has alw= ays 'just > > > > > > > > > > worked' in the past. > > > > > > > > > > > > > > > > > > > > With this patch now, I see: > > > > > > > > > > U-Boot 2022.01-rc1-00185-g262cfb5b15 (Feb 09 2022 - 05:= 23:42 -0600) > > > > > > > > > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Cl= ock 1 GHz > > > > > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Developme= nt Kit > > > > > > > > > > DRAM: 4 GiB > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > With the previous commit, 8018b9af57b5 ("pxe: Tidy up t= he is_pxe > > > > > > > > > > global"), it properly detects the RAM and fully boots. > > > > > > > > > > > > > > > > > > > > U-Boot 2022.01-rc1-00184-g8018b9af57 (Feb 09 2022 - 05:= 21:39 -0600) > > > > > > > > > > > > > > > > > > > > OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Cl= ock 1 GHz > > > > > > > > > > Model: LogicPD Zoom DM3730 Torpedo + Wireless Developme= nt 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 all > > > > > > > > > > defined, so I am having a hard time understanding why t= his would > > > > > > > > > > change behavior or stomp on the the structure that know= s the memory > > > > > > > > > > size. > > > > > > > > > > > > > > > > > > > > If I jump ahead to the current 'master' 531c0089457:("M= erge branch > > > > > > > > > > '2022-02-08-TI-platform-updates') and revert this patc= h, my board > > > > > > > > > > boots correctly again, but I am struggling to understan= d why. > > > > > > > + Marek Beh=FAn > > > > > > > > > > > > > > > > > > > > > > > > > > > Do you have any suggestions for me to try? > > > > > > > > > > > > > > > > > > I would suggest objdump disassemble U-Boot before/after a= nd see what > > > > > > > > > functions have changed. > > > > > > > > > > > > > > > > Keep an eye out for a BSS variable that is used before relo= cation, perhaps? > > > > > > > > > > > > > > I am still investigating, but disabling LTO appears to fix th= e issue > > > > > > > for me. I'd like to keep LTO, so I'm going to attempt to foc= us on the > > > > > > > differences in the affected functions and how this patch make= s 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 suggesti= ons 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 f= eeling > > > > > > > > > > I will be the first to admit thatI am not very good with the asse= mbly > > > > > 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.d= ump > > > > > > broken-working.diff > > > > > cat -n broken-working.diff > > > > > > > > > > The broken-working.diff file with common lines suppressed is 2362= 56 lines long. > > > > > > > > OK, I just use '-d' and not '-S', which might help a little bit. B= ut > > > > you're probably going to still need to edit the dumps and just glob= ally > > > > change all of the addresses to 'XXXXXXXX' so that you'll end up > > > > hopefully only seeing where functions were optimized differently. = But > > > > it might well end up being a bit trickier than that. > > > > > > It looks like none of the object files are showing any content with > > > objdump when LTO is enabled. With a little google search, it appears > > > we need lto-dump. I have some more meetings, but I'll try to spend > > > some more time on it this weekend. > > > > > > > > > > > > When I disable LTO for just pxe_utils.o and redo the same exercis= e, > > > > > the diff file with common-lines removed is 266573 lines long. > > > > > > > > > > Maybe I am not using objdump correctly. I am not all that famili= ar > > > > > 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 loo= k 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 iden= tical: > > > > > > > > > > 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 thoug= ht > > > > > 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 enabl= ed > > > > > 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 > > > > > > > > I feel like it can't be pxe_utils.o itself but rather how LTO is > > > > behaving before/after that change and sorting the object files > > > > differently. If modifying the dumps like I suggested above doesn't= lead > > > > > > That's what I was thinking too. > > > > > > > to more clues, and it doesn't seem to matter what toolchain is used= (are > > > > you using the gcc-11 from kernel.org that we use in docker and > > > > buildman?), I'll try and look as well. > > > > > > I am using GCC 11, but I'm using the version that come with Ubuntu 21= =2E10: > > > > > > Thread model: posix > > > Supported LTO compression algorithms: zlib zstd > > > gcc version 11.2.0 (Ubuntu 11.2.0-5ubuntu1) > > > > OK. FWIW, if it's easier to build and test, I would suggest also trying > > CFLAGS_REMOVE_xxx.o :=3D $(LTO_CFLAGS) > > for all of the obj files under arch/arm/ and board/ and then if that > > also works correctly, re-adding the flags a directory, then file, at a > > time until you've narrowed it down. >=20 > If I'm understanding this correctly, does this mean that you think > it's the stuff that's calling the pxe_utils.o and not the pxe_utils.o > itself? > Doing the CFLAGS_REMOVE on the pxe_utils.o fixes the issue. I think that the change here caused LTO to shuffle files around when linking the resulting binary and exposed a previously existing issue. The closer to root cause is some other early object, likely related to DDR detection, is doing something "special" and failing under LTO. Not LTO'ing that object would be a starting point to seeing what a more proper root fix is. --=20 Tom --Lv0dYwq55yjFwMRW Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmIGmSYACgkQFHw5/5Y0 tyzy6QwAgrC4axikvCQcOn8GvblN1TNuvFQUIV0XMGm6NkdsSOvUwadR7jkiUQz1 KSrUAobC5hCNPVMDMR03VP+feseioyB99nSF1xP75ZFreXw1jqHlRdAmoxgkWEaq QpTjqc2b7X+gFe82LpJoVFUoLvmyzfMB0mTSFA859DzaI77w6CpdyFEk9Ni8VLXP 0bAwH0iDJ3OLJjgeyvCZ/6G8MzQClu+lB8ToL/XWoEq1yXBz5+GkcNQqpxx4DbMe jgr6dn/t/r0ftcg/Kx4SDwsxNH7tLuScLNx8PE2+2qJQlFv1aw48W83o6p7c4Ahm p//QyQpXZxoHaJGS9p/J4LmWq0a3lsOlFDeBZ1oFFqa2+dpYKR9XjqNxm98BnOwH AQmt+7Pvsu1keDAkKgqtJ8k/Llali/HykFroT6WuRJviA8u7RkLEqTaLFY7Y5GQB S272neaaicbKUA0rZmPGu5D7d+LsIrPrfn3QhupECIxEsCPFaKV6QoD5bs0cGAHj fIsHRld9 =prCK -----END PGP SIGNATURE----- --Lv0dYwq55yjFwMRW--