Series comparison

-[PULL 0/3] target-arm queue
+[PULL 0/8] target-arm queue
-Arm patches for rc1:
+Handful of bugfixes for rc2. None of these are particularly critical
- * two final "remove the old API" patches for some API transitions
+or exciting.
  * bugfix for raspi/highbank Linux boot
-thanks
 -- PMM
-The following changes since commit 654efcb511d394c1d3f5292c28503d1d19e5b1d3:
+The following changes since commit 45a150aa2b3492acf6691c7bdbeb25a8545d8345:
-  Merge remote-tracking branch 'remotes/vivier/tags/q800-branch-pull-request' into staging (2019-11-11 09:23:46 +0000)
+  Merge remote-tracking branch 'remotes/ericb/tags/pull-bitmaps-2020-08-03' into staging (2020-08-03 15:13:49 +0100)
 are available in the Git repository at:
-  https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20191111
+  https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20200803
-for you to fetch changes up to 45c078f163fd47c35e7505d98928fae63baada7d:
+for you to fetch changes up to 13557fd392890cbd985bceba7f717e01efd674b8:
-  hw/arm/boot: Set NSACR.{CP11, CP10} in dummy SMC setup routine (2019-11-11 13:44:16 +0000)
+  hw/timer/imx_epit: Avoid assertion when CR.SWR is written (2020-08-03 17:56:11 +0100)
 ----------------------------------------------------------------
 target-arm queue:
- * Remove old unassigned_access CPU hook API
+ * hw/timer/imx_epit: Avoid assertion when CR.SWR is written
- * Remove old ptimer_init_with_bh() API
+ * netduino2, netduinoplus2, microbit: set system_clock_scale so that
- * hw/arm/boot: Set NSACR.{CP11, CP10} in dummy SMC setup routine
+   SysTick running on the CPU clock works
  * target/arm: Avoid maybe-uninitialized warning with gcc 4.9
  * target/arm: Fix AddPAC error indication
  * Make AIRCR.SYSRESETREQ actually reset the system for the
    microbit, mps2-*, musca-*, netduino* boards
 ----------------------------------------------------------------
-Clement Deschamps (1):
+Kaige Li (1):
-      hw/arm/boot: Set NSACR.{CP11, CP10} in dummy SMC setup routine
+      target/arm: Avoid maybe-uninitialized warning with gcc 4.9
-Peter Maydell (2):
+Peter Maydell (6):
-      ptimer: Remove old ptimer_init_with_bh() API
+      hw/arm/netduino2, netduinoplus2: Set system_clock_scale
-      Remove unassigned_access CPU hook
+      include/hw/irq.h: New function qemu_irq_is_connected()
       hw/intc/armv7m_nvic: Provide default "reset the system" behaviour for SYSRESETREQ
       msf2-soc, stellaris: Don't wire up SYSRESETREQ
       hw/arm/nrf51_soc: Set system_clock_scale
       hw/timer/imx_epit: Avoid assertion when CR.SWR is written
- include/hw/arm/boot.h |  7 ++--
+Richard Henderson (1):
- include/hw/core/cpu.h | 24 --------------
+      target/arm: Fix AddPAC error indication
  include/hw/ptimer.h   | 45 ++++++++++++-------------
  accel/tcg/cputlb.c    |  2 --
  hw/arm/boot.c         |  3 ++
  hw/core/ptimer.c      | 91 +++++++++------------------------------------------
  memory.c              |  7 ----
 files changed, 44 insertions(+), 135 deletions(-)
+ include/hw/arm/armv7m.h           |  4 +++-
+ include/hw/irq.h                  | 18 ++++++++++++++++++
+ hw/arm/msf2-soc.c                 | 11 -----------
+ hw/arm/netduino2.c                | 10 ++++++++++
+ hw/arm/netduinoplus2.c            | 10 ++++++++++
+ hw/arm/nrf51_soc.c                |  5 +++++
+ hw/arm/stellaris.c                | 12 ------------
+ hw/intc/armv7m_nvic.c             | 17 ++++++++++++++++-
+ hw/timer/imx_epit.c               | 13 ++++++++++---
+ target/arm/pauth_helper.c         |  6 +++++-
+ target/arm/translate-a64.c        |  2 +-
+ tests/tcg/aarch64/pauth-5.c       | 33 +++++++++++++++++++++++++++++++++
+ tests/tcg/aarch64/Makefile.target |  2 +-
+files changed, 112 insertions(+), 31 deletions(-)
+ create mode 100644 tests/tcg/aarch64/pauth-5.c

-New patch
+[PULL 1/8] hw/arm/netduino2, netduinoplus2: Set system_clock_scale
+The netduino2 and netduinoplus2 boards forgot to set the system_clock_scale
+global, which meant that if guest code used the systick timer in "use
+the processor clock" mode it would hang because time never advances.
+Set the global to match the documented CPU clock speed of these boards.
+Judging by the data sheet this is slightly simplistic because the
+SoC allows configuration of the SYSCLK source and frequency via the
+RCC (reset and clock control) module, but we don't model that.
+Fixes: https://bugs.launchpad.net/qemu/+bug/1876187
+Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
+Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
+Message-id: 20200727162617.26227-1-peter.maydell@linaro.org
+---
+ hw/arm/netduino2.c     | 10 ++++++++++
+ hw/arm/netduinoplus2.c | 10 ++++++++++
+files changed, 20 insertions(+)
+diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
+index XXXXXXX..XXXXXXX 100644
+--- a/hw/arm/netduino2.c
++++ b/hw/arm/netduino2.c
+@@ -XXX,XX +XXX,XX @@
+ #include "hw/arm/stm32f205_soc.h"
+ #include "hw/arm/boot.h"
++/* Main SYSCLK frequency in Hz (120MHz) */
++#define SYSCLK_FRQ 120000000ULL
++
+ static void netduino2_init(MachineState *machine)
+ {
+     DeviceState *dev;
++    /*
++     * TODO: ideally we would model the SoC RCC and let it handle
++     * system_clock_scale, including its ability to define different
++     * possible SYSCLK sources.
++     */
++    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
++
+     dev = qdev_new(TYPE_STM32F205_SOC);
+     qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
+     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+diff --git a/hw/arm/netduinoplus2.c b/hw/arm/netduinoplus2.c
+index XXXXXXX..XXXXXXX 100644
+--- a/hw/arm/netduinoplus2.c
++++ b/hw/arm/netduinoplus2.c
+@@ -XXX,XX +XXX,XX @@
+ #include "hw/arm/stm32f405_soc.h"
+ #include "hw/arm/boot.h"
++/* Main SYSCLK frequency in Hz (168MHz) */
++#define SYSCLK_FRQ 168000000ULL
++
+ static void netduinoplus2_init(MachineState *machine)
+ {
+     DeviceState *dev;
++    /*
++     * TODO: ideally we would model the SoC RCC and let it handle
++     * system_clock_scale, including its ability to define different
++     * possible SYSCLK sources.
++     */
++    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
++
+     dev = qdev_new(TYPE_STM32F405_SOC);
+     qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
+     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+--
+.20.1

-New patch
+[PULL 2/8] include/hw/irq.h: New function qemu_irq_is_connected()
+Mostly devices don't need to care whether one of their output
+qemu_irq lines is connected, because functions like qemu_set_irq()
+silently do nothing if there is nothing on the other end.  However
+sometimes a device might want to implement default behaviour for the
+case where the machine hasn't wired the line up to anywhere.
+Provide a function qemu_irq_is_connected() that devices can use for
+this purpose.  (The test is trivial but encapsulating it in a
+function makes it easier to see where we're doing it in case we need
+to change the implementation later.)
+Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
+Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
+Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
+Message-id: 20200728103744.6909-2-peter.maydell@linaro.org
+---
+ include/hw/irq.h | 18 ++++++++++++++++++
+file changed, 18 insertions(+)
+diff --git a/include/hw/irq.h b/include/hw/irq.h
+index XXXXXXX..XXXXXXX 100644
+--- a/include/hw/irq.h
++++ b/include/hw/irq.h
+@@ -XXX,XX +XXX,XX @@ qemu_irq qemu_irq_split(qemu_irq irq1, qemu_irq irq2);
+    on an existing vector of qemu_irq.  */
+ void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n);
++/**
++ * qemu_irq_is_connected: Return true if IRQ line is wired up
++ *
++ * If a qemu_irq has a device on the other (receiving) end of it,
++ * return true; otherwise return false.
++ *
++ * Usually device models don't need to care whether the machine model
++ * has wired up their outbound qemu_irq lines, because functions like
++ * qemu_set_irq() silently do nothing if there is nothing on the other
++ * end of the line. However occasionally a device model will want to
++ * provide default behaviour if its output is left floating, and
++ * it can use this function to identify when that is the case.
++ */
++static inline bool qemu_irq_is_connected(qemu_irq irq)
++{
++    return irq != NULL;
++}
++
+ #endif
+--
+.20.1

-New patch
+[PULL 3/8] hw/intc/armv7m_nvic: Provide default "reset the system" behaviour for SYSRESETREQ
+The NVIC provides an outbound qemu_irq "SYSRESETREQ" which it signals
+when the guest sets the SYSRESETREQ bit in the AIRCR register.  This
+matches the hardware design (where the CPU has a signal of this name
+and it is up to the SoC to connect that up to an actual reset
+mechanism), but in QEMU it mostly results in duplicated code in SoC
+objects and bugs where SoC model implementors forget to wire up the
+SYSRESETREQ line.
+Provide a default behaviour for the case where SYSRESETREQ is not
+actually connected to anything: use qemu_system_reset_request() to
+perform a system reset.  This will allow us to remove the
+implementations of SYSRESETREQ handling from the boards where that's
+exactly what it does, and also fixes the bugs in the board models
+which forgot to wire up the signal:
+ * microbit
+ * mps2-an385
+ * mps2-an505
+ * mps2-an511
+ * mps2-an521
+ * musca-a
+ * musca-b1
+ * netduino
+ * netduinoplus2
+We still allow the board to wire up the signal if it needs to, in case
+we need to model more complicated reset controller logic or to model
+buggy SoC hardware which forgot to wire up the line itself. But
+defaulting to "reset the system" is more often going to be correct
+than defaulting to "do nothing".
+Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
+Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
+Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
+Message-id: 20200728103744.6909-3-peter.maydell@linaro.org
+---
+ include/hw/arm/armv7m.h |  4 +++-
+ hw/intc/armv7m_nvic.c   | 17 ++++++++++++++++-
+files changed, 19 insertions(+), 2 deletions(-)
+diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
+index XXXXXXX..XXXXXXX 100644
+--- a/include/hw/arm/armv7m.h
++++ b/include/hw/arm/armv7m.h
+@@ -XXX,XX +XXX,XX @@ typedef struct {
+ /* ARMv7M container object.
+  * + Unnamed GPIO input lines: external IRQ lines for the NVIC
+- * + Named GPIO output SYSRESETREQ: signalled for guest AIRCR.SYSRESETREQ
++ * + Named GPIO output SYSRESETREQ: signalled for guest AIRCR.SYSRESETREQ.
++ *   If this GPIO is not wired up then the NVIC will default to performing
++ *   a qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET).
+  * + Property "cpu-type": CPU type to instantiate
+  * + Property "num-irq": number of external IRQ lines
+  * + Property "memory": MemoryRegion defining the physical address space
+diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
+index XXXXXXX..XXXXXXX 100644
+--- a/hw/intc/armv7m_nvic.c
++++ b/hw/intc/armv7m_nvic.c
+@@ -XXX,XX +XXX,XX @@
+ #include "hw/intc/armv7m_nvic.h"
+ #include "hw/irq.h"
+ #include "hw/qdev-properties.h"
++#include "sysemu/runstate.h"
+ #include "target/arm/cpu.h"
+ #include "exec/exec-all.h"
+ #include "exec/memop.h"
+@@ -XXX,XX +XXX,XX @@ static const uint8_t nvic_id[] = {
+x00, 0xb0, 0x1b, 0x00, 0x0d, 0xe0, 0x05, 0xb1
+ };
++static void signal_sysresetreq(NVICState *s)
++{
++    if (qemu_irq_is_connected(s->sysresetreq)) {
++        qemu_irq_pulse(s->sysresetreq);
++    } else {
++        /*
++         * Default behaviour if the SoC doesn't need to wire up
++         * SYSRESETREQ (eg to a system reset controller of some kind):
++         * perform a system reset via the usual QEMU API.
++         */
++        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
++    }
++}
++
+ static int nvic_pending_prio(NVICState *s)
+ {
+     /* return the group priority of the current pending interrupt,
+@@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value,
+             if (value & R_V7M_AIRCR_SYSRESETREQ_MASK) {
+                 if (attrs.secure ||
+                     !(cpu->env.v7m.aircr & R_V7M_AIRCR_SYSRESETREQS_MASK)) {
+-                    qemu_irq_pulse(s->sysresetreq);
++                    signal_sysresetreq(s);
+                 }
+             }
+             if (value & R_V7M_AIRCR_VECTCLRACTIVE_MASK) {
+--
+.20.1

-[PULL 2/3] Remove unassigned_access CPU hook
+[PULL 4/8] msf2-soc, stellaris: Don't wire up SYSRESETREQ
-All targets have now migrated away from the old unassigned_access
+The MSF2 SoC model and the Stellaris board code both wire
-hook to the new do_transaction_failed hook. This means we can remove
+SYSRESETREQ up to a function that just invokes
-the core-code infrastructure for that hook and the code that calls it.
+    qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
 This is now the default action that the NVIC does if the line is
 not connected, so we can delete the handling code.
 Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
-Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
 Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
-Message-id: 20191108173732.11816-1-peter.maydell@linaro.org
+Message-id: 20200728103744.6909-4-peter.maydell@linaro.org
 ---
- include/hw/core/cpu.h | 24 ------------------------
+ hw/arm/msf2-soc.c  | 11 -----------
- accel/tcg/cputlb.c    |  2 --
+ hw/arm/stellaris.c | 12 ------------
- memory.c              |  7 -------
+files changed, 23 deletions(-)
 files changed, 33 deletions(-)
-diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
+diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
 index XXXXXXX..XXXXXXX 100644
---- a/include/hw/core/cpu.h
+--- a/hw/arm/msf2-soc.c
-+++ b/include/hw/core/cpu.h
++++ b/hw/arm/msf2-soc.c
-@@ -XXX,XX +XXX,XX @@ typedef enum MMUAccessType {
+@@ -XXX,XX +XXX,XX @@
+ #include "hw/irq.h"
- typedef struct CPUWatchpoint CPUWatchpoint;
+ #include "hw/arm/msf2-soc.h"
+ #include "hw/misc/unimp.h"
--typedef void (*CPUUnassignedAccess)(CPUState *cpu, hwaddr addr,
+-#include "sysemu/runstate.h"
--                                    bool is_write, bool is_exec, int opaque,
+ #include "sysemu/sysemu.h"
--                                    unsigned size);
--
+ #define MSF2_TIMER_BASE       0x40004000
- struct TranslationBlock;
+@@ -XXX,XX +XXX,XX @@ static const int spi_irq[MSF2_NUM_SPIS] = { 2, 3 };
+ static const int uart_irq[MSF2_NUM_UARTS] = { 10, 11 };
- /**
+ static const int timer_irq[MSF2_NUM_TIMERS] = { 14, 15 };
-@@ -XXX,XX +XXX,XX @@ struct TranslationBlock;
-  * @reset_dump_flags: #CPUDumpFlags to use for reset logging.
+-static void do_sys_reset(void *opaque, int n, int level)
   * @has_work: Callback for checking if there is work to do.
   * @do_interrupt: Callback for interrupt handling.
 - * @do_unassigned_access: Callback for unassigned access handling.
 - * (this is deprecated: new targets should use do_transaction_failed instead)
   * @do_unaligned_access: Callback for unaligned access handling, if
   * the target defines #TARGET_ALIGNED_ONLY.
   * @do_transaction_failed: Callback for handling failed memory transactions
@@ -XXX,XX +XXX,XX @@ typedef struct CPUClass {
      int reset_dump_flags;
      bool (*has_work)(CPUState *cpu);
      void (*do_interrupt)(CPUState *cpu);
 -    CPUUnassignedAccess do_unassigned_access;
      void (*do_unaligned_access)(CPUState *cpu, vaddr addr,
                                  MMUAccessType access_type,
                                  int mmu_idx, uintptr_t retaddr);
@@ -XXX,XX +XXX,XX @@ struct CPUState {
       * we store some rarely used information in the CPU context.
       */
      uintptr_t mem_io_pc;
 -    /*
 -     * This is only needed for the legacy cpu_unassigned_access() hook;
 -     * when all targets using it have been converted to use
 -     * cpu_transaction_failed() instead it can be removed.
 -     */
 -    MMUAccessType mem_io_access_type;
      int kvm_fd;
      struct KVMState *kvm_state;
@@ -XXX,XX +XXX,XX @@ void cpu_interrupt(CPUState *cpu, int mask);
  #ifdef NEED_CPU_H
  #ifdef CONFIG_SOFTMMU
 -static inline void cpu_unassigned_access(CPUState *cpu, hwaddr addr,
 -                                         bool is_write, bool is_exec,
 -                                         int opaque, unsigned size)
 -{
--    CPUClass *cc = CPU_GET_CLASS(cpu);
+-    if (level) {
--
+-        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
 -    if (cc->do_unassigned_access) {
 -        cc->do_unassigned_access(cpu, addr, is_write, is_exec, opaque, size);
 -    }
 -}
 -
- static inline void cpu_unaligned_access(CPUState *cpu, vaddr addr,
+ static void m2sxxx_soc_initfn(Object *obj)
-                                         MMUAccessType access_type,
+ {
-                                         int mmu_idx, uintptr_t retaddr)
+     MSF2State *s = MSF2_SOC(obj);
-diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
+@@ -XXX,XX +XXX,XX @@ static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp)
          return;
      }
 -    qdev_connect_gpio_out_named(DEVICE(&s->armv7m.nvic), "SYSRESETREQ", 0,
 -                                qemu_allocate_irq(&do_sys_reset, NULL, 0));
 -
      system_clock_scale = NANOSECONDS_PER_SECOND / s->m3clk;
      for (i = 0; i < MSF2_NUM_UARTS; i++) {
 diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
 index XXXXXXX..XXXXXXX 100644
---- a/accel/tcg/cputlb.c
+--- a/hw/arm/stellaris.c
-+++ b/accel/tcg/cputlb.c
++++ b/hw/arm/stellaris.c
-@@ -XXX,XX +XXX,XX @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
+@@ -XXX,XX +XXX,XX @@
-         cpu_io_recompile(cpu, retaddr);
+ #include "hw/boards.h"
-     }
+ #include "qemu/log.h"
+ #include "exec/address-spaces.h"
--    cpu->mem_io_access_type = access_type;
+-#include "sysemu/runstate.h"
  #include "sysemu/sysemu.h"
  #include "hw/arm/armv7m.h"
  #include "hw/char/pl011.h"
@@ -XXX,XX +XXX,XX @@ static void stellaris_adc_init(Object *obj)
      qdev_init_gpio_in(dev, stellaris_adc_trigger, 1);
  }
 -static
 -void do_sys_reset(void *opaque, int n, int level)
 -{
 -    if (level) {
 -        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
 -    }
 -}
 -
-     if (mr->global_locking && !qemu_mutex_iothread_locked()) {
+ /* Board init.  */
-         qemu_mutex_lock_iothread();
+ static stellaris_board_info stellaris_boards[] = {
-         locked = true;
+   { "LM3S811EVB",
-diff --git a/memory.c b/memory.c
+@@ -XXX,XX +XXX,XX @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
-index XXXXXXX..XXXXXXX 100644
+     /* This will exit with an error if the user passed us a bad cpu_type */
---- a/memory.c
+     sysbus_realize_and_unref(SYS_BUS_DEVICE(nvic), &error_fatal);
-+++ b/memory.c
-@@ -XXX,XX +XXX,XX @@ static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
+-    qdev_connect_gpio_out_named(nvic, "SYSRESETREQ", 0,
- #ifdef DEBUG_UNASSIGNED
+-                                qemu_allocate_irq(&do_sys_reset, NULL, 0));
-     printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
+-
- #endif
+     if (board->dc1 & (1 << 16)) {
--    if (current_cpu != NULL) {
+         dev = sysbus_create_varargs(TYPE_STELLARIS_ADC, 0x40038000,
--        bool is_exec = current_cpu->mem_io_access_type == MMU_INST_FETCH;
+                                     qdev_get_gpio_in(nvic, 14),
 -        cpu_unassigned_access(current_cpu, addr, false, is_exec, 0, size);
 -    }
      return 0;
  }
@@ -XXX,XX +XXX,XX @@ static void unassigned_mem_write(void *opaque, hwaddr addr,
  #ifdef DEBUG_UNASSIGNED
      printf("Unassigned mem write " TARGET_FMT_plx " = 0x%"PRIx64"\n", addr, val);
  #endif
 -    if (current_cpu != NULL) {
 -        cpu_unassigned_access(current_cpu, addr, true, false, 0, size);
 -    }
  }
  static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
 --
 .20.1

-New patch
+[PULL 5/8] target/arm: Fix AddPAC error indication
+From: Richard Henderson <richard.henderson@linaro.org>
+The definition of top_bit used in this function is one higher
+than that used in the Arm ARM psuedo-code, which put the error
+indication at top_bit - 1 at the wrong place, which meant that
+it wasn't visible to Auth.
+Fixing the definition of top_bit requires more changes, because
+its most common use is for the count of bits in top_bit:bot_bit,
+which would then need to be computed as top_bit - bot_bit + 1.
+For now, prefer the minimal fix to the error indication alone.
+Fixes: 63ff0ca94cb
+Reported-by: Derrick McKee <derrick.mckee@gmail.com>
+Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
+Message-id: 20200728195706.11087-1-richard.henderson@linaro.org
+Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
+[PMM: added comment about the divergence from the pseudocode]
+Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
+---
+ target/arm/pauth_helper.c         |  6 +++++-
+ tests/tcg/aarch64/pauth-5.c       | 33 +++++++++++++++++++++++++++++++
+ tests/tcg/aarch64/Makefile.target |  2 +-
+files changed, 39 insertions(+), 2 deletions(-)
+ create mode 100644 tests/tcg/aarch64/pauth-5.c
+diff --git a/target/arm/pauth_helper.c b/target/arm/pauth_helper.c
+index XXXXXXX..XXXXXXX 100644
+--- a/target/arm/pauth_helper.c
++++ b/target/arm/pauth_helper.c
+@@ -XXX,XX +XXX,XX @@ static uint64_t pauth_addpac(CPUARMState *env, uint64_t ptr, uint64_t modifier,
+      */
+     test = sextract64(ptr, bot_bit, top_bit - bot_bit);
+     if (test != 0 && test != -1) {
+-        pac ^= MAKE_64BIT_MASK(top_bit - 1, 1);
++        /*
++         * Note that our top_bit is one greater than the pseudocode's
++         * version, hence "- 2" here.
++         */
++        pac ^= MAKE_64BIT_MASK(top_bit - 2, 1);
+     }
+     /*
+diff --git a/tests/tcg/aarch64/pauth-5.c b/tests/tcg/aarch64/pauth-5.c
+new file mode 100644
+index XXXXXXX..XXXXXXX
+--- /dev/null
++++ b/tests/tcg/aarch64/pauth-5.c
+@@ -XXX,XX +XXX,XX @@
++#include <assert.h>
++
++static int x;
++
++int main()
++{
++    int *p0 = &x, *p1, *p2, *p3;
++    unsigned long salt = 0;
++
++    /*
++     * With TBI enabled and a 48-bit VA, there are 7 bits of auth, and so
++     * a 1/128 chance of auth = pac(ptr,key,salt) producing zero.
++     * Find a salt that creates auth != 0.
++     */
++    do {
++        salt++;
++        asm("pacda %0, %1" : "=r"(p1) : "r"(salt), "0"(p0));
++    } while (p0 == p1);
++
++    /*
++     * This pac must fail, because the input pointer bears an encryption,
++     * and so is not properly extended within bits [55:47].  This will
++     * toggle bit 54 in the output...
++     */
++    asm("pacda %0, %1" : "=r"(p2) : "r"(salt), "0"(p1));
++
++    /* ... so that the aut must fail, setting bit 53 in the output ... */
++    asm("autda %0, %1" : "=r"(p3) : "r"(salt), "0"(p2));
++
++    /* ... which means this equality must not hold. */
++    assert(p3 != p0);
++    return 0;
++}
+diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
+index XXXXXXX..XXXXXXX 100644
+--- a/tests/tcg/aarch64/Makefile.target
++++ b/tests/tcg/aarch64/Makefile.target
+@@ -XXX,XX +XXX,XX @@ run-fcvt: fcvt
+ # Pauth Tests
+ ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_ARMV8_3),)
+-AARCH64_TESTS += pauth-1 pauth-2 pauth-4
++AARCH64_TESTS += pauth-1 pauth-2 pauth-4 pauth-5
+ pauth-%: CFLAGS += -march=armv8.3-a
+ run-pauth-%: QEMU_OPTS += -cpu max
+ run-plugin-pauth-%: QEMU_OPTS += -cpu max
+--
+.20.1

-[PULL 3/3] hw/arm/boot: Set NSACR.{CP11, CP10} in dummy SMC setup routine
+[PULL 6/8] target/arm: Avoid maybe-uninitialized warning with gcc 4.9
-From: Clement Deschamps <clement.deschamps@greensocs.com>
+From: Kaige Li <likaige@loongson.cn>
-The boot.c code usually puts the CPU into NS mode directly when it is
+GCC version 4.9.4 isn't clever enough to figure out that all
-booting a kernel.  Since fc1120a7f5f2d4b6 this has included a
+execution paths in disas_ldst() that use 'fn' will have initialized
-requirement to set NSACR to give NS state access to the FPU; we fixed
+it first, and so it warns:
 that for the usual code path in ece628fcf6.  However, it is also
 possible for a board model to request an alternative mode of booting,
 where its 'board_setup' code hook runs in Secure state and is
 responsible for doing the S->NS transition after it has done whatever
 work it must do in Secure state.  In this situation the board_setup
 code now also needs to update NSACR.
-This affects all boards which set info->secure_board_setup, which is
+/home/LiKaige/qemu/target/arm/translate-a64.c: In function ‘disas_ldst’:
-currently the 'raspi' and 'highbank' families.  They both use the
+/home/LiKaige/qemu/target/arm/translate-a64.c:3392:5: error: ‘fn’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
-common arm_write_secure_board_setup_dummy_smc().
+     fn(cpu_reg(s, rt), clean_addr, tcg_rs, get_mem_index(s),
      ^
 /home/LiKaige/qemu/target/arm/translate-a64.c:3318:22: note: ‘fn’ was declared here
      AtomicThreeOpFn *fn;
                       ^
-Set the NSACR CP11 and CP10 bits in the code written by that
+Make it happy by initializing the variable to NULL.
 function, to allow FPU access in Non-Secure state when using dummy
 SMC setup routine.  Otherwise an AArch32 kernel booted on the
 highbank or raspi boards will UNDEF as soon as it tries to use the
 FPU.
-Update the comment describing secure_board_setup to note the new
+Signed-off-by: Kaige Li <likaige@loongson.cn>
-requirements on users of it.
+Message-id: 1596110248-7366-2-git-send-email-likaige@loongson.cn
 This fixes a kernel panic when booting raspbian on raspi2.
 Successfully tested with:
 -01-11-raspbian-jessie-lite.img
 -11-13-raspbian-stretch-lite.img
 -07-10-raspbian-buster-lite.img
 Fixes: fc1120a7f5
 Signed-off-by: Clement Deschamps <clement.deschamps@greensocs.com>
 Tested-by: Laurent Bonnans <laurent.bonnans@here.com>
 Message-id: 20191104151137.81931-1-clement.deschamps@greensocs.com
 Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
-[PMM: updated comment to boot.h to note new requirement on
+[PMM: Clean up commit message and note which gcc version this was]
  users of secure_board_setup; edited/rewrote commit message]
 Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
 ---
- include/hw/arm/boot.h | 7 +++++--
+ target/arm/translate-a64.c | 2 +-
- hw/arm/boot.c         | 3 +++
+file changed, 1 insertion(+), 1 deletion(-)
 files changed, 8 insertions(+), 2 deletions(-)
-diff --git a/include/hw/arm/boot.h b/include/hw/arm/boot.h
+diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
 index XXXXXXX..XXXXXXX 100644
---- a/include/hw/arm/boot.h
+--- a/target/arm/translate-a64.c
-+++ b/include/hw/arm/boot.h
++++ b/target/arm/translate-a64.c
-@@ -XXX,XX +XXX,XX @@ struct arm_boot_info {
+@@ -XXX,XX +XXX,XX @@ static void disas_ldst_atomic(DisasContext *s, uint32_t insn,
-     void (*write_board_setup)(ARMCPU *cpu,
+     bool r = extract32(insn, 22, 1);
-                               const struct arm_boot_info *info);
+     bool a = extract32(insn, 23, 1);
+     TCGv_i64 tcg_rs, clean_addr;
--    /* If set, the board specific loader/setup blob will be run from secure
+-    AtomicThreeOpFn *fn;
-+    /*
++    AtomicThreeOpFn *fn = NULL;
-+     * If set, the board specific loader/setup blob will be run from secure
-      * mode, regardless of secure_boot. The blob becomes responsible for
+     if (is_vector || !dc_isar_feature(aa64_atomics, s)) {
--     * changing to non-secure state if implementing a non-secure boot
+         unallocated_encoding(s);
 +     * changing to non-secure state if implementing a non-secure boot,
 +     * including setting up EL3/Secure registers such as the NSACR as
 +     * required by the Linux booting ABI before the switch to non-secure.
       */
      bool secure_board_setup;
 diff --git a/hw/arm/boot.c b/hw/arm/boot.c
 index XXXXXXX..XXXXXXX 100644
 --- a/hw/arm/boot.c
 +++ b/hw/arm/boot.c
@@ -XXX,XX +XXX,XX @@ void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
      };
      uint32_t board_setup_blob[] = {
          /* board setup addr */
 +        0xee110f51, /* mrc     p15, 0, r0, c1, c1, 2  ;read NSACR */
 +        0xe3800b03, /* orr     r0, #0xc00             ;set CP11, CP10 */
 +        0xee010f51, /* mcr     p15, 0, r0, c1, c1, 2  ;write NSACR */
 xe3a00e00 + (mvbar_addr >> 4), /* mov r0, #mvbar_addr */
 xee0c0f30, /* mcr     p15, 0, r0, c12, c0, 1 ;set MVBAR */
 xee110f11, /* mrc     p15, 0, r0, c1 , c1, 0 ;read SCR */
 --
 .20.1

-[PULL 1/3] ptimer: Remove old ptimer_init_with_bh() API
+[PULL 7/8] hw/arm/nrf51_soc: Set system_clock_scale
-Now all the users of ptimers have converted to the transaction-based
+The nrf51 SoC model wasn't setting the system_clock_scale
-API, we can remove ptimer_init_with_bh() and all the code paths
+global.which meant that if guest code used the systick timer in "use
-that are used only by bottom-half based ptimers, and tidy up the
+the processor clock" mode it would hang because time never advances.
 documentation comments to consider the transaction-based API the
 only possibility.
-The code changes result from:
+Set the global to match the documented CPU clock speed for this SoC.
- * s->bh no longer exists
- * s->callback is now always non-NULL
+This SoC in fact doesn't have a SysTick timer (which is the only thing
 currently that cares about the system_clock_scale), because it's
 a configurable option in the Cortex-M0. However our Cortex-M0 and
 thus our nrf51 and our micro:bit board do provide a SysTick, so
 we ought to provide a functional one rather than a broken one.
 Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
-Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
+Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
-Message-id: 20191025142411.17085-1-peter.maydell@linaro.org
+Message-id: 20200727193458.31250-1-peter.maydell@linaro.org
 ---
- include/hw/ptimer.h | 45 +++++++++++-----------
+ hw/arm/nrf51_soc.c | 5 +++++
- hw/core/ptimer.c    | 91 ++++++++-------------------------------------
+file changed, 5 insertions(+)
 files changed, 36 insertions(+), 100 deletions(-)
-diff --git a/include/hw/ptimer.h b/include/hw/ptimer.h
+diff --git a/hw/arm/nrf51_soc.c b/hw/arm/nrf51_soc.c
 index XXXXXXX..XXXXXXX 100644
---- a/include/hw/ptimer.h
+--- a/hw/arm/nrf51_soc.c
-+++ b/include/hw/ptimer.h
++++ b/hw/arm/nrf51_soc.c
 @@ -XXX,XX +XXX,XX @@
- #include "qemu/timer.h"
+ #define BASE_TO_IRQ(base) ((base >> 12) & 0x1F)
--/* The ptimer API implements a simple periodic countdown timer.
++/* HCLK (the main CPU clock) on this SoC is always 16MHz */
-+/*
++#define HCLK_FRQ 16000000
-+ * The ptimer API implements a simple periodic countdown timer.
++
-  * The countdown timer has a value (which can be read and written via
+ static uint64_t clock_read(void *opaque, hwaddr addr, unsigned int size)
   * ptimer_get_count() and ptimer_set_count()). When it is enabled
   * using ptimer_run(), the value will count downwards at the frequency
   * which has been configured using ptimer_set_period() or ptimer_set_freq().
 - * When it reaches zero it will trigger a QEMU bottom half handler, and
 + * When it reaches zero it will trigger a callback function, and
   * can be set to either reload itself from a specified limit value
   * and keep counting down, or to stop (as a one-shot timer).
   *
 + * A transaction-based API is used for modifying ptimer state: all calls
 + * to functions which modify ptimer state must be between matched calls to
 + * ptimer_transaction_begin() and ptimer_transaction_commit().
 + * When ptimer_transaction_commit() is called it will evaluate the state
 + * of the timer after all the changes in the transaction, and call the
 + * callback if necessary. (See the ptimer_init() documentation for the full
 + * list of state-modifying functions and detailed semantics of the callback.)
 + *
   * Forgetting to set the period/frequency (or setting it to zero) is a
   * bug in the QEMU device and will cause warning messages to be printed
   * to stderr when the guest attempts to enable the timer.
@@ -XXX,XX +XXX,XX @@
   * ptimer_set_count() or ptimer_set_limit() will not trigger the timer
   * (though it will cause a reload). Only a counter decrement to "0"
   * will cause a trigger. Not compatible with NO_IMMEDIATE_TRIGGER;
 - * ptimer_init_with_bh() will assert() that you don't set both.
 + * ptimer_init() will assert() that you don't set both.
   */
  #define PTIMER_POLICY_TRIGGER_ONLY_ON_DECREMENT (1 << 5)
@@ -XXX,XX +XXX,XX @@
  typedef struct ptimer_state ptimer_state;
  typedef void (*ptimer_cb)(void *opaque);
 -/**
 - * ptimer_init_with_bh - Allocate and return a new ptimer
 - * @bh: QEMU bottom half which is run on timer expiry
 - * @policy: PTIMER_POLICY_* bits specifying behaviour
 - *
 - * The ptimer returned must be freed using ptimer_free().
 - * The ptimer takes ownership of @bh and will delete it
 - * when the ptimer is eventually freed.
 - */
 -ptimer_state *ptimer_init_with_bh(QEMUBH *bh, uint8_t policy_mask);
 -
  /**
   * ptimer_init - Allocate and return a new ptimer
   * @callback: function to call on ptimer expiry
@@ -XXX,XX +XXX,XX @@ ptimer_state *ptimer_init(ptimer_cb callback,
   * ptimer_free - Free a ptimer
   * @s: timer to free
   *
 - * Free a ptimer created using ptimer_init_with_bh() (including
 - * deleting the bottom half which it is using).
 + * Free a ptimer created using ptimer_init().
   */
  void ptimer_free(ptimer_state *s);
@@ -XXX,XX +XXX,XX @@ void ptimer_transaction_commit(ptimer_state *s);
   * may be more appropriate.
   *
   * This function will assert if it is called outside a
 - * ptimer_transaction_begin/commit block, unless this is a bottom-half ptimer.
 + * ptimer_transaction_begin/commit block.
   */
  void ptimer_set_period(ptimer_state *s, int64_t period);
@@ -XXX,XX +XXX,XX @@ void ptimer_set_period(ptimer_state *s, int64_t period);
   * precise to fractions of a nanosecond, avoiding rounding errors.
   *
   * This function will assert if it is called outside a
 - * ptimer_transaction_begin/commit block, unless this is a bottom-half ptimer.
 + * ptimer_transaction_begin/commit block.
   */
  void ptimer_set_freq(ptimer_state *s, uint32_t freq);
@@ -XXX,XX +XXX,XX @@ uint64_t ptimer_get_limit(ptimer_state *s);
   * reload the counter when their reload register is written to.
   *
   * This function will assert if it is called outside a
 - * ptimer_transaction_begin/commit block, unless this is a bottom-half ptimer.
 + * ptimer_transaction_begin/commit block.
   */
  void ptimer_set_limit(ptimer_state *s, uint64_t limit, int reload);
@@ -XXX,XX +XXX,XX @@ uint64_t ptimer_get_count(ptimer_state *s);
   * point in the future.
   *
   * This function will assert if it is called outside a
 - * ptimer_transaction_begin/commit block, unless this is a bottom-half ptimer.
 + * ptimer_transaction_begin/commit block.
   */
  void ptimer_set_count(ptimer_state *s, uint64_t count);
@@ -XXX,XX +XXX,XX @@ void ptimer_set_count(ptimer_state *s, uint64_t count);
   * @s: ptimer
   * @oneshot: non-zero if this timer should only count down once
   *
 - * Start a ptimer counting down; when it reaches zero the bottom half
 - * passed to ptimer_init_with_bh() will be invoked.
 + * Start a ptimer counting down; when it reaches zero the callback function
 + * passed to ptimer_init() will be invoked.
   * If the @oneshot argument is zero,
   * the counter value will then be reloaded from the limit and it will
   * start counting down again. If @oneshot is non-zero, then the counter
   * will disable itself when it reaches zero.
   *
   * This function will assert if it is called outside a
 - * ptimer_transaction_begin/commit block, unless this is a bottom-half ptimer.
 + * ptimer_transaction_begin/commit block.
   */
  void ptimer_run(ptimer_state *s, int oneshot);
@@ -XXX,XX +XXX,XX @@ void ptimer_run(ptimer_state *s, int oneshot);
   * restarted.
   *
   * This function will assert if it is called outside a
 - * ptimer_transaction_begin/commit block, unless this is a bottom-half ptimer.
 + * ptimer_transaction_begin/commit block.
   */
  void ptimer_stop(ptimer_state *s);
 diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
 index XXXXXXX..XXXXXXX 100644
 --- a/hw/core/ptimer.c
 +++ b/hw/core/ptimer.c
@@ -XXX,XX +XXX,XX @@ struct ptimer_state
      int64_t last_event;
      int64_t next_event;
      uint8_t policy_mask;
 -    QEMUBH *bh;
      QEMUTimer *timer;
      ptimer_cb callback;
      void *callback_opaque;
@@ -XXX,XX +XXX,XX @@ struct ptimer_state
  /* Use a bottom-half routine to avoid reentrancy issues.  */
  static void ptimer_trigger(ptimer_state *s)
  {
--    if (s->bh) {
+     qemu_log_mask(LOG_UNIMP, "%s: 0x%" HWADDR_PRIx " [%u]\n",
--        replay_bh_schedule_event(s->bh);
+@@ -XXX,XX +XXX,XX @@ static void nrf51_soc_realize(DeviceState *dev_soc, Error **errp)
--    }
+         return;
 -    if (s->callback) {
 -        s->callback(s->callback_opaque);
 -    }
 +    s->callback(s->callback_opaque);
  }
  static void ptimer_reload(ptimer_state *s, int delta_adjust)
@@ -XXX,XX +XXX,XX @@ uint64_t ptimer_get_count(ptimer_state *s)
  void ptimer_set_count(ptimer_state *s, uint64_t count)
  {
 -    assert(s->in_transaction || !s->callback);
 +    assert(s->in_transaction);
      s->delta = count;
      if (s->enabled) {
 -        if (!s->callback) {
 -            s->next_event = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 -            ptimer_reload(s, 0);
 -        } else {
 -            s->need_reload = true;
 -        }
 +        s->need_reload = true;
      }
- }
++    system_clock_scale = NANOSECONDS_PER_SECOND / HCLK_FRQ;
-@@ -XXX,XX +XXX,XX @@ void ptimer_run(ptimer_state *s, int oneshot)
++
- {
+     object_property_set_link(OBJECT(&s->cpu), "memory", OBJECT(&s->container),
-     bool was_disabled = !s->enabled;
+                              &error_abort);
+     if (!sysbus_realize(SYS_BUS_DEVICE(&s->cpu), errp)) {
 -    assert(s->in_transaction || !s->callback);
 +    assert(s->in_transaction);
      if (was_disabled && s->period == 0) {
          if (!qtest_enabled()) {
@@ -XXX,XX +XXX,XX @@ void ptimer_run(ptimer_state *s, int oneshot)
      }
      s->enabled = oneshot ? 2 : 1;
      if (was_disabled) {
 -        if (!s->callback) {
 -            s->next_event = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 -            ptimer_reload(s, 0);
 -        } else {
 -            s->need_reload = true;
 -        }
 +        s->need_reload = true;
      }
  }
@@ -XXX,XX +XXX,XX @@ void ptimer_run(ptimer_state *s, int oneshot)
     is immediately restarted.  */
  void ptimer_stop(ptimer_state *s)
  {
 -    assert(s->in_transaction || !s->callback);
 +    assert(s->in_transaction);
      if (!s->enabled)
          return;
@@ -XXX,XX +XXX,XX @@ void ptimer_stop(ptimer_state *s)
      s->delta = ptimer_get_count(s);
      timer_del(s->timer);
      s->enabled = 0;
 -    if (s->callback) {
 -        s->need_reload = false;
 -    }
 +    s->need_reload = false;
  }
  /* Set counter increment interval in nanoseconds.  */
  void ptimer_set_period(ptimer_state *s, int64_t period)
  {
 -    assert(s->in_transaction || !s->callback);
 +    assert(s->in_transaction);
      s->delta = ptimer_get_count(s);
      s->period = period;
      s->period_frac = 0;
      if (s->enabled) {
 -        if (!s->callback) {
 -            s->next_event = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 -            ptimer_reload(s, 0);
 -        } else {
 -            s->need_reload = true;
 -        }
 +        s->need_reload = true;
      }
  }
  /* Set counter frequency in Hz.  */
  void ptimer_set_freq(ptimer_state *s, uint32_t freq)
  {
 -    assert(s->in_transaction || !s->callback);
 +    assert(s->in_transaction);
      s->delta = ptimer_get_count(s);
      s->period = 1000000000ll / freq;
      s->period_frac = (1000000000ll << 32) / freq;
      if (s->enabled) {
 -        if (!s->callback) {
 -            s->next_event = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 -            ptimer_reload(s, 0);
 -        } else {
 -            s->need_reload = true;
 -        }
 +        s->need_reload = true;
      }
  }
@@ -XXX,XX +XXX,XX @@ void ptimer_set_freq(ptimer_state *s, uint32_t freq)
     count = limit.  */
  void ptimer_set_limit(ptimer_state *s, uint64_t limit, int reload)
  {
 -    assert(s->in_transaction || !s->callback);
 +    assert(s->in_transaction);
      s->limit = limit;
      if (reload)
          s->delta = limit;
      if (s->enabled && reload) {
 -        if (!s->callback) {
 -            s->next_event = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 -            ptimer_reload(s, 0);
 -        } else {
 -            s->need_reload = true;
 -        }
 +        s->need_reload = true;
      }
  }
@@ -XXX,XX +XXX,XX @@ uint64_t ptimer_get_limit(ptimer_state *s)
  void ptimer_transaction_begin(ptimer_state *s)
  {
 -    assert(!s->in_transaction || !s->callback);
 +    assert(!s->in_transaction);
      s->in_transaction = true;
      s->need_reload = false;
  }
@@ -XXX,XX +XXX,XX @@ const VMStateDescription vmstate_ptimer = {
      }
  };
 -ptimer_state *ptimer_init_with_bh(QEMUBH *bh, uint8_t policy_mask)
 -{
 -    ptimer_state *s;
 -
 -    s = (ptimer_state *)g_malloc0(sizeof(ptimer_state));
 -    s->bh = bh;
 -    s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ptimer_tick, s);
 -    s->policy_mask = policy_mask;
 -
 -    /*
 -     * These two policies are incompatible -- trigger-on-decrement implies
 -     * a timer trigger when the count becomes 0, but no-immediate-trigger
 -     * implies a trigger when the count stops being 0.
 -     */
 -    assert(!((policy_mask & PTIMER_POLICY_TRIGGER_ONLY_ON_DECREMENT) &&
 -             (policy_mask & PTIMER_POLICY_NO_IMMEDIATE_TRIGGER)));
 -    return s;
 -}
 -
  ptimer_state *ptimer_init(ptimer_cb callback, void *callback_opaque,
                            uint8_t policy_mask)
  {
      ptimer_state *s;
 -    /*
 -     * The callback function is mandatory; so we use it to distinguish
 -     * old-style QEMUBH ptimers from new transaction API ptimers.
 -     * (ptimer_init_with_bh() allows a NULL bh pointer and at least
 -     * one device (digic-timer) passes NULL, so it's not the case
 -     * that either s->bh != NULL or s->callback != NULL.)
 -     */
 +    /* The callback function is mandatory. */
      assert(callback);
      s = g_new0(ptimer_state, 1);
@@ -XXX,XX +XXX,XX @@ ptimer_state *ptimer_init(ptimer_cb callback, void *callback_opaque,
  void ptimer_free(ptimer_state *s)
  {
 -    if (s->bh) {
 -        qemu_bh_delete(s->bh);
 -    }
      timer_free(s->timer);
      g_free(s);
  }
 --
 .20.1

-New patch
+[PULL 8/8] hw/timer/imx_epit: Avoid assertion when CR.SWR is written
+The imx_epit device has a software-controllable reset triggered by
+setting the SWR bit in the CR register. An error in commit cc2722ec83ad9
+means that we will end up assert()ing if the guest does this, because
+the code in imx_epit_write() starts ptimer transactions, and then
+imx_epit_reset() also starts ptimer transactions, triggering
+"ptimer_transaction_begin: Assertion `!s->in_transaction' failed".
+The cleanest way to avoid this double-transaction is to move the
+start-transaction for the CR write handling down below the check of
+the SWR bit.
+Fixes: https://bugs.launchpad.net/qemu/+bug/1880424
+Fixes: cc2722ec83ad944505fe
+Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
+Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
+Message-id: 20200727154550.3409-1-peter.maydell@linaro.org
+---
+ hw/timer/imx_epit.c | 13 ++++++++++---
+file changed, 10 insertions(+), 3 deletions(-)
+diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
+index XXXXXXX..XXXXXXX 100644
+--- a/hw/timer/imx_epit.c
++++ b/hw/timer/imx_epit.c
+@@ -XXX,XX +XXX,XX @@ static void imx_epit_write(void *opaque, hwaddr offset, uint64_t value,
+     switch (offset >> 2) {
+     case 0: /* CR */
+-        ptimer_transaction_begin(s->timer_cmp);
+-        ptimer_transaction_begin(s->timer_reload);
+         oldcr = s->cr;
+         s->cr = value & 0x03ffffff;
+         if (s->cr & CR_SWR) {
+             /* handle the reset */
+             imx_epit_reset(DEVICE(s));
+-        } else {
++            /*
++             * TODO: could we 'break' here? following operations appear
++             * to duplicate the work imx_epit_reset() already did.
++             */
++        }
++
++        ptimer_transaction_begin(s->timer_cmp);
++        ptimer_transaction_begin(s->timer_reload);
++
++        if (!(s->cr & CR_SWR)) {
+             imx_epit_set_freq(s);
+         }
+--
+.20.1

Arm patches for rc1:
 * two final "remove the old API" patches for some API transitions
 * bugfix for raspi/highbank Linux boot

thanks
-- PMM

The following changes since commit 654efcb511d394c1d3f5292c28503d1d19e5b1d3:

Merge remote-tracking branch 'remotes/vivier/tags/q800-branch-pull-request' into staging (2019-11-11 09:23:46 +0000)

are available in the Git repository at:

https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20191111

for you to fetch changes up to 45c078f163fd47c35e7505d98928fae63baada7d:

hw/arm/boot: Set NSACR.{CP11, CP10} in dummy SMC setup routine (2019-11-11 13:44:16 +0000)

----------------------------------------------------------------
target-arm queue:
 * Remove old unassigned_access CPU hook API
 * Remove old ptimer_init_with_bh() API
 * hw/arm/boot: Set NSACR.{CP11, CP10} in dummy SMC setup routine

----------------------------------------------------------------
Clement Deschamps (1):
      hw/arm/boot: Set NSACR.{CP11, CP10} in dummy SMC setup routine

Peter Maydell (2):
      ptimer: Remove old ptimer_init_with_bh() API
      Remove unassigned_access CPU hook

Now all the users of ptimers have converted to the transaction-based
API, we can remove ptimer_init_with_bh() and all the code paths
that are used only by bottom-half based ptimers, and tidy up the
documentation comments to consider the transaction-based API the
only possibility.

The code changes result from:
 * s->bh no longer exists
 * s->callback is now always non-NULL

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20191025142411.17085-1-peter.maydell@linaro.org
---
 include/hw/ptimer.h | 45 +++++++++++-----------
 hw/core/ptimer.c    | 91 ++++++++-------------------------------------
 2 files changed, 36 insertions(+), 100 deletions(-)

diff --git a/include/hw/ptimer.h b/include/hw/ptimer.h
index XXXXXXX..XXXXXXX 100644
--- a/include/hw/ptimer.h
+++ b/include/hw/ptimer.h
@@ -XXX,XX +XXX,XX @@
 
 #include "qemu/timer.h"
 
-/* The ptimer API implements a simple periodic countdown timer.
+/*
+ * The ptimer API implements a simple periodic countdown timer.
  * The countdown timer has a value (which can be read and written via
  * ptimer_get_count() and ptimer_set_count()). When it is enabled
  * using ptimer_run(), the value will count downwards at the frequency
  * which has been configured using ptimer_set_period() or ptimer_set_freq().
- * When it reaches zero it will trigger a QEMU bottom half handler, and
+ * When it reaches zero it will trigger a callback function, and
  * can be set to either reload itself from a specified limit value
  * and keep counting down, or to stop (as a one-shot timer).
  *
+ * A transaction-based API is used for modifying ptimer state: all calls
+ * to functions which modify ptimer state must be between matched calls to
+ * ptimer_transaction_begin() and ptimer_transaction_commit().
+ * When ptimer_transaction_commit() is called it will evaluate the state
+ * of the timer after all the changes in the transaction, and call the
+ * callback if necessary. (See the ptimer_init() documentation for the full
+ * list of state-modifying functions and detailed semantics of the callback.)
+ *
  * Forgetting to set the period/frequency (or setting it to zero) is a
  * bug in the QEMU device and will cause warning messages to be printed
  * to stderr when the guest attempts to enable the timer.
@@ -XXX,XX +XXX,XX @@
  * ptimer_set_count() or ptimer_set_limit() will not trigger the timer
  * (though it will cause a reload). Only a counter decrement to "0"
  * will cause a trigger. Not compatible with NO_IMMEDIATE_TRIGGER;
- * ptimer_init_with_bh() will assert() that you don't set both.
+ * ptimer_init() will assert() that you don't set both.
  */
 #define PTIMER_POLICY_TRIGGER_ONLY_ON_DECREMENT (1 << 5)
 
@@ -XXX,XX +XXX,XX @@
 typedef struct ptimer_state ptimer_state;
 typedef void (*ptimer_cb)(void *opaque);
 
-/**
- * ptimer_init_with_bh - Allocate and return a new ptimer
- * @bh: QEMU bottom half which is run on timer expiry
- * @policy: PTIMER_POLICY_* bits specifying behaviour
- *
- * The ptimer returned must be freed using ptimer_free().
- * The ptimer takes ownership of @bh and will delete it
- * when the ptimer is eventually freed.
- */
-ptimer_state *ptimer_init_with_bh(QEMUBH *bh, uint8_t policy_mask);
-
 /**
  * ptimer_init - Allocate and return a new ptimer
  * @callback: function to call on ptimer expiry
@@ -XXX,XX +XXX,XX @@ ptimer_state *ptimer_init(ptimer_cb callback,
  * ptimer_free - Free a ptimer
  * @s: timer to free
  *
- * Free a ptimer created using ptimer_init_with_bh() (including
- * deleting the bottom half which it is using).
+ * Free a ptimer created using ptimer_init().
  */
 void ptimer_free(ptimer_state *s);
 
@@ -XXX,XX +XXX,XX @@ void ptimer_transaction_commit(ptimer_state *s);
  * may be more appropriate.
  *
  * This function will assert if it is called outside a
- * ptimer_transaction_begin/commit block, unless this is a bottom-half ptimer.
+ * ptimer_transaction_begin/commit block.
  */
 void ptimer_set_period(ptimer_state *s, int64_t period);
 
@@ -XXX,XX +XXX,XX @@ void ptimer_set_period(ptimer_state *s, int64_t period);
  * precise to fractions of a nanosecond, avoiding rounding errors.
  *
  * This function will assert if it is called outside a
- * ptimer_transaction_begin/commit block, unless this is a bottom-half ptimer.
+ * ptimer_transaction_begin/commit block.
  */
 void ptimer_set_freq(ptimer_state *s, uint32_t freq);
 
@@ -XXX,XX +XXX,XX @@ uint64_t ptimer_get_limit(ptimer_state *s);
  * reload the counter when their reload register is written to.
  *
  * This function will assert if it is called outside a
- * ptimer_transaction_begin/commit block, unless this is a bottom-half ptimer.
+ * ptimer_transaction_begin/commit block.
  */
 void ptimer_set_limit(ptimer_state *s, uint64_t limit, int reload);
 
@@ -XXX,XX +XXX,XX @@ uint64_t ptimer_get_count(ptimer_state *s);
  * point in the future.
  *
  * This function will assert if it is called outside a
- * ptimer_transaction_begin/commit block, unless this is a bottom-half ptimer.
+ * ptimer_transaction_begin/commit block.
  */
 void ptimer_set_count(ptimer_state *s, uint64_t count);
 
@@ -XXX,XX +XXX,XX @@ void ptimer_set_count(ptimer_state *s, uint64_t count);
  * @s: ptimer
  * @oneshot: non-zero if this timer should only count down once
  *
- * Start a ptimer counting down; when it reaches zero the bottom half
- * passed to ptimer_init_with_bh() will be invoked.
+ * Start a ptimer counting down; when it reaches zero the callback function
+ * passed to ptimer_init() will be invoked.
  * If the @oneshot argument is zero,
  * the counter value will then be reloaded from the limit and it will
  * start counting down again. If @oneshot is non-zero, then the counter
  * will disable itself when it reaches zero.
  *
  * This function will assert if it is called outside a
- * ptimer_transaction_begin/commit block, unless this is a bottom-half ptimer.
+ * ptimer_transaction_begin/commit block.
  */
 void ptimer_run(ptimer_state *s, int oneshot);
 
@@ -XXX,XX +XXX,XX @@ void ptimer_run(ptimer_state *s, int oneshot);
  * restarted.
  *
  * This function will assert if it is called outside a
- * ptimer_transaction_begin/commit block, unless this is a bottom-half ptimer.
+ * ptimer_transaction_begin/commit block.
  */
 void ptimer_stop(ptimer_state *s);
 
diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -XXX,XX +XXX,XX @@ struct ptimer_state
     int64_t last_event;
     int64_t next_event;
     uint8_t policy_mask;
-    QEMUBH *bh;
     QEMUTimer *timer;
     ptimer_cb callback;
     void *callback_opaque;
@@ -XXX,XX +XXX,XX @@ struct ptimer_state
 /* Use a bottom-half routine to avoid reentrancy issues.  */
 static void ptimer_trigger(ptimer_state *s)
 {
-    if (s->bh) {
-        replay_bh_schedule_event(s->bh);
-    }
-    if (s->callback) {
-        s->callback(s->callback_opaque);
-    }
+    s->callback(s->callback_opaque);
 }
 
 static void ptimer_reload(ptimer_state *s, int delta_adjust)
@@ -XXX,XX +XXX,XX @@ uint64_t ptimer_get_count(ptimer_state *s)
 
 void ptimer_set_count(ptimer_state *s, uint64_t count)
 {
-    assert(s->in_transaction || !s->callback);
+    assert(s->in_transaction);
     s->delta = count;
     if (s->enabled) {
-        if (!s->callback) {
-            s->next_event = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-            ptimer_reload(s, 0);
-        } else {
-            s->need_reload = true;
-        }
+        s->need_reload = true;
     }
 }
 
@@ -XXX,XX +XXX,XX @@ void ptimer_run(ptimer_state *s, int oneshot)
 {
     bool was_disabled = !s->enabled;
 
-    assert(s->in_transaction || !s->callback);
+    assert(s->in_transaction);
 
     if (was_disabled && s->period == 0) {
         if (!qtest_enabled()) {
@@ -XXX,XX +XXX,XX @@ void ptimer_run(ptimer_state *s, int oneshot)
     }
     s->enabled = oneshot ? 2 : 1;
     if (was_disabled) {
-        if (!s->callback) {
-            s->next_event = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-            ptimer_reload(s, 0);
-        } else {
-            s->need_reload = true;
-        }
+        s->need_reload = true;
     }
 }
 
@@ -XXX,XX +XXX,XX @@ void ptimer_run(ptimer_state *s, int oneshot)
    is immediately restarted.  */
 void ptimer_stop(ptimer_state *s)
 {
-    assert(s->in_transaction || !s->callback);
+    assert(s->in_transaction);
 
     if (!s->enabled)
         return;
@@ -XXX,XX +XXX,XX @@ void ptimer_stop(ptimer_state *s)
     s->delta = ptimer_get_count(s);
     timer_del(s->timer);
     s->enabled = 0;
-    if (s->callback) {
-        s->need_reload = false;
-    }
+    s->need_reload = false;
 }
 
 /* Set counter increment interval in nanoseconds.  */
 void ptimer_set_period(ptimer_state *s, int64_t period)
 {
-    assert(s->in_transaction || !s->callback);
+    assert(s->in_transaction);
     s->delta = ptimer_get_count(s);
     s->period = period;
     s->period_frac = 0;
     if (s->enabled) {
-        if (!s->callback) {
-            s->next_event = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-            ptimer_reload(s, 0);
-        } else {
-            s->need_reload = true;
-        }
+        s->need_reload = true;
     }
 }
 
 /* Set counter frequency in Hz.  */
 void ptimer_set_freq(ptimer_state *s, uint32_t freq)
 {
-    assert(s->in_transaction || !s->callback);
+    assert(s->in_transaction);
     s->delta = ptimer_get_count(s);
     s->period = 1000000000ll / freq;
     s->period_frac = (1000000000ll << 32) / freq;
     if (s->enabled) {
-        if (!s->callback) {
-            s->next_event = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-            ptimer_reload(s, 0);
-        } else {
-            s->need_reload = true;
-        }
+        s->need_reload = true;
     }
 }
 
@@ -XXX,XX +XXX,XX @@ void ptimer_set_freq(ptimer_state *s, uint32_t freq)
    count = limit.  */
 void ptimer_set_limit(ptimer_state *s, uint64_t limit, int reload)
 {
-    assert(s->in_transaction || !s->callback);
+    assert(s->in_transaction);
     s->limit = limit;
     if (reload)
         s->delta = limit;
     if (s->enabled && reload) {
-        if (!s->callback) {
-            s->next_event = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-            ptimer_reload(s, 0);
-        } else {
-            s->need_reload = true;
-        }
+        s->need_reload = true;
     }
 }
 
@@ -XXX,XX +XXX,XX @@ uint64_t ptimer_get_limit(ptimer_state *s)
 
 void ptimer_transaction_begin(ptimer_state *s)
 {
-    assert(!s->in_transaction || !s->callback);
+    assert(!s->in_transaction);
     s->in_transaction = true;
     s->need_reload = false;
 }
@@ -XXX,XX +XXX,XX @@ const VMStateDescription vmstate_ptimer = {
     }
 };
 
-ptimer_state *ptimer_init_with_bh(QEMUBH *bh, uint8_t policy_mask)
-{
-    ptimer_state *s;
-
-    s = (ptimer_state *)g_malloc0(sizeof(ptimer_state));
-    s->bh = bh;
-    s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ptimer_tick, s);
-    s->policy_mask = policy_mask;
-
-    /*
-     * These two policies are incompatible -- trigger-on-decrement implies
-     * a timer trigger when the count becomes 0, but no-immediate-trigger
-     * implies a trigger when the count stops being 0.
-     */
-    assert(!((policy_mask & PTIMER_POLICY_TRIGGER_ONLY_ON_DECREMENT) &&
-             (policy_mask & PTIMER_POLICY_NO_IMMEDIATE_TRIGGER)));
-    return s;
-}
-
 ptimer_state *ptimer_init(ptimer_cb callback, void *callback_opaque,
                           uint8_t policy_mask)
 {
     ptimer_state *s;
 
-    /*
-     * The callback function is mandatory; so we use it to distinguish
-     * old-style QEMUBH ptimers from new transaction API ptimers.
-     * (ptimer_init_with_bh() allows a NULL bh pointer and at least
-     * one device (digic-timer) passes NULL, so it's not the case
-     * that either s->bh != NULL or s->callback != NULL.)
-     */
+    /* The callback function is mandatory. */
     assert(callback);
 
     s = g_new0(ptimer_state, 1);
@@ -XXX,XX +XXX,XX @@ ptimer_state *ptimer_init(ptimer_cb callback, void *callback_opaque,
 
 void ptimer_free(ptimer_state *s)
 {
-    if (s->bh) {
-        qemu_bh_delete(s->bh);
-    }
     timer_free(s->timer);
     g_free(s);
 }
-- 
2.20.1

All targets have now migrated away from the old unassigned_access
hook to the new do_transaction_failed hook. This means we can remove
the core-code infrastructure for that hook and the code that calls it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20191108173732.11816-1-peter.maydell@linaro.org
---
 include/hw/core/cpu.h | 24 ------------------------
 accel/tcg/cputlb.c    |  2 --
 memory.c              |  7 -------
 3 files changed, 33 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index XXXXXXX..XXXXXXX 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -XXX,XX +XXX,XX @@ typedef enum MMUAccessType {
 
 typedef struct CPUWatchpoint CPUWatchpoint;
 
-typedef void (*CPUUnassignedAccess)(CPUState *cpu, hwaddr addr,
-                                    bool is_write, bool is_exec, int opaque,
-                                    unsigned size);
-
 struct TranslationBlock;
 
 /**
@@ -XXX,XX +XXX,XX @@ struct TranslationBlock;
  * @reset_dump_flags: #CPUDumpFlags to use for reset logging.
  * @has_work: Callback for checking if there is work to do.
  * @do_interrupt: Callback for interrupt handling.
- * @do_unassigned_access: Callback for unassigned access handling.
- * (this is deprecated: new targets should use do_transaction_failed instead)
  * @do_unaligned_access: Callback for unaligned access handling, if
  * the target defines #TARGET_ALIGNED_ONLY.
  * @do_transaction_failed: Callback for handling failed memory transactions
@@ -XXX,XX +XXX,XX @@ typedef struct CPUClass {
     int reset_dump_flags;
     bool (*has_work)(CPUState *cpu);
     void (*do_interrupt)(CPUState *cpu);
-    CPUUnassignedAccess do_unassigned_access;
     void (*do_unaligned_access)(CPUState *cpu, vaddr addr,
                                 MMUAccessType access_type,
                                 int mmu_idx, uintptr_t retaddr);
@@ -XXX,XX +XXX,XX @@ struct CPUState {
      * we store some rarely used information in the CPU context.
      */
     uintptr_t mem_io_pc;
-    /*
-     * This is only needed for the legacy cpu_unassigned_access() hook;
-     * when all targets using it have been converted to use
-     * cpu_transaction_failed() instead it can be removed.
-     */
-    MMUAccessType mem_io_access_type;
 
     int kvm_fd;
     struct KVMState *kvm_state;
@@ -XXX,XX +XXX,XX @@ void cpu_interrupt(CPUState *cpu, int mask);
 #ifdef NEED_CPU_H
 
 #ifdef CONFIG_SOFTMMU
-static inline void cpu_unassigned_access(CPUState *cpu, hwaddr addr,
-                                         bool is_write, bool is_exec,
-                                         int opaque, unsigned size)
-{
-    CPUClass *cc = CPU_GET_CLASS(cpu);
-
-    if (cc->do_unassigned_access) {
-        cc->do_unassigned_access(cpu, addr, is_write, is_exec, opaque, size);
-    }
-}
-
 static inline void cpu_unaligned_access(CPUState *cpu, vaddr addr,
                                         MMUAccessType access_type,
                                         int mmu_idx, uintptr_t retaddr)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index XXXXXXX..XXXXXXX 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -XXX,XX +XXX,XX @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
         cpu_io_recompile(cpu, retaddr);
     }
 
-    cpu->mem_io_access_type = access_type;
-
     if (mr->global_locking && !qemu_mutex_iothread_locked()) {
         qemu_mutex_lock_iothread();
         locked = true;
diff --git a/memory.c b/memory.c
index XXXXXXX..XXXXXXX 100644
--- a/memory.c
+++ b/memory.c
@@ -XXX,XX +XXX,XX @@ static uint64_t unassigned_mem_read(void *opaque, hwaddr addr,
 #ifdef DEBUG_UNASSIGNED
     printf("Unassigned mem read " TARGET_FMT_plx "\n", addr);
 #endif
-    if (current_cpu != NULL) {
-        bool is_exec = current_cpu->mem_io_access_type == MMU_INST_FETCH;
-        cpu_unassigned_access(current_cpu, addr, false, is_exec, 0, size);
-    }
     return 0;
 }
 
@@ -XXX,XX +XXX,XX @@ static void unassigned_mem_write(void *opaque, hwaddr addr,
 #ifdef DEBUG_UNASSIGNED
     printf("Unassigned mem write " TARGET_FMT_plx " = 0x%"PRIx64"\n", addr, val);
 #endif
-    if (current_cpu != NULL) {
-        cpu_unassigned_access(current_cpu, addr, true, false, 0, size);
-    }
 }
 
 static bool unassigned_mem_accepts(void *opaque, hwaddr addr,
-- 
2.20.1

From: Clement Deschamps <clement.deschamps@greensocs.com>

The boot.c code usually puts the CPU into NS mode directly when it is
booting a kernel.  Since fc1120a7f5f2d4b6 this has included a
requirement to set NSACR to give NS state access to the FPU; we fixed
that for the usual code path in ece628fcf6.  However, it is also
possible for a board model to request an alternative mode of booting,
where its 'board_setup' code hook runs in Secure state and is
responsible for doing the S->NS transition after it has done whatever
work it must do in Secure state.  In this situation the board_setup
code now also needs to update NSACR.

This affects all boards which set info->secure_board_setup, which is
currently the 'raspi' and 'highbank' families.  They both use the
common arm_write_secure_board_setup_dummy_smc().

Set the NSACR CP11 and CP10 bits in the code written by that
function, to allow FPU access in Non-Secure state when using dummy
SMC setup routine.  Otherwise an AArch32 kernel booted on the
highbank or raspi boards will UNDEF as soon as it tries to use the
FPU.

Update the comment describing secure_board_setup to note the new
requirements on users of it.

This fixes a kernel panic when booting raspbian on raspi2.

Successfully tested with:
  2017-01-11-raspbian-jessie-lite.img
  2018-11-13-raspbian-stretch-lite.img
  2019-07-10-raspbian-buster-lite.img

Fixes: fc1120a7f5
Signed-off-by: Clement Deschamps <clement.deschamps@greensocs.com>
Tested-by: Laurent Bonnans <laurent.bonnans@here.com>
Message-id: 20191104151137.81931-1-clement.deschamps@greensocs.com
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
[PMM: updated comment to boot.h to note new requirement on
 users of secure_board_setup; edited/rewrote commit message]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/arm/boot.h | 7 +++++--
 hw/arm/boot.c         | 3 +++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/hw/arm/boot.h b/include/hw/arm/boot.h
index XXXXXXX..XXXXXXX 100644
--- a/include/hw/arm/boot.h
+++ b/include/hw/arm/boot.h
@@ -XXX,XX +XXX,XX @@ struct arm_boot_info {
     void (*write_board_setup)(ARMCPU *cpu,
                               const struct arm_boot_info *info);
 
-    /* If set, the board specific loader/setup blob will be run from secure
+    /*
+     * If set, the board specific loader/setup blob will be run from secure
      * mode, regardless of secure_boot. The blob becomes responsible for
-     * changing to non-secure state if implementing a non-secure boot
+     * changing to non-secure state if implementing a non-secure boot,
+     * including setting up EL3/Secure registers such as the NSACR as
+     * required by the Linux booting ABI before the switch to non-secure.
      */
     bool secure_board_setup;
 
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -XXX,XX +XXX,XX @@ void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
     };
     uint32_t board_setup_blob[] = {
         /* board setup addr */
+        0xee110f51, /* mrc     p15, 0, r0, c1, c1, 2  ;read NSACR */
+        0xe3800b03, /* orr     r0, #0xc00             ;set CP11, CP10 */
+        0xee010f51, /* mcr     p15, 0, r0, c1, c1, 2  ;write NSACR */
         0xe3a00e00 + (mvbar_addr >> 4), /* mov r0, #mvbar_addr */
         0xee0c0f30, /* mcr     p15, 0, r0, c12, c0, 1 ;set MVBAR */
         0xee110f11, /* mrc     p15, 0, r0, c1 , c1, 0 ;read SCR */
-- 
2.20.1

Handful of bugfixes for rc2. None of these are particularly critical
or exciting.

-- PMM

The following changes since commit 45a150aa2b3492acf6691c7bdbeb25a8545d8345:

Merge remote-tracking branch 'remotes/ericb/tags/pull-bitmaps-2020-08-03' into staging (2020-08-03 15:13:49 +0100)

are available in the Git repository at:

https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20200803

for you to fetch changes up to 13557fd392890cbd985bceba7f717e01efd674b8:

hw/timer/imx_epit: Avoid assertion when CR.SWR is written (2020-08-03 17:56:11 +0100)

----------------------------------------------------------------
target-arm queue:
 * hw/timer/imx_epit: Avoid assertion when CR.SWR is written
 * netduino2, netduinoplus2, microbit: set system_clock_scale so that
   SysTick running on the CPU clock works
 * target/arm: Avoid maybe-uninitialized warning with gcc 4.9
 * target/arm: Fix AddPAC error indication
 * Make AIRCR.SYSRESETREQ actually reset the system for the
   microbit, mps2-*, musca-*, netduino* boards

----------------------------------------------------------------
Kaige Li (1):
      target/arm: Avoid maybe-uninitialized warning with gcc 4.9

Peter Maydell (6):
      hw/arm/netduino2, netduinoplus2: Set system_clock_scale
      include/hw/irq.h: New function qemu_irq_is_connected()
      hw/intc/armv7m_nvic: Provide default "reset the system" behaviour for SYSRESETREQ
      msf2-soc, stellaris: Don't wire up SYSRESETREQ
      hw/arm/nrf51_soc: Set system_clock_scale
      hw/timer/imx_epit: Avoid assertion when CR.SWR is written

Richard Henderson (1):
      target/arm: Fix AddPAC error indication

include/hw/arm/armv7m.h           |  4 +++-
 include/hw/irq.h                  | 18 ++++++++++++++++++
 hw/arm/msf2-soc.c                 | 11 -----------
 hw/arm/netduino2.c                | 10 ++++++++++
 hw/arm/netduinoplus2.c            | 10 ++++++++++
 hw/arm/nrf51_soc.c                |  5 +++++
 hw/arm/stellaris.c                | 12 ------------
 hw/intc/armv7m_nvic.c             | 17 ++++++++++++++++-
 hw/timer/imx_epit.c               | 13 ++++++++++---
 target/arm/pauth_helper.c         |  6 +++++-
 target/arm/translate-a64.c        |  2 +-
 tests/tcg/aarch64/pauth-5.c       | 33 +++++++++++++++++++++++++++++++++
 tests/tcg/aarch64/Makefile.target |  2 +-
 13 files changed, 112 insertions(+), 31 deletions(-)
 create mode 100644 tests/tcg/aarch64/pauth-5.c

The netduino2 and netduinoplus2 boards forgot to set the system_clock_scale
global, which meant that if guest code used the systick timer in "use
the processor clock" mode it would hang because time never advances.

Set the global to match the documented CPU clock speed of these boards.
Judging by the data sheet this is slightly simplistic because the
SoC allows configuration of the SYSCLK source and frequency via the
RCC (reset and clock control) module, but we don't model that.

Fixes: https://bugs.launchpad.net/qemu/+bug/1876187
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200727162617.26227-1-peter.maydell@linaro.org
---
 hw/arm/netduino2.c     | 10 ++++++++++
 hw/arm/netduinoplus2.c | 10 ++++++++++
 2 files changed, 20 insertions(+)

diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/arm/netduino2.c
+++ b/hw/arm/netduino2.c
@@ -XXX,XX +XXX,XX @@
 #include "hw/arm/stm32f205_soc.h"
 #include "hw/arm/boot.h"
 
+/* Main SYSCLK frequency in Hz (120MHz) */
+#define SYSCLK_FRQ 120000000ULL
+
 static void netduino2_init(MachineState *machine)
 {
     DeviceState *dev;
 
+    /*
+     * TODO: ideally we would model the SoC RCC and let it handle
+     * system_clock_scale, including its ability to define different
+     * possible SYSCLK sources.
+     */
+    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
+
     dev = qdev_new(TYPE_STM32F205_SOC);
     qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
diff --git a/hw/arm/netduinoplus2.c b/hw/arm/netduinoplus2.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/arm/netduinoplus2.c
+++ b/hw/arm/netduinoplus2.c
@@ -XXX,XX +XXX,XX @@
 #include "hw/arm/stm32f405_soc.h"
 #include "hw/arm/boot.h"
 
+/* Main SYSCLK frequency in Hz (168MHz) */
+#define SYSCLK_FRQ 168000000ULL
+
 static void netduinoplus2_init(MachineState *machine)
 {
     DeviceState *dev;
 
+    /*
+     * TODO: ideally we would model the SoC RCC and let it handle
+     * system_clock_scale, including its ability to define different
+     * possible SYSCLK sources.
+     */
+    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
+
     dev = qdev_new(TYPE_STM32F405_SOC);
     qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-- 
2.20.1

Mostly devices don't need to care whether one of their output
qemu_irq lines is connected, because functions like qemu_set_irq()
silently do nothing if there is nothing on the other end.  However
sometimes a device might want to implement default behaviour for the
case where the machine hasn't wired the line up to anywhere.

Provide a function qemu_irq_is_connected() that devices can use for
this purpose.  (The test is trivial but encapsulating it in a
function makes it easier to see where we're doing it in case we need
to change the implementation later.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200728103744.6909-2-peter.maydell@linaro.org
---
 include/hw/irq.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/include/hw/irq.h b/include/hw/irq.h
index XXXXXXX..XXXXXXX 100644
--- a/include/hw/irq.h
+++ b/include/hw/irq.h
@@ -XXX,XX +XXX,XX @@ qemu_irq qemu_irq_split(qemu_irq irq1, qemu_irq irq2);
    on an existing vector of qemu_irq.  */
 void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n);
 
+/**
+ * qemu_irq_is_connected: Return true if IRQ line is wired up
+ *
+ * If a qemu_irq has a device on the other (receiving) end of it,
+ * return true; otherwise return false.
+ *
+ * Usually device models don't need to care whether the machine model
+ * has wired up their outbound qemu_irq lines, because functions like
+ * qemu_set_irq() silently do nothing if there is nothing on the other
+ * end of the line. However occasionally a device model will want to
+ * provide default behaviour if its output is left floating, and
+ * it can use this function to identify when that is the case.
+ */
+static inline bool qemu_irq_is_connected(qemu_irq irq)
+{
+    return irq != NULL;
+}
+
 #endif
-- 
2.20.1

The NVIC provides an outbound qemu_irq "SYSRESETREQ" which it signals
when the guest sets the SYSRESETREQ bit in the AIRCR register.  This
matches the hardware design (where the CPU has a signal of this name
and it is up to the SoC to connect that up to an actual reset
mechanism), but in QEMU it mostly results in duplicated code in SoC
objects and bugs where SoC model implementors forget to wire up the
SYSRESETREQ line.

Provide a default behaviour for the case where SYSRESETREQ is not
actually connected to anything: use qemu_system_reset_request() to
perform a system reset.  This will allow us to remove the
implementations of SYSRESETREQ handling from the boards where that's
exactly what it does, and also fixes the bugs in the board models
which forgot to wire up the signal:

* microbit
 * mps2-an385
 * mps2-an505
 * mps2-an511
 * mps2-an521
 * musca-a
 * musca-b1
 * netduino
 * netduinoplus2

We still allow the board to wire up the signal if it needs to, in case
we need to model more complicated reset controller logic or to model
buggy SoC hardware which forgot to wire up the line itself. But
defaulting to "reset the system" is more often going to be correct
than defaulting to "do nothing".

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200728103744.6909-3-peter.maydell@linaro.org
---
 include/hw/arm/armv7m.h |  4 +++-
 hw/intc/armv7m_nvic.c   | 17 ++++++++++++++++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
index XXXXXXX..XXXXXXX 100644
--- a/include/hw/arm/armv7m.h
+++ b/include/hw/arm/armv7m.h
@@ -XXX,XX +XXX,XX @@ typedef struct {
 
 /* ARMv7M container object.
  * + Unnamed GPIO input lines: external IRQ lines for the NVIC
- * + Named GPIO output SYSRESETREQ: signalled for guest AIRCR.SYSRESETREQ
+ * + Named GPIO output SYSRESETREQ: signalled for guest AIRCR.SYSRESETREQ.
+ *   If this GPIO is not wired up then the NVIC will default to performing
+ *   a qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET).
  * + Property "cpu-type": CPU type to instantiate
  * + Property "num-irq": number of external IRQ lines
  * + Property "memory": MemoryRegion defining the physical address space
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -XXX,XX +XXX,XX @@
 #include "hw/intc/armv7m_nvic.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
+#include "sysemu/runstate.h"
 #include "target/arm/cpu.h"
 #include "exec/exec-all.h"
 #include "exec/memop.h"
@@ -XXX,XX +XXX,XX @@ static const uint8_t nvic_id[] = {
     0x00, 0xb0, 0x1b, 0x00, 0x0d, 0xe0, 0x05, 0xb1
 };
 
+static void signal_sysresetreq(NVICState *s)
+{
+    if (qemu_irq_is_connected(s->sysresetreq)) {
+        qemu_irq_pulse(s->sysresetreq);
+    } else {
+        /*
+         * Default behaviour if the SoC doesn't need to wire up
+         * SYSRESETREQ (eg to a system reset controller of some kind):
+         * perform a system reset via the usual QEMU API.
+         */
+        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+    }
+}
+
 static int nvic_pending_prio(NVICState *s)
 {
     /* return the group priority of the current pending interrupt,
@@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value,
             if (value & R_V7M_AIRCR_SYSRESETREQ_MASK) {
                 if (attrs.secure ||
                     !(cpu->env.v7m.aircr & R_V7M_AIRCR_SYSRESETREQS_MASK)) {
-                    qemu_irq_pulse(s->sysresetreq);
+                    signal_sysresetreq(s);
                 }
             }
             if (value & R_V7M_AIRCR_VECTCLRACTIVE_MASK) {
-- 
2.20.1

The MSF2 SoC model and the Stellaris board code both wire
SYSRESETREQ up to a function that just invokes
    qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
This is now the default action that the NVIC does if the line is
not connected, so we can delete the handling code.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-id: 20200728103744.6909-4-peter.maydell@linaro.org
---
 hw/arm/msf2-soc.c  | 11 -----------
 hw/arm/stellaris.c | 12 ------------
 2 files changed, 23 deletions(-)

diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/arm/msf2-soc.c
+++ b/hw/arm/msf2-soc.c
@@ -XXX,XX +XXX,XX @@
 #include "hw/irq.h"
 #include "hw/arm/msf2-soc.h"
 #include "hw/misc/unimp.h"
-#include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
 
 #define MSF2_TIMER_BASE       0x40004000
@@ -XXX,XX +XXX,XX @@ static const int spi_irq[MSF2_NUM_SPIS] = { 2, 3 };
 static const int uart_irq[MSF2_NUM_UARTS] = { 10, 11 };
 static const int timer_irq[MSF2_NUM_TIMERS] = { 14, 15 };
 
-static void do_sys_reset(void *opaque, int n, int level)
-{
-    if (level) {
-        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
-    }
-}
-
 static void m2sxxx_soc_initfn(Object *obj)
 {
     MSF2State *s = MSF2_SOC(obj);
@@ -XXX,XX +XXX,XX @@ static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp)
         return;
     }
 
-    qdev_connect_gpio_out_named(DEVICE(&s->armv7m.nvic), "SYSRESETREQ", 0,
-                                qemu_allocate_irq(&do_sys_reset, NULL, 0));
-
     system_clock_scale = NANOSECONDS_PER_SECOND / s->m3clk;
 
     for (i = 0; i < MSF2_NUM_UARTS; i++) {
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -XXX,XX +XXX,XX @@
 #include "hw/boards.h"
 #include "qemu/log.h"
 #include "exec/address-spaces.h"
-#include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
 #include "hw/arm/armv7m.h"
 #include "hw/char/pl011.h"
@@ -XXX,XX +XXX,XX @@ static void stellaris_adc_init(Object *obj)
     qdev_init_gpio_in(dev, stellaris_adc_trigger, 1);
 }
 
-static
-void do_sys_reset(void *opaque, int n, int level)
-{
-    if (level) {
-        qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
-    }
-}
-
 /* Board init.  */
 static stellaris_board_info stellaris_boards[] = {
   { "LM3S811EVB",
@@ -XXX,XX +XXX,XX @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
     /* This will exit with an error if the user passed us a bad cpu_type */
     sysbus_realize_and_unref(SYS_BUS_DEVICE(nvic), &error_fatal);
 
-    qdev_connect_gpio_out_named(nvic, "SYSRESETREQ", 0,
-                                qemu_allocate_irq(&do_sys_reset, NULL, 0));
-
     if (board->dc1 & (1 << 16)) {
         dev = sysbus_create_varargs(TYPE_STELLARIS_ADC, 0x40038000,
                                     qdev_get_gpio_in(nvic, 14),
-- 
2.20.1

From: Richard Henderson <richard.henderson@linaro.org>

The definition of top_bit used in this function is one higher
than that used in the Arm ARM psuedo-code, which put the error
indication at top_bit - 1 at the wrong place, which meant that
it wasn't visible to Auth.

Fixing the definition of top_bit requires more changes, because
its most common use is for the count of bits in top_bit:bot_bit,
which would then need to be computed as top_bit - bot_bit + 1.

For now, prefer the minimal fix to the error indication alone.

Fixes: 63ff0ca94cb
Reported-by: Derrick McKee <derrick.mckee@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20200728195706.11087-1-richard.henderson@linaro.org
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
[PMM: added comment about the divergence from the pseudocode]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/pauth_helper.c         |  6 +++++-
 tests/tcg/aarch64/pauth-5.c       | 33 +++++++++++++++++++++++++++++++
 tests/tcg/aarch64/Makefile.target |  2 +-
 3 files changed, 39 insertions(+), 2 deletions(-)
 create mode 100644 tests/tcg/aarch64/pauth-5.c

diff --git a/target/arm/pauth_helper.c b/target/arm/pauth_helper.c
index XXXXXXX..XXXXXXX 100644
--- a/target/arm/pauth_helper.c
+++ b/target/arm/pauth_helper.c
@@ -XXX,XX +XXX,XX @@ static uint64_t pauth_addpac(CPUARMState *env, uint64_t ptr, uint64_t modifier,
      */
     test = sextract64(ptr, bot_bit, top_bit - bot_bit);
     if (test != 0 && test != -1) {
-        pac ^= MAKE_64BIT_MASK(top_bit - 1, 1);
+        /*
+         * Note that our top_bit is one greater than the pseudocode's
+         * version, hence "- 2" here.
+         */
+        pac ^= MAKE_64BIT_MASK(top_bit - 2, 1);
     }
 
     /*
diff --git a/tests/tcg/aarch64/pauth-5.c b/tests/tcg/aarch64/pauth-5.c
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/tcg/aarch64/pauth-5.c
@@ -XXX,XX +XXX,XX @@
+#include <assert.h>
+
+static int x;
+
+int main()
+{
+    int *p0 = &x, *p1, *p2, *p3;
+    unsigned long salt = 0;
+
+    /*
+     * With TBI enabled and a 48-bit VA, there are 7 bits of auth, and so
+     * a 1/128 chance of auth = pac(ptr,key,salt) producing zero.
+     * Find a salt that creates auth != 0.
+     */
+    do {
+        salt++;
+        asm("pacda %0, %1" : "=r"(p1) : "r"(salt), "0"(p0));
+    } while (p0 == p1);
+
+    /*
+     * This pac must fail, because the input pointer bears an encryption,
+     * and so is not properly extended within bits [55:47].  This will
+     * toggle bit 54 in the output...
+     */
+    asm("pacda %0, %1" : "=r"(p2) : "r"(salt), "0"(p1));
+
+    /* ... so that the aut must fail, setting bit 53 in the output ... */
+    asm("autda %0, %1" : "=r"(p3) : "r"(salt), "0"(p2));
+
+    /* ... which means this equality must not hold. */
+    assert(p3 != p0);
+    return 0;
+}
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
index XXXXXXX..XXXXXXX 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -XXX,XX +XXX,XX @@ run-fcvt: fcvt
 
 # Pauth Tests
 ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_ARMV8_3),)
-AARCH64_TESTS += pauth-1 pauth-2 pauth-4
+AARCH64_TESTS += pauth-1 pauth-2 pauth-4 pauth-5
 pauth-%: CFLAGS += -march=armv8.3-a
 run-pauth-%: QEMU_OPTS += -cpu max
 run-plugin-pauth-%: QEMU_OPTS += -cpu max
-- 
2.20.1

From: Kaige Li <likaige@loongson.cn>

GCC version 4.9.4 isn't clever enough to figure out that all
execution paths in disas_ldst() that use 'fn' will have initialized
it first, and so it warns:

/home/LiKaige/qemu/target/arm/translate-a64.c: In function ‘disas_ldst’:
/home/LiKaige/qemu/target/arm/translate-a64.c:3392:5: error: ‘fn’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     fn(cpu_reg(s, rt), clean_addr, tcg_rs, get_mem_index(s),
     ^
/home/LiKaige/qemu/target/arm/translate-a64.c:3318:22: note: ‘fn’ was declared here
     AtomicThreeOpFn *fn;
                      ^

Make it happy by initializing the variable to NULL.

Signed-off-by: Kaige Li <likaige@loongson.cn>
Message-id: 1596110248-7366-2-git-send-email-likaige@loongson.cn
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
[PMM: Clean up commit message and note which gcc version this was]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/translate-a64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index XXXXXXX..XXXXXXX 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -XXX,XX +XXX,XX @@ static void disas_ldst_atomic(DisasContext *s, uint32_t insn,
     bool r = extract32(insn, 22, 1);
     bool a = extract32(insn, 23, 1);
     TCGv_i64 tcg_rs, clean_addr;
-    AtomicThreeOpFn *fn;
+    AtomicThreeOpFn *fn = NULL;
 
     if (is_vector || !dc_isar_feature(aa64_atomics, s)) {
         unallocated_encoding(s);
-- 
2.20.1

The nrf51 SoC model wasn't setting the system_clock_scale
global.which meant that if guest code used the systick timer in "use
the processor clock" mode it would hang because time never advances.

Set the global to match the documented CPU clock speed for this SoC.

This SoC in fact doesn't have a SysTick timer (which is the only thing
currently that cares about the system_clock_scale), because it's
a configurable option in the Cortex-M0. However our Cortex-M0 and
thus our nrf51 and our micro:bit board do provide a SysTick, so
we ought to provide a functional one rather than a broken one.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20200727193458.31250-1-peter.maydell@linaro.org
---
 hw/arm/nrf51_soc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/arm/nrf51_soc.c b/hw/arm/nrf51_soc.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/arm/nrf51_soc.c
+++ b/hw/arm/nrf51_soc.c
@@ -XXX,XX +XXX,XX @@
 
 #define BASE_TO_IRQ(base) ((base >> 12) & 0x1F)
 
+/* HCLK (the main CPU clock) on this SoC is always 16MHz */
+#define HCLK_FRQ 16000000
+
 static uint64_t clock_read(void *opaque, hwaddr addr, unsigned int size)
 {
     qemu_log_mask(LOG_UNIMP, "%s: 0x%" HWADDR_PRIx " [%u]\n",
@@ -XXX,XX +XXX,XX @@ static void nrf51_soc_realize(DeviceState *dev_soc, Error **errp)
         return;
     }
 
+    system_clock_scale = NANOSECONDS_PER_SECOND / HCLK_FRQ;
+
     object_property_set_link(OBJECT(&s->cpu), "memory", OBJECT(&s->container),
                              &error_abort);
     if (!sysbus_realize(SYS_BUS_DEVICE(&s->cpu), errp)) {
-- 
2.20.1

The imx_epit device has a software-controllable reset triggered by
setting the SWR bit in the CR register. An error in commit cc2722ec83ad9
means that we will end up assert()ing if the guest does this, because
the code in imx_epit_write() starts ptimer transactions, and then
imx_epit_reset() also starts ptimer transactions, triggering
"ptimer_transaction_begin: Assertion `!s->in_transaction' failed".

The cleanest way to avoid this double-transaction is to move the
start-transaction for the CR write handling down below the check of
the SWR bit.

Fixes: https://bugs.launchpad.net/qemu/+bug/1880424
Fixes: cc2722ec83ad944505fe
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20200727154550.3409-1-peter.maydell@linaro.org
---
 hw/timer/imx_epit.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/hw/timer/imx_epit.c b/hw/timer/imx_epit.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/timer/imx_epit.c
+++ b/hw/timer/imx_epit.c
@@ -XXX,XX +XXX,XX @@ static void imx_epit_write(void *opaque, hwaddr offset, uint64_t value,
 
     switch (offset >> 2) {
     case 0: /* CR */
-        ptimer_transaction_begin(s->timer_cmp);
-        ptimer_transaction_begin(s->timer_reload);
 
         oldcr = s->cr;
         s->cr = value & 0x03ffffff;
         if (s->cr & CR_SWR) {
             /* handle the reset */
             imx_epit_reset(DEVICE(s));
-        } else {
+            /*
+             * TODO: could we 'break' here? following operations appear
+             * to duplicate the work imx_epit_reset() already did.
+             */
+        }
+
+        ptimer_transaction_begin(s->timer_cmp);
+        ptimer_transaction_begin(s->timer_reload);
+
+        if (!(s->cr & CR_SWR)) {
             imx_epit_set_freq(s);
         }
 
-- 
2.20.1