Hi, I found some bugs in the way the IRQ number is calculated
at certain places in arm_gic.c. Perhaps there are a few more
errors that I didn't notice. These bugs were not noticeable
when running Linux as a guest, but I found them when running
my own porting of a multicore real-time OS (TOPPERS/FMP).
NOTE: the main problem I had was that the target CPUs where not
set correctly. Instead of GIC_SET_PENDING(irq + i, GIC_TARGET(irq));
it should read as GIC_SET_PENDING(irq + i, GIC_TARGET(irq + i));
I have tested the patch both with FMP and with a recent Linux, but
please review it since I'm not an expert on QEMU and this is my
first patch.
target-arm: bug fixes for arm_gic.c
The IRQ number was not calculated correctly in the
function gic_dist_writeb for accesses to set/clear
pending and set/clear enable.
Signed-off-by: Daniel Sangorrin <daniel.sangor...@gmail.com>
---
hw/arm_gic.c | 90 ++++++++++++++++++++++++++++++++--------------------------
1 file changed, 50 insertions(+), 40 deletions(-)
diff --git a/hw/arm_gic.c b/hw/arm_gic.c
index f9e423f..f3f233c 100644
--- a/hw/arm_gic.c
+++ b/hw/arm_gic.c
@@ -368,71 +368,81 @@ static void gic_dist_writeb(void *opaque, hwaddr offset,
} else if (offset < 0x180) {
/* Interrupt Set Enable. */
irq = (offset - 0x100) * 8 + GIC_BASE_IRQ;
+ for (i = 0; i < 8; i++) {
+ if (value & (1 << i)) {
+ irq = irq + i;
+ break;
+ }
+ }
if (irq >= s->num_irq)
goto bad_reg;
if (irq < 16)
- value = 0xff;
- for (i = 0; i < 8; i++) {
- if (value & (1 << i)) {
- int mask = (irq < GIC_INTERNAL) ? (1 << cpu) : GIC_TARGET(irq);
- int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
+ value = 0xff;
+ int mask = (irq < GIC_INTERNAL) ? (1 << cpu) : GIC_TARGET(irq);
+ int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
- if (!GIC_TEST_ENABLED(irq + i, cm)) {
- DPRINTF("Enabled IRQ %d\n", irq + i);
- }
- GIC_SET_ENABLED(irq + i, cm);
- /* If a raised level triggered IRQ enabled then mark
- is as pending. */
- if (GIC_TEST_LEVEL(irq + i, mask)
- && !GIC_TEST_TRIGGER(irq + i)) {
- DPRINTF("Set %d pending mask %x\n", irq + i, mask);
- GIC_SET_PENDING(irq + i, mask);
- }
- }
+ if (!GIC_TEST_ENABLED(irq, cm)) {
+ DPRINTF("Enabled IRQ %d\n", irq);
+ }
+ GIC_SET_ENABLED(irq, cm);
+ /* If a raised level triggered IRQ enabled then mark is as pending */
+ if (GIC_TEST_LEVEL(irq, mask) && !GIC_TEST_TRIGGER(irq)) {
+ DPRINTF("Set %d pending mask %x\n", irq, mask);
+ GIC_SET_PENDING(irq, mask);
}
} else if (offset < 0x200) {
/* Interrupt Clear Enable. */
irq = (offset - 0x180) * 8 + GIC_BASE_IRQ;
- if (irq >= s->num_irq)
- goto bad_reg;
- if (irq < 16)
- value = 0;
for (i = 0; i < 8; i++) {
if (value & (1 << i)) {
- int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
-
- if (GIC_TEST_ENABLED(irq + i, cm)) {
- DPRINTF("Disabled IRQ %d\n", irq + i);
- }
- GIC_CLEAR_ENABLED(irq + i, cm);
+ irq = irq + i;
+ break;
}
}
- } else if (offset < 0x280) {
- /* Interrupt Set Pending. */
- irq = (offset - 0x200) * 8 + GIC_BASE_IRQ;
if (irq >= s->num_irq)
goto bad_reg;
if (irq < 16)
- irq = 0;
+ value = 0;
+
+ int cm = (irq < GIC_INTERNAL) ? (1 << cpu) : ALL_CPU_MASK;
+ if (GIC_TEST_ENABLED(irq, cm)) {
+ DPRINTF("Disabled IRQ %d\n", irq);
+ }
+ GIC_CLEAR_ENABLED(irq, cm);
+ } else if (offset < 0x280) {
+ /* Interrupt Set Pending. */
+ irq = (offset - 0x200) * 8 + GIC_BASE_IRQ;
for (i = 0; i < 8; i++) {
if (value & (1 << i)) {
- GIC_SET_PENDING(irq + i, GIC_TARGET(irq));
+ irq = irq + i;
+ break;
}
}
+
+ if (irq >= s->num_irq) {
+ goto bad_reg;
+ }
+ if (irq < 16) {
+ irq = 0;
+ }
+
+ GIC_SET_PENDING(irq, GIC_TARGET(irq));
} else if (offset < 0x300) {
/* Interrupt Clear Pending. */
irq = (offset - 0x280) * 8 + GIC_BASE_IRQ;
- if (irq >= s->num_irq)
- goto bad_reg;
for (i = 0; i < 8; i++) {
- /* ??? This currently clears the pending bit for all CPUs, even
- for per-CPU interrupts. It's unclear whether this is the
- corect behavior. */
- if (value & (1 << i)) {
- GIC_CLEAR_PENDING(irq + i, ALL_CPU_MASK);
- }
+ if (value & (1 << i)) {
+ irq = irq + i;
+ break;
+ }
}
+
+ if (irq >= s->num_irq) {
+ goto bad_reg;
+ }
+
+ GIC_CLEAR_PENDING(irq, ALL_CPU_MASK);
} else if (offset < 0x400) {
/* Interrupt Active. */
goto bad_reg;
--
1.7.9.5
