Re: [Qemu-devel] [PATCH] hw/arm/boot: allow using a command line specified dtb without a kernel

[Michael Olbrich <m.olbrich@pengutronix.de>]

On Thu, Sep 22, 2016 at 05:23:17PM +0100, Peter Maydell wrote:
> On 10 September 2016 at 16:07, Michael Olbrich <m.olbrich@pengutronix.de> wrote:
> > When kernel and device tree are specified in the QEMU commandline, then
> > this device tree may be modified e.g. to add virtio_mmio devices.
> > With a bootloader e.g. on a flash device these extra devices are not
> > available.
> > With this change, the device tree can be specified at the QEMU commandline.
> > The modified device tree made available to the bootloader with the same
> > mechanism already supported by device trees fully generated by QEMU.
> 
> Would you mind explaining your usecase in a little more detail
> (for instance which machine model are you using) ?

I'm using qemu for system integration testing. Including some bootloader /
Linux kernel and userspace interaction. I'm currently using vexpress-a9 but
I plan to investigate if the new imx6 machine is suitable for my use-case.

In a setup without bootloader I already use a 9p rootfs. It's great for
debugging and a lot easier to set up than networking and a NFS server.

With this patch (and some bootloader patches to handle the device tree) I
can do the same when a bootloader is involved.

> > Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
> > ---
> >  hw/arm/boot.c | 4 ++--
> >  vl.c          | 5 -----
> >  2 files changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> > index 1b913a43ca65..942416d95a6f 100644
> > --- a/hw/arm/boot.c
> > +++ b/hw/arm/boot.c
> > @@ -773,6 +773,8 @@ static void arm_load_kernel_notify(Notifier *notifier, void *data)
> >       */
> >      assert(!(info->secure_board_setup && kvm_enabled()));
> >
> > +    info->dtb_filename = qemu_opt_get(qemu_get_machine_opts(), "dtb");
> > +
> >      /* Load the kernel.  */
> >      if (!info->kernel_filename || info->firmware_loaded) {
> >
> > @@ -833,8 +835,6 @@ static void arm_load_kernel_notify(Notifier *notifier, void *data)
> >          elf_machine = EM_ARM;
> >      }
> >
> > -    info->dtb_filename = qemu_opt_get(qemu_get_machine_opts(), "dtb");
> > -
> >      if (!info->secondary_cpu_reset_hook) {
> >          info->secondary_cpu_reset_hook = default_reset_secondary;
> >      }
> 
> This change definitely makes sense -- we check info->dtb_filename
> in have_dtb() so we need to set it before we call that function,
> not afterwards.
> 
> > diff --git a/vl.c b/vl.c
> > index ee557a1d3f8a..bbea51e0ce7d 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -4335,11 +4335,6 @@ int main(int argc, char **argv, char **envp)
> >          exit(1);
> >      }
> >
> > -    if (!linux_boot && qemu_opt_get(machine_opts, "dtb")) {
> > -        error_report("-dtb only allowed with -kernel option");
> > -        exit(1);
> > -    }
> > -
> 
> I can see why you want this change, but what worries me a little
> is that this is changing the behaviour of -dtb for all QEMU
> target architectures, not just ARM (they no longer get a helpful
> message on user error). I'm not sure how to address that, though.

Would a 'if !arm' be possible or useful here?

Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |