qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/4] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+
@ 2020-10-10 20:37 Philippe Mathieu-Daudé
  2020-10-10 20:37 ` [PATCH v4 1/4] hw/timer/bcm2835: Introduce BCM2835_SYSTIMER_COUNT definition Philippe Mathieu-Daudé
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-10 20:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, qemu-arm, Richard Henderson

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 v3:
- Addressed Richard review comments (arithmetic fix)

Since v2:
- Fixed issue in COMPARE register reported by Luc

Since v1:
- Extracted unrelated patches to previous series
  (which happened to be mis-rebased)

Philippe Mathieu-Daudé (4):
  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
  hw/arm/bcm2835_peripherals: Correctly wire the SYS_timer IRQs

 include/hw/timer/bcm2835_systmr.h | 17 ++++++---
 hw/arm/bcm2835_peripherals.c      | 13 +++++--
 hw/timer/bcm2835_systmr.c         | 57 +++++++++++++++++++------------
 hw/timer/trace-events             |  6 ++--
 4 files changed, 64 insertions(+), 29 deletions(-)

-- 
2.26.2



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v4 1/4] hw/timer/bcm2835: Introduce BCM2835_SYSTIMER_COUNT definition
  2020-10-10 20:37 [PATCH v4 0/4] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+ Philippe Mathieu-Daudé
@ 2020-10-10 20:37 ` Philippe Mathieu-Daudé
  2020-10-10 20:37 ` [PATCH v4 2/4] hw/timer/bcm2835: Rename variable holding CTRL_STATUS register Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-10 20:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Richard Henderson,
	Philippe Mathieu-Daudé, Andrew Baumann, Paul Zimmerman,
	qemu-arm, Luc Michel, Richard Henderson

Use the BCM2835_SYSTIMER_COUNT definition instead of the
magic '4' value.

Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
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 7ce8f6ef4d8..43df7ee488c 100644
--- a/include/hw/timer/bcm2835_systmr.h
+++ b/include/hw/timer/bcm2835_systmr.h
@@ -16,6 +16,8 @@
 #define TYPE_BCM2835_SYSTIMER "bcm2835-sys-timer"
 OBJECT_DECLARE_SIMPLE_TYPE(BCM2835SystemTimerState, BCM2835_SYSTIMER)
 
+#define BCM2835_SYSTIMER_COUNT 4
+
 struct BCM2835SystemTimerState {
     /*< private >*/
     SysBusDevice parent_obj;
@@ -26,7 +28,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] 7+ messages in thread

* [PATCH v4 2/4] hw/timer/bcm2835: Rename variable holding CTRL_STATUS register
  2020-10-10 20:37 [PATCH v4 0/4] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+ Philippe Mathieu-Daudé
  2020-10-10 20:37 ` [PATCH v4 1/4] hw/timer/bcm2835: Introduce BCM2835_SYSTIMER_COUNT definition Philippe Mathieu-Daudé
@ 2020-10-10 20:37 ` Philippe Mathieu-Daudé
  2020-10-10 20:37 ` [PATCH v4 3/4] hw/timer/bcm2835: Support the timer COMPARE registers Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-10 20:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Richard Henderson,
	Philippe Mathieu-Daudé, Andrew Baumann, Paul Zimmerman,
	qemu-arm, Luc Michel, Richard Henderson

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.

Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
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 43df7ee488c..f15788a78d8 100644
--- a/include/hw/timer/bcm2835_systmr.h
+++ b/include/hw/timer/bcm2835_systmr.h
@@ -27,7 +27,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] 7+ messages in thread

* [PATCH v4 3/4] hw/timer/bcm2835: Support the timer COMPARE registers
  2020-10-10 20:37 [PATCH v4 0/4] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+ Philippe Mathieu-Daudé
  2020-10-10 20:37 ` [PATCH v4 1/4] hw/timer/bcm2835: Introduce BCM2835_SYSTIMER_COUNT definition Philippe Mathieu-Daudé
  2020-10-10 20:37 ` [PATCH v4 2/4] hw/timer/bcm2835: Rename variable holding CTRL_STATUS register Philippe Mathieu-Daudé
@ 2020-10-10 20:37 ` Philippe Mathieu-Daudé
  2020-10-12  7:30   ` Luc Michel
  2020-10-10 20:37 ` [PATCH v4 4/4] hw/arm/bcm2835_peripherals: Correctly wire the SYS_timer IRQs Philippe Mathieu-Daudé
  2020-10-19 15:47 ` [PATCH v4 0/4] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+ Peter Maydell
  4 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-10 20:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Philippe Mathieu-Daudé,
	Andrew Baumann, Paul Zimmerman, qemu-arm, Richard Henderson

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>
---
v4:
- Fix arithmetic to correctly get value in future (Richard)
- Use 32-bit argument value (Richard)
v3:
- Only compare 32 least significant bits of the free running
  counter values (Luc)
---
 include/hw/timer/bcm2835_systmr.h | 11 +++++--
 hw/timer/bcm2835_systmr.c         | 48 ++++++++++++++++++++-----------
 hw/timer/trace-events             |  6 ++--
 3 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/include/hw/timer/bcm2835_systmr.h b/include/hw/timer/bcm2835_systmr.h
index f15788a78d8..bd3097d746b 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"
@@ -18,18 +19,24 @@ OBJECT_DECLARE_SIMPLE_TYPE(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..67669a57ff3 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,
@@ -75,19 +68,33 @@ static uint64_t bcm2835_systmr_read(void *opaque, hwaddr offset,
 }
 
 static void bcm2835_systmr_write(void *opaque, hwaddr offset,
-                                 uint64_t value, unsigned size)
+                                 uint64_t value64, unsigned size)
 {
     BCM2835SystemTimerState *s = BCM2835_SYSTIMER(opaque);
+    int index;
+    uint32_t value = value64;
+    uint32_t triggers_delay_us;
+    uint64_t now;
 
     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;
+        now = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL);
+        /* Compare lower 32-bits of the free-running counter. */
+        triggers_delay_us = value - now;
+        trace_bcm2835_systmr_run(index, triggers_delay_us);
+        timer_mod(&s->tmr[index].timer, now + triggers_delay_us);
         break;
     case A_COUNTER_LOW:
     case A_COUNTER_HIGH:
@@ -125,7 +132,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..7a4326d9566 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_write(uint64_t offset, uint32_t data) "timer write: offset 0x%" PRIx64 " data 0x%" PRIx32
+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] 7+ messages in thread

* [PATCH v4 4/4] hw/arm/bcm2835_peripherals: Correctly wire the SYS_timer IRQs
  2020-10-10 20:37 [PATCH v4 0/4] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+ Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2020-10-10 20:37 ` [PATCH v4 3/4] hw/timer/bcm2835: Support the timer COMPARE registers Philippe Mathieu-Daudé
@ 2020-10-10 20:37 ` Philippe Mathieu-Daudé
  2020-10-19 15:47 ` [PATCH v4 0/4] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+ Peter Maydell
  4 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-10 20:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Luc Michel, Richard Henderson,
	Philippe Mathieu-Daudé, Andrew Baumann, Paul Zimmerman,
	qemu-arm, Luc Michel, Richard Henderson

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")
Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
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] 7+ messages in thread

* Re: [PATCH v4 3/4] hw/timer/bcm2835: Support the timer COMPARE registers
  2020-10-10 20:37 ` [PATCH v4 3/4] hw/timer/bcm2835: Support the timer COMPARE registers Philippe Mathieu-Daudé
@ 2020-10-12  7:30   ` Luc Michel
  0 siblings, 0 replies; 7+ messages in thread
From: Luc Michel @ 2020-10-12  7:30 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, qemu-devel, Andrew Baumann, Paul Zimmerman,
	qemu-arm, Richard Henderson

On 22:37 Sat 10 Oct     , 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>

Reviewed-by: Luc Michel <luc@lmichel.fr>

> ---
> v4:
> - Fix arithmetic to correctly get value in future (Richard)
> - Use 32-bit argument value (Richard)
> v3:
> - Only compare 32 least significant bits of the free running
>   counter values (Luc)
> ---
>  include/hw/timer/bcm2835_systmr.h | 11 +++++--
>  hw/timer/bcm2835_systmr.c         | 48 ++++++++++++++++++++-----------
>  hw/timer/trace-events             |  6 ++--
>  3 files changed, 44 insertions(+), 21 deletions(-)
> 
> diff --git a/include/hw/timer/bcm2835_systmr.h b/include/hw/timer/bcm2835_systmr.h
> index f15788a78d8..bd3097d746b 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"
> @@ -18,18 +19,24 @@ OBJECT_DECLARE_SIMPLE_TYPE(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..67669a57ff3 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,
> @@ -75,19 +68,33 @@ static uint64_t bcm2835_systmr_read(void *opaque, hwaddr offset,
>  }
>  
>  static void bcm2835_systmr_write(void *opaque, hwaddr offset,
> -                                 uint64_t value, unsigned size)
> +                                 uint64_t value64, unsigned size)
>  {
>      BCM2835SystemTimerState *s = BCM2835_SYSTIMER(opaque);
> +    int index;
> +    uint32_t value = value64;
> +    uint32_t triggers_delay_us;
> +    uint64_t now;
>  
>      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;
> +        now = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL);
> +        /* Compare lower 32-bits of the free-running counter. */
> +        triggers_delay_us = value - now;
> +        trace_bcm2835_systmr_run(index, triggers_delay_us);
> +        timer_mod(&s->tmr[index].timer, now + triggers_delay_us);
>          break;
>      case A_COUNTER_LOW:
>      case A_COUNTER_HIGH:
> @@ -125,7 +132,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..7a4326d9566 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_write(uint64_t offset, uint32_t data) "timer write: offset 0x%" PRIx64 " data 0x%" PRIx32
> +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	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 0/4] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+
  2020-10-10 20:37 [PATCH v4 0/4] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+ Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2020-10-10 20:37 ` [PATCH v4 4/4] hw/arm/bcm2835_peripherals: Correctly wire the SYS_timer IRQs Philippe Mathieu-Daudé
@ 2020-10-19 15:47 ` Peter Maydell
  4 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2020-10-19 15:47 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Luc Michel, QEMU Developers, Andrew Baumann, Paul Zimmerman,
	qemu-arm, Richard Henderson

On Sat, 10 Oct 2020 at 21:37, Philippe Mathieu-Daudé <f4bug@amsat.org> 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
>


Applied to target-arm.next, thanks.

-- PMM


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-10-19 15:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-10 20:37 [PATCH v4 0/4] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+ Philippe Mathieu-Daudé
2020-10-10 20:37 ` [PATCH v4 1/4] hw/timer/bcm2835: Introduce BCM2835_SYSTIMER_COUNT definition Philippe Mathieu-Daudé
2020-10-10 20:37 ` [PATCH v4 2/4] hw/timer/bcm2835: Rename variable holding CTRL_STATUS register Philippe Mathieu-Daudé
2020-10-10 20:37 ` [PATCH v4 3/4] hw/timer/bcm2835: Support the timer COMPARE registers Philippe Mathieu-Daudé
2020-10-12  7:30   ` Luc Michel
2020-10-10 20:37 ` [PATCH v4 4/4] hw/arm/bcm2835_peripherals: Correctly wire the SYS_timer IRQs Philippe Mathieu-Daudé
2020-10-19 15:47 ` [PATCH v4 0/4] hw/arm/raspi: Fix SYS_timer to unbrick Linux kernels v3.7+ 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).