Re: [Qemu-devel] [PATCH 8/9] sparc64: interrupt trap handling

[Igor Kovalenko <igor.v.kovalenko@gmail.com>]

On Wed, Jan 6, 2010 at 8:00 PM, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Tue, Jan 5, 2010 at 11:19 PM, Igor V. Kovalenko
> <igor.v.kovalenko@gmail.com> wrote:
>> From: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
>>
>> 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 <igor.v.kovalenko@gmail.com>
>> ---
>>  cpu-exec.c |   40 ++++++++++++++++++++++------------------
>>  hw/sun4u.c |   52 +++++++++++++++++++++++++++++++++++++---------------
>>  2 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)
>>                         next_tb = 0;
>>                     }
>>  #elif defined(TARGET_SPARC)
>> -                    if ((interrupt_request & CPU_INTERRUPT_HARD) &&
>> -                       cpu_interrupts_enabled(env)) {
>> -                       int pil = env->interrupt_index & 15;
>> -                       int type = env->interrupt_index & 0xf0;
>> -
>> -                       if (((type == TT_EXTINT) &&
>> -                            (pil == 15 || pil > env->psrpil)) ||
>> -                           type != TT_EXTINT) {
>> -                           env->interrupt_request &= ~CPU_INTERRUPT_HARD;
>> -                            env->exception_index = env->interrupt_index;
>> -                            do_interrupt(env);
>> -                           env->interrupt_index = 0;
>> -                        next_tb = 0;
>> -                       }
>> -                   } else if (interrupt_request & CPU_INTERRUPT_TIMER) {
>> -                       //do_interrupt(0, 0, 0, 0, 0);
>> -                       env->interrupt_request &= ~CPU_INTERRUPT_TIMER;
>> -                   }
>> +                    if ((interrupt_request & CPU_INTERRUPT_HARD)) {
>> +                        if (cpu_interrupts_enabled(env)) {
>> +                            int pil = env->interrupt_index & 0xf;
>> +                            int type = env->interrupt_index & 0xf0;
>> +
>> +                            if (((type == TT_EXTINT) && (pil > env->psrpil)) ||
>> +                                            type != 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)
>>
>>  void cpu_check_irqs(CPUState *env)
>>  {
>> -    uint32_t pil = env->pil_in | (env->softint & ~SOFTINT_TIMER) |
>> -        ((env->softint & SOFTINT_TIMER) << 14);
>> +    uint32_t pil = env->pil_in | (env->softint & ~(SOFTINT_TM | SOFTINT_SM));
>> +
>> +    /* check if TM or SM in SOFTINT are set
>> +       setting these also causes interrupt 14 */
>> +    if (env->softint & (SOFTINT_TM | SOFTINT_SM))
>> +        pil |= 1 << 14;
>> +
>> +    if (!pil) {
>> +        if (env->interrupt_request & CPU_INTERRUPT_HARD) {
>> +            CPUIRQ_DPRINTF("Reset CPU IRQ (current interrupt %X)\n",
>> +                           env->interrupt_index);
>> +            env->interrupt_index = 0;
>> +            cpu_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.

-- 
Kind regards,
Igor V. Kovalenko