[Qemu-devel] [rfc 0/3] arm : dynamically choose initrd load address

[Qemu-devel] [rfc 0/3] arm : dynamically choose initrd load address

[Daniel Lezcano]

Hi all,

after compiling my kernel on the arm architecture I was not able to
start it because qemu was segfaulting or going to an infinite loop.

After google'ing I found on launchpad the bug:

https://bugs.launchpad.net/ubuntu/+source/linux/+bug/524893

Following the indications, I rebuilt qemu with an higher initrd load
address and the kernel booted correctly.

I am trying to make the things easier and/or to fail gracefully with
patchset but I am not familiar with the ARM architecture neither qemu
internals, so may be I am totally wrong :)

The first patch raise an error if there is an overlapping error.
But the two next patches makes to compute automatically an address
for initrd to loaded.

Daniel Lezcano (3):
  arm : raise an error if the kernel size will overlap the initrd
  arm : factor out set_kernel_args[_old]
  arm : make initrd load address dynamic

 hw/arm-misc.h |    1 +
 hw/arm_boot.c |   40 +++++++++++++++++++---------------------
 2 files changed, 20 insertions(+), 21 deletions(-)

[Qemu-devel] [rfc 1/3] arm : raise an error if the kernel size will overlap the initrd

[Daniel Lezcano]

If the kernel size is too big, it overwrite the initrd image in memory
without detecting the problem. Let't detect this error and exit gracefully.

Signed-off-by: Daniel Lezcano <daniel.lezcano@free.fr>
---
 hw/arm_boot.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index 620550b..50ec717 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -247,6 +247,13 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
     info->entry = entry;
     if (is_linux) {
         if (info->initrd_filename) {
+
+            if (KERNEL_LOAD_ADDR + kernel_size >= INITRD_LOAD_ADDR) {
+		    fprintf(stderr, "qemu: kernel is too big: %d Bytes\n",
+			    kernel_size);
+		    exit(1);
+	    }
+
             initrd_size = load_image_targphys(info->initrd_filename,
                                               info->loader_start
                                               + INITRD_LOAD_ADDR,
-- 
1.7.0.4

[Qemu-devel] [rfc 2/3] arm : factor out set_kernel_args[_old]

[Daniel Lezcano]

'initrd_size' and 'base' are already present in the 'info' structure,
passing them as parameter is redundant with the first parameter.

Signed-off-by: Daniel Lezcano <daniel.lezcano@free.fr>
---
 hw/arm_boot.c |   26 ++++++++++----------------
 1 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index 50ec717..16a33af 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -49,12 +49,12 @@ static uint32_t smpboot[] = {
     p += 4;                       \
 } while (0)
 
-static void set_kernel_args(struct arm_boot_info *info,
-                int initrd_size, target_phys_addr_t base)
+static void set_kernel_args(struct arm_boot_info *info)
 {
-    target_phys_addr_t p;
+    target_phys_addr_t base = info->loader_start;
+    target_phys_addr_t p = base + KERNEL_ARGS_ADDR;
+    int initrd_size = info->initrd_size;
 
-    p = base + KERNEL_ARGS_ADDR;
     /* ATAG_CORE */
     WRITE_WORD(p, 5);
     WRITE_WORD(p, 0x54410001);
@@ -102,15 +102,14 @@ static void set_kernel_args(struct arm_boot_info *info,
     WRITE_WORD(p, 0);
 }
 
-static void set_kernel_args_old(struct arm_boot_info *info,
-                int initrd_size, target_phys_addr_t base)
+static void set_kernel_args_old(struct arm_boot_info *info)
 {
-    target_phys_addr_t p;
+    target_phys_addr_t base = info->loader_start;
+    /* see linux/include/asm-arm/setup.h */
+    target_phys_addr_t p = base + KERNEL_ARGS_ADDR;
+    int initrd_size = info->initrd_size;
     const char *s;
 
-
-    /* see linux/include/asm-arm/setup.h */
-    p = base + KERNEL_ARGS_ADDR;
     /* page_size */
     WRITE_WORD(p, 4096);
     /* nr_pages */
@@ -188,12 +187,7 @@ static void main_cpu_reset(void *opaque)
             env->thumb = info->entry & 1;
         } else {
             env->regs[15] = info->loader_start;
-            if (old_param) {
-                set_kernel_args_old(info, info->initrd_size,
-                                    info->loader_start);
-            } else {
-                set_kernel_args(info, info->initrd_size, info->loader_start);
-            }
+            old_param ? set_kernel_args_old(info): set_kernel_args(info);
         }
     }
     /* TODO:  Reset secondary CPUs.  */
-- 
1.7.0.4

[Qemu-devel] [rfc 3/3] arm : make initrd load address dynamic

[Daniel Lezcano]

Instead of hardcoding a default value for initrd, let's compute
dynamically from the kernel load address and its size.
We go one page after the end of the kernel.

Signed-off-by: Daniel Lezcano <daniel.lezcano@free.fr>
---
 hw/arm-misc.h |    1 +
 hw/arm_boot.c |   19 ++++++++-----------
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/hw/arm-misc.h b/hw/arm-misc.h
index 010acb4..e72f87e 100644
--- a/hw/arm-misc.h
+++ b/hw/arm-misc.h
@@ -34,6 +34,7 @@ struct arm_boot_info {
     int (*atag_board)(struct arm_boot_info *info, void *p);
     /* Used internally by arm_boot.c */
     int is_linux;
+    target_phys_addr_t initrd_load_addr;
     target_phys_addr_t initrd_size;
     target_phys_addr_t entry;
 };
diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index 16a33af..638ef62 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -15,7 +15,6 @@
 
 #define KERNEL_ARGS_ADDR 0x100
 #define KERNEL_LOAD_ADDR 0x00010000
-#define INITRD_LOAD_ADDR 0x00800000
 
 /* The worlds second smallest bootloader.  Set r0-r2, then jump to kernel.  */
 static uint32_t bootloader[] = {
@@ -71,7 +70,7 @@ static void set_kernel_args(struct arm_boot_info *info)
         /* ATAG_INITRD2 */
         WRITE_WORD(p, 4);
         WRITE_WORD(p, 0x54420005);
-        WRITE_WORD(p, info->loader_start + INITRD_LOAD_ADDR);
+        WRITE_WORD(p, info->loader_start + info->initrd_load_addr);
         WRITE_WORD(p, initrd_size);
     }
     if (info->kernel_cmdline && *info->kernel_cmdline) {
@@ -147,7 +146,7 @@ static void set_kernel_args_old(struct arm_boot_info *info)
     WRITE_WORD(p, 0);
     /* initrd_start */
     if (initrd_size)
-        WRITE_WORD(p, info->loader_start + INITRD_LOAD_ADDR);
+        WRITE_WORD(p, info->loader_start + info->initrd_load_addr);
     else
         WRITE_WORD(p, 0);
     /* initrd_size */
@@ -201,6 +200,7 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
     int is_linux = 0;
     uint64_t elf_entry;
     target_phys_addr_t entry;
+    target_phys_addr_t initrd_load_addr = 0x0;
     int big_endian;
 
     /* Load the kernel.  */
@@ -242,16 +242,13 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
     if (is_linux) {
         if (info->initrd_filename) {
 
-            if (KERNEL_LOAD_ADDR + kernel_size >= INITRD_LOAD_ADDR) {
-		    fprintf(stderr, "qemu: kernel is too big: %d Bytes\n",
-			    kernel_size);
-		    exit(1);
-	    }
-
+	    initrd_load_addr = KERNEL_LOAD_ADDR + kernel_size +
+		    TARGET_PAGE_SIZE;
+	    initrd_load_addr = TARGET_PAGE_ALIGN(initrd_load_addr);
             initrd_size = load_image_targphys(info->initrd_filename,
                                               info->loader_start
-                                              + INITRD_LOAD_ADDR,
-                                              ram_size - INITRD_LOAD_ADDR);
+                                              + initrd_load_addr,
+                                              ram_size - initrd_load_addr);
             if (initrd_size < 0) {
                 fprintf(stderr, "qemu: could not load initrd '%s'\n",
                         info->initrd_filename);
-- 
1.7.0.4

Re: [Qemu-devel] [rfc 0/3] arm : dynamically choose initrd load address

[Daniel Lezcano]

On 09/06/2010 04:12 PM, Daniel Lezcano wrote:
> Hi all,
>
> after compiling my kernel on the arm architecture I was not able to
> start it because qemu was segfaulting or going to an infinite loop.
>
> After google'ing I found on launchpad the bug:
>
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/524893
>
> Following the indications, I rebuilt qemu with an higher initrd load
> address and the kernel booted correctly.
>
> I am trying to make the things easier and/or to fail gracefully with
> patchset but I am not familiar with the ARM architecture neither qemu
> internals, so may be I am totally wrong :)
>
> The first patch raise an error if there is an overlapping error.
> But the two next patches makes to compute automatically an address
> for initrd to loaded.
>
> Daniel Lezcano (3):
>    arm : raise an error if the kernel size will overlap the initrd
>    arm : factor out set_kernel_args[_old]
>    arm : make initrd load address dynamic
>
>   hw/arm-misc.h |    1 +
>   hw/arm_boot.c |   40 +++++++++++++++++++---------------------
>   2 files changed, 20 insertions(+), 21 deletions(-)
>    

Hi,

This patchset fix some issues Linaro people are facing when they are 
trying to boot a big kernel.
Any chance someone has time to review the patchset ? Is the patchset 
acceptable for upstream merge ?

thanks in advance
   -- Daniel