Re: [PATCH] riscv: Add semihosting support [v4]
Palmer Dabbelt writes: > I think, more concretely, the issues here are: > > * The only mention of "semihosting" in the RISC-V specification is in the > commentary, which is not considered a normative part of the > specification. Ok, we're starting the process of creating a 'real' RISC-V semihosting spec. This references the existing 2.0 version of the ARM semihosting specification, defining only the RISC-V specific details necessary. I've created a public github repository containing this work; that is likely to eventually move over to the RISC-V github repository. https://github.com/keith-packard/riscv-semihosting-spec/ The goal is to enable RISC-V semihosting implementations that share most of the underlying details with existing ARM semihosting implemenations. -- -keith signature.asc Description: PGP signature
Re: [PATCH] riscv: Add semihosting support [v4]
On Thu, 30 Jan 2020 at 11:38, Palmer Dabbelt wrote: > * The semihosting comment doesn't define the semihosting call numbers, just > the > sequence to get to a call. That said, we haven't written down the Linux ABI > either -- though there's a much larger breadth of software out there that > implements it and won't break ABI compatibility, so maybe that's considered > sufficient in Linux land where it's not for semihosting. I think the difference with Linux is that there's a clear single authoritative source for what the ABI is -- Linus's 'mainline' kernel (so for instance random out-of-tree forks, syscall patch proposals, etc that have not yet hit mainline don't count as being fixed-in-stone ABI). Semihosting doesn't have the same single clear "owning project", at least on the Arm side of things. I don't think a semihosting specification for RISC-V needs necessarily to be a very heavyweight thing -- it is, after all, a debug API at heart -- but I do agree that you should have one. thanks -- PMM
Re: [PATCH] riscv: Add semihosting support [v4]
On Thu, 30 Jan 2020 10:54:37 GMT (+), Peter Maydell wrote: On Wed, 29 Jan 2020 at 16:45, Keith Packard wrote: Peter Maydell writes: > True but irrelevant. You need to refer to a proper > risc-v specification for your semihosting. The RISC-V Foundation defined semihosting as relative to the existing ARM specification, so using a link to that is appropriate here. Here's the current specification of the unprivileged ISA, which includes the definition of semihosting https://riscv.org/specifications/ While it may be nice in some abstract sense to create a "better" semihosting spec, that's not what the RISC-V foundation has decided to do. We've gone round this several times. You can't just say "it's the arm spec", because you're not arm and your architecture is different. You need to actually document what the architecture-specific parts are, even if you want to mostly say "and we follow the Arm spec most of the time". If you really really want to say "32 bit RISC-V is gratuitously different from 64-bit RISC-V in these areas because we just blindly copied the way Arm happened to have historically developed" then you can do that if you like, but you need to actually write it down in a document somewhere. You're trying to implement an ABI which has multiple different implementations of both consumers and producers; it's not a good idea to shortcut the process of actually writing down what the requirements between the two ends are. I think, more concretely, the issues here are: * The only mention of "semihosting" in the RISC-V specification is in the commentary, which is not considered a normative part of the specification. * The interface that commentary contains is defined as maybe being obsolete already, but no new concrete interface is defined. * The semihosting comment doesn't define the semihosting call numbers, just the sequence to get to a call. That said, we haven't written down the Linux ABI either -- though there's a much larger breadth of software out there that implements it and won't break ABI compatibility, so maybe that's considered sufficient in Linux land where it's not for semihosting. It seems to me like the right way to go forward is to put together a semihosting ABI specification as a RISC-V specification. That would give everyone who cares about their pre-specification implementations a chance to get together to figure out which parts they want to keep, and give us a path to move forwards. There's more work that needs to be done here to put together a system that will actually work (for example, how to we insert breakpoints if we're using ebreak to indicate semihosting and haven't reserved NOP sequences?), so this does warrant a bit of thought. I know it's a bit of a headache in the short term to get a specification together, but we've had so many headaches of the "there's no spec" sort that at least it'll be a change of pace :) thanks -- PMM
Re: [PATCH] riscv: Add semihosting support [v4]
On Wed, 29 Jan 2020 at 16:45, Keith Packard wrote: > > Peter Maydell writes: > > > True but irrelevant. You need to refer to a proper > > risc-v specification for your semihosting. > > The RISC-V Foundation defined semihosting as relative to the existing > ARM specification, so using a link to that is appropriate here. > > Here's the current specification of the unprivileged ISA, which includes > the definition of semihosting > > https://riscv.org/specifications/ > > While it may be nice in some abstract sense to create a "better" > semihosting spec, that's not what the RISC-V foundation has decided to > do. We've gone round this several times. You can't just say "it's the arm spec", because you're not arm and your architecture is different. You need to actually document what the architecture-specific parts are, even if you want to mostly say "and we follow the Arm spec most of the time". If you really really want to say "32 bit RISC-V is gratuitously different from 64-bit RISC-V in these areas because we just blindly copied the way Arm happened to have historically developed" then you can do that if you like, but you need to actually write it down in a document somewhere. You're trying to implement an ABI which has multiple different implementations of both consumers and producers; it's not a good idea to shortcut the process of actually writing down what the requirements between the two ends are. thanks -- PMM
Re: [PATCH] riscv: Add semihosting support [v4]
The text you are referencing (the couple italic paragraphs below section 2.8 in the unprivileged ISA) is non-normative and "can be skipped if the reader is only interested in the specification itself". This convention of making indented italic text non-normative is described at the bottom of page 1 of the linked document. Jonathan On Wed, Jan 29, 2020 at 11:45 AM Keith Packard via wrote: > Peter Maydell writes: > > > True but irrelevant. You need to refer to a proper > > risc-v specification for your semihosting. > > The RISC-V Foundation defined semihosting as relative to the existing > ARM specification, so using a link to that is appropriate here. > > Here's the current specification of the unprivileged ISA, which includes > the definition of semihosting > > https://riscv.org/specifications/ > > While it may be nice in some abstract sense to create a "better" > semihosting spec, that's not what the RISC-V foundation has decided to > do. > > -- > -keith >
Re: [PATCH] riscv: Add semihosting support [v4]
Peter Maydell writes: > True but irrelevant. You need to refer to a proper > risc-v specification for your semihosting. The RISC-V Foundation defined semihosting as relative to the existing ARM specification, so using a link to that is appropriate here. Here's the current specification of the unprivileged ISA, which includes the definition of semihosting https://riscv.org/specifications/ While it may be nice in some abstract sense to create a "better" semihosting spec, that's not what the RISC-V foundation has decided to do. -- -keith signature.asc Description: PGP signature
Re: [PATCH] riscv: Add semihosting support [v4]
On Tue, 28 Jan 2020 at 23:34, Keith Packard via wrote:
>
> Adapt the arm semihosting support code for RISCV. This implementation
> is based on the standard for RISC-V semihosting as documented in
>
> https://riscv.org/specifications/
>
> Signed-off-by: Keith Packard
>
> ---
> + * ARM Semihosting is documented in:
> + * Semihosting for AArch32 and AArch64 Release 2.0
> + * https://static.docs.arm.com/100863/0200/semihosting.pdf
True but irrelevant. You need to refer to a proper
risc-v specification for your semihosting.
> +case TARGET_SYS_EXIT:
> +case TARGET_SYS_EXIT_EXTENDED:
> +if (nr == TARGET_SYS_EXIT_EXTENDED || sizeof(target_ulong) == 0) {
> +/*
> + * The A64 version of SYS_EXIT takes a parameter block,
> + * so the application-exit type can return a subcode which
> + * is the exit status code from the application.
> + * SYS_EXIT_EXTENDED is an a new-in-v2.0 optional function
> + * which allows A32/T32 guests to also provide a status code.
> + */
This code and comment describe Arm semihosting, where we
made this bad decision about the API for 32-bit Arm, fixed
it for 64-bit Arm and then put in an extension to add the more
sensible API to 32-bit as a backwards-compatible new function.
Nobody else should make this API choice from the start.
What does RISC-V want to do here? This should be in
your specification.
> +GET_ARG(0);
> +GET_ARG(1);
> +
> +if (arg0 == ADP_Stopped_ApplicationExit) {
> +ret = arg1;
> +} else {
> +ret = 1;
> +}
> +} else {
> +/*
> + * The A32/T32 version of SYS_EXIT specifies only
> + * Stopped_ApplicationExit as normal exit, but does not
> + * allow the guest to specify the exit status code.
> + * 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
> + * virtual address range. This is a nop for us since we don't
> + * implement caches. This is only present on A64.
> + */
> +if (sizeof(target_ulong) == 8) {
> +return 0;
> +}
> +/* fall through -- invalid for A32/T32 */
Again, this is an Arm-ism, where the old A32 ABI
doesn't have this function but the new A64 one does. What
does RISC-V want to specify here?
> +default:
> +fprintf(stderr, "qemu: Unsupported SemiHosting SWI 0x%02x\n", nr);
> +cpu_dump_state(cs, stderr, 0);
> +abort();
This is repeating the Arm ABI mistake of allowing implementations
to just crash and burn if they're handed a function call they don't
recognize. Ideally it should be avoided in a new ABI.
> +}
> +}
thanks
-- PMM
[PATCH] riscv: Add semihosting support [v4]
Adapt the arm semihosting support code for RISCV. This implementation
is based on the standard for RISC-V semihosting as documented in
https://riscv.org/specifications/
Signed-off-by: Keith Packard
---
v2:
Update PC after exception is handled to follow
change in the ARM version for SYS_READC
v3:
Disallow semihosting in user mode; report a regular
breakpoint in that case.
v4:
Fix errors reported by checkpatch
---
default-configs/riscv32-softmmu.mak |1 +
linux-user/qemu.h |2 +
qemu-options.hx |4 +-
target/riscv/Makefile.objs|2 +-
target/riscv/cpu.h|2 +
target/riscv/cpu_bits.h |1 +
target/riscv/cpu_helper.c |9 +
.../riscv/insn_trans/trans_privileged.inc.c | 24 +-
target/riscv/riscv-semi.c | 1084 +
target/riscv/translate.c | 11 +
10 files changed, 1136 insertions(+), 4 deletions(-)
create mode 100644 target/riscv/riscv-semi.c
diff --git a/default-configs/riscv32-softmmu.mak
b/default-configs/riscv32-softmmu.mak
index 1ae077ed87..21b7bedf19 100644
--- a/default-configs/riscv32-softmmu.mak
+++ b/default-configs/riscv32-softmmu.mak
@@ -3,6 +3,7 @@
# Uncomment the following lines to disable these optional devices:
#
#CONFIG_PCI_DEVICES=n
+CONFIG_SEMIHOSTING=y
# Boards:
#
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 560a68090e..122d95d6b6 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -105,6 +105,8 @@ typedef struct TaskState {
/* FPA state */
FPA11 fpa;
# endif
+#endif
+#if defined(TARGET_ARM) || defined(TARGET_RISCV)
int swi_errno;
#endif
#if defined(TARGET_I386) && !defined(TARGET_X86_64)
diff --git a/qemu-options.hx b/qemu-options.hx
index 224a8e8712..4892e6b12c 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4109,7 +4109,7 @@ ETEXI
DEF("semihosting", 0, QEMU_OPTION_semihosting,
"-semihostingsemihosting mode\n",
QEMU_ARCH_ARM | QEMU_ARCH_M68K | QEMU_ARCH_XTENSA | QEMU_ARCH_LM32 |
-QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2)
+QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
STEXI
@item -semihosting
@findex -semihosting
@@ -4119,7 +4119,7 @@ DEF("semihosting-config", HAS_ARG,
QEMU_OPTION_semihosting_config,
"-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)
+QEMU_ARCH_MIPS | QEMU_ARCH_NIOS2 | QEMU_ARCH_RISCV)
STEXI
@item -semihosting-config
[enable=on|off][,target=native|gdb|auto][,chardev=id][,arg=str[,...]]
@findex -semihosting-config
diff --git a/target/riscv/Makefile.objs b/target/riscv/Makefile.objs
index ff651f69f6..6fd7f40e29 100644
--- a/target/riscv/Makefile.objs
+++ b/target/riscv/Makefile.objs
@@ -1,4 +1,4 @@
-obj-y += translate.o op_helper.o cpu_helper.o cpu.o csr.o fpu_helper.o
gdbstub.o
+obj-y += translate.o op_helper.o cpu_helper.o cpu.o csr.o fpu_helper.o
gdbstub.o riscv-semi.o
obj-$(CONFIG_SOFTMMU) += pmp.o
ifeq ($(CONFIG_SOFTMMU),y)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index de0a8d893a..310ec6ab40 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -338,6 +338,8 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
typedef CPURISCVState CPUArchState;
typedef RISCVCPU ArchCPU;
+target_ulong do_riscv_semihosting(CPURISCVState *env);
+
#include "exec/cpu-all.h"
#endif /* RISCV_CPU_H */
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index e99834856c..c45745d73b 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -496,6 +496,7 @@
#define RISCV_EXCP_INST_PAGE_FAULT 0xc /* since: priv-1.10.0 */
#define RISCV_EXCP_LOAD_PAGE_FAULT 0xd /* since: priv-1.10.0 */
#define RISCV_EXCP_STORE_PAGE_FAULT0xf /* since: priv-1.10.0 */
+#define RISCV_EXCP_SEMIHOST0x10
#define RISCV_EXCP_INT_FLAG0x8000
#define RISCV_EXCP_INT_MASK0x7fff
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 85403da9c8..6a04469936 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -532,6 +532,15 @@ void riscv_cpu_do_interrupt(CPUState *cs)
[PRV_M] = RISCV_EXCP_M_ECALL
};
+if (cause == RISCV_EXCP_SEMIHOST) {
+if (env->priv >= PRV_S) {
+env->gpr[xA0] = do_riscv_semihosting(env);
+env->pc += 4;
+return;
+}
+cause = RISCV_EXCP_BREAKPOINT;
+}
+
if (!async) {
/* set tval to badaddr for traps with address information */
switch (cause) {
diff --git a/target/riscv/insn_trans/trans_privileged.inc.c
b/target/riscv/insn_trans/trans_privileged.
