From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:59587) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QNRrx-0004CY-EQ for qemu-devel@nongnu.org; Fri, 20 May 2011 11:44:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QNRrw-0000As-JW for qemu-devel@nongnu.org; Fri, 20 May 2011 11:44:05 -0400 Received: from mail-pz0-f45.google.com ([209.85.210.45]:56673) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QNRrw-0000Af-Bk for qemu-devel@nongnu.org; Fri, 20 May 2011 11:44:04 -0400 Received: by pzk30 with SMTP id 30so2151714pzk.4 for ; Fri, 20 May 2011 08:44:03 -0700 (PDT) Sender: Richard Henderson Message-ID: <4DD68C41.8080201@twiddle.net> Date: Fri, 20 May 2011 08:44:01 -0700 From: Richard Henderson MIME-Version: 1.0 References: <1305671572-5899-1-git-send-email-jcmvbkbc@gmail.com> <1305671572-5899-24-git-send-email-jcmvbkbc@gmail.com> In-Reply-To: <1305671572-5899-24-git-send-email-jcmvbkbc@gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 23/26] target-xtensa: implement interrupt option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Filippov Cc: qemu-devel@nongnu.org On 05/17/2011 03:32 PM, Max Filippov wrote: > + 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. > + 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? > + 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? > 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? > @@ -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. > +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. r~