[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 2/2] target/xtensa: Use semihosting/syscalls.h
From: |
Max Filippov |
Subject: |
Re: [PATCH v5 2/2] target/xtensa: Use semihosting/syscalls.h |
Date: |
Tue, 28 Jun 2022 06:38:48 -0700 |
On Tue, Jun 28, 2022 at 4:43 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This separates guest file descriptors from host file descriptors,
> and utilizes shared infrastructure for integration with gdbstub.
> Remove the xtensa custom console handing and rely on the
> generic -semihosting-config handling of chardevs.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/xtensa/cpu.h | 1 -
> hw/xtensa/sim.c | 3 -
> target/xtensa/xtensa-semi.c | 226 ++++++++----------------------------
> 3 files changed, 50 insertions(+), 180 deletions(-)
>
> diff --git a/target/xtensa/cpu.h b/target/xtensa/cpu.h
> index ea66895e7f..99ac3efd71 100644
> --- a/target/xtensa/cpu.h
> +++ b/target/xtensa/cpu.h
> @@ -612,7 +612,6 @@ void xtensa_translate_init(void);
> void **xtensa_get_regfile_by_name(const char *name, int entries, int bits);
> void xtensa_breakpoint_handler(CPUState *cs);
> void xtensa_register_core(XtensaConfigList *node);
> -void xtensa_sim_open_console(Chardev *chr);
> void check_interrupts(CPUXtensaState *s);
> void xtensa_irq_init(CPUXtensaState *env);
> qemu_irq *xtensa_get_extints(CPUXtensaState *env);
> diff --git a/hw/xtensa/sim.c b/hw/xtensa/sim.c
> index 946c71cb5b..5cca6a170e 100644
> --- a/hw/xtensa/sim.c
> +++ b/hw/xtensa/sim.c
> @@ -87,9 +87,6 @@ XtensaCPU *xtensa_sim_common_init(MachineState *machine)
> xtensa_create_memory_regions(&sysram, "xtensa.sysram",
> get_system_memory());
> }
> - if (serial_hd(0)) {
> - xtensa_sim_open_console(serial_hd(0));
> - }
I've noticed that with this change '-serial stdio' and its variants are still
accepted in the command line, but now they do nothing. This quiet
change of behavior is unfortunate. I wonder if it would be acceptable
to map the '-serial stdio' option in the presence of '-semihosting' to
something like '-chardev stdio,id=id1 -semihosting-config chardev=id1'?
> @@ -194,165 +169,64 @@ void xtensa_semihosting(CPUXtensaState *env)
...
> case TARGET_SYS_select_one:
> {
> - uint32_t fd = regs[3];
> - uint32_t rq = regs[4];
> - uint32_t target_tv = regs[5];
> - uint32_t target_tvv[2];
> + int timeout, events;
>
> - struct timeval tv = {0};
> + if (regs[5]) {
> + uint32_t tv_sec, tv_usec;
> + uint64_t msec;
>
> - if (target_tv) {
> - cpu_memory_rw_debug(cs, target_tv,
> - (uint8_t *)target_tvv, sizeof(target_tvv), 0);
> - tv.tv_sec = (int32_t)tswap32(target_tvv[0]);
> - tv.tv_usec = (int32_t)tswap32(target_tvv[1]);
> - }
> - if (fd < 3 && sim_console) {
> - if ((fd == 1 || fd == 2) && rq == SELECT_ONE_WRITE) {
> - regs[2] = 1;
> - } else if (fd == 0 && rq == SELECT_ONE_READ) {
> - regs[2] = sim_console->input.offset > 0;
> - } else {
> - regs[2] = 0;
> + if (get_user_u32(tv_sec, regs[5]) ||
> + get_user_u32(tv_usec, regs[5])) {
get_user_u32(tv_usec, regs[5] + 4)?
> + xtensa_cb(cs, -1, EFAULT);
> + return;
> }
> - regs[3] = 0;
> - } else {
> - fd_set fdset;
>
> - FD_ZERO(&fdset);
> - FD_SET(fd, &fdset);
> - regs[2] = select(fd + 1,
> - rq == SELECT_ONE_READ ? &fdset : NULL,
> - rq == SELECT_ONE_WRITE ? &fdset : NULL,
> - rq == SELECT_ONE_EXCEPT ? &fdset : NULL,
> - target_tv ? &tv : NULL);
> - regs[3] = errno_h2g(errno);
> + /* Poll timeout is in milliseconds; overflow to infinity. */
> + msec = tv_sec * 1000ull + DIV_ROUND_UP(tv_usec, 1000ull);
> + timeout = msec <= INT32_MAX ? msec : -1;
> + } else {
> + timeout = -1;
> }
> +
> + switch (regs[4]) {
> + case SELECT_ONE_READ:
> + events = G_IO_IN;
> + break;
> + case SELECT_ONE_WRITE:
> + events = G_IO_OUT;
> + break;
> + case SELECT_ONE_EXCEPT:
> + events = G_IO_PRI;
> + break;
> + default:
> + xtensa_cb(cs, -1, EINVAL);
This doesn't match what there used to be: it was possible to call
select_one with rq other than SELECT_ONE_* and that would've
passed NULL for all fd sets in the select invocation turning it into
a sleep. It would return 0 after the timeout.
--
Thanks.
-- Max
- [PATCH v5 0/2] target/xtensa: semihosting cleanup, Richard Henderson, 2022/06/28
- [PATCH v5 1/2] target/xtensa: Use an exception for semihosting, Richard Henderson, 2022/06/28
- [PATCH v5 2/2] target/xtensa: Use semihosting/syscalls.h, Richard Henderson, 2022/06/28
- Re: [PATCH v5 2/2] target/xtensa: Use semihosting/syscalls.h,
Max Filippov <=
- Re: [PATCH v5 2/2] target/xtensa: Use semihosting/syscalls.h, Richard Henderson, 2022/06/28
- Re: [PATCH v5 2/2] target/xtensa: Use semihosting/syscalls.h, Alex Bennée, 2022/06/29
- Re: [PATCH v5 2/2] target/xtensa: Use semihosting/syscalls.h, Max Filippov, 2022/06/29
- Re: [PATCH v5 2/2] target/xtensa: Use semihosting/syscalls.h, Alex Bennée, 2022/06/29
- Re: [PATCH v5 2/2] target/xtensa: Use semihosting/syscalls.h, Max Filippov, 2022/06/29
- Re: [PATCH v5 2/2] target/xtensa: Use semihosting/syscalls.h, Max Filippov, 2022/06/29
Re: [PATCH v5 0/2] target/xtensa: semihosting cleanup, Max Filippov, 2022/06/28