diff mbox series

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

Message ID 20201021205716.2359430-2-pbonzini@redhat.com
State New
Headers show
Series cleanup qemu_init and make sense of command line processing | expand

Commit Message

Paolo Bonzini Oct. 21, 2020, 8:56 p.m. UTC
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(-)

Comments

Alex Bennée Oct. 27, 2020, 11:18 a.m. UTC | #1
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
Paolo Bonzini Oct. 27, 2020, 1:32 p.m. UTC | #2
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
diff mbox series

Patch

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);