Mostly my stuff with a few easy patches from others. I know I have

a few big series in my to-review queue, but I've been too jetlagged

to try to tackle those :-(

The following changes since commit a26a98dfb9d448d7234d931ae3720feddf6f0651:

Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20171006' into staging (2017-10-06 13:19:03 +0100)

are available in the git repository at:

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

for you to fetch changes up to 04829ce334bece78d4fa1d0fdbc8bc27dae9b242:

nvic: Add missing code for writing SHCSR.HARDFAULTPENDED bit (2017-10-06 16:46:49 +0100)

target-arm:

* v8M: more preparatory work

* nvic: reset properly rather than leaving the nvic in a weird state

* xlnx-zynqmp: Mark the "xlnx, zynqmp" device with user_creatable = false

* sd: fix out-of-bounds check for multi block reads

* arm: Fix SMC reporting to EL2 when QEMU provides PSCI

Jan Kiszka (1):

arm: Fix SMC reporting to EL2 when QEMU provides PSCI

Michael Olbrich (1):

hw/sd: fix out-of-bounds check for multi block reads

Peter Maydell (17):

nvic: Clear the vector arrays and prigroup on reset

target/arm: Don't switch to target stack early in v7M exception return

target/arm: Prepare for CONTROL.SPSEL being nonzero in Handler mode

target/arm: Restore security state on exception return

target/arm: Restore SPSEL to correct CONTROL register on exception return

target/arm: Check for xPSR mismatch usage faults earlier for v8M

target/arm: Warn about restoring to unaligned stack

target/arm: Don't warn about exception return with PC low bit set for v8M

target/arm: Add new-in-v8M SFSR and SFAR

target/arm: Update excret sanity checks for v8M

target/arm: Add support for restoring v8M additional state context

target/arm: Add v8M support to exception entry code

nvic: Implement Security Attribution Unit registers

target/arm: Implement security attribute lookups for memory accesses

target/arm: Fix calculation of secure mm_idx values

target/arm: Factor out "get mmuidx for specified security state"

nvic: Add missing code for writing SHCSR.HARDFAULTPENDED bit

hw/arm/xlnx-zynqmp: Mark the "xlnx, zynqmp" device with user_creatable = false

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

target/arm/internals.h | 15 ++

hw/arm/xlnx-zynqmp.c | 2 +

hw/intc/armv7m_nvic.c | 158 ++++++++++-

hw/sd/sd.c | 12 +-

target/arm/cpu.c | 27 ++

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

target/arm/machine.c | 16 ++

target/arm/op_helper.c | 27 +-

9 files changed, 898 insertions(+), 110 deletions(-)

From: Michael Olbrich <m.olbrich@pengutronix.de>

The current code checks if the next block exceeds the size of the card.

This generates an error while reading the last block of the card.

Do the out-of-bounds check when starting to read a new block to fix this.

This issue became visible with increased error checking in Linux 4.13.

Cc: qemu-stable@nongnu.org

Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Message-id: 20170916091611.10241-1-m.olbrich@pengutronix.de

hw/sd/sd.c | 12 ++++++------

1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c

--- a/hw/sd/sd.c

+++ b/hw/sd/sd.c

@@ -XXX,XX +XXX,XX @@ uint8_t sd_read_data(SDState *sd)

break;

case 18:    /* CMD18: READ_MULTIPLE_BLOCK */

- if (sd->data_offset == 0)

+ if (sd->data_offset == 0) {

+ if (sd->data_start + io_len > sd->size) {

+ sd->card_status |= ADDRESS_ERROR;

+ return 0x00;

+ }

BLK_READ_BLOCK(sd->data_start, io_len);

+ }

ret = sd->data[sd->data_offset ++];

if (sd->data_offset >= io_len) {

@@ -XXX,XX +XXX,XX @@ uint8_t sd_read_data(SDState *sd)

break;

}

}

-

- if (sd->data_start + io_len > sd->size) {

- sd->card_status |= ADDRESS_ERROR;

- break;

- }

}

break;

2.7.4

Implement the security attribute lookups for memory accesses

in the get_phys_addr() functions, causing these to generate

various kinds of SecureFault for bad accesses.

The major subtlety in this code relates to handling of the

case when the security attributes the SAU assigns to the

address don't match the current security state of the CPU.

In the ARM ARM pseudocode for validating instruction

accesses, the security attributes of the address determine

whether the Secure or NonSecure MPU state is used. At face

value, handling this would require us to encode the relevant

bits of state into mmu_idx for both S and NS at once, which

would result in our needing 16 mmu indexes. Fortunately we

don't actually need to do this because a mismatch between

address attributes and CPU state means either:

* some kind of fault (usually a SecureFault, but in theory

perhaps a UserFault for unaligned access to Device memory)

* execution of the SG instruction in NS state from a

Secure & NonSecure code region

The purpose of SG is simply to flip the CPU into Secure

state, so we can handle it by emulating execution of that

instruction directly in arm_v7m_cpu_do_interrupt(), which

means we can treat all the mismatch cases as "throw an

exception" and we don't need to encode the state of the

other MPU bank into our mmu_idx values.

This commit doesn't include the actual emulation of SG;

it also doesn't include implementation of the IDAU, which

is a per-board way to specify hard-coded memory attributes

for addresses, which override the CPU-internal SAU if they

specify a more secure setting than the SAU is programmed to.

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

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

Message-id: 1506092407-26985-15-git-send-email-peter.maydell@linaro.org

---

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

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

2 files changed, 195 insertions(+), 2 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 @@ FIELD(V7M_EXCRET, DCRS, 5, 1)

FIELD(V7M_EXCRET, S, 6, 1)

FIELD(V7M_EXCRET, RES1, 7, 25) /* including the must-be-1 prefix */

+/* We use a few fake FSR values for internal purposes in M profile.

+ * M profile cores don't have A/R format FSRs, but currently our

+ * get_phys_addr() code assumes A/R profile and reports failures via

+ * an A/R format FSR value. We then translate that into the proper

+ * M profile exception and FSR status bit in arm_v7m_cpu_do_interrupt().

+ * Mostly the FSR values we use for this are those defined for v7PMSA,

+ * since we share some of that codepath. A few kinds of fault are

+ * only for M profile and have no A/R equivalent, though, so we have

+ * to pick a value from the reserved range (which we never otherwise

+ * generate) to use for these.

+ * These values will never be visible to the guest.

+ */

+#define M_FAKE_FSR_NSC_EXEC 0xf /* NS executing in S&NSC memory */

+#define M_FAKE_FSR_SFAULT 0xe /* SecureFault INVTRAN, INVEP or AUVIOL */

/*

* For AArch64, map a given EL to an index in the banked_spsr array.

* Note that this mapping and the AArch32 mapping defined in bank_number()

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

index XXXXXXX..XXXXXXX 100644

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

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

@@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,

target_ulong *page_size_ptr, uint32_t *fsr,

ARMMMUFaultInfo *fi);

+/* Security attributes for an address, as returned by v8m_security_lookup. */

+typedef struct V8M_SAttributes {

+ bool ns;

+ bool nsc;

+ uint8_t sregion;

+ bool srvalid;

+ uint8_t iregion;

+ bool irvalid;

+} V8M_SAttributes;

+

/* Definitions for the PMCCNTR and PMCR registers */

#define PMCRD 0x8

#define PMCRC 0x4

@@ -XXX,XX +XXX,XX @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)

* raises the fault, in the A profile short-descriptor format.

*/

switch (env->exception.fsr & 0xf) {

+ case M_FAKE_FSR_NSC_EXEC:

+ /* Exception generated when we try to execute code at an address

+ * which is marked as Secure & Non-Secure Callable and the CPU

+ * is in the Non-Secure state. The only instruction which can

+ * be executed like this is SG (and that only if both halves of

+ * the SG instruction have the same security attributes.)

+ * Everything else must generate an INVEP SecureFault, so we

+ * emulate the SG instruction here.

+ * TODO: actually emulate SG.

+ */

+ env->v7m.sfsr |= R_V7M_SFSR_INVEP_MASK;

+ armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SECURE, false);

+ qemu_log_mask(CPU_LOG_INT,

+ "...really SecureFault with SFSR.INVEP\n");

+ break;

+ case M_FAKE_FSR_SFAULT:

+ /* Various flavours of SecureFault for attempts to execute or

+ * access data in the wrong security state.

+ */

+ switch (cs->exception_index) {

+ case EXCP_PREFETCH_ABORT:

+ if (env->v7m.secure) {

+ env->v7m.sfsr |= R_V7M_SFSR_INVTRAN_MASK;

+ qemu_log_mask(CPU_LOG_INT,

+ "...really SecureFault with SFSR.INVTRAN\n");

+ } else {

+ env->v7m.sfsr |= R_V7M_SFSR_INVEP_MASK;

+ qemu_log_mask(CPU_LOG_INT,

+ "...really SecureFault with SFSR.INVEP\n");

+ }

+ break;

+ case EXCP_DATA_ABORT:

+ /* This must be an NS access to S memory */

+ env->v7m.sfsr |= R_V7M_SFSR_AUVIOL_MASK;

+ qemu_log_mask(CPU_LOG_INT,

+ "...really SecureFault with SFSR.AUVIOL\n");

+ break;

+ }

+ armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SECURE, false);

+ break;

case 0x8: /* External Abort */

switch (cs->exception_index) {

case EXCP_PREFETCH_ABORT:

@@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,

return !(*prot & (1 << access_type));

}

+static bool v8m_is_sau_exempt(CPUARMState *env,

+ uint32_t address, MMUAccessType access_type)

+{

+ /* The architecture specifies that certain address ranges are

+ * exempt from v8M SAU/IDAU checks.

+ */

+ return

+ (access_type == MMU_INST_FETCH && m_is_system_region(env, address)) ||

+ (address >= 0xe0000000 && address <= 0xe0002fff) ||

+ (address >= 0xe000e000 && address <= 0xe000efff) ||

+ (address >= 0xe002e000 && address <= 0xe002efff) ||

+ (address >= 0xe0040000 && address <= 0xe0041fff) ||

+ (address >= 0xe00ff000 && address <= 0xe00fffff);

+static void v8m_security_lookup(CPUARMState *env, uint32_t address,

+ MMUAccessType access_type, ARMMMUIdx mmu_idx,

+ V8M_SAttributes *sattrs)

+ /* Look up the security attributes for this address. Compare the

+ * pseudocode SecurityCheck() function.

+ * We assume the caller has zero-initialized *sattrs.

+ */

+ ARMCPU *cpu = arm_env_get_cpu(env);

+ int r;

+ /* TODO: implement IDAU */

+ if (access_type == MMU_INST_FETCH && extract32(address, 28, 4) == 0xf) {

+ /* 0xf0000000..0xffffffff is always S for insn fetches */

+ return;

+ if (v8m_is_sau_exempt(env, address, access_type)) {

+ sattrs->ns = !regime_is_secure(env, mmu_idx);

+ return;

+ switch (env->sau.ctrl & 3) {

+ case 0: /* SAU.ENABLE == 0, SAU.ALLNS == 0 */

+ break;

+ case 2: /* SAU.ENABLE == 0, SAU.ALLNS == 1 */

+ sattrs->ns = true;

+ break;

+ default: /* SAU.ENABLE == 1 */

+ for (r = 0; r < cpu->sau_sregion; r++) {

+ if (env->sau.rlar[r] & 1) {

+ uint32_t base = env->sau.rbar[r] & ~0x1f;

+ uint32_t limit = env->sau.rlar[r] | 0x1f;

+

+ if (base <= address && limit >= address) {

+ if (sattrs->srvalid) {

+ /* If we hit in more than one region then we must report

+ * as Secure, not NS-Callable, with no valid region

+ * number info.

+ */

+ sattrs->ns = false;

+ sattrs->nsc = false;

+ sattrs->sregion = 0;

+ sattrs->srvalid = false;

+ break;

+ } else {

+ if (env->sau.rlar[r] & 2) {

+ sattrs->nsc = true;

+ } else {

+ sattrs->ns = true;

+ }

+ sattrs->srvalid = true;

+ sattrs->sregion = r;

+ }

+ }

+ }

+ }

+

+ /* TODO when we support the IDAU then it may override the result here */

+ break;

+ }

+}

+

static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address,

MMUAccessType access_type, ARMMMUIdx mmu_idx,

- hwaddr *phys_ptr, int *prot, uint32_t *fsr)

+ hwaddr *phys_ptr, MemTxAttrs *txattrs,

+ int *prot, uint32_t *fsr)

{

ARMCPU *cpu = arm_env_get_cpu(env);

bool is_user = regime_is_user(env, mmu_idx);

@@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav8(CPUARMState *env, uint32_t address,

int n;

int matchregion = -1;

bool hit = false;

+ V8M_SAttributes sattrs = {};

*phys_ptr = address;

*prot = 0;

+ if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {

+ v8m_security_lookup(env, address, access_type, mmu_idx, &sattrs);

+ if (access_type == MMU_INST_FETCH) {

+ /* Instruction fetches always use the MMU bank and the

+ * transaction attribute determined by the fetch address,

+ * regardless of CPU state. This is painful for QEMU

+ * to handle, because it would mean we need to encode

+ * into the mmu_idx not just the (user, negpri) information

+ * for the current security state but also that for the

+ * other security state, which would balloon the number

+ * of mmu_idx values needed alarmingly.

+ * Fortunately we can avoid this because it's not actually

+ * possible to arbitrarily execute code from memory with

+ * the wrong security attribute: it will always generate

+ * an exception of some kind or another, apart from the

+ * special case of an NS CPU executing an SG instruction

+ * in S&NSC memory. So we always just fail the translation

+ * here and sort things out in the exception handler

+ * (including possibly emulating an SG instruction).

+ */

+ if (sattrs.ns != !secure) {

+ *fsr = sattrs.nsc ? M_FAKE_FSR_NSC_EXEC : M_FAKE_FSR_SFAULT;

+ return true;

+ }

+ } else {

+ /* For data accesses we always use the MMU bank indicated

+ * by the current CPU state, but the security attributes

+ * might downgrade a secure access to nonsecure.

+ */

+ if (sattrs.ns) {

+ txattrs->secure = false;

+ } else if (!secure) {

+ /* NS access to S memory must fault.

+ * Architecturally we should first check whether the

+ * MPU information for this address indicates that we

+ * are doing an unaligned access to Device memory, which

+ * should generate a UsageFault instead. QEMU does not

+ * currently check for that kind of unaligned access though.

+ * If we added it we would need to do so as a special case

+ * for M_FAKE_FSR_SFAULT in arm_v7m_cpu_do_interrupt().

+ */

+ *fsr = M_FAKE_FSR_SFAULT;

+ return true;

+ }

+ }

+ }

+

/* Unlike the ARM ARM pseudocode, we don't need to check whether this

* was an exception vector read from the vector table (which is always

* done using the default system address map), because those accesses

@@ -XXX,XX +XXX,XX @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,

if (arm_feature(env, ARM_FEATURE_V8)) {

/* PMSAv8 */

ret = get_phys_addr_pmsav8(env, address, access_type, mmu_idx,

- phys_ptr, prot, fsr);

+ phys_ptr, attrs, prot, fsr);

} else if (arm_feature(env, ARM_FEATURE_V7)) {

/* PMSAv7 */

ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx,

2.7.4

Add support for v8M and in particular the security extension

to the exception entry code. This requires changes to:

* calculation of the exception-return magic LR value

* push the callee-saves registers in certain cases

* clear registers when taking non-secure exceptions to avoid

leaking information from the interrupted secure code

* switch to the correct security state on entry

* use the vector table for the security state we're targeting

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

1 file changed, 145 insertions(+), 20 deletions(-)

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 uint32_t *get_v7m_sp_ptr(CPUARMState *env, bool secure, bool threadmode,

}

-static uint32_t arm_v7m_load_vector(ARMCPU *cpu)

+static uint32_t arm_v7m_load_vector(ARMCPU *cpu, bool targets_secure)

{

CPUState *cs = CPU(cpu);

CPUARMState *env = &cpu->env;

MemTxResult result;

- hwaddr vec = env->v7m.vecbase[env->v7m.secure] + env->v7m.exception * 4;

+ hwaddr vec = env->v7m.vecbase[targets_secure] + env->v7m.exception * 4;

uint32_t addr;

addr = address_space_ldl(cs->as, vec,

@@ -XXX,XX +XXX,XX @@ static uint32_t arm_v7m_load_vector(ARMCPU *cpu)

* Since we don't model Lockup, we just report this guest error

* via cpu_abort().

*/

- cpu_abort(cs, "Failed to read from exception vector table "

- "entry %08x\n", (unsigned)vec);

+ cpu_abort(cs, "Failed to read from %s exception vector table "

+ "entry %08x\n", targets_secure ? "secure" : "nonsecure",

+ (unsigned)vec);

}

return addr;

}

-static void v7m_exception_taken(ARMCPU *cpu, uint32_t lr)

+static void v7m_push_callee_stack(ARMCPU *cpu, uint32_t lr, bool dotailchain)

+ /* For v8M, push the callee-saves register part of the stack frame.

+ * Compare the v8M pseudocode PushCalleeStack().

+ * In the tailchaining case this may not be the current stack.

+ */

+ CPUARMState *env = &cpu->env;

+ CPUState *cs = CPU(cpu);

+ uint32_t *frame_sp_p;

+ uint32_t frameptr;

+ if (dotailchain) {

+ frame_sp_p = get_v7m_sp_ptr(env, true,

+ lr & R_V7M_EXCRET_MODE_MASK,

+ lr & R_V7M_EXCRET_SPSEL_MASK);

+ } else {

+ frame_sp_p = &env->regs[13];

+ }

+

+ frameptr = *frame_sp_p - 0x28;

+

+ stl_phys(cs->as, frameptr, 0xfefa125b);

+ stl_phys(cs->as, frameptr + 0x8, env->regs[4]);

+ stl_phys(cs->as, frameptr + 0xc, env->regs[5]);

+ stl_phys(cs->as, frameptr + 0x10, env->regs[6]);

+ stl_phys(cs->as, frameptr + 0x14, env->regs[7]);

+ stl_phys(cs->as, frameptr + 0x18, env->regs[8]);

+ stl_phys(cs->as, frameptr + 0x1c, env->regs[9]);

+ stl_phys(cs->as, frameptr + 0x20, env->regs[10]);

+ stl_phys(cs->as, frameptr + 0x24, env->regs[11]);

+

+ *frame_sp_p = frameptr;

+static void v7m_exception_taken(ARMCPU *cpu, uint32_t lr, bool dotailchain)

/* Do the "take the exception" parts of exception entry,

* but not the pushing of state to the stack. This is

@@ -XXX,XX +XXX,XX @@ static void v7m_exception_taken(ARMCPU *cpu, uint32_t lr)

CPUARMState *env = &cpu->env;

uint32_t addr;

+ bool targets_secure;

+ targets_secure = armv7m_nvic_acknowledge_irq(env->nvic);

- armv7m_nvic_acknowledge_irq(env->nvic);

+ if (arm_feature(env, ARM_FEATURE_V8)) {

+ if (arm_feature(env, ARM_FEATURE_M_SECURITY) &&

+ (lr & R_V7M_EXCRET_S_MASK)) {

+ /* The background code (the owner of the registers in the

+ * exception frame) is Secure. This means it may either already

+ * have or now needs to push callee-saves registers.

+ */

+ if (targets_secure) {

+ if (dotailchain && !(lr & R_V7M_EXCRET_ES_MASK)) {

+ /* We took an exception from Secure to NonSecure

+ * (which means the callee-saved registers got stacked)

+ * and are now tailchaining to a Secure exception.

+ * Clear DCRS so eventual return from this Secure

+ * exception unstacks the callee-saved registers.

+ */

+ lr &= ~R_V7M_EXCRET_DCRS_MASK;

+ }

+ } else {

+ /* We're going to a non-secure exception; push the

+ * callee-saves registers to the stack now, if they're

+ * not already saved.

+ */

+ if (lr & R_V7M_EXCRET_DCRS_MASK &&

+ !(dotailchain && (lr & R_V7M_EXCRET_ES_MASK))) {

+ v7m_push_callee_stack(cpu, lr, dotailchain);

+ }

+ lr |= R_V7M_EXCRET_DCRS_MASK;

+ }

+ }

+

+ lr &= ~R_V7M_EXCRET_ES_MASK;

+ if (targets_secure || !arm_feature(env, ARM_FEATURE_M_SECURITY)) {

+ lr |= R_V7M_EXCRET_ES_MASK;

+ }

+ lr &= ~R_V7M_EXCRET_SPSEL_MASK;

+ if (env->v7m.control[targets_secure] & R_V7M_CONTROL_SPSEL_MASK) {

+ lr |= R_V7M_EXCRET_SPSEL_MASK;

+ }

+

+ /* Clear registers if necessary to prevent non-secure exception

+ * code being able to see register values from secure code.

+ * Where register values become architecturally UNKNOWN we leave

+ * them with their previous values.

+ */

+ if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {

+ if (!targets_secure) {

+ /* Always clear the caller-saved registers (they have been

+ * pushed to the stack earlier in v7m_push_stack()).

+ * Clear callee-saved registers if the background code is

+ * Secure (in which case these regs were saved in

+ * v7m_push_callee_stack()).

+ */

+ int i;

+

+ for (i = 0; i < 13; i++) {

+ /* r4..r11 are callee-saves, zero only if EXCRET.S == 1 */

+ if (i < 4 || i > 11 || (lr & R_V7M_EXCRET_S_MASK)) {

+ env->regs[i] = 0;

+ }

+ }

+ /* Clear EAPSR */

+ xpsr_write(env, 0, XPSR_NZCV | XPSR_Q | XPSR_GE | XPSR_IT);

+ }

+ }

+ }

+

+ /* Switch to target security state -- must do this before writing SPSEL */

+ switch_v7m_security_state(env, targets_secure);

write_v7m_control_spsel(env, 0);

arm_clear_exclusive(env);

/* Clear IT bits */

env->condexec_bits = 0;

env->regs[14] = lr;

- addr = arm_v7m_load_vector(cpu);

+ addr = arm_v7m_load_vector(cpu, targets_secure);

env->regs[15] = addr & 0xfffffffe;

env->thumb = addr & 1;

}

@@ -XXX,XX +XXX,XX @@ static void do_v7m_exception_exit(ARMCPU *cpu)

if (sfault) {

env->v7m.sfsr |= R_V7M_SFSR_INVER_MASK;

armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SECURE, false);

- v7m_exception_taken(cpu, excret);

+ v7m_exception_taken(cpu, excret, true);

qemu_log_mask(CPU_LOG_INT, "...taking SecureFault on existing "

"stackframe: failed EXC_RETURN.ES validity check\n");

return;

@@ -XXX,XX +XXX,XX @@ static void do_v7m_exception_exit(ARMCPU *cpu)

*/

env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_INVPC_MASK;

armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE, env->v7m.secure);

- v7m_exception_taken(cpu, excret);

+ v7m_exception_taken(cpu, excret, true);

qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on existing "

"stackframe: failed exception return integrity check\n");

return;

@@ -XXX,XX +XXX,XX @@ static void do_v7m_exception_exit(ARMCPU *cpu)

/* Take a SecureFault on the current stack */

env->v7m.sfsr |= R_V7M_SFSR_INVIS_MASK;

armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SECURE, false);

- v7m_exception_taken(cpu, excret);

+ v7m_exception_taken(cpu, excret, true);

qemu_log_mask(CPU_LOG_INT, "...taking SecureFault on existing "

"stackframe: failed exception return integrity "

"signature check\n");

@@ -XXX,XX +XXX,XX @@ static void do_v7m_exception_exit(ARMCPU *cpu)

armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE,

env->v7m.secure);

env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_INVPC_MASK;

- v7m_exception_taken(cpu, excret);

+ v7m_exception_taken(cpu, excret, true);

qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on existing "

"stackframe: failed exception return integrity "

"check\n");

@@ -XXX,XX +XXX,XX @@ static void do_v7m_exception_exit(ARMCPU *cpu)

armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE, false);

env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_INVPC_MASK;

v7m_push_stack(cpu);

- v7m_exception_taken(cpu, excret);

+ v7m_exception_taken(cpu, excret, false);

qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on new stackframe: "

"failed exception return integrity check\n");

return;

@@ -XXX,XX +XXX,XX @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)

return; /* Never happens. Keep compiler happy. */

2.7.4

For the SG instruction and secure function return we are going

to want to do memory accesses using the MMU index of the CPU

in secure state, even though the CPU is currently in non-secure

state. Write arm_v7m_mmu_idx_for_secstate() to do this job,

and use it in cpu_mmu_index().

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

1 file changed, 21 insertions(+), 11 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 int arm_mmu_idx_to_el(ARMMMUIdx mmu_idx)

}

}

+/* Return the MMU index for a v7M CPU in the specified security state */

+static inline ARMMMUIdx arm_v7m_mmu_idx_for_secstate(CPUARMState *env,

+ bool secstate)

+{

+ int el = arm_current_el(env);

+ ARMMMUIdx mmu_idx;

+ if (el == 0) {

+ mmu_idx = secstate ? ARMMMUIdx_MSUser : ARMMMUIdx_MUser;

+ } else {

+ mmu_idx = secstate ? ARMMMUIdx_MSPriv : ARMMMUIdx_MPriv;

+ if (armv7m_nvic_neg_prio_requested(env->nvic, secstate)) {

+ mmu_idx = secstate ? ARMMMUIdx_MSNegPri : ARMMMUIdx_MNegPri;

+ }

+

+ return mmu_idx;

+}

+

/* Determine the current mmu_idx to use for normal loads/stores */

static inline int cpu_mmu_index(CPUARMState *env, bool ifetch)

{

int el = arm_current_el(env);

if (arm_feature(env, ARM_FEATURE_M)) {

- ARMMMUIdx mmu_idx;

-

- if (el == 0) {

- mmu_idx = env->v7m.secure ? ARMMMUIdx_MSUser : ARMMMUIdx_MUser;

- } else {

- mmu_idx = env->v7m.secure ? ARMMMUIdx_MSPriv : ARMMMUIdx_MPriv;

- }

-

- if (armv7m_nvic_neg_prio_requested(env->nvic, env->v7m.secure)) {

- mmu_idx = env->v7m.secure ? ARMMMUIdx_MSNegPri : ARMMMUIdx_MNegPri;

- }

+ ARMMMUIdx mmu_idx = arm_v7m_mmu_idx_for_secstate(env, env->v7m.secure);

return arm_to_core_mmu_idx(mmu_idx);

2.7.4

The device uses serial_hds in its realize function and thus can't be

used twice. Apart from that, the comma in its name makes it quite hard

to use for the user anyway, since a comma is normally used to separate

the device name from its properties when using the "-device" parameter

or the "device_add" HMP command.

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Message-id: 1506441116-16627-1-git-send-email-thuth@redhat.com

hw/arm/xlnx-zynqmp.c | 2 ++

1 file changed, 2 insertions(+)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c

--- a/hw/arm/xlnx-zynqmp.c

+++ b/hw/arm/xlnx-zynqmp.c

@@ -XXX,XX +XXX,XX @@ static void xlnx_zynqmp_class_init(ObjectClass *oc, void *data)

dc->props = xlnx_zynqmp_props;

dc->realize = xlnx_zynqmp_realize;

+ /* Reason: Uses serial_hds in realize function, thus can't be used twice */

+ dc->user_creatable = false;

}

static const TypeInfo xlnx_zynqmp_type_info = {

2.7.4

Currently our M profile exception return code switches to the

target stack pointer relatively early in the process, before

it tries to pop the exception frame off the stack. This is

awkward for v8M for two reasons:

* in v8M the process vs main stack pointer is not selected

purely by the value of CONTROL.SPSEL, so updating SPSEL

and relying on that to switch to the right stack pointer

won't work

* the stack we should be reading the stack frame from and

the stack we will eventually switch to might not be the

same if the guest is doing strange things

Change our exception return code to use a 'frame pointer'

to read the exception frame rather than assuming that we

can switch the live stack pointer this early.

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

1 file changed, 98 insertions(+), 32 deletions(-)

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 v7m_push(CPUARMState *env, uint32_t val)

stl_phys(cs->as, env->regs[13], val);

-static uint32_t v7m_pop(CPUARMState *env)

-{

- CPUState *cs = CPU(arm_env_get_cpu(env));

- uint32_t val;

-

- val = ldl_phys(cs->as, env->regs[13]);

- env->regs[13] += 4;

- return val;

-}

-

/* Return true if we're using the process stack pointer (not the MSP) */

static bool v7m_using_psp(CPUARMState *env)

{

@@ -XXX,XX +XXX,XX @@ void HELPER(v7m_bxns)(CPUARMState *env, uint32_t dest)

env->regs[15] = dest & ~1;

}

+static uint32_t *get_v7m_sp_ptr(CPUARMState *env, bool secure, bool threadmode,

+ bool spsel)

+{

+ /* Return a pointer to the location where we currently store the

+ * stack pointer for the requested security state and thread mode.

+ * This pointer will become invalid if the CPU state is updated

+ * such that the stack pointers are switched around (eg changing

+ * the SPSEL control bit).

+ * Compare the v8M ARM ARM pseudocode LookUpSP_with_security_mode().

+ * Unlike that pseudocode, we require the caller to pass us in the

+ * SPSEL control bit value; this is because we also use this

+ * function in handling of pushing of the callee-saves registers

+ * part of the v8M stack frame (pseudocode PushCalleeStack()),

+ * and in the tailchain codepath the SPSEL bit comes from the exception

+ * return magic LR value from the previous exception. The pseudocode

+ * opencodes the stack-selection in PushCalleeStack(), but we prefer

+ * to make this utility function generic enough to do the job.

+ */

+ bool want_psp = threadmode && spsel;

+

+ if (secure == env->v7m.secure) {

+ /* Currently switch_v7m_sp switches SP as it updates SPSEL,

+ * so the SP we want is always in regs[13].

+ * When we decouple SPSEL from the actually selected SP

+ * we need to check want_psp against v7m_using_psp()

+ * to see whether we need regs[13] or v7m.other_sp.

+ */

+ return &env->regs[13];

+ } else {

+ if (want_psp) {

+ return &env->v7m.other_ss_psp;

+ } else {

+ return &env->v7m.other_ss_msp;

+ }

+ }

+}

+

static uint32_t arm_v7m_load_vector(ARMCPU *cpu)

{

CPUState *cs = CPU(cpu);

@@ -XXX,XX +XXX,XX @@ static void v7m_push_stack(ARMCPU *cpu)

static void do_v7m_exception_exit(ARMCPU *cpu)

{

CPUARMState *env = &cpu->env;

+ CPUState *cs = CPU(cpu);

uint32_t excret;

uint32_t xpsr;

bool ufault = false;

@@ -XXX,XX +XXX,XX @@ static void do_v7m_exception_exit(ARMCPU *cpu)

bool return_to_handler = false;

bool rettobase = false;

bool exc_secure = false;

+ bool return_to_secure;

/* We can only get here from an EXCP_EXCEPTION_EXIT, and

* gen_bx_excret() enforces the architectural rule

@@ -XXX,XX +XXX,XX @@ static void do_v7m_exception_exit(ARMCPU *cpu)

g_assert_not_reached();

}

+ return_to_secure = arm_feature(env, ARM_FEATURE_M_SECURITY) &&

+ (excret & R_V7M_EXCRET_S_MASK);

+

switch (excret & 0xf) {

case 1: /* Return to Handler */

return_to_handler = true;

@@ -XXX,XX +XXX,XX @@ static void do_v7m_exception_exit(ARMCPU *cpu)

return;

}

- /* Switch to the target stack. */

+ /* Set CONTROL.SPSEL from excret.SPSEL. For QEMU this currently

+ * causes us to switch the active SP, but we will change this

+ * later to not do that so we can support v8M.

+ */

switch_v7m_sp(env, return_to_sp_process);

- /* Pop registers. */

- env->regs[0] = v7m_pop(env);

- env->regs[1] = v7m_pop(env);

- env->regs[2] = v7m_pop(env);

- env->regs[3] = v7m_pop(env);

- env->regs[12] = v7m_pop(env);

- env->regs[14] = v7m_pop(env);

- env->regs[15] = v7m_pop(env);

- if (env->regs[15] & 1) {

- qemu_log_mask(LOG_GUEST_ERROR,

- "M profile return from interrupt with misaligned "

- "PC is UNPREDICTABLE\n");

- /* Actual hardware seems to ignore the lsbit, and there are several

- * RTOSes out there which incorrectly assume the r15 in the stack

- * frame should be a Thumb-style "lsbit indicates ARM/Thumb" value.

+

+ {

+ /* The stack pointer we should be reading the exception frame from

+ * depends on bits in the magic exception return type value (and

+ * for v8M isn't necessarily the stack pointer we will eventually

+ * end up resuming execution with). Get a pointer to the location

+ * in the CPU state struct where the SP we need is currently being

+ * stored; we will use and modify it in place.

+ * We use this limited C variable scope so we don't accidentally

+ * use 'frame_sp_p' after we do something that makes it invalid.

+ */

+ uint32_t *frame_sp_p = get_v7m_sp_ptr(env,

+ return_to_secure,

+ !return_to_handler,

+ return_to_sp_process);

+ uint32_t frameptr = *frame_sp_p;

+

+ /* Pop registers. TODO: make these accesses use the correct

+ * attributes and address space (S/NS, priv/unpriv) and handle

+ * memory transaction failures.

*/

- env->regs[15] &= ~1U;

+ env->regs[0] = ldl_phys(cs->as, frameptr);

+ env->regs[1] = ldl_phys(cs->as, frameptr + 0x4);

+ env->regs[2] = ldl_phys(cs->as, frameptr + 0x8);

+ env->regs[3] = ldl_phys(cs->as, frameptr + 0xc);

+ env->regs[12] = ldl_phys(cs->as, frameptr + 0x10);

+ env->regs[14] = ldl_phys(cs->as, frameptr + 0x14);

+ env->regs[15] = ldl_phys(cs->as, frameptr + 0x18);

+ if (env->regs[15] & 1) {

+ qemu_log_mask(LOG_GUEST_ERROR,

+ "M profile return from interrupt with misaligned "

+ "PC is UNPREDICTABLE\n");

+ /* Actual hardware seems to ignore the lsbit, and there are several

+ * RTOSes out there which incorrectly assume the r15 in the stack

+ * frame should be a Thumb-style "lsbit indicates ARM/Thumb" value.

+ */

+ env->regs[15] &= ~1U;

+ }

+ xpsr = ldl_phys(cs->as, frameptr + 0x1c);

+

+ /* Commit to consuming the stack frame */

+ frameptr += 0x20;

+ /* Undo stack alignment (the SPREALIGN bit indicates that the original

+ * pre-exception SP was not 8-aligned and we added a padding word to

+ * align it, so we undo this by ORing in the bit that increases it

+ * from the current 8-aligned value to the 8-unaligned value. (Adding 4

+ * would work too but a logical OR is how the pseudocode specifies it.)

+ */

+ if (xpsr & XPSR_SPREALIGN) {

+ frameptr |= 4;

+ }

+ *frame_sp_p = frameptr;

}

- xpsr = v7m_pop(env);

+ /* This xpsr_write() will invalidate frame_sp_p as it may switch stack */

xpsr_write(env, xpsr, ~XPSR_SPREALIGN);

- /* Undo stack alignment. */

- if (xpsr & XPSR_SPREALIGN) {

- env->regs[13] |= 4;

- }

/* The restored xPSR exception field will be zero if we're

* resuming in Thread mode. If that doesn't match what the

2.7.4

From: Jan Kiszka <jan.kiszka@siemens.com>

This properly forwards SMC events to EL2 when PSCI is provided by QEMU

itself and, thus, ARM_FEATURE_EL3 is off.

Found and tested with the Jailhouse hypervisor. Solution based on