From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51225) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xmitz-0002cK-KV for qemu-devel@nongnu.org; Fri, 07 Nov 2014 07:44:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xmitt-0002Jp-Dg for qemu-devel@nongnu.org; Fri, 07 Nov 2014 07:44:31 -0500 Received: from mail-wg0-f50.google.com ([74.125.82.50]:43200) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xmitt-0002Jg-4g for qemu-devel@nongnu.org; Fri, 07 Nov 2014 07:44:25 -0500 Received: by mail-wg0-f50.google.com with SMTP id z12so3570418wgg.9 for ; Fri, 07 Nov 2014 04:44:21 -0800 (PST) Message-ID: <545CBE9C.4000508@linaro.org> Date: Fri, 07 Nov 2014 12:44:12 +0000 From: Daniel Thompson MIME-Version: 1.0 References: <1414707132-24588-1-git-send-email-greg.bellows@linaro.org> <1414707132-24588-17-git-send-email-greg.bellows@linaro.org> In-Reply-To: <1414707132-24588-17-git-send-email-greg.bellows@linaro.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 16/16] hw/intc/arm_gic: add gic_update() for grouping List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Bellows , qemu-devel@nongnu.org, peter.maydell@linaro.org, serge.fdrv@gmail.com, edgar.iglesias@gmail.com, aggelerf@ethz.ch, christoffer.dall@linaro.org On 30/10/14 22:12, Greg Bellows wrote: > From: Fabian Aggeler > > GICs with grouping (GICv2 or GICv1 with Security Extensions) have a > different exception generation model which is more complicated than > without interrupt grouping. We add a new function to handle this model. > > Signed-off-by: Fabian Aggeler > > --- > > v1 -> v2 > - Fix issue in gic_update_with_grouping() using the wrong combination of > flag and CPU control bank for checking if group 1 interrupts are enabled. > --- > hw/intc/arm_gic.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++- > hw/intc/gic_internal.h | 1 + > 2 files changed, 87 insertions(+), 1 deletion(-) > > diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c > index 808aa18..e33c470 100644 > --- a/hw/intc/arm_gic.c > +++ b/hw/intc/arm_gic.c > @@ -52,6 +52,87 @@ static inline bool ns_access(void) > return true; > } > > +inline void gic_update_with_grouping(GICState *s) > +{ > + int best_irq; > + int best_prio; > + int irq; > + int irq_level; > + int fiq_level; > + int cpu; > + int cm; > + bool next_int; > + bool next_grp0; > + bool gicc_grp0_enabled; > + bool gicc_grp1_enabled; > + > + for (cpu = 0; cpu < NUM_CPU(s); cpu++) { > + cm = 1 << cpu; > + gicc_grp0_enabled = s->cpu_control[cpu][0] & GICC_CTLR_S_EN_GRP0; > + gicc_grp1_enabled = s->cpu_control[cpu][1] & GICC_CTLR_NS_EN_GRP1; > + next_int = 0; > + next_grp0 = 0; > + > + s->current_pending[cpu] = 1023; > + if ((!s->enabled_grp[0] && !s->enabled_grp[1]) > + || (!gicc_grp0_enabled && !gicc_grp1_enabled)) { > + qemu_irq_lower(s->parent_irq[cpu]); > + qemu_irq_lower(s->parent_fiq[cpu]); > + return; Shouldn't that be continue? Otherwise the state of CPU[N-1] influences whether interrupts can be delivered to CPU[N]. > + } > + > + /* Determine highest priority pending interrupt */ > + best_prio = 0x100; > + best_irq = 1023; > + for (irq = 0; irq < s->num_irq; irq++) { > + if (GIC_TEST_ENABLED(irq, cm) && gic_test_pending(s, irq, cm)) { > + if (GIC_GET_PRIORITY(irq, cpu) < best_prio) { > + best_prio = GIC_GET_PRIORITY(irq, cpu); > + best_irq = irq; > + } > + } > + } > + > + /* Priority of IRQ higher than priority mask? */ > + if (best_prio < s->priority_mask[cpu]) { > + s->current_pending[cpu] = best_irq; > + if (GIC_TEST_GROUP0(best_irq, cm) && s->enabled_grp[0]) { > + /* TODO: Add subpriority handling (binary point register) */ > + if (best_prio < s->running_priority[cpu]) { > + next_int = true; > + next_grp0 = true; > + } > + } else if (!GIC_TEST_GROUP0(best_irq, cm) && s->enabled_grp[1]) { > + /* TODO: Add subpriority handling (binary point register) */ > + if (best_prio < s->running_priority[cpu]) { > + next_int = true; > + next_grp0 = false; > + } > + } > + } > + > + fiq_level = 0; > + irq_level = 0; > + if (next_int) { > + if (next_gr && (s->cpu_control[cpu][0] & GICC_CTLR_S_FIQ_EN)) { > + if (gicc_grp0_enabled) { > + fiq_level = 1; > + DPRINTF("Raised pending FIQ %d (cpu %d)\n", best_irq, cpu); > + } > + } else { > + if ((next_grp0 && gicc_grp0_enabled) > + || (!next_grp0 && gicc_grp1_enabled)) { > + irq_level = 1; > + DPRINTF("Raised pending IRQ %d (cpu %d)\n", best_irq, cpu); > + } > + } > + } > + /* Set IRQ/FIQ signal */ > + qemu_set_irq(s->parent_irq[cpu], irq_level); > + qemu_set_irq(s->parent_fiq[cpu], fiq_level); > + } > +} > + > inline void gic_update_no_grouping(GICState *s) > { > int best_irq; > @@ -95,7 +176,11 @@ inline void gic_update_no_grouping(GICState *s) > /* Update interrupt status after enabled or pending bits have been changed. */ > void gic_update(GICState *s) > { > - gic_update_no_grouping(s); > + if (s->revision >= 2 || s->security_extn) { > + gic_update_with_grouping(s); > + } else { > + gic_update_no_grouping(s); > + } > } > > void gic_set_pending_private(GICState *s, int cpu, int irq) > diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h > index e16a7e5..01859ed 100644 > --- a/hw/intc/gic_internal.h > +++ b/hw/intc/gic_internal.h > @@ -73,6 +73,7 @@ > void gic_set_pending_private(GICState *s, int cpu, int irq); > uint32_t gic_acknowledge_irq(GICState *s, int cpu); > void gic_complete_irq(GICState *s, int cpu, int irq); > +inline void gic_update_with_grouping(GICState *s); > inline void gic_update_no_grouping(GICState *s); > void gic_update(GICState *s); > void gic_init_irqs_and_distributor(GICState *s); >