[11/11] sunxi: Add limit with the MMC environment

Message ID 20171221124030.23721-12-maxime.ripard@free-electrons.com
State Superseded
Headers show
Series
  • sunxi: arm64 binary size fixes
Related show

Commit Message

Maxime Ripard Dec. 21, 2017, 12:40 p.m.
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(+)

Comments

André Przywara Dec. 21, 2017, 10:52 p.m. | #1
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
>
Maxime Ripard Dec. 22, 2017, 8:48 a.m. | #2
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

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Jagan Teki Dec. 22, 2017, 9:04 a.m. | #3
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.
Maxime Ripard Jan. 5, 2018, 9:33 a.m. | #4
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

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Patch

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