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

Paolo Bonzini posted 22 patches 5 years, 3 months ago
There is a newer version of this series
[PATCH 01/22] semihosting: fix order of initialization functions
Posted by Paolo Bonzini 5 years, 3 months ago
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.

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();
 
     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();
-
     /* If no default VGA is requested, the default is "none".  */
     if (default_vga) {
         vga_model = get_default_vga_model(machine_class);
-- 
2.26.2



Re: [PATCH 01/22] semihosting: fix order of initialization functions
Posted by Alex Bennée 5 years, 3 months ago
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

Re: [PATCH 01/22] semihosting: fix order of initialization functions
Posted by Paolo Bonzini 5 years, 3 months ago
On 27/10/20 12:18, Alex Bennée wrote:
>>  
>> -    /* 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();
> 
> 

Sounds good, thanks!

Paolo