From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NSw6T-0000e4-65 for qemu-devel@nongnu.org; Thu, 07 Jan 2010 12:24:57 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NSw6O-0000Yc-4S for qemu-devel@nongnu.org; Thu, 07 Jan 2010 12:24:56 -0500 Received: from [199.232.76.173] (port=35544 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NSw6N-0000YP-Su for qemu-devel@nongnu.org; Thu, 07 Jan 2010 12:24:52 -0500 Received: from mail-bw0-f212.google.com ([209.85.218.212]:55734) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NSw6N-0003IJ-1e for qemu-devel@nongnu.org; Thu, 07 Jan 2010 12:24:51 -0500 Received: by bwz4 with SMTP id 4so11763329bwz.2 for ; Thu, 07 Jan 2010 09:24:50 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <20100105231558.6526.44483.stgit@skyserv> <20100105231943.6526.42311.stgit@skyserv> Date: Thu, 7 Jan 2010 20:24:49 +0300 Message-ID: Subject: Re: [Qemu-devel] [PATCH 8/9] sparc64: interrupt trap handling From: Igor Kovalenko Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: qemu-devel@nongnu.org On Wed, Jan 6, 2010 at 8:00 PM, Blue Swirl wrote: > On Tue, Jan 5, 2010 at 11:19 PM, Igor V. Kovalenko > wrote: >> From: Igor V. Kovalenko >> >> cpu_check_irqs >> - handle SOFTINT register TICK and STICK timer bits >> - only check interrupt levels greater than PIL value >> - handle preemption by higher level traps >> >> cpu_exec >> - handle CPU_INTERRUPT_HARD only if interrupts are enabled >> - PIL 15 is not special level on sparcv9 >> >> Signed-off-by: Igor V. Kovalenko >> --- >> =A0cpu-exec.c | =A0 40 ++++++++++++++++++++++------------------ >> =A0hw/sun4u.c | =A0 52 +++++++++++++++++++++++++++++++++++++------------= --- >> =A02 files changed, 59 insertions(+), 33 deletions(-) >> >> diff --git a/cpu-exec.c b/cpu-exec.c >> index af4595b..65192c1 100644 >> --- a/cpu-exec.c >> +++ b/cpu-exec.c >> @@ -449,24 +449,28 @@ int cpu_exec(CPUState *env1) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 next_tb =3D 0; >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } >> =A0#elif defined(TARGET_SPARC) >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if ((interrupt_request & CPU_IN= TERRUPT_HARD) && >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cpu_interrupts_enabled(env= )) { >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int pil =3D env->interrupt= _index & 15; >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int type =3D env->interrup= t_index & 0xf0; >> - >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (((type =3D=3D TT_EXTIN= T) && >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(pil =3D=3D 15 = || pil > env->psrpil)) || >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 type !=3D TT_EXTIN= T) { >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 env->interrupt_req= uest &=3D ~CPU_INTERRUPT_HARD; >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0env->exception_= index =3D env->interrupt_index; >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0do_interrupt(en= v); >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 env->interrupt_ind= ex =3D 0; >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0next_tb =3D 0; >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } else if (interrupt_request & CPU= _INTERRUPT_TIMER) { >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 //do_interrupt(0, 0, 0, 0,= 0); >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 env->interrupt_request &= =3D ~CPU_INTERRUPT_TIMER; >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if ((interrupt_request & CPU_IN= TERRUPT_HARD)) { >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (cpu_interrupts_enab= led(env)) { >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0int pil =3D env= ->interrupt_index & 0xf; >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0int type =3D en= v->interrupt_index & 0xf0; >> + >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (((type =3D= =3D TT_EXTINT) && (pil > env->psrpil)) || >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0type !=3D TT_EXTINT) { > > This removes the check for level 15, which is non-maskable on V8. I'll implement cpu_pil_allowed() to hide v8 vs v9 difference wrt psrpil. >> diff --git a/hw/sun4u.c b/hw/sun4u.c >> index 9d46f08..84a8043 100644 >> --- a/hw/sun4u.c >> +++ b/hw/sun4u.c >> @@ -233,29 +233,51 @@ void irq_info(Monitor *mon) >> >> =A0void cpu_check_irqs(CPUState *env) >> =A0{ >> - =A0 =A0uint32_t pil =3D env->pil_in | (env->softint & ~SOFTINT_TIMER) = | >> - =A0 =A0 =A0 =A0((env->softint & SOFTINT_TIMER) << 14); >> + =A0 =A0uint32_t pil =3D env->pil_in | (env->softint & ~(SOFTINT_TM | S= OFTINT_SM)); >> + >> + =A0 =A0/* check if TM or SM in SOFTINT are set >> + =A0 =A0 =A0 setting these also causes interrupt 14 */ >> + =A0 =A0if (env->softint & (SOFTINT_TM | SOFTINT_SM)) >> + =A0 =A0 =A0 =A0pil |=3D 1 << 14; >> + >> + =A0 =A0if (!pil) { >> + =A0 =A0 =A0 =A0if (env->interrupt_request & CPU_INTERRUPT_HARD) { >> + =A0 =A0 =A0 =A0 =A0 =A0CPUIRQ_DPRINTF("Reset CPU IRQ (current interrup= t %X)\n", >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 env->interrupt_ind= ex); >> + =A0 =A0 =A0 =A0 =A0 =A0env->interrupt_index =3D 0; >> + =A0 =A0 =A0 =A0 =A0 =A0cpu_reset_interrupt(env, CPU_INTERRUPT_HARD); > > All other architectures have this code in cpu-exec.c, why move? Not really a move (from cpu-exec.c) - see below, it was there already, and sparc32 code is similar. PPC can do that in cpu-exec.c since it exposes pending_interrupts flag. Other arches also seem to reset HARD flag wherever fits. For sparc64 clearing flag in cpu-exec.c would require pulling some code to deal with softint bits. That way we could reduce cpu_check_irqs() to set separate flag "possible interrupt state change" and return from tb so cpu-exec.c code would pick the flag and deal with interrupt. That implementation I'd like less than consolidating trigger logic into cpu_check_irqs. > Overall, it looks like there are some unnecessary changes so that it's > hard to see what is the fix. Since code used to be tabbed as well, there is a great deal of churn here. I tried to outline fixes in the changeset header. --=20 Kind regards, Igor V. Kovalenko