[PATCH] arm/tegra: select AUTO_ZRELADDR by default

[PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Peter De Schrijver]

This patch causes the kernel uncompressor to determine the physical address
of the SDRAM at runtime. This allows the kernel to boot on both tegra20 and
tegra30 even though SDRAM is at different physical addresses on both SoCs.

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 arch/arm/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 472a7f8..474737b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -596,6 +596,7 @@ config ARCH_TEGRA
 	select HAVE_CLK
 	select HAVE_SCHED_CLOCK
 	select ARCH_HAS_CPUFREQ
+	select AUTO_ZRELADDR
 	help
 	  This enables support for NVIDIA Tegra based systems (Tegra APX,
 	  Tegra 6xx and Tegra 2 series).
-- 
1.7.0.4

RE: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Stephen Warren]

Peter De Schrijver wrote at Tuesday, September 27, 2011 7:08 PM:
> This patch causes the kernel uncompressor to determine the physical address
> of the SDRAM at runtime. This allows the kernel to boot on both tegra20 and
> tegra30 even though SDRAM is at different physical addresses on both SoCs.
> 
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>

An alternative would be to simply add that config option to the relevant
defconfig/.config file. I see both cases in use in the kernel. Still, the
code change this enables looks fine to just turn on all the time, and will
be needed for Tegra30 support, so I'm fine just selecting it as you have.

Acked-by: Stephen Warren <swarren@nvidia.com>

> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 472a7f8..474737b 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -596,6 +596,7 @@ config ARCH_TEGRA
>  	select HAVE_CLK
>  	select HAVE_SCHED_CLOCK
>  	select ARCH_HAS_CPUFREQ
> +	select AUTO_ZRELADDR
>  	help
>  	  This enables support for NVIDIA Tegra based systems (Tegra APX,
>  	  Tegra 6xx and Tegra 2 series).

P.S. Since this patch relates to Tegra, you should CC the Tegra maintainers
and list; I've done so on this message. I also added Arnd; he might take
this through the arm-soc tree.

-- 
nvpublic

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Olof Johansson]

Hi,

On Wed, Sep 28, 2011 at 10:50 AM, Stephen Warren <swarren@nvidia.com> wrote:
> Peter De Schrijver wrote at Tuesday, September 27, 2011 7:08 PM:
>> This patch causes the kernel uncompressor to determine the physical address
>> of the SDRAM at runtime. This allows the kernel to boot on both tegra20 and
>> tegra30 even though SDRAM is at different physical addresses on both SoCs.
>>
>> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
>
> An alternative would be to simply add that config option to the relevant
> defconfig/.config file. I see both cases in use in the kernel. Still, the
> code change this enables looks fine to just turn on all the time, and will
> be needed for Tegra30 support, so I'm fine just selecting it as you have.
>
> Acked-by: Stephen Warren <swarren@nvidia.com>
>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 472a7f8..474737b 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -596,6 +596,7 @@ config ARCH_TEGRA
>>       select HAVE_CLK
>>       select HAVE_SCHED_CLOCK
>>       select ARCH_HAS_CPUFREQ
>> +     select AUTO_ZRELADDR
>>       help
>>         This enables support for NVIDIA Tegra based systems (Tegra APX,
>>         Tegra 6xx and Tegra 2 series).
>
> P.S. Since this patch relates to Tegra, you should CC the Tegra maintainers
> and list; I've done so on this message. I also added Arnd; he might take
> this through the arm-soc tree.

While it does touch a file outside of arch/arm/mach-tegra, it's for
the tegra options and is not likely to cause conflicts upstream. I'll
apply it with the rest of tegra patches for 3.2 here.

So: Applied, thanks.


-Olof

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Russell King - ARM Linux]

On Thu, Oct 13, 2011 at 04:38:46PM -0700, Olof Johansson wrote:
> Hi,
> 
> On Wed, Sep 28, 2011 at 10:50 AM, Stephen Warren <swarren@nvidia.com> wrote:
> > Peter De Schrijver wrote at Tuesday, September 27, 2011 7:08 PM:
> >> This patch causes the kernel uncompressor to determine the physical address
> >> of the SDRAM at runtime. This allows the kernel to boot on both tegra20 and
> >> tegra30 even though SDRAM is at different physical addresses on both SoCs.
> >>
> >> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> >
> > An alternative would be to simply add that config option to the relevant
> > defconfig/.config file. I see both cases in use in the kernel. Still, the
> > code change this enables looks fine to just turn on all the time, and will
> > be needed for Tegra30 support, so I'm fine just selecting it as you have.
> >
> > Acked-by: Stephen Warren <swarren@nvidia.com>
> >
> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> >> index 472a7f8..474737b 100644
> >> --- a/arch/arm/Kconfig
> >> +++ b/arch/arm/Kconfig
> >> @@ -596,6 +596,7 @@ config ARCH_TEGRA
> >>       select HAVE_CLK
> >>       select HAVE_SCHED_CLOCK
> >>       select ARCH_HAS_CPUFREQ
> >> +     select AUTO_ZRELADDR
> >>       help
> >>         This enables support for NVIDIA Tegra based systems (Tegra APX,
> >>         Tegra 6xx and Tegra 2 series).
> >
> > P.S. Since this patch relates to Tegra, you should CC the Tegra maintainers
> > and list; I've done so on this message. I also added Arnd; he might take
> > this through the arm-soc tree.
> 
> While it does touch a file outside of arch/arm/mach-tegra, it's for
> the tegra options and is not likely to cause conflicts upstream. I'll
> apply it with the rest of tegra patches for 3.2 here.
> 
> So: Applied, thanks.

I'll point out that this makes Tegra incompatible with ZBOOT_ROM, which
can still be enabled.  ZBOOT_ROM=y AUTO_ZRELADDR=y is an invalid
configuration at runtime.

RE: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Stephen Warren]

Russell King wrote at Friday, October 14, 2011 1:15 AM:
> On Thu, Oct 13, 2011 at 04:38:46PM -0700, Olof Johansson wrote:
> > Hi,
> >
> > On Wed, Sep 28, 2011 at 10:50 AM, Stephen Warren <swarren@nvidia.com> wrote:
> > > Peter De Schrijver wrote at Tuesday, September 27, 2011 7:08 PM:
> > >> This patch causes the kernel uncompressor to determine the physical address
> > >> of the SDRAM at runtime. This allows the kernel to boot on both tegra20 and
> > >> tegra30 even though SDRAM is at different physical addresses on both SoCs.
> > >>
> > >> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> > >
> > > An alternative would be to simply add that config option to the relevant
> > > defconfig/.config file. I see both cases in use in the kernel. Still, the
> > > code change this enables looks fine to just turn on all the time, and will
> > > be needed for Tegra30 support, so I'm fine just selecting it as you have.
> > >
> > > Acked-by: Stephen Warren <swarren@nvidia.com>
> > >
> > >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > >> index 472a7f8..474737b 100644
> > >> --- a/arch/arm/Kconfig
> > >> +++ b/arch/arm/Kconfig
> > >> @@ -596,6 +596,7 @@ config ARCH_TEGRA
> > >>       select HAVE_CLK
> > >>       select HAVE_SCHED_CLOCK
> > >>       select ARCH_HAS_CPUFREQ
> > >> +     select AUTO_ZRELADDR
> > >>       help
> > >>         This enables support for NVIDIA Tegra based systems (Tegra APX,
> > >>         Tegra 6xx and Tegra 2 series).
> > >
> > > P.S. Since this patch relates to Tegra, you should CC the Tegra maintainers
> > > and list; I've done so on this message. I also added Arnd; he might take
> > > this through the arm-soc tree.
> >
> > While it does touch a file outside of arch/arm/mach-tegra, it's for
> > the tegra options and is not likely to cause conflicts upstream. I'll
> > apply it with the rest of tegra patches for 3.2 here.
> >
> > So: Applied, thanks.
> 
> I'll point out that this makes Tegra incompatible with ZBOOT_ROM, which
> can still be enabled.  ZBOOT_ROM=y AUTO_ZRELADDR=y is an invalid
> configuration at runtime.

I see that at least one machine solves that by doing:

config ARCH_EP93XX
...
        select AUTO_ZRELADDR if !ZBOOT_ROM

That said, given the way Tegra works, ZBOOT_ROM isn't useful; typically,
systems don't use NOR flash for storage of the kernel, so I don't think
ZBOOT_ROM is likely to be used. Perhaps we should just depend on !ZBOOT_ROM?

-- 
nvpublic

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Arnd Bergmann]

On Friday 14 October 2011, Stephen Warren wrote:
> Russell King wrote at Friday, October 14, 2011 1:15 AM:
> > On Thu, Oct 13, 2011 at 04:38:46PM -0700, Olof Johansson wrote:
> > 
> > I'll point out that this makes Tegra incompatible with ZBOOT_ROM, which
> > can still be enabled.  ZBOOT_ROM=y AUTO_ZRELADDR=y is an invalid
> > configuration at runtime.
> 
> I see that at least one machine solves that by doing:
> 
> config ARCH_EP93XX
> ...
>         select AUTO_ZRELADDR if !ZBOOT_ROM
> 
> That said, given the way Tegra works, ZBOOT_ROM isn't useful; typically,
> systems don't use NOR flash for storage of the kernel, so I don't think
> ZBOOT_ROM is likely to be used. Perhaps we should just depend on !ZBOOT_ROM?

That sounds like a correct fix, but also confusing because the top-level
subarchitecture selection should not depend on too many things that are
not obvious to someone configuring the kernel. From the perspective
of least suprise I would think that instead ZBOOTROM should depend on
!ARCH_TEGRA, but there is no precedent for this in the current Kconfig.

You mention that tegra30 will require AUTO_ZRELADDR. Does that mean that
with tegra30 merged you would no longer be able to build any tegra
kernel? If not, the best solution for now would probably be to
put AUTO_ZRELADDR into the defconfig for now (provided that tegra20 can
still boot without it) and make tegra30 depend on AUTO_ZRELADDR within
the tegra Kconfig.

This is only a temporary solution I think, because it is one of the
areas that we will have to revisit once we make it possible to combine
multiple subarchitectures into a single kernel. I have some ideas how
I would express that in Kconfig but I'm sure that we will have a lot
of debate over it before we coem to a solution that works for everyone.

	Arnd

RE: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Stephen Warren]

Arnd Bergmann wrote at Friday, October 14, 2011 9:30 AM:
...
> You mention that tegra30 will require AUTO_ZRELADDR.

Well, just to be clear, here's the situation I think:

Tegra20's SDRAM starts at physical address 0.

Tegra30's SDRAM starts at physical address 2G.

To support that, we could either:

a) Introduce a new Kconfig variable for Tegra30, make T20/T30 mutually
exclusive, and update arch/arm/mach-tegra/Makefile.boot to set zreladdr
etc. based on the new Tegra30 config variable too. Then, there's no need
for AUTO_ZRELADDR anywhere.

b) Have no new config variable, build a unified T20/T30 kernel, leave
Makefile.boot untouched, and rely on using AUTO_ZRELADDR for Tegra30 to
account for the different SDRAM physical addresses.

> Does that mean that with tegra30 merged you would no longer be able
> to build any tegra kernel?

Sorry, I don't quite understand that question.

-- 
nvpublic

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Arnd Bergmann]

On Friday 14 October 2011, Stephen Warren wrote:
> Arnd Bergmann wrote at Friday, October 14, 2011 9:30 AM:
> ...
> > You mention that tegra30 will require AUTO_ZRELADDR.
> 
> Well, just to be clear, here's the situation I think:
> 
> Tegra20's SDRAM starts at physical address 0.
> 
> Tegra30's SDRAM starts at physical address 2G.
> 
> To support that, we could either:
> 
> a) Introduce a new Kconfig variable for Tegra30, make T20/T30 mutually
> exclusive, and update arch/arm/mach-tegra/Makefile.boot to set zreladdr
> etc. based on the new Tegra30 config variable too. Then, there's no need
> for AUTO_ZRELADDR anywhere.
> 
> b) Have no new config variable, build a unified T20/T30 kernel, leave
> Makefile.boot untouched, and rely on using AUTO_ZRELADDR for Tegra30 to
> account for the different SDRAM physical addresses.

Ok, thanks for the explanation, that makes it much clearer. For
completeness, you could also do both of the above and make T20/T30 mutually
exclusive unless AUTO_ZRELADDR is set. That might be more complex than
necessary, I don't know.

	Arnd

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Olof Johansson]

On Fri, Oct 14, 2011 at 9:27 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 14 October 2011, Stephen Warren wrote:
>> Arnd Bergmann wrote at Friday, October 14, 2011 9:30 AM:
>> ...
>> > You mention that tegra30 will require AUTO_ZRELADDR.
>>
>> Well, just to be clear, here's the situation I think:
>>
>> Tegra20's SDRAM starts at physical address 0.
>>
>> Tegra30's SDRAM starts at physical address 2G.
>>
>> To support that, we could either:
>>
>> a) Introduce a new Kconfig variable for Tegra30, make T20/T30 mutually
>> exclusive, and update arch/arm/mach-tegra/Makefile.boot to set zreladdr
>> etc. based on the new Tegra30 config variable too. Then, there's no need
>> for AUTO_ZRELADDR anywhere.
>>
>> b) Have no new config variable, build a unified T20/T30 kernel, leave
>> Makefile.boot untouched, and rely on using AUTO_ZRELADDR for Tegra30 to
>> account for the different SDRAM physical addresses.
>
> Ok, thanks for the explanation, that makes it much clearer. For
> completeness, you could also do both of the above and make T20/T30 mutually
> exclusive unless AUTO_ZRELADDR is set. That might be more complex than
> necessary, I don't know.

That is likely to get messy.

Seems like there could be some use for a (silent) option for a
platform to indicate that it can do XIP kernel (or zImage), and thus
not able to use AUTO_ZRELADDR (or other options that require rewriting
text segment of zImage or kernel).

Language gets awkard though, since it'd be a negative option (or all
platforms would need to add it). MACH_XIP_UNSUPPORTED perhaps?


-Olof

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Nicolas Pitre]

On Fri, 14 Oct 2011, Olof Johansson wrote:

> On Fri, Oct 14, 2011 at 9:27 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Friday 14 October 2011, Stephen Warren wrote:
> >> Arnd Bergmann wrote at Friday, October 14, 2011 9:30 AM:
> >> ...
> >> > You mention that tegra30 will require AUTO_ZRELADDR.
> >>
> >> Well, just to be clear, here's the situation I think:
> >>
> >> Tegra20's SDRAM starts at physical address 0.
> >>
> >> Tegra30's SDRAM starts at physical address 2G.
> >>
> >> To support that, we could either:
> >>
> >> a) Introduce a new Kconfig variable for Tegra30, make T20/T30 mutually
> >> exclusive, and update arch/arm/mach-tegra/Makefile.boot to set zreladdr
> >> etc. based on the new Tegra30 config variable too. Then, there's no need
> >> for AUTO_ZRELADDR anywhere.
> >>
> >> b) Have no new config variable, build a unified T20/T30 kernel, leave
> >> Makefile.boot untouched, and rely on using AUTO_ZRELADDR for Tegra30 to
> >> account for the different SDRAM physical addresses.
> >
> > Ok, thanks for the explanation, that makes it much clearer. For
> > completeness, you could also do both of the above and make T20/T30 mutually
> > exclusive unless AUTO_ZRELADDR is set. That might be more complex than
> > necessary, I don't know.
> 
> That is likely to get messy.
> 
> Seems like there could be some use for a (silent) option for a
> platform to indicate that it can do XIP kernel (or zImage), and thus
> not able to use AUTO_ZRELADDR (or other options that require rewriting
> text segment of zImage or kernel).
> 
> Language gets awkard though, since it'd be a negative option (or all
> platforms would need to add it). MACH_XIP_UNSUPPORTED perhaps?

Please hold on -- I'm on it.


Nicolas

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Russell King - ARM Linux]

On Fri, Oct 14, 2011 at 09:44:34AM -0700, Olof Johansson wrote:
> That is likely to get messy.
> 
> Seems like there could be some use for a (silent) option for a
> platform to indicate that it can do XIP kernel (or zImage), and thus
> not able to use AUTO_ZRELADDR (or other options that require rewriting
> text segment of zImage or kernel).

It's not that.

The options are:

AUTO_ZRELADDR=n ZBOOT_ROM=n => fixed address for decompressed kernel
	image to be decompressed to from zreladdr make variable,
	decompressor can be loaded to any RAM address.

AUTO_ZRELADDR=y ZBOOT_ROM=n => address for decompressed kernel calculated
	from location of zImage in RAM, decompressor must be loaded to
	the correct place in RAM and must always copy itself out of
	the way prior to decompressing.

AUTO_ZRELADDR=n ZBOOT_ROM=y => fixed address for decompressed kernel
	image to be decompressed to from zreladdr make variable,
	decompressor can be loaded to any RAM address which doesn't
	overlap its BSS segment, or decompressor programmed into
	read-only memory at the address to which it was compiled.

AUTO_ZRELADDR=y ZBOOT_ROM=y => invalid (think about it - this results
	in a zImage which is built to be run from read-only memory,
	and try to write the decompressed image into that read-only
	memory.)

This has nothing to do with XIP or not - XIP is a completely separate story,
and if you're building an XIP kernel you're not using the decompressor (so
AUTO_ZRELADDR and ZBOOT_ROM don't even feature.)  Neither does the dynamic
code patching stuff like ARM_PATCH_PHYS_VIRT.

ARM_PATCH_PHYS_VIRT has no bearing on AUTO_ZRELADDR or ZBOOT_ROM; that is
a property of the decompressed kernel image itself, and while your
decompressor may restrict where it can decompress the kernel image to,
the decompressed image itself remains free of those dependencies (and
eg, can be turned into a uImage with the appropriate load address
for other platforms.)

As I've said in the past, what I'd _like_ to see is that ARM_PATCH_PHYS_VIRT
be enabled for everything irrespective of any other configuration, and
we're just left with AUTO_ZRELADDR / ZBOOT_ROM to worry about.  The
overhead from the P:V patching is soo small that it's not really worth
even having the option in the general case - the only time when P:V
patching doesn't work is with non-linear translations found on some
sparsemem platforms.

But... one thing to note is that it _is_ common to load the decompressor
at a _different_ address to that where the kernel ultimately ends up
residing to avoid the additional copy in the decompressor.  My experience
shows that this is quite common on the platforms I had supplied.  This
means that if we default to AUTO_ZRELADDR for !ZBOOT_ROM, we end up
having to have developers change their uboot setups to avoid unexpected
results.

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Nicolas Pitre]

On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:

> But... one thing to note is that it _is_ common to load the decompressor
> at a _different_ address to that where the kernel ultimately ends up
> residing to avoid the additional copy in the decompressor.  My experience
> shows that this is quite common on the platforms I had supplied.  This
> means that if we default to AUTO_ZRELADDR for !ZBOOT_ROM, we end up
> having to have developers change their uboot setups to avoid unexpected
> results.

Currently, U-Boot insists on having a uImage with a fixed absolute load 
address.  This is currently provided by the zreladdr value, whether or 
not AUTO_ZRELADDR is set.  I consider this as a persisting uImage 
limitation.

Either u-Boot gets fixed so it can work with plain zImage (and this 
certainly will happen once the pressure from people wanting a single 
kernel to work on targets with different load addresses increase.  
Tegra is one such example.

Or we create a u-Boot specific Kconfig menu for uImage options that 
would be common to all architectures and kick it out from the ARM 
specific makefile.  This is not solving the u-Boot limitation though.

In either cases this is a u-Boot problem that needs fixing on the u-Boot 
side in the end.


Nicolas

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Russell King - ARM Linux]

On Fri, Oct 14, 2011 at 04:06:21PM -0400, Nicolas Pitre wrote:
> Currently, U-Boot insists on having a uImage with a fixed absolute load 
> address.  This is currently provided by the zreladdr value, whether or 
> not AUTO_ZRELADDR is set.  I consider this as a persisting uImage 
> limitation.
> 
> Either u-Boot gets fixed so it can work with plain zImage (and this 
> certainly will happen once the pressure from people wanting a single 
> kernel to work on targets with different load addresses increase.  
> Tegra is one such example.
> 
> Or we create a u-Boot specific Kconfig menu for uImage options that 
> would be common to all architectures and kick it out from the ARM 
> specific makefile.  This is not solving the u-Boot limitation though.
> 
> In either cases this is a u-Boot problem that needs fixing on the u-Boot 
> side in the end.

I don't think that's so with the various flavours of platform specific
uboot which float around.  For instance, on the OMAP4430 SDP, the
following commands were used as supplied to load a uImage off the SD
card into RAM at a different address to which it was built for, and
execute it at that address:

mmcinit 0
fatload mmc 0 0x80300000 uImage
bootm 80300000

Whether the 'bootm' command then copied the image and called it there,
or whether it executed it at 0x80300000 I've no idea - but why then
load the image at a different address in the first place?

RE: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Stephen Warren]

Russell King wrote at Friday, October 14, 2011 2:12 PM:
> On Fri, Oct 14, 2011 at 04:06:21PM -0400, Nicolas Pitre wrote:
> > Currently, U-Boot insists on having a uImage with a fixed absolute load
> > address.  This is currently provided by the zreladdr value, whether or
> > not AUTO_ZRELADDR is set.  I consider this as a persisting uImage
> > limitation.
> >
> > Either u-Boot gets fixed so it can work with plain zImage (and this
> > certainly will happen once the pressure from people wanting a single
> > kernel to work on targets with different load addresses increase.
> > Tegra is one such example.
> >
> > Or we create a u-Boot specific Kconfig menu for uImage options that
> > would be common to all architectures and kick it out from the ARM
> > specific makefile.  This is not solving the u-Boot limitation though.
> >
> > In either cases this is a u-Boot problem that needs fixing on the u-Boot
> > side in the end.
> 
> I don't think that's so with the various flavours of platform specific
> uboot which float around.  For instance, on the OMAP4430 SDP, the
> following commands were used as supplied to load a uImage off the SD
> card into RAM at a different address to which it was built for, and
> execute it at that address:
> 
> mmcinit 0
> fatload mmc 0 0x80300000 uImage
> bootm 80300000
> 
> Whether the 'bootm' command then copied the image and called it there,
> or whether it executed it at 0x80300000 I've no idea - but why then
> load the image at a different address in the first place?

Yes, U-Boot's image handling looks at the image at 80300000, extracts the
"load address" from the image header, memcpy()'s the image to that address,
then jumps to the "entry address" in the image header.

-- 
nvpublic

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Russell King - ARM Linux]

On Fri, Oct 14, 2011 at 01:16:27PM -0700, Stephen Warren wrote:
> Russell King wrote at Friday, October 14, 2011 2:12 PM:
> > I don't think that's so with the various flavours of platform specific
> > uboot which float around.  For instance, on the OMAP4430 SDP, the
> > following commands were used as supplied to load a uImage off the SD
> > card into RAM at a different address to which it was built for, and
> > execute it at that address:
> > 
> > mmcinit 0
> > fatload mmc 0 0x80300000 uImage
> > bootm 80300000
> > 
> > Whether the 'bootm' command then copied the image and called it there,
> > or whether it executed it at 0x80300000 I've no idea - but why then
> > load the image at a different address in the first place?
> 
> Yes, U-Boot's image handling looks at the image at 80300000, extracts the
> "load address" from the image header, memcpy()'s the image to that address,
> then jumps to the "entry address" in the image header.

Ah, so specifying that address is just a total waste of space then,
because you might as well specify an address which results in the
copy not being necessary.

However, I'd expect that uboot is dumb enough to still do the copy
irrespective of whether its already in the right place.

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Tixy]

On Fri, 2011-10-14 at 21:19 +0100, Russell King - ARM Linux wrote:
> On Fri, Oct 14, 2011 at 01:16:27PM -0700, Stephen Warren wrote:
> > Russell King wrote at Friday, October 14, 2011 2:12 PM:
> > > I don't think that's so with the various flavours of platform specific
> > > uboot which float around.  For instance, on the OMAP4430 SDP, the
> > > following commands were used as supplied to load a uImage off the SD
> > > card into RAM at a different address to which it was built for, and
> > > execute it at that address:
> > > 
> > > mmcinit 0
> > > fatload mmc 0 0x80300000 uImage
> > > bootm 80300000
> > > 
> > > Whether the 'bootm' command then copied the image and called it there,
> > > or whether it executed it at 0x80300000 I've no idea - but why then
> > > load the image at a different address in the first place?
> > 
> > Yes, U-Boot's image handling looks at the image at 80300000, extracts the
> > "load address" from the image header, memcpy()'s the image to that address,
> > then jumps to the "entry address" in the image header.
> 
> Ah, so specifying that address is just a total waste of space then,
> because you might as well specify an address which results in the
> copy not being necessary.
> 
> However, I'd expect that uboot is dumb enough to still do the copy
> irrespective of whether its already in the right place.

U-Boot doesn't do an unnecessary copy if the body of the image is
already at the load address specified in the image header.

Unfortunately, it also doesn't do a copy if the header of the image is
at the load address, (I guess to support images constructed with an
embedded header). This means that

  fatload mmc 0 0x80300000 uImage
  bootm 0x80300000

will crash as it will attempt to boot Linux by executing the image
header. To successfully boot without requiring a relocation we would
need:

  fatload mmc 0 0x80300000-sizeof(header) uImage
  bootm 0x80300000

-- 
Tixy

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Nicolas Pitre]

On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:

> On Fri, Oct 14, 2011 at 04:06:21PM -0400, Nicolas Pitre wrote:
> > Currently, U-Boot insists on having a uImage with a fixed absolute load 
> > address.  This is currently provided by the zreladdr value, whether or 
> > not AUTO_ZRELADDR is set.  I consider this as a persisting uImage 
> > limitation.
> > 
> > Either u-Boot gets fixed so it can work with plain zImage (and this 
> > certainly will happen once the pressure from people wanting a single 
> > kernel to work on targets with different load addresses increase.  
> > Tegra is one such example.
> > 
> > Or we create a u-Boot specific Kconfig menu for uImage options that 
> > would be common to all architectures and kick it out from the ARM 
> > specific makefile.  This is not solving the u-Boot limitation though.
> > 
> > In either cases this is a u-Boot problem that needs fixing on the u-Boot 
> > side in the end.
> 
> I don't think that's so with the various flavours of platform specific
> uboot which float around.  For instance, on the OMAP4430 SDP, the
> following commands were used as supplied to load a uImage off the SD
> card into RAM at a different address to which it was built for, and
> execute it at that address:
> 
> mmcinit 0
> fatload mmc 0 0x80300000 uImage
> bootm 80300000
> 
> Whether the 'bootm' command then copied the image and called it there,
> or whether it executed it at 0x80300000 I've no idea - but why then
> load the image at a different address in the first place?

Maybe because the lower memory is already in use by other u-Boot related 
things, or more likely for no specific reasons at all.  But u-Boot 
_will_ relocate the image if it is not lot loaded at the address being 
recorded in its header as specified by the -a argument of mkimage.  And 
currently we use zreladdr for that when "make uImage" is used, meaning 
that by default no one is currently benefiting from the kernel 
decompressor relocation avoidance optimization on u-Boot.

Of course the u-Boot people are also claiming that u-Boot should take 
care of the kernel decompression not zImage, which I personally disagree 
with.


Nicolas

RE: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Stephen Warren]

Nicolas Pitre wrote at Friday, October 14, 2011 2:06 PM:
> On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:
> 
> > But... one thing to note is that it _is_ common to load the decompressor
> > at a _different_ address to that where the kernel ultimately ends up
> > residing to avoid the additional copy in the decompressor.  My experience
> > shows that this is quite common on the platforms I had supplied.  This
> > means that if we default to AUTO_ZRELADDR for !ZBOOT_ROM, we end up
> > having to have developers change their uboot setups to avoid unexpected
> > results.
> 
> Currently, U-Boot insists on having a uImage with a fixed absolute load
> address.  This is currently provided by the zreladdr value, whether or
> not AUTO_ZRELADDR is set.  I consider this as a persisting uImage
> limitation.

Well, that value only comes from zreladdr when running U-Boot's mkimage from
the kernel makefile rather than some other packaging script:-)

> Either u-Boot gets fixed so it can work with plain zImage (and this
> certainly will happen once the pressure from people wanting a single
> kernel to work on targets with different load addresses increase.
> Tegra is one such example.

I proposed to solve it by adding a new image format within the uImage;
"kernel-rel" rather than "kernel", where the load/entry address encoded
into the uImage is specified relative to "start of SDRAM" rather than
as an absolute address. This way, Tegra20 and Tegra30 will have the
same layout of U-Boot location, uImage load address, kernel execute
address etc. within SDRAM, but all those addresses might end up being
based at physical address 0, or physical address 2G.

Here's the patch I proposed:

http://patchwork.ozlabs.org/patch/119017/

What are your thoughts on this? I hope it will work for multi-SoC image
across vendors, although I suppose finding a common load address across
a bunch of different SoC's memory layouts might be tough?

I did originally briefly look into getting U-Boot to boot a zImage, but
that looked like a far more invasive patch. There were rumours of some
chip's custom U-Boot already having such support, but I couldn't find
it, nor any evidence of such support in mainline U-Boot.

> Or we create a u-Boot specific Kconfig menu for uImage options that
> would be common to all architectures and kick it out from the ARM
> specific makefile.  This is not solving the u-Boot limitation though.
> 
> In either cases this is a u-Boot problem that needs fixing on the u-Boot
> side in the end.

-- 
nvpublic

RE: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Nicolas Pitre]

On Fri, 14 Oct 2011, Stephen Warren wrote:

> Nicolas Pitre wrote at Friday, October 14, 2011 2:06 PM:
> > Currently, U-Boot insists on having a uImage with a fixed absolute load
> > address.  This is currently provided by the zreladdr value, whether or
> > not AUTO_ZRELADDR is set.  I consider this as a persisting uImage
> > limitation.
> 
> Well, that value only comes from zreladdr when running U-Boot's mkimage from
> the kernel makefile rather than some other packaging script:-)

So what?  The whole idea of a kernel that can be loaded anywhere is to 
_not_ have its load address packaged into it.  The fact that you need a 
separate packaging step just for the specification of that address is 
rather silly.

> > Either u-Boot gets fixed so it can work with plain zImage (and this
> > certainly will happen once the pressure from people wanting a single
> > kernel to work on targets with different load addresses increase.
> > Tegra is one such example.
> 
> I proposed to solve it by adding a new image format within the uImage;
> "kernel-rel" rather than "kernel", where the load/entry address encoded
> into the uImage is specified relative to "start of SDRAM" rather than
> as an absolute address. This way, Tegra20 and Tegra30 will have the
> same layout of U-Boot location, uImage load address, kernel execute
> address etc. within SDRAM, but all those addresses might end up being
> based at physical address 0, or physical address 2G.
> 
> Here's the patch I proposed:
> 
> http://patchwork.ozlabs.org/patch/119017/

By all means, please push your patch upstream and free us from this 
u-Boot misery!

> What are your thoughts on this? I hope it will work for multi-SoC image
> across vendors, although I suppose finding a common load address across
> a bunch of different SoC's memory layouts might be tough?

No it is not.  We know where the decompressed kernel will end up 
relative to the start of RAM, so it is always safe to simply load the 
uImage 
there.  Better yet is to load it with an additional offset corresponding 
to the decompressed kernel size (plus some small gap) in order to allow 
the decompressor skip the relocation step.  This is all trivial to 
determine.

> I did originally briefly look into getting U-Boot to boot a zImage, but
> that looked like a far more invasive patch. There were rumours of some
> chip's custom U-Boot already having such support, but I couldn't find
> it, nor any evidence of such support in mainline U-Boot.

FRom my clone of the u-Boot repo:

$ git grep -l zImage
README
arch/arm/cpu/ixp/npe/include/IxTimeSyncAcc.h
arch/sh/lib/zimageboot.c
arch/x86/include/asm/zimage.h
arch/x86/lib/zimage.c
doc/README.mx35pdk
include/configs/apollon.h
include/configs/mpc7448hpc2.h
include/configs/mpr2.h
include/configs/ms7720se.h
include/configs/pxa255_idp.h
include/configs/trizepsiv.h

Even the name of some of those files clearly hints zImage support.

In any case, loading zImage should be even simpler than loading uImage.  
It is the same as loading uImage except that you just have to skip the 
checking and relocating steps.


Nicolas

RE: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Stephen Warren]

Nicolas Pitre wrote at Friday, October 14, 2011 2:45 PM:
> On Fri, 14 Oct 2011, Stephen Warren wrote:
...
> > I did originally briefly look into getting U-Boot to boot a zImage, but
> > that looked like a far more invasive patch. There were rumours of some
> > chip's custom U-Boot already having such support, but I couldn't find
> > it, nor any evidence of such support in mainline U-Boot.
> 
> FRom my clone of the u-Boot repo:
> 
> $ git grep -l zImage
> README
> arch/sh/lib/zimageboot.c
> arch/x86/lib/zimage.c
> ...
> 
> Even the name of some of those files clearly hints zImage support.
> 
> In any case, loading zImage should be even simpler than loading uImage.
> It is the same as loading uImage except that you just have to skip the
> checking and relocating steps.

Just by way of background in case anyone is wondering why I wrote the
patch I did:

Those files both implement custom commands "zimageboot" and "zboot". I
was looking for integration with the existing "bootm" command.

The advantage of re-using "bootm" for this is that it already supports
all the stuff like setting up kernel command-lines, initrds, knowing how
to pass the FDT to the kernel, and whatever other OS-specific setup might
be required.

The disadvantage of adding zImage support to bootm is that I'd have to
teach a bunch of U-Boot image handling code about a new image format; it
already knows about "legacy uImage", "FIT" images, and I'd have to add a
third "zImage" format. Doing so would at least require adding a lot of
"case IMAGE_FORMAT_ZIMAGE" everywhere, but it'd probably be best to add
some kind of vtable for image formats to move all the image-format
knowledge into format-specific files, leaving the users of the images
with much smaller code.

I didn't feel like making such a large change. Hence, I chose to make a
small change to the existing uImage support.

Now admittedly I did say I didn't find any traces of zImage support, which
isn't what I'm saying here; I guess I forgot about the stuff I did find
soon after I chose the path of modifying the uImage formats.

-- 
nvpublic

RE: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Nicolas Pitre]

On Fri, 14 Oct 2011, Stephen Warren wrote:

> Nicolas Pitre wrote at Friday, October 14, 2011 2:45 PM:
> > On Fri, 14 Oct 2011, Stephen Warren wrote:
> ...
> > > I did originally briefly look into getting U-Boot to boot a zImage, but
> > > that looked like a far more invasive patch. There were rumours of some
> > > chip's custom U-Boot already having such support, but I couldn't find
> > > it, nor any evidence of such support in mainline U-Boot.
> > 
> > FRom my clone of the u-Boot repo:
> > 
> > $ git grep -l zImage
> > README
> > arch/sh/lib/zimageboot.c
> > arch/x86/lib/zimage.c
> > ...
> > 
> > Even the name of some of those files clearly hints zImage support.
> > 
> > In any case, loading zImage should be even simpler than loading uImage.
> > It is the same as loading uImage except that you just have to skip the
> > checking and relocating steps.
> 
> Just by way of background in case anyone is wondering why I wrote the
> patch I did:
> 
> Those files both implement custom commands "zimageboot" and "zboot". I
> was looking for integration with the existing "bootm" command.
> 
> The advantage of re-using "bootm" for this is that it already supports
> all the stuff like setting up kernel command-lines, initrds, knowing how
> to pass the FDT to the kernel, and whatever other OS-specific setup might
> be required.

Absolutely.  And that is a must-have.

> The disadvantage of adding zImage support to bootm is that I'd have to
> teach a bunch of U-Boot image handling code about a new image format; it
> already knows about "legacy uImage", "FIT" images, and I'd have to add a
> third "zImage" format. Doing so would at least require adding a lot of
> "case IMAGE_FORMAT_ZIMAGE" everywhere, but it'd probably be best to add
> some kind of vtable for image formats to move all the image-format
> knowledge into format-specific files, leaving the users of the images
> with much smaller code.
> 
> I didn't feel like making such a large change. Hence, I chose to make a
> small change to the existing uImage support.

And that change is certainly valuable.  Some people do want the extra 
feature from the uImage format.  REmoving its absolute address 
limitation is a good thing.

However, also supporting zImage directly on ARM doesn't have to be that 
hard.  Instead of adding "case IMAGE_FORMAT_ZIMAGE" everywhere, all you 
would have to do is to load zImage in memory according to the load 
address provided as an argument to the command, and then fake a uImage 
load by artificially creating the uImage header at run time with the 
data that u-Boot needs later on.


Nicolas

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Rob Herring]

On 10/14/2011 04:01 PM, Stephen Warren wrote:
> Nicolas Pitre wrote at Friday, October 14, 2011 2:45 PM:
>> On Fri, 14 Oct 2011, Stephen Warren wrote:
> ...
>>> I did originally briefly look into getting U-Boot to boot a zImage, but
>>> that looked like a far more invasive patch. There were rumours of some
>>> chip's custom U-Boot already having such support, but I couldn't find
>>> it, nor any evidence of such support in mainline U-Boot.
>>
>> FRom my clone of the u-Boot repo:
>>
>> $ git grep -l zImage
>> README
>> arch/sh/lib/zimageboot.c
>> arch/x86/lib/zimage.c
>> ...
>>
>> Even the name of some of those files clearly hints zImage support.
>>
>> In any case, loading zImage should be even simpler than loading uImage.
>> It is the same as loading uImage except that you just have to skip the
>> checking and relocating steps.
> 
> Just by way of background in case anyone is wondering why I wrote the
> patch I did:
> 
> Those files both implement custom commands "zimageboot" and "zboot". I
> was looking for integration with the existing "bootm" command.
> 
> The advantage of re-using "bootm" for this is that it already supports
> all the stuff like setting up kernel command-lines, initrds, knowing how
> to pass the FDT to the kernel, and whatever other OS-specific setup might
> be required.
> 
> The disadvantage of adding zImage support to bootm is that I'd have to
> teach a bunch of U-Boot image handling code about a new image format; it
> already knows about "legacy uImage", "FIT" images, and I'd have to add a
> third "zImage" format. Doing so would at least require adding a lot of
> "case IMAGE_FORMAT_ZIMAGE" everywhere, but it'd probably be best to add
> some kind of vtable for image formats to move all the image-format
> knowledge into format-specific files, leaving the users of the images
> with much smaller code.
> 
> I didn't feel like making such a large change. Hence, I chose to make a
> small change to the existing uImage support.
> 
> Now admittedly I did say I didn't find any traces of zImage support, which
> isn't what I'm saying here; I guess I forgot about the stuff I did find
> soon after I chose the path of modifying the uImage formats.
> 

FYI, this exact topic has been discussed on the Linaro weekly
boot-architecture call and list. The discussion has been more general in
terms of what can be done to make installing and updating kernels
easier/work for distros.

https://wiki.linaro.org/OfficeofCTO/BootArchitecture

Rob

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Nicolas Pitre]

On Fri, 14 Oct 2011, Arnd Bergmann wrote:

> On Friday 14 October 2011, Stephen Warren wrote:
> > Arnd Bergmann wrote at Friday, October 14, 2011 9:30 AM:
> > ...
> > > You mention that tegra30 will require AUTO_ZRELADDR.
> > 
> > Well, just to be clear, here's the situation I think:
> > 
> > Tegra20's SDRAM starts at physical address 0.
> > 
> > Tegra30's SDRAM starts at physical address 2G.
> > 
> > To support that, we could either:
> > 
> > a) Introduce a new Kconfig variable for Tegra30, make T20/T30 mutually
> > exclusive, and update arch/arm/mach-tegra/Makefile.boot to set zreladdr
> > etc. based on the new Tegra30 config variable too. Then, there's no need
> > for AUTO_ZRELADDR anywhere.
> > 
> > b) Have no new config variable, build a unified T20/T30 kernel, leave
> > Makefile.boot untouched, and rely on using AUTO_ZRELADDR for Tegra30 to
> > account for the different SDRAM physical addresses.
> 
> Ok, thanks for the explanation, that makes it much clearer. For
> completeness, you could also do both of the above and make T20/T30 mutually
> exclusive unless AUTO_ZRELADDR is set. That might be more complex than
> necessary, I don't know.

The way I'm restructuring things around this is that AUTO_ZRELADDR will 
always be active by default, just like ARM_PATCH_PHYS_VIRT now.  This 
platform specific exclusion thinking is a step backward so I'd prefer if 
people would refrain from going there for the moment.


Nicolas

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Russell King - ARM Linux]

On Fri, Oct 14, 2011 at 02:01:07PM -0400, Nicolas Pitre wrote:
> The way I'm restructuring things around this is that AUTO_ZRELADDR will 
> always be active by default, just like ARM_PATCH_PHYS_VIRT now.  This 
> platform specific exclusion thinking is a step backward so I'd prefer if 
> people would refrain from going there for the moment.

Are you expecting everyone to change the way they load the zImage
overnight then?

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Nicolas Pitre]

On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:

> On Fri, Oct 14, 2011 at 02:01:07PM -0400, Nicolas Pitre wrote:
> > The way I'm restructuring things around this is that AUTO_ZRELADDR will 
> > always be active by default, just like ARM_PATCH_PHYS_VIRT now.  This 
> > platform specific exclusion thinking is a step backward so I'd prefer if 
> > people would refrain from going there for the moment.
> 
> Are you expecting everyone to change the way they load the zImage
> overnight then?

No, of course.  But adding restrictions in the kernel build because 
u-Boot's own image format dictates such restrictions doesn't make sense.  
Those restrictions must be pushed towards the uImage encapsulation step, 
not higher the kernel config hierarchy.


Nicolas

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Russell King - ARM Linux]

On Fri, Oct 14, 2011 at 04:14:12PM -0400, Nicolas Pitre wrote:
> On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:
> 
> > On Fri, Oct 14, 2011 at 02:01:07PM -0400, Nicolas Pitre wrote:
> > > The way I'm restructuring things around this is that AUTO_ZRELADDR will 
> > > always be active by default, just like ARM_PATCH_PHYS_VIRT now.  This 
> > > platform specific exclusion thinking is a step backward so I'd prefer if 
> > > people would refrain from going there for the moment.
> > 
> > Are you expecting everyone to change the way they load the zImage
> > overnight then?
> 
> No, of course.  But adding restrictions in the kernel build because 
> u-Boot's own image format dictates such restrictions doesn't make sense.  
> Those restrictions must be pushed towards the uImage encapsulation step, 
> not higher the kernel config hierarchy.

You're not understanding again.

I'm talking about people who _explicitly_ load the zImage at a different
address to which the decompressed image ends up.  With AUTO_ZRELADDR=y
their setup will break unless they stop that behaviour, which takes
away one of the advantages of using the zImage format.

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Nicolas Pitre]

On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:

> On Fri, Oct 14, 2011 at 04:14:12PM -0400, Nicolas Pitre wrote:
> > On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:
> > 
> > > On Fri, Oct 14, 2011 at 02:01:07PM -0400, Nicolas Pitre wrote:
> > > > The way I'm restructuring things around this is that AUTO_ZRELADDR will 
> > > > always be active by default, just like ARM_PATCH_PHYS_VIRT now.  This 
> > > > platform specific exclusion thinking is a step backward so I'd prefer if 
> > > > people would refrain from going there for the moment.
> > > 
> > > Are you expecting everyone to change the way they load the zImage
> > > overnight then?
> > 
> > No, of course.  But adding restrictions in the kernel build because 
> > u-Boot's own image format dictates such restrictions doesn't make sense.  
> > Those restrictions must be pushed towards the uImage encapsulation step, 
> > not higher the kernel config hierarchy.
> 
> You're not understanding again.
> 
> I'm talking about people who _explicitly_ load the zImage at a different
> address to which the decompressed image ends up.  With AUTO_ZRELADDR=y
> their setup will break unless they stop that behaviour, which takes
> away one of the advantages of using the zImage format.

Would you care to explain where you got this from?  Because I really do 
not understand what you're saying indeed.

With AUTO_ZRELADDR=y you _still_ can load zImage to a different location 
from where the decompressed kernel ends up.


Nicolas

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Russell King - ARM Linux]

On Fri, Oct 14, 2011 at 04:31:12PM -0400, Nicolas Pitre wrote:
> On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:
> 
> > On Fri, Oct 14, 2011 at 04:14:12PM -0400, Nicolas Pitre wrote:
> > > On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:
> > > 
> > > > On Fri, Oct 14, 2011 at 02:01:07PM -0400, Nicolas Pitre wrote:
> > > > > The way I'm restructuring things around this is that AUTO_ZRELADDR will 
> > > > > always be active by default, just like ARM_PATCH_PHYS_VIRT now.  This 
> > > > > platform specific exclusion thinking is a step backward so I'd prefer if 
> > > > > people would refrain from going there for the moment.
> > > > 
> > > > Are you expecting everyone to change the way they load the zImage
> > > > overnight then?
> > > 
> > > No, of course.  But adding restrictions in the kernel build because 
> > > u-Boot's own image format dictates such restrictions doesn't make sense.  
> > > Those restrictions must be pushed towards the uImage encapsulation step, 
> > > not higher the kernel config hierarchy.
> > 
> > You're not understanding again.
> > 
> > I'm talking about people who _explicitly_ load the zImage at a different
> > address to which the decompressed image ends up.  With AUTO_ZRELADDR=y
> > their setup will break unless they stop that behaviour, which takes
> > away one of the advantages of using the zImage format.
> 
> Would you care to explain where you got this from?  Because I really do 
> not understand what you're saying indeed.

My I point out that it's you who decided that I was talking about u-boot
when I said no such thing in my message.  I merely pointed out about
those people who may be loading the zImage elsewhere in memory and using
that facility to cut down on the boot time.  u-boot can't load zImages
directly.

Yet you started nattering on about uboot - which we know is a pile of
crap for dealing with this stuff.

But ultimately, how people achieve the loading of the zImage is beyond
the scope of what I stated: whether that's not using u-boot but some
other boot loader, or maybe using mkimage outside of the kernel build,
or whatever.

> With AUTO_ZRELADDR=y you _still_ can load zImage to a different location 
> from where the decompressed kernel ends up.

You are correct for some values of 'different location' but not all -
and how can we know _what_ people are doing?  We don't.

#ifdef CONFIG_AUTO_ZRELADDR
                @ determine final kernel image address
                mov     r4, pc
                and     r4, r4, #0xf8000000
                add     r4, r4, #TEXT_OFFSET
#else
                ldr     r4, =zreladdr
#endif

So this means the decompressor _must_ run within the first 128MB chunk
of memory for the resulting kernel to be correctly placed at expected
place - at the beginning of system memory + TEXT_OFFSET.

Can we know that this is always the case?  I don't think so.
Can we expect there to be regressions if we force AUTO_ZRELADDR=y?  We'd
be stupid not to expect them.

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Nicolas Pitre]

On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:

> On Fri, Oct 14, 2011 at 04:31:12PM -0400, Nicolas Pitre wrote:
> > On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:
> > 
> > > On Fri, Oct 14, 2011 at 04:14:12PM -0400, Nicolas Pitre wrote:
> > > > On Fri, 14 Oct 2011, Russell King - ARM Linux wrote:
> > > > 
> > > > > On Fri, Oct 14, 2011 at 02:01:07PM -0400, Nicolas Pitre wrote:
> > > > > > The way I'm restructuring things around this is that AUTO_ZRELADDR will 
> > > > > > always be active by default, just like ARM_PATCH_PHYS_VIRT now.  This 
> > > > > > platform specific exclusion thinking is a step backward so I'd prefer if 
> > > > > > people would refrain from going there for the moment.
> > > > > 
> > > > > Are you expecting everyone to change the way they load the zImage
> > > > > overnight then?
> > > > 
> > > > No, of course.  But adding restrictions in the kernel build because 
> > > > u-Boot's own image format dictates such restrictions doesn't make sense.  
> > > > Those restrictions must be pushed towards the uImage encapsulation step, 
> > > > not higher the kernel config hierarchy.
> > > 
> > > You're not understanding again.
> > > 
> > > I'm talking about people who _explicitly_ load the zImage at a different
> > > address to which the decompressed image ends up.  With AUTO_ZRELADDR=y
> > > their setup will break unless they stop that behaviour, which takes
> > > away one of the advantages of using the zImage format.
> > 
> > Would you care to explain where you got this from?  Because I really do 
> > not understand what you're saying indeed.
> 
> My I point out that it's you who decided that I was talking about u-boot
> when I said no such thing in my message.  I merely pointed out about
> those people who may be loading the zImage elsewhere in memory and using
> that facility to cut down on the boot time.  u-boot can't load zImages
> directly.
> 
> Yet you started nattering on about uboot - which we know is a pile of
> crap for dealing with this stuff.

OK, sorry for confusing matters.

>From your statement I inferred that making AUTO_ZRELADDR=y would be 
useless, because this is actually true with u-Boot.  Making 
AUTO_ZRELADDR the default allows for cleaning up zreladdr away, but that 
will break "make uImage".  I've yet to find the best way around that.

> > With AUTO_ZRELADDR=y you _still_ can load zImage to a different location 
> > from where the decompressed kernel ends up.
> 
> You are correct for some values of 'different location' but not all -
> and how can we know _what_ people are doing?  We don't.
> 
> #ifdef CONFIG_AUTO_ZRELADDR
>                 @ determine final kernel image address
>                 mov     r4, pc
>                 and     r4, r4, #0xf8000000
>                 add     r4, r4, #TEXT_OFFSET
> #else
>                 ldr     r4, =zreladdr
> #endif
> 
> So this means the decompressor _must_ run within the first 128MB chunk
> of memory for the resulting kernel to be correctly placed at expected
> place - at the beginning of system memory + TEXT_OFFSET.
> 
> Can we know that this is always the case?  I don't think so.

This is certainly the case for a big chunk of legacy ARM machines that 
don't have more than 128MB of RAM.  Discontigous memory banks may break 
that assumption but that's still a large limiting factor.

> Can we expect there to be regressions if we force AUTO_ZRELADDR=y?  We'd
> be stupid not to expect them.

They should be expected indeed.  However we must weight the benefits 
against the possible regressions.

For example, on X86 they decided at some point that the zImage would no 
longer include the floppy boot sector with kernel loading code.  This 
was an explicit regression because simply copying zImage to a floppy and 
sticking that floppy into a PC no longer boots the kernel.  I'm sure 
some people were affected by that, and the official word was that you 
now have to use a proper boot loader, period.

So far, I've never seen any ARM machine where the kernel is loaded more 
than 64MB away from start of RAM.  And that 64MB happened only once and 
that was from my own doing.  Most people load zImage much closer to 
start of RAM, and almost all of them simply load zImage at zreladdr.  
Or worse, they wrap it into a uImage, load it higher thinking it is 
good, and u-Boot copies it to zreladdr because that's what 'make uImage' 
uses, forcing zImage to make a second copy.

Between you and I, we've seen quite a lot of ARM setups after all those 
years. We should be able to guess the probability for causing regression 
with AUTO_ZRELADDR=y by default.  Personally I'd say that, while not 
impossible, the probability is close to zero for 99% of the cases.


Nicolas

RE: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Nicolas Pitre]

On Fri, 14 Oct 2011, Stephen Warren wrote:

> Arnd Bergmann wrote at Friday, October 14, 2011 9:30 AM:
> ...
> > You mention that tegra30 will require AUTO_ZRELADDR.
> 
> Well, just to be clear, here's the situation I think:
> 
> Tegra20's SDRAM starts at physical address 0.
> 
> Tegra30's SDRAM starts at physical address 2G.
> 
> To support that, we could either:
> 
> a) Introduce a new Kconfig variable for Tegra30, make T20/T30 mutually
> exclusive, and update arch/arm/mach-tegra/Makefile.boot to set zreladdr
> etc. based on the new Tegra30 config variable too. Then, there's no need
> for AUTO_ZRELADDR anywhere.
> 
> b) Have no new config variable, build a unified T20/T30 kernel, leave
> Makefile.boot untouched, and rely on using AUTO_ZRELADDR for Tegra30 to
> account for the different SDRAM physical addresses.

As I explained yesterday, I'm working on more cleanups that involve
reworking the dependencies around ZBOOTROM, AUTO_ZRELADDR and friends, 
and removing zreladdr for all platforms.  So please do not come up with 
Tegra specific hacks to fix this up.  So for the time being I'd simply 
suggest using the minimal fix i.e.:

	select AUTO_ZRELADDR if !ZBOOT_ROM

And eventually this will disappear entirely in favor of a single global 
config option which is not platform dependent.


Nicolas

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Olof Johansson]

Hi,

On Fri, Oct 14, 2011 at 10:53 AM, Nicolas Pitre <nico@fluxnic.net> wrote:

> As I explained yesterday, I'm working on more cleanups that involve
> reworking the dependencies around ZBOOTROM, AUTO_ZRELADDR and friends,
> and removing zreladdr for all platforms.  So please do not come up with
> Tegra specific hacks to fix this up.  So for the time being I'd simply
> suggest using the minimal fix i.e.:
>
>        select AUTO_ZRELADDR if !ZBOOT_ROM
>
> And eventually this will disappear entirely in favor of a single global
> config option which is not platform dependent.

Odd, I'm not seeing any mention of the above on the list (especially
not posted yesterday), but that sounds good to me.

We've dropped the patch for now, if t30 needs it we can update the
defconfig when the time comes, and wait for your cleanup for the rest.


-Olof

Re: [PATCH] arm/tegra: select AUTO_ZRELADDR by default

[Olof Johansson]

On Fri, Oct 14, 2011 at 10:58 AM, Olof Johansson <olof@lixom.net> wrote:

> Odd, I'm not seeing any mention of the above on the list (especially
> not posted yesterday), but that sounds good to me.

Ah, didn't think to search for your linaro address. My bad.


-Olof

[PATCH] ARM: mutually exclude ZBOOT_ROM and AUTO_ZRELADDR

[Olof Johansson]

This way platforms that want it can select AUTO_ZRELADDR without issues caused by
someone manually also enabling ZBOOT_ROM.

Signed-off-by: Olof Johansson <olof@lixom.net>
---
 arch/arm/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)


Russell King wrote at Friday, October 14, 2011 1:15 AM:

> I'll point out that this makes Tegra incompatible with ZBOOT_ROM, which
> can still be enabled.  ZBOOT_ROM=y AUTO_ZRELADDR=y is an invalid
> configuration at runtime.

Ah, looks like the dependency is only one-way. Since they are mutually
exclusive, how about the below?


diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 82973b2..2ad58e8 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1814,6 +1814,7 @@ config ZBOOT_ROM_BSS
 config ZBOOT_ROM
 	bool "Compressed boot loader in ROM/flash"
 	depends on ZBOOT_ROM_TEXT != ZBOOT_ROM_BSS
+	depends on !AUTO_ZRELADDR
 	help
 	  Say Y here if you intend to execute your compressed kernel image
 	  (zImage) directly from ROM or flash.  If unsure, say N.
-- 
1.7.4.1

Re: [PATCH] ARM: mutually exclude ZBOOT_ROM and AUTO_ZRELADDR

[Arnd Bergmann]

On Friday 14 October 2011, Olof Johansson wrote:
> This way platforms that want it can select AUTO_ZRELADDR without issues caused by
> someone manually also enabling ZBOOT_ROM.
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 82973b2..2ad58e8 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1814,6 +1814,7 @@ config ZBOOT_ROM_BSS
>  config ZBOOT_ROM
>  	bool "Compressed boot loader in ROM/flash"
>  	depends on ZBOOT_ROM_TEXT != ZBOOT_ROM_BSS
> +	depends on !AUTO_ZRELADDR
>  	help
>  	  Say Y here if you intend to execute your compressed kernel image
>  	  (zImage) directly from ROM or flash.  If unsure, say N.

Unfortunately, you cannot have it both ways:

arnd@ocdc-kvm:~/linux-arm$ make O=obj-tmp CROSS_COMPILE=arm-linux-gnueabi- ARCH=arm  -sj20  menuconfig
arch/arm/Kconfig:1761:error: recursive dependency detected!
arch/arm/Kconfig:1761:  symbol ZBOOT_ROM depends on AUTO_ZRELADDR
arch/arm/Kconfig:1899:  symbol AUTO_ZRELADDR depends on ZBOOT_ROM

	Arnd

Re: [PATCH] ARM: mutually exclude ZBOOT_ROM and AUTO_ZRELADDR

[Nicolas Pitre]

On Fri, 14 Oct 2011, Arnd Bergmann wrote:

> On Friday 14 October 2011, Olof Johansson wrote:
> > This way platforms that want it can select AUTO_ZRELADDR without issues caused by
> > someone manually also enabling ZBOOT_ROM.
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index 82973b2..2ad58e8 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -1814,6 +1814,7 @@ config ZBOOT_ROM_BSS
> >  config ZBOOT_ROM
> >  	bool "Compressed boot loader in ROM/flash"
> >  	depends on ZBOOT_ROM_TEXT != ZBOOT_ROM_BSS
> > +	depends on !AUTO_ZRELADDR
> >  	help
> >  	  Say Y here if you intend to execute your compressed kernel image
> >  	  (zImage) directly from ROM or flash.  If unsure, say N.
> 
> Unfortunately, you cannot have it both ways:
> 
> arnd@ocdc-kvm:~/linux-arm$ make O=obj-tmp CROSS_COMPILE=arm-linux-gnueabi- ARCH=arm  -sj20  menuconfig
> arch/arm/Kconfig:1761:error: recursive dependency detected!
> arch/arm/Kconfig:1761:  symbol ZBOOT_ROM depends on AUTO_ZRELADDR

Ah, right.

OK, like I said I'm reworking the whole thing, so the cheapest fix for 
the time being  is "select AUTO_ZRELADDR if !ZBOOT_ROM".


Nicolas

Re: [PATCH] ARM: mutually exclude ZBOOT_ROM and AUTO_ZRELADDR

[Nicolas Pitre]

On Fri, 14 Oct 2011, Olof Johansson wrote:

> This way platforms that want it can select AUTO_ZRELADDR without issues caused by
> someone manually also enabling ZBOOT_ROM.
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>

Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>

Certainly harmless for the time being.



> ---
>  arch/arm/Kconfig |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> 
> Russell King wrote at Friday, October 14, 2011 1:15 AM:
> 
> > I'll point out that this makes Tegra incompatible with ZBOOT_ROM, which
> > can still be enabled.  ZBOOT_ROM=y AUTO_ZRELADDR=y is an invalid
> > configuration at runtime.
> 
> Ah, looks like the dependency is only one-way. Since they are mutually
> exclusive, how about the below?
> 
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 82973b2..2ad58e8 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1814,6 +1814,7 @@ config ZBOOT_ROM_BSS
>  config ZBOOT_ROM
>  	bool "Compressed boot loader in ROM/flash"
>  	depends on ZBOOT_ROM_TEXT != ZBOOT_ROM_BSS
> +	depends on !AUTO_ZRELADDR
>  	help
>  	  Say Y here if you intend to execute your compressed kernel image
>  	  (zImage) directly from ROM or flash.  If unsure, say N.
> -- 
> 1.7.4.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>