target-arm queue for 3.1: mostly bug fixes, but the "turn on

EL2 support for Cortex-A7 and -A15" is technically enabling

of a new feature... I think this is OK since we're only at rc1,

and it's easy to revert that feature bit flip if necessary.

The following changes since commit 5704c36d25ee84e7129722cb0db53df9faefe943:

Merge remote-tracking branch 'remotes/kraxel/tags/fixes-31-20181112-pull-request' into staging (2018-11-12 15:55:40 +0000)

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

for you to fetch changes up to 1a4c1a6dbf60aebddd07753f1013ea896c06ad29:

target/arm/cpu: Give Cortex-A15 and -A7 the EL2 feature (2018-11-12 16:52:29 +0000)

target/arm queue:

* Remove no-longer-needed workaround for small SAU regions for v8M

* Remove antique TODO comment

* MAINTAINERS: Add an entry for the 'collie' machine

* hw/arm/sysbus-fdt: Only call match_fn callback if the type matches

* Fix infinite recursion in tlbi_aa64_vmalle1_write()

* ARM KVM: fix various bugs in handling of guest debugging

* Correctly implement handling of HCR_EL2.{VI, VF}

* Hyp mode R14 is shared with User and System

* Give Cortex-A15 and -A7 the EL2 feature

Alex Bennée (6):

target/arm64: properly handle DBGVR RESS bits

target/arm64: hold BQL when calling do_interrupt()

target/arm64: kvm debug set target_el when passing exception to guest

tests/guest-debug: fix scoping of failcount

arm: use symbolic MDCR_TDE in arm_debug_target_el

arm: fix aa64_generate_debug_exceptions to work with EL2

Eric Auger (1):

hw/arm/sysbus-fdt: Only call match_fn callback if the type matches

Peter Maydell (7):

target/arm: Remove workaround for small SAU regions

target/arm: Remove antique TODO comment

Revert "target/arm: Implement HCR.VI and VF"

target/arm: Track the state of our irq lines from the GIC explicitly

target/arm: Correctly implement handling of HCR_EL2.{VI, VF}

target/arm: Hyp mode R14 is shared with User and System

target/arm/cpu: Give Cortex-A15 and -A7 the EL2 feature

Richard Henderson (1):

target/arm: Fix typo in tlbi_aa64_vmalle1_write

Thomas Huth (1):

MAINTAINERS: Add an entry for the 'collie' machine

target/arm/cpu.h | 44 +++++++++++------

target/arm/internals.h | 34 +++++++++++++

hw/arm/sysbus-fdt.c | 12 +++--

target/arm/cpu.c | 66 ++++++++++++++++++++++++-

target/arm/helper.c | 101 +++++++++++++-------------------------

target/arm/kvm32.c | 4 +-

target/arm/kvm64.c | 20 +++++++-

target/arm/machine.c | 51 +++++++++++++++++++

target/arm/op_helper.c | 4 +-

MAINTAINERS | 7 +++

tests/guest-debug/test-gdbstub.py | 1 +

11 files changed, 248 insertions(+), 96 deletions(-)

Hyp mode is an exception to the general rule that each AArch32

mode has its own r13, r14 and SPSR -- it has a banked r13 and

SPSR but shares its r14 with User and System mode. We were

incorrectly implementing it as banked, which meant that on

entry to Hyp mode r14 was 0 rather than the USR/SYS r14.

We provide a new function r14_bank_number() which is like

the existing bank_number() but provides the index into

env->banked_r14[]; bank_number() provides the index to use

for env->banked_r13[] and env->banked_cpsr[].

All the points in the code that were using bank_number()

to index into env->banked_r14[] are updated for consintency:

* switch_mode() -- this is the only place where we fix

an actual bug

* aarch64_sync_32_to_64() and aarch64_sync_64_to_32():

no behavioural change as we already special-cased Hyp R14

* kvm32.c: no behavioural change since the guest can't ever

be in Hyp mode, but conceptually the right thing to do

* msr_banked()/mrs_banked(): we can never get to the case

that accesses banked_r14[] with tgtmode == ARM_CPU_MODE_HYP,

so no behavioural change

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

Message-id: 20181109173553.22341-2-peter.maydell@linaro.org

---

target/arm/internals.h | 16 ++++++++++++++++

target/arm/helper.c | 29 +++++++++++++++--------------

target/arm/kvm32.c | 4 ++--

target/arm/op_helper.c | 4 ++--

4 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h

--- a/target/arm/internals.h

+++ b/target/arm/internals.h

@@ -XXX,XX +XXX,XX @@ static inline int bank_number(int mode)

g_assert_not_reached();

+/**

+ * r14_bank_number: Map CPU mode onto register bank for r14

+ *

+ * Given an AArch32 CPU mode, return the index into the saved register

+ * banks to use for the R14 (LR) in that mode. This is the same as

+ * bank_number(), except for the special case of Hyp mode, where

+ * R14 is shared with USR and SYS, unlike its R13 and SPSR.

+ * This should be used as the index into env->banked_r14[], and

+ * bank_number() used for the index into env->banked_r13[] and

+ * env->banked_spsr[].

+static inline int r14_bank_number(int mode)

+ return (mode == ARM_CPU_MODE_HYP) ? BANK_USRSYS : bank_number(mode);

void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu);

void arm_translate_init(void);

diff --git a/target/arm/helper.c b/target/arm/helper.c

--- a/target/arm/helper.c

+++ b/target/arm/helper.c

@@ -XXX,XX +XXX,XX @@ static void switch_mode(CPUARMState *env, int mode)

i = bank_number(old_mode);

env->banked_r13[i] = env->regs[13];

- env->banked_r14[i] = env->regs[14];

env->banked_spsr[i] = env->spsr;

i = bank_number(mode);

env->regs[13] = env->banked_r13[i];

- env->regs[14] = env->banked_r14[i];

env->spsr = env->banked_spsr[i];

+

+ env->banked_r14[r14_bank_number(old_mode)] = env->regs[14];

+ env->regs[14] = env->banked_r14[r14_bank_number(mode)];

/* Physical Interrupt Target EL Lookup Table

@@ -XXX,XX +XXX,XX @@ void aarch64_sync_32_to_64(CPUARMState *env)

if (mode == ARM_CPU_MODE_HYP) {

env->xregs[14] = env->regs[14];

- env->xregs[14] = env->banked_r14[bank_number(ARM_CPU_MODE_USR)];

+ env->xregs[14] = env->banked_r14[r14_bank_number(ARM_CPU_MODE_USR)];

@@ -XXX,XX +XXX,XX @@ void aarch64_sync_32_to_64(CPUARMState *env)

env->xregs[16] = env->regs[14];

env->xregs[17] = env->regs[13];

} else {

- env->xregs[16] = env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)];

+ env->xregs[16] = env->banked_r14[r14_bank_number(ARM_CPU_MODE_IRQ)];

env->xregs[17] = env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)];

@@ -XXX,XX +XXX,XX @@ void aarch64_sync_32_to_64(CPUARMState *env)

env->xregs[18] = env->regs[14];

env->xregs[19] = env->regs[13];

} else {

- env->xregs[18] = env->banked_r14[bank_number(ARM_CPU_MODE_SVC)];

+ env->xregs[18] = env->banked_r14[r14_bank_number(ARM_CPU_MODE_SVC)];

env->xregs[19] = env->banked_r13[bank_number(ARM_CPU_MODE_SVC)];

@@ -XXX,XX +XXX,XX @@ void aarch64_sync_32_to_64(CPUARMState *env)

env->xregs[20] = env->regs[14];

env->xregs[21] = env->regs[13];

} else {

- env->xregs[20] = env->banked_r14[bank_number(ARM_CPU_MODE_ABT)];

+ env->xregs[20] = env->banked_r14[r14_bank_number(ARM_CPU_MODE_ABT)];

env->xregs[21] = env->banked_r13[bank_number(ARM_CPU_MODE_ABT)];

@@ -XXX,XX +XXX,XX @@ void aarch64_sync_32_to_64(CPUARMState *env)

env->xregs[22] = env->regs[14];

env->xregs[23] = env->regs[13];

} else {

- env->xregs[22] = env->banked_r14[bank_number(ARM_CPU_MODE_UND)];

+ env->xregs[22] = env->banked_r14[r14_bank_number(ARM_CPU_MODE_UND)];

env->xregs[23] = env->banked_r13[bank_number(ARM_CPU_MODE_UND)];

}

@@ -XXX,XX +XXX,XX @@ void aarch64_sync_32_to_64(CPUARMState *env)

env->xregs[i] = env->fiq_regs[i - 24];

}

env->xregs[29] = env->banked_r13[bank_number(ARM_CPU_MODE_FIQ)];

- env->xregs[30] = env->banked_r14[bank_number(ARM_CPU_MODE_FIQ)];

+ env->xregs[30] = env->banked_r14[r14_bank_number(ARM_CPU_MODE_FIQ)];

}

env->pc = env->regs[15];

@@ -XXX,XX +XXX,XX @@ void aarch64_sync_64_to_32(CPUARMState *env)

if (mode == ARM_CPU_MODE_HYP) {

env->regs[14] = env->xregs[14];

} else {

- env->banked_r14[bank_number(ARM_CPU_MODE_USR)] = env->xregs[14];

+ env->banked_r14[r14_bank_number(ARM_CPU_MODE_USR)] = env->xregs[14];

}

}

@@ -XXX,XX +XXX,XX @@ void aarch64_sync_64_to_32(CPUARMState *env)

env->regs[14] = env->xregs[16];

env->regs[13] = env->xregs[17];

} else {

- env->banked_r14[bank_number(ARM_CPU_MODE_IRQ)] = env->xregs[16];

+ env->banked_r14[r14_bank_number(ARM_CPU_MODE_IRQ)] = env->xregs[16];

env->banked_r13[bank_number(ARM_CPU_MODE_IRQ)] = env->xregs[17];

}

@@ -XXX,XX +XXX,XX @@ void aarch64_sync_64_to_32(CPUARMState *env)

env->regs[14] = env->xregs[18];

env->regs[13] = env->xregs[19];

} else {

- env->banked_r14[bank_number(ARM_CPU_MODE_SVC)] = env->xregs[18];

+ env->banked_r14[r14_bank_number(ARM_CPU_MODE_SVC)] = env->xregs[18];

env->banked_r13[bank_number(ARM_CPU_MODE_SVC)] = env->xregs[19];

}

@@ -XXX,XX +XXX,XX @@ void aarch64_sync_64_to_32(CPUARMState *env)

env->regs[14] = env->xregs[20];

env->regs[13] = env->xregs[21];

} else {

- env->banked_r14[bank_number(ARM_CPU_MODE_ABT)] = env->xregs[20];

+ env->banked_r14[r14_bank_number(ARM_CPU_MODE_ABT)] = env->xregs[20];

env->banked_r13[bank_number(ARM_CPU_MODE_ABT)] = env->xregs[21];

}

@@ -XXX,XX +XXX,XX @@ void aarch64_sync_64_to_32(CPUARMState *env)

env->regs[14] = env->xregs[22];

env->regs[13] = env->xregs[23];

} else {

- env->banked_r14[bank_number(ARM_CPU_MODE_UND)] = env->xregs[22];

+ env->banked_r14[r14_bank_number(ARM_CPU_MODE_UND)] = env->xregs[22];

env->banked_r13[bank_number(ARM_CPU_MODE_UND)] = env->xregs[23];

}

@@ -XXX,XX +XXX,XX @@ void aarch64_sync_64_to_32(CPUARMState *env)

env->fiq_regs[i - 24] = env->xregs[i];

}

env->banked_r13[bank_number(ARM_CPU_MODE_FIQ)] = env->xregs[29];

- env->banked_r14[bank_number(ARM_CPU_MODE_FIQ)] = env->xregs[30];

+ env->banked_r14[r14_bank_number(ARM_CPU_MODE_FIQ)] = env->xregs[30];

}

env->regs[15] = env->pc;

diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/kvm32.c

+++ b/target/arm/kvm32.c

@@ -XXX,XX +XXX,XX @@ int kvm_arch_put_registers(CPUState *cs, int level)

memcpy(env->usr_regs, env->regs + 8, 5 * sizeof(uint32_t));

}

env->banked_r13[bn] = env->regs[13];

- env->banked_r14[bn] = env->regs[14];

env->banked_spsr[bn] = env->spsr;

+ env->banked_r14[r14_bank_number(mode)] = env->regs[14];

/* Now we can safely copy stuff down to the kernel */

for (i = 0; i < ARRAY_SIZE(regs); i++) {

@@ -XXX,XX +XXX,XX @@ int kvm_arch_get_registers(CPUState *cs)

memcpy(env->regs + 8, env->usr_regs, 5 * sizeof(uint32_t));

}

env->regs[13] = env->banked_r13[bn];

- env->regs[14] = env->banked_r14[bn];

env->spsr = env->banked_spsr[bn];

+ env->regs[14] = env->banked_r14[r14_bank_number(mode)];

/* VFP registers */

r.id = KVM_REG_ARM | KVM_REG_SIZE_U64 | KVM_REG_ARM_VFP;

diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/op_helper.c

+++ b/target/arm/op_helper.c

@@ -XXX,XX +XXX,XX @@ void HELPER(msr_banked)(CPUARMState *env, uint32_t value, uint32_t tgtmode,

env->banked_r13[bank_number(tgtmode)] = value;

break;

case 14:

- env->banked_r14[bank_number(tgtmode)] = value;

+ env->banked_r14[r14_bank_number(tgtmode)] = value;

break;

case 8 ... 12:

switch (tgtmode) {

@@ -XXX,XX +XXX,XX @@ uint32_t HELPER(mrs_banked)(CPUARMState *env, uint32_t tgtmode, uint32_t regno)

case 13:

return env->banked_r13[bank_number(tgtmode)];

case 14:

- return env->banked_r14[bank_number(tgtmode)];

+ return env->banked_r14[r14_bank_number(tgtmode)];

case 8 ... 12:

switch (tgtmode) {

case ARM_CPU_MODE_USR:

2.19.1

From: Alex Bennée <alex.bennee@linaro.org>

When we are debugging the guest all exceptions come our way but might

be for the guest's own debug exceptions. We use the ->do_interrupt()

infrastructure to inject the exception into the guest. However, we are

missing a full setup of the exception structure, causing an assert

later down the line.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

target/arm/kvm64.c | 1 +

1 file changed, 1 insertion(+)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c

--- a/target/arm/kvm64.c

+++ b/target/arm/kvm64.c

@@ -XXX,XX +XXX,XX @@ bool kvm_arm_handle_debug(CPUState *cs, struct kvm_debug_exit_arch *debug_exit)

cs->exception_index = EXCP_BKPT;

env->exception.syndrome = debug_exit->hsr;

env->exception.vaddress = debug_exit->far;

+ env->exception.target_el = 1;

qemu_mutex_lock_iothread();

cc->do_interrupt(cs);

qemu_mutex_unlock_iothread();

2.19.1

From: Alex Bennée <alex.bennee@linaro.org>

The test was incomplete and incorrectly caused debug exceptions to be

generated when returning to EL2 after a failed attempt to single-step

an EL1 instruction. Fix this while cleaning up the function a little.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20181109152119.9242-8-alex.bennee@linaro.org

target/arm/cpu.h | 39 ++++++++++++++++++++++++---------------

1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h

--- a/target/arm/cpu.h

+++ b/target/arm/cpu.h

@@ -XXX,XX +XXX,XX @@ static inline bool arm_v7m_csselr_razwi(ARMCPU *cpu)

return (cpu->clidr & R_V7M_CLIDR_CTYPE_ALL_MASK) != 0;

+/* See AArch64.GenerateDebugExceptionsFrom() in ARM ARM pseudocode */

static inline bool aa64_generate_debug_exceptions(CPUARMState *env)

- if (arm_is_secure(env)) {

- /* MDCR_EL3.SDD disables debug events from Secure state */

- if (extract32(env->cp15.mdcr_el3, 16, 1) != 0

- || arm_current_el(env) == 3) {

- return false;

+ int cur_el = arm_current_el(env);

+ int debug_el;

+

+ if (cur_el == 3) {

+ return false;

2.19.1

In commit 8a0fc3a29fc2315325400 we tried to implement HCR_EL2.{VI,VF},

but we got it wrong and had to revert it.

In that commit we implemented them as simply tracking whether there

is a pending virtual IRQ or virtual FIQ. This is not correct -- these

bits cause a software-generated VIRQ/VFIQ, which is distinct from

whether there is a hardware-generated VIRQ/VFIQ caused by the

external interrupt controller. So we need to track separately

the HCR_EL2 bit state and the external virq/vfiq line state, and

OR the two together to get the actual pending VIRQ/VFIQ state.

Fixes: 8a0fc3a29fc2315325400c738f807d0d4ae0ab7f

target/arm/internals.h | 18 ++++++++++++++++

target/arm/cpu.c | 48 +++++++++++++++++++++++++++++++++++++++++-

target/arm/helper.c | 20 ++++++++++++++++--

3 files changed, 83 insertions(+), 3 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h

--- a/target/arm/internals.h

+++ b/target/arm/internals.h

@@ -XXX,XX +XXX,XX @@ static inline const char *aarch32_mode_name(uint32_t psr)

return cpu_mode_names[psr & 0xf];

}

+/**

+ * arm_cpu_update_virq: Update CPU_INTERRUPT_VIRQ bit in cs->interrupt_request

+ *

+ * Update the CPU_INTERRUPT_VIRQ bit in cs->interrupt_request, following

+ * a change to either the input VIRQ line from the GIC or the HCR_EL2.VI bit.

+ * Must be called with the iothread lock held.

+ */

+void arm_cpu_update_virq(ARMCPU *cpu);

+

+/**

+ * arm_cpu_update_vfiq: Update CPU_INTERRUPT_VFIQ bit in cs->interrupt_request

+ *

+ * Update the CPU_INTERRUPT_VFIQ bit in cs->interrupt_request, following

+ * a change to either the input VFIQ line from the GIC or the HCR_EL2.VF bit.

+ * Must be called with the iothread lock held.

+ */

+void arm_cpu_update_vfiq(ARMCPU *cpu);

+

#endif

diff --git a/target/arm/cpu.c b/target/arm/cpu.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/cpu.c

+++ b/target/arm/cpu.c

@@ -XXX,XX +XXX,XX @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)

}

#endif

+void arm_cpu_update_virq(ARMCPU *cpu)

+ /*

+ * Update the interrupt level for VIRQ, which is the logical OR of

+ * the HCR_EL2.VI bit and the input line level from the GIC.

+ */

+ CPUARMState *env = &cpu->env;

+ CPUState *cs = CPU(cpu);

+

+ bool new_state = (env->cp15.hcr_el2 & HCR_VI) ||

+ (env->irq_line_state & CPU_INTERRUPT_VIRQ);

+

+ if (new_state != ((cs->interrupt_request & CPU_INTERRUPT_VIRQ) != 0)) {

+ if (new_state) {

+ cpu_interrupt(cs, CPU_INTERRUPT_VIRQ);

+ } else {

+ cpu_reset_interrupt(cs, CPU_INTERRUPT_VIRQ);

+ }

+void arm_cpu_update_vfiq(ARMCPU *cpu)

+ /*

+ * Update the interrupt level for VFIQ, which is the logical OR of

+ * the HCR_EL2.VF bit and the input line level from the GIC.

+ */

+ CPUARMState *env = &cpu->env;

+ CPUState *cs = CPU(cpu);

+

+ bool new_state = (env->cp15.hcr_el2 & HCR_VF) ||

+ (env->irq_line_state & CPU_INTERRUPT_VFIQ);

+

+ if (new_state != ((cs->interrupt_request & CPU_INTERRUPT_VFIQ) != 0)) {

+ if (new_state) {

+ cpu_interrupt(cs, CPU_INTERRUPT_VFIQ);

+ } else {

+ cpu_reset_interrupt(cs, CPU_INTERRUPT_VFIQ);

+ }

#ifndef CONFIG_USER_ONLY

static void arm_cpu_set_irq(void *opaque, int irq, int level)

@@ -XXX,XX +XXX,XX @@ static void arm_cpu_set_irq(void *opaque, int irq, int level)

switch (irq) {

case ARM_CPU_VIRQ:

+ assert(arm_feature(env, ARM_FEATURE_EL2));

+ arm_cpu_update_virq(cpu);

+ break;

case ARM_CPU_VFIQ:

assert(arm_feature(env, ARM_FEATURE_EL2));

- /* fall through */

+ arm_cpu_update_vfiq(cpu);

+ break;

case ARM_CPU_IRQ:

case ARM_CPU_FIQ:

if (level) {

diff --git a/target/arm/helper.c b/target/arm/helper.c

--- a/target/arm/helper.c

+++ b/target/arm/helper.c

@@ -XXX,XX +XXX,XX @@ static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)

tlb_flush(CPU(cpu));

}

env->cp15.hcr_el2 = value;

+

+ /*

+ * Updates to VI and VF require us to update the status of

+ * virtual interrupts, which are the logical OR of these bits

+ * and the state of the input lines from the GIC. (This requires

+ * that we have the iothread lock, which is done by marking the

+ * reginfo structs as ARM_CP_IO.)

+ * Note that if a write to HCR pends a VIRQ or VFIQ it is never

+ * possible for it to be taken immediately, because VIRQ and

+ * VFIQ are masked unless running at EL0 or EL1, and HCR

+ * can only be written at EL2.

+ */

+ g_assert(qemu_mutex_iothread_locked());

+ arm_cpu_update_virq(cpu);

+ arm_cpu_update_vfiq(cpu);

}

static void hcr_writehigh(CPUARMState *env, const ARMCPRegInfo *ri,

@@ -XXX,XX +XXX,XX @@ static void hcr_writelow(CPUARMState *env, const ARMCPRegInfo *ri,

static const ARMCPRegInfo el2_cp_reginfo[] = {

{ .name = "HCR_EL2", .state = ARM_CP_STATE_AA64,

+ .type = ARM_CP_IO,

.opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,

.access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.hcr_el2),

.writefn = hcr_write },

{ .name = "HCR", .state = ARM_CP_STATE_AA32,

- .type = ARM_CP_ALIAS,

+ .type = ARM_CP_ALIAS | ARM_CP_IO,

.cp = 15, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,

.access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.hcr_el2),

.writefn = hcr_writelow },

@@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo el2_cp_reginfo[] = {

static const ARMCPRegInfo el2_v8_cp_reginfo[] = {

{ .name = "HCR2", .state = ARM_CP_STATE_AA32,

- .type = ARM_CP_ALIAS,

+ .type = ARM_CP_ALIAS | ARM_CP_IO,

.cp = 15, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 4,

.access = PL2_RW,

.fieldoffset = offsetofhigh32(CPUARMState, cp15.hcr_el2),