Patchwork hw/arm_boot.c: bump initrd address (again) to accommodate large kernels

login
register
mail settings
Submitter Cole Robinson
Date Oct. 7, 2012, 10:27 p.m.
Message ID <75a3282f64955a723e86f90ef3ccf9bc48871ebc.1349647745.git.crobinso@redhat.com>
Download mbox | patch
Permalink /patch/189867/
State New
Headers show

Comments

Cole Robinson - Oct. 7, 2012, 10:27 p.m.
From: Paul Whalen <pwhalen@redhat.com>

Fedora ARM is generating kernels that exceed the hardcoded size limits:

https://bugzilla.redhat.com/show_bug.cgi?id=862766

Bump the load address, as was previously done in 756ba3b

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 hw/arm_boot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Peter Maydell - Oct. 7, 2012, 10:36 p.m.
On 7 October 2012 23:27, Cole Robinson <crobinso@redhat.com> wrote:
> From: Paul Whalen <pwhalen@redhat.com>
>
> Fedora ARM is generating kernels that exceed the hardcoded size limits:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=862766
>
> Bump the load address, as was previously done in 756ba3b
>
> Signed-off-by: Cole Robinson <crobinso@redhat.com>
> ---
>  hw/arm_boot.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm_boot.c b/hw/arm_boot.c
> index a6e9143..3cabb71 100644
> --- a/hw/arm_boot.c
> +++ b/hw/arm_boot.c
> @@ -18,7 +18,7 @@
>
>  #define KERNEL_ARGS_ADDR 0x100
>  #define KERNEL_LOAD_ADDR 0x00010000
> -#define INITRD_LOAD_ADDR 0x00d00000
> +#define INITRD_LOAD_ADDR 0x01d00000

This makes me sad but I still have no idea what we could
do that would be better than this...

-- PMM
Peter A. G. Crosthwaite - Oct. 8, 2012, 12:09 a.m.
On Mon, Oct 8, 2012 at 8:36 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 7 October 2012 23:27, Cole Robinson <crobinso@redhat.com> wrote:
>> From: Paul Whalen <pwhalen@redhat.com>
>>
>> Fedora ARM is generating kernels that exceed the hardcoded size limits:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=862766
>>
>> Bump the load address, as was previously done in 756ba3b
>>
>> Signed-off-by: Cole Robinson <crobinso@redhat.com>
>> ---
>>  hw/arm_boot.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/arm_boot.c b/hw/arm_boot.c
>> index a6e9143..3cabb71 100644
>> --- a/hw/arm_boot.c
>> +++ b/hw/arm_boot.c
>> @@ -18,7 +18,7 @@
>>
>>  #define KERNEL_ARGS_ADDR 0x100
>>  #define KERNEL_LOAD_ADDR 0x00010000
>> -#define INITRD_LOAD_ADDR 0x00d00000
>> +#define INITRD_LOAD_ADDR 0x01d00000
>
> This makes me sad but I still have no idea what we could
> do that would be better than this...
>

Way back when I tried to QOMify the arm bootloader, initrd location
was a device option for the boot-loader device:

qemu-system-arm -M foo -device arm-bootloader,initrdaddr=0xdeadbeef

Give it a sane default, and those who have a problem can then override
it. When the bootloader detects the collision between the image and
initrd blobs, a nice error message suggest a command line arg to
rearrange the locations for initrd, dtb and friends.

A comment In general, there are so many inflexibilites in the arm
bootloader and you have stated a long term goal of unifying the
bootloaders accross architectures. But so far the fixes for problems
like this are always blocked by the desire for backwards
compatilbilty. To that end, perhaps we can leave the arm bootloader as
is, deparacte it, then fork bootloader development with a highly
configurable multi-platform QOM bootloader that does have accessible
controls for properties like this?

Another completely different but workable solution is to dynamically
determine a initrd (and dtb) location. Any reason why the bootloader
can't track what memory real estate is in use by what pieces and just
pick a piece of vacant real estate big enough for the dtb or initrd?
Its hard if elfs are invloved, but for raw images it should be
trivial.

Regards,
Peter

> -- PMM
>
Peter Maydell - Oct. 8, 2012, 7:34 a.m.
On 8 October 2012 01:09, Peter Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> Another completely different but workable solution is to dynamically
> determine a initrd (and dtb) location. Any reason why the bootloader
> can't track what memory real estate is in use by what pieces and just
> pick a piece of vacant real estate big enough for the dtb or initrd?

The trouble is that the common case is a zImage and there's no way
to tell how big the zImage will be when uncompressed.

-- PMM
Cole Robinson - Oct. 22, 2012, 9:06 p.m.
On 10/07/2012 06:36 PM, Peter Maydell wrote:
> On 7 October 2012 23:27, Cole Robinson <crobinso@redhat.com> wrote:
>> From: Paul Whalen <pwhalen@redhat.com>
>>
>> Fedora ARM is generating kernels that exceed the hardcoded size limits:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=862766
>>
>> Bump the load address, as was previously done in 756ba3b
>>
>> Signed-off-by: Cole Robinson <crobinso@redhat.com>
>> ---
>>  hw/arm_boot.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/arm_boot.c b/hw/arm_boot.c
>> index a6e9143..3cabb71 100644
>> --- a/hw/arm_boot.c
>> +++ b/hw/arm_boot.c
>> @@ -18,7 +18,7 @@
>>
>>  #define KERNEL_ARGS_ADDR 0x100
>>  #define KERNEL_LOAD_ADDR 0x00010000
>> -#define INITRD_LOAD_ADDR 0x00d00000
>> +#define INITRD_LOAD_ADDR 0x01d00000
> 
> This makes me sad but I still have no idea what we could
> do that would be better than this...
> 

Hi Peter,

I agree it's unfortunate, but in the absence of a better solution, could this
patch be applied?

Thanks,
Cole
Peter Maydell - Oct. 26, 2012, 8:48 a.m.
On 22 October 2012 22:06, Cole Robinson <crobinso@redhat.com> wrote:
> On 10/07/2012 06:36 PM, Peter Maydell wrote:
>> On 7 October 2012 23:27, Cole Robinson <crobinso@redhat.com> wrote:
>>>  #define KERNEL_ARGS_ADDR 0x100
>>>  #define KERNEL_LOAD_ADDR 0x00010000
>>> -#define INITRD_LOAD_ADDR 0x00d00000
>>> +#define INITRD_LOAD_ADDR 0x01d00000
>>
>> This makes me sad but I still have no idea what we could
>> do that would be better than this...

> I agree it's unfortunate, but in the absence of a better solution, could this
> patch be applied?

This patch puts the initrd starting at 29MB (was 13MB). That
would probably break any machines with 32MB memory configurations.
So I need to check if there are any which might plausibly be run
with 32MB (and if so maybe set initrd load address based on
ram_size?)

Also 13MB is an absolutely ginormous kernel...

-- PMM
Peter Maydell - Oct. 26, 2012, 10:32 a.m.
On 26 October 2012 09:48, Peter Maydell <peter.maydell@linaro.org> wrote:
> This patch puts the initrd starting at 29MB (was 13MB). That
> would probably break any machines with 32MB memory configurations.
> So I need to check if there are any which might plausibly be run
> with 32MB (and if so maybe set initrd load address based on
> ram_size?)

musicpal, omap_sx1, cheetah, z2 all have 32MB RAM sizes and use
the arm_boot bootloader, so I can't take this patch as it stands.

-- PMM

Patch

diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index a6e9143..3cabb71 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -18,7 +18,7 @@ 
 
 #define KERNEL_ARGS_ADDR 0x100
 #define KERNEL_LOAD_ADDR 0x00010000
-#define INITRD_LOAD_ADDR 0x00d00000
+#define INITRD_LOAD_ADDR 0x01d00000
 
 /* The worlds second smallest bootloader.  Set r0-r2, then jump to kernel.  */
 static uint32_t bootloader[] = {