Re: "ARM: multi_v7_defconfig: Enable shmobile platforms" breaks Tegra20 multi_v7_defconfig boot

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/01/2015 01:17 PM, Paul Walmsley wrote:
On 04/01/2015 12:55 PM, Stephen Warren wrote:
(Dropping people likely not interested in Tegra U-Boot from explicit Cc)

On 04/01/2015 11:48 AM, Paul Walmsley wrote:
On 04/01/2015 11:14 AM, Stephen Warren wrote:
On 03/26/2015 12:57 PM, Stephen Warren wrote:
On 03/26/2015 12:35 PM, Paul Walmsley wrote:
On Wed, 25 Mar 2015, Stephen Warren wrote:

On 03/25/2015 04:00 PM, Paul Walmsley wrote:
On Wed, 25 Mar 2015, Tyler Baker wrote:
On 25 March 2015 at 11:03, Paul Walmsley <paul@xxxxxxxxx> wrote:
Looks like commit 4a3a6f86693922b29cf829c63f652b057f14619e ("ARM:
multi_v7_defconfig: Enable shmobile platforms") breaks Tegra20
multi_v7_defconfig boot.
...
Can you try to shift your kernel load address around a bit? From
experience with the boards from kernelci.org we find that as the
multi
v7 kernel size increases they can clobber memory when they get
decompressed.

Thanks, that was it:

http://nvt.pwsan.com/pub/pwalmsley-tester/testlogs/test_20150325144058_6af714b069dc278d5d8e1b7afc13568f71d9aba8/20150325144058/boot/tegra20-trimslice/tegra20-trimslice/multi_v7_defconfig_log.txt


...
Interesting. Do the values in U-Boot's default environment work
correctly

No idea, I haven't tried.  (The load addresses I used are
observable in
the boot logs above.)

Sure. I was hoping you'd try it out since you already had the setup to
repro the issue.

It'd be good if your test-bed used the built-in U-Boot variables
too, so
we're testing them.

I've reproduced the original problem, and then validated that using
the addresses from U-Boot's default environment (at least with a ToT
U-Boot that I just built) does indeed solve it.

I'd like to re-iterate that the test bed should be using the values
from U-Boot's environment rather than making up its own. That way:

* The value the test bed uses for the kernel load address likely
overlaps where the kernel decompressor writes to for even slightly
large kernels. This means the decompressor must move itself before
decompressing. IIUC, there's zero guarantee that moving the
decompressor won't overwrite the DTB, since IIUC the decompressor uses
zero knowledge of the DTB location. In other words, if the compressed
kernel is loaded somewhere that's likely to require it to move itself,
there's a real risk of the exact same problem cropping up again in the
future if the kernel happens to overwrite the DTB during relocation.

* The values are part of every Tegra U-Boot port (and hopefully for
other SoCs too given any distro using boot.scr with
config_distro_bootcmd.h will expect the variables to exist), so for
future (Tegra) chips you won't have to adjust the test bed if RAM
layout changes.

* Those values get testing, so we'll find out if the ever don't work.
We get more test coverage.

Sounds reasonable, I'll try to get to this at some point soon.

BTW, it might be worth changing U-boot CONFIG_LOADADDR to point to the
value you define for kernel_addr_r.  That would reinforce to folks who
aren't using the U-boot scripts that they should use that address for
loading their kernel.

Hmm. Our values for CONFIG_LOADADDR and CONFIG_SYS_LOADADDR in U-Boot
seem to be a mess.

Essentially they are used for the same semantic purpose, it's just
that different U-Boot features use one or the other. I propose we make
them the same value. There are certainly many other boards that do
(and many that don't, strangely).

That sounds fine to me.  I got totally confused trying to figure out the
difference between CONFIG_LOADADDR and CONFIG_SYS_LOAD_ADDR.


I'm not sure that pointing those at the same location as kernel_addr_r
is best. I'd expect modern scripts to explicitly use one of the
type-specific (kernel, DT, script, initrd, ...) variables and never
rely on simple $loadaddr. How about we point these variables somewhere
that doesn't overlap with any of the memory regions
"reserved"/intended for a specific purpose?

In other words, I'd suggest 0x90200000 as the value for Tegra30+;
that's consistent with the default values of $scriptaddr and
$pxefile_addr_r, i.e. 1MB beyond the latter, and well out of the way
from anything else.

Does all that sound reasonable? If so, I'll send a patch for U-Boot.

I always used to treat CONFIG_LOADADDR as the "recommended" load address
for the kernel, as did my colleagues.  But that was years ago in the
days of uImages and things have changed a lot since then.  So to be
perfectly honest, I don't know what the right path forward is.  I'd hate
to add more work to your plate, but maybe we should ask on the U-boot
mailing lists to see what folks there would recommend?  If you're busy
I'd be happy to send the initial query, just let me know.

Otherwise, I think you probably know U-boot better than I, so ultimately
I'm fine with your discretion.

Thinking about this more, there's a case where setting CONFIG_LOADADDR to 0x90200000 won't work (raw zImage loaded at that address, with AUTO_ZRELADDR in use, booted using U-Boot's bootz command which IIRC never relocates the zImage). I would not expect scripts to mix "old" variables such as $loadaddr with "new" variables such as $kernel_addr_r, so sharing the same location for them should work out fine. I'll just do that.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux