From mboxrd@z Thu Jan 1 00:00:00 1970 From: Denis Pynkin Date: Mon, 14 Sep 2020 20:08:48 +0300 Subject: [PATCH] mx6qsabrelite: increase offset for environment on SPI In-Reply-To: <20200914141111.GI5110@bill-the-cat> References: <20200914095600.1080364-1-denis.pynkin@collabora.com> <20200914123318.GF5110@bill-the-cat> <9beb9ce6-6769-f0d6-1ae7-dadf125021f6@collabora.com> <20200914132918.GH5110@bill-the-cat> <20200914141111.GI5110@bill-the-cat> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 14.09.2020 17:11, Tom Rini wrote: > On Mon, Sep 14, 2020 at 09:29:18AM -0400, Tom Rini wrote: >> On Mon, Sep 14, 2020 at 04:20:20PM +0300, Denis Pynkin wrote: >>> 14.09.2020 15:33, Tom Rini wrote?: >>>> On Mon, Sep 14, 2020 at 08:03:33AM -0300, Fabio Estevam wrote: >>> >>>>>> The size of the binary created with the default U-boot config is much >>>>>> greater than the default offset for environment `0x60000`. If the new >>>>>> version is flashed onto SPI it overlaps with the stored environment. >>>>>> This leads to U-Boot corruption in case of saving environment onto >>>>>> SPI and non-bootable SabreLite board. >>>>>> >>>>>> Signed-off-by: Denis Pynkin >>>>> >>>>> Reviewed-by: Fabio Estevam >>>>> >>>>> In case you want to detect errors like this again in the future, you could add >>>>> CONFIG_BOARD_SIZE_LIMIT that detects such overlaps in build-time. >>>>> >>>>> Please check commit 033f6ea5fa5f ("mx53loco: Fix U-Boot corruption >>>>> after saving the environment") >>>>> >>>>> The addition of CONFIG_BOARD_SIZE_LIMIT can be a separate patch though. >>>> >>>> Hold on. Can we shrink the board back down so that we don't need to >>>> move the environment? Moving the environment is bad as it will also >>>> break existing users. Thanks! >>> >>> I don't think so to be honest. >>> Current sizes even for `u-boot-nodtb.imx` in my builds are: >>> - 595308 (0x9156C) -- cross-compilation >>> - 470780 (0x72EFC) -- native build >> >> I'd like to have one thread where we see what on earth is going on >> there. That's a rather crazy size difference and very troubling. >> >>> which is much larger than current offset 393216 (0x60000) >>> >>> The offset for environment `CONFIG_ENV_OFFSET=0x60000` were added in >>> commit a09fea1 just a year ago. >>> Not sure if it was tested with SabreLite board tbh -- this is a bulk >>> commit aimed for ARMv8 and sizes of binaries produced with pre-a09fea1 >>> commit are also larger than the current offset. >> >> Ah, so here's what's going on then. Commit a09fea1 wasn't about ARMv8, >> it was about migrating options to the defconfig files. In this case, it >> should have been set to the value you're changing it to now, at least >> from a read of the patch (include/configs/mx6sabre_common.h would set it >> to 0xC0000 if CONFIG_ENV_IS_IN_MMC), but I thought I had done that >> migration with my hack to make a tool that had the in-use value be >> printed. So I'm going to re-check that whole thing to see what else >> might be wrong as well. Thanks! > > Ah, so, mx6qsabrelite falls in to the "nitrogen6x" family and not the > rest of the "sabre" board families. As such, it's always had the env > offset of 0x60000. Jumping back somewhat arbitrarily to v2014.10, I see > with gcc-4.9 a size of 404480 on u-boot.imx which is still bigger. > > The commit message isn't clear however, as environment is in MMC and not > SPI, so SPI booting should be fine. But MMC/eMMC is where this case can > happen (I'm not sure which device is SD slot and which is eMMC on this > hardware off-hand). Thanks! To my shame -- I didn't even thought about the booting from MMC, but yes -- should be the same issue in case if U-Boot is placed on SD-card. This device have 2MB SPI NOR flash there the U-Boot could be flashed with an environment. We are using this approach, so I attempt to describe the issue from this point of view. Should I try to re-word the description? Or may I ask you to add a better description? -- wbr, Denis