diff mbox

[U-Boot] OpenRD: relocate environment to 640kB

Message ID 1369647746-18120-1-git-send-email-t-uboot@infra-silbe.de
State Superseded
Delegated to: Albert ARIBAUD
Headers show

Commit Message

Sascha Silbe May 27, 2013, 9:42 a.m. UTC
U-Boot has become slightly larger than where the environment sector
was previously located. Saving the environment would brick the device.

Relocate the environment to where it is with the stock (i.e. Marvell
USP) U-Boot version. That should give plenty of room for U-Boot to
grow, including local customisations.

This obviously breaks compatibility with previous mainline versions of
U-Boot. Users will need to back up the environment before an update
and restore it afterwards, or manually copy it to the new address
before the update.

Signed-off-by: Sascha Silbe <t-uboot@infra-silbe.de>
---
 include/configs/openrd.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Albert ARIBAUD June 11, 2013, 10 a.m. UTC | #1
Hi Sascha,

On Mon, 27 May 2013 11:42:26 +0200, Sascha Silbe
<t-uboot@infra-silbe.de> wrote:

> U-Boot has become slightly larger than where the environment sector
> was previously located. Saving the environment would brick the device.
> 
> Relocate the environment to where it is with the stock (i.e. Marvell
> USP) U-Boot version. That should give plenty of room for U-Boot to
> grow, including local customisations.
> 
> This obviously breaks compatibility with previous mainline versions of
> U-Boot. Users will need to back up the environment before an update
> and restore it afterwards, or manually copy it to the new address
> before the update.
> 
> Signed-off-by: Sascha Silbe <t-uboot@infra-silbe.de>
> ---
>  include/configs/openrd.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/configs/openrd.h b/include/configs/openrd.h
> index 53bafe1..ea7f9aa 100644
> --- a/include/configs/openrd.h
> +++ b/include/configs/openrd.h
> @@ -90,8 +90,8 @@
>   * it has to be rounded to sector size
>   */
>  #define CONFIG_ENV_SIZE			0x20000	/* 128k */
> -#define CONFIG_ENV_ADDR			0x60000
> -#define CONFIG_ENV_OFFSET		0x60000	/* env starts here */
> +#define CONFIG_ENV_ADDR			0xa0000
> +#define CONFIG_ENV_OFFSET		0xa0000	/* env starts here */
>  
>  /*
>   * Default environment variables

CC:ing Tom.

This patch is for 2013.10, not 2013.07, but I prefer raising the issue
as early as possible.

If there is no way to make things smoother, then I think the 2013.10
release notes should contain a red, blinking, paragraph about this. I
would *hate* it if people were not warned and given a method to port
their current environment setting over.

Possibly even, the 2013.07 could have a warning about the change to
come, so that people have a better chance yet to prepare for the change.

Amicalement,
Albert ARIBAUD June 22, 2013, 9:29 a.m. UTC | #2
On Tue, 11 Jun 2013 12:00:19 +0200, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:

> Hi Sascha,
> 
> On Mon, 27 May 2013 11:42:26 +0200, Sascha Silbe
> <t-uboot@infra-silbe.de> wrote:
> 
> > U-Boot has become slightly larger than where the environment sector
> > was previously located. Saving the environment would brick the device.
> > 
> > Relocate the environment to where it is with the stock (i.e. Marvell
> > USP) U-Boot version. That should give plenty of room for U-Boot to
> > grow, including local customisations.
> > 
> > This obviously breaks compatibility with previous mainline versions of
> > U-Boot. Users will need to back up the environment before an update
> > and restore it afterwards, or manually copy it to the new address
> > before the update.
> > 
> > Signed-off-by: Sascha Silbe <t-uboot@infra-silbe.de>
> > ---
> >  include/configs/openrd.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/configs/openrd.h b/include/configs/openrd.h
> > index 53bafe1..ea7f9aa 100644
> > --- a/include/configs/openrd.h
> > +++ b/include/configs/openrd.h
> > @@ -90,8 +90,8 @@
> >   * it has to be rounded to sector size
> >   */
> >  #define CONFIG_ENV_SIZE			0x20000	/* 128k */
> > -#define CONFIG_ENV_ADDR			0x60000
> > -#define CONFIG_ENV_OFFSET		0x60000	/* env starts here */
> > +#define CONFIG_ENV_ADDR			0xa0000
> > +#define CONFIG_ENV_OFFSET		0xa0000	/* env starts here */
> >  
> >  /*
> >   * Default environment variables
> 
> CC:ing Tom.
> 
> This patch is for 2013.10, not 2013.07, but I prefer raising the issue
> as early as possible.
> 
> If there is no way to make things smoother, then I think the 2013.10
> release notes should contain a red, blinking, paragraph about this. I
> would *hate* it if people were not warned and given a method to port
> their current environment setting over.
> 
> Possibly even, the 2013.07 could have a warning about the change to
> come, so that people have a better chance yet to prepare for the change.
> 
> Amicalement,

Ping.

Amicalement,
Tom Rini June 22, 2013, 8:57 p.m. UTC | #3
On Sat, Jun 22, 2013 at 11:29:59AM +0200, Albert ARIBAUD wrote:

> On Tue, 11 Jun 2013 12:00:19 +0200, Albert ARIBAUD
> <albert.u.boot@aribaud.net> wrote:
> 
> > Hi Sascha,
> > 
> > On Mon, 27 May 2013 11:42:26 +0200, Sascha Silbe
> > <t-uboot@infra-silbe.de> wrote:
> > 
> > > U-Boot has become slightly larger than where the environment sector
> > > was previously located. Saving the environment would brick the device.
> > > 
> > > Relocate the environment to where it is with the stock (i.e. Marvell
> > > USP) U-Boot version. That should give plenty of room for U-Boot to
> > > grow, including local customisations.
> > > 
> > > This obviously breaks compatibility with previous mainline versions of
> > > U-Boot. Users will need to back up the environment before an update
> > > and restore it afterwards, or manually copy it to the new address
> > > before the update.
> > > 
> > > Signed-off-by: Sascha Silbe <t-uboot@infra-silbe.de>
> > > ---
> > >  include/configs/openrd.h | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/include/configs/openrd.h b/include/configs/openrd.h
> > > index 53bafe1..ea7f9aa 100644
> > > --- a/include/configs/openrd.h
> > > +++ b/include/configs/openrd.h
> > > @@ -90,8 +90,8 @@
> > >   * it has to be rounded to sector size
> > >   */
> > >  #define CONFIG_ENV_SIZE			0x20000	/* 128k */
> > > -#define CONFIG_ENV_ADDR			0x60000
> > > -#define CONFIG_ENV_OFFSET		0x60000	/* env starts here */
> > > +#define CONFIG_ENV_ADDR			0xa0000
> > > +#define CONFIG_ENV_OFFSET		0xa0000	/* env starts here */
> > >  
> > >  /*
> > >   * Default environment variables
> > 
> > CC:ing Tom.
> > 
> > This patch is for 2013.10, not 2013.07, but I prefer raising the issue
> > as early as possible.
> > 
> > If there is no way to make things smoother, then I think the 2013.10
> > release notes should contain a red, blinking, paragraph about this. I
> > would *hate* it if people were not warned and given a method to port
> > their current environment setting over.
> > 
> > Possibly even, the 2013.07 could have a warning about the change to
> > come, so that people have a better chance yet to prepare for the change.
> > 
> > Amicalement,
> 
> Ping.

Oh right, agree your idea here.
Sascha Silbe June 25, 2013, 9:42 a.m. UTC | #4
Hello Albert, hello Tom,

Albert ARIBAUD <albert.u.boot@aribaud.net> writes:

[Move environment to account for increase in U-Boot size]
> This patch is for 2013.10, not 2013.07, but I prefer raising the issue
> as early as possible.
>
> If there is no way to make things smoother, then I think the 2013.10
> release notes should contain a red, blinking, paragraph about this. I
> would *hate* it if people were not warned and given a method to port
> their current environment setting over.
>
> Possibly even, the 2013.07 could have a warning about the change to
> come, so that people have a better chance yet to prepare for the
> change.

The situation has gotten better recently and U-Boot fits into the
previous partition size of 384KiB again. So it isn't broken on OpenRD
anymore and the above would seem like a good approach.

Where are the U-Boot Release Notes located? Who is responsible for
editing them?

Sascha
Albert ARIBAUD June 27, 2013, 9:41 a.m. UTC | #5
Hi Sascha,

On Tue, 25 Jun 2013 11:42:53 +0200, Sascha Silbe
<t-uboot@infra-silbe.de> wrote:

> Hello Albert, hello Tom,
> 
> Albert ARIBAUD <albert.u.boot@aribaud.net> writes:
> 
> [Move environment to account for increase in U-Boot size]
> > This patch is for 2013.10, not 2013.07, but I prefer raising the issue
> > as early as possible.
> >
> > If there is no way to make things smoother, then I think the 2013.10
> > release notes should contain a red, blinking, paragraph about this. I
> > would *hate* it if people were not warned and given a method to port
> > their current environment setting over.
> >
> > Possibly even, the 2013.07 could have a warning about the change to
> > come, so that people have a better chance yet to prepare for the
> > change.
> 
> The situation has gotten better recently and U-Boot fits into the
> previous partition size of 384KiB again. So it isn't broken on OpenRD
> anymore and the above would seem like a good approach.

How well does it fit again, and do you have any idea what caused the
increase in size, and what caused the decrease?

> Where are the U-Boot Release Notes located? Who is responsible for
> editing them?
> 
> Sascha

Amicalement,
Tom Rini July 11, 2013, 4:15 p.m. UTC | #6
On Thu, Jun 27, 2013 at 11:41:59AM +0200, Albert ARIBAUD wrote:
> Hi Sascha,
> 
> On Tue, 25 Jun 2013 11:42:53 +0200, Sascha Silbe
> <t-uboot@infra-silbe.de> wrote:
> 
> > Hello Albert, hello Tom,
> > 
> > Albert ARIBAUD <albert.u.boot@aribaud.net> writes:
> > 
> > [Move environment to account for increase in U-Boot size]
> > > This patch is for 2013.10, not 2013.07, but I prefer raising the issue
> > > as early as possible.
> > >
> > > If there is no way to make things smoother, then I think the 2013.10
> > > release notes should contain a red, blinking, paragraph about this. I
> > > would *hate* it if people were not warned and given a method to port
> > > their current environment setting over.
> > >
> > > Possibly even, the 2013.07 could have a warning about the change to
> > > come, so that people have a better chance yet to prepare for the
> > > change.
> > 
> > The situation has gotten better recently and U-Boot fits into the
> > previous partition size of 384KiB again. So it isn't broken on OpenRD
> > anymore and the above would seem like a good approach.
> 
> How well does it fit again, and do you have any idea what caused the
> increase in size, and what caused the decrease?

I imagine that adding -ffunction-sections/-fdata-sections/--gc-sections
is what brought the size back down again.  We've been adding a lot of
kinda optional code, and to avoid having to ifdef the hell out of
everything, we've been relying on growth not being a big problem or just
ignoring it.
Albert ARIBAUD July 11, 2013, 5:40 p.m. UTC | #7
Hi Tom,

On Thu, 11 Jul 2013 12:15:50 -0400, Tom Rini <trini@ti.com> wrote:

> On Thu, Jun 27, 2013 at 11:41:59AM +0200, Albert ARIBAUD wrote:
> > Hi Sascha,
> > 
> > On Tue, 25 Jun 2013 11:42:53 +0200, Sascha Silbe
> > <t-uboot@infra-silbe.de> wrote:
> > 
> > > Hello Albert, hello Tom,
> > > 
> > > Albert ARIBAUD <albert.u.boot@aribaud.net> writes:
> > > 
> > > [Move environment to account for increase in U-Boot size]
> > > > This patch is for 2013.10, not 2013.07, but I prefer raising the issue
> > > > as early as possible.
> > > >
> > > > If there is no way to make things smoother, then I think the 2013.10
> > > > release notes should contain a red, blinking, paragraph about this. I
> > > > would *hate* it if people were not warned and given a method to port
> > > > their current environment setting over.
> > > >
> > > > Possibly even, the 2013.07 could have a warning about the change to
> > > > come, so that people have a better chance yet to prepare for the
> > > > change.
> > > 
> > > The situation has gotten better recently and U-Boot fits into the
> > > previous partition size of 384KiB again. So it isn't broken on OpenRD
> > > anymore and the above would seem like a good approach.
> > 
> > How well does it fit again, and do you have any idea what caused the
> > increase in size, and what caused the decrease?
> 
> I imagine that adding -ffunction-sections/-fdata-sections/--gc-sections
> is what brought the size back down again.  We've been adding a lot of
> kinda optional code, and to avoid having to ifdef the hell out of
> everything, we've been relying on growth not being a big problem or just
> ignoring it.

... Which in the end can bite back, see anonymous string issue. Are we
heading back to carefully selecting which files we build rather than
building then dropping?

Amicalement,
Tom Rini July 11, 2013, 6:08 p.m. UTC | #8
On Thu, Jul 11, 2013 at 07:40:28PM +0200, Albert ARIBAUD wrote:
> Hi Tom,
> 
> On Thu, 11 Jul 2013 12:15:50 -0400, Tom Rini <trini@ti.com> wrote:
> 
> > On Thu, Jun 27, 2013 at 11:41:59AM +0200, Albert ARIBAUD wrote:
> > > Hi Sascha,
> > > 
> > > On Tue, 25 Jun 2013 11:42:53 +0200, Sascha Silbe
> > > <t-uboot@infra-silbe.de> wrote:
> > > 
> > > > Hello Albert, hello Tom,
> > > > 
> > > > Albert ARIBAUD <albert.u.boot@aribaud.net> writes:
> > > > 
> > > > [Move environment to account for increase in U-Boot size]
> > > > > This patch is for 2013.10, not 2013.07, but I prefer raising the issue
> > > > > as early as possible.
> > > > >
> > > > > If there is no way to make things smoother, then I think the 2013.10
> > > > > release notes should contain a red, blinking, paragraph about this. I
> > > > > would *hate* it if people were not warned and given a method to port
> > > > > their current environment setting over.
> > > > >
> > > > > Possibly even, the 2013.07 could have a warning about the change to
> > > > > come, so that people have a better chance yet to prepare for the
> > > > > change.
> > > > 
> > > > The situation has gotten better recently and U-Boot fits into the
> > > > previous partition size of 384KiB again. So it isn't broken on OpenRD
> > > > anymore and the above would seem like a good approach.
> > > 
> > > How well does it fit again, and do you have any idea what caused the
> > > increase in size, and what caused the decrease?
> > 
> > I imagine that adding -ffunction-sections/-fdata-sections/--gc-sections
> > is what brought the size back down again.  We've been adding a lot of
> > kinda optional code, and to avoid having to ifdef the hell out of
> > everything, we've been relying on growth not being a big problem or just
> > ignoring it.
> 
> ... Which in the end can bite back, see anonymous string issue. Are we
> heading back to carefully selecting which files we build rather than
> building then dropping?

It depends on what we can get from the tools we have, cleanly.  I've
seen, for example, patches to kill all output from SPL, as that was
required to get the required big features into a size constrained SPL.
But I don't think it was clean enough for mainline.  On the other hand,
we've got some large files, and if splitting them up for logical uses
also gets us working around string issues, good for us.
Sascha Silbe July 15, 2013, 9:23 a.m. UTC | #9
Albert ARIBAUD <albert.u.boot@aribaud.net> writes:

>> The situation has gotten better recently and U-Boot fits into the
>> previous partition size of 384KiB again. So it isn't broken on OpenRD
>> anymore and the above would seem like a good approach.
> How well does it fit again, and do you have any idea what caused the
> increase in size, and what caused the decrease?

I had the same questions and tried a few buildman runs, but didn't get a
clear picture. The size was going up and down for various slices of
commits.

With v2013.07-rc3, we are now at 376344B (≈ 96% of 384KiB) for
openrd_ultimate when built on Debian Wheezy using
gcc-4.7-arm-linux-gnueabi from Emdebian.

Is there an equivalent to CONFIG_SPL_MAX_SIZE for the "regular" U-Boot?
Detecting the overlap at build time would prevent bricking the device
using saveenv at run time. As an additional benefit, commits that push
the size beyond the limit would also show up in buildman reports as
build failures.

Sascha
Albert ARIBAUD July 15, 2013, 11:55 a.m. UTC | #10
Hi Sascha,

On Mon, 15 Jul 2013 11:23:54 +0200, Sascha Silbe
<t-uboot@infra-silbe.de> wrote:

> Albert ARIBAUD <albert.u.boot@aribaud.net> writes:
> 
> >> The situation has gotten better recently and U-Boot fits into the
> >> previous partition size of 384KiB again. So it isn't broken on OpenRD
> >> anymore and the above would seem like a good approach.
> > How well does it fit again, and do you have any idea what caused the
> > increase in size, and what caused the decrease?
> 
> I had the same questions and tried a few buildman runs, but didn't get a
> clear picture. The size was going up and down for various slices of
> commits.
> 
> With v2013.07-rc3, we are now at 376344B (≈ 96% of 384KiB) for
> openrd_ultimate when built on Debian Wheezy using
> gcc-4.7-arm-linux-gnueabi from Emdebian.

Thanks for the info.

> Is there an equivalent to CONFIG_SPL_MAX_SIZE for the "regular" U-Boot?
> Detecting the overlap at build time would prevent bricking the device
> using saveenv at run time. As an additional benefit, commits that push
> the size beyond the limit would also show up in buildman reports as
> build failures.

I don't know of a feature like CONFIG_SPL_MAX_SIZE for regular U-Boot.
You could 'port it over' as something like CONFIG_MAX_IMAGE_SIZE for
U-Boot.

Somehow I gather that an approach like the one I sketched out in
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/165510
could bring benefits like making CONFIG_SPL_MAX_SIZE a feature of a
"constraints" component which could be built as well in U-Boot as in
SPL; but that's at the very early RFC stage anyway -- actually, it drew
no reaction at all so far, leading me to think it's not that useful.

> Sascha

Amicalement,
Tom Rini July 15, 2013, 12:19 p.m. UTC | #11
On Mon, Jul 15, 2013 at 11:23:54AM +0200, Sascha Silbe wrote:
> Albert ARIBAUD <albert.u.boot@aribaud.net> writes:
> 
> >> The situation has gotten better recently and U-Boot fits into the
> >> previous partition size of 384KiB again. So it isn't broken on OpenRD
> >> anymore and the above would seem like a good approach.
> > How well does it fit again, and do you have any idea what caused the
> > increase in size, and what caused the decrease?
> 
> I had the same questions and tried a few buildman runs, but didn't get a
> clear picture. The size was going up and down for various slices of
> commits.
> 
> With v2013.07-rc3, we are now at 376344B (??? 96% of 384KiB) for
> openrd_ultimate when built on Debian Wheezy using
> gcc-4.7-arm-linux-gnueabi from Emdebian.
> 
> Is there an equivalent to CONFIG_SPL_MAX_SIZE for the "regular" U-Boot?
> Detecting the overlap at build time would prevent bricking the device
> using saveenv at run time. As an additional benefit, commits that push
> the size beyond the limit would also show up in buildman reports as
> build failures.

Yes, you can try using CONFIG_BOARD_SIZE_LIMIT, which is missing from
the README, but does have a few examples (git grep around).  A patch to
document it, and then a patch to enable for openrd would be much
appreciated.  Thanks!

> 
> Sascha



> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Albert ARIBAUD July 29, 2013, 7:24 a.m. UTC | #12
Hi Sascha,

On Mon, 15 Jul 2013 08:19:57 -0400, Tom Rini <trini@ti.com> wrote:

> On Mon, Jul 15, 2013 at 11:23:54AM +0200, Sascha Silbe wrote:
> > Albert ARIBAUD <albert.u.boot@aribaud.net> writes:
> > 
> > >> The situation has gotten better recently and U-Boot fits into the
> > >> previous partition size of 384KiB again. So it isn't broken on OpenRD
> > >> anymore and the above would seem like a good approach.
> > > How well does it fit again, and do you have any idea what caused the
> > > increase in size, and what caused the decrease?
> > 
> > I had the same questions and tried a few buildman runs, but didn't get a
> > clear picture. The size was going up and down for various slices of
> > commits.
> > 
> > With v2013.07-rc3, we are now at 376344B (??? 96% of 384KiB) for
> > openrd_ultimate when built on Debian Wheezy using
> > gcc-4.7-arm-linux-gnueabi from Emdebian.
> > 
> > Is there an equivalent to CONFIG_SPL_MAX_SIZE for the "regular" U-Boot?
> > Detecting the overlap at build time would prevent bricking the device
> > using saveenv at run time. As an additional benefit, commits that push
> > the size beyond the limit would also show up in buildman reports as
> > build failures.
> 
> Yes, you can try using CONFIG_BOARD_SIZE_LIMIT, which is missing from
> the README, but does have a few examples (git grep around).  A patch to
> document it, and then a patch to enable for openrd would be much
> appreciated.  Thanks!

Sascha, does this mean the env relocate patch here is dropped in favor
of a CONFIG_BOARD_SIZE_LIMIT check at build time?

Amicalement,
Sascha Silbe Aug. 11, 2013, 2:40 p.m. UTC | #13
Tom Rini <trini@ti.com> writes:

> Yes, you can try using CONFIG_BOARD_SIZE_LIMIT, which is missing from
> the README, but does have a few examples (git grep around).  A patch to
> document it, and then a patch to enable for openrd would be much
> appreciated.  Thanks!

Thanks for the pointer. Here we go.

Sascha Silbe (3):
  README: document CONFIG_BOARD_SIZE_LIMIT
  Makefile: check native boot image sizes against
    CONFIG_BOARD_SIZE_LIMIT
  openrd: fail build if U-Boot would overlap with environment in flash

 Makefile                 | 3 +++
 README                   | 5 +++++
 include/configs/openrd.h | 5 +++++
 3 files changed, 13 insertions(+)
Sascha Silbe Aug. 11, 2013, 2:49 p.m. UTC | #14
Hi Albert,

Albert ARIBAUD <albert.u.boot@aribaud.net> writes:
> On Mon, 15 Jul 2013 08:19:57 -0400, Tom Rini <trini@ti.com> wrote:
>> On Mon, Jul 15, 2013 at 11:23:54AM +0200, Sascha Silbe wrote:

>>> With v2013.07-rc3, we are now at 376344B (~ 96% of 384KiB) for
>>> openrd_ultimate when built on Debian Wheezy using
>>> gcc-4.7-arm-linux-gnueabi from Emdebian.
[...]
>> Yes, you can try using CONFIG_BOARD_SIZE_LIMIT, which is missing from
>> the README, but does have a few examples (git grep around).  A patch to
>> document it, and then a patch to enable for openrd would be much
>> appreciated.  Thanks!

> Sascha, does this mean the env relocate patch here is dropped in favor
> of a CONFIG_BOARD_SIZE_LIMIT check at build time?

Given that we're already very close to the limit, we'll likely need the
environment relocation patch sooner or later anyway. I'd prefer it to go
in now as part of the regular release cycle, rather than as a hot-fix
whenever it finally spills over. That also reduces the risk of builds
failing only with particular toolchain versions.

Sascha
diff mbox

Patch

diff --git a/include/configs/openrd.h b/include/configs/openrd.h
index 53bafe1..ea7f9aa 100644
--- a/include/configs/openrd.h
+++ b/include/configs/openrd.h
@@ -90,8 +90,8 @@ 
  * it has to be rounded to sector size
  */
 #define CONFIG_ENV_SIZE			0x20000	/* 128k */
-#define CONFIG_ENV_ADDR			0x60000
-#define CONFIG_ENV_OFFSET		0x60000	/* env starts here */
+#define CONFIG_ENV_ADDR			0xa0000
+#define CONFIG_ENV_OFFSET		0xa0000	/* env starts here */
 
 /*
  * Default environment variables