On 02/04/19 10:23, Liran Alon wrote:
>> +#define SUCCESSIVE_IRQ_MAX_COUNT 10000
>> +
>> +static void ioapic_timer(void *opaque)
> I suggest rename method to something such as
> “delayed_ioapic_service_timer_handler()”.
>
I renamed them to delayed_ioapic_service_{timer,cb} and queued the patch.
>>
>> + if (((entry & IOAPIC_VECTOR_MASK) != vector) ||
>> + !(entry & IOAPIC_LVT_REMOTE_IRR)) {
>> + continue;
>> + }
>> +
>> + trace_ioapic_clear_remote_irr(n, vector);
>> + s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR;
>
> remote-irr is only set for level-triggered interrupt.
> Thus, I think in the “if” above you should “continue;” in case trigger-mode
> != IOAPIC_TRIGGER_LEVEL.
> i.e. Replace condition "!(entry & IOAPIC_LVT_REMOTE_IRR)” with “trigger-mode
> != IOAPIC_TRIGGER_LEVEL”.
> Then you can also remove the “if” below.
Like this?
@@ -232,19 +232,18 @@ void ioapic_eoi_broadcast(int vector)
for (n = 0; n < IOAPIC_NUM_PINS; n++) {
entry = s->ioredtbl[n];
- if (((entry & IOAPIC_VECTOR_MASK) != vector) ||
- !(entry & IOAPIC_LVT_REMOTE_IRR)) {
+ if ((entry & IOAPIC_VECTOR_MASK) != vector ||
+ ((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) !=
IOAPIC_TRIGGER_LEVEL) {
continue;
}
- trace_ioapic_clear_remote_irr(n, vector);
- s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR;
-
- if (((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) !=
- IOAPIC_TRIGGER_LEVEL) {
+ if (!(entry & IOAPIC_LVT_REMOTE_IRR)) {
continue;
}
+ trace_ioapic_clear_remote_irr(n, vector);
+ s->ioredtbl[n] = entry & ~IOAPIC_LVT_REMOTE_IRR;
+
if (!(entry & IOAPIC_LVT_MASKED) && (s->irr & (1 << n))) {
++s->irq_eoi[vector];
if (s->irq_eoi[vector] >= SUCCESSIVE_IRQ_MAX_COUNT) {
Paolo
