From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:55843) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QmQpa-0008G9-Jw for qemu-devel@nongnu.org; Thu, 28 Jul 2011 09:40:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QmQpZ-0000KD-9d for qemu-devel@nongnu.org; Thu, 28 Jul 2011 09:40:54 -0400 Received: from mail-pz0-f43.google.com ([209.85.210.43]:39197) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QmQpY-0000K7-Vp for qemu-devel@nongnu.org; Thu, 28 Jul 2011 09:40:53 -0400 Received: by pzk1 with SMTP id 1so4756002pzk.30 for ; Thu, 28 Jul 2011 06:40:52 -0700 (PDT) Message-ID: <4e3166e3.c323440a.2041.54f3@mx.google.com> Date: Thu, 28 Jul 2011 22:40:48 +0900 From: tsnsaito@gmail.com In-Reply-To: References: <316f198de781f7b819456f433d84ead284769a71.1311606610.git.atar4qemu@gmail.com> <4e313a7e.c926440a.2e83.4a11@mx.google.com> <4e315024.11948e0a.4077.2795@mx.google.com> MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] fix disabling interrupts in sun4u List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Artyom Tarasenko Cc: blauwirbel@gmail.com, qemu-devel@nongnu.org At Thu, 28 Jul 2011 14:50:57 +0200, Artyom Tarasenko wrote: > On Thu, Jul 28, 2011 at 2:03 PM, wrote: > > At Thu, 28 Jul 2011 13:51:08 +0200, > > Artyom Tarasenko wrote: > >> On Thu, Jul 28, 2011 at 12:31 PM,   wrote: > >> > Hi, > >> > > >> > At Mon, 25 Jul 2011 19:22:38 +0200, > >> > Artyom Tarasenko wrote: > >> > > >> >> clear interrupt request if the interrupt priority < CPU pil > >> >> clear hardware interrupt request if interrupts are disabled > >> > > >> > Not directly related to the fix, but I'd like to note a problem > >> > of hw/sun4u.c interrupt code: > >> > > >> > The interrupt code probably mixes hardware interrupts and > >> > software interrupts. > >> > %pil is for software interrupts (interrupt_level_n traps). > >> > %pil can not mask hardware interrupts (interrupt_vector traps); > >> > the CPU raises interrupt_vector traps even on %pil=3D15. > >> > But in cpu_check_irqs() and cpu_set_irq(), hardware interrupts > >> > seem to be masked by %pil. > >> > >> The interrupt_vector traps are currently not implemented, are they? > >> So it's hard to tell whether they are masked. > > > > Yes, interrupt_vector is not implemented yet. > > I failed to explain the problem. > > The problem is that cpu_set_irqs() should raise interrupt_vector > > traps but it raises interrupt_level_n traps actually. > > sun4uv_init() calls qemu_allocate_irqs() with cpu_set_irq as > > the 1st argument.  The allocated irqs (the irq variable) are > > passed to pci_apb_init().  APB should generate interrupt_vector > > traps (hardware interrupts), not the interrupt_vector_n traps. >=20 > Yes, this is true. But it's more complicated than this: cpu_check_irqs > also checks tick/stick/hstick interrupts. They should produce the > interrupt_level_n traps as they currently do. That's right. tick/stick/hstick must raise interrupt_level_n traps. > The patch merely fixes the problem of hanging on a interrupt_vector_n > trap if the trap handler uses pil for interrupt masking. The problem > exists independently from interrupt_vector trap generation (as you > pointed out). I understand what is the problem that your patch is going to fix. Thanks for the explanation. > Do you have objections to this patch in its current form? No, I don't have any objections. > > The interrupts from APB would be reported by cpu_set_irq(), > > but cpu_set_irq() seems to generate interrupt_vector_n traps. >=20 > For me it's not obvious. The interrupt vector not just one line, but > the vector, which is written in the corresponding CPU register (also > missing in the current qemu implementation). On the real hardware the > vector is created by the IOMMU (PBM/APB/...). If qemu intends to > support multiple chipsets, we should keep it the way it's done on the > real hardware (for instance the interrupt vectors for on-board devices > on Ultra-1 and E6500 are not the same). Sorry, I can't keep up with this vector thing... Does the CPU receive hardware interrupts as interrupt_vector traps (trap type=3D0x60) regardless of the kind of the interrupt controller, doesn't it? > I'd suggest APB shall use some other interface for communicating > interrupts to the CPU. Something like > cpu_receive_ivec(interrupt_vector). >=20 > >> >> Signed-off-by: Artyom Tarasenko > >> >> --- > >> >>  hw/sun4u.c |    6 ++++-- > >> >>  1 files changed, 4 insertions(+), 2 deletions(-) > >> >> > >> >> diff --git a/hw/sun4u.c b/hw/sun4u.c > >> >> index d7dcaf0..7f95aeb 100644 > >> >> --- a/hw/sun4u.c > >> >> +++ b/hw/sun4u.c > >> >> @@ -255,7 +255,7 @@ void cpu_check_irqs(CPUState *env) > >> >>          pil |=3D 1 << 14; > >> >>      } > >> >> > >> >> -    if (!pil) { > >> >> +    if (pil < (2 << env->psrpil)){ > >> >>          if (env->interrupt_request & CPU_INTERRUPT_HARD) { > >> >>              CPUIRQ_DPRINTF("Reset CPU IRQ (current interrup= t %x)\n", > >> >>                             env->interrupt_index); > >> >> @@ -287,10 +287,12 @@ void cpu_check_irqs(CPUState *env) > >> >>                  break; > >> >>              } > >> >>          } > >> >> -    } else { > >> >> +    } else if (env->interrupt_request & CPU_INTERRUPT_HARD) { > >> >>          CPUIRQ_DPRINTF("Interrupts disabled, pil=3D%08x pil_i= n=3D%08x softint=3D%08x " > >> >>                         "current interrupt %x\n", > >> >>                         pil, env->pil_in, env->softint,= env->interrupt_index); > >> >> +        env->interrupt_index =3D 0; > >> >> +        cpu_reset_interrupt(env, CPU_INTERRUPT_HARD); > >> >>      } > >> >>  } > >> >> > >> >> -- > >> >> 1.7.3.4 > >> > > >> > > >> > ---- > >> > Tsuneo Saito > >> > > >> > >> > >> > >> -- > >> Regards, > >> Artyom Tarasenko > >> > >> solaris/sparc under qemu blog: http://tyom.blogspot.com/ > > > > ---- > > Tsuneo Saito > > >=20 > --=20 > Regards, > Artyom Tarasenko >=20 > solaris/sparc under qemu blog: http://tyom.blogspot.com/ ---- Tsuneo Saito