| Message ID | 20171221124030.23721-12-maxime.ripard@free-electrons.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | sunxi: arm64 binary size fixes | expand |
Hi, On 21/12/17 12:40, Maxime Ripard wrote: > The MMC environment offset is getting very close to the end of the U-Boot > binary now. Since we want to make sure this will not overflow, add a size > limit in the board for arm64. arm32 has already that limit enforced in our > custom image generation. > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > --- > include/configs/sunxi-common.h | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h > index 3855c564f914..6236e129a89d 100644 > --- a/include/configs/sunxi-common.h > +++ b/include/configs/sunxi-common.h > @@ -147,6 +147,16 @@ > #endif > > #if defined(CONFIG_ENV_IS_IN_MMC) > + > +#ifdef CONFIG_ARM64 Why is that? Isn't the limit applicable to all sunxi boards using MMC env? Maybe that's actually the better check (thanks for digging this up, btw)? I guess this would avoid to break compatibility with older sunxi-fel versions (those provided by distros not carrying your fix), also avoids blowing up u-boot-sunxi-with-spl.bin to 504K always? Or does this break anything on 32-bit boards? Cheers, Andre. > +/* > + * This is actually (CONFIG_ENV_OFFSET - > + * (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512)), but the value will be used > + * directly in a makefile, without the preprocessor expansion. > + */ > +#define CONFIG_BOARD_SIZE_LIMIT 0x7e000 > +#endif > + > #if CONFIG_MMC_SUNXI_SLOT_EXTRA != -1 > /* If we have two devices (most likely eMMC + MMC), favour the eMMC */ > #define CONFIG_SYS_MMC_ENV_DEV 1 >
On Thu, Dec 21, 2017 at 10:52:04PM +0000, André Przywara wrote: > Hi, > > On 21/12/17 12:40, Maxime Ripard wrote: > > The MMC environment offset is getting very close to the end of the U-Boot > > binary now. Since we want to make sure this will not overflow, add a size > > limit in the board for arm64. arm32 has already that limit enforced in our > > custom image generation. > > > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > > --- > > include/configs/sunxi-common.h | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h > > index 3855c564f914..6236e129a89d 100644 > > --- a/include/configs/sunxi-common.h > > +++ b/include/configs/sunxi-common.h > > @@ -147,6 +147,16 @@ > > #endif > > > > #if defined(CONFIG_ENV_IS_IN_MMC) > > + > > +#ifdef CONFIG_ARM64 > > Why is that? Isn't the limit applicable to all sunxi boards using MMC > env? Maybe that's actually the better check (thanks for digging this up, > btw)? I guess this would avoid to break compatibility with older > sunxi-fel versions (those provided by distros not carrying your fix), > also avoids blowing up u-boot-sunxi-with-spl.bin to 504K always? > > Or does this break anything on 32-bit boards? So there's a couple of arguments there, which are probably all a bit weak, but the sum of them made me do it that way: - I tried to keep the changes as minimal as possible since it's going to be fixes, in one of the late -rc's. We know that the arm32 part works, so I didn't really want to disrupt that. - It's not really optimal, as the expression is used directly in the Makefile and therefore any arithmetic operations can't really be done. The arm32 part can, and is therefore correct in all cases. Here we just bet that the user will never have changed one of the values. Maxime
On Fri, Dec 22, 2017 at 2:18 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Thu, Dec 21, 2017 at 10:52:04PM +0000, André Przywara wrote: >> Hi, >> >> On 21/12/17 12:40, Maxime Ripard wrote: >> > The MMC environment offset is getting very close to the end of the U-Boot >> > binary now. Since we want to make sure this will not overflow, add a size >> > limit in the board for arm64. arm32 has already that limit enforced in our >> > custom image generation. >> > >> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> >> > --- >> > include/configs/sunxi-common.h | 10 ++++++++++ >> > 1 file changed, 10 insertions(+) >> > >> > diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h >> > index 3855c564f914..6236e129a89d 100644 >> > --- a/include/configs/sunxi-common.h >> > +++ b/include/configs/sunxi-common.h >> > @@ -147,6 +147,16 @@ >> > #endif >> > >> > #if defined(CONFIG_ENV_IS_IN_MMC) >> > + >> > +#ifdef CONFIG_ARM64 >> >> Why is that? Isn't the limit applicable to all sunxi boards using MMC >> env? Maybe that's actually the better check (thanks for digging this up, >> btw)? I guess this would avoid to break compatibility with older >> sunxi-fel versions (those provided by distros not carrying your fix), >> also avoids blowing up u-boot-sunxi-with-spl.bin to 504K always? >> >> Or does this break anything on 32-bit boards? > > So there's a couple of arguments there, which are probably all a bit > weak, but the sum of them made me do it that way: > - I tried to keep the changes as minimal as possible since it's > going to be fixes, in one of the late -rc's. We know that the > arm32 part works, so I didn't really want to disrupt that. > - It's not really optimal, as the expression is used directly in the > Makefile and therefore any arithmetic operations can't really be > done. The arm32 part can, and is therefore correct in all > cases. Here we just bet that the user will never have changed one > of the values. Based on the previous conversion, 2018.05 or future version will increase the u-boot partition size. Do we really need this in between which will anyway removed.
On Fri, Dec 22, 2017 at 02:34:35PM +0530, Jagan Teki wrote: > On Fri, Dec 22, 2017 at 2:18 PM, Maxime Ripard > <maxime.ripard@free-electrons.com> wrote: > > On Thu, Dec 21, 2017 at 10:52:04PM +0000, André Przywara wrote: > >> Hi, > >> > >> On 21/12/17 12:40, Maxime Ripard wrote: > >> > The MMC environment offset is getting very close to the end of the U-Boot > >> > binary now. Since we want to make sure this will not overflow, add a size > >> > limit in the board for arm64. arm32 has already that limit enforced in our > >> > custom image generation. > >> > > >> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > >> > --- > >> > include/configs/sunxi-common.h | 10 ++++++++++ > >> > 1 file changed, 10 insertions(+) > >> > > >> > diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h > >> > index 3855c564f914..6236e129a89d 100644 > >> > --- a/include/configs/sunxi-common.h > >> > +++ b/include/configs/sunxi-common.h > >> > @@ -147,6 +147,16 @@ > >> > #endif > >> > > >> > #if defined(CONFIG_ENV_IS_IN_MMC) > >> > + > >> > +#ifdef CONFIG_ARM64 > >> > >> Why is that? Isn't the limit applicable to all sunxi boards using MMC > >> env? Maybe that's actually the better check (thanks for digging this up, > >> btw)? I guess this would avoid to break compatibility with older > >> sunxi-fel versions (those provided by distros not carrying your fix), > >> also avoids blowing up u-boot-sunxi-with-spl.bin to 504K always? > >> > >> Or does this break anything on 32-bit boards? > > > > So there's a couple of arguments there, which are probably all a bit > > weak, but the sum of them made me do it that way: > > - I tried to keep the changes as minimal as possible since it's > > going to be fixes, in one of the late -rc's. We know that the > > arm32 part works, so I didn't really want to disrupt that. > > - It's not really optimal, as the expression is used directly in the > > Makefile and therefore any arithmetic operations can't really be > > done. The arm32 part can, and is therefore correct in all > > cases. Here we just bet that the user will never have changed one > > of the values. > > Based on the previous conversion, 2018.05 or future version will > increase the u-boot partition size. Do we really need this in between > which will anyway removed. Yes, otherwise we might end up again in a situation where we'll allow the compilation of a broken binary. Maxime
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 3855c564f914..6236e129a89d 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -147,6 +147,16 @@ #endif #if defined(CONFIG_ENV_IS_IN_MMC) + +#ifdef CONFIG_ARM64 +/* + * This is actually (CONFIG_ENV_OFFSET - + * (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512)), but the value will be used + * directly in a makefile, without the preprocessor expansion. + */ +#define CONFIG_BOARD_SIZE_LIMIT 0x7e000 +#endif + #if CONFIG_MMC_SUNXI_SLOT_EXTRA != -1 /* If we have two devices (most likely eMMC + MMC), favour the eMMC */ #define CONFIG_SYS_MMC_ENV_DEV 1
The MMC environment offset is getting very close to the end of the U-Boot binary now. Since we want to make sure this will not overflow, add a size limit in the board for arm64. arm32 has already that limit enforced in our custom image generation. Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> --- include/configs/sunxi-common.h | 10 ++++++++++ 1 file changed, 10 insertions(+)