[Qemu-devel] MIPS interrupt handling

[Qemu-devel] MIPS interrupt handling

[Fabrice Bellard]

I just looked at the MIPS file target-mips/op_helper.c and I don't 
understand why IRQs need to be handled in op_helper.c:do_mtc0() with reg 
= 12. IMHO, the corresponding code should be deleted because the TB is 
forced to terminate after mtc0 so that the IRQs can be handled in the 
main loop in cpu-exec.c.

Moreover, clearing CPU_INTERRUPT_HARD in do_mtc0() is almost surely a bug !

Fabrice.

Re: [Qemu-devel] MIPS interrupt handling

[Thiemo Seufer]

Fabrice Bellard wrote:
> I just looked at the MIPS file target-mips/op_helper.c and I don't 
> understand why IRQs need to be handled in op_helper.c:do_mtc0() with reg 
> = 12.

Register 12 is the cp0_status register, it defines which interrupts are
masked/enabled/disabled. Btw, I have a patch which moves this to op.c,
this should improve performance a bit (and avoids the TB stop for
most mtc0 writes).

> IMHO, the corresponding code should be deleted because the TB is 
> forced to terminate after mtc0 so that the IRQs can be handled in the 
> main loop in cpu-exec.c.
> 
> Moreover, clearing CPU_INTERRUPT_HARD in do_mtc0() is almost surely a bug !

Somehow the interrupt assert has to be prevented when St0_IE is cleared.
That's probably also a job for the main loop, but there may be a race
condition (haven't looked yet). Empirically, it works well. :-)


Thiemo

Re: [Qemu-devel] MIPS interrupt handling

[Fabrice Bellard]

Thiemo Seufer wrote:
> Fabrice Bellard wrote:
> 
>>I just looked at the MIPS file target-mips/op_helper.c and I don't 
>>understand why IRQs need to be handled in op_helper.c:do_mtc0() with reg 
>>= 12.
> 
> 
> Register 12 is the cp0_status register, it defines which interrupts are
> masked/enabled/disabled. Btw, I have a patch which moves this to op.c,
> this should improve performance a bit (and avoids the TB stop for
> most mtc0 writes).

op.c should only contain small functions so it is not a good idea. TB 
stop after mtc0 is needed at least when the TLB are modified or to 
handle the interrupts. The current handling of interrupts in mtc0 must 
be suppressed ASAP as it is not useful and complicates the code.

>>IMHO, the corresponding code should be deleted because the TB is 
>>forced to terminate after mtc0 so that the IRQs can be handled in the 
>>main loop in cpu-exec.c.
>>
>>Moreover, clearing CPU_INTERRUPT_HARD in do_mtc0() is almost surely a bug !
> 
> 
> Somehow the interrupt assert has to be prevented when St0_IE is cleared.
> That's probably also a job for the main loop, but there may be a race
> condition (haven't looked yet). Empirically, it works well. :-)

If ST0_IE means interrupt enable, the interrupt assert must not be 
suppressed, but I did not read yet this part of the MIPS spec...

Fabrice.

Re: [Qemu-devel] MIPS interrupt handling

[Thiemo Seufer]

Fabrice Bellard wrote:
> Thiemo Seufer wrote:
> >Fabrice Bellard wrote:
> >
> >>I just looked at the MIPS file target-mips/op_helper.c and I don't 
> >>understand why IRQs need to be handled in op_helper.c:do_mtc0() with reg 
> >>= 12.
> >
> >
> >Register 12 is the cp0_status register, it defines which interrupts are
> >masked/enabled/disabled. Btw, I have a patch which moves this to op.c,
> >this should improve performance a bit (and avoids the TB stop for
> >most mtc0 writes).
> 
> op.c should only contain small functions so it is not a good idea. TB 
> stop after mtc0 is needed at least when the TLB are modified or to 
> handle the interrupts.

Agreed for the cp0_status part, the other functions are small (probably
except cp_cause) and most of them don't need TB stop.

> The current handling of interrupts in mtc0 must 
> be suppressed ASAP as it is not useful and complicates the code.
> 
> >>IMHO, the corresponding code should be deleted because the TB is 
> >>forced to terminate after mtc0 so that the IRQs can be handled in the 
> >>main loop in cpu-exec.c.
> >>
> >>Moreover, clearing CPU_INTERRUPT_HARD in do_mtc0() is almost surely a bug 
> >>!
> >
> >
> >Somehow the interrupt assert has to be prevented when St0_IE is cleared.
> >That's probably also a job for the main loop, but there may be a race
> >condition (haven't looked yet). Empirically, it works well. :-)
> 
> If ST0_IE means interrupt enable, the interrupt assert must not be 
> suppressed, but I did not read yet this part of the MIPS spec...

As I tried to explain, I suspect there's a race between disabling
interrupts via ST0_IE and turning them off in the exception handling
code. For the enable part, it shouldn't matter that much.


Thiemo