On Mon, Aug 15, 2022 at 1:26 PM Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> On 8/15/22 14:03, Peter Maydell wrote:
> > The riscv target incorrectly enabled semihosting always, whether the
> > user asked for it or not. Call semihosting_enabled() passing the
> > correct value to the is_userspace argument, which fixes this and also
> > handles the userspace=on argument.
> >
> > Note that this is a behaviour change: we used to default to
> > semihosting being enabled, and now the user must pass
> > "-semihosting-config enable=on" if they want it.
> >
> > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> > ---
> > target/riscv/cpu_helper.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 59b3680b1b2..49c4ea98ac9 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -24,6 +24,7 @@
> > #include "exec/exec-all.h"
> > #include "tcg/tcg-op.h"
> > #include "trace.h"
> > +#include "semihosting/semihost.h"
> > #include "semihosting/common-semi.h"
> >
> > int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
> > @@ -1342,7 +1343,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> > target_ulong mtval2 = 0;
> >
> > if (cause == RISCV_EXCP_SEMIHOST) {
> > - if (env->priv >= PRV_S) {
> > + if (semihosting_enabled(env->priv < PRV_S)) {
> > do_common_semihosting(cs);
> > env->pc += 4;
> > return;
>
> I think this should be done in translate. We should not have the overhead of
> checking the
> three insn sequence around ebreak unless semihosting is enabled. Note that
> ctx->mem_idx
> == env->priv, per cpu_mem_index().
FWIW, the current series worked fine for my risc-v use case. Thanks, Peter!
>
>
> r~
>
