[U-Boot] [PATCH] image: Don't relocate ramdisk to highmem

[Thierry Reding]

On Thu, Jul 11, 2013 at 08:21:17PM +0200, Wolfgang Denk wrote:
> Dear Thierry Reding,
> 
> In message <20130711150014.GA2198 at dhcp-172-17-186-34.nvidia.com> you wrote:
> > 
> > > I'm pretty sure it's all architectures, and this is a problem for device
> > > trees as well.  The tricks done to deal with highmem mean it's not
> > > suitable for certain tasks, if I recall things right (it's been a
> > > while).
> > 
> > Yes, that's my understanding as well. The same changes were done for
> > fdt_high a few months back and ramdisks aren't any different in this
> > respect. I was a bit surprised that this hadn't been fixed yet, but
> > maybe people just aren't using ramdisks anymore these days, or they
> > worked around it by setting initrd_high explicitly.
> 
> This depends a lot on a number of things.  For example, you should be
> able to use a ramdisk in NOR flash directly, i. e. without loading it
> to RAM first - especially if it;s a comprtessed ramdis image, and the
> Kernel will copy/uncompress it anyway.  Depending on your memory map
> the address range of the NOR flash may be way outside (and above) that
> of the system RAM.  Are you sure your changes will not break any such
> usage?

As far as I can tell, the only place where initrd_high is used is to
allocate an area from RAM that's used to relocate the ramdisk to. In
case where the ramdisk is directly in NOR and isn't supposed to be
copied anyway, this code should never be run.

> > Yeah, I wondered whether maybe not relocating the ramdisk (and the FDT)
> > in the first place might even be a better default fallback. It makes the
> 
> We want to avoid any copy of the ramdisk at all, if possible.

Well, that's not what the current code does. It currently always
relocates unless you happen to explicitly set initrd_high = 0xffffffff.

So maybe one other alternative would be to check whether the current
location of the ramdisk is within the low memory area and only relocate
otherwise. Does that match your expectation?

> > > > Also, when changing the behaviour, you should also update the
> > > > comments.
> > >
> > > Agreed.
> > 
> > I'm not sure which comments you are referring to. I updated the one
> > immediately above the changed code and the one above the function
> > doesn't contain anything relating to the default behaviour in case
> > initrd_high is unset.
> 
> ...because default behaviour was do do nothing. Now you do something,
> so this should be documented.

The default behaviour was to not restrict loading to high memory at all,
which is what the comment said:

	/* not set, no restrictions to load high */
	initrd_high = ~0;

The patch that I posted changes the above to this:

	/* make sure to put ramdisk in low memory */
	initrd_high = getenv_bootm_low() + getenv_bootm_mapsize();

Doesn't that accurately describe the change?

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130711/ac1149a4/attachment.pgp>