* [Qemu-devel] [RFC PATCH 01/11] semihosting: move semihosting configuration into its own directory
2019-05-14 15:52 [Qemu-devel] [RFC PATCH 00/11] semihosting cleanup and re-factor Alex Bennée
@ 2019-05-14 15:52 ` Alex Bennée
2019-05-14 16:23 ` Philippe Mathieu-Daudé
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 02/11] semihosting: introduce CONFIG_SEMIHOSTING Alex Bennée
` (10 subsequent siblings)
11 siblings, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2019-05-14 15:52 UTC (permalink / raw)
To: qemu-devel
Cc: Marek Vasut, Peter Maydell, Aleksandar Rikalo, Chris Wulff,
Laurent Vivier, Max Filippov, Michael Walle, qemu-arm,
Aleksandar Markovic, Paolo Bonzini, Alex Bennée,
Aurelien Jarno
In preparation for having some more common semihosting code let's
excise the current config magic from vl.c into its own file. We shall
later add more conditionals to the build configurations so we can
avoid building this if we don't need it.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
gdbstub.c | 2 +-
hw/Makefile.objs | 1 +
hw/mips/mips_malta.c | 2 +-
hw/semihosting/Makefile.objs | 1 +
hw/semihosting/config.c | 160 ++++++++++++++++++++
include/{exec => hw/semihosting}/semihost.h | 10 +-
include/sysemu/sysemu.h | 1 +
target/arm/arm-semi.c | 2 +-
target/arm/helper.c | 2 +-
target/arm/translate-a64.c | 2 +-
target/arm/translate.c | 2 +-
target/lm32/helper.c | 2 +-
target/m68k/op_helper.c | 2 +-
target/mips/mips-semi.c | 2 +-
target/mips/translate.c | 2 +-
target/nios2/helper.c | 2 +-
target/xtensa/translate.c | 2 +-
target/xtensa/xtensa-semi.c | 2 +-
vl.c | 126 +--------------
19 files changed, 186 insertions(+), 139 deletions(-)
create mode 100644 hw/semihosting/Makefile.objs
create mode 100644 hw/semihosting/config.c
rename include/{exec => hw/semihosting}/semihost.h (85%)
diff --git a/gdbstub.c b/gdbstub.c
index d54abd17cc2..793218bb43a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -37,7 +37,7 @@
#include "qemu/sockets.h"
#include "sysemu/hw_accel.h"
#include "sysemu/kvm.h"
-#include "exec/semihost.h"
+#include "hw/semihosting/semihost.h"
#include "exec/exec-all.h"
#ifdef CONFIG_USER_ONLY
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 82aa7fab8e4..d770926ba96 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -36,6 +36,7 @@ devices-dirs-$(CONFIG_SOFTMMU) += watchdog/
devices-dirs-$(CONFIG_SOFTMMU) += xen/
devices-dirs-$(CONFIG_MEM_DEVICE) += mem/
devices-dirs-$(CONFIG_SOFTMMU) += smbios/
+devices-dirs-y += semihosting/
devices-dirs-y += core/
common-obj-y += $(devices-dirs-y)
obj-y += $(devices-dirs-y)
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 439665ab45e..3b009fda25f 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -55,7 +55,7 @@
#include "qemu/error-report.h"
#include "hw/empty_slot.h"
#include "sysemu/kvm.h"
-#include "exec/semihost.h"
+#include "hw/semihosting/semihost.h"
#include "hw/mips/cps.h"
#define ENVP_ADDR 0x80002000l
diff --git a/hw/semihosting/Makefile.objs b/hw/semihosting/Makefile.objs
new file mode 100644
index 00000000000..546954f4ff4
--- /dev/null
+++ b/hw/semihosting/Makefile.objs
@@ -0,0 +1 @@
+common-obj-$(CONFIG_SOFTMMU) += config.o
diff --git a/hw/semihosting/config.c b/hw/semihosting/config.c
new file mode 100644
index 00000000000..f1d3fe1e4c6
--- /dev/null
+++ b/hw/semihosting/config.c
@@ -0,0 +1,160 @@
+/*
+ * Semihosting configuration
+ *
+ * Copyright (c) 2015 Imagination Technologies
+ * Copyright (c) 2019 Linaro Ltd
+ *
+ * This controls the configuration of semihosting for all guest
+ * targets that support it. Architecture specific handling is handled
+ * in target/HW/HW-semi.c
+ *
+ * Semihosting is sightly strange in that it is also supported by some
+ * linux-user targets. However in that use case no configuration of
+ * the outputs and command lines is supported.
+ *
+ * The config module is common to all softmmu targets however as vl.c
+ * needs to link against the helpers.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/option.h"
+#include "qemu/config-file.h"
+#include "qemu/error-report.h"
+#include "hw/semihosting/semihost.h"
+
+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,
+ }, {
+ .name = "arg",
+ .type = QEMU_OPT_STRING,
+ },
+ { /* end of list */ }
+ },
+};
+
+typedef struct SemihostingConfig {
+ bool enabled;
+ SemihostingTarget target;
+ const char **argv;
+ int argc;
+ const char *cmdline; /* concatenated argv */
+} SemihostingConfig;
+
+static SemihostingConfig semihosting;
+
+bool semihosting_enabled(void)
+{
+ return semihosting.enabled;
+}
+
+SemihostingTarget semihosting_get_target(void)
+{
+ return semihosting.target;
+}
+
+const char *semihosting_get_arg(int i)
+{
+ if (i >= semihosting.argc) {
+ return NULL;
+ }
+ return semihosting.argv[i];
+}
+
+int semihosting_get_argc(void)
+{
+ return semihosting.argc;
+}
+
+const char *semihosting_get_cmdline(void)
+{
+ if (semihosting.cmdline == NULL && semihosting.argc > 0) {
+ semihosting.cmdline = g_strjoinv(" ", (gchar **)semihosting.argv);
+ }
+ return semihosting.cmdline;
+}
+
+static int add_semihosting_arg(void *opaque,
+ const char *name, const char *val,
+ Error **errp)
+{
+ SemihostingConfig *s = opaque;
+ if (strcmp(name, "arg") == 0) {
+ s->argc++;
+ /* one extra element as g_strjoinv() expects NULL-terminated array */
+ s->argv = g_realloc(s->argv, (s->argc + 1) * sizeof(void *));
+ s->argv[s->argc - 1] = val;
+ s->argv[s->argc] = NULL;
+ }
+ return 0;
+}
+
+/* Use strings passed via -kernel/-append to initialize semihosting.argv[] */
+void semihosting_arg_fallback(const char *file, const char *cmd)
+{
+ char *cmd_token;
+
+ /* argv[0] */
+ add_semihosting_arg(&semihosting, "arg", file, NULL);
+
+ /* split -append and initialize argv[1..n] */
+ cmd_token = strtok(g_strdup(cmd), " ");
+ while (cmd_token) {
+ add_semihosting_arg(&semihosting, "arg", cmd_token, NULL);
+ cmd_token = strtok(NULL, " ");
+ }
+}
+
+void qemu_semihosting_enable(void)
+{
+ semihosting.enabled = true;
+ semihosting.target = SEMIHOSTING_TARGET_AUTO;
+}
+
+int qemu_semihosting_config_options(const char *optarg)
+{
+ QemuOptsList *opt_list = qemu_find_opts("semihosting-config");
+ QemuOpts *opts = qemu_opts_parse_noisily(opt_list, optarg, false);
+
+ semihosting.enabled = true;
+
+ 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 {
+ error_report("unsupported semihosting-config %s",
+ optarg);
+ return 1;
+ }
+ } else {
+ semihosting.target = SEMIHOSTING_TARGET_AUTO;
+ }
+ /* Set semihosting argument count and vector */
+ qemu_opt_foreach(opts, add_semihosting_arg,
+ &semihosting, NULL);
+ } else {
+ error_report("unsupported semihosting-config %s", optarg);
+ return 1;
+ }
+
+ return 0;
+}
+
diff --git a/include/exec/semihost.h b/include/hw/semihosting/semihost.h
similarity index 85%
rename from include/exec/semihost.h
rename to include/hw/semihosting/semihost.h
index 5980939c7b8..07ea40a322f 100644
--- a/include/exec/semihost.h
+++ b/include/hw/semihosting/semihost.h
@@ -51,12 +51,16 @@ static inline const char *semihosting_get_cmdline(void)
{
return NULL;
}
-#else
+#else /* !CONFIG_USER_ONLY */
bool semihosting_enabled(void);
SemihostingTarget semihosting_get_target(void);
const char *semihosting_get_arg(int i);
int semihosting_get_argc(void);
const char *semihosting_get_cmdline(void);
-#endif
+void semihosting_arg_fallback(const char *file, const char *cmd);
+/* for vl.c hooks */
+void qemu_semihosting_enable(void);
+int qemu_semihosting_config_options(const char *opt);
+#endif /* CONFIG_USER_ONLY */
-#endif
+#endif /* SEMIHOST_H */
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 5f133cae837..61579ae71ef 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -193,5 +193,6 @@ extern QemuOptsList qemu_nic_opts;
extern QemuOptsList qemu_net_opts;
extern QemuOptsList qemu_global_opts;
extern QemuOptsList qemu_mon_opts;
+extern QemuOptsList qemu_semihosting_config_opts;
#endif
diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
index 8b5fd7bc6e3..3273306c8ea 100644
--- a/target/arm/arm-semi.c
+++ b/target/arm/arm-semi.c
@@ -21,7 +21,7 @@
#include "qemu/osdep.h"
#include "cpu.h"
-#include "exec/semihost.h"
+#include "hw/semihosting/semihost.h"
#ifdef CONFIG_USER_ONLY
#include "qemu.h"
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 1e6eb0d0f36..bc20cb3b9e2 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -16,7 +16,7 @@
#include "exec/cpu_ldst.h"
#include "arm_ldst.h"
#include <zlib.h> /* For crc32 */
-#include "exec/semihost.h"
+#include "hw/semihosting/semihost.h"
#include "sysemu/cpus.h"
#include "sysemu/kvm.h"
#include "fpu/softfloat.h"
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index b7c5a928b4a..8844a75e53d 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -29,7 +29,7 @@
#include "qemu/host-utils.h"
#include "qemu/qemu-print.h"
-#include "exec/semihost.h"
+#include "hw/semihosting/semihost.h"
#include "exec/gen-icount.h"
#include "exec/helper-proto.h"
diff --git a/target/arm/translate.c b/target/arm/translate.c
index dd053c80d62..f2a65c0b5b0 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -30,7 +30,7 @@
#include "qemu/bitops.h"
#include "qemu/qemu-print.h"
#include "arm_ldst.h"
-#include "exec/semihost.h"
+#include "hw/semihosting/semihost.h"
#include "exec/helper-proto.h"
#include "exec/helper-gen.h"
diff --git a/target/lm32/helper.c b/target/lm32/helper.c
index a039a993ffe..e83cdf43182 100644
--- a/target/lm32/helper.c
+++ b/target/lm32/helper.c
@@ -22,7 +22,7 @@
#include "exec/exec-all.h"
#include "qemu/host-utils.h"
#include "sysemu/sysemu.h"
-#include "exec/semihost.h"
+#include "hw/semihosting/semihost.h"
#include "exec/log.h"
int lm32_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size, int rw,
diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index 76f439985a0..ee578f8d60d 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -21,7 +21,7 @@
#include "exec/helper-proto.h"
#include "exec/exec-all.h"
#include "exec/cpu_ldst.h"
-#include "exec/semihost.h"
+#include "hw/semihosting/semihost.h"
#if defined(CONFIG_USER_ONLY)
diff --git a/target/mips/mips-semi.c b/target/mips/mips-semi.c
index a7aefbaefc8..eac8374fb34 100644
--- a/target/mips/mips-semi.c
+++ b/target/mips/mips-semi.c
@@ -22,7 +22,7 @@
#include "qemu/log.h"
#include "exec/helper-proto.h"
#include "exec/softmmu-semi.h"
-#include "exec/semihost.h"
+#include "hw/semihosting/semihost.h"
typedef enum UHIOp {
UHI_exit = 1,
diff --git a/target/mips/translate.c b/target/mips/translate.c
index f96c0d01ef1..3cd5b11b16b 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -32,7 +32,7 @@
#include "exec/helper-proto.h"
#include "exec/helper-gen.h"
-#include "exec/semihost.h"
+#include "hw/semihosting/semihost.h"
#include "target/mips/trace.h"
#include "trace-tcg.h"
diff --git a/target/nios2/helper.c b/target/nios2/helper.c
index e01fc1ff3e7..a3bd93e483d 100644
--- a/target/nios2/helper.c
+++ b/target/nios2/helper.c
@@ -26,7 +26,7 @@
#include "exec/cpu_ldst.h"
#include "exec/log.h"
#include "exec/helper-proto.h"
-#include "exec/semihost.h"
+#include "hw/semihosting/semihost.h"
#if defined(CONFIG_USER_ONLY)
diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index b063fa85f26..6bdc244b583 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -38,7 +38,7 @@
#include "qemu/qemu-print.h"
#include "sysemu/sysemu.h"
#include "exec/cpu_ldst.h"
-#include "exec/semihost.h"
+#include "hw/semihosting/semihost.h"
#include "exec/translator.h"
#include "exec/helper-proto.h"
diff --git a/target/xtensa/xtensa-semi.c b/target/xtensa/xtensa-semi.c
index 5f5ce4f344c..38efa3485af 100644
--- a/target/xtensa/xtensa-semi.c
+++ b/target/xtensa/xtensa-semi.c
@@ -29,7 +29,7 @@
#include "cpu.h"
#include "chardev/char-fe.h"
#include "exec/helper-proto.h"
-#include "exec/semihost.h"
+#include "hw/semihosting/semihost.h"
#include "qapi/error.h"
#include "qemu/log.h"
#include "sysemu/sysemu.h"
diff --git a/vl.c b/vl.c
index b6709514c1b..fba48b565c9 100644
--- a/vl.c
+++ b/vl.c
@@ -116,7 +116,7 @@ int main(int argc, char **argv)
#include "qapi/opts-visitor.h"
#include "qapi/clone-visitor.h"
#include "qom/object_interfaces.h"
-#include "exec/semihost.h"
+#include "hw/semihosting/semihost.h"
#include "crypto/init.h"
#include "sysemu/replay.h"
#include "qapi/qapi-events-run-state.h"
@@ -500,25 +500,6 @@ 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,
- }, {
- .name = "arg",
- .type = QEMU_OPT_STRING,
- },
- { /* end of list */ }
- },
-};
-
static QemuOptsList qemu_fw_cfg_opts = {
.name = "fw_cfg",
.implied_opt_name = "name",
@@ -1350,80 +1331,6 @@ static void configure_msg(QemuOpts *opts)
enable_timestamp_msg = qemu_opt_get_bool(opts, "timestamp", true);
}
-/***********************************************************/
-/* Semihosting */
-
-typedef struct SemihostingConfig {
- bool enabled;
- SemihostingTarget target;
- const char **argv;
- int argc;
- const char *cmdline; /* concatenated argv */
-} SemihostingConfig;
-
-static SemihostingConfig semihosting;
-
-bool semihosting_enabled(void)
-{
- return semihosting.enabled;
-}
-
-SemihostingTarget semihosting_get_target(void)
-{
- return semihosting.target;
-}
-
-const char *semihosting_get_arg(int i)
-{
- if (i >= semihosting.argc) {
- return NULL;
- }
- return semihosting.argv[i];
-}
-
-int semihosting_get_argc(void)
-{
- return semihosting.argc;
-}
-
-const char *semihosting_get_cmdline(void)
-{
- if (semihosting.cmdline == NULL && semihosting.argc > 0) {
- semihosting.cmdline = g_strjoinv(" ", (gchar **)semihosting.argv);
- }
- return semihosting.cmdline;
-}
-
-static int add_semihosting_arg(void *opaque,
- const char *name, const char *val,
- Error **errp)
-{
- SemihostingConfig *s = opaque;
- if (strcmp(name, "arg") == 0) {
- s->argc++;
- /* one extra element as g_strjoinv() expects NULL-terminated array */
- s->argv = g_realloc(s->argv, (s->argc + 1) * sizeof(void *));
- s->argv[s->argc - 1] = val;
- s->argv[s->argc] = NULL;
- }
- return 0;
-}
-
-/* Use strings passed via -kernel/-append to initialize semihosting.argv[] */
-static inline void semihosting_arg_fallback(const char *file, const char *cmd)
-{
- char *cmd_token;
-
- /* argv[0] */
- add_semihosting_arg(&semihosting, "arg", file, NULL);
-
- /* split -append and initialize argv[1..n] */
- cmd_token = strtok(g_strdup(cmd), " ");
- while (cmd_token) {
- add_semihosting_arg(&semihosting, "arg", cmd_token, NULL);
- cmd_token = strtok(NULL, " ");
- }
-}
/* Now we still need this for compatibility with XEN. */
bool has_igd_gfx_passthru;
@@ -3733,37 +3640,10 @@ int main(int argc, char **argv, char **envp)
nb_option_roms++;
break;
case QEMU_OPTION_semihosting:
- semihosting.enabled = true;
- semihosting.target = SEMIHOSTING_TARGET_AUTO;
+ qemu_semihosting_enable();
break;
case QEMU_OPTION_semihosting_config:
- semihosting.enabled = true;
- opts = qemu_opts_parse_noisily(qemu_find_opts("semihosting-config"),
- optarg, false);
- 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 {
- error_report("unsupported semihosting-config %s",
- optarg);
- exit(1);
- }
- } else {
- semihosting.target = SEMIHOSTING_TARGET_AUTO;
- }
- /* Set semihosting argument count and vector */
- qemu_opt_foreach(opts, add_semihosting_arg,
- &semihosting, NULL);
- } else {
- error_report("unsupported semihosting-config %s", optarg);
+ if (qemu_semihosting_config_options(optarg) != 0) {
exit(1);
}
break;
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 01/11] semihosting: move semihosting configuration into its own directory
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 01/11] semihosting: move semihosting configuration into its own directory Alex Bennée
@ 2019-05-14 16:23 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-14 16:23 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: Marek Vasut, Peter Maydell, Aleksandar Rikalo, Chris Wulff,
Laurent Vivier, Max Filippov, Michael Walle, qemu-arm,
Aleksandar Markovic, Paolo Bonzini, Aurelien Jarno
On 5/14/19 5:52 PM, Alex Bennée wrote:
> In preparation for having some more common semihosting code let's
> excise the current config magic from vl.c into its own file. We shall
> later add more conditionals to the build configurations so we can
> avoid building this if we don't need it.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> gdbstub.c | 2 +-
> hw/Makefile.objs | 1 +
> hw/mips/mips_malta.c | 2 +-
> hw/semihosting/Makefile.objs | 1 +
> hw/semihosting/config.c | 160 ++++++++++++++++++++
> include/{exec => hw/semihosting}/semihost.h | 10 +-
> include/sysemu/sysemu.h | 1 +
> target/arm/arm-semi.c | 2 +-
> target/arm/helper.c | 2 +-
> target/arm/translate-a64.c | 2 +-
> target/arm/translate.c | 2 +-
> target/lm32/helper.c | 2 +-
> target/m68k/op_helper.c | 2 +-
> target/mips/mips-semi.c | 2 +-
> target/mips/translate.c | 2 +-
> target/nios2/helper.c | 2 +-
> target/xtensa/translate.c | 2 +-
> target/xtensa/xtensa-semi.c | 2 +-
> vl.c | 126 +--------------
> 19 files changed, 186 insertions(+), 139 deletions(-)
> create mode 100644 hw/semihosting/Makefile.objs
> create mode 100644 hw/semihosting/config.c
> rename include/{exec => hw/semihosting}/semihost.h (85%)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index d54abd17cc2..793218bb43a 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -37,7 +37,7 @@
> #include "qemu/sockets.h"
> #include "sysemu/hw_accel.h"
> #include "sysemu/kvm.h"
> -#include "exec/semihost.h"
> +#include "hw/semihosting/semihost.h"
> #include "exec/exec-all.h"
>
> #ifdef CONFIG_USER_ONLY
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index 82aa7fab8e4..d770926ba96 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -36,6 +36,7 @@ devices-dirs-$(CONFIG_SOFTMMU) += watchdog/
> devices-dirs-$(CONFIG_SOFTMMU) += xen/
> devices-dirs-$(CONFIG_MEM_DEVICE) += mem/
> devices-dirs-$(CONFIG_SOFTMMU) += smbios/
> +devices-dirs-y += semihosting/
> devices-dirs-y += core/
> common-obj-y += $(devices-dirs-y)
> obj-y += $(devices-dirs-y)
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 439665ab45e..3b009fda25f 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -55,7 +55,7 @@
> #include "qemu/error-report.h"
> #include "hw/empty_slot.h"
> #include "sysemu/kvm.h"
> -#include "exec/semihost.h"
> +#include "hw/semihosting/semihost.h"
> #include "hw/mips/cps.h"
>
> #define ENVP_ADDR 0x80002000l
> diff --git a/hw/semihosting/Makefile.objs b/hw/semihosting/Makefile.objs
> new file mode 100644
> index 00000000000..546954f4ff4
> --- /dev/null
> +++ b/hw/semihosting/Makefile.objs
> @@ -0,0 +1 @@
> +common-obj-$(CONFIG_SOFTMMU) += config.o
> diff --git a/hw/semihosting/config.c b/hw/semihosting/config.c
> new file mode 100644
> index 00000000000..f1d3fe1e4c6
> --- /dev/null
> +++ b/hw/semihosting/config.c
> @@ -0,0 +1,160 @@
> +/*
> + * Semihosting configuration
> + *
> + * Copyright (c) 2015 Imagination Technologies
> + * Copyright (c) 2019 Linaro Ltd
> + *
> + * This controls the configuration of semihosting for all guest
> + * targets that support it. Architecture specific handling is handled
> + * in target/HW/HW-semi.c
> + *
> + * Semihosting is sightly strange in that it is also supported by some
> + * linux-user targets. However in that use case no configuration of
> + * the outputs and command lines is supported.
> + *
> + * The config module is common to all softmmu targets however as vl.c
> + * needs to link against the helpers.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/option.h"
> +#include "qemu/config-file.h"
> +#include "qemu/error-report.h"
> +#include "hw/semihosting/semihost.h"
> +
> +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,
> + }, {
> + .name = "arg",
> + .type = QEMU_OPT_STRING,
> + },
> + { /* end of list */ }
> + },
> +};
> +
> +typedef struct SemihostingConfig {
> + bool enabled;
> + SemihostingTarget target;
> + const char **argv;
> + int argc;
> + const char *cmdline; /* concatenated argv */
> +} SemihostingConfig;
> +
> +static SemihostingConfig semihosting;
> +
> +bool semihosting_enabled(void)
> +{
> + return semihosting.enabled;
> +}
> +
> +SemihostingTarget semihosting_get_target(void)
> +{
> + return semihosting.target;
> +}
> +
> +const char *semihosting_get_arg(int i)
> +{
> + if (i >= semihosting.argc) {
> + return NULL;
> + }
> + return semihosting.argv[i];
> +}
> +
> +int semihosting_get_argc(void)
> +{
> + return semihosting.argc;
> +}
> +
> +const char *semihosting_get_cmdline(void)
> +{
> + if (semihosting.cmdline == NULL && semihosting.argc > 0) {
> + semihosting.cmdline = g_strjoinv(" ", (gchar **)semihosting.argv);
> + }
> + return semihosting.cmdline;
> +}
> +
> +static int add_semihosting_arg(void *opaque,
> + const char *name, const char *val,
> + Error **errp)
> +{
> + SemihostingConfig *s = opaque;
> + if (strcmp(name, "arg") == 0) {
> + s->argc++;
> + /* one extra element as g_strjoinv() expects NULL-terminated array */
> + s->argv = g_realloc(s->argv, (s->argc + 1) * sizeof(void *));
> + s->argv[s->argc - 1] = val;
> + s->argv[s->argc] = NULL;
> + }
> + return 0;
> +}
> +
> +/* Use strings passed via -kernel/-append to initialize semihosting.argv[] */
> +void semihosting_arg_fallback(const char *file, const char *cmd)
> +{
> + char *cmd_token;
> +
> + /* argv[0] */
> + add_semihosting_arg(&semihosting, "arg", file, NULL);
> +
> + /* split -append and initialize argv[1..n] */
> + cmd_token = strtok(g_strdup(cmd), " ");
> + while (cmd_token) {
> + add_semihosting_arg(&semihosting, "arg", cmd_token, NULL);
> + cmd_token = strtok(NULL, " ");
> + }
> +}
> +
> +void qemu_semihosting_enable(void)
> +{
> + semihosting.enabled = true;
> + semihosting.target = SEMIHOSTING_TARGET_AUTO;
> +}
> +
> +int qemu_semihosting_config_options(const char *optarg)
> +{
> + QemuOptsList *opt_list = qemu_find_opts("semihosting-config");
> + QemuOpts *opts = qemu_opts_parse_noisily(opt_list, optarg, false);
> +
> + semihosting.enabled = true;
> +
> + 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 {
> + error_report("unsupported semihosting-config %s",
> + optarg);
> + return 1;
> + }
> + } else {
> + semihosting.target = SEMIHOSTING_TARGET_AUTO;
> + }
> + /* Set semihosting argument count and vector */
> + qemu_opt_foreach(opts, add_semihosting_arg,
> + &semihosting, NULL);
> + } else {
> + error_report("unsupported semihosting-config %s", optarg);
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> diff --git a/include/exec/semihost.h b/include/hw/semihosting/semihost.h
> similarity index 85%
> rename from include/exec/semihost.h
> rename to include/hw/semihosting/semihost.h
> index 5980939c7b8..07ea40a322f 100644
> --- a/include/exec/semihost.h
> +++ b/include/hw/semihosting/semihost.h
> @@ -51,12 +51,16 @@ static inline const char *semihosting_get_cmdline(void)
> {
> return NULL;
> }
> -#else
> +#else /* !CONFIG_USER_ONLY */
> bool semihosting_enabled(void);
> SemihostingTarget semihosting_get_target(void);
> const char *semihosting_get_arg(int i);
> int semihosting_get_argc(void);
> const char *semihosting_get_cmdline(void);
> -#endif
> +void semihosting_arg_fallback(const char *file, const char *cmd);
> +/* for vl.c hooks */
> +void qemu_semihosting_enable(void);
> +int qemu_semihosting_config_options(const char *opt);
> +#endif /* CONFIG_USER_ONLY */
>
> -#endif
> +#endif /* SEMIHOST_H */
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 5f133cae837..61579ae71ef 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -193,5 +193,6 @@ extern QemuOptsList qemu_nic_opts;
> extern QemuOptsList qemu_net_opts;
> extern QemuOptsList qemu_global_opts;
> extern QemuOptsList qemu_mon_opts;
> +extern QemuOptsList qemu_semihosting_config_opts;
>
> #endif
> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
> index 8b5fd7bc6e3..3273306c8ea 100644
> --- a/target/arm/arm-semi.c
> +++ b/target/arm/arm-semi.c
> @@ -21,7 +21,7 @@
> #include "qemu/osdep.h"
>
> #include "cpu.h"
> -#include "exec/semihost.h"
> +#include "hw/semihosting/semihost.h"
> #ifdef CONFIG_USER_ONLY
> #include "qemu.h"
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 1e6eb0d0f36..bc20cb3b9e2 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -16,7 +16,7 @@
> #include "exec/cpu_ldst.h"
> #include "arm_ldst.h"
> #include <zlib.h> /* For crc32 */
> -#include "exec/semihost.h"
> +#include "hw/semihosting/semihost.h"
> #include "sysemu/cpus.h"
> #include "sysemu/kvm.h"
> #include "fpu/softfloat.h"
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index b7c5a928b4a..8844a75e53d 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -29,7 +29,7 @@
> #include "qemu/host-utils.h"
> #include "qemu/qemu-print.h"
>
> -#include "exec/semihost.h"
> +#include "hw/semihosting/semihost.h"
> #include "exec/gen-icount.h"
>
> #include "exec/helper-proto.h"
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index dd053c80d62..f2a65c0b5b0 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -30,7 +30,7 @@
> #include "qemu/bitops.h"
> #include "qemu/qemu-print.h"
> #include "arm_ldst.h"
> -#include "exec/semihost.h"
> +#include "hw/semihosting/semihost.h"
>
> #include "exec/helper-proto.h"
> #include "exec/helper-gen.h"
> diff --git a/target/lm32/helper.c b/target/lm32/helper.c
> index a039a993ffe..e83cdf43182 100644
> --- a/target/lm32/helper.c
> +++ b/target/lm32/helper.c
> @@ -22,7 +22,7 @@
> #include "exec/exec-all.h"
> #include "qemu/host-utils.h"
> #include "sysemu/sysemu.h"
> -#include "exec/semihost.h"
> +#include "hw/semihosting/semihost.h"
> #include "exec/log.h"
>
> int lm32_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size, int rw,
> diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
> index 76f439985a0..ee578f8d60d 100644
> --- a/target/m68k/op_helper.c
> +++ b/target/m68k/op_helper.c
> @@ -21,7 +21,7 @@
> #include "exec/helper-proto.h"
> #include "exec/exec-all.h"
> #include "exec/cpu_ldst.h"
> -#include "exec/semihost.h"
> +#include "hw/semihosting/semihost.h"
>
> #if defined(CONFIG_USER_ONLY)
>
> diff --git a/target/mips/mips-semi.c b/target/mips/mips-semi.c
> index a7aefbaefc8..eac8374fb34 100644
> --- a/target/mips/mips-semi.c
> +++ b/target/mips/mips-semi.c
> @@ -22,7 +22,7 @@
> #include "qemu/log.h"
> #include "exec/helper-proto.h"
> #include "exec/softmmu-semi.h"
> -#include "exec/semihost.h"
> +#include "hw/semihosting/semihost.h"
>
> typedef enum UHIOp {
> UHI_exit = 1,
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index f96c0d01ef1..3cd5b11b16b 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -32,7 +32,7 @@
>
> #include "exec/helper-proto.h"
> #include "exec/helper-gen.h"
> -#include "exec/semihost.h"
> +#include "hw/semihosting/semihost.h"
>
> #include "target/mips/trace.h"
> #include "trace-tcg.h"
> diff --git a/target/nios2/helper.c b/target/nios2/helper.c
> index e01fc1ff3e7..a3bd93e483d 100644
> --- a/target/nios2/helper.c
> +++ b/target/nios2/helper.c
> @@ -26,7 +26,7 @@
> #include "exec/cpu_ldst.h"
> #include "exec/log.h"
> #include "exec/helper-proto.h"
> -#include "exec/semihost.h"
> +#include "hw/semihosting/semihost.h"
>
> #if defined(CONFIG_USER_ONLY)
>
> diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
> index b063fa85f26..6bdc244b583 100644
> --- a/target/xtensa/translate.c
> +++ b/target/xtensa/translate.c
> @@ -38,7 +38,7 @@
> #include "qemu/qemu-print.h"
> #include "sysemu/sysemu.h"
> #include "exec/cpu_ldst.h"
> -#include "exec/semihost.h"
> +#include "hw/semihosting/semihost.h"
> #include "exec/translator.h"
>
> #include "exec/helper-proto.h"
> diff --git a/target/xtensa/xtensa-semi.c b/target/xtensa/xtensa-semi.c
> index 5f5ce4f344c..38efa3485af 100644
> --- a/target/xtensa/xtensa-semi.c
> +++ b/target/xtensa/xtensa-semi.c
> @@ -29,7 +29,7 @@
> #include "cpu.h"
> #include "chardev/char-fe.h"
> #include "exec/helper-proto.h"
> -#include "exec/semihost.h"
> +#include "hw/semihosting/semihost.h"
> #include "qapi/error.h"
> #include "qemu/log.h"
> #include "sysemu/sysemu.h"
> diff --git a/vl.c b/vl.c
> index b6709514c1b..fba48b565c9 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -116,7 +116,7 @@ int main(int argc, char **argv)
> #include "qapi/opts-visitor.h"
> #include "qapi/clone-visitor.h"
> #include "qom/object_interfaces.h"
> -#include "exec/semihost.h"
> +#include "hw/semihosting/semihost.h"
> #include "crypto/init.h"
> #include "sysemu/replay.h"
> #include "qapi/qapi-events-run-state.h"
> @@ -500,25 +500,6 @@ 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,
> - }, {
> - .name = "arg",
> - .type = QEMU_OPT_STRING,
> - },
> - { /* end of list */ }
> - },
> -};
> -
> static QemuOptsList qemu_fw_cfg_opts = {
> .name = "fw_cfg",
> .implied_opt_name = "name",
> @@ -1350,80 +1331,6 @@ static void configure_msg(QemuOpts *opts)
> enable_timestamp_msg = qemu_opt_get_bool(opts, "timestamp", true);
> }
>
> -/***********************************************************/
> -/* Semihosting */
> -
> -typedef struct SemihostingConfig {
> - bool enabled;
> - SemihostingTarget target;
> - const char **argv;
> - int argc;
> - const char *cmdline; /* concatenated argv */
> -} SemihostingConfig;
> -
> -static SemihostingConfig semihosting;
> -
> -bool semihosting_enabled(void)
> -{
> - return semihosting.enabled;
> -}
> -
> -SemihostingTarget semihosting_get_target(void)
> -{
> - return semihosting.target;
> -}
> -
> -const char *semihosting_get_arg(int i)
> -{
> - if (i >= semihosting.argc) {
> - return NULL;
> - }
> - return semihosting.argv[i];
> -}
> -
> -int semihosting_get_argc(void)
> -{
> - return semihosting.argc;
> -}
> -
> -const char *semihosting_get_cmdline(void)
> -{
> - if (semihosting.cmdline == NULL && semihosting.argc > 0) {
> - semihosting.cmdline = g_strjoinv(" ", (gchar **)semihosting.argv);
> - }
> - return semihosting.cmdline;
> -}
> -
> -static int add_semihosting_arg(void *opaque,
> - const char *name, const char *val,
> - Error **errp)
> -{
> - SemihostingConfig *s = opaque;
> - if (strcmp(name, "arg") == 0) {
> - s->argc++;
> - /* one extra element as g_strjoinv() expects NULL-terminated array */
> - s->argv = g_realloc(s->argv, (s->argc + 1) * sizeof(void *));
> - s->argv[s->argc - 1] = val;
> - s->argv[s->argc] = NULL;
> - }
> - return 0;
> -}
> -
> -/* Use strings passed via -kernel/-append to initialize semihosting.argv[] */
> -static inline void semihosting_arg_fallback(const char *file, const char *cmd)
> -{
> - char *cmd_token;
> -
> - /* argv[0] */
> - add_semihosting_arg(&semihosting, "arg", file, NULL);
> -
> - /* split -append and initialize argv[1..n] */
> - cmd_token = strtok(g_strdup(cmd), " ");
> - while (cmd_token) {
> - add_semihosting_arg(&semihosting, "arg", cmd_token, NULL);
> - cmd_token = strtok(NULL, " ");
> - }
> -}
>
> /* Now we still need this for compatibility with XEN. */
> bool has_igd_gfx_passthru;
> @@ -3733,37 +3640,10 @@ int main(int argc, char **argv, char **envp)
> nb_option_roms++;
> break;
> case QEMU_OPTION_semihosting:
> - semihosting.enabled = true;
> - semihosting.target = SEMIHOSTING_TARGET_AUTO;
> + qemu_semihosting_enable();
> break;
> case QEMU_OPTION_semihosting_config:
> - semihosting.enabled = true;
> - opts = qemu_opts_parse_noisily(qemu_find_opts("semihosting-config"),
> - optarg, false);
> - 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 {
> - error_report("unsupported semihosting-config %s",
> - optarg);
> - exit(1);
> - }
> - } else {
> - semihosting.target = SEMIHOSTING_TARGET_AUTO;
> - }
> - /* Set semihosting argument count and vector */
> - qemu_opt_foreach(opts, add_semihosting_arg,
> - &semihosting, NULL);
> - } else {
> - error_report("unsupported semihosting-config %s", optarg);
> + if (qemu_semihosting_config_options(optarg) != 0) {
> exit(1);
> }
> break;
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [RFC PATCH 02/11] semihosting: introduce CONFIG_SEMIHOSTING
2019-05-14 15:52 [Qemu-devel] [RFC PATCH 00/11] semihosting cleanup and re-factor Alex Bennée
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 01/11] semihosting: move semihosting configuration into its own directory Alex Bennée
@ 2019-05-14 15:52 ` Alex Bennée
2019-05-14 16:25 ` Philippe Mathieu-Daudé
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 03/11] semihosting: implement a semihosting console Alex Bennée
` (9 subsequent siblings)
11 siblings, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2019-05-14 15:52 UTC (permalink / raw)
To: qemu-devel
Cc: Marek Vasut, Aleksandar Rikalo, Chris Wulff, Max Filippov,
qemu-arm, Aleksandar Markovic, Paolo Bonzini, Alex Bennée,
Aurelien Jarno
There isn't much point building semihosting for platforms that don't
support it. Introduce a new symbol and enable it only for the softmmu
targets that need it.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
default-configs/arm-softmmu.mak | 1 +
default-configs/lm32-softmmu.mak | 2 +
default-configs/m68k-softmmu.mak | 2 +
default-configs/mips-softmmu-common.mak | 1 +
default-configs/nios2-softmmu.mak | 2 +
default-configs/xtensa-softmmu.mak | 2 +
hw/Kconfig | 1 +
hw/semihosting/Kconfig | 3 ++
hw/semihosting/Makefile.objs | 2 +-
stubs/Makefile.objs | 1 +
stubs/semihost.c | 66 +++++++++++++++++++++++++
11 files changed, 82 insertions(+), 1 deletion(-)
create mode 100644 hw/semihosting/Kconfig
create mode 100644 stubs/semihost.c
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index f23ecfd5c58..1f2e0e7fdeb 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -39,3 +39,4 @@ CONFIG_MICROBIT=y
CONFIG_FSL_IMX25=y
CONFIG_FSL_IMX7=y
CONFIG_FSL_IMX6UL=y
+CONFIG_SEMIHOSTING=y
diff --git a/default-configs/lm32-softmmu.mak b/default-configs/lm32-softmmu.mak
index 6d259665d60..115b3e34c98 100644
--- a/default-configs/lm32-softmmu.mak
+++ b/default-configs/lm32-softmmu.mak
@@ -4,6 +4,8 @@
#
#CONFIG_MILKYMIST_TMU2=n # disabling it actually causes compile-time failures
+CONFIG_SEMIHOSTING=y
+
# Boards:
#
CONFIG_LM32=y
diff --git a/default-configs/m68k-softmmu.mak b/default-configs/m68k-softmmu.mak
index e17495e2a0b..4049a8f2ba9 100644
--- a/default-configs/m68k-softmmu.mak
+++ b/default-configs/m68k-softmmu.mak
@@ -1,5 +1,7 @@
# Default configuration for m68k-softmmu
+CONFIG_SEMIHOSTING=y
+
# Boards:
#
CONFIG_AN5206=y
diff --git a/default-configs/mips-softmmu-common.mak b/default-configs/mips-softmmu-common.mak
index 8e54a74b7a5..e10ac4b20c0 100644
--- a/default-configs/mips-softmmu-common.mak
+++ b/default-configs/mips-softmmu-common.mak
@@ -35,6 +35,7 @@ CONFIG_MIPS_CPS=y
CONFIG_MIPS_ITU=y
CONFIG_R4K=y
CONFIG_MALTA=y
+CONFIG_SEMIHOSTING=y
CONFIG_PCNET_PCI=y
CONFIG_MIPSSIM=y
CONFIG_ACPI_SMBUS=y
diff --git a/default-configs/nios2-softmmu.mak b/default-configs/nios2-softmmu.mak
index e130d024e62..1bc4082ea99 100644
--- a/default-configs/nios2-softmmu.mak
+++ b/default-configs/nios2-softmmu.mak
@@ -1,5 +1,7 @@
# Default configuration for nios2-softmmu
+CONFIG_SEMIHOSTING=y
+
# Boards:
#
CONFIG_NIOS2_10M50=y
diff --git a/default-configs/xtensa-softmmu.mak b/default-configs/xtensa-softmmu.mak
index 7e4d1cc0979..3aa20a47a7f 100644
--- a/default-configs/xtensa-softmmu.mak
+++ b/default-configs/xtensa-softmmu.mak
@@ -1,5 +1,7 @@
# Default configuration for Xtensa
+CONFIG_SEMIHOSTING=y
+
# Boards:
#
CONFIG_XTENSA_SIM=y
diff --git a/hw/Kconfig b/hw/Kconfig
index 88b9f150070..195f541e50c 100644
--- a/hw/Kconfig
+++ b/hw/Kconfig
@@ -29,6 +29,7 @@ source pci/Kconfig
source rdma/Kconfig
source scsi/Kconfig
source sd/Kconfig
+source semihosting/Kconfig
source smbios/Kconfig
source ssi/Kconfig
source timer/Kconfig
diff --git a/hw/semihosting/Kconfig b/hw/semihosting/Kconfig
new file mode 100644
index 00000000000..efe0a30734c
--- /dev/null
+++ b/hw/semihosting/Kconfig
@@ -0,0 +1,3 @@
+
+config SEMIHOSTING
+ bool
diff --git a/hw/semihosting/Makefile.objs b/hw/semihosting/Makefile.objs
index 546954f4ff4..09c19bf19ed 100644
--- a/hw/semihosting/Makefile.objs
+++ b/hw/semihosting/Makefile.objs
@@ -1 +1 @@
-common-obj-$(CONFIG_SOFTMMU) += config.o
+obj-$(CONFIG_SEMIHOSTING) += config.o
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 269dfa58326..e0d4f11b348 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -39,3 +39,4 @@ stub-obj-y += xen-hvm.o
stub-obj-y += pci-host-piix.o
stub-obj-y += ram-block.o
stub-obj-y += ramfb.o
+stub-obj-$(CONFIG_SOFTMMU) += semihost.o
diff --git a/stubs/semihost.c b/stubs/semihost.c
new file mode 100644
index 00000000000..1a4e88e5326
--- /dev/null
+++ b/stubs/semihost.c
@@ -0,0 +1,66 @@
+/*
+ * Semihosting Stubs for SoftMMU
+ *
+ * Copyright (c) 2019 Linaro Ltd
+ *
+ * Stubs for SoftMMU targets that don't actually do semihosting.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/option.h"
+#include "qemu/error-report.h"
+#include "hw/semihosting/semihost.h"
+
+/* Empty config */
+QemuOptsList qemu_semihosting_config_opts = {
+ .name = "",
+ .head = QTAILQ_HEAD_INITIALIZER(qemu_semihosting_config_opts.head),
+ .desc = {
+ { /* end of list */ }
+ },
+};
+
+/* Queries to config status default to off */
+bool semihosting_enabled(void)
+{
+ return false;
+}
+
+SemihostingTarget semihosting_get_target(void)
+{
+ return SEMIHOSTING_TARGET_AUTO;
+}
+
+/*
+ * All the rest are empty subs. We could g_assert_not_reached() but
+ * that adds extra weight to the final binary. Waste not want not.
+ */
+void qemu_semihosting_enable(void)
+{
+}
+
+int qemu_semihosting_config_options(const char *optarg)
+{
+ return 1;
+}
+
+const char *semihosting_get_arg(int i)
+{
+ return NULL;
+}
+
+int semihosting_get_argc(void)
+{
+ return 0;
+}
+
+const char *semihosting_get_cmdline(void)
+{
+ return NULL;
+}
+
+void semihosting_arg_fallback(const char *file, const char *cmd)
+{
+}
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 02/11] semihosting: introduce CONFIG_SEMIHOSTING
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 02/11] semihosting: introduce CONFIG_SEMIHOSTING Alex Bennée
@ 2019-05-14 16:25 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-14 16:25 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: Marek Vasut, Aleksandar Rikalo, Chris Wulff, Max Filippov,
qemu-arm, Aleksandar Markovic, Paolo Bonzini, Aurelien Jarno
On 5/14/19 5:52 PM, Alex Bennée wrote:
> There isn't much point building semihosting for platforms that don't
> support it. Introduce a new symbol and enable it only for the softmmu
> targets that need it.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> default-configs/arm-softmmu.mak | 1 +
> default-configs/lm32-softmmu.mak | 2 +
> default-configs/m68k-softmmu.mak | 2 +
> default-configs/mips-softmmu-common.mak | 1 +
> default-configs/nios2-softmmu.mak | 2 +
> default-configs/xtensa-softmmu.mak | 2 +
> hw/Kconfig | 1 +
> hw/semihosting/Kconfig | 3 ++
> hw/semihosting/Makefile.objs | 2 +-
> stubs/Makefile.objs | 1 +
> stubs/semihost.c | 66 +++++++++++++++++++++++++
> 11 files changed, 82 insertions(+), 1 deletion(-)
> create mode 100644 hw/semihosting/Kconfig
> create mode 100644 stubs/semihost.c
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index f23ecfd5c58..1f2e0e7fdeb 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -39,3 +39,4 @@ CONFIG_MICROBIT=y
> CONFIG_FSL_IMX25=y
> CONFIG_FSL_IMX7=y
> CONFIG_FSL_IMX6UL=y
> +CONFIG_SEMIHOSTING=y
> diff --git a/default-configs/lm32-softmmu.mak b/default-configs/lm32-softmmu.mak
> index 6d259665d60..115b3e34c98 100644
> --- a/default-configs/lm32-softmmu.mak
> +++ b/default-configs/lm32-softmmu.mak
> @@ -4,6 +4,8 @@
> #
> #CONFIG_MILKYMIST_TMU2=n # disabling it actually causes compile-time failures
>
> +CONFIG_SEMIHOSTING=y
> +
> # Boards:
> #
> CONFIG_LM32=y
> diff --git a/default-configs/m68k-softmmu.mak b/default-configs/m68k-softmmu.mak
> index e17495e2a0b..4049a8f2ba9 100644
> --- a/default-configs/m68k-softmmu.mak
> +++ b/default-configs/m68k-softmmu.mak
> @@ -1,5 +1,7 @@
> # Default configuration for m68k-softmmu
>
> +CONFIG_SEMIHOSTING=y
> +
> # Boards:
> #
> CONFIG_AN5206=y
> diff --git a/default-configs/mips-softmmu-common.mak b/default-configs/mips-softmmu-common.mak
> index 8e54a74b7a5..e10ac4b20c0 100644
> --- a/default-configs/mips-softmmu-common.mak
> +++ b/default-configs/mips-softmmu-common.mak
> @@ -35,6 +35,7 @@ CONFIG_MIPS_CPS=y
> CONFIG_MIPS_ITU=y
> CONFIG_R4K=y
> CONFIG_MALTA=y
> +CONFIG_SEMIHOSTING=y
> CONFIG_PCNET_PCI=y
> CONFIG_MIPSSIM=y
> CONFIG_ACPI_SMBUS=y
> diff --git a/default-configs/nios2-softmmu.mak b/default-configs/nios2-softmmu.mak
> index e130d024e62..1bc4082ea99 100644
> --- a/default-configs/nios2-softmmu.mak
> +++ b/default-configs/nios2-softmmu.mak
> @@ -1,5 +1,7 @@
> # Default configuration for nios2-softmmu
>
> +CONFIG_SEMIHOSTING=y
> +
> # Boards:
> #
> CONFIG_NIOS2_10M50=y
> diff --git a/default-configs/xtensa-softmmu.mak b/default-configs/xtensa-softmmu.mak
> index 7e4d1cc0979..3aa20a47a7f 100644
> --- a/default-configs/xtensa-softmmu.mak
> +++ b/default-configs/xtensa-softmmu.mak
> @@ -1,5 +1,7 @@
> # Default configuration for Xtensa
>
> +CONFIG_SEMIHOSTING=y
> +
> # Boards:
> #
> CONFIG_XTENSA_SIM=y
> diff --git a/hw/Kconfig b/hw/Kconfig
> index 88b9f150070..195f541e50c 100644
> --- a/hw/Kconfig
> +++ b/hw/Kconfig
> @@ -29,6 +29,7 @@ source pci/Kconfig
> source rdma/Kconfig
> source scsi/Kconfig
> source sd/Kconfig
> +source semihosting/Kconfig
> source smbios/Kconfig
> source ssi/Kconfig
> source timer/Kconfig
> diff --git a/hw/semihosting/Kconfig b/hw/semihosting/Kconfig
> new file mode 100644
> index 00000000000..efe0a30734c
> --- /dev/null
> +++ b/hw/semihosting/Kconfig
> @@ -0,0 +1,3 @@
> +
> +config SEMIHOSTING
> + bool
> diff --git a/hw/semihosting/Makefile.objs b/hw/semihosting/Makefile.objs
> index 546954f4ff4..09c19bf19ed 100644
> --- a/hw/semihosting/Makefile.objs
> +++ b/hw/semihosting/Makefile.objs
> @@ -1 +1 @@
> -common-obj-$(CONFIG_SOFTMMU) += config.o
> +obj-$(CONFIG_SEMIHOSTING) += config.o
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index 269dfa58326..e0d4f11b348 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -39,3 +39,4 @@ stub-obj-y += xen-hvm.o
> stub-obj-y += pci-host-piix.o
> stub-obj-y += ram-block.o
> stub-obj-y += ramfb.o
> +stub-obj-$(CONFIG_SOFTMMU) += semihost.o
> diff --git a/stubs/semihost.c b/stubs/semihost.c
> new file mode 100644
> index 00000000000..1a4e88e5326
> --- /dev/null
> +++ b/stubs/semihost.c
> @@ -0,0 +1,66 @@
> +/*
> + * Semihosting Stubs for SoftMMU
> + *
> + * Copyright (c) 2019 Linaro Ltd
> + *
> + * Stubs for SoftMMU targets that don't actually do semihosting.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/option.h"
> +#include "qemu/error-report.h"
> +#include "hw/semihosting/semihost.h"
> +
> +/* Empty config */
> +QemuOptsList qemu_semihosting_config_opts = {
> + .name = "",
> + .head = QTAILQ_HEAD_INITIALIZER(qemu_semihosting_config_opts.head),
> + .desc = {
> + { /* end of list */ }
> + },
> +};
> +
> +/* Queries to config status default to off */
> +bool semihosting_enabled(void)
> +{
> + return false;
> +}
> +
> +SemihostingTarget semihosting_get_target(void)
> +{
> + return SEMIHOSTING_TARGET_AUTO;
> +}
> +
> +/*
> + * All the rest are empty subs. We could g_assert_not_reached() but
> + * that adds extra weight to the final binary. Waste not want not.
> + */
> +void qemu_semihosting_enable(void)
> +{
> +}
> +
> +int qemu_semihosting_config_options(const char *optarg)
> +{
> + return 1;
> +}
> +
> +const char *semihosting_get_arg(int i)
> +{
> + return NULL;
> +}
> +
> +int semihosting_get_argc(void)
> +{
> + return 0;
> +}
> +
> +const char *semihosting_get_cmdline(void)
> +{
> + return NULL;
> +}
> +
> +void semihosting_arg_fallback(const char *file, const char *cmd)
> +{
> +}
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [RFC PATCH 03/11] semihosting: implement a semihosting console
2019-05-14 15:52 [Qemu-devel] [RFC PATCH 00/11] semihosting cleanup and re-factor Alex Bennée
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 01/11] semihosting: move semihosting configuration into its own directory Alex Bennée
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 02/11] semihosting: introduce CONFIG_SEMIHOSTING Alex Bennée
@ 2019-05-14 15:52 ` Alex Bennée
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 04/11] semihosting: enable chardev backed output for console Alex Bennée
` (8 subsequent siblings)
11 siblings, 0 replies; 34+ messages in thread
From: Alex Bennée @ 2019-05-14 15:52 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Alex Bennée
This provides two functions for handling console output that handle
the common backend behaviour for semihosting.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
gdbstub.c | 5 +++
hw/semihosting/Makefile.objs | 1 +
hw/semihosting/console.c | 70 ++++++++++++++++++++++++++++++++
include/exec/gdbstub.h | 11 +++++
include/hw/semihosting/console.h | 38 +++++++++++++++++
5 files changed, 125 insertions(+)
create mode 100644 hw/semihosting/console.c
create mode 100644 include/hw/semihosting/console.h
diff --git a/gdbstub.c b/gdbstub.c
index 793218bb43a..b4334014373 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1987,6 +1987,11 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...)
va_end(va);
}
+void gdb_do_console_out(target_ulong s, int len)
+{
+ gdb_do_syscall(NULL, "write,2,%x,1", s, len);
+}
+
static void gdb_read_byte(GDBState *s, int ch)
{
uint8_t reply;
diff --git a/hw/semihosting/Makefile.objs b/hw/semihosting/Makefile.objs
index 09c19bf19ed..4ad47c05c06 100644
--- a/hw/semihosting/Makefile.objs
+++ b/hw/semihosting/Makefile.objs
@@ -1 +1,2 @@
obj-$(CONFIG_SEMIHOSTING) += config.o
+obj-$(CONFIG_SEMIHOSTING) += console.o
diff --git a/hw/semihosting/console.c b/hw/semihosting/console.c
new file mode 100644
index 00000000000..ad6f67ecc71
--- /dev/null
+++ b/hw/semihosting/console.c
@@ -0,0 +1,70 @@
+/*
+ * Semihosting Console Support
+ *
+ * Copyright (c) 2015 Imagination Technologies
+ * Copyright (c) 2019 Linaro Ltd
+ *
+ * This provides support for outputting to a semihosting console.
+ *
+ * While most semihosting implementations support reading and writing
+ * to arbitrary file descriptors we treat the console as something
+ * specifically for debugging interaction. This means messages can be
+ * re-directed to gdb (if currently being used to debug) or even
+ * re-directed elsewhere.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "hw/semihosting/console.h"
+#include "exec/gdbstub.h"
+#include "qemu/log.h"
+
+int qemu_semihosting_log_out(const char *s, int len)
+{
+ return write(STDERR_FILENO, s, len);
+}
+
+/*
+ * A re-implementation of lock_user_string that we can use locally
+ * instead of relying on softmmu-semi. Hopefully we can deprecate that
+ * in time. We either copy len bytes if specified or until we find a NULL.
+ */
+static GString *copy_user_string(CPUArchState *env, target_ulong addr, int len)
+{
+ CPUState *cpu = ENV_GET_CPU(env);
+ GString *s = g_string_sized_new(len ? len : 128);
+ uint8_t c;
+ bool done;
+
+ do {
+ if (cpu_memory_rw_debug(cpu, addr++, &c, 1, 0) == 0) {
+ s = g_string_append_c(s, c);
+ done = len ? s->len == len : c == 0;
+ } else {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: passed inaccessible address " TARGET_FMT_lx,
+ __func__, addr);
+ done = true;
+ }
+ } while (!done);
+
+ return s;
+}
+
+
+int qemu_semihosting_console_out(CPUArchState *env, target_ulong addr, int len)
+{
+ GString *s = copy_user_string(env, addr, len);
+ int out = s->len;
+
+ if (use_gdb_syscalls()) {
+ gdb_do_console_out(addr, s->len);
+ } else {
+ out = qemu_semihosting_log_out(s->str, s->len);
+ }
+
+ g_string_free(s, true);
+ return out;
+}
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 08363969c14..b2963547c48 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -44,6 +44,17 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...);
* argument list.
*/
void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va);
+/**
+ * gdb_do_console_out:
+ * @gs: guest address of string to send
+ * @len: length of string
+ *
+ * Sends a string to gdb console. Unlike the system call interface
+ * there is no callback and we assume the system call always
+ * succeeds.
+ */
+void gdb_do_console_out(target_ulong s, int len);
+
int use_gdb_syscalls(void);
void gdb_set_stop_cpu(CPUState *cpu);
void gdb_exit(CPUArchState *, int);
diff --git a/include/hw/semihosting/console.h b/include/hw/semihosting/console.h
new file mode 100644
index 00000000000..30e66ae20aa
--- /dev/null
+++ b/include/hw/semihosting/console.h
@@ -0,0 +1,38 @@
+/*
+ * Semihosting Console
+ *
+ * Copyright (c) 2019 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef _SEMIHOST_CONSOLE_H_
+#define _SEMIHOST_CONSOLE_H_
+
+/**
+ * qemu_semihosting_console_out:
+ * @env: CPUArchState
+ * @s: host address of guest string
+ * @len: length of string or 0 (string is null terminated)
+ *
+ * Send a guest string to the debug console. This may be the remote
+ * gdb session if a softmmu guest is currently being debugged.
+ *
+ * Returns: number of bytes written.
+ */
+int qemu_semihosting_console_out(CPUArchState *env, target_ulong s, int len);
+
+/**
+ * qemu_semihosting_log_out:
+ * @s: pointer to string
+ * @len: length of string
+ *
+ * Send a string to the debug output. Unlike console_out these strings
+ * can't be sent to a remote gdb instance as they don't exist in guest
+ * memory.
+ *
+ * Returns: number of bytes written
+ */
+int qemu_semihosting_log_out(const char *s, int len);
+
+#endif /* _SEMIHOST_CONSOLE_H_ */
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [RFC PATCH 04/11] semihosting: enable chardev backed output for console
2019-05-14 15:52 [Qemu-devel] [RFC PATCH 00/11] semihosting cleanup and re-factor Alex Bennée
` (2 preceding siblings ...)
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 03/11] semihosting: implement a semihosting console Alex Bennée
@ 2019-05-14 15:52 ` Alex Bennée
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 05/11] target/arm: fixup some of the commentary for arm-semi Alex Bennée
` (7 subsequent siblings)
11 siblings, 0 replies; 34+ messages in thread
From: Alex Bennée @ 2019-05-14 15:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, qemu-arm, Alex Bennée
It will be useful for a number of use-cases to be able to re-direct
output to a file like we do with serial output. This does the wiring
to allow us to treat then semihosting console like just another
character output device.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
hw/semihosting/config.c | 26 ++++++++++++++++++++++++++
hw/semihosting/console.c | 9 ++++++++-
include/hw/semihosting/semihost.h | 7 +++++++
qemu-options.hx | 6 ++++--
stubs/semihost.c | 4 ++++
vl.c | 2 ++
6 files changed, 51 insertions(+), 3 deletions(-)
diff --git a/hw/semihosting/config.c b/hw/semihosting/config.c
index f1d3fe1e4c6..2a8e7e1045a 100644
--- a/hw/semihosting/config.c
+++ b/hw/semihosting/config.c
@@ -23,6 +23,7 @@
#include "qemu/config-file.h"
#include "qemu/error-report.h"
#include "hw/semihosting/semihost.h"
+#include "chardev/char.h"
QemuOptsList qemu_semihosting_config_opts = {
.name = "semihosting-config",
@@ -35,6 +36,9 @@ QemuOptsList qemu_semihosting_config_opts = {
}, {
.name = "target",
.type = QEMU_OPT_STRING,
+ }, {
+ .name = "chardev",
+ .type = QEMU_OPT_STRING,
}, {
.name = "arg",
.type = QEMU_OPT_STRING,
@@ -46,12 +50,14 @@ QemuOptsList qemu_semihosting_config_opts = {
typedef struct SemihostingConfig {
bool enabled;
SemihostingTarget target;
+ Chardev *chardev;
const char **argv;
int argc;
const char *cmdline; /* concatenated argv */
} SemihostingConfig;
static SemihostingConfig semihosting;
+static const char *semihost_chardev;
bool semihosting_enabled(void)
{
@@ -115,6 +121,11 @@ void semihosting_arg_fallback(const char *file, const char *cmd)
}
}
+Chardev *semihosting_get_chardev(void)
+{
+ return semihosting.chardev;
+}
+
void qemu_semihosting_enable(void)
{
semihosting.enabled = true;
@@ -132,6 +143,8 @@ int qemu_semihosting_config_options(const char *optarg)
semihosting.enabled = qemu_opt_get_bool(opts, "enable",
true);
const char *target = qemu_opt_get(opts, "target");
+ /* setup of chardev is deferred until they are initialised */
+ semihost_chardev = qemu_opt_get(opts, "chardev");
if (target != NULL) {
if (strcmp("native", target) == 0) {
semihosting.target = SEMIHOSTING_TARGET_NATIVE;
@@ -158,3 +171,16 @@ int qemu_semihosting_config_options(const char *optarg)
return 0;
}
+void qemu_semihosting_connect_chardevs(void)
+{
+ /* We had to defer this until chardevs were created */
+ if (semihost_chardev) {
+ Chardev *chr = qemu_chr_find(semihost_chardev);
+ if (chr == NULL) {
+ error_report("semihosting chardev '%s' not found",
+ semihost_chardev);
+ exit(1);
+ }
+ semihosting.chardev = chr;
+ }
+}
diff --git a/hw/semihosting/console.c b/hw/semihosting/console.c
index ad6f67ecc71..2fa440eb32a 100644
--- a/hw/semihosting/console.c
+++ b/hw/semihosting/console.c
@@ -17,13 +17,20 @@
#include "qemu/osdep.h"
#include "cpu.h"
+#include "hw/semihosting/semihost.h"
#include "hw/semihosting/console.h"
#include "exec/gdbstub.h"
#include "qemu/log.h"
+#include "chardev/char.h"
int qemu_semihosting_log_out(const char *s, int len)
{
- return write(STDERR_FILENO, s, len);
+ Chardev *chardev = semihosting_get_chardev();
+ if (chardev) {
+ return qemu_chr_write_all(chardev, (uint8_t *) s, len);
+ } else {
+ return write(STDERR_FILENO, s, len);
+ }
}
/*
diff --git a/include/hw/semihosting/semihost.h b/include/hw/semihosting/semihost.h
index 07ea40a322f..60fc42d851e 100644
--- a/include/hw/semihosting/semihost.h
+++ b/include/hw/semihosting/semihost.h
@@ -51,6 +51,11 @@ static inline const char *semihosting_get_cmdline(void)
{
return NULL;
}
+
+static inline Chardev *semihosting_get_chardev(void)
+{
+ return NULL;
+}
#else /* !CONFIG_USER_ONLY */
bool semihosting_enabled(void);
SemihostingTarget semihosting_get_target(void);
@@ -58,9 +63,11 @@ const char *semihosting_get_arg(int i);
int semihosting_get_argc(void);
const char *semihosting_get_cmdline(void);
void semihosting_arg_fallback(const char *file, const char *cmd);
+Chardev *semihosting_get_chardev(void);
/* for vl.c hooks */
void qemu_semihosting_enable(void);
int qemu_semihosting_config_options(const char *opt);
+void qemu_semihosting_connect_chardevs(void);
#endif /* CONFIG_USER_ONLY */
#endif /* SEMIHOST_H */
diff --git a/qemu-options.hx b/qemu-options.hx
index 51802cbb266..6aa3a08c2fb 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3975,12 +3975,12 @@ STEXI
Enable semihosting mode (ARM, M68K, Xtensa, MIPS, Nios II only).
ETEXI
DEF("semihosting-config", HAS_ARG, QEMU_OPTION_semihosting_config,
- "-semihosting-config [enable=on|off][,target=native|gdb|auto][,arg=str[,...]]\n" \
+ "-semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]\n" \
" semihosting configuration\n",
QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_LM32 |
QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2)
STEXI
-@item -semihosting-config [enable=on|off][,target=native|gdb|auto][,arg=str[,...]]
+@item -semihosting-config [enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]
@findex -semihosting-config
Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II only).
@table @option
@@ -3988,6 +3988,8 @@ Enable and configure semihosting (ARM, M68K, Xtensa, MIPS, Nios II only).
Defines 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.
+@item chardev=@var{str1}
+Send the output to a chardev backend output for native or auto output when not in gdb
@item arg=@var{str1},arg=@var{str2},...
Allows the user to pass input arguments, and can be used multiple times to build
up a list. The old-style @code{-kernel}/@code{-append} method of passing a
diff --git a/stubs/semihost.c b/stubs/semihost.c
index 1a4e88e5326..4d5b3c06530 100644
--- a/stubs/semihost.c
+++ b/stubs/semihost.c
@@ -64,3 +64,7 @@ const char *semihosting_get_cmdline(void)
void semihosting_arg_fallback(const char *file, const char *cmd)
{
}
+
+void qemu_semihosting_connect_chardevs(void)
+{
+}
diff --git a/vl.c b/vl.c
index fba48b565c9..49878a2a1bf 100644
--- a/vl.c
+++ b/vl.c
@@ -4156,6 +4156,8 @@ int main(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"),
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [RFC PATCH 05/11] target/arm: fixup some of the commentary for arm-semi
2019-05-14 15:52 [Qemu-devel] [RFC PATCH 00/11] semihosting cleanup and re-factor Alex Bennée
` (3 preceding siblings ...)
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 04/11] semihosting: enable chardev backed output for console Alex Bennée
@ 2019-05-14 15:52 ` Alex Bennée
2019-05-14 16:56 ` Philippe Mathieu-Daudé
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi Alex Bennée
` (6 subsequent siblings)
11 siblings, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2019-05-14 15:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, qemu-arm, Alex Bennée
This cleans up a number of the block comments to fit the proper style.
While we are at it we also reference the official specification and
document what the return register value can be.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
target/arm/arm-semi.c | 40 +++++++++++++++++++++++++++++++---------
1 file changed, 31 insertions(+), 9 deletions(-)
diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
index 3273306c8ea..9e5a414dd89 100644
--- a/target/arm/arm-semi.c
+++ b/target/arm/arm-semi.c
@@ -2,6 +2,7 @@
* Arm "Angel" semihosting syscalls
*
* Copyright (c) 2005, 2007 CodeSourcery.
+ * Copyright (c) 2019 Linaro
* Written by Paul Brook.
*
* This program is free software; you can redistribute it and/or modify
@@ -16,6 +17,10 @@
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ * ARM Semihosting is documented in:
+ * Semihosting for AArch32 and AArch64 Release 2.0
+ * https://static.docs.arm.com/100863/0200/semihosting.pdf
*/
#include "qemu/osdep.h"
@@ -240,6 +245,15 @@ static target_ulong arm_gdb_syscall(ARMCPU *cpu, gdb_syscall_complete_cb cb,
put_user_u64(val, args + (n) * 8) : \
put_user_u32(val, args + (n) * 4))
+/*
+ * Do a semihosting call.
+ *
+ * The specification always says that the "return register" either
+ * returns a specific value or is corrupted, so we don't need to
+ * report to our caller whether we are returning a value or trying to
+ * leave the register unchanged. We use 0xdeadbeef as the return value
+ * when there isn't a defined return value for the call.
+ */
target_ulong do_arm_semihosting(CPUARMState *env)
{
ARMCPU *cpu = arm_env_get_cpu(env);
@@ -510,14 +524,16 @@ target_ulong do_arm_semihosting(CPUARMState *env)
output_size = ts->info->arg_end - ts->info->arg_start;
if (!output_size) {
- /* We special-case the "empty command line" case (argc==0).
- Just provide the terminating 0. */
+ /*
+ * We special-case the "empty command line" case (argc==0).
+ * Just provide the terminating 0.
+ */
output_size = 1;
}
#endif
if (output_size > input_size) {
- /* Not enough space to store command-line arguments. */
+ /* Not enough space to store command-line arguments. */
return -1;
}
@@ -571,8 +587,10 @@ target_ulong do_arm_semihosting(CPUARMState *env)
GET_ARG(0);
#ifdef CONFIG_USER_ONLY
- /* Some C libraries assume the heap immediately follows .bss, so
- allocate it using sbrk. */
+ /*
+ * Some C libraries assume the heap immediately follows .bss, so
+ * allocate it using sbrk.
+ */
if (!ts->heap_limit) {
abi_ulong ret;
@@ -620,7 +638,8 @@ target_ulong do_arm_semihosting(CPUARMState *env)
}
case TARGET_SYS_EXIT:
if (is_a64(env)) {
- /* The A64 version of this call takes a parameter block,
+ /*
+ * The A64 version of this call takes a parameter block,
* so the application-exit type can return a subcode which
* is the exit status code from the application.
*/
@@ -633,14 +652,17 @@ target_ulong do_arm_semihosting(CPUARMState *env)
ret = 1;
}
} else {
- /* ARM specifies only Stopped_ApplicationExit as normal
- * exit, everything else is considered an error */
+ /*
+ * ARM specifies only Stopped_ApplicationExit as normal
+ * exit, everything else is considered an error
+ */
ret = (args == ADP_Stopped_ApplicationExit) ? 0 : 1;
}
gdb_exit(env, ret);
exit(ret);
case TARGET_SYS_SYNCCACHE:
- /* Clean the D-cache and invalidate the I-cache for the specified
+ /*
+ * Clean the D-cache and invalidate the I-cache for the specified
* virtual address range. This is a nop for us since we don't
* implement caches. This is only present on A64.
*/
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 05/11] target/arm: fixup some of the commentary for arm-semi
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 05/11] target/arm: fixup some of the commentary for arm-semi Alex Bennée
@ 2019-05-14 16:56 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-14 16:56 UTC (permalink / raw)
To: Alex Bennée, qemu-devel; +Cc: Peter Maydell, qemu-arm
On 5/14/19 5:52 PM, Alex Bennée wrote:
> This cleans up a number of the block comments to fit the proper style.
> While we are at it we also reference the official specification and
> document what the return register value can be.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> target/arm/arm-semi.c | 40 +++++++++++++++++++++++++++++++---------
> 1 file changed, 31 insertions(+), 9 deletions(-)
>
> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
> index 3273306c8ea..9e5a414dd89 100644
> --- a/target/arm/arm-semi.c
> +++ b/target/arm/arm-semi.c
> @@ -2,6 +2,7 @@
> * Arm "Angel" semihosting syscalls
> *
> * Copyright (c) 2005, 2007 CodeSourcery.
> + * Copyright (c) 2019 Linaro
> * Written by Paul Brook.
> *
> * This program is free software; you can redistribute it and/or modify
> @@ -16,6 +17,10 @@
> *
> * You should have received a copy of the GNU General Public License
> * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + *
> + * ARM Semihosting is documented in:
> + * Semihosting for AArch32 and AArch64 Release 2.0
> + * https://static.docs.arm.com/100863/0200/semihosting.pdf
> */
>
> #include "qemu/osdep.h"
> @@ -240,6 +245,15 @@ static target_ulong arm_gdb_syscall(ARMCPU *cpu, gdb_syscall_complete_cb cb,
> put_user_u64(val, args + (n) * 8) : \
> put_user_u32(val, args + (n) * 4))
>
> +/*
> + * Do a semihosting call.
> + *
> + * The specification always says that the "return register" either
> + * returns a specific value or is corrupted, so we don't need to
> + * report to our caller whether we are returning a value or trying to
> + * leave the register unchanged. We use 0xdeadbeef as the return value
> + * when there isn't a defined return value for the call.
> + */
> target_ulong do_arm_semihosting(CPUARMState *env)
> {
> ARMCPU *cpu = arm_env_get_cpu(env);
> @@ -510,14 +524,16 @@ target_ulong do_arm_semihosting(CPUARMState *env)
>
> output_size = ts->info->arg_end - ts->info->arg_start;
> if (!output_size) {
> - /* We special-case the "empty command line" case (argc==0).
> - Just provide the terminating 0. */
> + /*
> + * We special-case the "empty command line" case (argc==0).
> + * Just provide the terminating 0.
> + */
> output_size = 1;
> }
> #endif
>
> if (output_size > input_size) {
> - /* Not enough space to store command-line arguments. */
> + /* Not enough space to store command-line arguments. */
> return -1;
> }
>
> @@ -571,8 +587,10 @@ target_ulong do_arm_semihosting(CPUARMState *env)
> GET_ARG(0);
>
> #ifdef CONFIG_USER_ONLY
> - /* Some C libraries assume the heap immediately follows .bss, so
> - allocate it using sbrk. */
> + /*
> + * Some C libraries assume the heap immediately follows .bss, so
> + * allocate it using sbrk.
> + */
> if (!ts->heap_limit) {
> abi_ulong ret;
>
> @@ -620,7 +638,8 @@ target_ulong do_arm_semihosting(CPUARMState *env)
> }
> case TARGET_SYS_EXIT:
> if (is_a64(env)) {
> - /* The A64 version of this call takes a parameter block,
> + /*
> + * The A64 version of this call takes a parameter block,
> * so the application-exit type can return a subcode which
> * is the exit status code from the application.
> */
> @@ -633,14 +652,17 @@ target_ulong do_arm_semihosting(CPUARMState *env)
> ret = 1;
> }
> } else {
> - /* ARM specifies only Stopped_ApplicationExit as normal
> - * exit, everything else is considered an error */
> + /*
> + * ARM specifies only Stopped_ApplicationExit as normal
> + * exit, everything else is considered an error
> + */
> ret = (args == ADP_Stopped_ApplicationExit) ? 0 : 1;
> }
> gdb_exit(env, ret);
> exit(ret);
> case TARGET_SYS_SYNCCACHE:
> - /* Clean the D-cache and invalidate the I-cache for the specified
> + /*
> + * Clean the D-cache and invalidate the I-cache for the specified
> * virtual address range. This is a nop for us since we don't
> * implement caches. This is only present on A64.
> */
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
2019-05-14 15:52 [Qemu-devel] [RFC PATCH 00/11] semihosting cleanup and re-factor Alex Bennée
` (4 preceding siblings ...)
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 05/11] target/arm: fixup some of the commentary for arm-semi Alex Bennée
@ 2019-05-14 15:52 ` Alex Bennée
2019-05-31 9:12 ` Miroslav Rezanina
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 07/11] target/arm: add LOG_UNIMP messages to arm-semi Alex Bennée
` (5 subsequent siblings)
11 siblings, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2019-05-14 15:52 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Riku Voipio, qemu-arm, Alex Bennée, Laurent Vivier
Now we have a common semihosting console interface use that for our
string output. However ARM is currently unique in also supporting
semihosting for linux-user so we need to replicate the API in
linux-user. If other architectures gain this support we can move the
file later.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
linux-user/Makefile.objs | 2 ++
linux-user/arm/semihost.c | 24 ++++++++++++++++++++++++
target/arm/arm-semi.c | 31 ++++++-------------------------
3 files changed, 32 insertions(+), 25 deletions(-)
create mode 100644 linux-user/arm/semihost.c
diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
index 769b8d83362..285c5dfa17a 100644
--- a/linux-user/Makefile.objs
+++ b/linux-user/Makefile.objs
@@ -6,4 +6,6 @@ obj-y = main.o syscall.o strace.o mmap.o signal.o \
obj-$(TARGET_HAS_BFLT) += flatload.o
obj-$(TARGET_I386) += vm86.o
obj-$(TARGET_ARM) += arm/nwfpe/
+obj-$(TARGET_ARM) += arm/semihost.o
+obj-$(TARGET_AARCH64) += arm/semihost.o
obj-$(TARGET_M68K) += m68k-sim.o
diff --git a/linux-user/arm/semihost.c b/linux-user/arm/semihost.c
new file mode 100644
index 00000000000..9554102a855
--- /dev/null
+++ b/linux-user/arm/semihost.c
@@ -0,0 +1,24 @@
+/*
+ * ARM Semihosting Console Support
+ *
+ * Copyright (c) 2019 Linaro Ltd
+ *
+ * Currently ARM is unique in having support for semihosting support
+ * in linux-user. So for now we implement the common console API but
+ * just for arm linux-user.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "hw/semihosting/console.h"
+#include "qemu.h"
+
+int qemu_semihosting_console_out(CPUArchState *env, target_ulong addr, int len)
+{
+ void *s = lock_user_string(addr);
+ len = write(STDERR_FILENO, s, len ? len : strlen(s));
+ unlock_user(s, addr, 0);
+ return len;
+}
diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
index 9e5a414dd89..253c66b172a 100644
--- a/target/arm/arm-semi.c
+++ b/target/arm/arm-semi.c
@@ -27,6 +27,7 @@
#include "cpu.h"
#include "hw/semihosting/semihost.h"
+#include "hw/semihosting/console.h"
#ifdef CONFIG_USER_ONLY
#include "qemu.h"
@@ -314,32 +315,12 @@ target_ulong do_arm_semihosting(CPUARMState *env)
return set_swi_errno(ts, close(arg0));
}
case TARGET_SYS_WRITEC:
- {
- char c;
-
- if (get_user_u8(c, args))
- /* FIXME - should this error code be -TARGET_EFAULT ? */
- return (uint32_t)-1;
- /* Write to debug console. stderr is near enough. */
- if (use_gdb_syscalls()) {
- return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,1", args);
- } else {
- return write(STDERR_FILENO, &c, 1);
- }
- }
+ {
+ qemu_semihosting_console_out(env, args, 1);
+ return 0xdeadbeef;
+ }
case TARGET_SYS_WRITE0:
- if (!(s = lock_user_string(args)))
- /* FIXME - should this error code be -TARGET_EFAULT ? */
- return (uint32_t)-1;
- len = strlen(s);
- if (use_gdb_syscalls()) {
- return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,%x",
- args, len);
- } else {
- ret = write(STDERR_FILENO, s, len);
- }
- unlock_user(s, args, 0);
- return ret;
+ return qemu_semihosting_console_out(env, args, 0);
case TARGET_SYS_WRITE:
GET_ARG(0);
GET_ARG(1);
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi Alex Bennée
@ 2019-05-31 9:12 ` Miroslav Rezanina
2019-05-31 10:42 ` Philippe Mathieu-Daudé
2019-05-31 11:08 ` Alex Bennée
0 siblings, 2 replies; 34+ messages in thread
From: Miroslav Rezanina @ 2019-05-31 9:12 UTC (permalink / raw)
To: Alex Bennée
Cc: Peter Maydell, Riku Voipio, qemu-devel, qemu-arm, Laurent Vivier
On Tue, May 14, 2019 at 04:52:56PM +0100, Alex Bennée wrote:
> Now we have a common semihosting console interface use that for our
> string output. However ARM is currently unique in also supporting
> semihosting for linux-user so we need to replicate the API in
> linux-user. If other architectures gain this support we can move the
> file later.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> linux-user/Makefile.objs | 2 ++
> linux-user/arm/semihost.c | 24 ++++++++++++++++++++++++
> target/arm/arm-semi.c | 31 ++++++-------------------------
> 3 files changed, 32 insertions(+), 25 deletions(-)
> create mode 100644 linux-user/arm/semihost.c
>
> diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
> index 769b8d83362..285c5dfa17a 100644
> --- a/linux-user/Makefile.objs
> +++ b/linux-user/Makefile.objs
> @@ -6,4 +6,6 @@ obj-y = main.o syscall.o strace.o mmap.o signal.o \
> obj-$(TARGET_HAS_BFLT) += flatload.o
> obj-$(TARGET_I386) += vm86.o
> obj-$(TARGET_ARM) += arm/nwfpe/
> +obj-$(TARGET_ARM) += arm/semihost.o
> +obj-$(TARGET_AARCH64) += arm/semihost.o
> obj-$(TARGET_M68K) += m68k-sim.o
> diff --git a/linux-user/arm/semihost.c b/linux-user/arm/semihost.c
> new file mode 100644
> index 00000000000..9554102a855
> --- /dev/null
> +++ b/linux-user/arm/semihost.c
> @@ -0,0 +1,24 @@
> +/*
> + * ARM Semihosting Console Support
> + *
> + * Copyright (c) 2019 Linaro Ltd
> + *
> + * Currently ARM is unique in having support for semihosting support
> + * in linux-user. So for now we implement the common console API but
> + * just for arm linux-user.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +#include "hw/semihosting/console.h"
> +#include "qemu.h"
> +
> +int qemu_semihosting_console_out(CPUArchState *env, target_ulong addr, int len)
> +{
> + void *s = lock_user_string(addr);
> + len = write(STDERR_FILENO, s, len ? len : strlen(s));
> + unlock_user(s, addr, 0);
> + return len;
> +}
> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
> index 9e5a414dd89..253c66b172a 100644
> --- a/target/arm/arm-semi.c
> +++ b/target/arm/arm-semi.c
> @@ -27,6 +27,7 @@
>
> #include "cpu.h"
> #include "hw/semihosting/semihost.h"
> +#include "hw/semihosting/console.h"
> #ifdef CONFIG_USER_ONLY
> #include "qemu.h"
>
> @@ -314,32 +315,12 @@ target_ulong do_arm_semihosting(CPUARMState *env)
> return set_swi_errno(ts, close(arg0));
> }
> case TARGET_SYS_WRITEC:
> - {
> - char c;
> -
> - if (get_user_u8(c, args))
> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> - return (uint32_t)-1;
> - /* Write to debug console. stderr is near enough. */
> - if (use_gdb_syscalls()) {
> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,1", args);
> - } else {
> - return write(STDERR_FILENO, &c, 1);
> - }
> - }
> + {
> + qemu_semihosting_console_out(env, args, 1);
> + return 0xdeadbeef;
> + }
> case TARGET_SYS_WRITE0:
> - if (!(s = lock_user_string(args)))
> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> - return (uint32_t)-1;
> - len = strlen(s);
> - if (use_gdb_syscalls()) {
> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,%x",
> - args, len);
> - } else {
> - ret = write(STDERR_FILENO, s, len);
> - }
> - unlock_user(s, args, 0);
> - return ret;
> + return qemu_semihosting_console_out(env, args, 0);
> case TARGET_SYS_WRITE:
> GET_ARG(0);
> GET_ARG(1);
> --
> 2.20.1
>
>
Hi Alex,
this patch breaks build for softmmu target when CONFIG_SEMIHOSTING is not enabled as qemu_semihosting_console_out
is not defined in such case - neither linux-user/arm/semihost.c nor hw/semihosting/console.c compiled and function
is not in stubs/semihost.c
Mirek
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
2019-05-31 9:12 ` Miroslav Rezanina
@ 2019-05-31 10:42 ` Philippe Mathieu-Daudé
2019-05-31 10:44 ` Philippe Mathieu-Daudé
2019-05-31 10:53 ` Miroslav Rezanina
2019-05-31 11:08 ` Alex Bennée
1 sibling, 2 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-31 10:42 UTC (permalink / raw)
To: Miroslav Rezanina, Alex Bennée
Cc: Peter Maydell, Riku Voipio, qemu-devel, Laurent Vivier, qemu-arm,
Aleksandar Markovic
Hi Miroslav,
On 5/31/19 11:12 AM, Miroslav Rezanina wrote:
> On Tue, May 14, 2019 at 04:52:56PM +0100, Alex Bennée wrote:
>> Now we have a common semihosting console interface use that for our
>> string output. However ARM is currently unique in also supporting
>> semihosting for linux-user so we need to replicate the API in
>> linux-user. If other architectures gain this support we can move the
>> file later.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>> linux-user/Makefile.objs | 2 ++
>> linux-user/arm/semihost.c | 24 ++++++++++++++++++++++++
>> target/arm/arm-semi.c | 31 ++++++-------------------------
>> 3 files changed, 32 insertions(+), 25 deletions(-)
>> create mode 100644 linux-user/arm/semihost.c
>>
>> diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
>> index 769b8d83362..285c5dfa17a 100644
>> --- a/linux-user/Makefile.objs
>> +++ b/linux-user/Makefile.objs
>> @@ -6,4 +6,6 @@ obj-y = main.o syscall.o strace.o mmap.o signal.o \
>> obj-$(TARGET_HAS_BFLT) += flatload.o
>> obj-$(TARGET_I386) += vm86.o
>> obj-$(TARGET_ARM) += arm/nwfpe/
>> +obj-$(TARGET_ARM) += arm/semihost.o
>> +obj-$(TARGET_AARCH64) += arm/semihost.o
>> obj-$(TARGET_M68K) += m68k-sim.o
>> diff --git a/linux-user/arm/semihost.c b/linux-user/arm/semihost.c
>> new file mode 100644
>> index 00000000000..9554102a855
>> --- /dev/null
>> +++ b/linux-user/arm/semihost.c
>> @@ -0,0 +1,24 @@
>> +/*
>> + * ARM Semihosting Console Support
>> + *
>> + * Copyright (c) 2019 Linaro Ltd
>> + *
>> + * Currently ARM is unique in having support for semihosting support
>> + * in linux-user. So for now we implement the common console API but
>> + * just for arm linux-user.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "cpu.h"
>> +#include "hw/semihosting/console.h"
>> +#include "qemu.h"
>> +
>> +int qemu_semihosting_console_out(CPUArchState *env, target_ulong addr, int len)
>> +{
>> + void *s = lock_user_string(addr);
>> + len = write(STDERR_FILENO, s, len ? len : strlen(s));
>> + unlock_user(s, addr, 0);
>> + return len;
>> +}
>> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
>> index 9e5a414dd89..253c66b172a 100644
>> --- a/target/arm/arm-semi.c
>> +++ b/target/arm/arm-semi.c
>> @@ -27,6 +27,7 @@
>>
>> #include "cpu.h"
>> #include "hw/semihosting/semihost.h"
>> +#include "hw/semihosting/console.h"
>> #ifdef CONFIG_USER_ONLY
>> #include "qemu.h"
>>
>> @@ -314,32 +315,12 @@ target_ulong do_arm_semihosting(CPUARMState *env)
>> return set_swi_errno(ts, close(arg0));
>> }
>> case TARGET_SYS_WRITEC:
>> - {
>> - char c;
>> -
>> - if (get_user_u8(c, args))
>> - /* FIXME - should this error code be -TARGET_EFAULT ? */
>> - return (uint32_t)-1;
>> - /* Write to debug console. stderr is near enough. */
>> - if (use_gdb_syscalls()) {
>> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,1", args);
>> - } else {
>> - return write(STDERR_FILENO, &c, 1);
>> - }
>> - }
>> + {
>> + qemu_semihosting_console_out(env, args, 1);
>> + return 0xdeadbeef;
>> + }
>> case TARGET_SYS_WRITE0:
>> - if (!(s = lock_user_string(args)))
>> - /* FIXME - should this error code be -TARGET_EFAULT ? */
>> - return (uint32_t)-1;
>> - len = strlen(s);
>> - if (use_gdb_syscalls()) {
>> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,%x",
>> - args, len);
>> - } else {
>> - ret = write(STDERR_FILENO, s, len);
>> - }
>> - unlock_user(s, args, 0);
>> - return ret;
>> + return qemu_semihosting_console_out(env, args, 0);
>> case TARGET_SYS_WRITE:
>> GET_ARG(0);
>> GET_ARG(1);
>> --
>> 2.20.1
>>
>>
>
> Hi Alex,
>
> this patch breaks build for softmmu target when CONFIG_SEMIHOSTING is not enabled as qemu_semihosting_console_out
> is not defined in such case - neither linux-user/arm/semihost.c nor hw/semihosting/console.c compiled and function
> is not in stubs/semihost.c
Kinda funny, I noticed the same issue at the same time, and was chatting
with Alex about it.
I prepared a patch expliciting we can not disable CONFIG_SEMIHOSTING on
the MIPS arch. Would that work for you?
Regards,
Phil.
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
2019-05-31 10:42 ` Philippe Mathieu-Daudé
@ 2019-05-31 10:44 ` Philippe Mathieu-Daudé
2019-05-31 10:53 ` Miroslav Rezanina
1 sibling, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-31 10:44 UTC (permalink / raw)
To: Miroslav Rezanina, Alex Bennée
Cc: Peter Maydell, Riku Voipio, QEMU Developers, Laurent Vivier,
qemu-arm, Aleksandar Markovic
On Fri, May 31, 2019 at 12:42 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Hi Miroslav,
>
> On 5/31/19 11:12 AM, Miroslav Rezanina wrote:
> > On Tue, May 14, 2019 at 04:52:56PM +0100, Alex Bennée wrote:
> >> Now we have a common semihosting console interface use that for our
> >> string output. However ARM is currently unique in also supporting
> >> semihosting for linux-user so we need to replicate the API in
> >> linux-user. If other architectures gain this support we can move the
> >> file later.
> >>
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >> ---
> >> linux-user/Makefile.objs | 2 ++
> >> linux-user/arm/semihost.c | 24 ++++++++++++++++++++++++
> >> target/arm/arm-semi.c | 31 ++++++-------------------------
> >> 3 files changed, 32 insertions(+), 25 deletions(-)
> >> create mode 100644 linux-user/arm/semihost.c
> >>
> >> diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
> >> index 769b8d83362..285c5dfa17a 100644
> >> --- a/linux-user/Makefile.objs
> >> +++ b/linux-user/Makefile.objs
> >> @@ -6,4 +6,6 @@ obj-y = main.o syscall.o strace.o mmap.o signal.o \
> >> obj-$(TARGET_HAS_BFLT) += flatload.o
> >> obj-$(TARGET_I386) += vm86.o
> >> obj-$(TARGET_ARM) += arm/nwfpe/
> >> +obj-$(TARGET_ARM) += arm/semihost.o
> >> +obj-$(TARGET_AARCH64) += arm/semihost.o
> >> obj-$(TARGET_M68K) += m68k-sim.o
> >> diff --git a/linux-user/arm/semihost.c b/linux-user/arm/semihost.c
> >> new file mode 100644
> >> index 00000000000..9554102a855
> >> --- /dev/null
> >> +++ b/linux-user/arm/semihost.c
> >> @@ -0,0 +1,24 @@
> >> +/*
> >> + * ARM Semihosting Console Support
> >> + *
> >> + * Copyright (c) 2019 Linaro Ltd
> >> + *
> >> + * Currently ARM is unique in having support for semihosting support
> >> + * in linux-user. So for now we implement the common console API but
> >> + * just for arm linux-user.
> >> + *
> >> + * SPDX-License-Identifier: GPL-2.0-or-later
> >> + */
> >> +
> >> +#include "qemu/osdep.h"
> >> +#include "cpu.h"
> >> +#include "hw/semihosting/console.h"
> >> +#include "qemu.h"
> >> +
> >> +int qemu_semihosting_console_out(CPUArchState *env, target_ulong addr, int len)
> >> +{
> >> + void *s = lock_user_string(addr);
> >> + len = write(STDERR_FILENO, s, len ? len : strlen(s));
> >> + unlock_user(s, addr, 0);
> >> + return len;
> >> +}
> >> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
> >> index 9e5a414dd89..253c66b172a 100644
> >> --- a/target/arm/arm-semi.c
> >> +++ b/target/arm/arm-semi.c
> >> @@ -27,6 +27,7 @@
> >>
> >> #include "cpu.h"
> >> #include "hw/semihosting/semihost.h"
> >> +#include "hw/semihosting/console.h"
> >> #ifdef CONFIG_USER_ONLY
> >> #include "qemu.h"
> >>
> >> @@ -314,32 +315,12 @@ target_ulong do_arm_semihosting(CPUARMState *env)
> >> return set_swi_errno(ts, close(arg0));
> >> }
> >> case TARGET_SYS_WRITEC:
> >> - {
> >> - char c;
> >> -
> >> - if (get_user_u8(c, args))
> >> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> >> - return (uint32_t)-1;
> >> - /* Write to debug console. stderr is near enough. */
> >> - if (use_gdb_syscalls()) {
> >> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,1", args);
> >> - } else {
> >> - return write(STDERR_FILENO, &c, 1);
> >> - }
> >> - }
> >> + {
> >> + qemu_semihosting_console_out(env, args, 1);
> >> + return 0xdeadbeef;
> >> + }
> >> case TARGET_SYS_WRITE0:
> >> - if (!(s = lock_user_string(args)))
> >> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> >> - return (uint32_t)-1;
> >> - len = strlen(s);
> >> - if (use_gdb_syscalls()) {
> >> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,%x",
> >> - args, len);
> >> - } else {
> >> - ret = write(STDERR_FILENO, s, len);
> >> - }
> >> - unlock_user(s, args, 0);
> >> - return ret;
> >> + return qemu_semihosting_console_out(env, args, 0);
> >> case TARGET_SYS_WRITE:
> >> GET_ARG(0);
> >> GET_ARG(1);
> >> --
> >> 2.20.1
> >>
> >>
> >
> > Hi Alex,
> >
> > this patch breaks build for softmmu target when CONFIG_SEMIHOSTING is not enabled as qemu_semihosting_console_out
> > is not defined in such case - neither linux-user/arm/semihost.c nor hw/semihosting/console.c compiled and function
> > is not in stubs/semihost.c
linux-user/arm/semihost.c is not built on softmmu-only build.
> Kinda funny, I noticed the same issue at the same time, and was chatting
> with Alex about it.
>
> I prepared a patch expliciting we can not disable CONFIG_SEMIHOSTING on
> the MIPS arch. Would that work for you?
>
> Regards,
>
> Phil.
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
2019-05-31 10:42 ` Philippe Mathieu-Daudé
2019-05-31 10:44 ` Philippe Mathieu-Daudé
@ 2019-05-31 10:53 ` Miroslav Rezanina
1 sibling, 0 replies; 34+ messages in thread
From: Miroslav Rezanina @ 2019-05-31 10:53 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Riku Voipio, qemu-devel, Laurent Vivier, qemu-arm,
Aleksandar Markovic, Alex Bennée
----- Original Message -----
> From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
> To: "Miroslav Rezanina" <mrezanin@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>
> Cc: "Peter Maydell" <peter.maydell@linaro.org>, "Riku Voipio" <riku.voipio@iki.fi>, qemu-devel@nongnu.org,
> qemu-arm@nongnu.org, "Laurent Vivier" <laurent@vivier.eu>, "Aleksandar Markovic" <amarkovic@wavecomp.com>
> Sent: Friday, May 31, 2019 12:42:46 PM
> Subject: Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
>
> Hi Miroslav,
>
> On 5/31/19 11:12 AM, Miroslav Rezanina wrote:
> > On Tue, May 14, 2019 at 04:52:56PM +0100, Alex Bennée wrote:
> >> Now we have a common semihosting console interface use that for our
> >> string output. However ARM is currently unique in also supporting
> >> semihosting for linux-user so we need to replicate the API in
> >> linux-user. If other architectures gain this support we can move the
> >> file later.
> >>
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >> ---
> >> linux-user/Makefile.objs | 2 ++
> >> linux-user/arm/semihost.c | 24 ++++++++++++++++++++++++
> >> target/arm/arm-semi.c | 31 ++++++-------------------------
> >> 3 files changed, 32 insertions(+), 25 deletions(-)
> >> create mode 100644 linux-user/arm/semihost.c
> >>
> >> diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
> >> index 769b8d83362..285c5dfa17a 100644
> >> --- a/linux-user/Makefile.objs
> >> +++ b/linux-user/Makefile.objs
> >> @@ -6,4 +6,6 @@ obj-y = main.o syscall.o strace.o mmap.o signal.o \
> >> obj-$(TARGET_HAS_BFLT) += flatload.o
> >> obj-$(TARGET_I386) += vm86.o
> >> obj-$(TARGET_ARM) += arm/nwfpe/
> >> +obj-$(TARGET_ARM) += arm/semihost.o
> >> +obj-$(TARGET_AARCH64) += arm/semihost.o
> >> obj-$(TARGET_M68K) += m68k-sim.o
> >> diff --git a/linux-user/arm/semihost.c b/linux-user/arm/semihost.c
> >> new file mode 100644
> >> index 00000000000..9554102a855
> >> --- /dev/null
> >> +++ b/linux-user/arm/semihost.c
> >> @@ -0,0 +1,24 @@
> >> +/*
> >> + * ARM Semihosting Console Support
> >> + *
> >> + * Copyright (c) 2019 Linaro Ltd
> >> + *
> >> + * Currently ARM is unique in having support for semihosting support
> >> + * in linux-user. So for now we implement the common console API but
> >> + * just for arm linux-user.
> >> + *
> >> + * SPDX-License-Identifier: GPL-2.0-or-later
> >> + */
> >> +
> >> +#include "qemu/osdep.h"
> >> +#include "cpu.h"
> >> +#include "hw/semihosting/console.h"
> >> +#include "qemu.h"
> >> +
> >> +int qemu_semihosting_console_out(CPUArchState *env, target_ulong addr,
> >> int len)
> >> +{
> >> + void *s = lock_user_string(addr);
> >> + len = write(STDERR_FILENO, s, len ? len : strlen(s));
> >> + unlock_user(s, addr, 0);
> >> + return len;
> >> +}
> >> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
> >> index 9e5a414dd89..253c66b172a 100644
> >> --- a/target/arm/arm-semi.c
> >> +++ b/target/arm/arm-semi.c
> >> @@ -27,6 +27,7 @@
> >>
> >> #include "cpu.h"
> >> #include "hw/semihosting/semihost.h"
> >> +#include "hw/semihosting/console.h"
> >> #ifdef CONFIG_USER_ONLY
> >> #include "qemu.h"
> >>
> >> @@ -314,32 +315,12 @@ target_ulong do_arm_semihosting(CPUARMState *env)
> >> return set_swi_errno(ts, close(arg0));
> >> }
> >> case TARGET_SYS_WRITEC:
> >> - {
> >> - char c;
> >> -
> >> - if (get_user_u8(c, args))
> >> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> >> - return (uint32_t)-1;
> >> - /* Write to debug console. stderr is near enough. */
> >> - if (use_gdb_syscalls()) {
> >> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,1",
> >> args);
> >> - } else {
> >> - return write(STDERR_FILENO, &c, 1);
> >> - }
> >> - }
> >> + {
> >> + qemu_semihosting_console_out(env, args, 1);
> >> + return 0xdeadbeef;
> >> + }
> >> case TARGET_SYS_WRITE0:
> >> - if (!(s = lock_user_string(args)))
> >> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> >> - return (uint32_t)-1;
> >> - len = strlen(s);
> >> - if (use_gdb_syscalls()) {
> >> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,%x",
> >> - args, len);
> >> - } else {
> >> - ret = write(STDERR_FILENO, s, len);
> >> - }
> >> - unlock_user(s, args, 0);
> >> - return ret;
> >> + return qemu_semihosting_console_out(env, args, 0);
> >> case TARGET_SYS_WRITE:
> >> GET_ARG(0);
> >> GET_ARG(1);
> >> --
> >> 2.20.1
> >>
> >>
> >
> > Hi Alex,
> >
> > this patch breaks build for softmmu target when CONFIG_SEMIHOSTING is not
> > enabled as qemu_semihosting_console_out
> > is not defined in such case - neither linux-user/arm/semihost.c nor
> > hw/semihosting/console.c compiled and function
> > is not in stubs/semihost.c
>
> Kinda funny, I noticed the same issue at the same time, and was chatting
> with Alex about it.
>
> I prepared a patch expliciting we can not disable CONFIG_SEMIHOSTING on
> the MIPS arch. Would that work for you?
>
> Regards,
>
> Phil.
>
Hi Phil,
we've got problem with build for AArch64 where SEMIHOSTING cannot be disabled too:
target/arm/helper.c use do_arm_semihosting
Mirek
--
Miroslav Rezanina
Software Engineer - Virtualization Team Maintainer
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
2019-05-31 9:12 ` Miroslav Rezanina
2019-05-31 10:42 ` Philippe Mathieu-Daudé
@ 2019-05-31 11:08 ` Alex Bennée
2019-05-31 11:28 ` Miroslav Rezanina
1 sibling, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2019-05-31 11:08 UTC (permalink / raw)
To: Miroslav Rezanina
Cc: Peter Maydell, Riku Voipio, qemu-devel, qemu-arm, Laurent Vivier
Miroslav Rezanina <mrezanin@redhat.com> writes:
> On Tue, May 14, 2019 at 04:52:56PM +0100, Alex Bennée wrote:
>> Now we have a common semihosting console interface use that for our
>> string output. However ARM is currently unique in also supporting
>> semihosting for linux-user so we need to replicate the API in
>> linux-user. If other architectures gain this support we can move the
>> file later.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>> linux-user/Makefile.objs | 2 ++
>> linux-user/arm/semihost.c | 24 ++++++++++++++++++++++++
>> target/arm/arm-semi.c | 31 ++++++-------------------------
>> 3 files changed, 32 insertions(+), 25 deletions(-)
>> create mode 100644 linux-user/arm/semihost.c
>>
>> diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
>> index 769b8d83362..285c5dfa17a 100644
>> --- a/linux-user/Makefile.objs
>> +++ b/linux-user/Makefile.objs
>> @@ -6,4 +6,6 @@ obj-y = main.o syscall.o strace.o mmap.o signal.o \
>> obj-$(TARGET_HAS_BFLT) += flatload.o
>> obj-$(TARGET_I386) += vm86.o
>> obj-$(TARGET_ARM) += arm/nwfpe/
>> +obj-$(TARGET_ARM) += arm/semihost.o
>> +obj-$(TARGET_AARCH64) += arm/semihost.o
>> obj-$(TARGET_M68K) += m68k-sim.o
>> diff --git a/linux-user/arm/semihost.c b/linux-user/arm/semihost.c
>> new file mode 100644
>> index 00000000000..9554102a855
>> --- /dev/null
>> +++ b/linux-user/arm/semihost.c
>> @@ -0,0 +1,24 @@
>> +/*
>> + * ARM Semihosting Console Support
>> + *
>> + * Copyright (c) 2019 Linaro Ltd
>> + *
>> + * Currently ARM is unique in having support for semihosting support
>> + * in linux-user. So for now we implement the common console API but
>> + * just for arm linux-user.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "cpu.h"
>> +#include "hw/semihosting/console.h"
>> +#include "qemu.h"
>> +
>> +int qemu_semihosting_console_out(CPUArchState *env, target_ulong addr, int len)
>> +{
>> + void *s = lock_user_string(addr);
>> + len = write(STDERR_FILENO, s, len ? len : strlen(s));
>> + unlock_user(s, addr, 0);
>> + return len;
>> +}
>> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
>> index 9e5a414dd89..253c66b172a 100644
>> --- a/target/arm/arm-semi.c
>> +++ b/target/arm/arm-semi.c
>> @@ -27,6 +27,7 @@
>>
>> #include "cpu.h"
>> #include "hw/semihosting/semihost.h"
>> +#include "hw/semihosting/console.h"
>> #ifdef CONFIG_USER_ONLY
>> #include "qemu.h"
>>
>> @@ -314,32 +315,12 @@ target_ulong do_arm_semihosting(CPUARMState *env)
>> return set_swi_errno(ts, close(arg0));
>> }
>> case TARGET_SYS_WRITEC:
>> - {
>> - char c;
>> -
>> - if (get_user_u8(c, args))
>> - /* FIXME - should this error code be -TARGET_EFAULT ? */
>> - return (uint32_t)-1;
>> - /* Write to debug console. stderr is near enough. */
>> - if (use_gdb_syscalls()) {
>> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,1", args);
>> - } else {
>> - return write(STDERR_FILENO, &c, 1);
>> - }
>> - }
>> + {
>> + qemu_semihosting_console_out(env, args, 1);
>> + return 0xdeadbeef;
>> + }
>> case TARGET_SYS_WRITE0:
>> - if (!(s = lock_user_string(args)))
>> - /* FIXME - should this error code be -TARGET_EFAULT ? */
>> - return (uint32_t)-1;
>> - len = strlen(s);
>> - if (use_gdb_syscalls()) {
>> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,%x",
>> - args, len);
>> - } else {
>> - ret = write(STDERR_FILENO, s, len);
>> - }
>> - unlock_user(s, args, 0);
>> - return ret;
>> + return qemu_semihosting_console_out(env, args, 0);
>> case TARGET_SYS_WRITE:
>> GET_ARG(0);
>> GET_ARG(1);
>> --
>> 2.20.1
>>
>>
>
> Hi Alex,
>
> this patch breaks build for softmmu target when CONFIG_SEMIHOSTING is not enabled as qemu_semihosting_console_out
> is not defined in such case - neither linux-user/arm/semihost.c nor hw/semihosting/console.c compiled and function
> is not in stubs/semihost.c
How do you do that? I tried ../../configure --without-default-devices
and that still builds for me.
But I suspect what's needed is to change:
#ifndef CONFIG_USER_ONLY
to
#ifdef CONFIG_SEMIHOSTING
to the relevant headers and helper bits.
>
> Mirek
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
2019-05-31 11:08 ` Alex Bennée
@ 2019-05-31 11:28 ` Miroslav Rezanina
2019-05-31 13:16 ` Alex Bennée
0 siblings, 1 reply; 34+ messages in thread
From: Miroslav Rezanina @ 2019-05-31 11:28 UTC (permalink / raw)
To: Alex Bennée
Cc: Peter Maydell, Riku Voipio, qemu-devel, qemu-arm, Laurent Vivier
----- Original Message -----
> From: "Alex Bennée" <alex.bennee@linaro.org>
> To: "Miroslav Rezanina" <mrezanin@redhat.com>
> Cc: qemu-devel@nongnu.org, "Peter Maydell" <peter.maydell@linaro.org>, "Riku Voipio" <riku.voipio@iki.fi>,
> qemu-arm@nongnu.org, "Laurent Vivier" <laurent@vivier.eu>
> Sent: Friday, May 31, 2019 1:08:06 PM
> Subject: Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
>
>
> Miroslav Rezanina <mrezanin@redhat.com> writes:
>
> > On Tue, May 14, 2019 at 04:52:56PM +0100, Alex Bennée wrote:
> >> Now we have a common semihosting console interface use that for our
> >> string output. However ARM is currently unique in also supporting
> >> semihosting for linux-user so we need to replicate the API in
> >> linux-user. If other architectures gain this support we can move the
> >> file later.
> >>
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >> ---
> >> linux-user/Makefile.objs | 2 ++
> >> linux-user/arm/semihost.c | 24 ++++++++++++++++++++++++
> >> target/arm/arm-semi.c | 31 ++++++-------------------------
> >> 3 files changed, 32 insertions(+), 25 deletions(-)
> >> create mode 100644 linux-user/arm/semihost.c
> >>
> >> diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
> >> index 769b8d83362..285c5dfa17a 100644
> >> --- a/linux-user/Makefile.objs
> >> +++ b/linux-user/Makefile.objs
> >> @@ -6,4 +6,6 @@ obj-y = main.o syscall.o strace.o mmap.o signal.o \
> >> obj-$(TARGET_HAS_BFLT) += flatload.o
> >> obj-$(TARGET_I386) += vm86.o
> >> obj-$(TARGET_ARM) += arm/nwfpe/
> >> +obj-$(TARGET_ARM) += arm/semihost.o
> >> +obj-$(TARGET_AARCH64) += arm/semihost.o
> >> obj-$(TARGET_M68K) += m68k-sim.o
> >> diff --git a/linux-user/arm/semihost.c b/linux-user/arm/semihost.c
> >> new file mode 100644
> >> index 00000000000..9554102a855
> >> --- /dev/null
> >> +++ b/linux-user/arm/semihost.c
> >> @@ -0,0 +1,24 @@
> >> +/*
> >> + * ARM Semihosting Console Support
> >> + *
> >> + * Copyright (c) 2019 Linaro Ltd
> >> + *
> >> + * Currently ARM is unique in having support for semihosting support
> >> + * in linux-user. So for now we implement the common console API but
> >> + * just for arm linux-user.
> >> + *
> >> + * SPDX-License-Identifier: GPL-2.0-or-later
> >> + */
> >> +
> >> +#include "qemu/osdep.h"
> >> +#include "cpu.h"
> >> +#include "hw/semihosting/console.h"
> >> +#include "qemu.h"
> >> +
> >> +int qemu_semihosting_console_out(CPUArchState *env, target_ulong addr,
> >> int len)
> >> +{
> >> + void *s = lock_user_string(addr);
> >> + len = write(STDERR_FILENO, s, len ? len : strlen(s));
> >> + unlock_user(s, addr, 0);
> >> + return len;
> >> +}
> >> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
> >> index 9e5a414dd89..253c66b172a 100644
> >> --- a/target/arm/arm-semi.c
> >> +++ b/target/arm/arm-semi.c
> >> @@ -27,6 +27,7 @@
> >>
> >> #include "cpu.h"
> >> #include "hw/semihosting/semihost.h"
> >> +#include "hw/semihosting/console.h"
> >> #ifdef CONFIG_USER_ONLY
> >> #include "qemu.h"
> >>
> >> @@ -314,32 +315,12 @@ target_ulong do_arm_semihosting(CPUARMState *env)
> >> return set_swi_errno(ts, close(arg0));
> >> }
> >> case TARGET_SYS_WRITEC:
> >> - {
> >> - char c;
> >> -
> >> - if (get_user_u8(c, args))
> >> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> >> - return (uint32_t)-1;
> >> - /* Write to debug console. stderr is near enough. */
> >> - if (use_gdb_syscalls()) {
> >> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,1",
> >> args);
> >> - } else {
> >> - return write(STDERR_FILENO, &c, 1);
> >> - }
> >> - }
> >> + {
> >> + qemu_semihosting_console_out(env, args, 1);
> >> + return 0xdeadbeef;
> >> + }
> >> case TARGET_SYS_WRITE0:
> >> - if (!(s = lock_user_string(args)))
> >> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> >> - return (uint32_t)-1;
> >> - len = strlen(s);
> >> - if (use_gdb_syscalls()) {
> >> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,%x",
> >> - args, len);
> >> - } else {
> >> - ret = write(STDERR_FILENO, s, len);
> >> - }
> >> - unlock_user(s, args, 0);
> >> - return ret;
> >> + return qemu_semihosting_console_out(env, args, 0);
> >> case TARGET_SYS_WRITE:
> >> GET_ARG(0);
> >> GET_ARG(1);
> >> --
> >> 2.20.1
> >>
> >>
> >
> > Hi Alex,
> >
> > this patch breaks build for softmmu target when CONFIG_SEMIHOSTING is not
> > enabled as qemu_semihosting_console_out
> > is not defined in such case - neither linux-user/arm/semihost.c nor
> > hw/semihosting/console.c compiled and function
> > is not in stubs/semihost.c
>
> How do you do that? I tried ../../configure --without-default-devices
> and that still builds for me.
It's usual RHEL way - use --without-default-devices and use specific
list of enabled devices (this mean disable CONFIG_SEMIHOSTING in
default_config/* file).
>
> But I suspect what's needed is to change:
>
> #ifndef CONFIG_USER_ONLY
>
> to
>
> #ifdef CONFIG_SEMIHOSTING
>
> to the relevant headers and helper bits.
Yeah, have to find out what are relevant pieces.
Mirek
>
> >
> > Mirek
>
>
> --
> Alex Bennée
>
--
Miroslav Rezanina
Software Engineer - Virtualization Team Maintainer
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
2019-05-31 11:28 ` Miroslav Rezanina
@ 2019-05-31 13:16 ` Alex Bennée
2019-05-31 13:59 ` Miroslav Rezanina
0 siblings, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2019-05-31 13:16 UTC (permalink / raw)
To: Miroslav Rezanina
Cc: Peter Maydell, Riku Voipio, qemu-devel, qemu-arm, Laurent Vivier
Miroslav Rezanina <mrezanin@redhat.com> writes:
> ----- Original Message -----
>> From: "Alex Bennée" <alex.bennee@linaro.org>
>> To: "Miroslav Rezanina" <mrezanin@redhat.com>
>> Cc: qemu-devel@nongnu.org, "Peter Maydell" <peter.maydell@linaro.org>, "Riku Voipio" <riku.voipio@iki.fi>,
>> qemu-arm@nongnu.org, "Laurent Vivier" <laurent@vivier.eu>
>> Sent: Friday, May 31, 2019 1:08:06 PM
>> Subject: Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
>>
>>
>> Miroslav Rezanina <mrezanin@redhat.com> writes:
>>
>> > On Tue, May 14, 2019 at 04:52:56PM +0100, Alex Bennée wrote:
>> >> Now we have a common semihosting console interface use that for our
>> >> string output. However ARM is currently unique in also supporting
>> >> semihosting for linux-user so we need to replicate the API in
>> >> linux-user. If other architectures gain this support we can move the
>> >> file later.
>> >>
>> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> >> ---
>> >> linux-user/Makefile.objs | 2 ++
>> >> linux-user/arm/semihost.c | 24 ++++++++++++++++++++++++
>> >> target/arm/arm-semi.c | 31 ++++++-------------------------
>> >> 3 files changed, 32 insertions(+), 25 deletions(-)
>> >> create mode 100644 linux-user/arm/semihost.c
>> >>
>> >> diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
>> >> index 769b8d83362..285c5dfa17a 100644
>> >> --- a/linux-user/Makefile.objs
>> >> +++ b/linux-user/Makefile.objs
>> >> @@ -6,4 +6,6 @@ obj-y = main.o syscall.o strace.o mmap.o signal.o \
>> >> obj-$(TARGET_HAS_BFLT) += flatload.o
>> >> obj-$(TARGET_I386) += vm86.o
>> >> obj-$(TARGET_ARM) += arm/nwfpe/
>> >> +obj-$(TARGET_ARM) += arm/semihost.o
>> >> +obj-$(TARGET_AARCH64) += arm/semihost.o
>> >> obj-$(TARGET_M68K) += m68k-sim.o
>> >> diff --git a/linux-user/arm/semihost.c b/linux-user/arm/semihost.c
>> >> new file mode 100644
>> >> index 00000000000..9554102a855
>> >> --- /dev/null
>> >> +++ b/linux-user/arm/semihost.c
>> >> @@ -0,0 +1,24 @@
>> >> +/*
>> >> + * ARM Semihosting Console Support
>> >> + *
>> >> + * Copyright (c) 2019 Linaro Ltd
>> >> + *
>> >> + * Currently ARM is unique in having support for semihosting support
>> >> + * in linux-user. So for now we implement the common console API but
>> >> + * just for arm linux-user.
>> >> + *
>> >> + * SPDX-License-Identifier: GPL-2.0-or-later
>> >> + */
>> >> +
>> >> +#include "qemu/osdep.h"
>> >> +#include "cpu.h"
>> >> +#include "hw/semihosting/console.h"
>> >> +#include "qemu.h"
>> >> +
>> >> +int qemu_semihosting_console_out(CPUArchState *env, target_ulong addr,
>> >> int len)
>> >> +{
>> >> + void *s = lock_user_string(addr);
>> >> + len = write(STDERR_FILENO, s, len ? len : strlen(s));
>> >> + unlock_user(s, addr, 0);
>> >> + return len;
>> >> +}
>> >> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
>> >> index 9e5a414dd89..253c66b172a 100644
>> >> --- a/target/arm/arm-semi.c
>> >> +++ b/target/arm/arm-semi.c
>> >> @@ -27,6 +27,7 @@
>> >>
>> >> #include "cpu.h"
>> >> #include "hw/semihosting/semihost.h"
>> >> +#include "hw/semihosting/console.h"
>> >> #ifdef CONFIG_USER_ONLY
>> >> #include "qemu.h"
>> >>
>> >> @@ -314,32 +315,12 @@ target_ulong do_arm_semihosting(CPUARMState *env)
>> >> return set_swi_errno(ts, close(arg0));
>> >> }
>> >> case TARGET_SYS_WRITEC:
>> >> - {
>> >> - char c;
>> >> -
>> >> - if (get_user_u8(c, args))
>> >> - /* FIXME - should this error code be -TARGET_EFAULT ? */
>> >> - return (uint32_t)-1;
>> >> - /* Write to debug console. stderr is near enough. */
>> >> - if (use_gdb_syscalls()) {
>> >> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,1",
>> >> args);
>> >> - } else {
>> >> - return write(STDERR_FILENO, &c, 1);
>> >> - }
>> >> - }
>> >> + {
>> >> + qemu_semihosting_console_out(env, args, 1);
>> >> + return 0xdeadbeef;
>> >> + }
>> >> case TARGET_SYS_WRITE0:
>> >> - if (!(s = lock_user_string(args)))
>> >> - /* FIXME - should this error code be -TARGET_EFAULT ? */
>> >> - return (uint32_t)-1;
>> >> - len = strlen(s);
>> >> - if (use_gdb_syscalls()) {
>> >> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,%x",
>> >> - args, len);
>> >> - } else {
>> >> - ret = write(STDERR_FILENO, s, len);
>> >> - }
>> >> - unlock_user(s, args, 0);
>> >> - return ret;
>> >> + return qemu_semihosting_console_out(env, args, 0);
>> >> case TARGET_SYS_WRITE:
>> >> GET_ARG(0);
>> >> GET_ARG(1);
>> >> --
>> >> 2.20.1
>> >>
>> >>
>> >
>> > Hi Alex,
>> >
>> > this patch breaks build for softmmu target when CONFIG_SEMIHOSTING is not
>> > enabled as qemu_semihosting_console_out
>> > is not defined in such case - neither linux-user/arm/semihost.c nor
>> > hw/semihosting/console.c compiled and function
>> > is not in stubs/semihost.c
>>
>> How do you do that? I tried ../../configure --without-default-devices
>> and that still builds for me.
>
> It's usual RHEL way - use --without-default-devices and use specific
> list of enabled devices (this mean disable CONFIG_SEMIHOSTING in
> default_config/* file).
OK - so from the upstream source tree CONFIG_SEMIHOSTING is still =y
(but I can see most of them are now =n). Isn't the simplest solution to
fix-up your version of the default_config file to include SEMIHOSTING?
Is this an out-of-tree RHEL addition?
>>
>> But I suspect what's needed is to change:
>>
>> #ifndef CONFIG_USER_ONLY
>>
>> to
>>
>> #ifdef CONFIG_SEMIHOSTING
>>
>> to the relevant headers and helper bits.
>
> Yeah, have to find out what are relevant pieces.
>
> Mirek
>
>>
>> >
>> > Mirek
>>
>>
>> --
>> Alex Bennée
>>
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
2019-05-31 13:16 ` Alex Bennée
@ 2019-05-31 13:59 ` Miroslav Rezanina
2019-05-31 14:28 ` Alex Bennée
0 siblings, 1 reply; 34+ messages in thread
From: Miroslav Rezanina @ 2019-05-31 13:59 UTC (permalink / raw)
To: Alex Bennée
Cc: Peter Maydell, Riku Voipio, qemu-devel, qemu-arm, Laurent Vivier
----- Original Message -----
> From: "Alex Bennée" <alex.bennee@linaro.org>
> To: "Miroslav Rezanina" <mrezanin@redhat.com>
> Cc: qemu-devel@nongnu.org, "Peter Maydell" <peter.maydell@linaro.org>, "Riku Voipio" <riku.voipio@iki.fi>,
> qemu-arm@nongnu.org, "Laurent Vivier" <laurent@vivier.eu>
> Sent: Friday, May 31, 2019 3:16:38 PM
> Subject: Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
>
>
> Miroslav Rezanina <mrezanin@redhat.com> writes:
>
> > ----- Original Message -----
> >> From: "Alex Bennée" <alex.bennee@linaro.org>
> >> To: "Miroslav Rezanina" <mrezanin@redhat.com>
> >> Cc: qemu-devel@nongnu.org, "Peter Maydell" <peter.maydell@linaro.org>,
> >> "Riku Voipio" <riku.voipio@iki.fi>,
> >> qemu-arm@nongnu.org, "Laurent Vivier" <laurent@vivier.eu>
> >> Sent: Friday, May 31, 2019 1:08:06 PM
> >> Subject: Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common
> >> interface for WRITE0/WRITEC in arm-semi
> >>
> >>
> >> Miroslav Rezanina <mrezanin@redhat.com> writes:
> >>
> >> > On Tue, May 14, 2019 at 04:52:56PM +0100, Alex Bennée wrote:
> >> >> Now we have a common semihosting console interface use that for our
> >> >> string output. However ARM is currently unique in also supporting
> >> >> semihosting for linux-user so we need to replicate the API in
> >> >> linux-user. If other architectures gain this support we can move the
> >> >> file later.
> >> >>
> >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >> >> ---
> >> >> linux-user/Makefile.objs | 2 ++
> >> >> linux-user/arm/semihost.c | 24 ++++++++++++++++++++++++
> >> >> target/arm/arm-semi.c | 31 ++++++-------------------------
> >> >> 3 files changed, 32 insertions(+), 25 deletions(-)
> >> >> create mode 100644 linux-user/arm/semihost.c
> >> >>
> >> >> diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
> >> >> index 769b8d83362..285c5dfa17a 100644
> >> >> --- a/linux-user/Makefile.objs
> >> >> +++ b/linux-user/Makefile.objs
> >> >> @@ -6,4 +6,6 @@ obj-y = main.o syscall.o strace.o mmap.o signal.o \
> >> >> obj-$(TARGET_HAS_BFLT) += flatload.o
> >> >> obj-$(TARGET_I386) += vm86.o
> >> >> obj-$(TARGET_ARM) += arm/nwfpe/
> >> >> +obj-$(TARGET_ARM) += arm/semihost.o
> >> >> +obj-$(TARGET_AARCH64) += arm/semihost.o
> >> >> obj-$(TARGET_M68K) += m68k-sim.o
> >> >> diff --git a/linux-user/arm/semihost.c b/linux-user/arm/semihost.c
> >> >> new file mode 100644
> >> >> index 00000000000..9554102a855
> >> >> --- /dev/null
> >> >> +++ b/linux-user/arm/semihost.c
> >> >> @@ -0,0 +1,24 @@
> >> >> +/*
> >> >> + * ARM Semihosting Console Support
> >> >> + *
> >> >> + * Copyright (c) 2019 Linaro Ltd
> >> >> + *
> >> >> + * Currently ARM is unique in having support for semihosting support
> >> >> + * in linux-user. So for now we implement the common console API but
> >> >> + * just for arm linux-user.
> >> >> + *
> >> >> + * SPDX-License-Identifier: GPL-2.0-or-later
> >> >> + */
> >> >> +
> >> >> +#include "qemu/osdep.h"
> >> >> +#include "cpu.h"
> >> >> +#include "hw/semihosting/console.h"
> >> >> +#include "qemu.h"
> >> >> +
> >> >> +int qemu_semihosting_console_out(CPUArchState *env, target_ulong addr,
> >> >> int len)
> >> >> +{
> >> >> + void *s = lock_user_string(addr);
> >> >> + len = write(STDERR_FILENO, s, len ? len : strlen(s));
> >> >> + unlock_user(s, addr, 0);
> >> >> + return len;
> >> >> +}
> >> >> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
> >> >> index 9e5a414dd89..253c66b172a 100644
> >> >> --- a/target/arm/arm-semi.c
> >> >> +++ b/target/arm/arm-semi.c
> >> >> @@ -27,6 +27,7 @@
> >> >>
> >> >> #include "cpu.h"
> >> >> #include "hw/semihosting/semihost.h"
> >> >> +#include "hw/semihosting/console.h"
> >> >> #ifdef CONFIG_USER_ONLY
> >> >> #include "qemu.h"
> >> >>
> >> >> @@ -314,32 +315,12 @@ target_ulong do_arm_semihosting(CPUARMState *env)
> >> >> return set_swi_errno(ts, close(arg0));
> >> >> }
> >> >> case TARGET_SYS_WRITEC:
> >> >> - {
> >> >> - char c;
> >> >> -
> >> >> - if (get_user_u8(c, args))
> >> >> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> >> >> - return (uint32_t)-1;
> >> >> - /* Write to debug console. stderr is near enough. */
> >> >> - if (use_gdb_syscalls()) {
> >> >> - return arm_gdb_syscall(cpu, arm_semi_cb,
> >> >> "write,2,%x,1",
> >> >> args);
> >> >> - } else {
> >> >> - return write(STDERR_FILENO, &c, 1);
> >> >> - }
> >> >> - }
> >> >> + {
> >> >> + qemu_semihosting_console_out(env, args, 1);
> >> >> + return 0xdeadbeef;
> >> >> + }
> >> >> case TARGET_SYS_WRITE0:
> >> >> - if (!(s = lock_user_string(args)))
> >> >> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> >> >> - return (uint32_t)-1;
> >> >> - len = strlen(s);
> >> >> - if (use_gdb_syscalls()) {
> >> >> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,%x",
> >> >> - args, len);
> >> >> - } else {
> >> >> - ret = write(STDERR_FILENO, s, len);
> >> >> - }
> >> >> - unlock_user(s, args, 0);
> >> >> - return ret;
> >> >> + return qemu_semihosting_console_out(env, args, 0);
> >> >> case TARGET_SYS_WRITE:
> >> >> GET_ARG(0);
> >> >> GET_ARG(1);
> >> >> --
> >> >> 2.20.1
> >> >>
> >> >>
> >> >
> >> > Hi Alex,
> >> >
> >> > this patch breaks build for softmmu target when CONFIG_SEMIHOSTING is
> >> > not
> >> > enabled as qemu_semihosting_console_out
> >> > is not defined in such case - neither linux-user/arm/semihost.c nor
> >> > hw/semihosting/console.c compiled and function
> >> > is not in stubs/semihost.c
> >>
> >> How do you do that? I tried ../../configure --without-default-devices
> >> and that still builds for me.
> >
> > It's usual RHEL way - use --without-default-devices and use specific
> > list of enabled devices (this mean disable CONFIG_SEMIHOSTING in
> > default_config/* file).
>
> OK - so from the upstream source tree CONFIG_SEMIHOSTING is still =y
> (but I can see most of them are now =n). Isn't the simplest solution to
> fix-up your version of the default_config file to include SEMIHOSTING?
>
It's fix but it goes against our policy of handling CONFIG options so we
would prefer to have this fixed - otherwise there's no meaning in having
config option if you can't disable it.
> Is this an out-of-tree RHEL addition?
>
No, it's RHEL device config handling.
Mirek
> >>
> >> But I suspect what's needed is to change:
> >>
> >> #ifndef CONFIG_USER_ONLY
> >>
> >> to
> >>
> >> #ifdef CONFIG_SEMIHOSTING
> >>
> >> to the relevant headers and helper bits.
> >
> > Yeah, have to find out what are relevant pieces.
> >
> > Mirek
> >
> >>
> >> >
> >> > Mirek
> >>
> >>
> >> --
> >> Alex Bennée
> >>
>
>
> --
> Alex Bennée
>
--
Miroslav Rezanina
Software Engineer - Virtualization Team Maintainer
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
2019-05-31 13:59 ` Miroslav Rezanina
@ 2019-05-31 14:28 ` Alex Bennée
2019-05-31 14:38 ` Peter Maydell
2019-05-31 16:50 ` Miroslav Rezanina
0 siblings, 2 replies; 34+ messages in thread
From: Alex Bennée @ 2019-05-31 14:28 UTC (permalink / raw)
To: Miroslav Rezanina
Cc: Peter Maydell, Riku Voipio, qemu-devel, qemu-arm, Laurent Vivier
Miroslav Rezanina <mrezanin@redhat.com> writes:
> ----- Original Message -----
>> From: "Alex Bennée" <alex.bennee@linaro.org>
>> To: "Miroslav Rezanina" <mrezanin@redhat.com>
>> Cc: qemu-devel@nongnu.org, "Peter Maydell" <peter.maydell@linaro.org>, "Riku Voipio" <riku.voipio@iki.fi>,
>> qemu-arm@nongnu.org, "Laurent Vivier" <laurent@vivier.eu>
>> Sent: Friday, May 31, 2019 3:16:38 PM
>> Subject: Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
>>
>>
>> Miroslav Rezanina <mrezanin@redhat.com> writes:
>>
>> > ----- Original Message -----
>> >> From: "Alex Bennée" <alex.bennee@linaro.org>
>> >> To: "Miroslav Rezanina" <mrezanin@redhat.com>
>> >> Cc: qemu-devel@nongnu.org, "Peter Maydell" <peter.maydell@linaro.org>,
>> >> "Riku Voipio" <riku.voipio@iki.fi>,
>> >> qemu-arm@nongnu.org, "Laurent Vivier" <laurent@vivier.eu>
>> >> Sent: Friday, May 31, 2019 1:08:06 PM
>> >> Subject: Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common
>> >> interface for WRITE0/WRITEC in arm-semi
>> >>
>> >>
>> >> Miroslav Rezanina <mrezanin@redhat.com> writes:
>> >>
>> >> > On Tue, May 14, 2019 at 04:52:56PM +0100, Alex Bennée wrote:
>> >> >> Now we have a common semihosting console interface use that for our
>> >> >> string output. However ARM is currently unique in also supporting
>> >> >> semihosting for linux-user so we need to replicate the API in
>> >> >> linux-user. If other architectures gain this support we can move the
>> >> >> file later.
>> >> >>
>> >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> >> >> ---
>> >> >> linux-user/Makefile.objs | 2 ++
>> >> >> linux-user/arm/semihost.c | 24 ++++++++++++++++++++++++
>> >> >> target/arm/arm-semi.c | 31 ++++++-------------------------
>> >> >> 3 files changed, 32 insertions(+), 25 deletions(-)
>> >> >> create mode 100644 linux-user/arm/semihost.c
>> >> >>
>> >> >> diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
>> >> >> index 769b8d83362..285c5dfa17a 100644
>> >> >> --- a/linux-user/Makefile.objs
>> >> >> +++ b/linux-user/Makefile.objs
>> >> >> @@ -6,4 +6,6 @@ obj-y = main.o syscall.o strace.o mmap.o signal.o \
>> >> >> obj-$(TARGET_HAS_BFLT) += flatload.o
>> >> >> obj-$(TARGET_I386) += vm86.o
>> >> >> obj-$(TARGET_ARM) += arm/nwfpe/
>> >> >> +obj-$(TARGET_ARM) += arm/semihost.o
>> >> >> +obj-$(TARGET_AARCH64) += arm/semihost.o
>> >> >> obj-$(TARGET_M68K) += m68k-sim.o
>> >> >> diff --git a/linux-user/arm/semihost.c b/linux-user/arm/semihost.c
>> >> >> new file mode 100644
>> >> >> index 00000000000..9554102a855
>> >> >> --- /dev/null
>> >> >> +++ b/linux-user/arm/semihost.c
>> >> >> @@ -0,0 +1,24 @@
>> >> >> +/*
>> >> >> + * ARM Semihosting Console Support
>> >> >> + *
>> >> >> + * Copyright (c) 2019 Linaro Ltd
>> >> >> + *
>> >> >> + * Currently ARM is unique in having support for semihosting support
>> >> >> + * in linux-user. So for now we implement the common console API but
>> >> >> + * just for arm linux-user.
>> >> >> + *
>> >> >> + * SPDX-License-Identifier: GPL-2.0-or-later
>> >> >> + */
>> >> >> +
>> >> >> +#include "qemu/osdep.h"
>> >> >> +#include "cpu.h"
>> >> >> +#include "hw/semihosting/console.h"
>> >> >> +#include "qemu.h"
>> >> >> +
>> >> >> +int qemu_semihosting_console_out(CPUArchState *env, target_ulong addr,
>> >> >> int len)
>> >> >> +{
>> >> >> + void *s = lock_user_string(addr);
>> >> >> + len = write(STDERR_FILENO, s, len ? len : strlen(s));
>> >> >> + unlock_user(s, addr, 0);
>> >> >> + return len;
>> >> >> +}
>> >> >> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
>> >> >> index 9e5a414dd89..253c66b172a 100644
>> >> >> --- a/target/arm/arm-semi.c
>> >> >> +++ b/target/arm/arm-semi.c
>> >> >> @@ -27,6 +27,7 @@
>> >> >>
>> >> >> #include "cpu.h"
>> >> >> #include "hw/semihosting/semihost.h"
>> >> >> +#include "hw/semihosting/console.h"
>> >> >> #ifdef CONFIG_USER_ONLY
>> >> >> #include "qemu.h"
>> >> >>
>> >> >> @@ -314,32 +315,12 @@ target_ulong do_arm_semihosting(CPUARMState *env)
>> >> >> return set_swi_errno(ts, close(arg0));
>> >> >> }
>> >> >> case TARGET_SYS_WRITEC:
>> >> >> - {
>> >> >> - char c;
>> >> >> -
>> >> >> - if (get_user_u8(c, args))
>> >> >> - /* FIXME - should this error code be -TARGET_EFAULT ? */
>> >> >> - return (uint32_t)-1;
>> >> >> - /* Write to debug console. stderr is near enough. */
>> >> >> - if (use_gdb_syscalls()) {
>> >> >> - return arm_gdb_syscall(cpu, arm_semi_cb,
>> >> >> "write,2,%x,1",
>> >> >> args);
>> >> >> - } else {
>> >> >> - return write(STDERR_FILENO, &c, 1);
>> >> >> - }
>> >> >> - }
>> >> >> + {
>> >> >> + qemu_semihosting_console_out(env, args, 1);
>> >> >> + return 0xdeadbeef;
>> >> >> + }
>> >> >> case TARGET_SYS_WRITE0:
>> >> >> - if (!(s = lock_user_string(args)))
>> >> >> - /* FIXME - should this error code be -TARGET_EFAULT ? */
>> >> >> - return (uint32_t)-1;
>> >> >> - len = strlen(s);
>> >> >> - if (use_gdb_syscalls()) {
>> >> >> - return arm_gdb_syscall(cpu, arm_semi_cb, "write,2,%x,%x",
>> >> >> - args, len);
>> >> >> - } else {
>> >> >> - ret = write(STDERR_FILENO, s, len);
>> >> >> - }
>> >> >> - unlock_user(s, args, 0);
>> >> >> - return ret;
>> >> >> + return qemu_semihosting_console_out(env, args, 0);
>> >> >> case TARGET_SYS_WRITE:
>> >> >> GET_ARG(0);
>> >> >> GET_ARG(1);
>> >> >> --
>> >> >> 2.20.1
>> >> >>
>> >> >>
>> >> >
>> >> > Hi Alex,
>> >> >
>> >> > this patch breaks build for softmmu target when CONFIG_SEMIHOSTING is
>> >> > not
>> >> > enabled as qemu_semihosting_console_out
>> >> > is not defined in such case - neither linux-user/arm/semihost.c nor
>> >> > hw/semihosting/console.c compiled and function
>> >> > is not in stubs/semihost.c
>> >>
>> >> How do you do that? I tried ../../configure --without-default-devices
>> >> and that still builds for me.
>> >
>> > It's usual RHEL way - use --without-default-devices and use specific
>> > list of enabled devices (this mean disable CONFIG_SEMIHOSTING in
>> > default_config/* file).
>>
>> OK - so from the upstream source tree CONFIG_SEMIHOSTING is still =y
>> (but I can see most of them are now =n). Isn't the simplest solution to
>> fix-up your version of the default_config file to include SEMIHOSTING?
>>
>
> It's fix but it goes against our policy of handling CONFIG options so we
> would prefer to have this fixed - otherwise there's no meaning in having
> config option if you can't disable it.
Is that what it means? For my part it means we don't build in
CONFIG_SEMIHOSTING for the arches that don't need it (which we were
before). Granted it only really simplified the vl.c parsing and dropped
support for semihosting for the linux-user targets (except ARM).
>
>> Is this an out-of-tree RHEL addition?
>>
>
> No, it's RHEL device config handling.
I mean how do I replicate this failure on the upstream source tree?
>
> Mirek
>
>> >>
>> >> But I suspect what's needed is to change:
>> >>
>> >> #ifndef CONFIG_USER_ONLY
>> >>
>> >> to
>> >>
>> >> #ifdef CONFIG_SEMIHOSTING
>> >>
>> >> to the relevant headers and helper bits.
>> >
>> > Yeah, have to find out what are relevant pieces.
>> >
>> > Mirek
>> >
>> >>
>> >> >
>> >> > Mirek
>> >>
>> >>
>> >> --
>> >> Alex Bennée
>> >>
>>
>>
>> --
>> Alex Bennée
>>
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
2019-05-31 14:28 ` Alex Bennée
@ 2019-05-31 14:38 ` Peter Maydell
2019-05-31 16:47 ` Miroslav Rezanina
2019-05-31 16:50 ` Miroslav Rezanina
1 sibling, 1 reply; 34+ messages in thread
From: Peter Maydell @ 2019-05-31 14:38 UTC (permalink / raw)
To: Alex Bennée
Cc: Miroslav Rezanina, Riku Voipio, QEMU Developers, qemu-arm,
Laurent Vivier
On Fri, 31 May 2019 at 15:28, Alex Bennée <alex.bennee@linaro.org> wrote:
> Miroslav Rezanina <mrezanin@redhat.com> writes:
> >From: "Alex Bennée" <alex.bennee@linaro.org>
> >> OK - so from the upstream source tree CONFIG_SEMIHOSTING is still =y
> >> (but I can see most of them are now =n). Isn't the simplest solution to
> >> fix-up your version of the default_config file to include SEMIHOSTING?
> >>
> >
> > It's fix but it goes against our policy of handling CONFIG options so we
> > would prefer to have this fixed - otherwise there's no meaning in having
> > config option if you can't disable it.
>
> Is that what it means? For my part it means we don't build in
> CONFIG_SEMIHOSTING for the arches that don't need it (which we were
> before). Granted it only really simplified the vl.c parsing and dropped
> support for semihosting for the linux-user targets (except ARM).
Yes, that would be my interpretation of it. If we had
a 'config FOO' stanza for CPUs, then Arm CPUs would
"select SEMIHOSTING". If RedHat would like it to be possible
to build Arm CPUs without CONFIG_SEMIHOSTING then they're
free to submit patches for that, but that's a new feature
upstream doesn't currently support, not a bug in upstream.
(Also I'd be a bit dubious because it means that previously working
guest setups that use semihosting will break.)
PS: if we had a 'config FOO' stanza for CPUs that would also
allow us to say "building Arm CPUs requires the NVIC" and
similarly for things which in QEMU are devices but which are
architecturally tightly-coupled non-optional parts of the CPU.
thanks
-- PMM
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
2019-05-31 14:38 ` Peter Maydell
@ 2019-05-31 16:47 ` Miroslav Rezanina
0 siblings, 0 replies; 34+ messages in thread
From: Miroslav Rezanina @ 2019-05-31 16:47 UTC (permalink / raw)
To: Peter Maydell
Cc: Riku Voipio, Alex Bennée, QEMU Developers, qemu-arm, Laurent Vivier
----- Original Message -----
> From: "Peter Maydell" <peter.maydell@linaro.org>
> To: "Alex Bennée" <alex.bennee@linaro.org>
> Cc: "Miroslav Rezanina" <mrezanin@redhat.com>, "QEMU Developers" <qemu-devel@nongnu.org>, "Riku Voipio"
> <riku.voipio@iki.fi>, "qemu-arm" <qemu-arm@nongnu.org>, "Laurent Vivier" <laurent@vivier.eu>
> Sent: Friday, May 31, 2019 4:38:04 PM
> Subject: Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
>
> On Fri, 31 May 2019 at 15:28, Alex Bennée <alex.bennee@linaro.org> wrote:
> > Miroslav Rezanina <mrezanin@redhat.com> writes:
> > >From: "Alex Bennée" <alex.bennee@linaro.org>
> > >> OK - so from the upstream source tree CONFIG_SEMIHOSTING is still =y
> > >> (but I can see most of them are now =n). Isn't the simplest solution to
> > >> fix-up your version of the default_config file to include SEMIHOSTING?
> > >>
> > >
> > > It's fix but it goes against our policy of handling CONFIG options so we
> > > would prefer to have this fixed - otherwise there's no meaning in having
> > > config option if you can't disable it.
> >
> > Is that what it means? For my part it means we don't build in
> > CONFIG_SEMIHOSTING for the arches that don't need it (which we were
> > before). Granted it only really simplified the vl.c parsing and dropped
> > support for semihosting for the linux-user targets (except ARM).
>
> Yes, that would be my interpretation of it. If we had
> a 'config FOO' stanza for CPUs, then Arm CPUs would
> "select SEMIHOSTING". If RedHat would like it to be possible
> to build Arm CPUs without CONFIG_SEMIHOSTING then they're
> free to submit patches for that, but that's a new feature
> upstream doesn't currently support, not a bug in upstream.
> (Also I'd be a bit dubious because it means that previously working
> guest setups that use semihosting will break.)
I partially agree here - I see difference between disabling
config and omitting it. We are not not disabling CONFIG_SEMIHOSTING,
we just ignore it. So we got error because it is not properly handled.
Proper handling should be either auto-include it as dependency or
successful build with option disabled.
As there's currently no way to auto-include it through dependency,
it would be good to have comment in default_config file next to it stating
that it's required option. This will allow us to see it and
add to our default_config we used instead upstream one.
Mirek
>
> PS: if we had a 'config FOO' stanza for CPUs that would also
> allow us to say "building Arm CPUs requires the NVIC" and
> similarly for things which in QEMU are devices but which are
> architecturally tightly-coupled non-optional parts of the CPU.
>
> thanks
> -- PMM
>
--
Miroslav Rezanina
Software Engineer - Virtualization Team Maintainer
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
2019-05-31 14:28 ` Alex Bennée
2019-05-31 14:38 ` Peter Maydell
@ 2019-05-31 16:50 ` Miroslav Rezanina
1 sibling, 0 replies; 34+ messages in thread
From: Miroslav Rezanina @ 2019-05-31 16:50 UTC (permalink / raw)
To: Alex Bennée
Cc: Peter Maydell, Riku Voipio, qemu-devel, qemu-arm, Laurent Vivier
----- Original Message -----
> From: "Alex Bennée" <alex.bennee@linaro.org>
> To: "Miroslav Rezanina" <mrezanin@redhat.com>
> Cc: qemu-devel@nongnu.org, "Peter Maydell" <peter.maydell@linaro.org>, "Riku Voipio" <riku.voipio@iki.fi>,
> qemu-arm@nongnu.org, "Laurent Vivier" <laurent@vivier.eu>
> Sent: Friday, May 31, 2019 4:28:02 PM
> Subject: Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi
>
>
> Miroslav Rezanina <mrezanin@redhat.com> writes:
>
> > ----- Original Message -----
> >> From: "Alex Bennée" <alex.bennee@linaro.org>
> >> To: "Miroslav Rezanina" <mrezanin@redhat.com>
> >> Cc: qemu-devel@nongnu.org, "Peter Maydell" <peter.maydell@linaro.org>,
> >> "Riku Voipio" <riku.voipio@iki.fi>,
> >> qemu-arm@nongnu.org, "Laurent Vivier" <laurent@vivier.eu>
> >> Sent: Friday, May 31, 2019 3:16:38 PM
> >> Subject: Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common
> >> interface for WRITE0/WRITEC in arm-semi
> >>
> >>
> >> Miroslav Rezanina <mrezanin@redhat.com> writes:
> >>
> >> > ----- Original Message -----
> >> >> From: "Alex Bennée" <alex.bennee@linaro.org>
> >> >> To: "Miroslav Rezanina" <mrezanin@redhat.com>
> >> >> Cc: qemu-devel@nongnu.org, "Peter Maydell" <peter.maydell@linaro.org>,
> >> >> "Riku Voipio" <riku.voipio@iki.fi>,
> >> >> qemu-arm@nongnu.org, "Laurent Vivier" <laurent@vivier.eu>
> >> >> Sent: Friday, May 31, 2019 1:08:06 PM
> >> >> Subject: Re: [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common
> >> >> interface for WRITE0/WRITEC in arm-semi
> >> >>
> >> >>
> >> >> Miroslav Rezanina <mrezanin@redhat.com> writes:
> >> >>
> >> >> > On Tue, May 14, 2019 at 04:52:56PM +0100, Alex Bennée wrote:
> >> >> >> Now we have a common semihosting console interface use that for our
> >> >> >> string output. However ARM is currently unique in also supporting
> >> >> >> semihosting for linux-user so we need to replicate the API in
> >> >> >> linux-user. If other architectures gain this support we can move the
> >> >> >> file later.
> >> >> >>
> >> >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >> >> >> ---
> >> >> >> linux-user/Makefile.objs | 2 ++
> >> >> >> linux-user/arm/semihost.c | 24 ++++++++++++++++++++++++
> >> >> >> target/arm/arm-semi.c | 31 ++++++-------------------------
> >> >> >> 3 files changed, 32 insertions(+), 25 deletions(-)
> >> >> >> create mode 100644 linux-user/arm/semihost.c
> >> >> >>
> >> >> >> diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
> >> >> >> index 769b8d83362..285c5dfa17a 100644
> >> >> >> --- a/linux-user/Makefile.objs
> >> >> >> +++ b/linux-user/Makefile.objs
> >> >> >> @@ -6,4 +6,6 @@ obj-y = main.o syscall.o strace.o mmap.o signal.o \
> >> >> >> obj-$(TARGET_HAS_BFLT) += flatload.o
> >> >> >> obj-$(TARGET_I386) += vm86.o
> >> >> >> obj-$(TARGET_ARM) += arm/nwfpe/
> >> >> >> +obj-$(TARGET_ARM) += arm/semihost.o
> >> >> >> +obj-$(TARGET_AARCH64) += arm/semihost.o
> >> >> >> obj-$(TARGET_M68K) += m68k-sim.o
> >> >> >> diff --git a/linux-user/arm/semihost.c b/linux-user/arm/semihost.c
> >> >> >> new file mode 100644
> >> >> >> index 00000000000..9554102a855
> >> >> >> --- /dev/null
> >> >> >> +++ b/linux-user/arm/semihost.c
> >> >> >> @@ -0,0 +1,24 @@
> >> >> >> +/*
> >> >> >> + * ARM Semihosting Console Support
> >> >> >> + *
> >> >> >> + * Copyright (c) 2019 Linaro Ltd
> >> >> >> + *
> >> >> >> + * Currently ARM is unique in having support for semihosting
> >> >> >> support
> >> >> >> + * in linux-user. So for now we implement the common console API
> >> >> >> but
> >> >> >> + * just for arm linux-user.
> >> >> >> + *
> >> >> >> + * SPDX-License-Identifier: GPL-2.0-or-later
> >> >> >> + */
> >> >> >> +
> >> >> >> +#include "qemu/osdep.h"
> >> >> >> +#include "cpu.h"
> >> >> >> +#include "hw/semihosting/console.h"
> >> >> >> +#include "qemu.h"
> >> >> >> +
> >> >> >> +int qemu_semihosting_console_out(CPUArchState *env, target_ulong
> >> >> >> addr,
> >> >> >> int len)
> >> >> >> +{
> >> >> >> + void *s = lock_user_string(addr);
> >> >> >> + len = write(STDERR_FILENO, s, len ? len : strlen(s));
> >> >> >> + unlock_user(s, addr, 0);
> >> >> >> + return len;
> >> >> >> +}
> >> >> >> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
> >> >> >> index 9e5a414dd89..253c66b172a 100644
> >> >> >> --- a/target/arm/arm-semi.c
> >> >> >> +++ b/target/arm/arm-semi.c
> >> >> >> @@ -27,6 +27,7 @@
> >> >> >>
> >> >> >> #include "cpu.h"
> >> >> >> #include "hw/semihosting/semihost.h"
> >> >> >> +#include "hw/semihosting/console.h"
> >> >> >> #ifdef CONFIG_USER_ONLY
> >> >> >> #include "qemu.h"
> >> >> >>
> >> >> >> @@ -314,32 +315,12 @@ target_ulong do_arm_semihosting(CPUARMState
> >> >> >> *env)
> >> >> >> return set_swi_errno(ts, close(arg0));
> >> >> >> }
> >> >> >> case TARGET_SYS_WRITEC:
> >> >> >> - {
> >> >> >> - char c;
> >> >> >> -
> >> >> >> - if (get_user_u8(c, args))
> >> >> >> - /* FIXME - should this error code be -TARGET_EFAULT ?
> >> >> >> */
> >> >> >> - return (uint32_t)-1;
> >> >> >> - /* Write to debug console. stderr is near enough. */
> >> >> >> - if (use_gdb_syscalls()) {
> >> >> >> - return arm_gdb_syscall(cpu, arm_semi_cb,
> >> >> >> "write,2,%x,1",
> >> >> >> args);
> >> >> >> - } else {
> >> >> >> - return write(STDERR_FILENO, &c, 1);
> >> >> >> - }
> >> >> >> - }
> >> >> >> + {
> >> >> >> + qemu_semihosting_console_out(env, args, 1);
> >> >> >> + return 0xdeadbeef;
> >> >> >> + }
> >> >> >> case TARGET_SYS_WRITE0:
> >> >> >> - if (!(s = lock_user_string(args)))
> >> >> >> - /* FIXME - should this error code be -TARGET_EFAULT ?
> >> >> >> */
> >> >> >> - return (uint32_t)-1;
> >> >> >> - len = strlen(s);
> >> >> >> - if (use_gdb_syscalls()) {
> >> >> >> - return arm_gdb_syscall(cpu, arm_semi_cb,
> >> >> >> "write,2,%x,%x",
> >> >> >> - args, len);
> >> >> >> - } else {
> >> >> >> - ret = write(STDERR_FILENO, s, len);
> >> >> >> - }
> >> >> >> - unlock_user(s, args, 0);
> >> >> >> - return ret;
> >> >> >> + return qemu_semihosting_console_out(env, args, 0);
> >> >> >> case TARGET_SYS_WRITE:
> >> >> >> GET_ARG(0);
> >> >> >> GET_ARG(1);
> >> >> >> --
> >> >> >> 2.20.1
> >> >> >>
> >> >> >>
> >> >> >
> >> >> > Hi Alex,
> >> >> >
> >> >> > this patch breaks build for softmmu target when CONFIG_SEMIHOSTING is
> >> >> > not
> >> >> > enabled as qemu_semihosting_console_out
> >> >> > is not defined in such case - neither linux-user/arm/semihost.c nor
> >> >> > hw/semihosting/console.c compiled and function
> >> >> > is not in stubs/semihost.c
> >> >>
> >> >> How do you do that? I tried ../../configure --without-default-devices
> >> >> and that still builds for me.
> >> >
> >> > It's usual RHEL way - use --without-default-devices and use specific
> >> > list of enabled devices (this mean disable CONFIG_SEMIHOSTING in
> >> > default_config/* file).
> >>
> >> OK - so from the upstream source tree CONFIG_SEMIHOSTING is still =y
> >> (but I can see most of them are now =n). Isn't the simplest solution to
> >> fix-up your version of the default_config file to include SEMIHOSTING?
> >>
> >
> > It's fix but it goes against our policy of handling CONFIG options so we
> > would prefer to have this fixed - otherwise there's no meaning in having
> > config option if you can't disable it.
>
> Is that what it means? For my part it means we don't build in
> CONFIG_SEMIHOSTING for the arches that don't need it (which we were
> before). Granted it only really simplified the vl.c parsing and dropped
> support for semihosting for the linux-user targets (except ARM).
>
> >
> >> Is this an out-of-tree RHEL addition?
> >>
> >
> > No, it's RHEL device config handling.
>
> I mean how do I replicate this failure on the upstream source tree?
Easiest way should be probably commenting out the CONFIG_SEMIHOSTING=y
line in default_config/arm-softmmu.mak. It is not auto-include so the
build will be run without it and fail.
Mirek
>
> >
> > Mirek
> >
> >> >>
> >> >> But I suspect what's needed is to change:
> >> >>
> >> >> #ifndef CONFIG_USER_ONLY
> >> >>
> >> >> to
> >> >>
> >> >> #ifdef CONFIG_SEMIHOSTING
> >> >>
> >> >> to the relevant headers and helper bits.
> >> >
> >> > Yeah, have to find out what are relevant pieces.
> >> >
> >> > Mirek
> >> >
> >> >>
> >> >> >
> >> >> > Mirek
> >> >>
> >> >>
> >> >> --
> >> >> Alex Bennée
> >> >>
> >>
> >>
> >> --
> >> Alex Bennée
> >>
>
>
> --
> Alex Bennée
>
--
Miroslav Rezanina
Software Engineer - Virtualization Team Maintainer
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [RFC PATCH 07/11] target/arm: add LOG_UNIMP messages to arm-semi
2019-05-14 15:52 [Qemu-devel] [RFC PATCH 00/11] semihosting cleanup and re-factor Alex Bennée
` (5 preceding siblings ...)
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 06/11] target/arm: use the common interface for WRITE0/WRITEC in arm-semi Alex Bennée
@ 2019-05-14 15:52 ` Alex Bennée
2019-05-14 16:15 ` Philippe Mathieu-Daudé
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 08/11] target/arm: correct return values for WRITE/READ in arm-semi Alex Bennée
` (4 subsequent siblings)
11 siblings, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2019-05-14 15:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, qemu-arm, Alex Bennée
Clean-up our unimplemented bits with a proper message.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
target/arm/arm-semi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
index 253c66b172a..a3bbef18ef7 100644
--- a/target/arm/arm-semi.c
+++ b/target/arm/arm-semi.c
@@ -28,6 +28,7 @@
#include "cpu.h"
#include "hw/semihosting/semihost.h"
#include "hw/semihosting/console.h"
+#include "qemu/log.h"
#ifdef CONFIG_USER_ONLY
#include "qemu.h"
@@ -366,7 +367,7 @@ target_ulong do_arm_semihosting(CPUARMState *env)
return len - ret;
}
case TARGET_SYS_READC:
- /* XXX: Read from debug console. Not implemented. */
+ qemu_log_mask(LOG_UNIMP, "%s: SYS_READC not implemented", __func__);
return 0;
case TARGET_SYS_ISTTY:
GET_ARG(0);
@@ -400,7 +401,7 @@ target_ulong do_arm_semihosting(CPUARMState *env)
return buf.st_size;
}
case TARGET_SYS_TMPNAM:
- /* XXX: Not implemented. */
+ qemu_log_mask(LOG_UNIMP, "%s: SYS_TMPNAM not implemented", __func__);
return -1;
case TARGET_SYS_REMOVE:
GET_ARG(0);
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 07/11] target/arm: add LOG_UNIMP messages to arm-semi
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 07/11] target/arm: add LOG_UNIMP messages to arm-semi Alex Bennée
@ 2019-05-14 16:15 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-14 16:15 UTC (permalink / raw)
To: Alex Bennée, qemu-devel; +Cc: Peter Maydell, qemu-arm
On 5/14/19 5:52 PM, Alex Bennée wrote:
> Clean-up our unimplemented bits with a proper message.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> target/arm/arm-semi.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
> index 253c66b172a..a3bbef18ef7 100644
> --- a/target/arm/arm-semi.c
> +++ b/target/arm/arm-semi.c
> @@ -28,6 +28,7 @@
> #include "cpu.h"
> #include "hw/semihosting/semihost.h"
> #include "hw/semihosting/console.h"
> +#include "qemu/log.h"
> #ifdef CONFIG_USER_ONLY
> #include "qemu.h"
>
> @@ -366,7 +367,7 @@ target_ulong do_arm_semihosting(CPUARMState *env)
> return len - ret;
> }
> case TARGET_SYS_READC:
> - /* XXX: Read from debug console. Not implemented. */
> + qemu_log_mask(LOG_UNIMP, "%s: SYS_READC not implemented", __func__);
> return 0;
> case TARGET_SYS_ISTTY:
> GET_ARG(0);
> @@ -400,7 +401,7 @@ target_ulong do_arm_semihosting(CPUARMState *env)
> return buf.st_size;
> }
> case TARGET_SYS_TMPNAM:
> - /* XXX: Not implemented. */
> + qemu_log_mask(LOG_UNIMP, "%s: SYS_TMPNAM not implemented", __func__);
> return -1;
> case TARGET_SYS_REMOVE:
> GET_ARG(0);
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [RFC PATCH 08/11] target/arm: correct return values for WRITE/READ in arm-semi
2019-05-14 15:52 [Qemu-devel] [RFC PATCH 00/11] semihosting cleanup and re-factor Alex Bennée
` (6 preceding siblings ...)
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 07/11] target/arm: add LOG_UNIMP messages to arm-semi Alex Bennée
@ 2019-05-14 15:52 ` Alex Bennée
2019-05-14 16:57 ` Philippe Mathieu-Daudé
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 09/11] target/mips: only build mips-semi for softmmu Alex Bennée
` (3 subsequent siblings)
11 siblings, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2019-05-14 15:52 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, qemu-arm, Alex Bennée
The documentation says the write should return the number of bytes not
written on an error (0 means everything was written). Read provides a
buffer length and the return value should be the buffer length - bytes
actually read. Remove the incorrect FIXME's and return the correct
values.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
target/arm/arm-semi.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
index a3bbef18ef7..3ae8f05d51b 100644
--- a/target/arm/arm-semi.c
+++ b/target/arm/arm-semi.c
@@ -334,13 +334,15 @@ target_ulong do_arm_semihosting(CPUARMState *env)
} else {
s = lock_user(VERIFY_READ, arg1, len, 1);
if (!s) {
- /* FIXME - should this error code be -TARGET_EFAULT ? */
- return (uint32_t)-1;
+ /* Return bytes not written on error */
+ return len;
}
ret = set_swi_errno(ts, write(arg0, s, len));
unlock_user(s, arg1, 0);
- if (ret == (uint32_t)-1)
- return -1;
+ if (ret == (uint32_t)-1) {
+ ret = 0;
+ }
+ /* Return bytes not written */
return len - ret;
}
case TARGET_SYS_READ:
@@ -355,15 +357,17 @@ target_ulong do_arm_semihosting(CPUARMState *env)
} else {
s = lock_user(VERIFY_WRITE, arg1, len, 0);
if (!s) {
- /* FIXME - should this error code be -TARGET_EFAULT ? */
- return (uint32_t)-1;
+ /* return bytes not read */
+ return len;
}
do {
ret = set_swi_errno(ts, read(arg0, s, len));
} while (ret == -1 && errno == EINTR);
unlock_user(s, arg1, len);
- if (ret == (uint32_t)-1)
- return -1;
+ if (ret == (uint32_t)-1) {
+ ret = 0;
+ }
+ /* Return bytes not read */
return len - ret;
}
case TARGET_SYS_READC:
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 08/11] target/arm: correct return values for WRITE/READ in arm-semi
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 08/11] target/arm: correct return values for WRITE/READ in arm-semi Alex Bennée
@ 2019-05-14 16:57 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-14 16:57 UTC (permalink / raw)
To: Alex Bennée, qemu-devel; +Cc: Peter Maydell, qemu-arm
On 5/14/19 5:52 PM, Alex Bennée wrote:
> The documentation says the write should return the number of bytes not
> written on an error (0 means everything was written). Read provides a
> buffer length and the return value should be the buffer length - bytes
> actually read. Remove the incorrect FIXME's and return the correct
> values.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> target/arm/arm-semi.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
> index a3bbef18ef7..3ae8f05d51b 100644
> --- a/target/arm/arm-semi.c
> +++ b/target/arm/arm-semi.c
> @@ -334,13 +334,15 @@ target_ulong do_arm_semihosting(CPUARMState *env)
> } else {
> s = lock_user(VERIFY_READ, arg1, len, 1);
> if (!s) {
> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> - return (uint32_t)-1;
> + /* Return bytes not written on error */
> + return len;
> }
> ret = set_swi_errno(ts, write(arg0, s, len));
> unlock_user(s, arg1, 0);
> - if (ret == (uint32_t)-1)
> - return -1;
> + if (ret == (uint32_t)-1) {
> + ret = 0;
> + }
> + /* Return bytes not written */
> return len - ret;
> }
> case TARGET_SYS_READ:
> @@ -355,15 +357,17 @@ target_ulong do_arm_semihosting(CPUARMState *env)
> } else {
> s = lock_user(VERIFY_WRITE, arg1, len, 0);
> if (!s) {
> - /* FIXME - should this error code be -TARGET_EFAULT ? */
> - return (uint32_t)-1;
> + /* return bytes not read */
> + return len;
> }
> do {
> ret = set_swi_errno(ts, read(arg0, s, len));
> } while (ret == -1 && errno == EINTR);
> unlock_user(s, arg1, len);
> - if (ret == (uint32_t)-1)
> - return -1;
> + if (ret == (uint32_t)-1) {
> + ret = 0;
> + }
> + /* Return bytes not read */
> return len - ret;
> }
> case TARGET_SYS_READC:
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [RFC PATCH 09/11] target/mips: only build mips-semi for softmmu
2019-05-14 15:52 [Qemu-devel] [RFC PATCH 00/11] semihosting cleanup and re-factor Alex Bennée
` (7 preceding siblings ...)
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 08/11] target/arm: correct return values for WRITE/READ in arm-semi Alex Bennée
@ 2019-05-14 15:52 ` Alex Bennée
2019-05-14 16:59 ` Philippe Mathieu-Daudé
2019-05-20 15:53 ` Aleksandar Markovic
2019-05-14 15:53 ` [Qemu-devel] [RFC PATCH 10/11] target/mips: convert UHI_plog to use common semihosting code Alex Bennée
` (2 subsequent siblings)
11 siblings, 2 replies; 34+ messages in thread
From: Alex Bennée @ 2019-05-14 15:52 UTC (permalink / raw)
To: qemu-devel
Cc: Aleksandar Rikalo, qemu-arm, Alex Bennée,
Aleksandar Markovic, Aurelien Jarno
The is_uhi gates all semihosting calls and always returns false for
CONFIG_USER_ONLY builds. There is no reason to build and link
mips-semi for these builds so lets fix that.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
target/mips/Makefile.objs | 3 ++-
target/mips/helper.h | 2 ++
target/mips/translate.c | 8 ++++++++
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/target/mips/Makefile.objs b/target/mips/Makefile.objs
index 651f36f5176..3448ad5e193 100644
--- a/target/mips/Makefile.objs
+++ b/target/mips/Makefile.objs
@@ -1,4 +1,5 @@
obj-y += translate.o dsp_helper.o op_helper.o lmi_helper.o helper.o cpu.o
-obj-y += gdbstub.o msa_helper.o mips-semi.o
+obj-y += gdbstub.o msa_helper.o
+obj-$(CONFIG_SOFTMMU) += mips-semi.o
obj-$(CONFIG_SOFTMMU) += machine.o cp0_timer.o
obj-$(CONFIG_KVM) += kvm.o
diff --git a/target/mips/helper.h b/target/mips/helper.h
index a6d687e3405..90ab03b76e3 100644
--- a/target/mips/helper.h
+++ b/target/mips/helper.h
@@ -2,7 +2,9 @@ DEF_HELPER_3(raise_exception_err, noreturn, env, i32, int)
DEF_HELPER_2(raise_exception, noreturn, env, i32)
DEF_HELPER_1(raise_exception_debug, noreturn, env)
+#ifndef CONFIG_USER_ONLY
DEF_HELPER_1(do_semihosting, void, env)
+#endif
#ifdef TARGET_MIPS64
DEF_HELPER_4(sdl, void, env, tl, tl, int)
diff --git a/target/mips/translate.c b/target/mips/translate.c
index 3cd5b11b16b..fb905c88b57 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -13726,6 +13726,14 @@ static inline bool is_uhi(int sdbbp_code)
#endif
}
+#ifdef CONFIG_USER_ONLY
+/* The above should dead-code away any calls to this..*/
+static inline void gen_helper_do_semihosting(void *env)
+{
+ g_assert_not_reached();
+}
+#endif
+
static int decode_mips16_opc (CPUMIPSState *env, DisasContext *ctx)
{
int rx, ry;
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 09/11] target/mips: only build mips-semi for softmmu
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 09/11] target/mips: only build mips-semi for softmmu Alex Bennée
@ 2019-05-14 16:59 ` Philippe Mathieu-Daudé
2019-05-20 15:53 ` Aleksandar Markovic
1 sibling, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-14 16:59 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: Aleksandar Rikalo, qemu-arm, Aurelien Jarno, Aleksandar Markovic
On 5/14/19 5:52 PM, Alex Bennée wrote:
> The is_uhi gates all semihosting calls and always returns false for
> CONFIG_USER_ONLY builds. There is no reason to build and link
> mips-semi for these builds so lets fix that.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> target/mips/Makefile.objs | 3 ++-
> target/mips/helper.h | 2 ++
> target/mips/translate.c | 8 ++++++++
> 3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/target/mips/Makefile.objs b/target/mips/Makefile.objs
> index 651f36f5176..3448ad5e193 100644
> --- a/target/mips/Makefile.objs
> +++ b/target/mips/Makefile.objs
> @@ -1,4 +1,5 @@
> obj-y += translate.o dsp_helper.o op_helper.o lmi_helper.o helper.o cpu.o
> -obj-y += gdbstub.o msa_helper.o mips-semi.o
> +obj-y += gdbstub.o msa_helper.o
> +obj-$(CONFIG_SOFTMMU) += mips-semi.o
> obj-$(CONFIG_SOFTMMU) += machine.o cp0_timer.o
> obj-$(CONFIG_KVM) += kvm.o
> diff --git a/target/mips/helper.h b/target/mips/helper.h
> index a6d687e3405..90ab03b76e3 100644
> --- a/target/mips/helper.h
> +++ b/target/mips/helper.h
> @@ -2,7 +2,9 @@ DEF_HELPER_3(raise_exception_err, noreturn, env, i32, int)
> DEF_HELPER_2(raise_exception, noreturn, env, i32)
> DEF_HELPER_1(raise_exception_debug, noreturn, env)
>
> +#ifndef CONFIG_USER_ONLY
> DEF_HELPER_1(do_semihosting, void, env)
> +#endif
>
> #ifdef TARGET_MIPS64
> DEF_HELPER_4(sdl, void, env, tl, tl, int)
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 3cd5b11b16b..fb905c88b57 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -13726,6 +13726,14 @@ static inline bool is_uhi(int sdbbp_code)
> #endif
> }
>
> +#ifdef CONFIG_USER_ONLY
> +/* The above should dead-code away any calls to this..*/
> +static inline void gen_helper_do_semihosting(void *env)
> +{
> + g_assert_not_reached();
> +}
> +#endif
> +
> static int decode_mips16_opc (CPUMIPSState *env, DisasContext *ctx)
> {
> int rx, ry;
>
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 09/11] target/mips: only build mips-semi for softmmu
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 09/11] target/mips: only build mips-semi for softmmu Alex Bennée
2019-05-14 16:59 ` Philippe Mathieu-Daudé
@ 2019-05-20 15:53 ` Aleksandar Markovic
1 sibling, 0 replies; 34+ messages in thread
From: Aleksandar Markovic @ 2019-05-20 15:53 UTC (permalink / raw)
To: Alex Bennée
Cc: Aleksandar Rikalo, Aurelien Jarno, qemu-arm, qemu-devel,
Aleksandar Markovic
On May 14, 2019 6:04 PM, "Alex Bennée" <alex.bennee@linaro.org> wrote:
>
> The is_uhi gates all semihosting calls and always returns false for
> CONFIG_USER_ONLY builds. There is no reason to build and link
> mips-semi for these builds so lets fix that.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> target/mips/Makefile.objs | 3 ++-
> target/mips/helper.h | 2 ++
> target/mips/translate.c | 8 ++++++++
> 3 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/target/mips/Makefile.objs b/target/mips/Makefile.objs
> index 651f36f5176..3448ad5e193 100644
> --- a/target/mips/Makefile.objs
> +++ b/target/mips/Makefile.objs
> @@ -1,4 +1,5 @@
> obj-y += translate.o dsp_helper.o op_helper.o lmi_helper.o helper.o cpu.o
> -obj-y += gdbstub.o msa_helper.o mips-semi.o
> +obj-y += gdbstub.o msa_helper.o
> +obj-$(CONFIG_SOFTMMU) += mips-semi.o
> obj-$(CONFIG_SOFTMMU) += machine.o cp0_timer.o
> obj-$(CONFIG_KVM) += kvm.o
> diff --git a/target/mips/helper.h b/target/mips/helper.h
> index a6d687e3405..90ab03b76e3 100644
> --- a/target/mips/helper.h
> +++ b/target/mips/helper.h
> @@ -2,7 +2,9 @@ DEF_HELPER_3(raise_exception_err, noreturn, env, i32, int)
> DEF_HELPER_2(raise_exception, noreturn, env, i32)
> DEF_HELPER_1(raise_exception_debug, noreturn, env)
>
> +#ifndef CONFIG_USER_ONLY
> DEF_HELPER_1(do_semihosting, void, env)
> +#endif
>
> #ifdef TARGET_MIPS64
> DEF_HELPER_4(sdl, void, env, tl, tl, int)
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 3cd5b11b16b..fb905c88b57 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -13726,6 +13726,14 @@ static inline bool is_uhi(int sdbbp_code)
> #endif
> }
>
> +#ifdef CONFIG_USER_ONLY
> +/* The above should dead-code away any calls to this..*/
> +static inline void gen_helper_do_semihosting(void *env)
> +{
> + g_assert_not_reached();
> +}
> +#endif
> +
> static int decode_mips16_opc (CPUMIPSState *env, DisasContext *ctx)
> {
> int rx, ry;
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [RFC PATCH 10/11] target/mips: convert UHI_plog to use common semihosting code
2019-05-14 15:52 [Qemu-devel] [RFC PATCH 00/11] semihosting cleanup and re-factor Alex Bennée
` (8 preceding siblings ...)
2019-05-14 15:52 ` [Qemu-devel] [RFC PATCH 09/11] target/mips: only build mips-semi for softmmu Alex Bennée
@ 2019-05-14 15:53 ` Alex Bennée
2019-05-20 15:53 ` Aleksandar Markovic
2019-05-14 15:53 ` [Qemu-devel] [RFC PATCH 11/11] MAINTAINERS: update for semihostings new home Alex Bennée
2019-05-20 13:03 ` [Qemu-devel] [RFC PATCH 00/11] semihosting cleanup and re-factor Alex Bennée
11 siblings, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2019-05-14 15:53 UTC (permalink / raw)
To: qemu-devel
Cc: Aleksandar Rikalo, qemu-arm, Alex Bennée,
Aleksandar Markovic, Aurelien Jarno
Rather than printing directly to stdout lets use our common
semihosting code. There is one minor difference in that the output
currently defaults to stderr instead of stdout however this can be
controlled by connecting semihosting to a chardev.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
target/mips/mips-semi.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/mips/mips-semi.c b/target/mips/mips-semi.c
index eac8374fb34..35bdfd7c77e 100644
--- a/target/mips/mips-semi.c
+++ b/target/mips/mips-semi.c
@@ -23,6 +23,7 @@
#include "exec/helper-proto.h"
#include "exec/softmmu-semi.h"
#include "hw/semihosting/semihost.h"
+#include "hw/semihosting/console.h"
typedef enum UHIOp {
UHI_exit = 1,
@@ -329,13 +330,12 @@ void helper_do_semihosting(CPUMIPSState *env)
p2 = strstr(p, "%d");
if (p2) {
int char_num = p2 - p;
- char *buf = g_malloc(char_num + 1);
- strncpy(buf, p, char_num);
- buf[char_num] = '\0';
- gpr[2] = printf("%s%d%s", buf, (int)gpr[5], p2 + 2);
- g_free(buf);
+ GString *s = g_string_new_len(p, char_num);
+ g_string_append_printf(s, "%d%s", (int)gpr[5], p2 + 2);
+ gpr[2] = qemu_semihosting_log_out(s->str, s->len);
+ g_string_free(s, true);
} else {
- gpr[2] = printf("%s", p);
+ gpr[2] = qemu_semihosting_log_out(p, strlen(p));
}
FREE_TARGET_STRING(p, gpr[4]);
break;
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 10/11] target/mips: convert UHI_plog to use common semihosting code
2019-05-14 15:53 ` [Qemu-devel] [RFC PATCH 10/11] target/mips: convert UHI_plog to use common semihosting code Alex Bennée
@ 2019-05-20 15:53 ` Aleksandar Markovic
0 siblings, 0 replies; 34+ messages in thread
From: Aleksandar Markovic @ 2019-05-20 15:53 UTC (permalink / raw)
To: Alex Bennée
Cc: Aleksandar Rikalo, Aurelien Jarno, qemu-arm, qemu-devel,
Aleksandar Markovic
On May 14, 2019 6:02 PM, "Alex Bennée" <alex.bennee@linaro.org> wrote:
>
> Rather than printing directly to stdout lets use our common
> semihosting code. There is one minor difference in that the output
> currently defaults to stderr instead of stdout however this can be
> controlled by connecting semihosting to a chardev.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> target/mips/mips-semi.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/target/mips/mips-semi.c b/target/mips/mips-semi.c
> index eac8374fb34..35bdfd7c77e 100644
> --- a/target/mips/mips-semi.c
> +++ b/target/mips/mips-semi.c
> @@ -23,6 +23,7 @@
> #include "exec/helper-proto.h"
> #include "exec/softmmu-semi.h"
> #include "hw/semihosting/semihost.h"
> +#include "hw/semihosting/console.h"
>
> typedef enum UHIOp {
> UHI_exit = 1,
> @@ -329,13 +330,12 @@ void helper_do_semihosting(CPUMIPSState *env)
> p2 = strstr(p, "%d");
> if (p2) {
> int char_num = p2 - p;
> - char *buf = g_malloc(char_num + 1);
> - strncpy(buf, p, char_num);
> - buf[char_num] = '\0';
> - gpr[2] = printf("%s%d%s", buf, (int)gpr[5], p2 + 2);
> - g_free(buf);
> + GString *s = g_string_new_len(p, char_num);
> + g_string_append_printf(s, "%d%s", (int)gpr[5], p2 + 2);
> + gpr[2] = qemu_semihosting_log_out(s->str, s->len);
> + g_string_free(s, true);
> } else {
> - gpr[2] = printf("%s", p);
> + gpr[2] = qemu_semihosting_log_out(p, strlen(p));
> }
> FREE_TARGET_STRING(p, gpr[4]);
> break;
> --
> 2.20.1
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [RFC PATCH 11/11] MAINTAINERS: update for semihostings new home
2019-05-14 15:52 [Qemu-devel] [RFC PATCH 00/11] semihosting cleanup and re-factor Alex Bennée
` (9 preceding siblings ...)
2019-05-14 15:53 ` [Qemu-devel] [RFC PATCH 10/11] target/mips: convert UHI_plog to use common semihosting code Alex Bennée
@ 2019-05-14 15:53 ` Alex Bennée
2019-05-14 17:00 ` Philippe Mathieu-Daudé
2019-05-20 13:03 ` [Qemu-devel] [RFC PATCH 00/11] semihosting cleanup and re-factor Alex Bennée
11 siblings, 1 reply; 34+ messages in thread
From: Alex Bennée @ 2019-05-14 15:53 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-arm, Alex Bennée
Seeing as I touched it I should at least keep an eye on it.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index a73a61a5465..e9050beb222 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2559,6 +2559,13 @@ F: docs/pvrdma.txt
F: contrib/rdmacm-mux/*
F: qapi/rdma.json
+Semihosting
+M: Alex Bennée <alex.bennee@linaro.org>
+L: qemu-devel@nongnu.org
+S: Maintained
+F: hw/semihosting/
+F: include/hw/semihosting/
+
Build and test automation
-------------------------
Build and test automation
--
2.20.1
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [RFC PATCH 11/11] MAINTAINERS: update for semihostings new home
2019-05-14 15:53 ` [Qemu-devel] [RFC PATCH 11/11] MAINTAINERS: update for semihostings new home Alex Bennée
@ 2019-05-14 17:00 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 34+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-14 17:00 UTC (permalink / raw)
To: Alex Bennée, qemu-devel; +Cc: qemu-arm
On 5/14/19 5:53 PM, Alex Bennée wrote:
> Seeing as I touched it I should at least keep an eye on it.
Thanks for that.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
FWIW:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> MAINTAINERS | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a73a61a5465..e9050beb222 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2559,6 +2559,13 @@ F: docs/pvrdma.txt
> F: contrib/rdmacm-mux/*
> F: qapi/rdma.json
>
> +Semihosting
> +M: Alex Bennée <alex.bennee@linaro.org>
> +L: qemu-devel@nongnu.org
> +S: Maintained
> +F: hw/semihosting/
> +F: include/hw/semihosting/
> +
> Build and test automation
> -------------------------
> Build and test automation
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [RFC PATCH 00/11] semihosting cleanup and re-factor
2019-05-14 15:52 [Qemu-devel] [RFC PATCH 00/11] semihosting cleanup and re-factor Alex Bennée
` (10 preceding siblings ...)
2019-05-14 15:53 ` [Qemu-devel] [RFC PATCH 11/11] MAINTAINERS: update for semihostings new home Alex Bennée
@ 2019-05-20 13:03 ` Alex Bennée
11 siblings, 0 replies; 34+ messages in thread
From: Alex Bennée @ 2019-05-20 13:03 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Aleksandar Rikalo, riku.voipio, Laurent Vivier,
qemu-arm, Aleksandar Markovic, Aurelien Jarno
Alex Bennée <alex.bennee@linaro.org> writes:
> Hi,
>
> This collects together some fixes from last weeks RFC clean-up patch
> as well as a generalised version of the chardev console tweak I made
> last week for testing/next. As it happens it only really made sense
> for ARM and MIPs logging semihost calls but there is certainly scope
> for handling all the semihost syscalls in a more common way. I didn't
> make the changes to xtensa as that already has a bi-directional
> console-via-chardev setup and that would be quite a bit of extra work
> to support.
Ping?
Philippe has already done a bunch of review (no changes suggested so
far) but could ARM/MIPS people look over the changes I made to their
architectures?
> I've added myself to the MAINTAINERS section for the common code but
> my focus at the moment is really just to improve the use of
> semihosting in our expanding system tests. Hopefully this is enough to
> ensure future enhancements (common open/read/write/close?) can be done
> and easily enabled for all semihost targets.
>
> Please review.
>
>
> Alex Bennée (11):
> semihosting: move semihosting configuration into its own directory
> semihosting: introduce CONFIG_SEMIHOSTING
> semihosting: implement a semihosting console
> semihosting: enable chardev backed output for console
> target/arm: fixup some of the commentary for arm-semi
> target/arm: use the common interface for WRITE0/WRITEC in arm-semi
> target/arm: add LOG_UNIMP messages to arm-semi
> target/arm: correct return values for WRITE/READ in arm-semi
> target/mips: only build mips-semi for softmmu
> target/mips: convert UHI_plog to use common semihosting code
> MAINTAINERS: update for semihostings new home
>
> MAINTAINERS | 7 +
> default-configs/arm-softmmu.mak | 1 +
> default-configs/lm32-softmmu.mak | 2 +
> default-configs/m68k-softmmu.mak | 2 +
> default-configs/mips-softmmu-common.mak | 1 +
> default-configs/nios2-softmmu.mak | 2 +
> default-configs/xtensa-softmmu.mak | 2 +
> gdbstub.c | 7 +-
> hw/Kconfig | 1 +
> hw/Makefile.objs | 1 +
> hw/mips/mips_malta.c | 2 +-
> hw/semihosting/Kconfig | 3 +
> hw/semihosting/Makefile.objs | 2 +
> hw/semihosting/config.c | 186 ++++++++++++++++++++
> hw/semihosting/console.c | 77 ++++++++
> include/exec/gdbstub.h | 11 ++
> include/hw/semihosting/console.h | 38 ++++
> include/{exec => hw/semihosting}/semihost.h | 17 +-
> include/sysemu/sysemu.h | 1 +
> linux-user/Makefile.objs | 2 +
> linux-user/arm/semihost.c | 24 +++
> qemu-options.hx | 6 +-
> stubs/Makefile.objs | 1 +
> stubs/semihost.c | 70 ++++++++
> target/arm/arm-semi.c | 98 ++++++-----
> target/arm/helper.c | 2 +-
> target/arm/translate-a64.c | 2 +-
> target/arm/translate.c | 2 +-
> target/lm32/helper.c | 2 +-
> target/m68k/op_helper.c | 2 +-
> target/mips/Makefile.objs | 3 +-
> target/mips/helper.h | 2 +
> target/mips/mips-semi.c | 14 +-
> target/mips/translate.c | 10 +-
> target/nios2/helper.c | 2 +-
> target/xtensa/translate.c | 2 +-
> target/xtensa/xtensa-semi.c | 2 +-
> vl.c | 128 +-------------
> 38 files changed, 545 insertions(+), 192 deletions(-)
> create mode 100644 hw/semihosting/Kconfig
> create mode 100644 hw/semihosting/Makefile.objs
> create mode 100644 hw/semihosting/config.c
> create mode 100644 hw/semihosting/console.c
> create mode 100644 include/hw/semihosting/console.h
> rename include/{exec => hw/semihosting}/semihost.h (78%)
> create mode 100644 linux-user/arm/semihost.c
> create mode 100644 stubs/semihost.c
--
Alex Bennée
^ permalink raw reply [flat|nested] 34+ messages in thread