On 5 April 2017 at 00:40, Alistair Francis <[email protected]> wrote: > Only trigger multi-queue GEM interrupts if the interrupt status register > is set. This logic was already used for non multi-queue interrupts but > it also applies to multi-queue interrupts. > > Signed-off-by: Alistair Francis <[email protected]> > --- > > hw/net/cadence_gem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c > index 3e37665..b9eaed4 100644 > --- a/hw/net/cadence_gem.c > +++ b/hw/net/cadence_gem.c > @@ -518,7 +518,7 @@ static void gem_update_int_status(CadenceGEMState *s) > } > > for (i = 0; i < s->num_priority_queues; ++i) { > - if (s->regs[GEM_INT_Q1_STATUS + i]) { > + if (s->regs[GEM_INT_Q1_STATUS + i] && s->regs[GEM_ISR]) { > DB_PRINT("asserting int. (q=%d)\n", i); > qemu_set_irq(s->irq[i], 1); > }
With this change, if s->regs[GEM_ISR] is zero then the entire function never does anything, so you could hoist that up to the start, rather than testing it inside the loop and in the previous conditional. Also the comment says "raise or lower interrupt based on current status", but the code will only ever do qemu_set_irq(..., 1), never 0. Which is right? thanks -- PMM
