Re: [PATCH v4 2/2] target/xtensa: Use semihosting/syscalls.h

2022-06-13 Thread Richard Henderson

On 6/13/22 18:06, Max Filippov wrote:

+++ b/hw/xtensa/sim.c
@@ -87,9 +87,6 @@ XtensaCPU *xtensa_sim_common_init(MachineState *machine)
  xtensa_create_memory_regions(, "xtensa.sysram",
   get_system_memory());
  }
-if (serial_hd(0)) {
-xtensa_sim_open_console(serial_hd(0));
-}


Do I understand correctly that the sim machine will no longer
support the -serial option with this change?


No, -serial is still fine.  However, -serial is no longer the semihosting "console" -- 
that will get its own output stream.



+#include "semihosting/syscalls.h"


This does not build on top of the current master, is there a branch where
it's buildable?


Yes, see the cover letter and the Based-on tag, or
https://patchew.org/QEMU/20220608053650.811947-1-richard.hender...@linaro.org/

and the git fetch link there.


git fetch https://github.com/patchew-project/qemu 
tags/patchew/20220608053650.811947-1-richard.hender...@linaro.org



-#ifdef ENOTBLK
-case ENOTBLK:   return TARGET_ENOTBLK;
-#endif


AFAIR there were reports that qemu doesn't build on some
systems because they were missing ENOTBLK and other
error codes that were made conditional here.


Ok, I'll have a dig back.


+E(LOOP);


I'm not sure mangling error code names is a good idea.


Mangling?


r~




Re: [PATCH v4 2/2] target/xtensa: Use semihosting/syscalls.h

2022-06-13 Thread Max Filippov
On Tue, Jun 7, 2022 at 10:36 PM Richard Henderson
 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 
> ---
>  target/xtensa/cpu.h |   1 -
>  hw/xtensa/sim.c |   3 -
>  target/xtensa/xtensa-semi.c | 323 +++-
>  3 files changed, 97 insertions(+), 230 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(, "xtensa.sysram",
>   get_system_memory());
>  }
> -if (serial_hd(0)) {
> -xtensa_sim_open_console(serial_hd(0));
> -}

Do I understand correctly that the sim machine will no longer
support the -serial option with this change?

>  return cpu;
>  }
>
> diff --git a/target/xtensa/xtensa-semi.c b/target/xtensa/xtensa-semi.c
> index 5375f106fc..7ef4be353e 100644
> --- a/target/xtensa/xtensa-semi.c
> +++ b/target/xtensa/xtensa-semi.c
> @@ -27,8 +27,10 @@
>
>  #include "qemu/osdep.h"
>  #include "cpu.h"
> -#include "chardev/char-fe.h"
> +#include "exec/gdbstub.h"
>  #include "semihosting/semihost.h"
> +#include "semihosting/syscalls.h"

This does not build on top of the current master, is there a branch where
it's buildable?

...

> -switch (host_errno) {
> -case 0: return 0;
> -case EPERM: return TARGET_EPERM;
> -case ENOENT:return TARGET_ENOENT;
> -case ESRCH: return TARGET_ESRCH;
> -case EINTR: return TARGET_EINTR;
> -case EIO:   return TARGET_EIO;
> -case ENXIO: return TARGET_ENXIO;
> -case E2BIG: return TARGET_E2BIG;
> -case ENOEXEC:   return TARGET_ENOEXEC;
> -case EBADF: return TARGET_EBADF;
> -case ECHILD:return TARGET_ECHILD;
> -case EAGAIN:return TARGET_EAGAIN;
> -case ENOMEM:return TARGET_ENOMEM;
> -case EACCES:return TARGET_EACCES;
> -case EFAULT:return TARGET_EFAULT;
> -#ifdef ENOTBLK
> -case ENOTBLK:   return TARGET_ENOTBLK;
> -#endif

AFAIR there were reports that qemu doesn't build on some
systems because they were missing ENOTBLK and other
error codes that were made conditional here.

...

> +#define E(N) case E##N: err = TARGET_E##N; break
...
> +E(PERM);
> +E(NOENT);
> +E(SRCH);
> +E(INTR);
> +E(IO);
> +E(NXIO);
> +E(2BIG);
> +E(NOEXEC);
> +E(BADF);
> +E(CHILD);
> +E(AGAIN);
> +E(NOMEM);
> +E(ACCES);
> +E(FAULT);
> +E(NOTBLK);
> +E(BUSY);
> +E(EXIST);
> +E(XDEV);
> +E(NODEV);
> +E(NOTDIR);
> +E(ISDIR);
> +E(INVAL);
> +E(NFILE);
> +E(MFILE);
> +E(NOTTY);
> +E(TXTBSY);
> +E(FBIG);
> +E(NOSPC);
> +E(SPIPE);
> +E(ROFS);
> +E(MLINK);
> +E(PIPE);
> +E(DOM);
> +E(RANGE);
> +E(NOSYS);
> +E(LOOP);

I'm not sure mangling error code names is a good idea.

-- 
Thanks.
-- Max



[PATCH v4 2/2] target/xtensa: Use semihosting/syscalls.h

2022-06-07 Thread Richard Henderson
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 
---
 target/xtensa/cpu.h |   1 -
 hw/xtensa/sim.c |   3 -
 target/xtensa/xtensa-semi.c | 323 +++-
 3 files changed, 97 insertions(+), 230 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(, "xtensa.sysram",
  get_system_memory());
 }
-if (serial_hd(0)) {
-xtensa_sim_open_console(serial_hd(0));
-}
 return cpu;
 }
 
diff --git a/target/xtensa/xtensa-semi.c b/target/xtensa/xtensa-semi.c
index 5375f106fc..7ef4be353e 100644
--- a/target/xtensa/xtensa-semi.c
+++ b/target/xtensa/xtensa-semi.c
@@ -27,8 +27,10 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "chardev/char-fe.h"
+#include "exec/gdbstub.h"
 #include "semihosting/semihost.h"
+#include "semihosting/syscalls.h"
+#include "semihosting/softmmu-uaccess.h"
 #include "qapi/error.h"
 #include "qemu/log.h"
 
@@ -92,99 +94,69 @@ enum {
 TARGET_ELOOP= 92,
 };
 
-static uint32_t errno_h2g(int host_errno)
+static void xtensa_cb(CPUState *cs, uint64_t ret, int err)
 {
-switch (host_errno) {
-case 0: return 0;
-case EPERM: return TARGET_EPERM;
-case ENOENT:return TARGET_ENOENT;
-case ESRCH: return TARGET_ESRCH;
-case EINTR: return TARGET_EINTR;
-case EIO:   return TARGET_EIO;
-case ENXIO: return TARGET_ENXIO;
-case E2BIG: return TARGET_E2BIG;
-case ENOEXEC:   return TARGET_ENOEXEC;
-case EBADF: return TARGET_EBADF;
-case ECHILD:return TARGET_ECHILD;
-case EAGAIN:return TARGET_EAGAIN;
-case ENOMEM:return TARGET_ENOMEM;
-case EACCES:return TARGET_EACCES;
-case EFAULT:return TARGET_EFAULT;
-#ifdef ENOTBLK
-case ENOTBLK:   return TARGET_ENOTBLK;
-#endif
-case EBUSY: return TARGET_EBUSY;
-case EEXIST:return TARGET_EEXIST;
-case EXDEV: return TARGET_EXDEV;
-case ENODEV:return TARGET_ENODEV;
-case ENOTDIR:   return TARGET_ENOTDIR;
-case EISDIR:return TARGET_EISDIR;
-case EINVAL:return TARGET_EINVAL;
-case ENFILE:return TARGET_ENFILE;
-case EMFILE:return TARGET_EMFILE;
-case ENOTTY:return TARGET_ENOTTY;
-#ifdef ETXTBSY
-case ETXTBSY:   return TARGET_ETXTBSY;
-#endif
-case EFBIG: return TARGET_EFBIG;
-case ENOSPC:return TARGET_ENOSPC;
-case ESPIPE:return TARGET_ESPIPE;
-case EROFS: return TARGET_EROFS;
-case EMLINK:return TARGET_EMLINK;
-case EPIPE: return TARGET_EPIPE;
-case EDOM:  return TARGET_EDOM;
-case ERANGE:return TARGET_ERANGE;
-case ENOSYS:return TARGET_ENOSYS;
-#ifdef ELOOP
-case ELOOP: return TARGET_ELOOP;
-#endif
-};
+CPUXtensaState *env = cs->env_ptr;
 
-return TARGET_EINVAL;
-}
+#define E(N) case E##N: err = TARGET_E##N; break
 
-typedef struct XtensaSimConsole {
-CharBackend be;
-struct {
-char buffer[16];
-size_t offset;
-} input;
-} XtensaSimConsole;
-
-static XtensaSimConsole *sim_console;
-
-static IOCanReadHandler sim_console_can_read;
-static int sim_console_can_read(void *opaque)
-{
-XtensaSimConsole *p = opaque;
-
-return sizeof(p->input.buffer) - p->input.offset;
-}
-
-static IOReadHandler sim_console_read;
-static void sim_console_read(void *opaque, const uint8_t *buf, int size)
-{
-XtensaSimConsole *p = opaque;
-size_t copy = sizeof(p->input.buffer) - p->input.offset;
-
-if (size < copy) {
-copy = size;
+switch (err) {
+case 0:
+break;
+E(PERM);
+E(NOENT);
+E(SRCH);
+E(INTR);
+E(IO);
+E(NXIO);
+E(2BIG);
+E(NOEXEC);
+E(BADF);
+E(CHILD);
+E(AGAIN);
+E(NOMEM);
+E(ACCES);
+E(FAULT);
+E(NOTBLK);
+E(BUSY);
+E(EXIST);
+E(XDEV);
+E(NODEV);
+E(NOTDIR);
+E(ISDIR);
+E(INVAL);
+E(NFILE);
+E(MFILE);
+E(NOTTY);
+E(TXTBSY);
+E(FBIG);
+