Arm patches for rc1:

* two final "remove the old API" patches for some API transitions

* bugfix for raspi/highbank Linux boot

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)

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)

* 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

include/hw/arm/boot.h | 7 ++--

include/hw/core/cpu.h | 24 --------------

include/hw/ptimer.h | 45 ++++++++++++-------------

accel/tcg/cputlb.c | 2 --

hw/arm/boot.c | 3 ++

hw/core/ptimer.c | 91 +++++++++------------------------------------------

memory.c | 7 ----

7 files changed, 44 insertions(+), 135 deletions(-)

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

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

--- 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

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

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

--- 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.

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

--- 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