* [Qemu-devel] [PATCH] arm_gic: handle banked enable bits for per-cpu interrupts
@ 2011-10-28 17:40 Rabin Vincent
2011-11-01 22:31 ` Peter Maydell
0 siblings, 1 reply; 5+ messages in thread
From: Rabin Vincent @ 2011-10-28 17:40 UTC (permalink / raw)
To: qemu-devel; +Cc: Rabin Vincent, Peter Maydell
The first enable set/clear register (which controls the PPIs and SGIs)
is supposed to be banked for each processor. Currently it is just
handled globally and this prevents recent SMP Linux kernels from
booting, because CPU0 stops receiving localtimer interrupts when CPU1
disables them locally.
To fix this, allow the enable bits to be enabled per-cpu. For SPIs,
always enable/disable ALL_CPU_MASK.
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Rabin Vincent <rabin@rab.in>
---
hw/arm_gic.c | 35 ++++++++++++++++++++---------------
1 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/hw/arm_gic.c b/hw/arm_gic.c
index 8dd8742..f3f3516 100644
--- a/hw/arm_gic.c
+++ b/hw/arm_gic.c
@@ -37,9 +37,8 @@ static const uint8_t gic_id[] =
typedef struct gic_irq_state
{
- /* ??? The documentation seems to imply the enable bits are global, even
- for per-cpu interrupts. This seems strange. */
- unsigned enabled:1;
+ /* The enable bits are only banked for per-cpu interrupts. */
+ unsigned enabled:NCPU;
unsigned pending:NCPU;
unsigned active:NCPU;
unsigned level:NCPU;
@@ -54,9 +53,9 @@ typedef struct gic_irq_state
#define NUM_CPU(s) 1
#endif
-#define GIC_SET_ENABLED(irq) s->irq_state[irq].enabled = 1
-#define GIC_CLEAR_ENABLED(irq) s->irq_state[irq].enabled = 0
-#define GIC_TEST_ENABLED(irq) s->irq_state[irq].enabled
+#define GIC_SET_ENABLED(irq, cm) s->irq_state[irq].enabled |= (cm)
+#define GIC_CLEAR_ENABLED(irq, cm) s->irq_state[irq].enabled &= ~(cm)
+#define GIC_TEST_ENABLED(irq, cm) ((s->irq_state[irq].enabled & (cm)) != 0)
#define GIC_SET_PENDING(irq, cm) s->irq_state[irq].pending |= (cm)
#define GIC_CLEAR_PENDING(irq, cm) s->irq_state[irq].pending &= ~(cm)
#define GIC_TEST_PENDING(irq, cm) ((s->irq_state[irq].pending & (cm)) != 0)
@@ -128,7 +127,7 @@ static void gic_update(gic_state *s)
best_prio = 0x100;
best_irq = 1023;
for (irq = 0; irq < GIC_NIRQ; irq++) {
- if (GIC_TEST_ENABLED(irq) && GIC_TEST_PENDING(irq, cm)) {
+ if (GIC_TEST_ENABLED(irq, cm) && GIC_TEST_PENDING(irq, cm)) {
if (GIC_GET_PRIORITY(irq, cpu) < best_prio) {
best_prio = GIC_GET_PRIORITY(irq, cpu);
best_irq = irq;
@@ -171,7 +170,7 @@ static void gic_set_irq(void *opaque, int irq, int level)
if (level) {
GIC_SET_LEVEL(irq, ALL_CPU_MASK);
- if (GIC_TEST_TRIGGER(irq) || GIC_TEST_ENABLED(irq)) {
+ if (GIC_TEST_TRIGGER(irq) || GIC_TEST_ENABLED(irq, ALL_CPU_MASK)) {
DPRINTF("Set %d pending mask %x\n", irq, GIC_TARGET(irq));
GIC_SET_PENDING(irq, GIC_TARGET(irq));
}
@@ -221,7 +220,7 @@ static void gic_complete_irq(gic_state * s, int cpu, int irq)
if (irq != 1023) {
/* Mark level triggered interrupts as pending if they are still
raised. */
- if (!GIC_TEST_TRIGGER(irq) && GIC_TEST_ENABLED(irq)
+ if (!GIC_TEST_TRIGGER(irq) && GIC_TEST_ENABLED(irq, cm)
&& GIC_TEST_LEVEL(irq, cm) && (GIC_TARGET(irq) & cm) != 0) {
DPRINTF("Set %d pending mask %x\n", irq, cm);
GIC_SET_PENDING(irq, cm);
@@ -280,7 +279,7 @@ static uint32_t gic_dist_readb(void *opaque, target_phys_addr_t offset)
goto bad_reg;
res = 0;
for (i = 0; i < 8; i++) {
- if (GIC_TEST_ENABLED(irq + i)) {
+ if (GIC_TEST_ENABLED(irq + i, cm)) {
res |= (1 << i);
}
}
@@ -412,9 +411,12 @@ static void gic_dist_writeb(void *opaque, target_phys_addr_t offset,
for (i = 0; i < 8; i++) {
if (value & (1 << i)) {
int mask = (irq < 32) ? (1 << cpu) : GIC_TARGET(irq);
- if (!GIC_TEST_ENABLED(irq + i))
+ int cm = (irq < 32) ? (1 << cpu) : ALL_CPU_MASK;
+
+ if (!GIC_TEST_ENABLED(irq + i, cm)) {
DPRINTF("Enabled IRQ %d\n", irq + i);
- GIC_SET_ENABLED(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)
@@ -433,9 +435,12 @@ static void gic_dist_writeb(void *opaque, target_phys_addr_t offset,
value = 0;
for (i = 0; i < 8; i++) {
if (value & (1 << i)) {
- if (GIC_TEST_ENABLED(irq + i))
+ int cm = (irq < 32) ? (1 << cpu) : ALL_CPU_MASK;
+
+ if (GIC_TEST_ENABLED(irq + i, cm)) {
DPRINTF("Disabled IRQ %d\n", irq + i);
- GIC_CLEAR_ENABLED(irq + i);
+ }
+ GIC_CLEAR_ENABLED(irq + i, cm);
}
}
} else if (offset < 0x280) {
@@ -638,7 +643,7 @@ static void gic_reset(gic_state *s)
#endif
}
for (i = 0; i < 16; i++) {
- GIC_SET_ENABLED(i);
+ GIC_SET_ENABLED(i, ALL_CPU_MASK);
GIC_SET_TRIGGER(i);
}
#ifdef NVIC
--
1.7.7
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] arm_gic: handle banked enable bits for per-cpu interrupts
2011-10-28 17:40 Rabin Vincent
@ 2011-11-01 22:31 ` Peter Maydell
2011-11-01 22:42 ` Peter Maydell
0 siblings, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2011-11-01 22:31 UTC (permalink / raw)
To: Rabin Vincent; +Cc: qemu-devel
On 28 October 2011 18:40, Rabin Vincent <rabin@rab.in> wrote:
> The first enable set/clear register (which controls the PPIs and SGIs)
> is supposed to be banked for each processor. Currently it is just
> handled globally and this prevents recent SMP Linux kernels from
> booting, because CPU0 stops receiving localtimer interrupts when CPU1
> disables them locally.
>
> To fix this, allow the enable bits to be enabled per-cpu. For SPIs,
> always enable/disable ALL_CPU_MASK.
> typedef struct gic_irq_state
> {
> - /* ??? The documentation seems to imply the enable bits are global, even
> - for per-cpu interrupts. This seems strange. */
> - unsigned enabled:1;
> + /* The enable bits are only banked for per-cpu interrupts. */
> + unsigned enabled:NCPU;
The ??? comment here is referring to an ambiguity in the 11MPCore
TRM, which doesn't explicitly say that the enable bits for the
per-cpu interrupts are a banked register. (The GIC manual which
covers the A9 and up is clear here that they are banked.)
I have checked with real 11MPCore hardware, and the enable
register is banked between CPUs for the per-cpu interrupt,
so this patch is correct for 11MPCore as well as A9.
Incidentally, this code is shared with the M profile NVIC,
where the first enable register is not special at all. However
since M profile has only one core the special casing is harmless
and this patch only makes it match the other (pending etc)
registers, so I think that's OK.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] arm_gic: handle banked enable bits for per-cpu interrupts
2011-11-01 22:31 ` Peter Maydell
@ 2011-11-01 22:42 ` Peter Maydell
0 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2011-11-01 22:42 UTC (permalink / raw)
To: Rabin Vincent; +Cc: Anthony Liguori, qemu-devel
On 1 November 2011 22:31, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 28 October 2011 18:40, Rabin Vincent <rabin@rab.in> wrote:
>> The first enable set/clear register (which controls the PPIs and SGIs)
>> is supposed to be banked for each processor. Currently it is just
>> handled globally and this prevents recent SMP Linux kernels from
>> booting, because CPU0 stops receiving localtimer interrupts when CPU1
>> disables them locally.
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
This has just missed the 1.0rc0 deadline but as it's a bug fix
we can put it into rc1.
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PULL 1.0rc1] arm-devs patch for 1.0
@ 2011-11-06 16:25 Peter Maydell
2011-11-06 16:25 ` [Qemu-devel] [PATCH] arm_gic: handle banked enable bits for per-cpu interrupts Peter Maydell
0 siblings, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2011-11-06 16:25 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
This pull request includes the single arm-gic patch which
didn't quite make it in before 1.0rc0 (fixes "newer kernels
don't boot"). Please pull.
thanks
-- PMM
The following changes since commit 932eacc158c064935c7bab920c88a93a629e1ca4:
Merge branch 'xtensa' of git://jcmvbkbc.spb.ru/dumb/qemu-xtensa (2011-11-02 20:52:23 +0000)
are available in the git repository at:
git://git.linaro.org/people/pmaydell/qemu-arm.git arm-devs.for-upstream
Rabin Vincent (1):
arm_gic: handle banked enable bits for per-cpu interrupts
hw/arm_gic.c | 35 ++++++++++++++++++++---------------
1 files changed, 20 insertions(+), 15 deletions(-)
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH] arm_gic: handle banked enable bits for per-cpu interrupts
2011-11-06 16:25 [Qemu-devel] [PULL 1.0rc1] arm-devs patch for 1.0 Peter Maydell
@ 2011-11-06 16:25 ` Peter Maydell
0 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2011-11-06 16:25 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
From: Rabin Vincent <rabin@rab.in>
The first enable set/clear register (which controls the PPIs and SGIs)
is supposed to be banked for each processor. Currently it is just
handled globally and this prevents recent SMP Linux kernels from
booting, because CPU0 stops receiving localtimer interrupts when CPU1
disables them locally.
To fix this, allow the enable bits to be enabled per-cpu. For SPIs,
always enable/disable ALL_CPU_MASK.
Signed-off-by: Rabin Vincent <rabin@rab.in>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm_gic.c | 35 ++++++++++++++++++++---------------
1 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/hw/arm_gic.c b/hw/arm_gic.c
index 8dd8742..f3f3516 100644
--- a/hw/arm_gic.c
+++ b/hw/arm_gic.c
@@ -37,9 +37,8 @@ static const uint8_t gic_id[] =
typedef struct gic_irq_state
{
- /* ??? The documentation seems to imply the enable bits are global, even
- for per-cpu interrupts. This seems strange. */
- unsigned enabled:1;
+ /* The enable bits are only banked for per-cpu interrupts. */
+ unsigned enabled:NCPU;
unsigned pending:NCPU;
unsigned active:NCPU;
unsigned level:NCPU;
@@ -54,9 +53,9 @@ typedef struct gic_irq_state
#define NUM_CPU(s) 1
#endif
-#define GIC_SET_ENABLED(irq) s->irq_state[irq].enabled = 1
-#define GIC_CLEAR_ENABLED(irq) s->irq_state[irq].enabled = 0
-#define GIC_TEST_ENABLED(irq) s->irq_state[irq].enabled
+#define GIC_SET_ENABLED(irq, cm) s->irq_state[irq].enabled |= (cm)
+#define GIC_CLEAR_ENABLED(irq, cm) s->irq_state[irq].enabled &= ~(cm)
+#define GIC_TEST_ENABLED(irq, cm) ((s->irq_state[irq].enabled & (cm)) != 0)
#define GIC_SET_PENDING(irq, cm) s->irq_state[irq].pending |= (cm)
#define GIC_CLEAR_PENDING(irq, cm) s->irq_state[irq].pending &= ~(cm)
#define GIC_TEST_PENDING(irq, cm) ((s->irq_state[irq].pending & (cm)) != 0)
@@ -128,7 +127,7 @@ static void gic_update(gic_state *s)
best_prio = 0x100;
best_irq = 1023;
for (irq = 0; irq < GIC_NIRQ; irq++) {
- if (GIC_TEST_ENABLED(irq) && GIC_TEST_PENDING(irq, cm)) {
+ if (GIC_TEST_ENABLED(irq, cm) && GIC_TEST_PENDING(irq, cm)) {
if (GIC_GET_PRIORITY(irq, cpu) < best_prio) {
best_prio = GIC_GET_PRIORITY(irq, cpu);
best_irq = irq;
@@ -171,7 +170,7 @@ static void gic_set_irq(void *opaque, int irq, int level)
if (level) {
GIC_SET_LEVEL(irq, ALL_CPU_MASK);
- if (GIC_TEST_TRIGGER(irq) || GIC_TEST_ENABLED(irq)) {
+ if (GIC_TEST_TRIGGER(irq) || GIC_TEST_ENABLED(irq, ALL_CPU_MASK)) {
DPRINTF("Set %d pending mask %x\n", irq, GIC_TARGET(irq));
GIC_SET_PENDING(irq, GIC_TARGET(irq));
}
@@ -221,7 +220,7 @@ static void gic_complete_irq(gic_state * s, int cpu, int irq)
if (irq != 1023) {
/* Mark level triggered interrupts as pending if they are still
raised. */
- if (!GIC_TEST_TRIGGER(irq) && GIC_TEST_ENABLED(irq)
+ if (!GIC_TEST_TRIGGER(irq) && GIC_TEST_ENABLED(irq, cm)
&& GIC_TEST_LEVEL(irq, cm) && (GIC_TARGET(irq) & cm) != 0) {
DPRINTF("Set %d pending mask %x\n", irq, cm);
GIC_SET_PENDING(irq, cm);
@@ -280,7 +279,7 @@ static uint32_t gic_dist_readb(void *opaque, target_phys_addr_t offset)
goto bad_reg;
res = 0;
for (i = 0; i < 8; i++) {
- if (GIC_TEST_ENABLED(irq + i)) {
+ if (GIC_TEST_ENABLED(irq + i, cm)) {
res |= (1 << i);
}
}
@@ -412,9 +411,12 @@ static void gic_dist_writeb(void *opaque, target_phys_addr_t offset,
for (i = 0; i < 8; i++) {
if (value & (1 << i)) {
int mask = (irq < 32) ? (1 << cpu) : GIC_TARGET(irq);
- if (!GIC_TEST_ENABLED(irq + i))
+ int cm = (irq < 32) ? (1 << cpu) : ALL_CPU_MASK;
+
+ if (!GIC_TEST_ENABLED(irq + i, cm)) {
DPRINTF("Enabled IRQ %d\n", irq + i);
- GIC_SET_ENABLED(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)
@@ -433,9 +435,12 @@ static void gic_dist_writeb(void *opaque, target_phys_addr_t offset,
value = 0;
for (i = 0; i < 8; i++) {
if (value & (1 << i)) {
- if (GIC_TEST_ENABLED(irq + i))
+ int cm = (irq < 32) ? (1 << cpu) : ALL_CPU_MASK;
+
+ if (GIC_TEST_ENABLED(irq + i, cm)) {
DPRINTF("Disabled IRQ %d\n", irq + i);
- GIC_CLEAR_ENABLED(irq + i);
+ }
+ GIC_CLEAR_ENABLED(irq + i, cm);
}
}
} else if (offset < 0x280) {
@@ -638,7 +643,7 @@ static void gic_reset(gic_state *s)
#endif
}
for (i = 0; i < 16; i++) {
- GIC_SET_ENABLED(i);
+ GIC_SET_ENABLED(i, ALL_CPU_MASK);
GIC_SET_TRIGGER(i);
}
#ifdef NVIC
--
1.7.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-11-06 16:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-06 16:25 [Qemu-devel] [PULL 1.0rc1] arm-devs patch for 1.0 Peter Maydell
2011-11-06 16:25 ` [Qemu-devel] [PATCH] arm_gic: handle banked enable bits for per-cpu interrupts Peter Maydell
-- strict thread matches above, loose matches on Subject: below --
2011-10-28 17:40 Rabin Vincent
2011-11-01 22:31 ` Peter Maydell
2011-11-01 22:42 ` Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).