> > + if (xtensa_option_enabled(env->config, XTENSA_OPTION_TIMER_INTERRUPT))
> > {
> > + int i;
> > + for (i = 0; i < env->config->nccompare; ++i) {
> > + if (env->sregs[CCOMPARE + i] - old_ccount <= d) {
> > + env->halted = 0;
> > + xtensa_timer_irq(env, i, 1);
>
> I don't think you should be writing to halted here; this is done by
> the code in cpu-exec.c, when noticing when cpu_has_work. Which will
> be true as a function of env->interrupt_request and the interrupt mask.
I do it here to distinguish interrupt caused by CCOMPARE match, for which I
want exact CCOUNT value, from other interrupt sources, when CCOUNT may be
advanced approximately.
> > + if (env->halted) {
> > + xtensa_advance_ccount(env,
> > + muldiv64(qemu_get_clock_ns(vm_clock) -
> > env->halt_clock,
> > + env->config->clock_freq_khz, 1000000));
> > + }
>
> Why are you polling the vm_clock rather than setting up a timer?
I'm not polling, I'm adjusting ccount according to vm_clock time passed since
we were halted.
> > + env->ccompare_timer =
> > + qemu_new_timer_ns(vm_clock, &xtensa_ccompare_cb, env);
>
> ... er, actually you are setting up a timer. So why aren't you using it?
I'm using it: it is wound up by HELPER(waiti) when there's no currently pending
interrupts and xtensa_ccompare_cb calls xtensa_advance_ccount, which in turn
calls xtensa_timer_irq.
> > void do_interrupt(CPUState *env)
> > {
> > switch (env->exception_index) {
> > + case EXC_IRQ:
> > + if (handle_interrupt(env)) {
> > + break;
> > + }
> > + /* not handled interrupt falls through,
> > + * env->exception_index is updated
> > + */
>
> Do you really want to fall through, rather than restart the switch?
Handle_interrupt will handle high-priority interrupt requests, leaving only
level-1 interrupts, which it converts into EXC_USER or EXC_KERNEL. If only for
the sake of readability...
> > @@ -124,12 +198,16 @@ void do_interrupt(CPUState *env)
> > if (env->config->exception_vector[env->exception_index]) {
> > env->pc = env->config->exception_vector[env->exception_index];
> > env->exception_taken = 1;
> > + env->interrupt_request |= CPU_INTERRUPT_EXITTB;
>
> Huh? What are you trying to accomplish here?
> EXITTB is supposed to be used when a device external to the cpu
> changes the memory mapping of the system. E.g. the x86 a20 line.
I used it to have next_tb = 0 in cpu_exec, after return from do_interrupt, but
now it is done unconditionally, so there's no need in CPU_INTERRUPT_EXITTB.
By the way, do I understand it right that if I chain TBs than I need to
periodically check for pending interrupts myself, otherwise e.g. "j $" will
create uninterruptible infinite loop?
> > +DEF_HELPER_0(check_interrupts, void)
> > +DEF_HELPER_2(waiti, void, i32, i32)
> > +DEF_HELPER_2(timer_irq, void, i32, i32)
> > +DEF_HELPER_1(advance_ccount, void, i32)
>
> You shouldn't have to manage any of this from within the translator.
Please explain.
Thanks.
-- Max
