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

[Richard Henderson <richard.henderson@linaro.org>]

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~