From 03ae85f858fc46495258a5dd4551fff2c34bd495 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:51 +0100 Subject: target/arm: Use MMUAccessType enum rather than int MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the ARM get_phys_addr() code, switch to using the MMUAccessType enum and its MMU_* values rather than int and literal 0/1/2. Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-id: 1501692241-23310-2-git-send-email-peter.maydell@linaro.org --- target/arm/helper.c | 30 +++++++++++++++--------------- target/arm/internals.h | 3 ++- 2 files changed, 17 insertions(+), 16 deletions(-) (limited to 'target') diff --git a/target/arm/helper.c b/target/arm/helper.c index 0ec92d3214..8e148be3b2 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -20,13 +20,13 @@ #ifndef CONFIG_USER_ONLY static bool get_phys_addr(CPUARMState *env, target_ulong address, - int access_type, ARMMMUIdx mmu_idx, + MMUAccessType access_type, ARMMMUIdx mmu_idx, hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot, target_ulong *page_size, uint32_t *fsr, ARMMMUFaultInfo *fi); static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, - int access_type, ARMMMUIdx mmu_idx, + MMUAccessType access_type, ARMMMUIdx mmu_idx, hwaddr *phys_ptr, MemTxAttrs *txattrs, int *prot, target_ulong *page_size_ptr, uint32_t *fsr, ARMMMUFaultInfo *fi); @@ -2135,7 +2135,7 @@ static CPAccessResult ats_access(CPUARMState *env, const ARMCPRegInfo *ri, } static uint64_t do_ats_write(CPUARMState *env, uint64_t value, - int access_type, ARMMMUIdx mmu_idx) + MMUAccessType access_type, ARMMMUIdx mmu_idx) { hwaddr phys_addr; target_ulong page_size; @@ -2194,7 +2194,7 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value, static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) { - int access_type = ri->opc2 & 1; + MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : MMU_DATA_LOAD; uint64_t par64; ARMMMUIdx mmu_idx; int el = arm_current_el(env); @@ -2253,7 +2253,7 @@ static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) static void ats1h_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) { - int access_type = ri->opc2 & 1; + MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : MMU_DATA_LOAD; uint64_t par64; par64 = do_ats_write(env, value, access_type, ARMMMUIdx_S2NS); @@ -2273,7 +2273,7 @@ static CPAccessResult at_s1e2_access(CPUARMState *env, const ARMCPRegInfo *ri, static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) { - int access_type = ri->opc2 & 1; + MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : MMU_DATA_LOAD; ARMMMUIdx mmu_idx; int secure = arm_is_secure_below_el3(env); @@ -7505,7 +7505,7 @@ static uint64_t arm_ldq_ptw(CPUState *cs, hwaddr addr, bool is_secure, } static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, - int access_type, ARMMMUIdx mmu_idx, + MMUAccessType access_type, ARMMMUIdx mmu_idx, hwaddr *phys_ptr, int *prot, target_ulong *page_size, uint32_t *fsr, ARMMMUFaultInfo *fi) @@ -7621,7 +7621,7 @@ do_fault: } static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, - int access_type, ARMMMUIdx mmu_idx, + MMUAccessType access_type, ARMMMUIdx mmu_idx, hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot, target_ulong *page_size, uint32_t *fsr, ARMMMUFaultInfo *fi) @@ -7728,7 +7728,7 @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, if (pxn && !regime_is_user(env, mmu_idx)) { xn = 1; } - if (xn && access_type == 2) + if (xn && access_type == MMU_INST_FETCH) goto do_fault; if (arm_feature(env, ARM_FEATURE_V6K) && @@ -7843,7 +7843,7 @@ static bool check_s2_mmu_setup(ARMCPU *cpu, bool is_aa64, int level, } static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, - int access_type, ARMMMUIdx mmu_idx, + MMUAccessType access_type, ARMMMUIdx mmu_idx, hwaddr *phys_ptr, MemTxAttrs *txattrs, int *prot, target_ulong *page_size_ptr, uint32_t *fsr, ARMMMUFaultInfo *fi) @@ -8251,7 +8251,7 @@ static inline bool m_is_system_region(CPUARMState *env, uint32_t address) } static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, - int access_type, ARMMMUIdx mmu_idx, + MMUAccessType access_type, ARMMMUIdx mmu_idx, hwaddr *phys_ptr, int *prot, uint32_t *fsr) { ARMCPU *cpu = arm_env_get_cpu(env); @@ -8410,7 +8410,7 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address, } static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address, - int access_type, ARMMMUIdx mmu_idx, + MMUAccessType access_type, ARMMMUIdx mmu_idx, hwaddr *phys_ptr, int *prot, uint32_t *fsr) { int n; @@ -8437,7 +8437,7 @@ static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address, return true; } - if (access_type == 2) { + if (access_type == MMU_INST_FETCH) { mask = env->cp15.pmsav5_insn_ap; } else { mask = env->cp15.pmsav5_data_ap; @@ -8508,7 +8508,7 @@ static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address, * @fsr: set to the DFSR/IFSR value on failure */ static bool get_phys_addr(CPUARMState *env, target_ulong address, - int access_type, ARMMMUIdx mmu_idx, + MMUAccessType access_type, ARMMMUIdx mmu_idx, hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot, target_ulong *page_size, uint32_t *fsr, ARMMMUFaultInfo *fi) @@ -8621,7 +8621,7 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address, * fsr with ARM DFSR/IFSR fault register format value on failure. */ bool arm_tlb_fill(CPUState *cs, vaddr address, - int access_type, int mmu_idx, uint32_t *fsr, + MMUAccessType access_type, int mmu_idx, uint32_t *fsr, ARMMMUFaultInfo *fi) { ARMCPU *cpu = ARM_CPU(cs); diff --git a/target/arm/internals.h b/target/arm/internals.h index 1f6efef7c4..bb06946607 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -457,7 +457,8 @@ struct ARMMMUFaultInfo { }; /* Do a page table walk and add page to TLB if possible */ -bool arm_tlb_fill(CPUState *cpu, vaddr address, int rw, int mmu_idx, +bool arm_tlb_fill(CPUState *cpu, vaddr address, + MMUAccessType access_type, int mmu_idx, uint32_t *fsr, ARMMMUFaultInfo *fi); /* Return true if the stage 1 translation regime is using LPAE format page -- cgit 1.4.1 From 0e2845689ebdb4ea7174f96f6797e2d8942bd114 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:51 +0100 Subject: target/arm: Don't trap WFI/WFE for M profile M profile cores can never trap on WFI or WFE instructions. Check for M profile in check_wfx_trap() to ensure this. The existing code will do the right thing for v7M cores because the hcr_el2 and scr_el3 registers will be all-zeroes and so we won't attempt to trap, but when we start setting ARM_FEATURE_V8 for v8M cores the v8A handling of SCTLR.nTWE and .nTWI will not give the right results. Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias Reviewed-by: Richard Henderson Message-id: 1501692241-23310-3-git-send-email-peter.maydell@linaro.org --- target/arm/op_helper.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'target') diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index 2a85666579..5a94a5f2ba 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -370,6 +370,11 @@ static inline int check_wfx_trap(CPUARMState *env, bool is_wfe) int cur_el = arm_current_el(env); uint64_t mask; + if (arm_feature(env, ARM_FEATURE_M)) { + /* M profile cores can never trap WFI/WFE. */ + return 0; + } + /* If we are currently in EL0 then we need to check if SCTLR is set up for * WFx instructions being trapped to EL1. These trap bits don't exist in v7. */ -- cgit 1.4.1 From 3279adb95e34dd3d67c66d729458f7784747cf8d Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:51 +0100 Subject: target/arm: Consolidate PMSA handling in get_phys_addr() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently get_phys_addr() has PMSAv7 handling before the "is translation disabled?" check, and then PMSAv5 after it. Tidy this up by making the PMSAv5 code handle the "MPU disabled" case itself, so that we have all the PMSA code in one place. This will make adding the PMSAv8 code slightly cleaner, and also means that pre-v7 PMSA cores benefit from the MPU lookup logging that the PMSAv7 codepath had. Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-id: 1501692241-23310-4-git-send-email-peter.maydell@linaro.org --- target/arm/helper.c | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) (limited to 'target') diff --git a/target/arm/helper.c b/target/arm/helper.c index 8e148be3b2..81906825e6 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -8418,6 +8418,13 @@ static bool get_phys_addr_pmsav5(CPUARMState *env, uint32_t address, uint32_t base; bool is_user = regime_is_user(env, mmu_idx); + if (regime_translation_disabled(env, mmu_idx)) { + /* MPU disabled. */ + *phys_ptr = address; + *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; + return false; + } + *phys_ptr = address; for (n = 7; n >= 0; n--) { base = env->cp15.c6_region[n]; @@ -8567,16 +8574,20 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address, } } - /* pmsav7 has special handling for when MPU is disabled so call it before - * the common MMU/MPU disabled check below. - */ - if (arm_feature(env, ARM_FEATURE_PMSA) && - arm_feature(env, ARM_FEATURE_V7)) { + if (arm_feature(env, ARM_FEATURE_PMSA)) { bool ret; *page_size = TARGET_PAGE_SIZE; - ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx, - phys_ptr, prot, fsr); - qemu_log_mask(CPU_LOG_MMU, "PMSAv7 MPU lookup for %s at 0x%08" PRIx32 + + if (arm_feature(env, ARM_FEATURE_V7)) { + /* PMSAv7 */ + ret = get_phys_addr_pmsav7(env, address, access_type, mmu_idx, + phys_ptr, prot, fsr); + } else { + /* Pre-v7 MPU */ + ret = get_phys_addr_pmsav5(env, address, access_type, mmu_idx, + phys_ptr, prot, fsr); + } + qemu_log_mask(CPU_LOG_MMU, "PMSA MPU lookup for %s at 0x%08" PRIx32 " mmu_idx %u -> %s (prot %c%c%c)\n", access_type == MMU_DATA_LOAD ? "reading" : (access_type == MMU_DATA_STORE ? "writing" : "execute"), @@ -8589,21 +8600,16 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address, return ret; } + /* Definitely a real MMU, not an MPU */ + if (regime_translation_disabled(env, mmu_idx)) { - /* MMU/MPU disabled. */ + /* MMU disabled. */ *phys_ptr = address; *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; *page_size = TARGET_PAGE_SIZE; return 0; } - if (arm_feature(env, ARM_FEATURE_PMSA)) { - /* Pre-v7 MPU */ - *page_size = TARGET_PAGE_SIZE; - return get_phys_addr_pmsav5(env, address, access_type, mmu_idx, - phys_ptr, prot, fsr); - } - if (regime_using_lpae_format(env, mmu_idx)) { return get_phys_addr_lpae(env, address, access_type, mmu_idx, phys_ptr, attrs, prot, page_size, fsr, fi); -- cgit 1.4.1 From ebfe27c593e5b222aa2a1fc545b447be3d995faa Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:51 +0100 Subject: target/arm: Tighten up Thumb decode where new v8M insns will be Tighten up the T32 decoder in the places where new v8M instructions will be: * TT/TTT/TTA/TTAT are in what was nominally LDREX/STREX r15, ... which is UNPREDICTABLE: make the UNPREDICTABLE behaviour be to UNDEF * BXNS/BLXNS are distinguished from BX/BLX via the low 3 bits, which in previous architectural versions are SBZ: enforce the SBZ via UNDEF rather than ignoring it, and move the "ARCH(5)" UNDEF case up so we don't leak a TCG temporary * SG is in the encoding which would be LDRD/STRD with rn = r15; this is UNPREDICTABLE and we currently UNDEF: move this check further up the code so that we don't leak TCG temporaries in the UNDEF case and have a better place to put the SG decode. This means that if a v8M binary is accidentally run on v7M or if a test case hits something that we haven't implemented yet the behaviour will be obvious (UNDEF) rather than obscure (plough on treating it as a different instruction). In the process, add some comments about the instruction patterns at these points in the decode. Our Thumb and ARM decoders are very difficult to understand currently, but gradually adding comments like this should help to clarify what exactly has been decoded when. Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias Reviewed-by: Richard Henderson Message-id: 1501692241-23310-5-git-send-email-peter.maydell@linaro.org --- target/arm/translate.c | 48 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 9 deletions(-) (limited to 'target') diff --git a/target/arm/translate.c b/target/arm/translate.c index d1a5f56998..3c14cb024c 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -9735,10 +9735,23 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw abort(); case 4: if (insn & (1 << 22)) { - /* Other load/store, table branch. */ + /* 0b1110_100x_x1xx_xxxx_xxxx_xxxx_xxxx_xxxx + * - load/store doubleword, load/store exclusive, ldacq/strel, + * table branch. + */ if (insn & 0x01200000) { - /* Load/store doubleword. */ + /* 0b1110_1000_x11x_xxxx_xxxx_xxxx_xxxx_xxxx + * - load/store dual (post-indexed) + * 0b1111_1001_x10x_xxxx_xxxx_xxxx_xxxx_xxxx + * - load/store dual (literal and immediate) + * 0b1111_1001_x11x_xxxx_xxxx_xxxx_xxxx_xxxx + * - load/store dual (pre-indexed) + */ if (rn == 15) { + if (insn & (1 << 21)) { + /* UNPREDICTABLE */ + goto illegal_op; + } addr = tcg_temp_new_i32(); tcg_gen_movi_i32(addr, s->pc & ~3); } else { @@ -9772,15 +9785,18 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw } if (insn & (1 << 21)) { /* Base writeback. */ - if (rn == 15) - goto illegal_op; tcg_gen_addi_i32(addr, addr, offset - 4); store_reg(s, rn, addr); } else { tcg_temp_free_i32(addr); } } else if ((insn & (1 << 23)) == 0) { - /* Load/store exclusive word. */ + /* 0b1110_1000_010x_xxxx_xxxx_xxxx_xxxx_xxxx + * - load/store exclusive word + */ + if (rs == 15) { + goto illegal_op; + } addr = tcg_temp_local_new_i32(); load_reg_var(s, addr, rn); tcg_gen_addi_i32(addr, addr, (insn & 0xff) << 2); @@ -11137,7 +11153,9 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s) break; } if (insn & (1 << 10)) { - /* data processing extended or blx */ + /* 0b0100_01xx_xxxx_xxxx + * - data processing extended, branch and exchange + */ rd = (insn & 7) | ((insn >> 4) & 8); rm = (insn >> 3) & 0xf; op = (insn >> 8) & 3; @@ -11160,10 +11178,21 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s) tmp = load_reg(s, rm); store_reg(s, rd, tmp); break; - case 3:/* branch [and link] exchange thumb register */ - tmp = load_reg(s, rm); - if (insn & (1 << 7)) { + case 3: + { + /* 0b0100_0111_xxxx_xxxx + * - branch [and link] exchange thumb register + */ + bool link = insn & (1 << 7); + + if (insn & 7) { + goto undef; + } + if (link) { ARCH(5); + } + tmp = load_reg(s, rm); + if (link) { val = (uint32_t)s->pc | 1; tmp2 = tcg_temp_new_i32(); tcg_gen_movi_i32(tmp2, val); @@ -11175,6 +11204,7 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s) } break; } + } break; } -- cgit 1.4.1 From 59e4972c3fc63d981e8b613ebb3bb01a05848075 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:52 +0100 Subject: target/arm: Remove incorrect comment about MPU_CTRL Remove the comment that claims that some MPU_CTRL bits are stored in sctlr_el[1]. This has never been true since MPU_CTRL was added in commit 29c483a50607 -- the comment is a leftover from Michael Davidsaver's original implementation, which I modified not to use sctlr_el[1]; I forgot to delete the comment then. Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias Reviewed-by: Richard Henderson Message-id: 1501692241-23310-7-git-send-email-peter.maydell@linaro.org --- target/arm/cpu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'target') diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 5932ef1e22..2f2aa872f7 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -416,7 +416,7 @@ typedef struct CPUARMState { uint32_t dfsr; /* Debug Fault Status Register */ uint32_t mmfar; /* MemManage Fault Address */ uint32_t bfar; /* BusFault Address */ - unsigned mpu_ctrl; /* MPU_CTRL (some bits kept in sctlr_el[1]) */ + unsigned mpu_ctrl; /* MPU_CTRL */ int exception; } v7m; -- cgit 1.4.1 From 9d17da4b68a05fc78daa47f0f3d914eea5d802ea Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:52 +0100 Subject: target/arm: Fix outdated comment about exception exit When we switched our handling of exception exit to detect the magic addresses at translate time rather than via a do_unassigned_access hook, we forgot to update a comment; correct the omission. Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias Reviewed-by: Richard Henderson Message-id: 1501692241-23310-8-git-send-email-peter.maydell@linaro.org --- target/arm/helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'target') diff --git a/target/arm/helper.c b/target/arm/helper.c index 81906825e6..2fb0202a28 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -6138,7 +6138,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu) bool rettobase = false; /* We can only get here from an EXCP_EXCEPTION_EXIT, and - * arm_v7m_do_unassigned_access() enforces the architectural rule + * gen_bx_excret() enforces the architectural rule * that jumps to magic addresses don't have magic behaviour unless * we're in Handler mode (compare pseudocode BXWritePC()). */ -- cgit 1.4.1 From 987ab45e108953c1c98126c338c2119c243c372b Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:52 +0100 Subject: target/arm: Define and use XPSR bit masks The M profile XPSR is almost the same format as the A profile CPSR, but not quite. Define some XPSR_* macros and use them where we definitely dealing with an XPSR rather than reusing the CPSR ones. Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias Reviewed-by: Richard Henderson Message-id: 1501692241-23310-9-git-send-email-peter.maydell@linaro.org --- target/arm/cpu.h | 38 ++++++++++++++++++++++++++++---------- target/arm/helper.c | 15 ++++++++------- 2 files changed, 36 insertions(+), 17 deletions(-) (limited to 'target') diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 2f2aa872f7..0b9f9377f8 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -882,6 +882,22 @@ void pmccntr_sync(CPUARMState *env); /* Mask of bits which may be set by exception return copying them from SPSR */ #define CPSR_ERET_MASK (~CPSR_RESERVED) +/* Bit definitions for M profile XPSR. Most are the same as CPSR. */ +#define XPSR_EXCP 0x1ffU +#define XPSR_SPREALIGN (1U << 9) /* Only set in exception stack frames */ +#define XPSR_IT_2_7 CPSR_IT_2_7 +#define XPSR_GE CPSR_GE +#define XPSR_SFPA (1U << 20) /* Only set in exception stack frames */ +#define XPSR_T (1U << 24) /* Not the same as CPSR_T ! */ +#define XPSR_IT_0_1 CPSR_IT_0_1 +#define XPSR_Q CPSR_Q +#define XPSR_V CPSR_V +#define XPSR_C CPSR_C +#define XPSR_Z CPSR_Z +#define XPSR_N CPSR_N +#define XPSR_NZCV CPSR_NZCV +#define XPSR_IT CPSR_IT + #define TTBCR_N (7U << 0) /* TTBCR.EAE==0 */ #define TTBCR_T0SZ (7U << 0) /* TTBCR.EAE==1 */ #define TTBCR_PD0 (1U << 4) @@ -986,26 +1002,28 @@ static inline uint32_t xpsr_read(CPUARMState *env) /* Set the xPSR. Note that some bits of mask must be all-set or all-clear. */ static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask) { - if (mask & CPSR_NZCV) { - env->ZF = (~val) & CPSR_Z; + if (mask & XPSR_NZCV) { + env->ZF = (~val) & XPSR_Z; env->NF = val; env->CF = (val >> 29) & 1; env->VF = (val << 3) & 0x80000000; } - if (mask & CPSR_Q) - env->QF = ((val & CPSR_Q) != 0); - if (mask & (1 << 24)) - env->thumb = ((val & (1 << 24)) != 0); - if (mask & CPSR_IT_0_1) { + if (mask & XPSR_Q) { + env->QF = ((val & XPSR_Q) != 0); + } + if (mask & XPSR_T) { + env->thumb = ((val & XPSR_T) != 0); + } + if (mask & XPSR_IT_0_1) { env->condexec_bits &= ~3; env->condexec_bits |= (val >> 25) & 3; } - if (mask & CPSR_IT_2_7) { + if (mask & XPSR_IT_2_7) { env->condexec_bits &= 3; env->condexec_bits |= (val >> 8) & 0xfc; } - if (mask & 0x1ff) { - env->v7m.exception = val & 0x1ff; + if (mask & XPSR_EXCP) { + env->v7m.exception = val & XPSR_EXCP; } } diff --git a/target/arm/helper.c b/target/arm/helper.c index 2fb0202a28..439ad86d49 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -6114,7 +6114,7 @@ static void v7m_push_stack(ARMCPU *cpu) /* Align stack pointer if the guest wants that */ if ((env->regs[13] & 4) && (env->v7m.ccr & R_V7M_CCR_STKALIGN_MASK)) { env->regs[13] -= 4; - xpsr |= 0x200; + xpsr |= XPSR_SPREALIGN; } /* Switch to the handler mode. */ v7m_push(env, xpsr); @@ -6239,10 +6239,11 @@ static void do_v7m_exception_exit(ARMCPU *cpu) env->regs[15] &= ~1U; } xpsr = v7m_pop(env); - xpsr_write(env, xpsr, 0xfffffdff); + xpsr_write(env, xpsr, ~XPSR_SPREALIGN); /* Undo stack alignment. */ - if (xpsr & 0x200) + 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 @@ -8688,10 +8689,10 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) case 0 ... 7: /* xPSR sub-fields */ mask = 0; if ((reg & 1) && el) { - mask |= 0x000001ff; /* IPSR (unpriv. reads as zero) */ + mask |= XPSR_EXCP; /* IPSR (unpriv. reads as zero) */ } if (!(reg & 4)) { - mask |= 0xf8000000; /* APSR */ + mask |= XPSR_NZCV | XPSR_Q; /* APSR */ } /* EPSR reads as zero */ return xpsr_read(env) & mask; @@ -8749,10 +8750,10 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) uint32_t apsrmask = 0; if (mask & 8) { - apsrmask |= 0xf8000000; /* APSR NZCVQ */ + apsrmask |= XPSR_NZCV | XPSR_Q; } if ((mask & 4) && arm_feature(env, ARM_FEATURE_THUMB_DSP)) { - apsrmask |= 0x000f0000; /* APSR GE[3:0] */ + apsrmask |= XPSR_GE; } xpsr_write(env, val, apsrmask); } -- cgit 1.4.1 From e6ae5981ea4b0f6feb223009a5108582e7644f8f Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:52 +0100 Subject: target/arm: Don't store M profile PRIMASK and FAULTMASK in daif We currently store the M profile CPU register state PRIMASK and FAULTMASK in the daif field of the CPU state in its I and F bits. This is a legacy from the original implementation, which tried to share the cpu_exec_interrupt code between A profile and M profile. We've since separated out the two cases because they are significantly different, so now there is no common code between M and A profile which looks at env->daif: all the uses are either in A-only or M-only code paths. Sharing the state fields now is just confusing, and will make things awkward when we implement v8M, where the PRIMASK and FAULTMASK registers are banked between security states. Switch M profile over to using v7m.faultmask and v7m.primask fields for these registers. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 1501692241-23310-10-git-send-email-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 4 ++-- target/arm/cpu.c | 5 ----- target/arm/cpu.h | 4 +++- target/arm/helper.c | 18 +++++------------- target/arm/machine.c | 33 +++++++++++++++++++++++++++++++++ 5 files changed, 43 insertions(+), 21 deletions(-) (limited to 'target') diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index 2e8166a5d6..343bc160c3 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -167,9 +167,9 @@ static inline int nvic_exec_prio(NVICState *s) CPUARMState *env = &s->cpu->env; int running; - if (env->daif & PSTATE_F) { /* FAULTMASK */ + if (env->v7m.faultmask) { running = -1; - } else if (env->daif & PSTATE_I) { /* PRIMASK */ + } else if (env->v7m.primask) { running = 0; } else if (env->v7m.basepri > 0) { running = env->v7m.basepri & nvic_gprio_mask(s); diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 05c038bf17..b241a634cf 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -185,11 +185,6 @@ static void arm_cpu_reset(CPUState *s) uint32_t initial_pc; /* Loaded from 0x4 */ uint8_t *rom; - /* For M profile we store FAULTMASK and PRIMASK in the - * PSTATE F and I bits; these are both clear at reset. - */ - env->daif &= ~(PSTATE_I | PSTATE_F); - /* The reset value of this bit is IMPDEF, but ARM recommends * that it resets to 1, so QEMU always does that rather than making * it dependent on CPU model. diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 0b9f9377f8..8ef552a0a2 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -418,6 +418,8 @@ typedef struct CPUARMState { uint32_t bfar; /* BusFault Address */ unsigned mpu_ctrl; /* MPU_CTRL */ int exception; + uint32_t primask; + uint32_t faultmask; } v7m; /* Information associated with an exception about to be taken: @@ -2178,7 +2180,7 @@ static inline int cpu_mmu_index(CPUARMState *env, bool ifetch) * we're in a HardFault or NMI handler. */ if ((env->v7m.exception > 0 && env->v7m.exception <= 3) - || env->daif & PSTATE_F) { + || env->v7m.faultmask) { return arm_to_core_mmu_idx(ARMMMUIdx_MNegPri); } diff --git a/target/arm/helper.c b/target/arm/helper.c index 439ad86d49..941085690b 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -6167,7 +6167,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu) if (env->v7m.exception != ARMV7M_EXCP_NMI) { /* Auto-clear FAULTMASK on return from other than NMI */ - env->daif &= ~PSTATE_F; + env->v7m.faultmask = 0; } switch (armv7m_nvic_complete_irq(env->nvic, env->v7m.exception)) { @@ -8713,12 +8713,12 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg) return (env->v7m.control & R_V7M_CONTROL_SPSEL_MASK) ? env->regs[13] : env->v7m.other_sp; case 16: /* PRIMASK */ - return (env->daif & PSTATE_I) != 0; + return env->v7m.primask; case 17: /* BASEPRI */ case 18: /* BASEPRI_MAX */ return env->v7m.basepri; case 19: /* FAULTMASK */ - return (env->daif & PSTATE_F) != 0; + return env->v7m.faultmask; default: qemu_log_mask(LOG_GUEST_ERROR, "Attempt to read unknown special" " register %d\n", reg); @@ -8773,11 +8773,7 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) } break; case 16: /* PRIMASK */ - if (val & 1) { - env->daif |= PSTATE_I; - } else { - env->daif &= ~PSTATE_I; - } + env->v7m.primask = val & 1; break; case 17: /* BASEPRI */ env->v7m.basepri = val & 0xff; @@ -8788,11 +8784,7 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) env->v7m.basepri = val; break; case 19: /* FAULTMASK */ - if (val & 1) { - env->daif |= PSTATE_F; - } else { - env->daif &= ~PSTATE_F; - } + env->v7m.faultmask = val & 1; break; case 20: /* CONTROL */ /* Writing to the SPSEL bit only has an effect if we are in diff --git a/target/arm/machine.c b/target/arm/machine.c index 1f66da4a2c..2fb4b76296 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -97,6 +97,17 @@ static bool m_needed(void *opaque) return arm_feature(env, ARM_FEATURE_M); } +static const VMStateDescription vmstate_m_faultmask_primask = { + .name = "cpu/m/faultmask-primask", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT32(env.v7m.faultmask, ARMCPU), + VMSTATE_UINT32(env.v7m.primask, ARMCPU), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_m = { .name = "cpu/m", .version_id = 4, @@ -115,6 +126,10 @@ static const VMStateDescription vmstate_m = { VMSTATE_UINT32(env.v7m.mpu_ctrl, ARMCPU), VMSTATE_INT32(env.v7m.exception, ARMCPU), VMSTATE_END_OF_LIST() + }, + .subsections = (const VMStateDescription*[]) { + &vmstate_m_faultmask_primask, + NULL } }; @@ -201,6 +216,24 @@ static int get_cpsr(QEMUFile *f, void *opaque, size_t size, CPUARMState *env = &cpu->env; uint32_t val = qemu_get_be32(f); + if (arm_feature(env, ARM_FEATURE_M)) { + /* If the I or F bits are set then this is a migration from + * an old QEMU which still stored the M profile FAULTMASK + * and PRIMASK in env->daif. Set v7m.faultmask and v7m.primask + * accordingly, and then clear the bits so they don't confuse + * cpsr_write(). For a new QEMU, the bits here will always be + * clear, and the data is transferred using the + * vmstate_m_faultmask_primask subsection. + */ + if (val & CPSR_F) { + env->v7m.faultmask = 1; + } + if (val & CPSR_I) { + env->v7m.primask = 1; + } + val &= ~(CPSR_F | CPSR_I); + } + env->aarch64 = ((val & PSTATE_nRW) == 0); if (is_a64(env)) { -- cgit 1.4.1 From eeade0017698ada9d3049ec9021d094d96304f58 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:52 +0100 Subject: target/arm: Don't use cpsr_write/cpsr_read to transfer M profile XPSR For M profile the XPSR is a similar but not identical format to the A profile CPSR/SPSR. (For instance the Thumb bit is in a different place.) For guest accesses we make the M profile code go through xpsr_read() and xpsr_write() which handle the different layout. However for migration we use cpsr_read() and cpsr_write() to marshal state into and out of the migration data stream. This is pretty confusing and works more by luck than anything else. Make M profile migration use xpsr_read() and xpsr_write() instead. The most complicated part of this is handling the possibility that the migration source is an older QEMU which hands us a CPSR format value; helpfully we can always tell the two apart. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Message-id: 1501692241-23310-11-git-send-email-peter.maydell@linaro.org --- target/arm/machine.c | 49 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 15 deletions(-) (limited to 'target') diff --git a/target/arm/machine.c b/target/arm/machine.c index 2fb4b76296..3193b00b04 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -217,21 +217,37 @@ static int get_cpsr(QEMUFile *f, void *opaque, size_t size, uint32_t val = qemu_get_be32(f); if (arm_feature(env, ARM_FEATURE_M)) { - /* If the I or F bits are set then this is a migration from - * an old QEMU which still stored the M profile FAULTMASK - * and PRIMASK in env->daif. Set v7m.faultmask and v7m.primask - * accordingly, and then clear the bits so they don't confuse - * cpsr_write(). For a new QEMU, the bits here will always be - * clear, and the data is transferred using the - * vmstate_m_faultmask_primask subsection. - */ - if (val & CPSR_F) { - env->v7m.faultmask = 1; - } - if (val & CPSR_I) { - env->v7m.primask = 1; + if (val & XPSR_EXCP) { + /* This is a CPSR format value from an older QEMU. (We can tell + * because values transferred in XPSR format always have zero + * for the EXCP field, and CPSR format will always have bit 4 + * set in CPSR_M.) Rearrange it into XPSR format. The significant + * differences are that the T bit is not in the same place, the + * primask/faultmask info may be in the CPSR I and F bits, and + * we do not want the mode bits. + */ + uint32_t newval = val; + + newval &= (CPSR_NZCV | CPSR_Q | CPSR_IT | CPSR_GE); + if (val & CPSR_T) { + newval |= XPSR_T; + } + /* If the I or F bits are set then this is a migration from + * an old QEMU which still stored the M profile FAULTMASK + * and PRIMASK in env->daif. For a new QEMU, the data is + * transferred using the vmstate_m_faultmask_primask subsection. + */ + if (val & CPSR_F) { + env->v7m.faultmask = 1; + } + if (val & CPSR_I) { + env->v7m.primask = 1; + } + val = newval; } - val &= ~(CPSR_F | CPSR_I); + /* Ignore the low bits, they are handled by vmstate_m. */ + xpsr_write(env, val, ~XPSR_EXCP); + return 0; } env->aarch64 = ((val & PSTATE_nRW) == 0); @@ -252,7 +268,10 @@ static int put_cpsr(QEMUFile *f, void *opaque, size_t size, CPUARMState *env = &cpu->env; uint32_t val; - if (is_a64(env)) { + if (arm_feature(env, ARM_FEATURE_M)) { + /* The low 9 bits are v7m.exception, which is handled by vmstate_m. */ + val = xpsr_read(env) & ~XPSR_EXCP; + } else if (is_a64(env)) { val = pstate_read(env); } else { val = cpsr_read(env); -- cgit 1.4.1 From 5b906f3589443a3c69d8feeaac37263843ecfb8d Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:52 +0100 Subject: target/arm: Make arm_cpu_dump_state() handle the M-profile XPSR Make the arm_cpu_dump_state() debug logging handle the M-profile XPSR rather than assuming it's an A-profile CPSR. On M profile the PSR line of a register dump will now look like this: XPSR=41000000 -Z-- T priv-thread Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias Reviewed-by: Richard Henderson Message-id: 1501692241-23310-12-git-send-email-peter.maydell@linaro.org --- target/arm/translate.c | 58 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 18 deletions(-) (limited to 'target') diff --git a/target/arm/translate.c b/target/arm/translate.c index 3c14cb024c..e52a6d7622 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -12215,8 +12215,6 @@ void arm_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf, ARMCPU *cpu = ARM_CPU(cs); CPUARMState *env = &cpu->env; int i; - uint32_t psr; - const char *ns_status; if (is_a64(env)) { aarch64_cpu_dump_state(cs, f, cpu_fprintf, flags); @@ -12230,24 +12228,48 @@ void arm_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf, else cpu_fprintf(f, " "); } - psr = cpsr_read(env); - if (arm_feature(env, ARM_FEATURE_EL3) && - (psr & CPSR_M) != ARM_CPU_MODE_MON) { - ns_status = env->cp15.scr_el3 & SCR_NS ? "NS " : "S "; + if (arm_feature(env, ARM_FEATURE_M)) { + uint32_t xpsr = xpsr_read(env); + const char *mode; + + if (xpsr & XPSR_EXCP) { + mode = "handler"; + } else { + if (env->v7m.control & R_V7M_CONTROL_NPRIV_MASK) { + mode = "unpriv-thread"; + } else { + mode = "priv-thread"; + } + } + + cpu_fprintf(f, "XPSR=%08x %c%c%c%c %c %s\n", + xpsr, + xpsr & XPSR_N ? 'N' : '-', + xpsr & XPSR_Z ? 'Z' : '-', + xpsr & XPSR_C ? 'C' : '-', + xpsr & XPSR_V ? 'V' : '-', + xpsr & XPSR_T ? 'T' : 'A', + mode); } else { - ns_status = ""; - } - - cpu_fprintf(f, "PSR=%08x %c%c%c%c %c %s%s%d\n", - psr, - psr & (1 << 31) ? 'N' : '-', - psr & (1 << 30) ? 'Z' : '-', - psr & (1 << 29) ? 'C' : '-', - psr & (1 << 28) ? 'V' : '-', - psr & CPSR_T ? 'T' : 'A', - ns_status, - cpu_mode_names[psr & 0xf], (psr & 0x10) ? 32 : 26); + uint32_t psr = cpsr_read(env); + const char *ns_status = ""; + + if (arm_feature(env, ARM_FEATURE_EL3) && + (psr & CPSR_M) != ARM_CPU_MODE_MON) { + ns_status = env->cp15.scr_el3 & SCR_NS ? "NS " : "S "; + } + + cpu_fprintf(f, "PSR=%08x %c%c%c%c %c %s%s%d\n", + psr, + psr & CPSR_N ? 'N' : '-', + psr & CPSR_Z ? 'Z' : '-', + psr & CPSR_C ? 'C' : '-', + psr & CPSR_V ? 'V' : '-', + psr & CPSR_T ? 'T' : 'A', + ns_status, + cpu_mode_names[psr & 0xf], (psr & 0x10) ? 32 : 26); + } if (flags & CPU_DUMP_FPU) { int numvfpregs = 0; -- cgit 1.4.1 From bd70b29ba92e4446f9e4eb8b9acc19ef6ff4a4d5 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:53 +0100 Subject: target/arm: Don't calculate lr in arm_v7m_cpu_do_interrupt() until needed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the code in arm_v7m_cpu_do_interrupt() that calculates the magic LR value down to when we're actually going to use it. Having the calculation and use so far apart makes the code a little harder to understand than it needs to be. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Edgar E. Iglesias Reviewed-by: Richard Henderson Message-id: 1501692241-23310-13-git-send-email-peter.maydell@linaro.org --- target/arm/helper.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'target') diff --git a/target/arm/helper.c b/target/arm/helper.c index 941085690b..267a170a79 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -6306,13 +6306,6 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) arm_log_exception(cs->exception_index); - lr = 0xfffffff1; - if (env->v7m.control & R_V7M_CONTROL_SPSEL_MASK) { - lr |= 4; - } - if (env->v7m.exception == 0) - lr |= 8; - /* For exceptions we just mark as pending on the NVIC, and let that handle it. */ switch (cs->exception_index) { @@ -6403,6 +6396,14 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) return; /* Never happens. Keep compiler happy. */ } + lr = 0xfffffff1; + if (env->v7m.control & R_V7M_CONTROL_SPSEL_MASK) { + lr |= 4; + } + if (env->v7m.exception == 0) { + lr |= 8; + } + v7m_push_stack(cpu); v7m_exception_taken(cpu, lr); qemu_log_mask(CPU_LOG_INT, "... as %d\n", env->v7m.exception); -- cgit 1.4.1 From 15b3f556bab4f961bf92141eb8521c8da3df5eb2 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:53 +0100 Subject: target/arm: Create and use new function arm_v7m_is_handler_mode() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a utility function for testing whether the CPU is in Handler mode; this is just a check whether v7m.exception is non-zero, but we do it in several places and it makes the code a bit easier to read to not have to mentally figure out what the test is testing. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Edgar E. Iglesias Reviewed-by: Richard Henderson Message-id: 1501692241-23310-14-git-send-email-peter.maydell@linaro.org --- target/arm/cpu.h | 10 ++++++++-- target/arm/helper.c | 8 ++++---- 2 files changed, 12 insertions(+), 6 deletions(-) (limited to 'target') diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 8ef552a0a2..eabef00a34 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -1629,13 +1629,19 @@ static inline int arm_highest_el(CPUARMState *env) return 1; } +/* Return true if a v7M CPU is in Handler mode */ +static inline bool arm_v7m_is_handler_mode(CPUARMState *env) +{ + return env->v7m.exception != 0; +} + /* Return the current Exception Level (as per ARMv8; note that this differs * from the ARMv7 Privilege Level). */ static inline int arm_current_el(CPUARMState *env) { if (arm_feature(env, ARM_FEATURE_M)) { - return !((env->v7m.exception == 0) && (env->v7m.control & 1)); + return arm_v7m_is_handler_mode(env) || !(env->v7m.control & 1); } if (is_a64(env)) { @@ -2635,7 +2641,7 @@ static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc, } *flags |= fp_exception_el(env) << ARM_TBFLAG_FPEXC_EL_SHIFT; - if (env->v7m.exception != 0) { + if (arm_v7m_is_handler_mode(env)) { *flags |= ARM_TBFLAG_HANDLER_MASK; } diff --git a/target/arm/helper.c b/target/arm/helper.c index 267a170a79..37e7fd980e 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -6142,7 +6142,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu) * that jumps to magic addresses don't have magic behaviour unless * we're in Handler mode (compare pseudocode BXWritePC()). */ - assert(env->v7m.exception != 0); + assert(arm_v7m_is_handler_mode(env)); /* In the spec pseudocode ExceptionReturn() is called directly * from BXWritePC() and gets the full target PC value including @@ -6249,7 +6249,7 @@ static void do_v7m_exception_exit(ARMCPU *cpu) * resuming in Thread mode. If that doesn't match what the * exception return type specified then this is a UsageFault. */ - if (return_to_handler == (env->v7m.exception == 0)) { + if (return_to_handler != arm_v7m_is_handler_mode(env)) { /* Take an INVPC UsageFault by pushing the stack again. */ armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE); env->v7m.cfsr |= R_V7M_CFSR_INVPC_MASK; @@ -6400,7 +6400,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) if (env->v7m.control & R_V7M_CONTROL_SPSEL_MASK) { lr |= 4; } - if (env->v7m.exception == 0) { + if (!arm_v7m_is_handler_mode(env)) { lr |= 8; } @@ -8793,7 +8793,7 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val) * switch_v7m_sp() deals with updating the SPSEL bit in * env->v7m.control, so we only need update the others. */ - if (env->v7m.exception == 0) { + if (!arm_v7m_is_handler_mode(env)) { switch_v7m_sp(env, (val & R_V7M_CONTROL_SPSEL_MASK) != 0); } env->v7m.control &= ~R_V7M_CONTROL_NPRIV_MASK; -- cgit 1.4.1 From 07f48730bc4ec99f128901705336eb455f674694 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Mon, 4 Sep 2017 15:21:53 +0100 Subject: hw/arm/virt: add pmu interrupt state Mimicking gicv3-maintenance-interrupt, add the PMU's interrupt to CPU state. Signed-off-by: Andrew Jones Reviewed-by: Peter Maydell Message-id: 1500471597-2517-2-git-send-email-drjones@redhat.com Signed-off-by: Peter Maydell --- hw/arm/virt.c | 3 +++ target/arm/cpu.c | 2 ++ target/arm/cpu.h | 2 ++ 3 files changed, 7 insertions(+) (limited to 'target') diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 6b7a0fefc4..a06ec13f08 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -610,6 +610,9 @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic) qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", 0, qdev_get_gpio_in(gicdev, ppibase + ARCH_GICV3_MAINT_IRQ)); + qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0, + qdev_get_gpio_in(gicdev, ppibase + + VIRTUAL_PMU_IRQ)); sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ)); sysbus_connect_irq(gicbusdev, i + smp_cpus, diff --git a/target/arm/cpu.c b/target/arm/cpu.c index b241a634cf..41ae6ba3c2 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -508,6 +508,8 @@ static void arm_cpu_initfn(Object *obj) qdev_init_gpio_out_named(DEVICE(cpu), &cpu->gicv3_maintenance_interrupt, "gicv3-maintenance-interrupt", 1); + qdev_init_gpio_out_named(DEVICE(cpu), &cpu->pmu_interrupt, + "pmu-interrupt", 1); #endif /* DTB consumers generally don't in fact care what the 'compatible' diff --git a/target/arm/cpu.h b/target/arm/cpu.h index eabef00a34..92771d3790 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -585,6 +585,8 @@ struct ARMCPU { qemu_irq gt_timer_outputs[NUM_GTIMERS]; /* GPIO output for GICv3 maintenance interrupt signal */ qemu_irq gicv3_maintenance_interrupt; + /* GPIO output for the PMU interrupt */ + qemu_irq pmu_interrupt; /* MemoryRegion to use for secure physical accesses */ MemoryRegion *secure_memory; -- cgit 1.4.1 From 3f07cb2aabdfcc27ee30cf85c39619a82d364f83 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Mon, 4 Sep 2017 15:21:54 +0100 Subject: target/arm/kvm: pmu: split init and set-irq stages When adding a PMU with a userspace irqchip we skip the set-irq stage of device creation. Split the 'create' function into two functions 'init' and 'set-irq' so they may be called separately. Signed-off-by: Andrew Jones Reviewed-by: Christoffer Dall Message-id: 1500471597-2517-3-git-send-email-drjones@redhat.com Signed-off-by: Peter Maydell --- hw/arm/virt.c | 11 +++++++++-- target/arm/kvm32.c | 8 +++++++- target/arm/kvm64.c | 52 +++++++++++++++++++++++++--------------------------- target/arm/kvm_arm.h | 10 ++++++++-- 4 files changed, 49 insertions(+), 32 deletions(-) (limited to 'target') diff --git a/hw/arm/virt.c b/hw/arm/virt.c index a06ec13f08..d6e2486fbc 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -492,10 +492,17 @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms) CPU_FOREACH(cpu) { armcpu = ARM_CPU(cpu); - if (!arm_feature(&armcpu->env, ARM_FEATURE_PMU) || - (kvm_enabled() && !kvm_arm_pmu_create(cpu, PPI(VIRTUAL_PMU_IRQ)))) { + if (!arm_feature(&armcpu->env, ARM_FEATURE_PMU)) { return; } + if (kvm_enabled()) { + if (!kvm_arm_pmu_set_irq(cpu, PPI(VIRTUAL_PMU_IRQ))) { + return; + } + if (!kvm_arm_pmu_init(cpu)) { + return; + } + } } if (vms->gic_version == 2) { diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c index 069da0c5fd..e3aab89a1a 100644 --- a/target/arm/kvm32.c +++ b/target/arm/kvm32.c @@ -522,7 +522,13 @@ bool kvm_arm_hw_debug_active(CPUState *cs) return false; } -int kvm_arm_pmu_create(CPUState *cs, int irq) +int kvm_arm_pmu_set_irq(CPUState *cs, int irq) +{ + qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); + return 0; +} + +int kvm_arm_pmu_init(CPUState *cs) { qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); return 0; diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index a16abc8d12..e26638a6fa 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -381,46 +381,44 @@ static CPUWatchpoint *find_hw_watchpoint(CPUState *cpu, target_ulong addr) return NULL; } -static bool kvm_arm_pmu_support_ctrl(CPUState *cs, struct kvm_device_attr *attr) -{ - return kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, attr) == 0; -} - -int kvm_arm_pmu_create(CPUState *cs, int irq) +static bool kvm_arm_pmu_set_attr(CPUState *cs, struct kvm_device_attr *attr) { int err; - struct kvm_device_attr attr = { - .group = KVM_ARM_VCPU_PMU_V3_CTRL, - .addr = (intptr_t)&irq, - .attr = KVM_ARM_VCPU_PMU_V3_IRQ, - .flags = 0, - }; - - if (!kvm_arm_pmu_support_ctrl(cs, &attr)) { - return 0; + err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, attr); + if (err != 0) { + return false; } - err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, &attr); + err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, attr); if (err < 0) { fprintf(stderr, "KVM_SET_DEVICE_ATTR failed: %s\n", strerror(-err)); abort(); } - attr.group = KVM_ARM_VCPU_PMU_V3_CTRL; - attr.attr = KVM_ARM_VCPU_PMU_V3_INIT; - attr.addr = 0; - attr.flags = 0; + return true; +} - err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, &attr); - if (err < 0) { - fprintf(stderr, "KVM_SET_DEVICE_ATTR failed: %s\n", - strerror(-err)); - abort(); - } +int kvm_arm_pmu_init(CPUState *cs) +{ + struct kvm_device_attr attr = { + .group = KVM_ARM_VCPU_PMU_V3_CTRL, + .attr = KVM_ARM_VCPU_PMU_V3_INIT, + }; + + return kvm_arm_pmu_set_attr(cs, &attr); +} + +int kvm_arm_pmu_set_irq(CPUState *cs, int irq) +{ + struct kvm_device_attr attr = { + .group = KVM_ARM_VCPU_PMU_V3_CTRL, + .addr = (intptr_t)&irq, + .attr = KVM_ARM_VCPU_PMU_V3_IRQ, + }; - return 1; + return kvm_arm_pmu_set_attr(cs, &attr); } static inline void set_feature(uint64_t *features, int feature) diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h index 633d08828a..cab5ea9be5 100644 --- a/target/arm/kvm_arm.h +++ b/target/arm/kvm_arm.h @@ -195,7 +195,8 @@ int kvm_arm_sync_mpstate_to_qemu(ARMCPU *cpu); int kvm_arm_vgic_probe(void); -int kvm_arm_pmu_create(CPUState *cs, int irq); +int kvm_arm_pmu_set_irq(CPUState *cs, int irq); +int kvm_arm_pmu_init(CPUState *cs); #else @@ -204,7 +205,12 @@ static inline int kvm_arm_vgic_probe(void) return 0; } -static inline int kvm_arm_pmu_create(CPUState *cs, int irq) +static inline int kvm_arm_pmu_set_irq(CPUState *cs, int irq) +{ + return 0; +} + +static inline int kvm_arm_pmu_init(CPUState *cs) { return 0; } -- cgit 1.4.1 From b16595275bc9b9ce6a36bfb0344d514ab77e6b98 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Mon, 4 Sep 2017 15:21:54 +0100 Subject: hw/arm/virt: allow pmu instantiation with userspace irqchip Move the in-kernel-irqchip test to only guard the set-irq stage, not the init stage of the PMU. Also add the PMU to the KVM device irq line synchronization to enable its use. Signed-off-by: Andrew Jones Reviewed-by: Christoffer Dall Message-id: 1500471597-2517-4-git-send-email-drjones@redhat.com Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/arm/virt.c | 3 ++- target/arm/kvm.c | 6 +++++- target/arm/kvm64.c | 3 +-- 3 files changed, 8 insertions(+), 4 deletions(-) (limited to 'target') diff --git a/hw/arm/virt.c b/hw/arm/virt.c index d6e2486fbc..999f448af2 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -496,7 +496,8 @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms) return; } if (kvm_enabled()) { - if (!kvm_arm_pmu_set_irq(cpu, PPI(VIRTUAL_PMU_IRQ))) { + if (kvm_irqchip_in_kernel() && + !kvm_arm_pmu_set_irq(cpu, PPI(VIRTUAL_PMU_IRQ))) { return; } if (!kvm_arm_pmu_init(cpu)) { diff --git a/target/arm/kvm.c b/target/arm/kvm.c index 7c17f0d629..211a7bf7be 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -567,7 +567,11 @@ MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run) switched_level &= ~KVM_ARM_DEV_EL1_PTIMER; } - /* XXX PMU IRQ is missing */ + if (switched_level & KVM_ARM_DEV_PMU) { + qemu_set_irq(cpu->pmu_interrupt, + !!(run->s.regs.device_irq_level & KVM_ARM_DEV_PMU)); + switched_level &= ~KVM_ARM_DEV_PMU; + } if (switched_level) { qemu_log_mask(LOG_UNIMP, "%s: unhandled in-kernel device IRQ %x\n", diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index e26638a6fa..ec7d85314a 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -506,8 +506,7 @@ int kvm_arch_init_vcpu(CPUState *cs) if (!arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_EL1_32BIT; } - if (!kvm_irqchip_in_kernel() || - !kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PMU_V3)) { + if (!kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PMU_V3)) { cpu->has_pmu = false; } if (cpu->has_pmu) { -- cgit 1.4.1 From b2bfe9f7f1f7e3aa5edf9c3c4c7408082778ae17 Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Mon, 4 Sep 2017 15:21:54 +0100 Subject: target/arm/kvm: pmu: improve error handling If a KVM PMU init or set-irq attr call fails we just silently stop the PMU DT node generation. The only way they could fail, though, is if the attr's respective KVM has-attr call fails. But that should never happen if KVM advertises the PMU capability, because both attrs have been available since the capability was introduced. Let's just abort if this should-never-happen stuff does happen, because, if it does, then something is obviously horribly wrong. Signed-off-by: Andrew Jones Reviewed-by: Christoffer Dall Message-id: 1500471597-2517-5-git-send-email-drjones@redhat.com [PMM: change kvm32.c kvm_arm_pmu_init() to the new API too] Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/arm/virt.c | 9 +++------ target/arm/kvm32.c | 6 ++---- target/arm/kvm64.c | 28 ++++++++++++++++++++-------- target/arm/kvm_arm.h | 15 ++++----------- 4 files changed, 29 insertions(+), 29 deletions(-) (limited to 'target') diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 999f448af2..fe96557997 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -496,13 +496,10 @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms) return; } if (kvm_enabled()) { - if (kvm_irqchip_in_kernel() && - !kvm_arm_pmu_set_irq(cpu, PPI(VIRTUAL_PMU_IRQ))) { - return; - } - if (!kvm_arm_pmu_init(cpu)) { - return; + if (kvm_irqchip_in_kernel()) { + kvm_arm_pmu_set_irq(cpu, PPI(VIRTUAL_PMU_IRQ)); } + kvm_arm_pmu_init(cpu); } } diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c index e3aab89a1a..f925a21481 100644 --- a/target/arm/kvm32.c +++ b/target/arm/kvm32.c @@ -522,14 +522,12 @@ bool kvm_arm_hw_debug_active(CPUState *cs) return false; } -int kvm_arm_pmu_set_irq(CPUState *cs, int irq) +void kvm_arm_pmu_set_irq(CPUState *cs, int irq) { qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); - return 0; } -int kvm_arm_pmu_init(CPUState *cs) +void kvm_arm_pmu_init(CPUState *cs) { qemu_log_mask(LOG_UNIMP, "%s: not implemented\n", __func__); - return 0; } diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index ec7d85314a..6554c30007 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -387,30 +387,36 @@ static bool kvm_arm_pmu_set_attr(CPUState *cs, struct kvm_device_attr *attr) err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, attr); if (err != 0) { + error_report("PMU: KVM_HAS_DEVICE_ATTR: %s", strerror(-err)); return false; } err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, attr); - if (err < 0) { - fprintf(stderr, "KVM_SET_DEVICE_ATTR failed: %s\n", - strerror(-err)); - abort(); + if (err != 0) { + error_report("PMU: KVM_SET_DEVICE_ATTR: %s", strerror(-err)); + return false; } return true; } -int kvm_arm_pmu_init(CPUState *cs) +void kvm_arm_pmu_init(CPUState *cs) { struct kvm_device_attr attr = { .group = KVM_ARM_VCPU_PMU_V3_CTRL, .attr = KVM_ARM_VCPU_PMU_V3_INIT, }; - return kvm_arm_pmu_set_attr(cs, &attr); + if (!ARM_CPU(cs)->has_pmu) { + return; + } + if (!kvm_arm_pmu_set_attr(cs, &attr)) { + error_report("failed to init PMU"); + abort(); + } } -int kvm_arm_pmu_set_irq(CPUState *cs, int irq) +void kvm_arm_pmu_set_irq(CPUState *cs, int irq) { struct kvm_device_attr attr = { .group = KVM_ARM_VCPU_PMU_V3_CTRL, @@ -418,7 +424,13 @@ int kvm_arm_pmu_set_irq(CPUState *cs, int irq) .attr = KVM_ARM_VCPU_PMU_V3_IRQ, }; - return kvm_arm_pmu_set_attr(cs, &attr); + if (!ARM_CPU(cs)->has_pmu) { + return; + } + if (!kvm_arm_pmu_set_attr(cs, &attr)) { + error_report("failed to set irq for PMU"); + abort(); + } } static inline void set_feature(uint64_t *features, int feature) diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h index cab5ea9be5..ff53e9fafb 100644 --- a/target/arm/kvm_arm.h +++ b/target/arm/kvm_arm.h @@ -195,8 +195,8 @@ int kvm_arm_sync_mpstate_to_qemu(ARMCPU *cpu); int kvm_arm_vgic_probe(void); -int kvm_arm_pmu_set_irq(CPUState *cs, int irq); -int kvm_arm_pmu_init(CPUState *cs); +void kvm_arm_pmu_set_irq(CPUState *cs, int irq); +void kvm_arm_pmu_init(CPUState *cs); #else @@ -205,15 +205,8 @@ static inline int kvm_arm_vgic_probe(void) return 0; } -static inline int kvm_arm_pmu_set_irq(CPUState *cs, int irq) -{ - return 0; -} - -static inline int kvm_arm_pmu_init(CPUState *cs) -{ - return 0; -} +static inline void kvm_arm_pmu_set_irq(CPUState *cs, int irq) {} +static inline void kvm_arm_pmu_init(CPUState *cs) {} #endif -- cgit 1.4.1 From aac43da1d772a50778ab1252c13c08c2eb31fb39 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:55 +0100 Subject: target/arm: Factor out fault delivery code We currently have some similar code in tlb_fill() and in arm_cpu_do_unaligned_access() for delivering a data abort or prefetch abort. We're also going to want to do the same thing to handle external aborts. Factor out the common code into a new function deliver_fault(). Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Acked-by: Edgar E. Iglesias --- target/arm/op_helper.c | 110 +++++++++++++++++++++++++------------------------ 1 file changed, 57 insertions(+), 53 deletions(-) (limited to 'target') diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index 5a94a5f2ba..6114597fe4 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -115,6 +115,51 @@ static inline uint32_t merge_syn_data_abort(uint32_t template_syn, return syn; } +static void deliver_fault(ARMCPU *cpu, vaddr addr, MMUAccessType access_type, + uint32_t fsr, uint32_t fsc, ARMMMUFaultInfo *fi) +{ + CPUARMState *env = &cpu->env; + int target_el; + bool same_el; + uint32_t syn, exc; + + target_el = exception_target_el(env); + if (fi->stage2) { + target_el = 2; + env->cp15.hpfar_el2 = extract64(fi->s2addr, 12, 47) << 4; + } + same_el = (arm_current_el(env) == target_el); + + if (fsc == 0x3f) { + /* Caller doesn't have a long-format fault status code. This + * should only happen if this fault will never actually be reported + * to an EL that uses a syndrome register. Check that here. + * 0x3f is a (currently) reserved FSC code, in case the constructed + * syndrome does leak into the guest somehow. + */ + assert(target_el != 2 && !arm_el_is_aa64(env, target_el)); + } + + if (access_type == MMU_INST_FETCH) { + syn = syn_insn_abort(same_el, 0, fi->s1ptw, fsc); + exc = EXCP_PREFETCH_ABORT; + } else { + syn = merge_syn_data_abort(env->exception.syndrome, target_el, + same_el, fi->s1ptw, + access_type == MMU_DATA_STORE, + fsc); + if (access_type == MMU_DATA_STORE + && arm_feature(env, ARM_FEATURE_V6)) { + fsr |= (1 << 11); + } + exc = EXCP_DATA_ABORT; + } + + env->exception.vaddress = addr; + env->exception.fsr = fsr; + raise_exception(env, exc, syn, target_el); +} + /* try to fill the TLB and return an exception if error. If retaddr is * NULL, it means that the function was called in C code (i.e. not * from generated code or from helper.c) @@ -129,23 +174,13 @@ void tlb_fill(CPUState *cs, target_ulong addr, MMUAccessType access_type, ret = arm_tlb_fill(cs, addr, access_type, mmu_idx, &fsr, &fi); if (unlikely(ret)) { ARMCPU *cpu = ARM_CPU(cs); - CPUARMState *env = &cpu->env; - uint32_t syn, exc, fsc; - unsigned int target_el; - bool same_el; + uint32_t fsc; if (retaddr) { /* now we have a real cpu fault */ cpu_restore_state(cs, retaddr); } - target_el = exception_target_el(env); - if (fi.stage2) { - target_el = 2; - env->cp15.hpfar_el2 = extract64(fi.s2addr, 12, 47) << 4; - } - same_el = arm_current_el(env) == target_el; - if (fsr & (1 << 9)) { /* LPAE format fault status register : bottom 6 bits are * status code in the same form as needed for syndrome @@ -153,34 +188,15 @@ void tlb_fill(CPUState *cs, target_ulong addr, MMUAccessType access_type, fsc = extract32(fsr, 0, 6); } else { /* Short format FSR : this fault will never actually be reported - * to an EL that uses a syndrome register. Check that here, - * and use a (currently) reserved FSR code in case the constructed - * syndrome does leak into the guest somehow. + * to an EL that uses a syndrome register. Use a (currently) + * reserved FSR code in case the constructed syndrome does leak + * into the guest somehow. deliver_fault will assert that + * we don't target an EL using the syndrome. */ - assert(target_el != 2 && !arm_el_is_aa64(env, target_el)); fsc = 0x3f; } - /* For insn and data aborts we assume there is no instruction syndrome - * information; this is always true for exceptions reported to EL1. - */ - if (access_type == MMU_INST_FETCH) { - syn = syn_insn_abort(same_el, 0, fi.s1ptw, fsc); - exc = EXCP_PREFETCH_ABORT; - } else { - syn = merge_syn_data_abort(env->exception.syndrome, target_el, - same_el, fi.s1ptw, - access_type == MMU_DATA_STORE, fsc); - if (access_type == MMU_DATA_STORE - && arm_feature(env, ARM_FEATURE_V6)) { - fsr |= (1 << 11); - } - exc = EXCP_DATA_ABORT; - } - - env->exception.vaddress = addr; - env->exception.fsr = fsr; - raise_exception(env, exc, syn, target_el); + deliver_fault(cpu, addr, access_type, fsr, fsc, &fi); } } @@ -191,9 +207,8 @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, { ARMCPU *cpu = ARM_CPU(cs); CPUARMState *env = &cpu->env; - int target_el; - bool same_el; - uint32_t syn; + uint32_t fsr, fsc; + ARMMMUFaultInfo fi = {}; ARMMMUIdx arm_mmu_idx = core_to_arm_mmu_idx(env, mmu_idx); if (retaddr) { @@ -201,28 +216,17 @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, cpu_restore_state(cs, retaddr); } - target_el = exception_target_el(env); - same_el = (arm_current_el(env) == target_el); - - env->exception.vaddress = vaddr; - /* the DFSR for an alignment fault depends on whether we're using * the LPAE long descriptor format, or the short descriptor format */ if (arm_s1_regime_using_lpae_format(env, arm_mmu_idx)) { - env->exception.fsr = (1 << 9) | 0x21; + fsr = (1 << 9) | 0x21; } else { - env->exception.fsr = 0x1; - } - - if (access_type == MMU_DATA_STORE && arm_feature(env, ARM_FEATURE_V6)) { - env->exception.fsr |= (1 << 11); + fsr = 0x1; } + fsc = 0x21; - syn = merge_syn_data_abort(env->exception.syndrome, target_el, - same_el, 0, access_type == MMU_DATA_STORE, - 0x21); - raise_exception(env, EXCP_DATA_ABORT, syn, target_el); + deliver_fault(cpu, vaddr, access_type, fsr, fsc, &fi); } #endif /* !defined(CONFIG_USER_ONLY) */ -- cgit 1.4.1 From c528af7aa64f159eb30b46e567b650c5440fc117 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 4 Sep 2017 15:21:55 +0100 Subject: target/arm: Allow deliver_fault() caller to specify EA bit For external aborts, we will want to be able to specify the EA (external abort type) bit in the syndrome field. Allow callers of deliver_fault() to do that by adding a field to ARMMMUFaultInfo which we use when constructing the syndrome values. Signed-off-by: Peter Maydell Reviewed-by: Richard Henderson Reviewed-by: Edgar E. Iglesias --- target/arm/internals.h | 2 ++ target/arm/op_helper.c | 10 +++++----- 2 files changed, 7 insertions(+), 5 deletions(-) (limited to 'target') diff --git a/target/arm/internals.h b/target/arm/internals.h index bb06946607..461f55859b 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -448,12 +448,14 @@ void arm_handle_psci_call(ARMCPU *cpu); * @s2addr: Address that caused a fault at stage 2 * @stage2: True if we faulted at stage 2 * @s1ptw: True if we faulted at stage 2 while doing a stage 1 page-table walk + * @ea: True if we should set the EA (external abort type) bit in syndrome */ typedef struct ARMMMUFaultInfo ARMMMUFaultInfo; struct ARMMMUFaultInfo { target_ulong s2addr; bool stage2; bool s1ptw; + bool ea; }; /* Do a page table walk and add page to TLB if possible */ diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index 6114597fe4..8f6db8043f 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -80,7 +80,7 @@ uint32_t HELPER(neon_tbl)(CPUARMState *env, uint32_t ireg, uint32_t def, static inline uint32_t merge_syn_data_abort(uint32_t template_syn, unsigned int target_el, - bool same_el, + bool same_el, bool ea, bool s1ptw, bool is_write, int fsc) { @@ -99,7 +99,7 @@ static inline uint32_t merge_syn_data_abort(uint32_t template_syn, */ if (!(template_syn & ARM_EL_ISV) || target_el != 2 || s1ptw) { syn = syn_data_abort_no_iss(same_el, - 0, 0, s1ptw, is_write, fsc); + ea, 0, s1ptw, is_write, fsc); } else { /* Fields: IL, ISV, SAS, SSE, SRT, SF and AR come from the template * syndrome created at translation time. @@ -107,7 +107,7 @@ static inline uint32_t merge_syn_data_abort(uint32_t template_syn, */ syn = syn_data_abort_with_iss(same_el, 0, 0, 0, 0, 0, - 0, 0, s1ptw, is_write, fsc, + ea, 0, s1ptw, is_write, fsc, false); /* Merge the runtime syndrome with the template syndrome. */ syn |= template_syn; @@ -141,11 +141,11 @@ static void deliver_fault(ARMCPU *cpu, vaddr addr, MMUAccessType access_type, } if (access_type == MMU_INST_FETCH) { - syn = syn_insn_abort(same_el, 0, fi->s1ptw, fsc); + syn = syn_insn_abort(same_el, fi->ea, fi->s1ptw, fsc); exc = EXCP_PREFETCH_ABORT; } else { syn = merge_syn_data_abort(env->exception.syndrome, target_el, - same_el, fi->s1ptw, + same_el, fi->ea, fi->s1ptw, access_type == MMU_DATA_STORE, fsc); if (access_type == MMU_DATA_STORE -- cgit 1.4.1 From 3e4d91b94ce400326fae0850578d9e9f30a71adb Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 4 Sep 2017 15:21:56 +0100 Subject: target/arm: Fix aa64 ldp register writeback For "ldp x0, x1, [x0]", if the second load is on a second page and the second page is unmapped, the exception would be raised with x0 already modified. This means the instruction couldn't be restarted. Cc: qemu-arm@nongnu.org Cc: qemu-stable@nongnu.org Reported-by: Andrew Signed-off-by: Richard Henderson Message-id: 20170825224833.4463-1-richard.henderson@linaro.org Fixes: https://bugs.launchpad.net/qemu/+bug/1713066 Signed-off-by: Richard Henderson [PMM: tweaked comment format] Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- target/arm/translate-a64.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) (limited to 'target') diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index 2200e25be0..cb44632d16 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -2217,29 +2217,34 @@ static void disas_ldst_pair(DisasContext *s, uint32_t insn) } else { do_fp_st(s, rt, tcg_addr, size); } - } else { - TCGv_i64 tcg_rt = cpu_reg(s, rt); - if (is_load) { - do_gpr_ld(s, tcg_rt, tcg_addr, size, is_signed, false, - false, 0, false, false); - } else { - do_gpr_st(s, tcg_rt, tcg_addr, size, - false, 0, false, false); - } - } - tcg_gen_addi_i64(tcg_addr, tcg_addr, 1 << size); - if (is_vector) { + tcg_gen_addi_i64(tcg_addr, tcg_addr, 1 << size); if (is_load) { do_fp_ld(s, rt2, tcg_addr, size); } else { do_fp_st(s, rt2, tcg_addr, size); } } else { + TCGv_i64 tcg_rt = cpu_reg(s, rt); TCGv_i64 tcg_rt2 = cpu_reg(s, rt2); + if (is_load) { + TCGv_i64 tmp = tcg_temp_new_i64(); + + /* Do not modify tcg_rt before recognizing any exception + * from the second load. + */ + do_gpr_ld(s, tmp, tcg_addr, size, is_signed, false, + false, 0, false, false); + tcg_gen_addi_i64(tcg_addr, tcg_addr, 1 << size); do_gpr_ld(s, tcg_rt2, tcg_addr, size, is_signed, false, false, 0, false, false); + + tcg_gen_mov_i64(tcg_rt, tmp); + tcg_temp_free_i64(tmp); } else { + do_gpr_st(s, tcg_rt, tcg_addr, size, + false, 0, false, false); + tcg_gen_addi_i64(tcg_addr, tcg_addr, 1 << size); do_gpr_st(s, tcg_rt2, tcg_addr, size, false, 0, false, false); } -- cgit 1.4.1