On Sat, Jul 30, 2011 at 3:25 PM, Blue Swirl <blauwir...@gmail.com> wrote:
> On Sat, Jul 30, 2011 at 3:31 PM, Artyom Tarasenko <atar4q...@gmail.com> wrote:
>> On Sat, Jul 30, 2011 at 11:09 AM, Blue Swirl <blauwir...@gmail.com> wrote:
>>> On Mon, Jul 25, 2011 at 8:22 PM, Artyom Tarasenko <atar4q...@gmail.com>
>>> wrote:
>>>> clear interrupt request if the interrupt priority < CPU pil
>>>> clear hardware interrupt request if interrupts are disabled
>>>>
>>>> Signed-off-by: Artyom Tarasenko <atar4q...@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)){
>>>
>>> Sorry, I don't understand the patch. Where is this '2' coming from?
>>
>> We shouldn't interrupt at levels <= psrpil. The bit corresponding to
>> psrpil is (1<< psrpil),
>> the next bit is (2 << psrpil).
>
> Now I see. The check below "i > env->psrpil" does something similar
> but it doesn't reset interrupt.
>
> How about pil < (1 << (env->psrpil + 1))? I think that makes the
> purpose clearer.
But it's also one operation more. Shall I just add a comment?
>>>
>>>> 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);
>>>
>>> Why reset the index? The idea is that the interrupt is left pending a
>>> change to PIL etc.
>>
>> But it is kept in env->pil_in and env->softint . Am I missing some
>> scenario where it's not enough?
>
> cpu-exec.c:378 checks interrupt_index, not pil_in etc.
The scenario this patch is trying to fix:
There comes an interrupt N, the PIL is small enough to proceed,
proceeding disables the interrupts.
Then the interrupt handler deasserts N and enables the interrupts. The
interrupt N comes again but is not processed because of old_interrupt
!= new_interrupt check.
--
Regards,
Artyom Tarasenko
solaris/sparc under qemu blog: http://tyom.blogspot.com/
