[Qemu-devel] [PATCH v2 0/2] These two patches add some flexibilities to the arm bootloader.

[Qemu-devel] [PATCH v2 0/2] These two patches add some flexibilities to the arm bootloader.

[Peter A. G. Crosthwaite]

Set up the the bootloader so that if -dtb is specified, then the Linux bootflow is assumed.

Conditionalised dtb command line update on there being a -append argument. Means you can use a dtb with a command line already present and boot without QEMU nuking your command line.

The patches are independent. A block on patch one whould not block patch 2 and vice versa. Sent as one series for reviewer convenience.

changed since v1:
replaced old patch one (used to be cmd line arg for linux - now is just dtb implies linux)
tweaked implementation of patch 2

Peter A. G. Crosthwaite (2):
  arm_boot: Assume Linux boot flow when -dtb given
  arm_boot: Conditionalised DTB command line update

 hw/arm_boot.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

-- 
1.7.3.2

[Qemu-devel] [PATCH v2 1/2] arm_boot: Assume Linux boot flow when -dtb given

[Peter A. G. Crosthwaite]

If the user boots with a -dtb assume the Linux boot flow, even when handling an
elf.

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
 hw/arm_boot.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index 7447f5c..f0fa23c 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -360,6 +360,11 @@ void arm_load_kernel(CPUARMState *env, struct arm_boot_info *info)
         exit(1);
     }
     info->entry = entry;
+
+    if (info->dtb_filename) {
+        is_linux = 1;
+    }
+
     if (is_linux) {
         if (info->initrd_filename) {
             initrd_size = load_image_targphys(info->initrd_filename,
-- 
1.7.3.2

Re: [Qemu-devel] [PATCH v2 1/2] arm_boot: Assume Linux boot flow when -dtb given

[Peter Maydell]

On 18 June 2012 02:35, Peter A. G. Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> If the user boots with a -dtb assume the Linux boot flow, even when handling an
> elf.

We don't do this for -initrd, why should we do it for -dtb ?

-- PMM

Re: [Qemu-devel] [PATCH v2 1/2] arm_boot: Assume Linux boot flow when -dtb given

[Peter Crosthwaite]

It matches my flow in the real hardware.

Heres the scenario where we need this (FYI applies to both microblaze and arm):

User creates a Linux elf that includes a built in dtb. Slave mode
bootloader boots the real hardware with the elf (my actual real JTAG
bootloader work with elfs).

However QEMU doesn't support all the hardware devices the kernel
supports, so we need the QEMU linux bootloader to override the built
in DTB (with -dtb) to stop the kernel probing non-existent devices.
So, if the user specifies a dtb, adopt the linux bootloader flow
instead, before jumping to the elf entry point. r2 get set to the -dtb
argument and it boots with the modified dtb.

Make sense?

It all comes down to I need a bootloader functionality (mainly dtb but
SMP stuff would be nice too) before booting my elf.

Every time i touch this arm bootloader we end up in a discussion about
how it needs to be rewritten cos of flawed assumptions etc (like elfs
are not Linux). Can we just accept this and throw it out with the
trash when someone refactors the arm bootloader properly?

Regards,
Peter

On Tue, Jun 19, 2012 at 11:06 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 18 June 2012 02:35, Peter A. G. Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
>> If the user boots with a -dtb assume the Linux boot flow, even when handling an
>> elf.
>
> We don't do this for -initrd, why should we do it for -dtb ?

Maybe we should do it for initrd too. If the user specifies either of
these options they are trying to boot Linux.

>
> -- PMM

Re: [Qemu-devel] [PATCH v2 1/2] arm_boot: Assume Linux boot flow when -dtb given

[Peter Crosthwaite]

Ping!

Any thoughts Peter?

Regards,
Peter

On Wed, Jun 20, 2012 at 11:45 AM, Peter Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> It matches my flow in the real hardware.
>
> Heres the scenario where we need this (FYI applies to both microblaze and arm):
>
> User creates a Linux elf that includes a built in dtb. Slave mode
> bootloader boots the real hardware with the elf (my actual real JTAG
> bootloader work with elfs).
>
> However QEMU doesn't support all the hardware devices the kernel
> supports, so we need the QEMU linux bootloader to override the built
> in DTB (with -dtb) to stop the kernel probing non-existent devices.
> So, if the user specifies a dtb, adopt the linux bootloader flow
> instead, before jumping to the elf entry point. r2 get set to the -dtb
> argument and it boots with the modified dtb.
>
> Make sense?
>
> It all comes down to I need a bootloader functionality (mainly dtb but
> SMP stuff would be nice too) before booting my elf.
>
> Every time i touch this arm bootloader we end up in a discussion about
> how it needs to be rewritten cos of flawed assumptions etc (like elfs
> are not Linux). Can we just accept this and throw it out with the
> trash when someone refactors the arm bootloader properly?
>
> Regards,
> Peter
>
> On Tue, Jun 19, 2012 at 11:06 PM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> On 18 June 2012 02:35, Peter A. G. Crosthwaite
>> <peter.crosthwaite@petalogix.com> wrote:
>>> If the user boots with a -dtb assume the Linux boot flow, even when handling an
>>> elf.
>>
>> We don't do this for -initrd, why should we do it for -dtb ?
>
> Maybe we should do it for initrd too. If the user specifies either of
> these options they are trying to boot Linux.
>
>>
>> -- PMM

Re: [Qemu-devel] [PATCH v2 1/2] arm_boot: Assume Linux boot flow when -dtb given

[Peter Maydell]

On 22 June 2012 14:27, Peter Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> Ping!
>
> Any thoughts Peter?

Still sounds too specific to your odd use case and hardware to me.

I'd accept some reasonable way of saying "this ELF file is a Linux kernel",
but magically doing it if you also said -dtb isn't it. I also care about
backcompat with previous command lines and with being consistent about
how -kernel works across architectures where possible. (I know this is
a somewhat contradictory set of requirements.)

-- PMM

Re: [Qemu-devel] [PATCH v2 1/2] arm_boot: Assume Linux boot flow when -dtb given

[Peter Crosthwaite]

On Fri, Jun 22, 2012 at 11:36 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 22 June 2012 14:27, Peter Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
>> Ping!
>>
>> Any thoughts Peter?
>
> Still sounds too specific to your odd use case and hardware to me.
>
> I'd accept some reasonable way of saying "this ELF file is a Linux kernel",
> but magically doing it if you also said -dtb isn't it.

Then is there no objection to v1 of this series where I made a
is_linux machine opt? I made this patch as a less intrusive workaround
to this problem...

I also care about
> backcompat with previous command lines and with being consistent about
> how -kernel works across architectures where possible. (I know this is
> a somewhat contradictory set of requirements.)
>
> -- PMM

Re: [Qemu-devel] [PATCH v2 1/2] arm_boot: Assume Linux boot flow when -dtb given

[Peter Crosthwaite]

On Fri, Jun 22, 2012 at 11:36 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 22 June 2012 14:27, Peter Crosthwaite
> <peter.crosthwaite@petalogix.com> wrote:
>> Ping!
>>
>> Any thoughts Peter?
>
> Still sounds too specific to your odd use case and hardware to me.
>
> I'd accept some reasonable way of saying "this ELF file is a Linux kernel",
> but magically doing it if you also said -dtb isn't it. I also care about
> backcompat with previous command lines and with being consistent about
> how -kernel works across architectures where possible.

Speaking of other archs, microblaze will accept -dtb with an elf and
implement the r2=foo style behviour before loading the elf entry. A
provocative thought, how bout always using a Linux style bootloader no
matter what? Does it really matter if your elf is bootstrapped by half
a dozen instructions?

Further to that, can we get rid of the bootloader blob altogether and
just setup the cpu->env like other architectures do?

  23 /* The worlds second smallest bootloader.  Set r0-r2, then jump
to kernel.  */
  24 static uint32_t bootloader[] = {
  25   0xe3a00000, /* mov     r0, #0 */
  26   0xe59f1004, /* ldr     r1, [pc, #4] */
  27   0xe59f2004, /* ldr     r2, [pc, #4] */
  28   0xe59ff004, /* ldr     pc, [pc, #4] */
  29   0, /* Board ID */
  30   0, /* Address of kernel args.  Set by integratorcp_init.  */
  31   0  /* Kernel entry point.  Set by integratorcp_init.  */
  32 };

All this code does is set registers and an entry point, which several
bootloaders do just though cpu->env manipulation.

> a somewhat contradictory set of requirements.)
>
> -- PMM

[Qemu-devel] [PATCH v2 2/2] arm_boot: Conditionalised DTB command line update

[Peter A. G. Crosthwaite]

The DTB command line should only be overwritten if the user provides a command
line with -apend. Otherwise whatever command line was in the DTB should stay
unchanged.

Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
---
changed since v1:
checked cmd line string in binfo rather than machine opt

 hw/arm_boot.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index f0fa23c..1b110ca 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -240,10 +240,12 @@ static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo)
         fprintf(stderr, "couldn't set /memory/reg\n");
     }
 
-    rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
-                                      binfo->kernel_cmdline);
-    if (rc < 0) {
-        fprintf(stderr, "couldn't set /chosen/bootargs\n");
+    if (binfo->kernel_cmdline && *binfo->kernel_cmdline) {
+        rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
+                                          binfo->kernel_cmdline);
+        if (rc < 0) {
+            fprintf(stderr, "couldn't set /chosen/bootargs\n");
+        }
     }
 
     if (binfo->initrd_size) {
-- 
1.7.3.2

Re: [Qemu-devel] [PATCH v2 2/2] arm_boot: Conditionalised DTB command line update

[Peter Maydell]

On 18 June 2012 02:35, Peter A. G. Crosthwaite
<peter.crosthwaite@petalogix.com> wrote:
> The DTB command line should only be overwritten if the user provides a command
> line with -apend. Otherwise whatever command line was in the DTB should stay
> unchanged.
>
> Signed-off-by: Peter A. G. Crosthwaite <peter.crosthwaite@petalogix.com>
> ---
> changed since v1:
> checked cmd line string in binfo rather than machine opt

Yep, this looks nicer and matches how we handle it in the ATAGS code
path.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM