* [PATCH 1/4] [sim/riscv] Add basic semi-hosting support
@ 2023-10-17 5:53 Jaydeep Patil
2023-10-18 12:37 ` Andrew Burgess
0 siblings, 1 reply; 2+ messages in thread
From: Jaydeep Patil @ 2023-10-17 5:53 UTC (permalink / raw)
To: gdb-patches; +Cc: aburgess, vapier, Joseph Faulls, Bhushan Attarde
[-- Attachment #1: Type: text/plain, Size: 11756 bytes --]
Added support for basic semi-hosting calls OPEN, EXIT and GET_CMDLINE.
Added gdb.arch/riscv-exit-getcmd.c to test it.
---
gdb/testsuite/gdb.arch/riscv-exit-getcmd.c | 6 +
gdb/testsuite/gdb.arch/riscv-exit-getcmd.exp | 31 +++
sim/riscv/riscv-sim.h | 29 +++
sim/riscv/sim-main.c | 205 ++++++++++++++++++-
4 files changed, 266 insertions(+), 5 deletions(-)
create mode 100644 gdb/testsuite/gdb.arch/riscv-exit-getcmd.c
create mode 100644 gdb/testsuite/gdb.arch/riscv-exit-getcmd.exp
diff --git a/gdb/testsuite/gdb.arch/riscv-exit-getcmd.c b/gdb/testsuite/gdb.arch/riscv-exit-getcmd.c
new file mode 100644
index 00000000000..2d396e530f6
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-exit-getcmd.c
@@ -0,0 +1,6 @@
+
+int main (int argc, char **argv) {
+ if (argc != 4)
+ return 1;
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/riscv-exit-getcmd.exp b/gdb/testsuite/gdb.arch/riscv-exit-getcmd.exp
new file mode 100644
index 00000000000..672b3c4aa10
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/riscv-exit-getcmd.exp
@@ -0,0 +1,31 @@
+# Copyright 2020-2022 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see http://www.gnu.org/licenses/.
+
+# Test basic semi-hosting calls SYS_GET_CMDLINE and SYS_EXIT
+
+if {![istarget "riscv*-*-*"]} {
+ verbose "Skipping ${gdb_test_file_name}."
+ return
+}
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
+ {debug quiet}] } {
+ unsupported "failed to compile"
+ return -1
+}
+
+gdb_test "run 1 2 3" ".*Inferior.*process.*exited normally.*"
diff --git a/sim/riscv/riscv-sim.h b/sim/riscv/riscv-sim.h
index 1bc9aa12156..c9e78a756c4 100644
--- a/sim/riscv/riscv-sim.h
+++ b/sim/riscv/riscv-sim.h
@@ -75,4 +75,33 @@ extern void initialize_env (SIM_DESC, const char * const *argv,
#define RISCV_XLEN(cpu) MACH_WORD_BITSIZE (CPU_MACH (cpu))
+#define ApplicationExit 0x20026
+
+#define SYS_OPEN 0x01
+#define SYS_GET_CMDLINE 0x15
+#define SYS_EXIT 0x18
+
+#define GDB_O_RDONLY 0x000
+#define GDB_O_WRONLY 0x001
+#define GDB_O_RDWR 0x002
+#define GDB_O_APPEND 0x008
+#define GDB_O_CREAT 0x200
+#define GDB_O_TRUNC 0x400
+#define GDB_O_BINARY 0
+
+static int gdb_open_modeflags[12] = {
+ GDB_O_RDONLY,
+ GDB_O_RDONLY | GDB_O_BINARY,
+ GDB_O_RDWR,
+ GDB_O_RDWR | GDB_O_BINARY,
+ GDB_O_WRONLY | GDB_O_CREAT | GDB_O_TRUNC,
+ GDB_O_WRONLY | GDB_O_CREAT | GDB_O_TRUNC | GDB_O_BINARY,
+ GDB_O_RDWR | GDB_O_CREAT | GDB_O_TRUNC,
+ GDB_O_RDWR | GDB_O_CREAT | GDB_O_TRUNC | GDB_O_BINARY,
+ GDB_O_WRONLY | GDB_O_CREAT | GDB_O_APPEND,
+ GDB_O_WRONLY | GDB_O_CREAT | GDB_O_APPEND | GDB_O_BINARY,
+ GDB_O_RDWR | GDB_O_CREAT | GDB_O_APPEND,
+ GDB_O_RDWR | GDB_O_CREAT | GDB_O_APPEND | GDB_O_BINARY
+};
+
#endif
diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
index 250791634a1..2b184aea554 100644
--- a/sim/riscv/sim-main.c
+++ b/sim/riscv/sim-main.c
@@ -136,6 +136,155 @@ store_csr (SIM_CPU *cpu, const char *name, int csr, unsigned_word *reg,
TRACE_REGISTER (cpu, "wrote CSR %s = %#" PRIxTW, name, val);
}
+static uintptr_t
+get_core_data (SIM_CPU *cpu, unsigned_word addr, unsigned_word index)
+{
+ uintptr_t param;
+ int xlen = RISCV_XLEN (cpu);
+ struct riscv_sim_cpu *riscv_cpu = RISCV_SIM_CPU (cpu);
+
+ if (xlen == 64)
+ param = sim_core_read_unaligned_8 (cpu, riscv_cpu->pc, read_map,
+ addr + (index * 8));
+ else
+ param = sim_core_read_unaligned_4 (cpu, riscv_cpu->pc, read_map,
+ addr + (index * 4));
+
+ return param;
+}
+
+static void
+set_core_string (SIM_CPU *cpu, unsigned_word core_addr, char *host_buf,
+ int len)
+{
+ struct riscv_sim_cpu *riscv_cpu = RISCV_SIM_CPU (cpu);
+ for (int i = 0; i < len; i++)
+ {
+ sim_core_write_unaligned_1 (cpu, riscv_cpu->pc, write_map,
+ core_addr + i, host_buf[i]);
+ }
+}
+
+static char *
+get_core_string_with_len (SIM_CPU *cpu, unsigned_word addr,
+ unsigned_word len)
+{
+ struct riscv_sim_cpu *riscv_cpu = RISCV_SIM_CPU (cpu);
+ char * str;
+ str = (char *) malloc (len + 1);
+
+ for (int i = 0; i < len; i++)
+ {
+ str[i] = sim_core_read_unaligned_1 (cpu, riscv_cpu->pc, read_map,
+ addr + i);
+ }
+ str[len] = 0;
+
+ return str;
+}
+
+static void
+semihosting_open (SIM_CPU *cpu)
+{
+ uintptr_t fname_addr;
+ uintptr_t flags;
+ uintptr_t fname_len;
+ char *name;
+ struct riscv_sim_cpu *riscv_cpu = RISCV_SIM_CPU (cpu);
+
+ fname_addr = (uintptr_t) get_core_data (cpu, riscv_cpu->a1, 0);
+ flags = (uintptr_t) get_core_data (cpu, riscv_cpu->a1, 1);
+ fname_len = (uintptr_t) get_core_data (cpu, riscv_cpu->a1, 2);
+
+ if (fname_len <= 0)
+ {
+ riscv_cpu->a0 = -1;
+ return;
+ }
+
+ name = get_core_string_with_len (cpu, fname_addr, fname_len);
+ riscv_cpu->a0 = sim_io_open (CPU_STATE (cpu), name,
+ gdb_open_modeflags[flags]);
+ free (name);
+}
+
+static void
+semihosting_exit (SIM_CPU *cpu)
+{
+ uintptr_t app_code, exit_code;
+ SIM_DESC sd = CPU_STATE (cpu);
+ struct riscv_sim_cpu *riscv_cpu = RISCV_SIM_CPU (cpu);
+ if (RISCV_XLEN (cpu) == 32)
+ app_code = riscv_cpu->a1;
+ else
+ {
+ app_code = (uintptr_t) get_core_data (cpu, riscv_cpu->a1, 0);
+ exit_code = (uintptr_t) get_core_data (cpu, riscv_cpu->a1, 1);
+ }
+ if (app_code == ApplicationExit)
+ exit_code = 0;
+ else
+ exit_code = 1;
+ riscv_cpu->a0 = exit_code;
+ sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc, sim_exited, exit_code);
+}
+
+static void
+semihosting_get_cmdline (SIM_CPU *cpu)
+{
+ int i = 0, len = 0, total_len = 0;
+ uintptr_t buf_addr, max_buf_len;
+ SIM_DESC sd = CPU_STATE (cpu);
+ char *space = " ";
+ struct riscv_sim_cpu *riscv_cpu = RISCV_SIM_CPU (cpu);
+
+ char **prog_argv = STATE_PROG_ARGV (sd);
+ if (prog_argv == NULL)
+ {
+ riscv_cpu->a0 = 1; // return non-zero to indicate error
+ return;
+ }
+
+ buf_addr = (uintptr_t) get_core_data (cpu, riscv_cpu->a1, 0);
+ max_buf_len = (uintptr_t) get_core_data (cpu, riscv_cpu->a1, 1);
+
+ while (prog_argv[i])
+ {
+ len = strlen (prog_argv[i]);
+ if ((total_len + len) > max_buf_len)
+ break;
+ set_core_string (cpu, buf_addr, prog_argv[i], len);
+ set_core_string (cpu, buf_addr + len, space, 1);
+ len++; // terminate it with space
+ buf_addr += len;
+ total_len += len;
+ i++;
+ }
+ riscv_cpu->a0 = 0; // no error
+}
+
+static int
+do_semihosting (SIM_CPU *cpu)
+{
+ struct riscv_sim_cpu *riscv_cpu = RISCV_SIM_CPU (cpu);
+ switch (riscv_cpu->a0)
+ {
+ case SYS_OPEN:
+ semihosting_open (cpu);
+ break;
+ case SYS_GET_CMDLINE:
+ semihosting_get_cmdline (cpu);
+ break;
+ case SYS_EXIT:
+ semihosting_exit (cpu);
+ break;
+ default:
+ return -1; // semi-hosting call not supported
+ }
+
+ return 0;
+}
+
static inline unsigned_word
ashiftrt (unsigned_word val, unsigned_word shift)
{
@@ -623,11 +772,56 @@ execute_i (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
TRACE_INSN (cpu, "fence.i;");
break;
case MATCH_EBREAK:
- TRACE_INSN (cpu, "ebreak;");
- /* GDB expects us to step over EBREAK. */
- sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc + 4, sim_stopped,
- SIM_SIGTRAP);
- break;
+ {
+ /* RISC-V semi-hosting call is flagged using these three
+ * instructions
+ * slli zero, zero, 0x1f 0x01f01013
+ * ebreak 0x00100073
+ * srai zero, zero, 0x7 0x40705013
+ * Register a0 holds the system call number and a1 holds the
+ * pointer to parameter buffer. Do not read 4 bytes in one go
+ * as we might read malformed 4 byte instruction. */
+ int iw_len;
+ sim_cia pre_pc = riscv_cpu->pc - 4;
+ unsigned_word pre_iw;
+ pre_iw = sim_core_read_aligned_2 (cpu, pre_pc, exec_map, pre_pc);
+ iw_len = riscv_insn_length (pre_iw);
+ if (iw_len == 4)
+ pre_iw |= ((unsigned_word) sim_core_read_aligned_2 (
+ cpu, pre_pc, exec_map, pre_pc + 2) << 16);
+
+ if (pre_iw == 0x01f01013)
+ {
+ sim_cia post_pc = riscv_cpu->pc + 4;
+ unsigned_word post_iw;
+ post_iw = sim_core_read_aligned_2 (cpu, post_pc, exec_map,
+ post_pc);
+ iw_len = riscv_insn_length (post_iw);
+ if (iw_len == 4)
+ post_iw |= ((unsigned_word) sim_core_read_aligned_2 (
+ cpu, post_pc, exec_map, post_pc + 2) << 16);
+
+ if (post_iw == 0x40705013)
+ {
+ TRACE_INSN (cpu, "semi-hosting a0=%lx,a1=%lx;",
+ riscv_cpu->a0, riscv_cpu->a1);
+ if (do_semihosting (cpu))
+ {
+ /* Invalid semi-hosting call. */
+ TRACE_INSN (cpu, "ebreak;");
+ sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc,
+ sim_stopped, SIM_SIGTRAP);
+ }
+ else
+ pc = pc + 4; /* post srai. */
+ break;
+ }
+ }
+ TRACE_INSN (cpu, "ebreak;");
+ sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc, sim_stopped,
+ SIM_SIGTRAP);
+ break;
+ }
case MATCH_ECALL:
TRACE_INSN (cpu, "ecall;");
riscv_cpu->a0 = sim_syscall (cpu, riscv_cpu->a7, riscv_cpu->a0,
@@ -990,6 +1184,7 @@ execute_one (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
case INSN_CLASS_A:
return execute_a (cpu, iw, op);
case INSN_CLASS_I:
+ case INSN_CLASS_ZICSR:
return execute_i (cpu, iw, op);
case INSN_CLASS_M:
case INSN_CLASS_ZMMUL:
--
2.25.1
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [PATCH 1/4] [sim/riscv] Add basic semi-hosting support
2023-10-17 5:53 [PATCH 1/4] [sim/riscv] Add basic semi-hosting support Jaydeep Patil
@ 2023-10-18 12:37 ` Andrew Burgess
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Burgess @ 2023-10-18 12:37 UTC (permalink / raw)
To: Jaydeep Patil, gdb-patches; +Cc: vapier, Joseph Faulls, Bhushan Attarde
Jaydeep Patil <Jaydeep.Patil@imgtec.com> writes:
> Added support for basic semi-hosting calls OPEN, EXIT and GET_CMDLINE.
> Added gdb.arch/riscv-exit-getcmd.c to test it.
> ---
> gdb/testsuite/gdb.arch/riscv-exit-getcmd.c | 6 +
> gdb/testsuite/gdb.arch/riscv-exit-getcmd.exp | 31 +++
> sim/riscv/riscv-sim.h | 29 +++
> sim/riscv/sim-main.c | 205 ++++++++++++++++++-
> 4 files changed, 266 insertions(+), 5 deletions(-)
> create mode 100644 gdb/testsuite/gdb.arch/riscv-exit-getcmd.c
> create mode 100644 gdb/testsuite/gdb.arch/riscv-exit-getcmd.exp
>
> diff --git a/gdb/testsuite/gdb.arch/riscv-exit-getcmd.c b/gdb/testsuite/gdb.arch/riscv-exit-getcmd.c
> new file mode 100644
> index 00000000000..2d396e530f6
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/riscv-exit-getcmd.c
> @@ -0,0 +1,6 @@
New test source files should include a copyright header -- all but the
oldest files will have one you can copy.
> +
> +int main (int argc, char **argv) {
You should follow the GDB coding standard wherever possible when writing
tests, so function name should start a new line, and '{' should be on
its own line.
> + if (argc != 4)
> + return 1;
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.arch/riscv-exit-getcmd.exp b/gdb/testsuite/gdb.arch/riscv-exit-getcmd.exp
> new file mode 100644
> index 00000000000..672b3c4aa10
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/riscv-exit-getcmd.exp
> @@ -0,0 +1,31 @@
> +# Copyright 2020-2022 Free Software Foundation, Inc.
Unless this patch was posted to the list back in 2020, the date here
should be just '2023'.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see http://www.gnu.org/licenses/.
> +
> +# Test basic semi-hosting calls SYS_GET_CMDLINE and SYS_EXIT
Comments should be complete sentences, so end with a period.
> +
> +if {![istarget "riscv*-*-*"]} {
> + verbose "Skipping ${gdb_test_file_name}."
> + return
> +}
This should be replaced with:
require {istarget "riscv*-*-*"}
> +
> +standard_testfile
> +
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
> + {debug quiet}] } {
> + unsupported "failed to compile"
I believe this unsupported is not required. prepare_for_testing should
already report if this test fails to prepare.
> + return -1
> +}
> +
> +gdb_test "run 1 2 3" ".*Inferior.*process.*exited normally.*"
> diff --git a/sim/riscv/riscv-sim.h b/sim/riscv/riscv-sim.h
> index 1bc9aa12156..c9e78a756c4 100644
> --- a/sim/riscv/riscv-sim.h
> +++ b/sim/riscv/riscv-sim.h
> @@ -75,4 +75,33 @@ extern void initialize_env (SIM_DESC, const char * const *argv,
>
> #define RISCV_XLEN(cpu) MACH_WORD_BITSIZE (CPU_MACH (cpu))
>
> +#define ApplicationExit 0x20026
> +
> +#define SYS_OPEN 0x01
> +#define SYS_GET_CMDLINE 0x15
> +#define SYS_EXIT 0x18
> +
> +#define GDB_O_RDONLY 0x000
> +#define GDB_O_WRONLY 0x001
> +#define GDB_O_RDWR 0x002
> +#define GDB_O_APPEND 0x008
> +#define GDB_O_CREAT 0x200
> +#define GDB_O_TRUNC 0x400
> +#define GDB_O_BINARY 0
> +
> +static int gdb_open_modeflags[12] = {
> + GDB_O_RDONLY,
> + GDB_O_RDONLY | GDB_O_BINARY,
> + GDB_O_RDWR,
> + GDB_O_RDWR | GDB_O_BINARY,
> + GDB_O_WRONLY | GDB_O_CREAT | GDB_O_TRUNC,
> + GDB_O_WRONLY | GDB_O_CREAT | GDB_O_TRUNC | GDB_O_BINARY,
> + GDB_O_RDWR | GDB_O_CREAT | GDB_O_TRUNC,
> + GDB_O_RDWR | GDB_O_CREAT | GDB_O_TRUNC | GDB_O_BINARY,
> + GDB_O_WRONLY | GDB_O_CREAT | GDB_O_APPEND,
> + GDB_O_WRONLY | GDB_O_CREAT | GDB_O_APPEND | GDB_O_BINARY,
> + GDB_O_RDWR | GDB_O_CREAT | GDB_O_APPEND,
> + GDB_O_RDWR | GDB_O_CREAT | GDB_O_APPEND | GDB_O_BINARY
> +};
> +
> #endif
> diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c
> index 250791634a1..2b184aea554 100644
> --- a/sim/riscv/sim-main.c
> +++ b/sim/riscv/sim-main.c
> @@ -136,6 +136,155 @@ store_csr (SIM_CPU *cpu, const char *name, int csr, unsigned_word *reg,
> TRACE_REGISTER (cpu, "wrote CSR %s = %#" PRIxTW, name, val);
> }
>
> +static uintptr_t
> +get_core_data (SIM_CPU *cpu, unsigned_word addr, unsigned_word index)
All these new static functions should ideally have a header comment
explaining what they do, and what the parameters are for.
> +{
> + uintptr_t param;
> + int xlen = RISCV_XLEN (cpu);
> + struct riscv_sim_cpu *riscv_cpu = RISCV_SIM_CPU (cpu);
> +
> + if (xlen == 64)
> + param = sim_core_read_unaligned_8 (cpu, riscv_cpu->pc, read_map,
> + addr + (index * 8));
> + else
> + param = sim_core_read_unaligned_4 (cpu, riscv_cpu->pc, read_map,
> + addr + (index * 4));
> +
> + return param;
> +}
> +
> +static void
> +set_core_string (SIM_CPU *cpu, unsigned_word core_addr, char *host_buf,
> + int len)
> +{
> + struct riscv_sim_cpu *riscv_cpu = RISCV_SIM_CPU (cpu);
> + for (int i = 0; i < len; i++)
> + {
> + sim_core_write_unaligned_1 (cpu, riscv_cpu->pc, write_map,
> + core_addr + i, host_buf[i]);
> + }
> +}
> +
> +static char *
> +get_core_string_with_len (SIM_CPU *cpu, unsigned_word addr,
> + unsigned_word len)
> +{
> + struct riscv_sim_cpu *riscv_cpu = RISCV_SIM_CPU (cpu);
> + char * str;
> + str = (char *) malloc (len + 1);
> +
> + for (int i = 0; i < len; i++)
> + {
> + str[i] = sim_core_read_unaligned_1 (cpu, riscv_cpu->pc, read_map,
> + addr + i);
> + }
> + str[len] = 0;
> +
> + return str;
> +}
> +
> +static void
> +semihosting_open (SIM_CPU *cpu)
> +{
> + uintptr_t fname_addr;
> + uintptr_t flags;
> + uintptr_t fname_len;
> + char *name;
> + struct riscv_sim_cpu *riscv_cpu = RISCV_SIM_CPU (cpu);
> +
> + fname_addr = (uintptr_t) get_core_data (cpu, riscv_cpu->a1, 0);
> + flags = (uintptr_t) get_core_data (cpu, riscv_cpu->a1, 1);
> + fname_len = (uintptr_t) get_core_data (cpu, riscv_cpu->a1, 2);
> +
> + if (fname_len <= 0)
> + {
> + riscv_cpu->a0 = -1;
> + return;
> + }
> +
> + name = get_core_string_with_len (cpu, fname_addr, fname_len);
> + riscv_cpu->a0 = sim_io_open (CPU_STATE (cpu), name,
> + gdb_open_modeflags[flags]);
> + free (name);
> +}
> +
> +static void
> +semihosting_exit (SIM_CPU *cpu)
> +{
> + uintptr_t app_code, exit_code;
> + SIM_DESC sd = CPU_STATE (cpu);
> + struct riscv_sim_cpu *riscv_cpu = RISCV_SIM_CPU (cpu);
> + if (RISCV_XLEN (cpu) == 32)
> + app_code = riscv_cpu->a1;
> + else
> + {
> + app_code = (uintptr_t) get_core_data (cpu, riscv_cpu->a1, 0);
> + exit_code = (uintptr_t) get_core_data (cpu, riscv_cpu->a1, 1);
> + }
> + if (app_code == ApplicationExit)
GNU style doesn't use camel case. Read-only constants can be
APPLICATION_EXIT, or just 'application_exit'.
> + exit_code = 0;
> + else
> + exit_code = 1;
> + riscv_cpu->a0 = exit_code;
> + sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc, sim_exited, exit_code);
> +}
> +
> +static void
> +semihosting_get_cmdline (SIM_CPU *cpu)
> +{
> + int i = 0, len = 0, total_len = 0;
> + uintptr_t buf_addr, max_buf_len;
> + SIM_DESC sd = CPU_STATE (cpu);
> + char *space = " ";
> + struct riscv_sim_cpu *riscv_cpu = RISCV_SIM_CPU (cpu);
> +
> + char **prog_argv = STATE_PROG_ARGV (sd);
> + if (prog_argv == NULL)
> + {
> + riscv_cpu->a0 = 1; // return non-zero to indicate error
GNU style is for /* ... */ comments, not // style.
> + return;
> + }
> +
> + buf_addr = (uintptr_t) get_core_data (cpu, riscv_cpu->a1, 0);
> + max_buf_len = (uintptr_t) get_core_data (cpu, riscv_cpu->a1, 1);
> +
> + while (prog_argv[i])
> + {
> + len = strlen (prog_argv[i]);
> + if ((total_len + len) > max_buf_len)
> + break;
> + set_core_string (cpu, buf_addr, prog_argv[i], len);
> + set_core_string (cpu, buf_addr + len, space, 1);
> + len++; // terminate it with space
> + buf_addr += len;
> + total_len += len;
> + i++;
> + }
> + riscv_cpu->a0 = 0; // no error
> +}
> +
> +static int
> +do_semihosting (SIM_CPU *cpu)
> +{
> + struct riscv_sim_cpu *riscv_cpu = RISCV_SIM_CPU (cpu);
> + switch (riscv_cpu->a0)
> + {
> + case SYS_OPEN:
> + semihosting_open (cpu);
> + break;
> + case SYS_GET_CMDLINE:
> + semihosting_get_cmdline (cpu);
> + break;
> + case SYS_EXIT:
> + semihosting_exit (cpu);
> + break;
> + default:
> + return -1; // semi-hosting call not supported
> + }
> +
> + return 0;
> +}
> +
> static inline unsigned_word
> ashiftrt (unsigned_word val, unsigned_word shift)
> {
> @@ -623,11 +772,56 @@ execute_i (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
> TRACE_INSN (cpu, "fence.i;");
> break;
> case MATCH_EBREAK:
> - TRACE_INSN (cpu, "ebreak;");
> - /* GDB expects us to step over EBREAK. */
> - sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc + 4, sim_stopped,
> - SIM_SIGTRAP);
> - break;
> + {
> + /* RISC-V semi-hosting call is flagged using these three
> + * instructions
> + * slli zero, zero, 0x1f 0x01f01013
> + * ebreak 0x00100073
> + * srai zero, zero, 0x7 0x40705013
> + * Register a0 holds the system call number and a1 holds the
> + * pointer to parameter buffer. Do not read 4 bytes in one go
> + * as we might read malformed 4 byte instruction. */
Comments should follow GNU style; so drop all the extra '*'.
> + int iw_len;
> + sim_cia pre_pc = riscv_cpu->pc - 4;
> + unsigned_word pre_iw;
> + pre_iw = sim_core_read_aligned_2 (cpu, pre_pc, exec_map, pre_pc);
> + iw_len = riscv_insn_length (pre_iw);
> + if (iw_len == 4)
> + pre_iw |= ((unsigned_word) sim_core_read_aligned_2 (
> + cpu, pre_pc, exec_map, pre_pc + 2) << 16);
> +
> + if (pre_iw == 0x01f01013)
At a minimum, these magic numbers needs a good comment. But I wonder if
we can do better, maybe we can make use of MASK_* and MATCH_* macros
from include/opcode/riscv-opc.h?
> + {
> + sim_cia post_pc = riscv_cpu->pc + 4;
> + unsigned_word post_iw;
> + post_iw = sim_core_read_aligned_2 (cpu, post_pc, exec_map,
> + post_pc);
> + iw_len = riscv_insn_length (post_iw);
> + if (iw_len == 4)
> + post_iw |= ((unsigned_word) sim_core_read_aligned_2 (
> + cpu, post_pc, exec_map, post_pc + 2) << 16);
GNU style places the opening '(' onto the new line, rather than trailing
at the end of the previous line.
> +
> + if (post_iw == 0x40705013)
> + {
The indentation looks wrong here, but I'm not sure if that's from your
original patch, or just a result of how the email was sent.
Thanks,
Andrew
> + TRACE_INSN (cpu, "semi-hosting a0=%lx,a1=%lx;",
> + riscv_cpu->a0, riscv_cpu->a1);
> + if (do_semihosting (cpu))
> + {
> + /* Invalid semi-hosting call. */
> + TRACE_INSN (cpu, "ebreak;");
> + sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc,
> + sim_stopped, SIM_SIGTRAP);
> + }
> + else
> + pc = pc + 4; /* post srai. */
> + break;
> + }
> + }
> + TRACE_INSN (cpu, "ebreak;");
> + sim_engine_halt (sd, cpu, NULL, riscv_cpu->pc, sim_stopped,
> + SIM_SIGTRAP);
> + break;
> + }
> case MATCH_ECALL:
> TRACE_INSN (cpu, "ecall;");
> riscv_cpu->a0 = sim_syscall (cpu, riscv_cpu->a7, riscv_cpu->a0,
> @@ -990,6 +1184,7 @@ execute_one (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op)
> case INSN_CLASS_A:
> return execute_a (cpu, iw, op);
> case INSN_CLASS_I:
> + case INSN_CLASS_ZICSR:
> return execute_i (cpu, iw, op);
> case INSN_CLASS_M:
> case INSN_CLASS_ZMMUL:
> --
> 2.25.1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-10-18 12:38 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-17 5:53 [PATCH 1/4] [sim/riscv] Add basic semi-hosting support Jaydeep Patil
2023-10-18 12:37 ` Andrew Burgess
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).