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

[Peter Maydell]

On 10 September 2016 at 16:07, Michael Olbrich <address@hidden> 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) ?

> Signed-off-by: Michael Olbrich <address@hidden>
> ---
>  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.

thanks
-- PMM