Re: [Qemu-devel] [PATCH 2/4] target-lm32: add semihosting support

[Michael Walle]

Am 2014-04-28 19:27, schrieb Peter Maydell:
> On 22 April 2014 19:18, Michael Walle <address@hidden> wrote:
> > Intercept certain system calls if semihosting is enabled. This should
> > behave like the GDB simulator.
> > @@ -608,8 +609,20 @@ static void dec_scall(DisasContext *dc)
> >          break;
> >      case 7:
> >          LOG_DIS("scall\n");
> > -        tcg_gen_movi_tl(cpu_pc, dc->pc);
> > -        t_gen_raise_exception(dc, EXCP_SYSTEMCALL);
> > +        if (unlikely(semihosting_enabled)) {
> > +            TCGv t0 = tcg_temp_new();
> > +            int l1 = gen_new_label();
> > +
> > +            gen_helper_semihosting(t0, cpu_env);
> > +            tcg_gen_brcondi_tl(TCG_COND_EQ, t0, 0, l1);
> > +            tcg_gen_movi_tl(cpu_pc, dc->pc);
> > +            t_gen_raise_exception(dc, EXCP_SYSTEMCALL);
> > +            gen_set_label(l1);
> > +            tcg_temp_free(t0);
> > +        } else {
> > +            tcg_gen_movi_tl(cpu_pc, dc->pc);
> > +            t_gen_raise_exception(dc, EXCP_SYSTEMCALL);
> > +        }
> >          break;
> >      default:
> >          qemu_log_mask(LOG_GUEST_ERROR, "invalid opcode @0x%x", 
> > dc->pc);
>
> This seems a bit odd to me -- if semihosting is enabled,
> we generate a call to a helper. If that helper returns
> failure (eg attempt to open nonexistent file) we raise
> an exception.

this is not an indication of a failure, but an indication if the syscall 
was unhandled, eg. not a known semicall and thus will be handled as a 
'normal' syscall.

> Why not just do what the ARM semihosting
> does and put the call to the semihosting handling code
> in the lm32_cpu_do_interrupt() function?

mhh, i'll look into that. actually, that sounds better.

> (It's a bit sad that the semihosting API for lm32
> doesn't seem to provide any way of distinguishing
> "syscall for semihosting" from "other syscall",
> incidentally.)

You mean within qemu? See above.

Thanks for the review.

-michael