[PATCH] hw/timer/armv7m_systick: assert that board code set system_clock_scale

[PATCH] hw/timer/armv7m_systick: assert that board code set system_clock_scale

[Peter Maydell]

It is the responsibility of board code for an armv7m system to set
system_clock_scale appropriately for the CPU speed of the core.
If it forgets to do this, then QEMU will hang if the guest tries
to use the systick timer in the "tick at the CPU clock frequency" mode.

We forgot that in a couple of our boards (see commits ce4f70e81ed23c93f,
e7e5a9595ab1136). Add an assertion in the systick reset method so
we don't let any new boards in with the same bug.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
In the longer term we should make this a clocktree input and
plumb it through the armv7m container and so on, but for the
moment this assert() improves the current situation.
---
 hw/timer/armv7m_systick.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/timer/armv7m_systick.c b/hw/timer/armv7m_systick.c
index 74c58bcf245..a8cec7eb56b 100644
--- a/hw/timer/armv7m_systick.c
+++ b/hw/timer/armv7m_systick.c
@@ -202,6 +202,14 @@ static void systick_reset(DeviceState *dev)
 {
     SysTickState *s = SYSTICK(dev);
 
+    /*
+     * Forgetting to set system_clock_scale is always a board code
+     * bug. We can't check this earlier because for some boards
+     * (like stellaris) it is not yet configured at the point where
+     * the systick device is realized.
+     */
+    assert(system_clock_scale != 0);
+
     s->control = 0;
     s->reload = 0;
     s->tick = 0;
-- 
2.20.1

Re: [PATCH] hw/timer/armv7m_systick: assert that board code set system_clock_scale

[Philippe Mathieu-Daudé]

[-- Attachment #1: Type: text/plain, Size: 1685 bytes --]

Le mar. 25 août 2020 18:09, Peter Maydell <peter.maydell@linaro.org> a
écrit :

> It is the responsibility of board code for an armv7m system to set
> system_clock_scale appropriately for the CPU speed of the core.
> If it forgets to do this, then QEMU will hang if the guest tries
> to use the systick timer in the "tick at the CPU clock frequency" mode.
>
> We forgot that in a couple of our boards (see commits ce4f70e81ed23c93f,
> e7e5a9595ab1136). Add an assertion in the systick reset method so
> we don't let any new boards in with the same bug.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>


Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

---
> In the longer term we should make this a clocktree input and
> plumb it through the armv7m container and so on, but for the
> moment this assert() improves the current situation.
> ---
>  hw/timer/armv7m_systick.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/hw/timer/armv7m_systick.c b/hw/timer/armv7m_systick.c
> index 74c58bcf245..a8cec7eb56b 100644
> --- a/hw/timer/armv7m_systick.c
> +++ b/hw/timer/armv7m_systick.c
> @@ -202,6 +202,14 @@ static void systick_reset(DeviceState *dev)
>  {
>      SysTickState *s = SYSTICK(dev);
>
> +    /*
> +     * Forgetting to set system_clock_scale is always a board code
> +     * bug. We can't check this earlier because for some boards
> +     * (like stellaris) it is not yet configured at the point where
> +     * the systick device is realized.
> +     */
> +    assert(system_clock_scale != 0);
> +
>      s->control = 0;
>      s->reload = 0;
>      s->tick = 0;
> --
> 2.20.1
>
>
>

[-- Attachment #2: Type: text/html, Size: 2806 bytes --]

[PATCH 00/10] target/arm: Various v8.1M minor features

[Peter Maydell]

This patchseries implements various minor v8.1M new features,
notably the branch-future and low-overhead-loop extensions.

(None of this will get enabled until we have enough to implement
a CPU model which has v8.1M, which will be the Cortex-M55, but
as usual we can get stuff into the tree gradually.)

Patch 1 is a decodetree fix suggested by Richard that is
necessary to avoid wrong-decode of the changes to t32.decode
by later patches.

thanks
-- PMM

Peter Maydell (10):
  decodetree: Fix codegen for non-overlapping group inside overlapping
    group
  target/arm: Implement v8.1M NOCP handling
  target/arm: Implement v8.1M conditional-select insns
  target/arm: Make the t32 insn[25:23]=111 group non-overlapping
  target/arm: Don't allow BLX imm for M-profile
  target/arm: Implement v8.1M branch-future insns (as NOPs)
  target/arm: Implement v8.1M low-overhead-loop instructions
  target/arm: Fix has_vfp/has_neon ID reg squashing for M-profile
  target/arm: Implement FPSCR.LTPSIZE for M-profile LOB extension
  target/arm: Fix writing to FPSCR.FZ16 on M-profile

 target/arm/cpu.h               |   7 ++
 target/arm/m-nocp.decode       |  10 ++-
 target/arm/t32.decode          |  50 +++++++----
 target/arm/cpu.c               |  34 ++++---
 target/arm/translate.c         | 157 +++++++++++++++++++++++++++++++++
 target/arm/vfp_helper.c        |  30 +++++--
 scripts/decodetree.py          |   2 +-
 target/arm/translate-vfp.c.inc |  17 +++-
 8 files changed, 268 insertions(+), 39 deletions(-)

-- 
2.20.1

[PATCH] hw/timer/armv7m_systick: assert that board code set system_clock_scale

[Peter Maydell]

It is the responsibility of board code for an armv7m system to set
system_clock_scale appropriately for the CPU speed of the core.
If it forgets to do this, then QEMU will hang if the guest tries
to use the systick timer in the "tick at the CPU clock frequency" mode.

We forgot that in a couple of our boards (see commits ce4f70e81ed23c93f,
e7e5a9595ab1136). Add an assertion in the systick reset method so
we don't let any new boards in with the same bug.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
In the longer term we should make this a clocktree input and
plumb it through the armv7m container and so on, but for the
moment this assert() improves the current situation.
---
 hw/timer/armv7m_systick.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/timer/armv7m_systick.c b/hw/timer/armv7m_systick.c
index 74c58bcf245..a8cec7eb56b 100644
--- a/hw/timer/armv7m_systick.c
+++ b/hw/timer/armv7m_systick.c
@@ -202,6 +202,14 @@ static void systick_reset(DeviceState *dev)
 {
     SysTickState *s = SYSTICK(dev);
 
+    /*
+     * Forgetting to set system_clock_scale is always a board code
+     * bug. We can't check this earlier because for some boards
+     * (like stellaris) it is not yet configured at the point where
+     * the systick device is realized.
+     */
+    assert(system_clock_scale != 0);
+
     s->control = 0;
     s->reload = 0;
     s->tick = 0;
-- 
2.20.1