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
>
