[Qemu-devel] [PATCH] hw/intc/arm_gic: Fix GIC_SET_LEVEL

[Qemu-devel] [PATCH] hw/intc/arm_gic: Fix GIC_SET_LEVEL

[Christoffer Dall]

The GIC_SET_LEVEL macro unfortunately overwrote the entire level
bitmask instead of just or'ing on the necessary bits, causing active
level PPIs on a core to clear PPIs on other cores.

I introduced this bug, sorry about that.

Reported-by: Rob Herring <rob.herring@linaro.org>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 hw/intc/gic_internal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index 92a6f7a..48a58d7 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -40,7 +40,7 @@
 #define GIC_SET_MODEL(irq) s->irq_state[irq].model = true
 #define GIC_CLEAR_MODEL(irq) s->irq_state[irq].model = false
 #define GIC_TEST_MODEL(irq) s->irq_state[irq].model
-#define GIC_SET_LEVEL(irq, cm) s->irq_state[irq].level = (cm)
+#define GIC_SET_LEVEL(irq, cm) s->irq_state[irq].level |= (cm)
 #define GIC_CLEAR_LEVEL(irq, cm) s->irq_state[irq].level &= ~(cm)
 #define GIC_TEST_LEVEL(irq, cm) ((s->irq_state[irq].level & (cm)) != 0)
 #define GIC_SET_EDGE_TRIGGER(irq) s->irq_state[irq].edge_trigger = true
-- 
1.8.5.2

Re: [Qemu-devel] [PATCH] hw/intc/arm_gic: Fix GIC_SET_LEVEL

[Christoffer Dall]

On 21 February 2014 17:03, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> The GIC_SET_LEVEL macro unfortunately overwrote the entire level
> bitmask instead of just or'ing on the necessary bits, causing active
> level PPIs on a core to clear PPIs on other cores.
>
> I introduced this bug, sorry about that.
>
Actually it turns out this was ancient, I was a little too quick to
blame myself there.

-Christoffer

Re: [Qemu-devel] [PATCH] hw/intc/arm_gic: Fix GIC_SET_LEVEL

[Peter Maydell]

On 23 February 2014 16:32, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> On 21 February 2014 17:03, Christoffer Dall <christoffer.dall@linaro.org> wrote:
>> The GIC_SET_LEVEL macro unfortunately overwrote the entire level
>> bitmask instead of just or'ing on the necessary bits, causing active
>> level PPIs on a core to clear PPIs on other cores.
>>
>> I introduced this bug, sorry about that.
>>
> Actually it turns out this was ancient, I was a little too quick to
> blame myself there.

Yes, it's been present since 2007 or so. Probably not noticed
at that time because all the 11MPcore PPIs are edge rather than
level triggered.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

I'll remove the line about it being your fault when I apply
this to target-arm.next :-)

thanks
-- PMM