Re: [PATCH 01/22] semihosting: fix order of initialization functions

From: "Alex Bennée" <alex.bennee@linaro.org>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, berrange@redhat.com, jsnow@redhat.com,
	ehabkost@redhat.com, imammedo@redhat.com
Subject: Re: [PATCH 01/22] semihosting: fix order of initialization functions
Date: Tue, 27 Oct 2020 11:18:43 +0000	[thread overview]
Message-ID: <87sg9zex2k.fsf@linaro.org> (raw)
In-Reply-To: <20201021205716.2359430-2-pbonzini@redhat.com>

Paolo Bonzini <pbonzini@redhat.com> writes:

> qemu_semihosting_console_init uses semihosting.chardev which is set
> by qemu_semihosting_connect_chardevs.  Thus qemu_semihosting_connect_chardevs
> has to be called first.

It looks like this is reverting 619985e9 ("semihosting: defer
connect_chardevs a little more to use serialx"). Looking back at the
history it seems the two calls had different results:

  Right - can confirm the difference between:

    ./aarch64-softmmu/qemu-system-aarch64 -cpu max -serial mon:stdio -M virt -display none -semihosting -kernel ./tests/tcg/aarch64-softmmu/memory

  and

    ./aarch64-softmmu/qemu-system-aarch64 -cpu max -serial mon:stdio -M virt -display none -semihosting-config chardev=serial0 -kernel ./tests/tcg/aarch64-softmmu/memory

With this patch applied it breaks the later invocation:

  ./aarch64-softmmu/qemu-system-aarch64  -cpu max -serial mon:stdio -M virt -display none -semihosting-config chardev=serial0 -kernel ./tests/tcg/aarch64-softmmu/memory
  qemu-system-aarch64: semihosting chardev 'serial0' not found

>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  softmmu/vl.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 6f5b000f07..42314e6ff9 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -4288,7 +4288,8 @@ void qemu_init(int argc, char **argv, char **envp)
>      qemu_opts_foreach(qemu_find_opts("mon"),
>                        mon_init_func, NULL, &error_fatal);
>  
> -    /* connect semihosting console input if requested */
> +    /* now chardevs have been created we may have semihosting to connect */
> +    qemu_semihosting_connect_chardevs();
>      qemu_semihosting_console_init();

Maybe instead of this we should:

>  
>      if (foreach_device_config(DEV_SERIAL, serial_parse) < 0)
> @@ -4298,9 +4299,6 @@ void qemu_init(int argc, char **argv, char **envp)
>      if (foreach_device_config(DEV_DEBUGCON, debugcon_parse) < 0)
>          exit(1);
>  
> -    /* now chardevs have been created we may have semihosting to connect */
> -    qemu_semihosting_connect_chardevs();
> -

Move both here:

    if (foreach_device_config(DEV_DEBUGCON, debugcon_parse) < 0)
        exit(1);

    /* now chardevs have been created we may have semihosting to connect */
    qemu_semihosting_connect_chardevs();
    qemu_semihosting_console_init();

>      /* If no default VGA is requested, the default is "none".  */
>      if (default_vga) {
>          vga_model = get_default_vga_model(machine_class);

-- 
Alex Bennée