target-arm queue:

* mostly my latest v8M stuff, plus a couple of minor patches

The following changes since commit a0b261db8c030813e30a39eae47359ac2a37f7e2:

Merge remote-tracking branch 'remotes/ehabkost/tags/python-next-pull-request' into staging (2017-10-12 10:02:09 +0100)

are available in the git repository at:

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

for you to fetch changes up to cf5f7937b05c84d5565134f058c00cd48304a117:

nvic: Fix miscalculation of offsets into ITNS array (2017-10-12 16:33:16 +0100)

* v8M: SG, BLXNS, secure-return

* v8M: fixes for coverity issues in previous patches

* arm: fix armv7m_init() declaration to match definition

* watchdog/aspeed: fix variable type to store reload value

Cédric Le Goater (1):

watchdog/aspeed: fix variable type to store reload value

Igor Mammedov (1):

arm: fix armv7m_init() declaration to match definition

Peter Maydell (11):

target/arm: Add M profile secure MMU index values to get_a32_user_mem_index()

target/arm: Implement SG instruction

target/arm: Implement BLXNS

target/arm: Implement secure function return

target-arm: Don't check for "Thumb2 or M profile" for not-Thumb1

target/arm: Pull Thumb insn word loads up to top level

target-arm: Simplify insn_crosses_page()

target/arm: Support some Thumb insns being always unconditional

target/arm: Implement SG instruction corner cases

nvic: Add missing 'break'

nvic: Fix miscalculation of offsets into ITNS array

include/hw/arm/arm.h | 2 +-

target/arm/helper.h | 1 +

target/arm/internals.h | 8 ++

hw/intc/armv7m_nvic.c | 5 +-

hw/watchdog/wdt_aspeed.c | 4 +-

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

target/arm/translate.c | 310 ++++++++++++++++++++++++++++++++---------------

7 files changed, 521 insertions(+), 115 deletions(-)

Coverity points out that we forgot the 'break' for

the SAU_CTRL write case (CID1381683). This has

no actual visible consequences because it happens

that the following case is effectively a no-op.

hw/intc/armv7m_nvic.c | 1 +

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c

--- a/hw/intc/armv7m_nvic.c

+++ b/hw/intc/armv7m_nvic.c

@@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value,

return;

}

cpu->env.sau.ctrl = value & 3;

+ break;

case 0xdd4: /* SAU_TYPE */

if (!arm_feature(&cpu->env, ARM_FEATURE_V8)) {

goto bad_offset;

2.7.4

This calculation of the first exception vector in

the ITNS<n> register being accessed:

int startvec = 32 * (offset - 0x380) + NVIC_FIRST_IRQ;

is incorrect, because offset is in bytes, so we only want

to multiply by 8.

Spotted by Coverity (CID 1381484, CID 1381488), though it is

not correct that it actually overflows the buffer, because

we have a 'startvec + i < s->num_irq' guard.

Message-id: 1507650856-11718-1-git-send-email-peter.maydell@linaro.org

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

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

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c

--- a/hw/intc/armv7m_nvic.c

+++ b/hw/intc/armv7m_nvic.c

@@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs)

return ((s->num_irq - NVIC_FIRST_IRQ) / 32) - 1;

case 0x380 ... 0x3bf: /* NVIC_ITNS<n> */

{

- int startvec = 32 * (offset - 0x380) + NVIC_FIRST_IRQ;

+ int startvec = 8 * (offset - 0x380) + NVIC_FIRST_IRQ;

int i;

if (!arm_feature(&cpu->env, ARM_FEATURE_V8)) {

@@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value,

switch (offset) {

case 0x380 ... 0x3bf: /* NVIC_ITNS<n> */

{

- int startvec = 32 * (offset - 0x380) + NVIC_FIRST_IRQ;

+ int startvec = 8 * (offset - 0x380) + NVIC_FIRST_IRQ;

int i;

if (!arm_feature(&cpu->env, ARM_FEATURE_V8)) {

2.7.4

The common situation of the SG instruction is that it is

executed from S&NSC memory by a CPU in NS state. That case

is handled by v7m_handle_execute_nsc(). However the instruction

also has defined behaviour in a couple of other cases:

* SG instruction in NS memory (behaves as a NOP)

* SG in S memory but CPU already secure (clears IT bits and

does nothing else)

* SG instruction in v8M without Security Extension (NOP)

These can be implemented in translate.c.

Message-id: 1507556919-24992-10-git-send-email-peter.maydell@linaro.org

target/arm/translate.c | 23 ++++++++++++++++++++++-

1 file changed, 22 insertions(+), 1 deletion(-)

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

--- a/target/arm/translate.c

+++ b/target/arm/translate.c

@@ -XXX,XX +XXX,XX @@ static int disas_thumb2_insn(DisasContext *s, uint32_t insn)

* - load/store doubleword, load/store exclusive, ldacq/strel,

* table branch.

*/

- if (insn & 0x01200000) {

+ if (insn == 0xe97fe97f && arm_dc_feature(s, ARM_FEATURE_M) &&

+ arm_dc_feature(s, ARM_FEATURE_V8)) {

+ /* 0b1110_1001_0111_1111_1110_1001_0111_111

+ * - SG (v8M only)

+ * The bulk of the behaviour for this instruction is implemented

+ * in v7m_handle_execute_nsc(), which deals with the insn when

+ * it is executed by a CPU in non-secure state from memory

+ * which is Secure & NonSecure-Callable.

+ * Here we only need to handle the remaining cases:

+ * * in NS memory (including the "security extension not

+ * implemented" case) : NOP

+ * * in S memory but CPU already secure (clear IT bits)

+ * We know that the attribute for the memory this insn is

+ * in must match the current CPU state, because otherwise

+ * get_phys_addr_pmsav8 would have generated an exception.

+ */

+ if (s->v8m_secure) {

+ /* Like the IT insn, we don't need to generate any code */

+ s->condexec_cond = 0;

+ s->condexec_mask = 0;

+ }

+ } else if (insn & 0x01200000) {

/* 0b1110_1000_x11x_xxxx_xxxx_xxxx_xxxx_xxxx

* - load/store dual (post-indexed)

* 0b1111_1001_x10x_xxxx_xxxx_xxxx_xxxx_xxxx

2.7.4

Implement the SG instruction, which we emulate 'by hand' in the

exception handling code path.

Message-id: 1507556919-24992-3-git-send-email-peter.maydell@linaro.org

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

1 file changed, 127 insertions(+), 5 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 @@ typedef struct V8M_SAttributes {

bool irvalid;

} V8M_SAttributes;

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

+ MMUAccessType access_type, ARMMMUIdx mmu_idx,

+ V8M_SAttributes *sattrs);

+

/* Definitions for the PMCCNTR and PMCR registers */

#define PMCRD 0x8

#define PMCRC 0x4

@@ -XXX,XX +XXX,XX @@ static void arm_log_exception(int idx)

+static bool v7m_read_half_insn(ARMCPU *cpu, ARMMMUIdx mmu_idx,

+ uint32_t addr, uint16_t *insn)

+ /* Load a 16-bit portion of a v7M instruction, returning true on success,

+ * or false on failure (in which case we will have pended the appropriate

+ * exception).

+ * We need to do the instruction fetch's MPU and SAU checks

+ * like this because there is no MMU index that would allow

+ * doing the load with a single function call. Instead we must

+ * first check that the security attributes permit the load

+ * and that they don't mismatch on the two halves of the instruction,

+ * and then we do the load as a secure load (ie using the security

+ * attributes of the address, not the CPU, as architecturally required).

+ CPUState *cs = CPU(cpu);

+ CPUARMState *env = &cpu->env;

+ V8M_SAttributes sattrs = {};

+ MemTxAttrs attrs = {};

+ ARMMMUFaultInfo fi = {};

+ MemTxResult txres;

+ target_ulong page_size;

+ hwaddr physaddr;

+ int prot;

+ uint32_t fsr;

+

+ v8m_security_lookup(env, addr, MMU_INST_FETCH, mmu_idx, &sattrs);

+ if (!sattrs.nsc || sattrs.ns) {

+ /* This must be the second half of the insn, and it straddles a

+ * region boundary with the second half not being S&NSC.

+ 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");

+ return false;

+static bool v7m_handle_execute_nsc(ARMCPU *cpu)

+{

+ /* Check whether this attempt to execute code in a Secure & NS-Callable

+ * memory region is for an SG instruction; if so, then emulate the

+ * effect of the SG instruction and return true. Otherwise pend

+ * the correct kind of exception and return false.

+ */

+ CPUARMState *env = &cpu->env;

+ ARMMMUIdx mmu_idx;

+ uint16_t insn;

+

+ /* We should never get here unless get_phys_addr_pmsav8() caused

+ * an exception for NS executing in S&NSC memory.

+ */

+ assert(!env->v7m.secure);

+ assert(arm_feature(env, ARM_FEATURE_M_SECURITY));

+

+ /* We want to do the MPU lookup as secure; work out what mmu_idx that is */

+ mmu_idx = arm_v7m_mmu_idx_for_secstate(env, true);

+

+ if (!v7m_read_half_insn(cpu, mmu_idx, env->regs[15], &insn)) {

+ return false;

+ }

+

+ if (!env->thumb) {

+ goto gen_invep;

+ }

+

+ if (insn != 0xe97f) {

+ /* Not an SG instruction first half (we choose the IMPDEF

+ * early-SG-check option).

+ */

+ goto gen_invep;

+ }

+

+ if (!v7m_read_half_insn(cpu, mmu_idx, env->regs[15] + 2, &insn)) {

+ return false;

+ }

+

+ if (insn != 0xe97f) {

+ /* Not an SG instruction second half (yes, both halves of the SG

+ * insn have the same hex value)

+ */

+ goto gen_invep;

+ }

+

+ /* OK, we have confirmed that we really have an SG instruction.

+ * We know we're NS in S memory so don't need to repeat those checks.

+ */

+ qemu_log_mask(CPU_LOG_INT, "...really an SG instruction at 0x%08" PRIx32

+ ", executing it\n", env->regs[15]);

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

+ switch_v7m_security_state(env, true);

+ xpsr_write(env, 0, XPSR_IT);

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

+ return true;

+

+gen_invep:

+ 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");

+ return false;

+}

+

void arm_v7m_cpu_do_interrupt(CPUState *cs)

ARMCPU *cpu = ARM_CPU(cs);

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

* 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");

+ if (v7m_handle_execute_nsc(cpu)) {

+ return;

+ }

break;

case M_FAKE_FSR_SFAULT:

/* Various flavours of SecureFault for attempts to execute or

2.7.4

A few Thumb instructions are always unconditional even inside an

IT block (as opposed to being UNPREDICTABLE if used inside an

IT block): BKPT, the v8M SG instruction, and the A profile

HLT (debug halt) instruction.

This means we need to suppress the jump-over-instruction-on-condfail

code generation (though the IT state still advances as usual and

subsequent insns in the IT block may be conditional).

Message-id: 1507556919-24992-9-git-send-email-peter.maydell@linaro.org

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

1 file changed, 47 insertions(+), 1 deletion(-)

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

--- a/target/arm/translate.c

+++ b/target/arm/translate.c

@@ -XXX,XX +XXX,XX @@ static void arm_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)

in init_disas_context by adjusting max_insns. */

2.7.4

Recent changes have left insn_crosses_page() more complicated

than it needed to be:

* it's only called from thumb_tr_translate_insn() so we know

for certain that we're looking at a Thumb insn

* the caller's check for dc->pc >= dc->next_page_start - 3

means that dc->pc can't possibly be 4 aligned, so there's

no need to check that (the check was partly there to ensure

that we didn't treat an ARM insn as Thumb, I think)

* we now have thumb_insn_is_16bit() which lets us do a precise

check of the length of the next insn, rather than opencoding

an inaccurate check

Simplify it down to just loading the first half of the insn

and calling thumb_insn_is_16bit() on it.

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

Message-id: 1507556919-24992-8-git-send-email-peter.maydell@linaro.org

target/arm/translate.c | 27 ++++++---------------------

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

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

--- a/target/arm/translate.c

+++ b/target/arm/translate.c

@@ -XXX,XX +XXX,XX @@ static bool insn_crosses_page(CPUARMState *env, DisasContext *s)

{

/* Return true if the insn at dc->pc might cross a page boundary.

* (False positives are OK, false negatives are not.)

+ * We know this is a Thumb insn, and our caller ensures we are

+ * only called if dc->pc is less than 4 bytes from the page

+ * boundary, so we cross the page if the first 16 bits indicate

+ * that this is a 32 bit insn.

*/

- uint16_t insn;

+ uint16_t insn = arm_lduw_code(env, s->pc, s->sctlr_b);

- if ((s->pc & 3) == 0) {

- /* At a 4-aligned address we can't be crossing a page */

- return false;

- }

-

- /* This must be a Thumb insn */

- insn = arm_lduw_code(env, s->pc, s->sctlr_b);

-

- if ((insn >> 11) >= 0x1d) {

- /* Top five bits 0b11101 / 0b11110 / 0b11111 : this is the

- * First half of a 32-bit Thumb insn. Thumb-1 cores might

- * end up actually treating this as two 16-bit insns (see the

- * code at the start of disas_thumb2_insn()) but we don't bother

- * to check for that as it is unlikely, and false positives here

- * are harmless.

- */

- return true;

- }

- /* Definitely a 16-bit insn, can't be crossing a page. */

- return false;

+ return !thumb_insn_is_16bit(s, insn);

}

static int arm_tr_init_disas_context(DisasContextBase *dcbase,

2.7.4

From: Cédric Le Goater <clg@kaod.org>

Initially from Anton D. Kachalov" <mouse@yandex-team.ru> but the SoB was

missing.

Signed-off-by: Cédric Le Goater <clg@kaod.org>

Acked-by: Andrew Jeffery <andrew@aj.id.au>

Message-id: 20170920064915.30027-1-clg@kaod.org

[clg: change commit log and subject

replace UL suffix by ULL ]

Signed-off-by: Cédric Le Goater <clg@kaod.org>

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

hw/watchdog/wdt_aspeed.c | 4 ++--

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

diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c

--- a/hw/watchdog/wdt_aspeed.c

+++ b/hw/watchdog/wdt_aspeed.c

@@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_wdt_read(void *opaque, hwaddr offset, unsigned size)

static void aspeed_wdt_reload(AspeedWDTState *s, bool pclk)

- uint32_t reload;

+ uint64_t reload;

if (pclk) {

reload = muldiv64(s->regs[WDT_RELOAD_VALUE], NANOSECONDS_PER_SECOND,

s->pclk_freq);

} else {

- reload = s->regs[WDT_RELOAD_VALUE] * 1000;

+ reload = s->regs[WDT_RELOAD_VALUE] * 1000ULL;

if (aspeed_wdt_is_enabled(s)) {

2.7.4