Re: [Qemu-devel] [PATCH] fix disabling interrupts in sun4u

[tsnsaito@gmail.com]

At Thu, 28 Jul 2011 13:51:08 +0200,
Artyom Tarasenko wrote:
> On Thu, Jul 28, 2011 at 12:31 PM,  <tsnsaito@gmail.com> 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=15.
> > 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.
The interrupts from APB would be reported by cpu_set_irq(),
but cpu_set_irq() seems to generate interrupt_vector_n traps.

> >> Signed-off-by: Artyom Tarasenko <atar4qemu@gmail.com>
> >> ---
> >>  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 |= 1 << 14;
> >>      }
> >>
> >> -    if (!pil) {
> >> +    if (pil < (2 << env->psrpil)){
> >>          if (env->interrupt_request & CPU_INTERRUPT_HARD) {
> >>              CPUIRQ_DPRINTF("Reset CPU IRQ (current interrupt %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=%08x pil_in=%08x softint=%08x "
> >>                         "current interrupt %x\n",
> >>                         pil, env->pil_in, env->softint, env->interrupt_index);
> >> +        env->interrupt_index = 0;
> >> +        cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
> >>      }
> >>  }
> >>
> >> --
> >> 1.7.3.4
> >
> >
> > ----
> > Tsuneo Saito <tsnsaito@gmail.com>
> >
> 
> 
> 
> -- 
> Regards,
> Artyom Tarasenko
> 
> solaris/sparc under qemu blog: http://tyom.blogspot.com/

---- 
Tsuneo Saito <tsnsaito@gmail.com>