* [PATCH v2 1/5] hw/intc/bcm2835_ic: Trace GPU/CPU IRQ handlers
2020-09-21 3:52 [PATCH v2 0/5] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+ Philippe Mathieu-Daudé
@ 2020-09-21 3:52 ` Philippe Mathieu-Daudé
2020-09-21 19:41 ` Luc Michel
2020-09-21 3:52 ` [PATCH v2 2/5] hw/timer/bcm2835: Introduce BCM2835_SYSTIMER_COUNT definition Philippe Mathieu-Daudé
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-21 3:52 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé,
Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
Luc Michel
Add trace events for GPU and CPU IRQs.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/intc/bcm2835_ic.c | 4 +++-
hw/intc/trace-events | 4 ++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/intc/bcm2835_ic.c b/hw/intc/bcm2835_ic.c
index 53ab8f58810..9000d995e81 100644
--- a/hw/intc/bcm2835_ic.c
+++ b/hw/intc/bcm2835_ic.c
@@ -18,6 +18,7 @@
#include "migration/vmstate.h"
#include "qemu/log.h"
#include "qemu/module.h"
+#include "trace.h"
#define GPU_IRQS 64
#define ARM_IRQS 8
@@ -51,7 +52,6 @@ static void bcm2835_ic_update(BCM2835ICState *s)
set = (s->gpu_irq_level & s->gpu_irq_enable)
|| (s->arm_irq_level & s->arm_irq_enable);
qemu_set_irq(s->irq, set);
-
}
static void bcm2835_ic_set_gpu_irq(void *opaque, int irq, int level)
@@ -59,6 +59,7 @@ static void bcm2835_ic_set_gpu_irq(void *opaque, int irq, int level)
BCM2835ICState *s = opaque;
assert(irq >= 0 && irq < 64);
+ trace_bcm2835_ic_set_gpu_irq(irq, level);
s->gpu_irq_level = deposit64(s->gpu_irq_level, irq, 1, level != 0);
bcm2835_ic_update(s);
}
@@ -68,6 +69,7 @@ static void bcm2835_ic_set_arm_irq(void *opaque, int irq, int level)
BCM2835ICState *s = opaque;
assert(irq >= 0 && irq < 8);
+ trace_bcm2835_ic_set_cpu_irq(irq, level);
s->arm_irq_level = deposit32(s->arm_irq_level, irq, 1, level != 0);
bcm2835_ic_update(s);
}
diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index 527c3f76cae..22782b3f089 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -199,3 +199,7 @@ nvic_sysreg_write(uint64_t addr, uint32_t value, unsigned size) "NVIC sysreg wri
heathrow_write(uint64_t addr, unsigned int n, uint64_t value) "0x%"PRIx64" %u: 0x%"PRIx64
heathrow_read(uint64_t addr, unsigned int n, uint64_t value) "0x%"PRIx64" %u: 0x%"PRIx64
heathrow_set_irq(int num, int level) "set_irq: num=0x%02x level=%d"
+
+# bcm2835_ic.c
+bcm2835_ic_set_gpu_irq(int irq, int level) "GPU irq #%d level %d"
+bcm2835_ic_set_cpu_irq(int irq, int level) "CPU irq #%d level %d"
--
2.26.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/5] hw/intc/bcm2835_ic: Trace GPU/CPU IRQ handlers
2020-09-21 3:52 ` [PATCH v2 1/5] hw/intc/bcm2835_ic: Trace GPU/CPU IRQ handlers Philippe Mathieu-Daudé
@ 2020-09-21 19:41 ` Luc Michel
0 siblings, 0 replies; 13+ messages in thread
From: Luc Michel @ 2020-09-21 19:41 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Peter Maydell, Niek Linnenbank, qemu-arm, Andrew Baumann, Paul Zimmerman
On 9/21/20 5:52 AM, Philippe Mathieu-Daudé wrote:
> Add trace events for GPU and CPU IRQs.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Luc Michel <luc.michel@greensocs.com>
> ---
> hw/intc/bcm2835_ic.c | 4 +++-
> hw/intc/trace-events | 4 ++++
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/hw/intc/bcm2835_ic.c b/hw/intc/bcm2835_ic.c
> index 53ab8f58810..9000d995e81 100644
> --- a/hw/intc/bcm2835_ic.c
> +++ b/hw/intc/bcm2835_ic.c
> @@ -18,6 +18,7 @@
> #include "migration/vmstate.h"
> #include "qemu/log.h"
> #include "qemu/module.h"
> +#include "trace.h"
>
> #define GPU_IRQS 64
> #define ARM_IRQS 8
> @@ -51,7 +52,6 @@ static void bcm2835_ic_update(BCM2835ICState *s)
> set = (s->gpu_irq_level & s->gpu_irq_enable)
> || (s->arm_irq_level & s->arm_irq_enable);
> qemu_set_irq(s->irq, set);
> -
> }
>
> static void bcm2835_ic_set_gpu_irq(void *opaque, int irq, int level)
> @@ -59,6 +59,7 @@ static void bcm2835_ic_set_gpu_irq(void *opaque, int irq, int level)
> BCM2835ICState *s = opaque;
>
> assert(irq >= 0 && irq < 64);
> + trace_bcm2835_ic_set_gpu_irq(irq, level);
> s->gpu_irq_level = deposit64(s->gpu_irq_level, irq, 1, level != 0);
> bcm2835_ic_update(s);
> }
> @@ -68,6 +69,7 @@ static void bcm2835_ic_set_arm_irq(void *opaque, int irq, int level)
> BCM2835ICState *s = opaque;
>
> assert(irq >= 0 && irq < 8);
> + trace_bcm2835_ic_set_cpu_irq(irq, level);
> s->arm_irq_level = deposit32(s->arm_irq_level, irq, 1, level != 0);
> bcm2835_ic_update(s);
> }
> diff --git a/hw/intc/trace-events b/hw/intc/trace-events
> index 527c3f76cae..22782b3f089 100644
> --- a/hw/intc/trace-events
> +++ b/hw/intc/trace-events
> @@ -199,3 +199,7 @@ nvic_sysreg_write(uint64_t addr, uint32_t value, unsigned size) "NVIC sysreg wri
> heathrow_write(uint64_t addr, unsigned int n, uint64_t value) "0x%"PRIx64" %u: 0x%"PRIx64
> heathrow_read(uint64_t addr, unsigned int n, uint64_t value) "0x%"PRIx64" %u: 0x%"PRIx64
> heathrow_set_irq(int num, int level) "set_irq: num=0x%02x level=%d"
> +
> +# bcm2835_ic.c
> +bcm2835_ic_set_gpu_irq(int irq, int level) "GPU irq #%d level %d"
> +bcm2835_ic_set_cpu_irq(int irq, int level) "CPU irq #%d level %d"
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/5] hw/timer/bcm2835: Introduce BCM2835_SYSTIMER_COUNT definition
2020-09-21 3:52 [PATCH v2 0/5] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+ Philippe Mathieu-Daudé
2020-09-21 3:52 ` [PATCH v2 1/5] hw/intc/bcm2835_ic: Trace GPU/CPU IRQ handlers Philippe Mathieu-Daudé
@ 2020-09-21 3:52 ` Philippe Mathieu-Daudé
2020-09-21 19:43 ` Luc Michel
2020-09-21 3:52 ` [PATCH v2 3/5] hw/timer/bcm2835: Rename variable holding CTRL_STATUS register Philippe Mathieu-Daudé
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-21 3:52 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé,
Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
Luc Michel
Use the BCM2835_SYSTIMER_COUNT definition instead of the
magic '4' value.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/timer/bcm2835_systmr.h | 4 +++-
hw/timer/bcm2835_systmr.c | 3 ++-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/include/hw/timer/bcm2835_systmr.h b/include/hw/timer/bcm2835_systmr.h
index 64166bd7120..11272837a6b 100644
--- a/include/hw/timer/bcm2835_systmr.h
+++ b/include/hw/timer/bcm2835_systmr.h
@@ -18,6 +18,8 @@ typedef struct BCM2835SystemTimerState BCM2835SystemTimerState;
DECLARE_INSTANCE_CHECKER(BCM2835SystemTimerState, BCM2835_SYSTIMER,
TYPE_BCM2835_SYSTIMER)
+#define BCM2835_SYSTIMER_COUNT 4
+
struct BCM2835SystemTimerState {
/*< private >*/
SysBusDevice parent_obj;
@@ -28,7 +30,7 @@ struct BCM2835SystemTimerState {
struct {
uint32_t status;
- uint32_t compare[4];
+ uint32_t compare[BCM2835_SYSTIMER_COUNT];
} reg;
};
diff --git a/hw/timer/bcm2835_systmr.c b/hw/timer/bcm2835_systmr.c
index 3387a6214a2..ff8c5536610 100644
--- a/hw/timer/bcm2835_systmr.c
+++ b/hw/timer/bcm2835_systmr.c
@@ -134,7 +134,8 @@ static const VMStateDescription bcm2835_systmr_vmstate = {
.minimum_version_id = 1,
.fields = (VMStateField[]) {
VMSTATE_UINT32(reg.status, BCM2835SystemTimerState),
- VMSTATE_UINT32_ARRAY(reg.compare, BCM2835SystemTimerState, 4),
+ VMSTATE_UINT32_ARRAY(reg.compare, BCM2835SystemTimerState,
+ BCM2835_SYSTIMER_COUNT),
VMSTATE_END_OF_LIST()
}
};
--
2.26.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 2/5] hw/timer/bcm2835: Introduce BCM2835_SYSTIMER_COUNT definition
2020-09-21 3:52 ` [PATCH v2 2/5] hw/timer/bcm2835: Introduce BCM2835_SYSTIMER_COUNT definition Philippe Mathieu-Daudé
@ 2020-09-21 19:43 ` Luc Michel
0 siblings, 0 replies; 13+ messages in thread
From: Luc Michel @ 2020-09-21 19:43 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Peter Maydell, Niek Linnenbank, qemu-arm, Andrew Baumann, Paul Zimmerman
On 9/21/20 5:52 AM, Philippe Mathieu-Daudé wrote:
> Use the BCM2835_SYSTIMER_COUNT definition instead of the
> magic '4' value.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Luc Michel <luc.michel@greensocs.com>
> ---
> include/hw/timer/bcm2835_systmr.h | 4 +++-
> hw/timer/bcm2835_systmr.c | 3 ++-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/timer/bcm2835_systmr.h b/include/hw/timer/bcm2835_systmr.h
> index 64166bd7120..11272837a6b 100644
> --- a/include/hw/timer/bcm2835_systmr.h
> +++ b/include/hw/timer/bcm2835_systmr.h
> @@ -18,6 +18,8 @@ typedef struct BCM2835SystemTimerState BCM2835SystemTimerState;
> DECLARE_INSTANCE_CHECKER(BCM2835SystemTimerState, BCM2835_SYSTIMER,
> TYPE_BCM2835_SYSTIMER)
>
> +#define BCM2835_SYSTIMER_COUNT 4
> +
> struct BCM2835SystemTimerState {
> /*< private >*/
> SysBusDevice parent_obj;
> @@ -28,7 +30,7 @@ struct BCM2835SystemTimerState {
>
> struct {
> uint32_t status;
> - uint32_t compare[4];
> + uint32_t compare[BCM2835_SYSTIMER_COUNT];
> } reg;
> };
>
> diff --git a/hw/timer/bcm2835_systmr.c b/hw/timer/bcm2835_systmr.c
> index 3387a6214a2..ff8c5536610 100644
> --- a/hw/timer/bcm2835_systmr.c
> +++ b/hw/timer/bcm2835_systmr.c
> @@ -134,7 +134,8 @@ static const VMStateDescription bcm2835_systmr_vmstate = {
> .minimum_version_id = 1,
> .fields = (VMStateField[]) {
> VMSTATE_UINT32(reg.status, BCM2835SystemTimerState),
> - VMSTATE_UINT32_ARRAY(reg.compare, BCM2835SystemTimerState, 4),
> + VMSTATE_UINT32_ARRAY(reg.compare, BCM2835SystemTimerState,
> + BCM2835_SYSTIMER_COUNT),
> VMSTATE_END_OF_LIST()
> }
> };
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 3/5] hw/timer/bcm2835: Rename variable holding CTRL_STATUS register
2020-09-21 3:52 [PATCH v2 0/5] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+ Philippe Mathieu-Daudé
2020-09-21 3:52 ` [PATCH v2 1/5] hw/intc/bcm2835_ic: Trace GPU/CPU IRQ handlers Philippe Mathieu-Daudé
2020-09-21 3:52 ` [PATCH v2 2/5] hw/timer/bcm2835: Introduce BCM2835_SYSTIMER_COUNT definition Philippe Mathieu-Daudé
@ 2020-09-21 3:52 ` Philippe Mathieu-Daudé
2020-09-21 19:44 ` Luc Michel
2020-09-21 3:52 ` [PATCH v2 4/5] hw/timer/bcm2835: Support the timer COMPARE registers Philippe Mathieu-Daudé
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-21 3:52 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé,
Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
Luc Michel
The variable holding the CTRL_STATUS register is misnamed
'status'. Rename it 'ctrl_status' to make it more obvious
this register is also used to control the peripheral.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/timer/bcm2835_systmr.h | 2 +-
hw/timer/bcm2835_systmr.c | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/hw/timer/bcm2835_systmr.h b/include/hw/timer/bcm2835_systmr.h
index 11272837a6b..e0db9e9e12b 100644
--- a/include/hw/timer/bcm2835_systmr.h
+++ b/include/hw/timer/bcm2835_systmr.h
@@ -29,7 +29,7 @@ struct BCM2835SystemTimerState {
qemu_irq irq;
struct {
- uint32_t status;
+ uint32_t ctrl_status;
uint32_t compare[BCM2835_SYSTIMER_COUNT];
} reg;
};
diff --git a/hw/timer/bcm2835_systmr.c b/hw/timer/bcm2835_systmr.c
index ff8c5536610..b234e83824f 100644
--- a/hw/timer/bcm2835_systmr.c
+++ b/hw/timer/bcm2835_systmr.c
@@ -30,7 +30,7 @@ REG32(COMPARE3, 0x18)
static void bcm2835_systmr_update_irq(BCM2835SystemTimerState *s)
{
- bool enable = !!s->reg.status;
+ bool enable = !!s->reg.ctrl_status;
trace_bcm2835_systmr_irq(enable);
qemu_set_irq(s->irq, enable);
@@ -52,7 +52,7 @@ static uint64_t bcm2835_systmr_read(void *opaque, hwaddr offset,
switch (offset) {
case A_CTRL_STATUS:
- r = s->reg.status;
+ r = s->reg.ctrl_status;
break;
case A_COMPARE0 ... A_COMPARE3:
r = s->reg.compare[(offset - A_COMPARE0) >> 2];
@@ -82,7 +82,7 @@ static void bcm2835_systmr_write(void *opaque, hwaddr offset,
trace_bcm2835_systmr_write(offset, value);
switch (offset) {
case A_CTRL_STATUS:
- s->reg.status &= ~value; /* Ack */
+ s->reg.ctrl_status &= ~value; /* Ack */
bcm2835_systmr_update_irq(s);
break;
case A_COMPARE0 ... A_COMPARE3:
@@ -133,7 +133,7 @@ static const VMStateDescription bcm2835_systmr_vmstate = {
.version_id = 1,
.minimum_version_id = 1,
.fields = (VMStateField[]) {
- VMSTATE_UINT32(reg.status, BCM2835SystemTimerState),
+ VMSTATE_UINT32(reg.ctrl_status, BCM2835SystemTimerState),
VMSTATE_UINT32_ARRAY(reg.compare, BCM2835SystemTimerState,
BCM2835_SYSTIMER_COUNT),
VMSTATE_END_OF_LIST()
--
2.26.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 3/5] hw/timer/bcm2835: Rename variable holding CTRL_STATUS register
2020-09-21 3:52 ` [PATCH v2 3/5] hw/timer/bcm2835: Rename variable holding CTRL_STATUS register Philippe Mathieu-Daudé
@ 2020-09-21 19:44 ` Luc Michel
0 siblings, 0 replies; 13+ messages in thread
From: Luc Michel @ 2020-09-21 19:44 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Peter Maydell, Niek Linnenbank, qemu-arm, Andrew Baumann, Paul Zimmerman
On 9/21/20 5:52 AM, Philippe Mathieu-Daudé wrote:
> The variable holding the CTRL_STATUS register is misnamed
> 'status'. Rename it 'ctrl_status' to make it more obvious
> this register is also used to control the peripheral.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Luc Michel <luc.michel@greensocs.com>
> ---
> include/hw/timer/bcm2835_systmr.h | 2 +-
> hw/timer/bcm2835_systmr.c | 8 ++++----
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/hw/timer/bcm2835_systmr.h b/include/hw/timer/bcm2835_systmr.h
> index 11272837a6b..e0db9e9e12b 100644
> --- a/include/hw/timer/bcm2835_systmr.h
> +++ b/include/hw/timer/bcm2835_systmr.h
> @@ -29,7 +29,7 @@ struct BCM2835SystemTimerState {
> qemu_irq irq;
>
> struct {
> - uint32_t status;
> + uint32_t ctrl_status;
> uint32_t compare[BCM2835_SYSTIMER_COUNT];
> } reg;
> };
> diff --git a/hw/timer/bcm2835_systmr.c b/hw/timer/bcm2835_systmr.c
> index ff8c5536610..b234e83824f 100644
> --- a/hw/timer/bcm2835_systmr.c
> +++ b/hw/timer/bcm2835_systmr.c
> @@ -30,7 +30,7 @@ REG32(COMPARE3, 0x18)
>
> static void bcm2835_systmr_update_irq(BCM2835SystemTimerState *s)
> {
> - bool enable = !!s->reg.status;
> + bool enable = !!s->reg.ctrl_status;
>
> trace_bcm2835_systmr_irq(enable);
> qemu_set_irq(s->irq, enable);
> @@ -52,7 +52,7 @@ static uint64_t bcm2835_systmr_read(void *opaque, hwaddr offset,
>
> switch (offset) {
> case A_CTRL_STATUS:
> - r = s->reg.status;
> + r = s->reg.ctrl_status;
> break;
> case A_COMPARE0 ... A_COMPARE3:
> r = s->reg.compare[(offset - A_COMPARE0) >> 2];
> @@ -82,7 +82,7 @@ static void bcm2835_systmr_write(void *opaque, hwaddr offset,
> trace_bcm2835_systmr_write(offset, value);
> switch (offset) {
> case A_CTRL_STATUS:
> - s->reg.status &= ~value; /* Ack */
> + s->reg.ctrl_status &= ~value; /* Ack */
> bcm2835_systmr_update_irq(s);
> break;
> case A_COMPARE0 ... A_COMPARE3:
> @@ -133,7 +133,7 @@ static const VMStateDescription bcm2835_systmr_vmstate = {
> .version_id = 1,
> .minimum_version_id = 1,
> .fields = (VMStateField[]) {
> - VMSTATE_UINT32(reg.status, BCM2835SystemTimerState),
> + VMSTATE_UINT32(reg.ctrl_status, BCM2835SystemTimerState),
> VMSTATE_UINT32_ARRAY(reg.compare, BCM2835SystemTimerState,
> BCM2835_SYSTIMER_COUNT),
> VMSTATE_END_OF_LIST()
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 4/5] hw/timer/bcm2835: Support the timer COMPARE registers
2020-09-21 3:52 [PATCH v2 0/5] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+ Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2020-09-21 3:52 ` [PATCH v2 3/5] hw/timer/bcm2835: Rename variable holding CTRL_STATUS register Philippe Mathieu-Daudé
@ 2020-09-21 3:52 ` Philippe Mathieu-Daudé
2020-09-21 19:39 ` Luc Michel
2020-09-21 3:52 ` [PATCH v2 5/5] hw/arm/bcm2835_peripherals: Correctly wire the SYS_timer IRQs Philippe Mathieu-Daudé
2020-09-24 11:21 ` [PATCH v2 0/5] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+ Philippe Mathieu-Daudé
5 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-21 3:52 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé,
Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
Luc Michel
This peripheral has 1 free-running timer and 4 compare registers.
Only the free-running timer is implemented. Add support the
COMPARE registers (each register is wired to an IRQ).
Reference: "BCM2835 ARM Peripherals" datasheet [*]
chapter 12 "System Timer":
The System Timer peripheral provides four 32-bit timer channels
and a single 64-bit free running counter. Each channel has an
output compare register, which is compared against the 32 least
significant bits of the free running counter values. When the
two values match, the system timer peripheral generates a signal
to indicate a match for the appropriate channel. The match signal
is then fed into the interrupt controller.
This peripheral is used since Linux 3.7, commit ee4af5696720
("ARM: bcm2835: add system timer").
[*] https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/timer/bcm2835_systmr.h | 11 +++++++--
hw/timer/bcm2835_systmr.c | 41 +++++++++++++++++++------------
hw/timer/trace-events | 4 ++-
3 files changed, 37 insertions(+), 19 deletions(-)
diff --git a/include/hw/timer/bcm2835_systmr.h b/include/hw/timer/bcm2835_systmr.h
index e0db9e9e12b..17fdd9d67b2 100644
--- a/include/hw/timer/bcm2835_systmr.h
+++ b/include/hw/timer/bcm2835_systmr.h
@@ -11,6 +11,7 @@
#include "hw/sysbus.h"
#include "hw/irq.h"
+#include "qemu/timer.h"
#include "qom/object.h"
#define TYPE_BCM2835_SYSTIMER "bcm2835-sys-timer"
@@ -20,18 +21,24 @@ DECLARE_INSTANCE_CHECKER(BCM2835SystemTimerState, BCM2835_SYSTIMER,
#define BCM2835_SYSTIMER_COUNT 4
+typedef struct {
+ unsigned id;
+ QEMUTimer timer;
+ qemu_irq irq;
+ BCM2835SystemTimerState *state;
+} BCM2835SystemTimerCompare;
+
struct BCM2835SystemTimerState {
/*< private >*/
SysBusDevice parent_obj;
/*< public >*/
MemoryRegion iomem;
- qemu_irq irq;
-
struct {
uint32_t ctrl_status;
uint32_t compare[BCM2835_SYSTIMER_COUNT];
} reg;
+ BCM2835SystemTimerCompare tmr[BCM2835_SYSTIMER_COUNT];
};
#endif
diff --git a/hw/timer/bcm2835_systmr.c b/hw/timer/bcm2835_systmr.c
index b234e83824f..43e644f5e45 100644
--- a/hw/timer/bcm2835_systmr.c
+++ b/hw/timer/bcm2835_systmr.c
@@ -28,20 +28,13 @@ REG32(COMPARE1, 0x10)
REG32(COMPARE2, 0x14)
REG32(COMPARE3, 0x18)
-static void bcm2835_systmr_update_irq(BCM2835SystemTimerState *s)
+static void bcm2835_systmr_timer_expire(void *opaque)
{
- bool enable = !!s->reg.ctrl_status;
+ BCM2835SystemTimerCompare *tmr = opaque;
- trace_bcm2835_systmr_irq(enable);
- qemu_set_irq(s->irq, enable);
-}
-
-static void bcm2835_systmr_update_compare(BCM2835SystemTimerState *s,
- unsigned timer_index)
-{
- /* TODO fow now, since neither Linux nor U-boot use these timers. */
- qemu_log_mask(LOG_UNIMP, "COMPARE register %u not implemented\n",
- timer_index);
+ trace_bcm2835_systmr_timer_expired(tmr->id);
+ tmr->state->reg.ctrl_status |= 1 << tmr->id;
+ qemu_set_irq(tmr->irq, 1);
}
static uint64_t bcm2835_systmr_read(void *opaque, hwaddr offset,
@@ -78,16 +71,25 @@ static void bcm2835_systmr_write(void *opaque, hwaddr offset,
uint64_t value, unsigned size)
{
BCM2835SystemTimerState *s = BCM2835_SYSTIMER(opaque);
+ int index;
trace_bcm2835_systmr_write(offset, value);
switch (offset) {
case A_CTRL_STATUS:
s->reg.ctrl_status &= ~value; /* Ack */
- bcm2835_systmr_update_irq(s);
+ for (index = 0; index < ARRAY_SIZE(s->tmr); index++) {
+ if (extract32(value, index, 1)) {
+ trace_bcm2835_systmr_irq_ack(index);
+ qemu_set_irq(s->tmr[index].irq, 0);
+ }
+ }
break;
case A_COMPARE0 ... A_COMPARE3:
- s->reg.compare[(offset - A_COMPARE0) >> 2] = value;
- bcm2835_systmr_update_compare(s, (offset - A_COMPARE0) >> 2);
+ index = (offset - A_COMPARE0) >> 2;
+ s->reg.compare[index] = value;
+ timer_mod(&s->tmr[index].timer, value);
+ trace_bcm2835_systmr_run(index,
+ value - qemu_clock_get_us(QEMU_CLOCK_VIRTUAL));
break;
case A_COUNTER_LOW:
case A_COUNTER_HIGH:
@@ -125,7 +127,14 @@ static void bcm2835_systmr_realize(DeviceState *dev, Error **errp)
memory_region_init_io(&s->iomem, OBJECT(dev), &bcm2835_systmr_ops,
s, "bcm2835-sys-timer", 0x20);
sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
- sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
+
+ for (size_t i = 0; i < ARRAY_SIZE(s->tmr); i++) {
+ s->tmr[i].id = i;
+ s->tmr[i].state = s;
+ sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->tmr[i].irq);
+ timer_init_us(&s->tmr[i].timer, QEMU_CLOCK_VIRTUAL,
+ bcm2835_systmr_timer_expire, &s->tmr[i]);
+ }
}
static const VMStateDescription bcm2835_systmr_vmstate = {
diff --git a/hw/timer/trace-events b/hw/timer/trace-events
index b996d992000..f4ca31d4951 100644
--- a/hw/timer/trace-events
+++ b/hw/timer/trace-events
@@ -77,9 +77,11 @@ nrf51_timer_write(uint8_t timer_id, uint64_t addr, uint32_t value, unsigned size
nrf51_timer_set_count(uint8_t timer_id, uint8_t counter_id, uint32_t value) "timer %u counter %u count 0x%" PRIx32
# bcm2835_systmr.c
-bcm2835_systmr_irq(bool enable) "timer irq state %u"
+bcm2835_systmr_timer_expired(unsigned id) "timer #%u expired"
+bcm2835_systmr_irq_ack(unsigned id) "timer #%u acked"
bcm2835_systmr_read(uint64_t offset, uint64_t data) "timer read: offset 0x%" PRIx64 " data 0x%" PRIx64
bcm2835_systmr_write(uint64_t offset, uint64_t data) "timer write: offset 0x%" PRIx64 " data 0x%" PRIx64
+bcm2835_systmr_run(unsigned id, uint64_t delay_us) "timer #%u expiring in %"PRIu64" us"
# avr_timer16.c
avr_timer16_read(uint8_t addr, uint8_t value) "timer16 read addr:%u value:%u"
--
2.26.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 4/5] hw/timer/bcm2835: Support the timer COMPARE registers
2020-09-21 3:52 ` [PATCH v2 4/5] hw/timer/bcm2835: Support the timer COMPARE registers Philippe Mathieu-Daudé
@ 2020-09-21 19:39 ` Luc Michel
2020-09-25 12:57 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 13+ messages in thread
From: Luc Michel @ 2020-09-21 19:39 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Peter Maydell, Niek Linnenbank, qemu-arm, Andrew Baumann, Paul Zimmerman
Hi Phil,
On 9/21/20 5:52 AM, Philippe Mathieu-Daudé wrote:
> This peripheral has 1 free-running timer and 4 compare registers.
>
> Only the free-running timer is implemented. Add support the
> COMPARE registers (each register is wired to an IRQ).
>
> Reference: "BCM2835 ARM Peripherals" datasheet [*]
> chapter 12 "System Timer":
>
> The System Timer peripheral provides four 32-bit timer channels
> and a single 64-bit free running counter. Each channel has an
> output compare register, which is compared against the 32 least
> significant bits of the free running counter values. When the
> two values match, the system timer peripheral generates a signal
> to indicate a match for the appropriate channel. The match signal
> is then fed into the interrupt controller.
>
> This peripheral is used since Linux 3.7, commit ee4af5696720
> ("ARM: bcm2835: add system timer").
>
> [*] https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> include/hw/timer/bcm2835_systmr.h | 11 +++++++--
> hw/timer/bcm2835_systmr.c | 41 +++++++++++++++++++------------
> hw/timer/trace-events | 4 ++-
> 3 files changed, 37 insertions(+), 19 deletions(-)
>
> diff --git a/include/hw/timer/bcm2835_systmr.h b/include/hw/timer/bcm2835_systmr.h
> index e0db9e9e12b..17fdd9d67b2 100644
> --- a/include/hw/timer/bcm2835_systmr.h
> +++ b/include/hw/timer/bcm2835_systmr.h
> @@ -11,6 +11,7 @@
>
> #include "hw/sysbus.h"
> #include "hw/irq.h"
> +#include "qemu/timer.h"
> #include "qom/object.h"
>
> #define TYPE_BCM2835_SYSTIMER "bcm2835-sys-timer"
> @@ -20,18 +21,24 @@ DECLARE_INSTANCE_CHECKER(BCM2835SystemTimerState, BCM2835_SYSTIMER,
>
> #define BCM2835_SYSTIMER_COUNT 4
>
> +typedef struct {
> + unsigned id;
> + QEMUTimer timer;
> + qemu_irq irq;
> + BCM2835SystemTimerState *state;
> +} BCM2835SystemTimerCompare;
> +
> struct BCM2835SystemTimerState {
> /*< private >*/
> SysBusDevice parent_obj;
>
> /*< public >*/
> MemoryRegion iomem;
> - qemu_irq irq;
> -
> struct {
> uint32_t ctrl_status;
> uint32_t compare[BCM2835_SYSTIMER_COUNT];
> } reg;
> + BCM2835SystemTimerCompare tmr[BCM2835_SYSTIMER_COUNT];
> };
>
> #endif
> diff --git a/hw/timer/bcm2835_systmr.c b/hw/timer/bcm2835_systmr.c
> index b234e83824f..43e644f5e45 100644
> --- a/hw/timer/bcm2835_systmr.c
> +++ b/hw/timer/bcm2835_systmr.c
> @@ -28,20 +28,13 @@ REG32(COMPARE1, 0x10)
> REG32(COMPARE2, 0x14)
> REG32(COMPARE3, 0x18)
>
> -static void bcm2835_systmr_update_irq(BCM2835SystemTimerState *s)
> +static void bcm2835_systmr_timer_expire(void *opaque)
> {
> - bool enable = !!s->reg.ctrl_status;
> + BCM2835SystemTimerCompare *tmr = opaque;
>
> - trace_bcm2835_systmr_irq(enable);
> - qemu_set_irq(s->irq, enable);
> -}
> -
> -static void bcm2835_systmr_update_compare(BCM2835SystemTimerState *s,
> - unsigned timer_index)
> -{
> - /* TODO fow now, since neither Linux nor U-boot use these timers. */
> - qemu_log_mask(LOG_UNIMP, "COMPARE register %u not implemented\n",
> - timer_index);
> + trace_bcm2835_systmr_timer_expired(tmr->id);
> + tmr->state->reg.ctrl_status |= 1 << tmr->id;
> + qemu_set_irq(tmr->irq, 1);
> }
>
> static uint64_t bcm2835_systmr_read(void *opaque, hwaddr offset,
> @@ -78,16 +71,25 @@ static void bcm2835_systmr_write(void *opaque, hwaddr offset,
> uint64_t value, unsigned size)
> {
> BCM2835SystemTimerState *s = BCM2835_SYSTIMER(opaque);
> + int index;
>
> trace_bcm2835_systmr_write(offset, value);
> switch (offset) {
> case A_CTRL_STATUS:
> s->reg.ctrl_status &= ~value; /* Ack */
> - bcm2835_systmr_update_irq(s);
> + for (index = 0; index < ARRAY_SIZE(s->tmr); index++) {
> + if (extract32(value, index, 1)) {
> + trace_bcm2835_systmr_irq_ack(index);
> + qemu_set_irq(s->tmr[index].irq, 0);
> + }
> + }
> break;
> case A_COMPARE0 ... A_COMPARE3:
> - s->reg.compare[(offset - A_COMPARE0) >> 2] = value;
> - bcm2835_systmr_update_compare(s, (offset - A_COMPARE0) >> 2);
> + index = (offset - A_COMPARE0) >> 2;
> + s->reg.compare[index] = value;
> + timer_mod(&s->tmr[index].timer, value);
I think "value" is wrong here. timer_mod takes an absolute time value.
Here "value" is a 32 bits truncated view of "current_time + some_time".
> + trace_bcm2835_systmr_run(index,
> + value - qemu_clock_get_us(QEMU_CLOCK_VIRTUAL));
Here also.
I think you can do something like (untested):
{
uint64_t now, triggers_at;
uint32_t clo, triggers_in;
index = (offset - A_COMPARE0) >> 2;
s->reg.compare[index] = value;
/* get the current clock and its truncated 32 bits view */
now = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL);
clo = now;
/* will overflow in case clo > value. We still get the
correct value on 32 bits (which is "UINT32_MAX - (clo - value)") */
triggers_in = value - clo;
/* timer_mod takes an absolute time value, go back to 64
bits values */
triggers_at = now + triggers_in;
timer_mod(&s->tmr[index].timer, triggers_at);
trace_bcm2835_systmr_run(index, triggers_in);
}
The rest is OK to me.
--
Luc
> break;
> case A_COUNTER_LOW:
> case A_COUNTER_HIGH:
> @@ -125,7 +127,14 @@ static void bcm2835_systmr_realize(DeviceState *dev, Error **errp)
> memory_region_init_io(&s->iomem, OBJECT(dev), &bcm2835_systmr_ops,
> s, "bcm2835-sys-timer", 0x20);
> sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
> - sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
> +
> + for (size_t i = 0; i < ARRAY_SIZE(s->tmr); i++) {
> + s->tmr[i].id = i;
> + s->tmr[i].state = s;
> + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->tmr[i].irq);
> + timer_init_us(&s->tmr[i].timer, QEMU_CLOCK_VIRTUAL,
> + bcm2835_systmr_timer_expire, &s->tmr[i]);
> + }
> }
>
> static const VMStateDescription bcm2835_systmr_vmstate = {
> diff --git a/hw/timer/trace-events b/hw/timer/trace-events
> index b996d992000..f4ca31d4951 100644
> --- a/hw/timer/trace-events
> +++ b/hw/timer/trace-events
> @@ -77,9 +77,11 @@ nrf51_timer_write(uint8_t timer_id, uint64_t addr, uint32_t value, unsigned size
> nrf51_timer_set_count(uint8_t timer_id, uint8_t counter_id, uint32_t value) "timer %u counter %u count 0x%" PRIx32
>
> # bcm2835_systmr.c
> -bcm2835_systmr_irq(bool enable) "timer irq state %u"
> +bcm2835_systmr_timer_expired(unsigned id) "timer #%u expired"
> +bcm2835_systmr_irq_ack(unsigned id) "timer #%u acked"
> bcm2835_systmr_read(uint64_t offset, uint64_t data) "timer read: offset 0x%" PRIx64 " data 0x%" PRIx64
> bcm2835_systmr_write(uint64_t offset, uint64_t data) "timer write: offset 0x%" PRIx64 " data 0x%" PRIx64
> +bcm2835_systmr_run(unsigned id, uint64_t delay_us) "timer #%u expiring in %"PRIu64" us"
>
> # avr_timer16.c
> avr_timer16_read(uint8_t addr, uint8_t value) "timer16 read addr:%u value:%u"
>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 4/5] hw/timer/bcm2835: Support the timer COMPARE registers
2020-09-21 19:39 ` Luc Michel
@ 2020-09-25 12:57 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-25 12:57 UTC (permalink / raw)
To: Luc Michel, qemu-devel
Cc: Peter Maydell, Niek Linnenbank, qemu-arm, Andrew Baumann, Paul Zimmerman
On 9/21/20 9:39 PM, Luc Michel wrote:
> Hi Phil,
>
> On 9/21/20 5:52 AM, Philippe Mathieu-Daudé wrote:
>> This peripheral has 1 free-running timer and 4 compare registers.
>>
>> Only the free-running timer is implemented. Add support the
>> COMPARE registers (each register is wired to an IRQ).
>>
>> Reference: "BCM2835 ARM Peripherals" datasheet [*]
>> chapter 12 "System Timer":
>>
>> The System Timer peripheral provides four 32-bit timer channels
>> and a single 64-bit free running counter. Each channel has an
>> output compare register, which is compared against the 32 least
>> significant bits of the free running counter values. When the
>> two values match, the system timer peripheral generates a signal
>> to indicate a match for the appropriate channel. The match signal
>> is then fed into the interrupt controller.
>>
>> This peripheral is used since Linux 3.7, commit ee4af5696720
>> ("ARM: bcm2835: add system timer").
>>
>> [*]
>> https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
>>
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> include/hw/timer/bcm2835_systmr.h | 11 +++++++--
>> hw/timer/bcm2835_systmr.c | 41 +++++++++++++++++++------------
>> hw/timer/trace-events | 4 ++-
>> 3 files changed, 37 insertions(+), 19 deletions(-)
>>
>> diff --git a/include/hw/timer/bcm2835_systmr.h
>> b/include/hw/timer/bcm2835_systmr.h
>> index e0db9e9e12b..17fdd9d67b2 100644
>> --- a/include/hw/timer/bcm2835_systmr.h
>> +++ b/include/hw/timer/bcm2835_systmr.h
>> @@ -11,6 +11,7 @@
>> #include "hw/sysbus.h"
>> #include "hw/irq.h"
>> +#include "qemu/timer.h"
>> #include "qom/object.h"
>> #define TYPE_BCM2835_SYSTIMER "bcm2835-sys-timer"
>> @@ -20,18 +21,24 @@ DECLARE_INSTANCE_CHECKER(BCM2835SystemTimerState,
>> BCM2835_SYSTIMER,
>> #define BCM2835_SYSTIMER_COUNT 4
>> +typedef struct {
>> + unsigned id;
>> + QEMUTimer timer;
>> + qemu_irq irq;
>> + BCM2835SystemTimerState *state;
>> +} BCM2835SystemTimerCompare;
>> +
>> struct BCM2835SystemTimerState {
>> /*< private >*/
>> SysBusDevice parent_obj;
>> /*< public >*/
>> MemoryRegion iomem;
>> - qemu_irq irq;
>> -
>> struct {
>> uint32_t ctrl_status;
>> uint32_t compare[BCM2835_SYSTIMER_COUNT];
>> } reg;
>> + BCM2835SystemTimerCompare tmr[BCM2835_SYSTIMER_COUNT];
>> };
>> #endif
>> diff --git a/hw/timer/bcm2835_systmr.c b/hw/timer/bcm2835_systmr.c
>> index b234e83824f..43e644f5e45 100644
>> --- a/hw/timer/bcm2835_systmr.c
>> +++ b/hw/timer/bcm2835_systmr.c
>> @@ -28,20 +28,13 @@ REG32(COMPARE1, 0x10)
>> REG32(COMPARE2, 0x14)
>> REG32(COMPARE3, 0x18)
>> -static void bcm2835_systmr_update_irq(BCM2835SystemTimerState *s)
>> +static void bcm2835_systmr_timer_expire(void *opaque)
>> {
>> - bool enable = !!s->reg.ctrl_status;
>> + BCM2835SystemTimerCompare *tmr = opaque;
>> - trace_bcm2835_systmr_irq(enable);
>> - qemu_set_irq(s->irq, enable);
>> -}
>> -
>> -static void bcm2835_systmr_update_compare(BCM2835SystemTimerState *s,
>> - unsigned timer_index)
>> -{
>> - /* TODO fow now, since neither Linux nor U-boot use these timers. */
>> - qemu_log_mask(LOG_UNIMP, "COMPARE register %u not implemented\n",
>> - timer_index);
>> + trace_bcm2835_systmr_timer_expired(tmr->id);
>> + tmr->state->reg.ctrl_status |= 1 << tmr->id;
>> + qemu_set_irq(tmr->irq, 1);
>> }
>> static uint64_t bcm2835_systmr_read(void *opaque, hwaddr offset,
>> @@ -78,16 +71,25 @@ static void bcm2835_systmr_write(void *opaque,
>> hwaddr offset,
>> uint64_t value, unsigned size)
>> {
>> BCM2835SystemTimerState *s = BCM2835_SYSTIMER(opaque);
>> + int index;
>> trace_bcm2835_systmr_write(offset, value);
>> switch (offset) {
>> case A_CTRL_STATUS:
>> s->reg.ctrl_status &= ~value; /* Ack */
>> - bcm2835_systmr_update_irq(s);
>> + for (index = 0; index < ARRAY_SIZE(s->tmr); index++) {
>> + if (extract32(value, index, 1)) {
>> + trace_bcm2835_systmr_irq_ack(index);
>> + qemu_set_irq(s->tmr[index].irq, 0);
>> + }
>> + }
>> break;
>> case A_COMPARE0 ... A_COMPARE3:
>> - s->reg.compare[(offset - A_COMPARE0) >> 2] = value;
>> - bcm2835_systmr_update_compare(s, (offset - A_COMPARE0) >> 2);
>> + index = (offset - A_COMPARE0) >> 2;
>> + s->reg.compare[index] = value;
>> + timer_mod(&s->tmr[index].timer, value);
>
> I think "value" is wrong here. timer_mod takes an absolute time value.
> Here "value" is a 32 bits truncated view of "current_time + some_time".
Yes, you are correct. The reduced datasheet is not easy to understand.
>> + trace_bcm2835_systmr_run(index,
>> + value -
>> qemu_clock_get_us(QEMU_CLOCK_VIRTUAL));
> Here also.
>
> I think you can do something like (untested):
> {
> uint64_t now, triggers_at;
> uint32_t clo, triggers_in;
>
> index = (offset - A_COMPARE0) >> 2;
> s->reg.compare[index] = value;
>
> /* get the current clock and its truncated 32 bits view */
> now = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL);
> clo = now;
>
> /* will overflow in case clo > value. We still get the
> correct value on 32 bits (which is "UINT32_MAX - (clo - value)") */
> triggers_in = value - clo;
>
> /* timer_mod takes an absolute time value, go back to 64
> bits values */
> triggers_at = now + triggers_in;
> timer_mod(&s->tmr[index].timer, triggers_at);
>
> trace_bcm2835_systmr_run(index, triggers_in);
This matches the datasheet description, thanks!
> }
>
>
> The rest is OK to me.
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 5/5] hw/arm/bcm2835_peripherals: Correctly wire the SYS_timer IRQs
2020-09-21 3:52 [PATCH v2 0/5] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+ Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2020-09-21 3:52 ` [PATCH v2 4/5] hw/timer/bcm2835: Support the timer COMPARE registers Philippe Mathieu-Daudé
@ 2020-09-21 3:52 ` Philippe Mathieu-Daudé
2020-09-21 19:50 ` Luc Michel
2020-09-24 11:21 ` [PATCH v2 0/5] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+ Philippe Mathieu-Daudé
5 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-21 3:52 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé,
Andrew Baumann, Paul Zimmerman, Niek Linnenbank, qemu-arm,
Luc Michel
The SYS_timer is not directly wired to the ARM core, but to the
SoC (peripheral) interrupt controller.
Fixes: 0e5bbd74064 ("hw/arm/bcm2835_peripherals: Use the SYS_timer")
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/arm/bcm2835_peripherals.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 15c5c72e465..48909a43c32 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -171,8 +171,17 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
memory_region_add_subregion(&s->peri_mr, ST_OFFSET,
sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->systmr), 0));
sysbus_connect_irq(SYS_BUS_DEVICE(&s->systmr), 0,
- qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_ARM_IRQ,
- INTERRUPT_ARM_TIMER));
+ qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ,
+ INTERRUPT_TIMER0));
+ sysbus_connect_irq(SYS_BUS_DEVICE(&s->systmr), 1,
+ qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ,
+ INTERRUPT_TIMER1));
+ sysbus_connect_irq(SYS_BUS_DEVICE(&s->systmr), 2,
+ qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ,
+ INTERRUPT_TIMER2));
+ sysbus_connect_irq(SYS_BUS_DEVICE(&s->systmr), 3,
+ qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ,
+ INTERRUPT_TIMER3));
/* UART0 */
qdev_prop_set_chr(DEVICE(&s->uart0), "chardev", serial_hd(0));
--
2.26.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 5/5] hw/arm/bcm2835_peripherals: Correctly wire the SYS_timer IRQs
2020-09-21 3:52 ` [PATCH v2 5/5] hw/arm/bcm2835_peripherals: Correctly wire the SYS_timer IRQs Philippe Mathieu-Daudé
@ 2020-09-21 19:50 ` Luc Michel
0 siblings, 0 replies; 13+ messages in thread
From: Luc Michel @ 2020-09-21 19:50 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Peter Maydell, Niek Linnenbank, qemu-arm, Andrew Baumann, Paul Zimmerman
On 9/21/20 5:52 AM, Philippe Mathieu-Daudé wrote:
> The SYS_timer is not directly wired to the ARM core, but to the
> SoC (peripheral) interrupt controller.
>
> Fixes: 0e5bbd74064 ("hw/arm/bcm2835_peripherals: Use the SYS_timer")
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Luc Michel <luc.michel@greensocs.com>
> ---
> hw/arm/bcm2835_peripherals.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
> index 15c5c72e465..48909a43c32 100644
> --- a/hw/arm/bcm2835_peripherals.c
> +++ b/hw/arm/bcm2835_peripherals.c
> @@ -171,8 +171,17 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
> memory_region_add_subregion(&s->peri_mr, ST_OFFSET,
> sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->systmr), 0));
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->systmr), 0,
> - qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_ARM_IRQ,
> - INTERRUPT_ARM_TIMER));
> + qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ,
> + INTERRUPT_TIMER0));
> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->systmr), 1,
> + qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ,
> + INTERRUPT_TIMER1));
> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->systmr), 2,
> + qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ,
> + INTERRUPT_TIMER2));
> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->systmr), 3,
> + qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ,
> + INTERRUPT_TIMER3));
>
> /* UART0 */
> qdev_prop_set_chr(DEVICE(&s->uart0), "chardev", serial_hd(0));
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/5] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+
2020-09-21 3:52 [PATCH v2 0/5] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+ Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2020-09-21 3:52 ` [PATCH v2 5/5] hw/arm/bcm2835_peripherals: Correctly wire the SYS_timer IRQs Philippe Mathieu-Daudé
@ 2020-09-24 11:21 ` Philippe Mathieu-Daudé
5 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-24 11:21 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Baumann, Paul Zimmerman, Niek Linnenbank,
qemu-arm, Luc Michel
On 9/21/20 5:52 AM, Philippe Mathieu-Daudé wrote:
> In this series we implement the COMPARE registers of the
> SYS_timer, since they are used by Linux.
>
> This fixes the hang reported by Niek here:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg682090.html
>
> Since v1:
> - Extracted unrelated patches to previous series
> (which happened to be mis-rebased)
>
> Based-on: <20200921034729.432931-1-f4bug@amsat.org>
> Supersedes: <20200920175825.417680-1-f4bug@amsat.org>
>
> Philippe Mathieu-Daudé (5):
> hw/intc/bcm2835_ic: Trace GPU/CPU IRQ handlers
> hw/timer/bcm2835: Introduce BCM2835_SYSTIMER_COUNT definition
> hw/timer/bcm2835: Rename variable holding CTRL_STATUS register
> hw/timer/bcm2835: Support the timer COMPARE registers
I'll respin this series trying to address Luc's comments on patch 4.
^ permalink raw reply [flat|nested] 13+ messages in thread