[RFC] some semihosting interrogation

[RFC] some semihosting interrogation

[Fred Konrad]

Hi all,

Just wonderring if there is any reason not to be able to defer
qemu_semihosting_connect_chardevs a little more to be able to specify
chardev=serial0?

Like:

diff --git a/softmmu/vl.c b/softmmu/vl.c
index 6390cf0..9fa1553 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -4333,8 +4333,6 @@ void qemu_init(int argc, char **argv, char **envp)

      qemu_opts_foreach(qemu_find_opts("chardev"),
                        chardev_init_func, NULL, &error_fatal);
-    /* now chardevs have been created we may have semihosting to connect */
-    qemu_semihosting_connect_chardevs();

  #ifdef CONFIG_VIRTFS
      qemu_opts_foreach(qemu_find_opts("fsdev"),
@@ -4484,6 +4482,9 @@ 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);

Also I found out that the trailing \0 is sent to the chardev 
(console.c:copy_user_string) is that expected in case of semihosting?

static GString *copy_user_string(CPUArchState *env, target_ulong addr)
{
     CPUState *cpu = env_cpu(env);
     GString *s = g_string_sized_new(128);
     uint8_t c;

     do {
         if (cpu_memory_rw_debug(cpu, addr++, &c, 1, 0) == 0) {
             if (c) {
                 s = g_string_append_c(s, c);
             }
         } else {
             qemu_log_mask(LOG_GUEST_ERROR,
                           "%s: passed inaccessible address " TARGET_FMT_lx,
                           __func__, addr);
             break;
         }
     } while (c!=0);

     return s;
}

I can roll out two patches if needed..

Cheers,
Fred

Re: [RFC] some semihosting interrogation

[Philippe Mathieu-Daudé]

Hi Fred,

On 5/28/20 11:44 AM, Fred Konrad wrote:
> Hi all,
> 
> Just wonderring if there is any reason not to be able to defer
> qemu_semihosting_connect_chardevs a little more to be able to specify
> chardev=serial0?
> 
> Like:
> 
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 6390cf0..9fa1553 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -4333,8 +4333,6 @@ void qemu_init(int argc, char **argv, char **envp)
> 
>      qemu_opts_foreach(qemu_find_opts("chardev"),
>                        chardev_init_func, NULL, &error_fatal);
> -    /* now chardevs have been created we may have semihosting to
> connect */
> -    qemu_semihosting_connect_chardevs();
> 
>  #ifdef CONFIG_VIRTFS
>      qemu_opts_foreach(qemu_find_opts("fsdev"),
> @@ -4484,6 +4482,9 @@ 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();

Cc'ing Markus for this part because he had headaches recently moving
things around there; but the change looks.

> +
>      /* If no default VGA is requested, the default is "none".  */
>      if (default_vga) {
>          vga_model = get_default_vga_model(machine_class);
> 
> Also I found out that the trailing \0 is sent to the chardev
> (console.c:copy_user_string) is that expected in case of semihosting?

No, your patch fixes a bug.

> 
> static GString *copy_user_string(CPUArchState *env, target_ulong addr)
> {
>     CPUState *cpu = env_cpu(env);
>     GString *s = g_string_sized_new(128);
>     uint8_t c;
> 
>     do {
>         if (cpu_memory_rw_debug(cpu, addr++, &c, 1, 0) == 0) {
>             if (c) {
>                 s = g_string_append_c(s, c);
>             }
>         } else {
>             qemu_log_mask(LOG_GUEST_ERROR,
>                           "%s: passed inaccessible address " TARGET_FMT_lx,
>                           __func__, addr);
>             break;
>         }
>     } while (c!=0);
> 
>     return s;
> }
> 
> I can roll out two patches if needed..
> 
> Cheers,
> Fred
> 

Re: [RFC] some semihosting interrogation

[Fred Konrad]

Le 5/28/20 à 2:00 PM, Philippe Mathieu-Daudé a écrit :
> Hi Fred,
> 
> On 5/28/20 11:44 AM, Fred Konrad wrote:
>> Hi all,
>>
>> Just wonderring if there is any reason not to be able to defer
>> qemu_semihosting_connect_chardevs a little more to be able to specify
>> chardev=serial0?
>>
>> Like:
>>
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index 6390cf0..9fa1553 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -4333,8 +4333,6 @@ void qemu_init(int argc, char **argv, char **envp)
>>
>>       qemu_opts_foreach(qemu_find_opts("chardev"),
>>                         chardev_init_func, NULL, &error_fatal);
>> -    /* now chardevs have been created we may have semihosting to
>> connect */
>> -    qemu_semihosting_connect_chardevs();
>>
>>   #ifdef CONFIG_VIRTFS
>>       qemu_opts_foreach(qemu_find_opts("fsdev"),
>> @@ -4484,6 +4482,9 @@ 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();
> 
> Cc'ing Markus for this part because he had headaches recently moving
> things around there; but the change looks.
> 
>> +
>>       /* If no default VGA is requested, the default is "none".  */
>>       if (default_vga) {
>>           vga_model = get_default_vga_model(machine_class);
>>
>> Also I found out that the trailing \0 is sent to the chardev
>> (console.c:copy_user_string) is that expected in case of semihosting?
> 
> No, your patch fixes a bug.

Great thanks for your feedback.