Message ID | 20110206092920.GZ14984@redhat.com |
---|---|
State | New |
Headers | show |
2011/2/6 Gleb Natapov <gleb@redhat.com>: > > TPR blocks all interrupts in a priority class, so simple "less or > equal" check is not enough. > > Signed-off-by: Gleb Natapov <gleb@redhat.com> > diff --git a/hw/apic.c b/hw/apic.c > index 2f8376a..cbac75d 100644 > --- a/hw/apic.c > +++ b/hw/apic.c > @@ -372,19 +372,33 @@ static int apic_get_arb_pri(APICState *s) > return 0; > } > > -/* signal the CPU if an irq is pending */ > -static void apic_update_irq(APICState *s) > + > +/* > + * <0 - low prio interrupt, > + * 0 - no interrupt, > + * >0 - interrupt number > + */ > +static int apic_irq_pending(APICState *s) > { > int irrv, ppr; > - if (!(s->spurious_vec & APIC_SV_ENABLE)) > - return; > irrv = get_highest_priority_int(s->irr); > if (irrv < 0) > - return; > + return 0; > ppr = apic_get_ppr(s); > if (ppr && (irrv & 0xf0) <= (ppr & 0xf0)) > + return -1; Just a minor point: braces are missing. checkpatch.pl would help. Yoshi > + > + return irrv; > +} > + > +/* signal the CPU if an irq is pending */ > +static void apic_update_irq(APICState *s) > +{ > + if (!(s->spurious_vec & APIC_SV_ENABLE)) > return; > - cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD); > + if (apic_irq_pending(s) > 0) { > + cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD); > + } > } > > void apic_reset_irq_delivered(void) > @@ -590,12 +604,13 @@ int apic_get_interrupt(DeviceState *d) > if (!(s->spurious_vec & APIC_SV_ENABLE)) > return -1; > > - /* XXX: spurious IRQ handling */ > - intno = get_highest_priority_int(s->irr); > - if (intno < 0) > + intno = apic_irq_pending(s); > + > + if (intno == 0) { > return -1; > - if (s->tpr && intno <= s->tpr) > + } else if (intno < 0) { > return s->spurious_vec & 0xff; > + } > reset_bit(s->irr, intno); > set_bit(s->isr, intno); > apic_update_irq(s); > -- > Gleb. > >
On 2011-02-07 00:45, Yoshiaki Tamura wrote: > 2011/2/6 Gleb Natapov <gleb@redhat.com>: >> >> TPR blocks all interrupts in a priority class, so simple "less or >> equal" check is not enough. >> >> Signed-off-by: Gleb Natapov <gleb@redhat.com> >> diff --git a/hw/apic.c b/hw/apic.c >> index 2f8376a..cbac75d 100644 >> --- a/hw/apic.c >> +++ b/hw/apic.c >> @@ -372,19 +372,33 @@ static int apic_get_arb_pri(APICState *s) >> return 0; >> } >> >> -/* signal the CPU if an irq is pending */ >> -static void apic_update_irq(APICState *s) >> + >> +/* >> + * <0 - low prio interrupt, >> + * 0 - no interrupt, >> + * >0 - interrupt number >> + */ >> +static int apic_irq_pending(APICState *s) >> { >> int irrv, ppr; >> - if (!(s->spurious_vec & APIC_SV_ENABLE)) >> - return; >> irrv = get_highest_priority_int(s->irr); >> if (irrv < 0) >> - return; >> + return 0; >> ppr = apic_get_ppr(s); >> if (ppr && (irrv & 0xf0) <= (ppr & 0xf0)) >> + return -1; > > Just a minor point: braces are missing. checkpatch.pl would > help. Actually, the code above does not touch lines with missing braces (though I prefer to clean up the close environment of my patches as well) but.. >> + >> + return irrv; >> +} >> + >> +/* signal the CPU if an irq is pending */ >> +static void apic_update_irq(APICState *s) >> +{ >> + if (!(s->spurious_vec & APIC_SV_ENABLE)) >> return; ...here checkpatch would complain. Also about a trailing whitespace earlier. Besides those minor issues, patch looks good and works fine. So you can add Reviewed-by: Jan Kiszka <jan.kiszka@siemens.com> on resubmission. Please also add a 0.14 tag, this is a must-have for stable IMO. Jan >> - cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD); >> + if (apic_irq_pending(s) > 0) { >> + cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD); >> + } >> } >> >> void apic_reset_irq_delivered(void) >> @@ -590,12 +604,13 @@ int apic_get_interrupt(DeviceState *d) >> if (!(s->spurious_vec & APIC_SV_ENABLE)) >> return -1; >> >> - /* XXX: spurious IRQ handling */ >> - intno = get_highest_priority_int(s->irr); >> - if (intno < 0) >> + intno = apic_irq_pending(s); >> + >> + if (intno == 0) { >> return -1; >> - if (s->tpr && intno <= s->tpr) >> + } else if (intno < 0) { >> return s->spurious_vec & 0xff; >> + } >> reset_bit(s->irr, intno); >> set_bit(s->isr, intno); >> apic_update_irq(s); >> -- >> Gleb. >> >>
diff --git a/hw/apic.c b/hw/apic.c index 2f8376a..cbac75d 100644 --- a/hw/apic.c +++ b/hw/apic.c @@ -372,19 +372,33 @@ static int apic_get_arb_pri(APICState *s) return 0; } -/* signal the CPU if an irq is pending */ -static void apic_update_irq(APICState *s) + +/* + * <0 - low prio interrupt, + * 0 - no interrupt, + * >0 - interrupt number + */ +static int apic_irq_pending(APICState *s) { int irrv, ppr; - if (!(s->spurious_vec & APIC_SV_ENABLE)) - return; irrv = get_highest_priority_int(s->irr); if (irrv < 0) - return; + return 0; ppr = apic_get_ppr(s); if (ppr && (irrv & 0xf0) <= (ppr & 0xf0)) + return -1; + + return irrv; +} + +/* signal the CPU if an irq is pending */ +static void apic_update_irq(APICState *s) +{ + if (!(s->spurious_vec & APIC_SV_ENABLE)) return; - cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD); + if (apic_irq_pending(s) > 0) { + cpu_interrupt(s->cpu_env, CPU_INTERRUPT_HARD); + } } void apic_reset_irq_delivered(void) @@ -590,12 +604,13 @@ int apic_get_interrupt(DeviceState *d) if (!(s->spurious_vec & APIC_SV_ENABLE)) return -1; - /* XXX: spurious IRQ handling */ - intno = get_highest_priority_int(s->irr); - if (intno < 0) + intno = apic_irq_pending(s); + + if (intno == 0) { return -1; - if (s->tpr && intno <= s->tpr) + } else if (intno < 0) { return s->spurious_vec & 0xff; + } reset_bit(s->irr, intno); set_bit(s->isr, intno); apic_update_irq(s);
TPR blocks all interrupts in a priority class, so simple "less or equal" check is not enough. Signed-off-by: Gleb Natapov <gleb@redhat.com> -- Gleb.