Re: [PATCH] riscv: Make semihosting configurable for all privilege modes

[Richard Henderson]

On 8/12/22 16:27, Richard Henderson wrote:
> On 8/11/22 13:41, Furquan Shaikh wrote:
> > Unlike ARM, RISC-V does not define a separate breakpoint type for
> > semihosting. Instead, it is entirely ABI. Thus, we need an option
> > to allow users to configure what the ebreak behavior should be for
> > different privilege levels - M, S, U, VS, VU. As per the RISC-V
> > privilege specification[1], ebreak traps into the execution
> > environment. However, RISC-V debug specification[2] provides
> > ebreak{m,s,u,vs,vu} configuration bits to allow ebreak behavior to
> > be configured to trap into debug mode instead. This change adds
> > settable properties for RISC-V CPUs - `ebreakm`, `ebreaks`, `ebreaku`,
> > `ebreakvs` and `ebreakvu` to allow user to configure whether qemu
> > should treat ebreak as semihosting traps or trap according to the
> > privilege specification.
> >
> > [1] 
> > https://github.com/riscv/riscv-isa-manual/releases/download/draft-20220723-10eea63/riscv-privileged.pdf 
> >
> > [2] 
> > https://github.com/riscv/riscv-debug-spec/blob/release/riscv-debug-release.pdf
>
> I don't see why you need to change anything at all.
>
> Semihosting doesn't only use 'ebreak', but a sequence of 3 insns:
>
>      slli x0, x0, 0x1f       # 0x01f01013    Entry NOP
>      ebreak                  # 0x00100073    Break to debugger
>      srai x0, x0, 7          # 0x40705013    NOP encoding the semihosting call 
> number 7
>
> If the -semihosting command-line argument is absent, then the new DSCR fields apply as 
> normal.  If the -semihosting command-line argument is present, and the ebreak is not 
> surrounded by the required nops, then the new DSCR fields apply as normal.  But if the 
> command-line argument is present and the nops are present, then semihosting overrides the 
> architecture and DSCR does not apply at all.

I note that there's a missing test of semihosting_enabled() in 
target/riscv/insn_trans/trans_privileged.c.inc, and the PRV_S check in 
riscv_cpu_do_interrupt can be done at translation via ctx->mem_idx >= PRV_S.


r~