diff mbox

add semihosting-config option

Message ID 1416337210-90793-2-git-send-email-ilg@livius.net
State New
Headers show

Commit Message

Liviu Ionescu Nov. 18, 2014, 7 p.m. UTC
- allow to define where the semihosting calls will be addressed

Signed-off-by: Liviu Ionescu <ilg@livius.net>
---
 gdbstub.c               | 12 ++++++++++++
 include/sysemu/sysemu.h |  6 ++++++
 qemu-options.hx         | 12 +++++++++++-
 vl.c                    | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 78 insertions(+), 1 deletion(-)

Comments

Peter Maydell Nov. 18, 2014, 7:44 p.m. UTC | #1
On 18 November 2014 19:00, Liviu Ionescu <ilg@livius.net> wrote:
> - allow to define where the semihosting calls will be addressed
>
> Signed-off-by: Liviu Ionescu <ilg@livius.net>
> ---
>  gdbstub.c               | 12 ++++++++++++
>  include/sysemu/sysemu.h |  6 ++++++
>  qemu-options.hx         | 12 +++++++++++-
>  vl.c                    | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 78 insertions(+), 1 deletion(-)

Thanks for sending this as a standalone patch. It looks pretty good,
there are just a few nits:

>
> diff --git a/gdbstub.c b/gdbstub.c
> index 0faca56..372aa67 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -355,6 +355,18 @@ static enum {
>     remote gdb syscalls.  Otherwise use native file IO.  */

The comment here ^^ is now out of date...

>  int use_gdb_syscalls(void)
>  {
> +    if (semihosting_target == SEMIHOSTING_TARGET_NATIVE) {
> +        if (gdb_syscall_mode == GDB_SYS_UNKNOWN) {
> +            gdb_syscall_mode = GDB_SYS_DISABLED;
> +        }
> +        return FALSE;
> +    } else if (semihosting_target == SEMIHOSTING_TARGET_GDB) {
> +        if (gdb_syscall_mode == GDB_SYS_UNKNOWN) {
> +            gdb_syscall_mode = GDB_SYS_ENABLED;
> +        }

...and there isn't much point in these if()s I think
since we will never look at gdb_syscall_mode if
the semihosting_target is not auto; so we needn't set it.

> +        return TRUE;

Also we prefer 'true' and 'false' to TRUE and FALSE.

If you like I can just fix these minor things and put the
patch into my target-arm queue, or you could reroll the patch
yourself if you prefer that.
(The change won't hit master until after we release 2.2 in early
December, so there's no immediate hurry either way.)

(PS: if you're sending a single standalone patch you don't
need to send a cover letter mail as well; the cover letter
is really for providing an overall description of a
multi-patch email series.)

-- PMM
Liviu Ionescu Nov. 18, 2014, 8:23 p.m. UTC | #2
On 18 Nov 2014, at 21:44, Peter Maydell <peter.maydell@linaro.org> wrote:

> ...and there isn't much point in these if()s

right, removed

> Also we prefer 'true' and 'false' to TRUE and FALSE.

fixed

> (The change won't hit master until after we release 2.2 in early
> December, so there's no immediate hurry either way.)

ok, I'll resume work on the rest of the patches then.

> no need to send a cover letter mail 

ok


regards,

Liviu
diff mbox

Patch

diff --git a/gdbstub.c b/gdbstub.c
index 0faca56..372aa67 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -355,6 +355,18 @@  static enum {
    remote gdb syscalls.  Otherwise use native file IO.  */
 int use_gdb_syscalls(void)
 {
+    if (semihosting_target == SEMIHOSTING_TARGET_NATIVE) {
+        if (gdb_syscall_mode == GDB_SYS_UNKNOWN) {
+            gdb_syscall_mode = GDB_SYS_DISABLED;
+        }
+        return FALSE;
+    } else if (semihosting_target == SEMIHOSTING_TARGET_GDB) {
+        if (gdb_syscall_mode == GDB_SYS_UNKNOWN) {
+            gdb_syscall_mode = GDB_SYS_ENABLED;
+        }
+        return TRUE;
+    }
+
     if (gdb_syscall_mode == GDB_SYS_UNKNOWN) {
         gdb_syscall_mode = (gdbserver_state ? GDB_SYS_ENABLED
                                             : GDB_SYS_DISABLED);
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 9fea3bc..c145d94 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -127,7 +127,13 @@  extern int cursor_hide;
 extern int graphic_rotate;
 extern int no_quit;
 extern int no_shutdown;
+
 extern int semihosting_enabled;
+extern int semihosting_target;
+#define SEMIHOSTING_TARGET_AUTO     0
+#define SEMIHOSTING_TARGET_NATIVE   1
+#define SEMIHOSTING_TARGET_GDB      2
+
 extern int old_param;
 extern int boot_menu;
 extern bool boot_strict;
diff --git a/qemu-options.hx b/qemu-options.hx
index da9851d..3e81222 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3216,7 +3216,17 @@  DEF("semihosting", 0, QEMU_OPTION_semihosting,
 STEXI
 @item -semihosting
 @findex -semihosting
-Semihosting mode (ARM, M68K, Xtensa only).
+Enable semihosting mode (ARM, M68K, Xtensa only).
+ETEXI
+DEF("semihosting-config", HAS_ARG, QEMU_OPTION_semihosting_config,
+    "-semihosting-config [enable=on|off,]target=native|gdb|auto   semihosting configuration\n",
+QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_LM32)
+STEXI
+@item -semihosting-config [enable=on|off,]target=native|gdb|auto
+@findex -semihosting-config
+Enable semihosting and define where the semihosting calls will be addressed,
+to QEMU (@code{native}) or to GDB (@code{gdb}). The default is @code{auto}, which means
+@code{gdb} during debug sessions and @code{native} otherwise (ARM, M68K, Xtensa only).
 ETEXI
 DEF("old-param", 0, QEMU_OPTION_old_param,
     "-old-param      old param mode\n", QEMU_ARCH_ARM)
diff --git a/vl.c b/vl.c
index f4a6e5e..8844983 100644
--- a/vl.c
+++ b/vl.c
@@ -172,6 +172,7 @@  const char *watchdog;
 QEMUOptionRom option_rom[MAX_OPTION_ROMS];
 int nb_option_roms;
 int semihosting_enabled = 0;
+int semihosting_target = SEMIHOSTING_TARGET_AUTO;
 int old_param = 0;
 const char *qemu_name;
 int alt_grab = 0;
@@ -554,6 +555,22 @@  static QemuOptsList qemu_icount_opts = {
     },
 };
 
+static QemuOptsList qemu_semihosting_config_opts = {
+    .name = "semihosting-config",
+    .implied_opt_name = "enable",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_semihosting_config_opts.head),
+    .desc = {
+        {
+            .name = "enable",
+            .type = QEMU_OPT_BOOL,
+        }, {
+            .name = "target",
+            .type = QEMU_OPT_STRING,
+        },
+        { /* end of list */ }
+    },
+};
+
 /**
  * Get machine options
  *
@@ -2811,6 +2828,7 @@  int main(int argc, char **argv, char **envp)
     qemu_add_opts(&qemu_name_opts);
     qemu_add_opts(&qemu_numa_opts);
     qemu_add_opts(&qemu_icount_opts);
+    qemu_add_opts(&qemu_semihosting_config_opts);
 
     runstate_init();
 
@@ -3618,6 +3636,37 @@  int main(int argc, char **argv, char **envp)
 		break;
             case QEMU_OPTION_semihosting:
                 semihosting_enabled = 1;
+                semihosting_target = SEMIHOSTING_TARGET_AUTO;
+                break;
+            case QEMU_OPTION_semihosting_config:
+                semihosting_enabled = 1;
+                opts = qemu_opts_parse(qemu_find_opts("semihosting-config"),
+                                           optarg, 0);
+                if (opts != NULL) {
+                    semihosting_enabled = qemu_opt_get_bool(opts, "enable",
+                                                            true);
+                    const char *target = qemu_opt_get(opts, "target");
+                    if (target != NULL) {
+                        if (strcmp("native", target) == 0) {
+                            semihosting_target = SEMIHOSTING_TARGET_NATIVE;
+                        } else if (strcmp("gdb", target) == 0) {
+                            semihosting_target = SEMIHOSTING_TARGET_GDB;
+                        } else  if (strcmp("auto", target) == 0) {
+                            semihosting_target = SEMIHOSTING_TARGET_AUTO;
+                        } else {
+                            fprintf(stderr, "Unsupported semihosting-config"
+                                    " %s\n",
+                                optarg);
+                            exit(1);
+                        }
+                    } else {
+                        semihosting_target = SEMIHOSTING_TARGET_AUTO;
+                    }
+                } else {
+                    fprintf(stderr, "Unsupported semihosting-config %s\n",
+                            optarg);
+                    exit(1);
+                }
                 break;
             case QEMU_OPTION_tdf:
                 fprintf(stderr, "Warning: user space PIT time drift fix "