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

[Furquan Shaikh]

On Fri, Aug 12, 2022 at 4:28 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 11 Aug 2022 at 21:47, Furquan Shaikh <furquan@rivosinc.com> 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
>
> As a general rule we don't allow userspace to make semihosting
> calls, as a (rather weak) attempt at fencing off unprivileged
> guest code from being able to scribble all over the host
> filesystem. We should try to be consistent across architectures
> about that, and in particular about how we enable it.
>
> I have a half-finished patchset where I was planning to add
> a --semihosting-config userspace-enable=on option or similar
> to that effect.
>
> It sounds like these ebreak bits are somewhat architectural,
> so maybe they make sense as a riscv specific thing, but we
> should consider how they ought to interact with the general
> behaviour of semihosting. As it stands in QEMU today, we
> (at least in theory) ought not to permit userspace to make
> semihosting ebreak calls at all I think.

Thanks for the feedback, Peter. IIUC, the option that you are planning
to add "--semihosting-config userspace-enable=on" would allow
userspace to make semihosting calls on all architectures. The ebreak
bits here are risc-v specific and hence this change adds it as a
property for RISC-V CPUs. I agree with you that we should be
consistent about how we enable semihosting behavior within qemu.
Without seeing more details of how the config you plan to implement
works, it might be difficult to say how both these fit together. But,
just based on the above understanding, I am thinking we can use the
generic semihosting-config as the top-level config to allow
non-supervisor semihosting. Once your change is ready, we can apply
the additional restriction that these RISC-V ebreak bits for user mode
be settable only if the top-level config is enabled. There are other
ways of handling this too. I am new here and still learning, so open
to any ideas that might make this integration easier.

>
> thanks
> -- PMM