| Message ID | 141c5b32-31cd-1a1d-b1aa-692b9002dac9@adacore.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] some semihosting interrogation | expand |
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 >
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.
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);
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: 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