First arm pullreq of the 2.11 cycle. I know I still have some more stuff on my queue to review, but 36 patches is big enough as it is; I expect I'll do another pull later this week. thanks -- PMM The following changes since commit 32f0f68bb77289b75a82925f712bb52e16eac3ba: Merge remote-tracking branch 'remotes/ehabkost/tags/x86-and-machine-pull-request' into staging (2017-09-01 17:28:54 +0100) are available in the git repository at: git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20170904 for you to fetch changes up to 1e35c4ce33a94cf78dbf639695cb877ef35920b0: arm_gicv3_kvm: Fix compile warning (2017-09-04 12:09:32 +0100) ---------------------------------------------------------------- target-arm: * collection of M profile cleanups and minor bugfixes * loader: handle ELF files with overlapping zero-init data * virt: allow PMU instantiation with userspace irqchip * wdt_aspeed: Add support for the reset width register * cpu: Define new cpu_transaction_failed() hook * arm: Support generating CPU exceptions on memory transaction failures (bus faults) * Mark some SoC devices as not user-creatable * arm: Fix aa64 ldp register writeback * arm_gicv3_kvm: Fix compile warning ---------------------------------------------------------------- Andrew Jeffery (2): watchdog: wdt_aspeed: Add support for the reset width register aspeed_soc: Propagate silicon-rev to watchdog Andrew Jones (4): hw/arm/virt: add pmu interrupt state target/arm/kvm: pmu: split init and set-irq stages hw/arm/virt: allow pmu instantiation with userspace irqchip target/arm/kvm: pmu: improve error handling Peter Maydell (25): target/arm: Use MMUAccessType enum rather than int target/arm: Don't trap WFI/WFE for M profile target/arm: Consolidate PMSA handling in get_phys_addr() target/arm: Tighten up Thumb decode where new v8M insns will be hw/intc/armv7m_nvic.c: Remove out of date comment target/arm: Remove incorrect comment about MPU_CTRL target/arm: Fix outdated comment about exception exit target/arm: Define and use XPSR bit masks target/arm: Don't store M profile PRIMASK and FAULTMASK in daif target/arm: Don't use cpsr_write/cpsr_read to transfer M profile XPSR target/arm: Make arm_cpu_dump_state() handle the M-profile XPSR target/arm: Don't calculate lr in arm_v7m_cpu_do_interrupt() until needed target/arm: Create and use new function arm_v7m_is_handler_mode() armv7m_nvic.h: Move from include/hw/arm to include/hw/intc nvic: Implement "user accesses BusFault" SCS region behaviour loader: Handle ELF files with overlapping zero-initialized data loader: Ignore zero-sized ELF segments memory.h: Move MemTxResult type to memattrs.h cpu: Define new cpu_transaction_failed() hook cputlb: Support generating CPU exceptions on memory transaction failures boards.h: Define new flag ignore_memory_transaction_failures hw/arm: Set ignore_memory_transaction_failures for most ARM boards target/arm: Factor out fault delivery code target/arm: Allow deliver_fault() caller to specify EA bit target/arm: Implement new do_transaction_failed hook Philippe Mathieu-Daudé (1): hw/arm: use defined type name instead of hard-coded string Pranith Kumar (1): arm_gicv3_kvm: Fix compile warning Richard Henderson (1): target/arm: Fix aa64 ldp register writeback Thomas Huth (2): hw/arm/aspeed_soc: Mark devices as user_creatable = false hw/arm/digic: Mark device with user_creatable = false include/exec/memattrs.h | 10 +++ include/exec/memory.h | 10 --- include/hw/arm/armv7m.h | 2 +- include/hw/boards.h | 11 +++ include/hw/elf_ops.h | 72 +++++++++++++-- include/hw/{arm => intc}/armv7m_nvic.h | 0 include/hw/watchdog/wdt_aspeed.h | 2 + include/qom/cpu.h | 27 ++++++ softmmu_template.h | 4 +- target/arm/cpu.h | 56 +++++++++--- target/arm/internals.h | 15 +++- target/arm/kvm_arm.h | 9 +- accel/tcg/cputlb.c | 32 ++++++- hw/arm/armv7m.c | 4 +- hw/arm/aspeed.c | 3 + hw/arm/aspeed_soc.c | 4 + hw/arm/collie.c | 1 + hw/arm/cubieboard.c | 1 + hw/arm/digic.c | 2 + hw/arm/digic_boards.c | 1 + hw/arm/exynos4210.c | 4 +- hw/arm/exynos4_boards.c | 2 + hw/arm/gumstix.c | 2 + hw/arm/highbank.c | 13 ++- hw/arm/imx25_pdk.c | 1 + hw/arm/integratorcp.c | 1 + hw/arm/kzm.c | 1 + hw/arm/mainstone.c | 1 + hw/arm/musicpal.c | 1 + hw/arm/netduino2.c | 1 + hw/arm/nseries.c | 2 + hw/arm/omap_sx1.c | 2 + hw/arm/palm.c | 1 + hw/arm/raspi.c | 1 + hw/arm/realview.c | 10 ++- hw/arm/sabrelite.c | 1 + hw/arm/spitz.c | 4 + hw/arm/stellaris.c | 2 + hw/arm/tosa.c | 1 + hw/arm/versatilepb.c | 2 + hw/arm/vexpress.c | 7 +- hw/arm/virt.c | 12 ++- hw/arm/xilinx_zynq.c | 15 ++-- hw/arm/xlnx-ep108.c | 2 + hw/arm/z2.c | 1 + hw/intc/arm_gicv3_kvm.c | 2 +- hw/intc/armv7m_nvic.c | 68 +++++++++----- hw/watchdog/wdt_aspeed.c | 93 ++++++++++++++++--- qom/cpu.c | 7 ++ target/arm/cpu.c | 8 +- target/arm/helper.c | 124 ++++++++++++------------- target/arm/kvm.c | 6 +- target/arm/kvm32.c | 7 +- target/arm/kvm64.c | 63 +++++++------ target/arm/machine.c | 54 ++++++++++- target/arm/op_helper.c | 160 ++++++++++++++++++++++----------- target/arm/translate-a64.c | 29 +++--- target/arm/translate.c | 106 ++++++++++++++++------ 58 files changed, 795 insertions(+), 288 deletions(-) rename include/hw/{arm => intc}/armv7m_nvic.h (100%) 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 <peter.maydell@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1501692241-23310-2-git-send-email-peter.maydell@linaro.org --- target/arm/internals.h | 3 ++- target/arm/helper.c | 30 +++++++++++++++--------------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ 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 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 @@ #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); @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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); @@ -XXX,XX +XXX,XX @@ 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); @@ -XXX,XX +XXX,XX @@ 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); @@ -XXX,XX +XXX,XX @@ 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) @@ -XXX,XX +XXX,XX @@ 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) @@ -XXX,XX +XXX,XX @@ 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) && @@ -XXX,XX +XXX,XX @@ 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) @@ -XXX,XX +XXX,XX @@ 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); @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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) @@ -XXX,XX +XXX,XX @@ 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); -- 2.7.4 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 <peter.maydell@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1501692241-23310-3-git-send-email-peter.maydell@linaro.org --- target/arm/op_helper.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -XXX,XX +XXX,XX @@ 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. */ -- 2.7.4 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 <peter.maydell@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1501692241-23310-4-git-send-email-peter.maydell@linaro.org --- target/arm/helper.c | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) 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_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]; @@ -XXX,XX +XXX,XX @@ 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"), @@ -XXX,XX +XXX,XX @@ 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); -- 2.7.4 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 <peter.maydell@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1501692241-23310-5-git-send-email-peter.maydell@linaro.org --- target/arm/translate.c | 48 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ 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 { @@ -XXX,XX +XXX,XX @@ 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); @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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); @@ -XXX,XX +XXX,XX @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s) } break; } + } break; } -- 2.7.4 Remove an out of date comment which says there's only one item in the NVIC container region -- we put systick into its own device object a while back and so now there are two things in the container. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1501692241-23310-6-git-send-email-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static void armv7m_nvic_realize(DeviceState *dev, Error **errp) * 0xd00..0xd3c - SCS registers * 0xd40..0xeff - Reserved or Not implemented * 0xf00 - STIR - * - * At the moment there is only one thing in the container region, - * but we leave it in place to allow us to pull systick out into - * its own device object later. */ memory_region_init(&s->container, OBJECT(s), "nvic", 0x1000); /* The system register region goes at the bottom of the priority -- 2.7.4 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 <peter.maydell@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1501692241-23310-7-git-send-email-peter.maydell@linaro.org --- target/arm/cpu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ 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; -- 2.7.4 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 <peter.maydell@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1501692241-23310-8-git-send-email-peter.maydell@linaro.org --- target/arm/helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 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()). */ -- 2.7.4 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 <peter.maydell@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> 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(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ 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) @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ 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); @@ -XXX,XX +XXX,XX @@ 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 @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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); } -- 2.7.4 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 <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1501692241-23310-10-git-send-email-peter.maydell@linaro.org --- target/arm/cpu.h | 4 +++- hw/intc/armv7m_nvic.c | 4 ++-- target/arm/cpu.c | 5 ----- target/arm/helper.c | 18 +++++------------- target/arm/machine.c | 33 +++++++++++++++++++++++++++++++++ 5 files changed, 43 insertions(+), 21 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ 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: @@ -XXX,XX +XXX,XX @@ 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/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -XXX,XX +XXX,XX @@ 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/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 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)) { @@ -XXX,XX +XXX,XX @@ 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); @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -XXX,XX +XXX,XX @@ 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, @@ -XXX,XX +XXX,XX @@ 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 } }; @@ -XXX,XX +XXX,XX @@ 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)) { -- 2.7.4 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 <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1501692241-23310-11-git-send-email-peter.maydell@linaro.org --- target/arm/machine.c | 49 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/target/arm/machine.c b/target/arm/machine.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/machine.c +++ b/target/arm/machine.c @@ -XXX,XX +XXX,XX @@ 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); @@ -XXX,XX +XXX,XX @@ 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); -- 2.7.4 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 <peter.maydell@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1501692241-23310-12-git-send-email-peter.maydell@linaro.org --- target/arm/translate.c | 58 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 18 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ 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); @@ -XXX,XX +XXX,XX @@ 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; -- 2.7.4 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 <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1501692241-23310-13-git-send-email-peter.maydell@linaro.org --- target/arm/helper.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) 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 @@ 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) { @@ -XXX,XX +XXX,XX @@ 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); -- 2.7.4 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 <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> 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(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ 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)) { @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ 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 @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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; } @@ -XXX,XX +XXX,XX @@ 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; -- 2.7.4 The armv7m_nvic.h header file was accidentally placed in include/hw/arm; move it to include/hw/intc to match where its corresponding .c file lives. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1501692241-23310-15-git-send-email-peter.maydell@linaro.org --- include/hw/arm/armv7m.h | 2 +- include/hw/{arm => intc}/armv7m_nvic.h | 0 hw/intc/armv7m_nvic.c | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename include/hw/{arm => intc}/armv7m_nvic.h (100%) diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/armv7m.h +++ b/include/hw/arm/armv7m.h @@ -XXX,XX +XXX,XX @@ #define HW_ARM_ARMV7M_H #include "hw/sysbus.h" -#include "hw/arm/armv7m_nvic.h" +#include "hw/intc/armv7m_nvic.h" #define TYPE_BITBAND "ARM,bitband-memory" #define BITBAND(obj) OBJECT_CHECK(BitBandState, (obj), TYPE_BITBAND) diff --git a/include/hw/arm/armv7m_nvic.h b/include/hw/intc/armv7m_nvic.h similarity index 100% rename from include/hw/arm/armv7m_nvic.h rename to include/hw/intc/armv7m_nvic.h diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ #include "hw/sysbus.h" #include "qemu/timer.h" #include "hw/arm/arm.h" -#include "hw/arm/armv7m_nvic.h" +#include "hw/intc/armv7m_nvic.h" #include "target/arm/cpu.h" #include "exec/exec-all.h" #include "qemu/log.h" -- 2.7.4 The ARMv7M architecture specifies that most of the addresses in the PPB region (which includes the NVIC, systick and system registers) are not accessible to unprivileged accesses, which should BusFault with a few exceptions: * the STIR is configurably user-accessible * the ITM (which we don't implement at all) is always user-accessible Implement this by switching the register access functions to the _with_attrs scheme that lets us distinguish user mode accesses. This allows us to pull the handling of the CCR.USERSETMPEND flag up to the level where we can make it generate a BusFault as it should for non-permitted accesses. Note that until the core ARM CPU code implements turning MEMTX_ERROR into a BusFault the registers will continue to act as RAZ/WI to user accesses. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1501692241-23310-16-git-send-email-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 58 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- 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) } case 0xf00: /* Software Triggered Interrupt Register */ { - /* user mode can only write to STIR if CCR.USERSETMPEND permits it */ int excnum = (value & 0x1ff) + NVIC_FIRST_IRQ; - if (excnum < s->num_irq && - (arm_current_el(&cpu->env) || - (cpu->env.v7m.ccr & R_V7M_CCR_USERSETMPEND_MASK))) { + if (excnum < s->num_irq) { armv7m_nvic_set_pending(s, excnum); } break; @@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value) } } -static uint64_t nvic_sysreg_read(void *opaque, hwaddr addr, - unsigned size) +static bool nvic_user_access_ok(NVICState *s, hwaddr offset) +{ + /* Return true if unprivileged access to this register is permitted. */ + switch (offset) { + case 0xf00: /* STIR: accessible only if CCR.USERSETMPEND permits */ + return s->cpu->env.v7m.ccr & R_V7M_CCR_USERSETMPEND_MASK; + default: + /* All other user accesses cause a BusFault unconditionally */ + return false; + } +} + +static MemTxResult nvic_sysreg_read(void *opaque, hwaddr addr, + uint64_t *data, unsigned size, + MemTxAttrs attrs) { NVICState *s = (NVICState *)opaque; uint32_t offset = addr; unsigned i, startvec, end; uint32_t val; + if (attrs.user && !nvic_user_access_ok(s, addr)) { + /* Generate BusFault for unprivileged accesses */ + return MEMTX_ERROR; + } + switch (offset) { /* reads of set and clear both return the status */ case 0x100 ... 0x13f: /* NVIC Set enable */ @@ -XXX,XX +XXX,XX @@ static uint64_t nvic_sysreg_read(void *opaque, hwaddr addr, } trace_nvic_sysreg_read(addr, val, size); - return val; + *data = val; + return MEMTX_OK; } -static void nvic_sysreg_write(void *opaque, hwaddr addr, - uint64_t value, unsigned size) +static MemTxResult nvic_sysreg_write(void *opaque, hwaddr addr, + uint64_t value, unsigned size, + MemTxAttrs attrs) { NVICState *s = (NVICState *)opaque; uint32_t offset = addr; @@ -XXX,XX +XXX,XX @@ static void nvic_sysreg_write(void *opaque, hwaddr addr, trace_nvic_sysreg_write(addr, value, size); + if (attrs.user && !nvic_user_access_ok(s, addr)) { + /* Generate BusFault for unprivileged accesses */ + return MEMTX_ERROR; + } + switch (offset) { case 0x100 ... 0x13f: /* NVIC Set enable */ offset += 0x80; @@ -XXX,XX +XXX,XX @@ static void nvic_sysreg_write(void *opaque, hwaddr addr, } } nvic_irq_update(s); - return; + return MEMTX_OK; case 0x200 ... 0x23f: /* NVIC Set pend */ /* the special logic in armv7m_nvic_set_pending() * is not needed since IRQs are never escalated @@ -XXX,XX +XXX,XX @@ static void nvic_sysreg_write(void *opaque, hwaddr addr, } } nvic_irq_update(s); - return; + return MEMTX_OK; case 0x300 ... 0x33f: /* NVIC Active */ - return; /* R/O */ + return MEMTX_OK; /* R/O */ case 0x400 ... 0x5ef: /* NVIC Priority */ startvec = 8 * (offset - 0x400) + NVIC_FIRST_IRQ; /* vector # */ @@ -XXX,XX +XXX,XX @@ static void nvic_sysreg_write(void *opaque, hwaddr addr, set_prio(s, startvec + i, (value >> (i * 8)) & 0xff); } nvic_irq_update(s); - return; + return MEMTX_OK; case 0xd18 ... 0xd23: /* System Handler Priority. */ for (i = 0; i < size; i++) { unsigned hdlidx = (offset - 0xd14) + i; set_prio(s, hdlidx, (value >> (i * 8)) & 0xff); } nvic_irq_update(s); - return; + return MEMTX_OK; } if (size == 4) { nvic_writel(s, offset, value); - return; + return MEMTX_OK; } qemu_log_mask(LOG_GUEST_ERROR, "NVIC: Bad write of size %d at offset 0x%x\n", size, offset); + /* This is UNPREDICTABLE; treat as RAZ/WI */ + return MEMTX_OK; } static const MemoryRegionOps nvic_sysreg_ops = { - .read = nvic_sysreg_read, - .write = nvic_sysreg_write, + .read_with_attrs = nvic_sysreg_read, + .write_with_attrs = nvic_sysreg_write, .endianness = DEVICE_NATIVE_ENDIAN, }; -- 2.7.4 For embedded systems, notably ARM, one common use of ELF file segments is that the 'physical addresses' represent load addresses and the 'virtual addresses' execution addresses, such that the load addresses are packed into ROM or flash, and the relocation and zero-initialization of data is done at runtime. This means that the 'memsz' in the segment header represents the runtime size of the segment, but the size that needs to be loaded is only the 'filesz'. In particular, paddr+memsz may overlap with the next segment to be loaded, as in this example: 0x70000001 off 0x00007f68 vaddr 0x00008150 paddr 0x00008150 align 2**2 filesz 0x00000008 memsz 0x00000008 flags r-- LOAD off 0x000000f4 vaddr 0x00000000 paddr 0x00000000 align 2**2 filesz 0x00000124 memsz 0x00000124 flags r-- LOAD off 0x00000218 vaddr 0x00000400 paddr 0x00000400 align 2**3 filesz 0x00007d58 memsz 0x00007d58 flags r-x LOAD off 0x00007f70 vaddr 0x20000140 paddr 0x00008158 align 2**3 filesz 0x00000a80 memsz 0x000022f8 flags rw- LOAD off 0x000089f0 vaddr 0x20002438 paddr 0x00008bd8 align 2**0 filesz 0x00000000 memsz 0x00004000 flags rw- LOAD off 0x000089f0 vaddr 0x20000000 paddr 0x20000000 align 2**0 filesz 0x00000000 memsz 0x00000140 flags rw- where the segment at paddr 0x8158 has a memsz of 0x2258 and would overlap with the segment at paddr 0x8bd8 if QEMU's loader tried to honour it. (At runtime the segments will not overlap since their vaddrs are more widely spaced than their paddrs.) Currently if you try to load an ELF file like this with QEMU then it will fail with an error "rom: requested regions overlap", because we create a ROM image for each segment using the memsz as the size. Support ELF files using this scheme, by truncating the zero-initialized part of the segment if it would overlap another segment. This will retain the existing loader behaviour for all ELF files we currently accept, and also accept ELF files which only need 'filesz' bytes to be loaded. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1502116754-18867-2-git-send-email-peter.maydell@linaro.org --- include/hw/elf_ops.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/elf_ops.h +++ b/include/hw/elf_ops.h @@ -XXX,XX +XXX,XX @@ static int glue(load_elf, SZ)(const char *name, int fd, goto fail; } } + + /* The ELF spec is somewhat vague about the purpose of the + * physical address field. One common use in the embedded world + * is that physical address field specifies the load address + * and the virtual address field specifies the execution address. + * Segments are packed into ROM or flash, and the relocation + * and zero-initialization of data is done at runtime. This + * means that the memsz header represents the runtime size of the + * segment, but the filesz represents the loadtime size. If + * we try to honour the memsz value for an ELF file like this + * we will end up with overlapping segments (which the + * loader.c code will later reject). + * We support ELF files using this scheme by by checking whether + * paddr + memsz for this segment would overlap with any other + * segment. If so, then we assume it's using this scheme and + * truncate the loaded segment to the filesz size. + * If the segment considered as being memsz size doesn't overlap + * then we use memsz for the segment length, to handle ELF files + * which assume that the loader will do the zero-initialization. + */ + if (mem_size > file_size) { + /* If this segment's zero-init portion overlaps another + * segment's data or zero-init portion, then truncate this one. + * Invalid ELF files where the segments overlap even when + * only file_size bytes are loaded will be rejected by + * the ROM overlap check in loader.c, so we don't try to + * explicitly detect those here. + */ + int j; + elf_word zero_start = ph->p_paddr + file_size; + elf_word zero_end = ph->p_paddr + mem_size; + + for (j = 0; j < ehdr.e_phnum; j++) { + struct elf_phdr *jph = &phdr[j]; + + if (i != j && jph->p_type == PT_LOAD) { + elf_word other_start = jph->p_paddr; + elf_word other_end = jph->p_paddr + jph->p_memsz; + + if (!(other_start >= zero_end || + zero_start >= other_end)) { + mem_size = file_size; + break; + } + } + } + } + /* address_offset is hack for kernel images that are linked at the wrong physical address. */ if (translate_fn) { -- 2.7.4 Some ELF files have program headers that specify segments that are of zero size. Ignore them, rather than trying to create zero-length ROM blobs for them, because the zero-length blob can falsely trigger the overlapping-ROM-blobs check. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: Hua Yanghao <huayanghao@gmail.com> Message-id: 1502116754-18867-3-git-send-email-peter.maydell@linaro.org --- include/hw/elf_ops.h | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/elf_ops.h +++ b/include/hw/elf_ops.h @@ -XXX,XX +XXX,XX @@ static int glue(load_elf, SZ)(const char *name, int fd, *pentry = ehdr.e_entry - ph->p_vaddr + ph->p_paddr; } - if (load_rom) { - snprintf(label, sizeof(label), "phdr #%d: %s", i, name); - - /* rom_add_elf_program() seize the ownership of 'data' */ - rom_add_elf_program(label, data, file_size, mem_size, addr, as); - } else { - cpu_physical_memory_write(addr, data, file_size); + if (mem_size == 0) { + /* Some ELF files really do have segments of zero size; + * just ignore them rather than trying to create empty + * ROM blobs, because the zero-length blob can falsely + * trigger the overlapping-ROM-blobs check. + */ g_free(data); + } else { + if (load_rom) { + snprintf(label, sizeof(label), "phdr #%d: %s", i, name); + + /* rom_add_elf_program() seize the ownership of 'data' */ + rom_add_elf_program(label, data, file_size, mem_size, + addr, as); + } else { + cpu_physical_memory_write(addr, data, file_size); + g_free(data); + } } total_size += mem_size; -- 2.7.4 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/armv7m.c | 4 ++-- hw/arm/exynos4210.c | 4 ++-- hw/arm/highbank.c | 11 +++++++---- hw/arm/realview.c | 6 ++++-- hw/arm/vexpress.c | 6 ++++-- hw/arm/xilinx_zynq.c | 14 ++++++++------ 6 files changed, 27 insertions(+), 18 deletions(-) diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/armv7m.c +++ b/hw/arm/armv7m.c @@ -XXX,XX +XXX,XX @@ static void armv7m_instance_init(Object *obj) &error_abort); memory_region_init(&s->container, obj, "armv7m-container", UINT64_MAX); - object_initialize(&s->nvic, sizeof(s->nvic), "armv7m_nvic"); + object_initialize(&s->nvic, sizeof(s->nvic), TYPE_NVIC); qdev_set_parent_bus(DEVICE(&s->nvic), sysbus_get_default()); object_property_add_alias(obj, "num-irq", OBJECT(&s->nvic), "num-irq", &error_abort); @@ -XXX,XX +XXX,XX @@ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, cpu_model = "cortex-m3"; } - armv7m = qdev_create(NULL, "armv7m"); + armv7m = qdev_create(NULL, TYPE_ARMV7M); qdev_prop_set_uint32(armv7m, "num-irq", num_irq); qdev_prop_set_string(armv7m, "cpu-model", cpu_model); object_property_set_link(OBJECT(armv7m), OBJECT(get_system_memory()), diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/exynos4210.c +++ b/hw/arm/exynos4210.c @@ -XXX,XX +XXX,XX @@ #include "hw/arm/arm.h" #include "hw/loader.h" #include "hw/arm/exynos4210.h" -#include "hw/sd/sd.h" +#include "hw/sd/sdhci.h" #include "hw/usb/hcd-ehci.h" #define EXYNOS4210_CHIPID_ADDR 0x10000000 @@ -XXX,XX +XXX,XX @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem) BlockBackend *blk; DriveInfo *di; - dev = qdev_create(NULL, "generic-sdhci"); + dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI); qdev_prop_set_uint32(dev, "capareg", EXYNOS4210_SDHCI_CAPABILITIES); qdev_init_nofail(dev); diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/highbank.c +++ b/hw/arm/highbank.c @@ -XXX,XX +XXX,XX @@ #include "exec/address-spaces.h" #include "qemu/error-report.h" #include "hw/char/pl011.h" +#include "hw/ide/ahci.h" +#include "hw/cpu/a9mpcore.h" +#include "hw/cpu/a15mpcore.h" #define SMP_BOOT_ADDR 0x100 #define SMP_BOOT_REG 0x40 @@ -XXX,XX +XXX,XX @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id) busdev = SYS_BUS_DEVICE(dev); sysbus_mmio_map(busdev, 0, 0xfff12000); - dev = qdev_create(NULL, "a9mpcore_priv"); + dev = qdev_create(NULL, TYPE_A9MPCORE_PRIV); break; case CALXEDA_MIDWAY: - dev = qdev_create(NULL, "a15mpcore_priv"); + dev = qdev_create(NULL, TYPE_A15MPCORE_PRIV); break; } qdev_prop_set_uint32(dev, "num-cpu", smp_cpus); @@ -XXX,XX +XXX,XX @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id) sysbus_connect_irq(busdev, 0, pic[18]); pl011_create(0xfff36000, pic[20], serial_hds[0]); - dev = qdev_create(NULL, "highbank-regs"); + dev = qdev_create(NULL, TYPE_HIGHBANK_REGISTERS); qdev_init_nofail(dev); busdev = SYS_BUS_DEVICE(dev); sysbus_mmio_map(busdev, 0, 0xfff3c000); @@ -XXX,XX +XXX,XX @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id) sysbus_create_simple("pl031", 0xfff35000, pic[19]); sysbus_create_simple("pl022", 0xfff39000, pic[23]); - sysbus_create_simple("sysbus-ahci", 0xffe08000, pic[83]); + sysbus_create_simple(TYPE_SYSBUS_AHCI, 0xffe08000, pic[83]); if (nd_table[0].used) { qemu_check_nic_model(&nd_table[0], "xgmac"); diff --git a/hw/arm/realview.c b/hw/arm/realview.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/realview.c +++ b/hw/arm/realview.c @@ -XXX,XX +XXX,XX @@ #include "exec/address-spaces.h" #include "qemu/error-report.h" #include "hw/char/pl011.h" +#include "hw/cpu/a9mpcore.h" +#include "hw/intc/realview_gic.h" #define SMP_BOOT_ADDR 0xe0000000 #define SMP_BOOTREG_ADDR 0x10000030 @@ -XXX,XX +XXX,XX @@ static void realview_init(MachineState *machine, sysbus_mmio_map(SYS_BUS_DEVICE(sysctl), 0, 0x10000000); if (is_mpcore) { - dev = qdev_create(NULL, is_pb ? "a9mpcore_priv": "realview_mpcore"); + dev = qdev_create(NULL, is_pb ? TYPE_A9MPCORE_PRIV : "realview_mpcore"); qdev_prop_set_uint32(dev, "num-cpu", smp_cpus); qdev_init_nofail(dev); busdev = SYS_BUS_DEVICE(dev); @@ -XXX,XX +XXX,XX @@ static void realview_init(MachineState *machine, } else { uint32_t gic_addr = is_pb ? 0x1e000000 : 0x10040000; /* For now just create the nIRQ GIC, and ignore the others. */ - dev = sysbus_create_simple("realview_gic", gic_addr, cpu_irq[0]); + dev = sysbus_create_simple(TYPE_REALVIEW_GIC, gic_addr, cpu_irq[0]); } for (n = 0; n < 64; n++) { pic[n] = qdev_get_gpio_in(dev, n); diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/vexpress.c +++ b/hw/arm/vexpress.c @@ -XXX,XX +XXX,XX @@ #include "qemu/error-report.h" #include <libfdt.h> #include "hw/char/pl011.h" +#include "hw/cpu/a9mpcore.h" +#include "hw/cpu/a15mpcore.h" #define VEXPRESS_BOARD_ID 0x8e0 #define VEXPRESS_FLASH_SIZE (64 * 1024 * 1024) @@ -XXX,XX +XXX,XX @@ static void a9_daughterboard_init(const VexpressMachineState *vms, memory_region_add_subregion(sysmem, 0x60000000, ram); /* 0x1e000000 A9MPCore (SCU) private memory region */ - init_cpus(cpu_model, "a9mpcore_priv", 0x1e000000, pic, vms->secure); + init_cpus(cpu_model, TYPE_A9MPCORE_PRIV, 0x1e000000, pic, vms->secure); /* Daughterboard peripherals : 0x10020000 .. 0x20000000 */ @@ -XXX,XX +XXX,XX @@ static void a15_daughterboard_init(const VexpressMachineState *vms, memory_region_add_subregion(sysmem, 0x80000000, ram); /* 0x2c000000 A15MPCore private memory region (GIC) */ - init_cpus(cpu_model, "a15mpcore_priv", 0x2c000000, pic, vms->secure); + init_cpus(cpu_model, TYPE_A15MPCORE_PRIV, 0x2c000000, pic, vms->secure); /* A15 daughterboard peripherals: */ diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/xilinx_zynq.c +++ b/hw/arm/xilinx_zynq.c @@ -XXX,XX +XXX,XX @@ #include "hw/misc/zynq-xadc.h" #include "hw/ssi/ssi.h" #include "qemu/error-report.h" -#include "hw/sd/sd.h" +#include "hw/sd/sdhci.h" #include "hw/char/cadence_uart.h" +#include "hw/net/cadence_gem.h" +#include "hw/cpu/a9mpcore.h" #define NUM_SPI_FLASHES 4 #define NUM_QSPI_FLASHES 2 @@ -XXX,XX +XXX,XX @@ static void gem_init(NICInfo *nd, uint32_t base, qemu_irq irq) DeviceState *dev; SysBusDevice *s; - dev = qdev_create(NULL, "cadence_gem"); + dev = qdev_create(NULL, TYPE_CADENCE_GEM); if (nd->used) { - qemu_check_nic_model(nd, "cadence_gem"); + qemu_check_nic_model(nd, TYPE_CADENCE_GEM); qdev_set_nic_properties(dev, nd); } qdev_init_nofail(dev); @@ -XXX,XX +XXX,XX @@ static void zynq_init(MachineState *machine) qdev_init_nofail(dev); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8000000); - dev = qdev_create(NULL, "a9mpcore_priv"); + dev = qdev_create(NULL, TYPE_A9MPCORE_PRIV); qdev_prop_set_uint32(dev, "num-cpu", 1); qdev_init_nofail(dev); busdev = SYS_BUS_DEVICE(dev); @@ -XXX,XX +XXX,XX @@ static void zynq_init(MachineState *machine) gem_init(&nd_table[0], 0xE000B000, pic[54-IRQ_OFFSET]); gem_init(&nd_table[1], 0xE000C000, pic[77-IRQ_OFFSET]); - dev = qdev_create(NULL, "generic-sdhci"); + dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI); qdev_init_nofail(dev); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0100000); sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[56-IRQ_OFFSET]); @@ -XXX,XX +XXX,XX @@ static void zynq_init(MachineState *machine) qdev_prop_set_drive(carddev, "drive", blk, &error_fatal); object_property_set_bool(OBJECT(carddev), true, "realized", &error_fatal); - dev = qdev_create(NULL, "generic-sdhci"); + dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI); qdev_init_nofail(dev); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0101000); sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[79-IRQ_OFFSET]); -- 2.7.4 From: Andrew Jones <drjones@redhat.com> Mimicking gicv3-maintenance-interrupt, add the PMU's interrupt to CPU state. Signed-off-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 1500471597-2517-2-git-send-email-drjones@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 2 ++ hw/arm/virt.c | 3 +++ target/arm/cpu.c | 2 ++ 3 files changed, 7 insertions(+) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ 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; diff --git a/hw/arm/virt.c b/hw/arm/virt.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -XXX,XX +XXX,XX @@ 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' -- 2.7.4 From: Andrew Jones <drjones@redhat.com> 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 <drjones@redhat.com> Reviewed-by: Christoffer Dall <cdall@linaro.org> Message-id: 1500471597-2517-3-git-send-email-drjones@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/kvm_arm.h | 10 ++++++++-- hw/arm/virt.c | 11 +++++++++-- target/arm/kvm32.c | 8 +++++++- target/arm/kvm64.c | 52 +++++++++++++++++++++++++--------------------------- 4 files changed, 49 insertions(+), 32 deletions(-) diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm_arm.h +++ b/target/arm/kvm_arm.h @@ -XXX,XX +XXX,XX @@ 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 @@ -XXX,XX +XXX,XX @@ 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; } diff --git a/hw/arm/virt.c b/hw/arm/virt.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm32.c +++ b/target/arm/kvm32.c @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -XXX,XX +XXX,XX @@ 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) -- 2.7.4 From: Andrew Jones <drjones@redhat.com> 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 <drjones@redhat.com> Reviewed-by: Christoffer Dall <cdall@linaro.org> Message-id: 1500471597-2517-4-git-send-email-drjones@redhat.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt.c | 3 ++- target/arm/kvm.c | 6 +++++- target/arm/kvm64.c | 3 +-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -XXX,XX +XXX,XX @@ 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) { -- 2.7.4 From: Andrew Jones <drjones@redhat.com> 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 <drjones@redhat.com> Reviewed-by: Christoffer Dall <cdall@linaro.org> Message-id: 1500471597-2517-5-git-send-email-drjones@redhat.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/kvm_arm.h | 15 ++++----------- hw/arm/virt.c | 9 +++------ target/arm/kvm32.c | 3 +-- target/arm/kvm64.c | 28 ++++++++++++++++++++-------- 4 files changed, 28 insertions(+), 27 deletions(-) diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm_arm.h +++ b/target/arm/kvm_arm.h @@ -XXX,XX +XXX,XX @@ 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 @@ -XXX,XX +XXX,XX @@ 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 diff --git a/hw/arm/virt.c b/hw/arm/virt.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm32.c +++ b/target/arm/kvm32.c @@ -XXX,XX +XXX,XX @@ 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) diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -XXX,XX +XXX,XX @@ 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, @@ -XXX,XX +XXX,XX @@ 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) -- 2.7.4 From: Andrew Jeffery <andrew@aj.id.au> The reset width register controls how the pulse on the SoC's WDTRST{1,2} pins behaves. A pulse is emitted if the external reset bit is set in WDT_CTRL. On the AST2500 WDT_RESET_WIDTH can consume magic bit patterns to configure push-pull/open-drain and active-high/active-low behaviours and thus needs some special handling in the write path. As some of the capabilities depend on the SoC version a silicon-rev property is introduced, which is used to guard version-specific behaviour. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Reviewed-by: Cédric Le Goater <clg@kaod.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/watchdog/wdt_aspeed.h | 2 + hw/watchdog/wdt_aspeed.c | 93 +++++++++++++++++++++++++++++++++++----- 2 files changed, 84 insertions(+), 11 deletions(-) diff --git a/include/hw/watchdog/wdt_aspeed.h b/include/hw/watchdog/wdt_aspeed.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/watchdog/wdt_aspeed.h +++ b/include/hw/watchdog/wdt_aspeed.h @@ -XXX,XX +XXX,XX @@ typedef struct AspeedWDTState { uint32_t regs[ASPEED_WDT_REGS_MAX]; uint32_t pclk_freq; + uint32_t silicon_rev; + uint32_t ext_pulse_width_mask; } AspeedWDTState; #endif /* ASPEED_WDT_H */ diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c index XXXXXXX..XXXXXXX 100644 --- a/hw/watchdog/wdt_aspeed.c +++ b/hw/watchdog/wdt_aspeed.c @@ -XXX,XX +XXX,XX @@ */ #include "qemu/osdep.h" + +#include "qapi/error.h" #include "qemu/log.h" +#include "qemu/timer.h" #include "sysemu/watchdog.h" +#include "hw/misc/aspeed_scu.h" #include "hw/sysbus.h" -#include "qemu/timer.h" #include "hw/watchdog/wdt_aspeed.h" -#define WDT_STATUS (0x00 / 4) -#define WDT_RELOAD_VALUE (0x04 / 4) -#define WDT_RESTART (0x08 / 4) -#define WDT_CTRL (0x0C / 4) +#define WDT_STATUS (0x00 / 4) +#define WDT_RELOAD_VALUE (0x04 / 4) +#define WDT_RESTART (0x08 / 4) +#define WDT_CTRL (0x0C / 4) #define WDT_CTRL_RESET_MODE_SOC (0x00 << 5) #define WDT_CTRL_RESET_MODE_FULL_CHIP (0x01 << 5) #define WDT_CTRL_1MHZ_CLK BIT(4) @@ -XXX,XX +XXX,XX @@ #define WDT_CTRL_WDT_INTR BIT(2) #define WDT_CTRL_RESET_SYSTEM BIT(1) #define WDT_CTRL_ENABLE BIT(0) +#define WDT_RESET_WIDTH (0x18 / 4) +#define WDT_RESET_WIDTH_ACTIVE_HIGH BIT(31) +#define WDT_POLARITY_MASK (0xFF << 24) +#define WDT_ACTIVE_HIGH_MAGIC (0xA5 << 24) +#define WDT_ACTIVE_LOW_MAGIC (0x5A << 24) +#define WDT_RESET_WIDTH_PUSH_PULL BIT(30) +#define WDT_DRIVE_TYPE_MASK (0xFF << 24) +#define WDT_PUSH_PULL_MAGIC (0xA8 << 24) +#define WDT_OPEN_DRAIN_MAGIC (0x8A << 24) -#define WDT_TIMEOUT_STATUS (0x10 / 4) -#define WDT_TIMEOUT_CLEAR (0x14 / 4) -#define WDT_RESET_WDITH (0x18 / 4) +#define WDT_TIMEOUT_STATUS (0x10 / 4) +#define WDT_TIMEOUT_CLEAR (0x14 / 4) -#define WDT_RESTART_MAGIC 0x4755 +#define WDT_RESTART_MAGIC 0x4755 static bool aspeed_wdt_is_enabled(const AspeedWDTState *s) { return s->regs[WDT_CTRL] & WDT_CTRL_ENABLE; } +static bool is_ast2500(const AspeedWDTState *s) +{ + switch (s->silicon_rev) { + case AST2500_A0_SILICON_REV: + case AST2500_A1_SILICON_REV: + return true; + case AST2400_A0_SILICON_REV: + case AST2400_A1_SILICON_REV: + default: + break; + } + + return false; +} + static uint64_t aspeed_wdt_read(void *opaque, hwaddr offset, unsigned size) { AspeedWDTState *s = ASPEED_WDT(opaque); @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_wdt_read(void *opaque, hwaddr offset, unsigned size) return 0; case WDT_CTRL: return s->regs[WDT_CTRL]; + case WDT_RESET_WIDTH: + return s->regs[WDT_RESET_WIDTH]; case WDT_TIMEOUT_STATUS: case WDT_TIMEOUT_CLEAR: - case WDT_RESET_WDITH: qemu_log_mask(LOG_UNIMP, "%s: uninmplemented read at offset 0x%" HWADDR_PRIx "\n", __func__, offset); @@ -XXX,XX +XXX,XX @@ static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data, timer_del(s->timer); } break; + case WDT_RESET_WIDTH: + { + uint32_t property = data & WDT_POLARITY_MASK; + + if (property && is_ast2500(s)) { + if (property == WDT_ACTIVE_HIGH_MAGIC) { + s->regs[WDT_RESET_WIDTH] |= WDT_RESET_WIDTH_ACTIVE_HIGH; + } else if (property == WDT_ACTIVE_LOW_MAGIC) { + s->regs[WDT_RESET_WIDTH] &= ~WDT_RESET_WIDTH_ACTIVE_HIGH; + } else if (property == WDT_PUSH_PULL_MAGIC) { + s->regs[WDT_RESET_WIDTH] |= WDT_RESET_WIDTH_PUSH_PULL; + } else if (property == WDT_OPEN_DRAIN_MAGIC) { + s->regs[WDT_RESET_WIDTH] &= ~WDT_RESET_WIDTH_PUSH_PULL; + } + } + s->regs[WDT_RESET_WIDTH] &= ~s->ext_pulse_width_mask; + s->regs[WDT_RESET_WIDTH] |= data & s->ext_pulse_width_mask; + break; + } case WDT_TIMEOUT_STATUS: case WDT_TIMEOUT_CLEAR: - case WDT_RESET_WDITH: qemu_log_mask(LOG_UNIMP, "%s: uninmplemented write at offset 0x%" HWADDR_PRIx "\n", __func__, offset); @@ -XXX,XX +XXX,XX @@ static void aspeed_wdt_reset(DeviceState *dev) s->regs[WDT_RELOAD_VALUE] = 0x03EF1480; s->regs[WDT_RESTART] = 0; s->regs[WDT_CTRL] = 0; + s->regs[WDT_RESET_WIDTH] = 0xFF; timer_del(s->timer); } @@ -XXX,XX +XXX,XX @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp) SysBusDevice *sbd = SYS_BUS_DEVICE(dev); AspeedWDTState *s = ASPEED_WDT(dev); + if (!is_supported_silicon_rev(s->silicon_rev)) { + error_setg(errp, "Unknown silicon revision: 0x%" PRIx32, + s->silicon_rev); + return; + } + + switch (s->silicon_rev) { + case AST2400_A0_SILICON_REV: + case AST2400_A1_SILICON_REV: + s->ext_pulse_width_mask = 0xff; + break; + case AST2500_A0_SILICON_REV: + case AST2500_A1_SILICON_REV: + s->ext_pulse_width_mask = 0xfffff; + break; + default: + g_assert_not_reached(); + } + s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, aspeed_wdt_timer_expired, dev); /* FIXME: This setting should be derived from the SCU hw strapping @@ -XXX,XX +XXX,XX @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp) sysbus_init_mmio(sbd, &s->iomem); } +static Property aspeed_wdt_properties[] = { + DEFINE_PROP_UINT32("silicon-rev", AspeedWDTState, silicon_rev, 0), + DEFINE_PROP_END_OF_LIST(), +}; + static void aspeed_wdt_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static void aspeed_wdt_class_init(ObjectClass *klass, void *data) dc->reset = aspeed_wdt_reset; set_bit(DEVICE_CATEGORY_MISC, dc->categories); dc->vmsd = &vmstate_aspeed_wdt; + dc->props = aspeed_wdt_properties; } static const TypeInfo aspeed_wdt_info = { -- 2.7.4 From: Andrew Jeffery <andrew@aj.id.au> This is required to configure differences in behaviour between the AST2400 and AST2500 watchdog IPs. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Reviewed-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/aspeed_soc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_init(Object *obj) object_initialize(&s->wdt[i], sizeof(s->wdt[i]), TYPE_ASPEED_WDT); object_property_add_child(obj, "wdt[*]", OBJECT(&s->wdt[i]), NULL); qdev_set_parent_bus(DEVICE(&s->wdt[i]), sysbus_get_default()); + qdev_prop_set_uint32(DEVICE(&s->wdt[i]), "silicon-rev", + sc->info->silicon_rev); } object_initialize(&s->ftgmac100, sizeof(s->ftgmac100), TYPE_FTGMAC100); -- 2.7.4 Move the MemTxResult type to memattrs.h. We're going to want to use it in cpu/qom.h, which doesn't want to include all of memory.h. In practice MemTxResult and MemTxAttrs are pretty closely linked since both are used for the new-style read_with_attrs and write_with_attrs callbacks, so memattrs.h is a reasonable home for this rather than creating a whole new header file for it. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com> --- include/exec/memattrs.h | 10 ++++++++++ include/exec/memory.h | 10 ---------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/memattrs.h +++ b/include/exec/memattrs.h @@ -XXX,XX +XXX,XX @@ typedef struct MemTxAttrs { */ #define MEMTXATTRS_UNSPECIFIED ((MemTxAttrs) { .unspecified = 1 }) +/* New-style MMIO accessors can indicate that the transaction failed. + * A zero (MEMTX_OK) response means success; anything else is a failure + * of some kind. The memory subsystem will bitwise-OR together results + * if it is synthesizing an operation from multiple smaller accesses. + */ +#define MEMTX_OK 0 +#define MEMTX_ERROR (1U << 0) /* device returned an error */ +#define MEMTX_DECODE_ERROR (1U << 1) /* nothing at that address */ +typedef uint32_t MemTxResult; + #endif diff --git a/include/exec/memory.h b/include/exec/memory.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -XXX,XX +XXX,XX @@ static inline void iommu_notifier_init(IOMMUNotifier *n, IOMMUNotify fn, n->end = end; } -/* New-style MMIO accessors can indicate that the transaction failed. - * A zero (MEMTX_OK) response means success; anything else is a failure - * of some kind. The memory subsystem will bitwise-OR together results - * if it is synthesizing an operation from multiple smaller accesses. - */ -#define MEMTX_OK 0 -#define MEMTX_ERROR (1U << 0) /* device returned an error */ -#define MEMTX_DECODE_ERROR (1U << 1) /* nothing at that address */ -typedef uint32_t MemTxResult; - /* * Memory region callbacks */ -- 2.7.4 Currently we have a rather half-baked setup for allowing CPUs to generate exceptions on accesses to invalid memory: the CPU has a cpu_unassigned_access() hook which the memory system calls in unassigned_mem_write() and unassigned_mem_read() if the current_cpu pointer is non-NULL. This was originally designed before we implemented the MemTxResult type that allows memory operations to report a success or failure code, which is why the hook is called right at the bottom of the memory system. The major problem with this is that it means that the hook can be called even when the access was not actually done by the CPU: for instance if the CPU writes to a DMA engine register which causes the DMA engine to begin a transaction which has been set up by the guest to operate on invalid memory then this will casue the CPU to take an exception incorrectly. Another minor problem is that currently if a device returns a transaction error then this won't turn into a CPU exception at all. The right way to do this is to have allow the CPU to respond to memory system transaction failures at the point where the CPU specific code calls into the memory system. Define a new QOM CPU method and utility function cpu_transaction_failed() which is called in these cases. The functionality here overlaps with the existing cpu_unassigned_access() because individual target CPUs will need some work to convert them to the new system. When this transition is complete we can remove the old cpu_unassigned_access() code. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> --- include/qom/cpu.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/include/qom/cpu.h b/include/qom/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -XXX,XX +XXX,XX @@ struct TranslationBlock; * @has_work: Callback for checking if there is work to do. * @do_interrupt: Callback for interrupt handling. * @do_unassigned_access: Callback for unassigned access handling. + * (this is deprecated: new targets should use do_transaction_failed instead) * @do_unaligned_access: Callback for unaligned access handling, if * the target defines #ALIGNED_ONLY. + * @do_transaction_failed: Callback for handling failed memory transactions + * (ie bus faults or external aborts; not MMU faults) * @virtio_is_big_endian: Callback to return %true if a CPU which supports * runtime configurable endianness is currently big-endian. Non-configurable * CPUs can use the default implementation of this method. This method should @@ -XXX,XX +XXX,XX @@ typedef struct CPUClass { void (*do_unaligned_access)(CPUState *cpu, vaddr addr, MMUAccessType access_type, int mmu_idx, uintptr_t retaddr); + void (*do_transaction_failed)(CPUState *cpu, hwaddr physaddr, vaddr addr, + unsigned size, MMUAccessType access_type, + int mmu_idx, MemTxAttrs attrs, + MemTxResult response, uintptr_t retaddr); bool (*virtio_is_big_endian)(CPUState *cpu); int (*memory_rw_debug)(CPUState *cpu, vaddr addr, uint8_t *buf, int len, bool is_write); @@ -XXX,XX +XXX,XX @@ static inline void cpu_unaligned_access(CPUState *cpu, vaddr addr, cc->do_unaligned_access(cpu, addr, access_type, mmu_idx, retaddr); } + +static inline void cpu_transaction_failed(CPUState *cpu, hwaddr physaddr, + vaddr addr, unsigned size, + MMUAccessType access_type, + int mmu_idx, MemTxAttrs attrs, + MemTxResult response, + uintptr_t retaddr) +{ + CPUClass *cc = CPU_GET_CLASS(cpu); + + if (cc->do_transaction_failed) { + cc->do_transaction_failed(cpu, physaddr, addr, size, access_type, + mmu_idx, attrs, response, retaddr); + } +} #endif #endif /* NEED_CPU_H */ -- 2.7.4 Call the new cpu_transaction_failed() hook at the places where CPU generated code interacts with the memory system: io_readx() io_writex() get_page_addr_code() Any access from C code (eg via cpu_physical_memory_rw(), address_space_rw(), ld/st_*_phys()) will *not* trigger CPU exceptions via cpu_transaction_failed(). Handling for transactions failures for this kind of call should be done by using a function which returns a MemTxResult and treating the failure case appropriately in the calling code. In an ideal world we would not generate CPU exceptions for instruction fetch failures in get_page_addr_code() but instead wait until the code translation process tried a load and it failed; however that change would require too great a restructuring and redesign to attempt at this point. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> --- softmmu_template.h | 4 ++-- accel/tcg/cputlb.c | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/softmmu_template.h b/softmmu_template.h index XXXXXXX..XXXXXXX 100644 --- a/softmmu_template.h +++ b/softmmu_template.h @@ -XXX,XX +XXX,XX @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env, uintptr_t retaddr) { CPUIOTLBEntry *iotlbentry = &env->iotlb[mmu_idx][index]; - return io_readx(env, iotlbentry, addr, retaddr, DATA_SIZE); + return io_readx(env, iotlbentry, mmu_idx, addr, retaddr, DATA_SIZE); } #endif @@ -XXX,XX +XXX,XX @@ static inline void glue(io_write, SUFFIX)(CPUArchState *env, uintptr_t retaddr) { CPUIOTLBEntry *iotlbentry = &env->iotlb[mmu_idx][index]; - return io_writex(env, iotlbentry, val, addr, retaddr, DATA_SIZE); + return io_writex(env, iotlbentry, mmu_idx, val, addr, retaddr, DATA_SIZE); } void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val, diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c index XXXXXXX..XXXXXXX 100644 --- a/accel/tcg/cputlb.c +++ b/accel/tcg/cputlb.c @@ -XXX,XX +XXX,XX @@ static inline ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr) } static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry, + int mmu_idx, target_ulong addr, uintptr_t retaddr, int size) { CPUState *cpu = ENV_GET_CPU(env); @@ -XXX,XX +XXX,XX @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry, MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs); uint64_t val; bool locked = false; + MemTxResult r; physaddr = (physaddr & TARGET_PAGE_MASK) + addr; cpu->mem_io_pc = retaddr; @@ -XXX,XX +XXX,XX @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry, qemu_mutex_lock_iothread(); locked = true; } - memory_region_dispatch_read(mr, physaddr, &val, size, iotlbentry->attrs); + r = memory_region_dispatch_read(mr, physaddr, + &val, size, iotlbentry->attrs); + if (r != MEMTX_OK) { + cpu_transaction_failed(cpu, physaddr, addr, size, MMU_DATA_LOAD, + mmu_idx, iotlbentry->attrs, r, retaddr); + } if (locked) { qemu_mutex_unlock_iothread(); } @@ -XXX,XX +XXX,XX @@ static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry, } static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry, + int mmu_idx, uint64_t val, target_ulong addr, uintptr_t retaddr, int size) { @@ -XXX,XX +XXX,XX @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry, hwaddr physaddr = iotlbentry->addr; MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs); bool locked = false; + MemTxResult r; physaddr = (physaddr & TARGET_PAGE_MASK) + addr; if (mr != &io_mem_rom && mr != &io_mem_notdirty && !cpu->can_do_io) { @@ -XXX,XX +XXX,XX @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry, qemu_mutex_lock_iothread(); locked = true; } - memory_region_dispatch_write(mr, physaddr, val, size, iotlbentry->attrs); + r = memory_region_dispatch_write(mr, physaddr, + val, size, iotlbentry->attrs); + if (r != MEMTX_OK) { + cpu_transaction_failed(cpu, physaddr, addr, size, MMU_DATA_STORE, + mmu_idx, iotlbentry->attrs, r, retaddr); + } if (locked) { qemu_mutex_unlock_iothread(); } @@ -XXX,XX +XXX,XX @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr) MemoryRegion *mr; CPUState *cpu = ENV_GET_CPU(env); CPUIOTLBEntry *iotlbentry; + hwaddr physaddr; index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); mmu_idx = cpu_mmu_index(env, true); @@ -XXX,XX +XXX,XX @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr) } qemu_mutex_unlock_iothread(); + /* Give the new-style cpu_transaction_failed() hook first chance + * to handle this. + * This is not the ideal place to detect and generate CPU + * exceptions for instruction fetch failure (for instance + * we don't know the length of the access that the CPU would + * use, and it would be better to go ahead and try the access + * and use the MemTXResult it produced). However it is the + * simplest place we have currently available for the check. + */ + physaddr = (iotlbentry->addr & TARGET_PAGE_MASK) + addr; + cpu_transaction_failed(cpu, physaddr, addr, 0, MMU_INST_FETCH, mmu_idx, + iotlbentry->attrs, MEMTX_DECODE_ERROR, 0); + cpu_unassigned_access(cpu, addr, false, true, 0, 4); /* The CPU's unassigned access hook might have longjumped out * with an exception. If it didn't (or there was no hook) then -- 2.7.4 Define a new MachineClass field ignore_memory_transaction_failures. If this is flag is true then the CPU will ignore memory transaction failures which should cause the CPU to take an exception due to an access to an unassigned physical address; the transaction will instead return zero (for a read) or be ignored (for a write). This should be set only by legacy board models which rely on the old RAZ/WI behaviour for handling devices that QEMU does not yet model. New board models should instead use "unimplemented-device" for all memory ranges where the guest will attempt to probe for a device that QEMU doesn't implement and a stub device is required. We need this for ARM boards, where we're about to implement support for generating external aborts on memory transaction failures. Too many of our legacy board models rely on the RAZ/WI behaviour and we would break currently working guests when their "probe for device" code provoked an external abort rather than a RAZ. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/boards.h | 11 +++++++++++ include/qom/cpu.h | 7 ++++++- qom/cpu.c | 7 +++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/include/hw/boards.h b/include/hw/boards.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -XXX,XX +XXX,XX @@ typedef struct { * size than the target architecture's minimum. (Attempting to create * such a CPU will fail.) Note that changing this is a migration * compatibility break for the machine. + * @ignore_memory_transaction_failures: + * If this is flag is true then the CPU will ignore memory transaction + * failures which should cause the CPU to take an exception due to an + * access to an unassigned physical address; the transaction will instead + * return zero (for a read) or be ignored (for a write). This should be + * set only by legacy board models which rely on the old RAZ/WI behaviour + * for handling devices that QEMU does not yet model. New board models + * should instead use "unimplemented-device" for all memory ranges where + * the guest will attempt to probe for a device that QEMU doesn't + * implement and a stub device is required. */ struct MachineClass { /*< private >*/ @@ -XXX,XX +XXX,XX @@ struct MachineClass { bool rom_file_has_mr; int minimum_page_bits; bool has_hotpluggable_cpus; + bool ignore_memory_transaction_failures; int numa_mem_align_shift; void (*numa_auto_assign_ram)(MachineClass *mc, NodeInfo *nodes, int nb_nodes, ram_addr_t size); diff --git a/include/qom/cpu.h b/include/qom/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -XXX,XX +XXX,XX @@ struct qemu_work_item; * @trace_dstate_delayed: Delayed changes to trace_dstate (includes all changes * to @trace_dstate). * @trace_dstate: Dynamic tracing state of events for this vCPU (bitmask). + * @ignore_memory_transaction_failures: Cached copy of the MachineState + * flag of the same name: allows the board to suppress calling of the + * CPU do_transaction_failed hook function. * * State of one CPU core or thread. */ @@ -XXX,XX +XXX,XX @@ struct CPUState { */ bool throttle_thread_scheduled; + bool ignore_memory_transaction_failures; + /* Note that this is accessed at the start of every TB via a negative offset from AREG0. Leave this field at the end so as to make the (absolute value) offset as small as possible. This reduces code @@ -XXX,XX +XXX,XX @@ static inline void cpu_transaction_failed(CPUState *cpu, hwaddr physaddr, { CPUClass *cc = CPU_GET_CLASS(cpu); - if (cc->do_transaction_failed) { + if (!cpu->ignore_memory_transaction_failures && cc->do_transaction_failed) { cc->do_transaction_failed(cpu, physaddr, addr, size, access_type, mmu_idx, attrs, response, retaddr); } diff --git a/qom/cpu.c b/qom/cpu.c index XXXXXXX..XXXXXXX 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -XXX,XX +XXX,XX @@ #include "exec/cpu-common.h" #include "qemu/error-report.h" #include "sysemu/sysemu.h" +#include "hw/boards.h" #include "hw/qdev-properties.h" #include "trace-root.h" @@ -XXX,XX +XXX,XX @@ static void cpu_common_parse_features(const char *typename, char *features, static void cpu_common_realizefn(DeviceState *dev, Error **errp) { CPUState *cpu = CPU(dev); + Object *machine = qdev_get_machine(); + ObjectClass *oc = object_get_class(machine); + MachineClass *mc = MACHINE_CLASS(oc); + + cpu->ignore_memory_transaction_failures = + mc->ignore_memory_transaction_failures; if (dev->hotplugged) { cpu_synchronize_post_init(cpu); -- 2.7.4 Set the MachineClass flag ignore_memory_transaction_failures for almost all ARM boards. This means they retain the legacy behaviour that accesses to unimplemented addresses will RAZ/WI rather than aborting, when a subsequent commit adds support for external aborts. The exceptions are: * virt -- we know that guests won't try to prod devices that we don't describe in the device tree or ACPI tables * mps2 -- this board was written to use unimplemented-device for all the ranges with devices we don't yet handle New boards should not set the flag, but instead be written like the mps2. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> For the Xilinx boards: Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> --- hw/arm/aspeed.c | 3 +++ hw/arm/collie.c | 1 + hw/arm/cubieboard.c | 1 + hw/arm/digic_boards.c | 1 + hw/arm/exynos4_boards.c | 2 ++ hw/arm/gumstix.c | 2 ++ hw/arm/highbank.c | 2 ++ hw/arm/imx25_pdk.c | 1 + hw/arm/integratorcp.c | 1 + hw/arm/kzm.c | 1 + hw/arm/mainstone.c | 1 + hw/arm/musicpal.c | 1 + hw/arm/netduino2.c | 1 + hw/arm/nseries.c | 2 ++ hw/arm/omap_sx1.c | 2 ++ hw/arm/palm.c | 1 + hw/arm/raspi.c | 1 + hw/arm/realview.c | 4 ++++ hw/arm/sabrelite.c | 1 + hw/arm/spitz.c | 4 ++++ hw/arm/stellaris.c | 2 ++ hw/arm/tosa.c | 1 + hw/arm/versatilepb.c | 2 ++ hw/arm/vexpress.c | 1 + hw/arm/xilinx_zynq.c | 1 + hw/arm/xlnx-ep108.c | 2 ++ hw/arm/z2.c | 1 + 27 files changed, 43 insertions(+) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -XXX,XX +XXX,XX @@ static void palmetto_bmc_class_init(ObjectClass *oc, void *data) mc->no_floppy = 1; mc->no_cdrom = 1; mc->no_parallel = 1; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo palmetto_bmc_type = { @@ -XXX,XX +XXX,XX @@ static void ast2500_evb_class_init(ObjectClass *oc, void *data) mc->no_floppy = 1; mc->no_cdrom = 1; mc->no_parallel = 1; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo ast2500_evb_type = { @@ -XXX,XX +XXX,XX @@ static void romulus_bmc_class_init(ObjectClass *oc, void *data) mc->no_floppy = 1; mc->no_cdrom = 1; mc->no_parallel = 1; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo romulus_bmc_type = { diff --git a/hw/arm/collie.c b/hw/arm/collie.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/collie.c +++ b/hw/arm/collie.c @@ -XXX,XX +XXX,XX @@ static void collie_machine_init(MachineClass *mc) { mc->desc = "Sharp SL-5500 (Collie) PDA (SA-1110)"; mc->init = collie_init; + mc->ignore_memory_transaction_failures = true; } DEFINE_MACHINE("collie", collie_machine_init) diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/cubieboard.c +++ b/hw/arm/cubieboard.c @@ -XXX,XX +XXX,XX @@ static void cubieboard_machine_init(MachineClass *mc) mc->init = cubieboard_init; mc->block_default_type = IF_IDE; mc->units_per_default_bus = 1; + mc->ignore_memory_transaction_failures = true; } DEFINE_MACHINE("cubieboard", cubieboard_machine_init) diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/digic_boards.c +++ b/hw/arm/digic_boards.c @@ -XXX,XX +XXX,XX @@ static void canon_a1100_machine_init(MachineClass *mc) { mc->desc = "Canon PowerShot A1100 IS"; mc->init = &canon_a1100_init; + mc->ignore_memory_transaction_failures = true; } DEFINE_MACHINE("canon-a1100", canon_a1100_machine_init) diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/exynos4_boards.c +++ b/hw/arm/exynos4_boards.c @@ -XXX,XX +XXX,XX @@ static void nuri_class_init(ObjectClass *oc, void *data) mc->desc = "Samsung NURI board (Exynos4210)"; mc->init = nuri_init; mc->max_cpus = EXYNOS4210_NCPUS; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo nuri_type = { @@ -XXX,XX +XXX,XX @@ static void smdkc210_class_init(ObjectClass *oc, void *data) mc->desc = "Samsung SMDKC210 board (Exynos4210)"; mc->init = smdkc210_init; mc->max_cpus = EXYNOS4210_NCPUS; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo smdkc210_type = { diff --git a/hw/arm/gumstix.c b/hw/arm/gumstix.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/gumstix.c +++ b/hw/arm/gumstix.c @@ -XXX,XX +XXX,XX @@ static void connex_class_init(ObjectClass *oc, void *data) mc->desc = "Gumstix Connex (PXA255)"; mc->init = connex_init; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo connex_type = { @@ -XXX,XX +XXX,XX @@ static void verdex_class_init(ObjectClass *oc, void *data) mc->desc = "Gumstix Verdex (PXA270)"; mc->init = verdex_init; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo verdex_type = { diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/highbank.c +++ b/hw/arm/highbank.c @@ -XXX,XX +XXX,XX @@ static void highbank_class_init(ObjectClass *oc, void *data) mc->block_default_type = IF_IDE; mc->units_per_default_bus = 1; mc->max_cpus = 4; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo highbank_type = { @@ -XXX,XX +XXX,XX @@ static void midway_class_init(ObjectClass *oc, void *data) mc->block_default_type = IF_IDE; mc->units_per_default_bus = 1; mc->max_cpus = 4; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo midway_type = { diff --git a/hw/arm/imx25_pdk.c b/hw/arm/imx25_pdk.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/imx25_pdk.c +++ b/hw/arm/imx25_pdk.c @@ -XXX,XX +XXX,XX @@ static void imx25_pdk_machine_init(MachineClass *mc) { mc->desc = "ARM i.MX25 PDK board (ARM926)"; mc->init = imx25_pdk_init; + mc->ignore_memory_transaction_failures = true; } DEFINE_MACHINE("imx25-pdk", imx25_pdk_machine_init) diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/integratorcp.c +++ b/hw/arm/integratorcp.c @@ -XXX,XX +XXX,XX @@ static void integratorcp_machine_init(MachineClass *mc) { mc->desc = "ARM Integrator/CP (ARM926EJ-S)"; mc->init = integratorcp_init; + mc->ignore_memory_transaction_failures = true; } DEFINE_MACHINE("integratorcp", integratorcp_machine_init) diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/kzm.c +++ b/hw/arm/kzm.c @@ -XXX,XX +XXX,XX @@ static void kzm_machine_init(MachineClass *mc) { mc->desc = "ARM KZM Emulation Baseboard (ARM1136)"; mc->init = kzm_init; + mc->ignore_memory_transaction_failures = true; } DEFINE_MACHINE("kzm", kzm_machine_init) diff --git a/hw/arm/mainstone.c b/hw/arm/mainstone.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/mainstone.c +++ b/hw/arm/mainstone.c @@ -XXX,XX +XXX,XX @@ static void mainstone2_machine_init(MachineClass *mc) { mc->desc = "Mainstone II (PXA27x)"; mc->init = mainstone_init; + mc->ignore_memory_transaction_failures = true; } DEFINE_MACHINE("mainstone", mainstone2_machine_init) diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/musicpal.c +++ b/hw/arm/musicpal.c @@ -XXX,XX +XXX,XX @@ static void musicpal_machine_init(MachineClass *mc) { mc->desc = "Marvell 88w8618 / MusicPal (ARM926EJ-S)"; mc->init = musicpal_init; + mc->ignore_memory_transaction_failures = true; } DEFINE_MACHINE("musicpal", musicpal_machine_init) diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/netduino2.c +++ b/hw/arm/netduino2.c @@ -XXX,XX +XXX,XX @@ static void netduino2_machine_init(MachineClass *mc) { mc->desc = "Netduino 2 Machine"; mc->init = netduino2_init; + mc->ignore_memory_transaction_failures = true; } DEFINE_MACHINE("netduino2", netduino2_machine_init) diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/nseries.c +++ b/hw/arm/nseries.c @@ -XXX,XX +XXX,XX @@ static void n800_class_init(ObjectClass *oc, void *data) mc->desc = "Nokia N800 tablet aka. RX-34 (OMAP2420)"; mc->init = n800_init; mc->default_boot_order = ""; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo n800_type = { @@ -XXX,XX +XXX,XX @@ static void n810_class_init(ObjectClass *oc, void *data) mc->desc = "Nokia N810 tablet aka. RX-44 (OMAP2420)"; mc->init = n810_init; mc->default_boot_order = ""; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo n810_type = { diff --git a/hw/arm/omap_sx1.c b/hw/arm/omap_sx1.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/omap_sx1.c +++ b/hw/arm/omap_sx1.c @@ -XXX,XX +XXX,XX @@ static void sx1_machine_v2_class_init(ObjectClass *oc, void *data) mc->desc = "Siemens SX1 (OMAP310) V2"; mc->init = sx1_init_v2; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo sx1_machine_v2_type = { @@ -XXX,XX +XXX,XX @@ static void sx1_machine_v1_class_init(ObjectClass *oc, void *data) mc->desc = "Siemens SX1 (OMAP310) V1"; mc->init = sx1_init_v1; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo sx1_machine_v1_type = { diff --git a/hw/arm/palm.c b/hw/arm/palm.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/palm.c +++ b/hw/arm/palm.c @@ -XXX,XX +XXX,XX @@ static void palmte_machine_init(MachineClass *mc) { mc->desc = "Palm Tungsten|E aka. Cheetah PDA (OMAP310)"; mc->init = palmte_init; + mc->ignore_memory_transaction_failures = true; } DEFINE_MACHINE("cheetah", palmte_machine_init) diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/raspi.c +++ b/hw/arm/raspi.c @@ -XXX,XX +XXX,XX @@ static void raspi2_machine_init(MachineClass *mc) mc->no_cdrom = 1; mc->max_cpus = BCM2836_NCPUS; mc->default_ram_size = 1024 * 1024 * 1024; + mc->ignore_memory_transaction_failures = true; }; DEFINE_MACHINE("raspi2", raspi2_machine_init) diff --git a/hw/arm/realview.c b/hw/arm/realview.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/realview.c +++ b/hw/arm/realview.c @@ -XXX,XX +XXX,XX @@ static void realview_eb_class_init(ObjectClass *oc, void *data) mc->desc = "ARM RealView Emulation Baseboard (ARM926EJ-S)"; mc->init = realview_eb_init; mc->block_default_type = IF_SCSI; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo realview_eb_type = { @@ -XXX,XX +XXX,XX @@ static void realview_eb_mpcore_class_init(ObjectClass *oc, void *data) mc->init = realview_eb_mpcore_init; mc->block_default_type = IF_SCSI; mc->max_cpus = 4; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo realview_eb_mpcore_type = { @@ -XXX,XX +XXX,XX @@ static void realview_pb_a8_class_init(ObjectClass *oc, void *data) mc->desc = "ARM RealView Platform Baseboard for Cortex-A8"; mc->init = realview_pb_a8_init; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo realview_pb_a8_type = { @@ -XXX,XX +XXX,XX @@ static void realview_pbx_a9_class_init(ObjectClass *oc, void *data) mc->desc = "ARM RealView Platform Baseboard Explore for Cortex-A9"; mc->init = realview_pbx_a9_init; mc->max_cpus = 4; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo realview_pbx_a9_type = { diff --git a/hw/arm/sabrelite.c b/hw/arm/sabrelite.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/sabrelite.c +++ b/hw/arm/sabrelite.c @@ -XXX,XX +XXX,XX @@ static void sabrelite_machine_init(MachineClass *mc) mc->desc = "Freescale i.MX6 Quad SABRE Lite Board (Cortex A9)"; mc->init = sabrelite_init; mc->max_cpus = FSL_IMX6_NUM_CPUS; + mc->ignore_memory_transaction_failures = true; } DEFINE_MACHINE("sabrelite", sabrelite_machine_init) diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/spitz.c +++ b/hw/arm/spitz.c @@ -XXX,XX +XXX,XX @@ static void akitapda_class_init(ObjectClass *oc, void *data) mc->desc = "Sharp SL-C1000 (Akita) PDA (PXA270)"; mc->init = akita_init; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo akitapda_type = { @@ -XXX,XX +XXX,XX @@ static void spitzpda_class_init(ObjectClass *oc, void *data) mc->desc = "Sharp SL-C3000 (Spitz) PDA (PXA270)"; mc->init = spitz_init; mc->block_default_type = IF_IDE; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo spitzpda_type = { @@ -XXX,XX +XXX,XX @@ static void borzoipda_class_init(ObjectClass *oc, void *data) mc->desc = "Sharp SL-C3100 (Borzoi) PDA (PXA270)"; mc->init = borzoi_init; mc->block_default_type = IF_IDE; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo borzoipda_type = { @@ -XXX,XX +XXX,XX @@ static void terrierpda_class_init(ObjectClass *oc, void *data) mc->desc = "Sharp SL-C3200 (Terrier) PDA (PXA270)"; mc->init = terrier_init; mc->block_default_type = IF_IDE; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo terrierpda_type = { diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -XXX,XX +XXX,XX @@ static void lm3s811evb_class_init(ObjectClass *oc, void *data) mc->desc = "Stellaris LM3S811EVB"; mc->init = lm3s811evb_init; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo lm3s811evb_type = { @@ -XXX,XX +XXX,XX @@ static void lm3s6965evb_class_init(ObjectClass *oc, void *data) mc->desc = "Stellaris LM3S6965EVB"; mc->init = lm3s6965evb_init; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo lm3s6965evb_type = { diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/tosa.c +++ b/hw/arm/tosa.c @@ -XXX,XX +XXX,XX @@ static void tosapda_machine_init(MachineClass *mc) mc->desc = "Sharp SL-6000 (Tosa) PDA (PXA255)"; mc->init = tosa_init; mc->block_default_type = IF_IDE; + mc->ignore_memory_transaction_failures = true; } DEFINE_MACHINE("tosa", tosapda_machine_init) diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/versatilepb.c +++ b/hw/arm/versatilepb.c @@ -XXX,XX +XXX,XX @@ static void versatilepb_class_init(ObjectClass *oc, void *data) mc->desc = "ARM Versatile/PB (ARM926EJ-S)"; mc->init = vpb_init; mc->block_default_type = IF_SCSI; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo versatilepb_type = { @@ -XXX,XX +XXX,XX @@ static void versatileab_class_init(ObjectClass *oc, void *data) mc->desc = "ARM Versatile/AB (ARM926EJ-S)"; mc->init = vab_init; mc->block_default_type = IF_SCSI; + mc->ignore_memory_transaction_failures = true; } static const TypeInfo versatileab_type = { diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/vexpress.c +++ b/hw/arm/vexpress.c @@ -XXX,XX +XXX,XX @@ static void vexpress_class_init(ObjectClass *oc, void *data) mc->desc = "ARM Versatile Express"; mc->init = vexpress_common_init; mc->max_cpus = 4; + mc->ignore_memory_transaction_failures = true; } static void vexpress_a9_class_init(ObjectClass *oc, void *data) diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/xilinx_zynq.c +++ b/hw/arm/xilinx_zynq.c @@ -XXX,XX +XXX,XX @@ static void zynq_machine_init(MachineClass *mc) mc->init = zynq_init; mc->max_cpus = 1; mc->no_sdcard = 1; + mc->ignore_memory_transaction_failures = true; } DEFINE_MACHINE("xilinx-zynq-a9", zynq_machine_init) diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/xlnx-ep108.c +++ b/hw/arm/xlnx-ep108.c @@ -XXX,XX +XXX,XX @@ static void xlnx_ep108_machine_init(MachineClass *mc) mc->init = xlnx_ep108_init; mc->block_default_type = IF_IDE; mc->units_per_default_bus = 1; + mc->ignore_memory_transaction_failures = true; } DEFINE_MACHINE("xlnx-ep108", xlnx_ep108_machine_init) @@ -XXX,XX +XXX,XX @@ static void xlnx_zcu102_machine_init(MachineClass *mc) mc->init = xlnx_ep108_init; mc->block_default_type = IF_IDE; mc->units_per_default_bus = 1; + mc->ignore_memory_transaction_failures = true; } DEFINE_MACHINE("xlnx-zcu102", xlnx_zcu102_machine_init) diff --git a/hw/arm/z2.c b/hw/arm/z2.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/z2.c +++ b/hw/arm/z2.c @@ -XXX,XX +XXX,XX @@ static void z2_machine_init(MachineClass *mc) { mc->desc = "Zipit Z2 (PXA27x)"; mc->init = z2_init; + mc->ignore_memory_transaction_failures = true; } DEFINE_MACHINE("z2", z2_machine_init) -- 2.7.4 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 <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Acked-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> --- target/arm/op_helper.c | 110 +++++++++++++++++++++++++------------------------ 1 file changed, 57 insertions(+), 53 deletions(-) diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -XXX,XX +XXX,XX @@ 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) @@ -XXX,XX +XXX,XX @@ 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 @@ -XXX,XX +XXX,XX @@ 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); } } @@ -XXX,XX +XXX,XX @@ 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) { @@ -XXX,XX +XXX,XX @@ 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) */ -- 2.7.4 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 <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> --- target/arm/internals.h | 2 ++ target/arm/op_helper.c | 10 +++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -XXX,XX +XXX,XX @@ 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) { @@ -XXX,XX +XXX,XX @@ 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. @@ -XXX,XX +XXX,XX @@ 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; @@ -XXX,XX +XXX,XX @@ 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 -- 2.7.4 Implement the new do_transaction_failed hook for ARM, which should cause the CPU to take a prefetch abort or data abort. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> --- target/arm/internals.h | 10 ++++++++++ target/arm/cpu.c | 1 + target/arm/op_helper.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, MMUAccessType access_type, int mmu_idx, uintptr_t retaddr); +/* arm_cpu_do_transaction_failed: handle a memory system error response + * (eg "no device/memory present at address") by raising an external abort + * exception + */ +void arm_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr, + vaddr addr, unsigned size, + MMUAccessType access_type, + int mmu_idx, MemTxAttrs attrs, + MemTxResult response, uintptr_t retaddr); + /* Call the EL change hook if one has been registered */ static inline void arm_call_el_change_hook(ARMCPU *cpu) { diff --git a/target/arm/cpu.c b/target/arm/cpu.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -XXX,XX +XXX,XX @@ static void arm_cpu_class_init(ObjectClass *oc, void *data) #else cc->do_interrupt = arm_cpu_do_interrupt; cc->do_unaligned_access = arm_cpu_do_unaligned_access; + cc->do_transaction_failed = arm_cpu_do_transaction_failed; cc->get_phys_page_attrs_debug = arm_cpu_get_phys_page_attrs_debug; cc->asidx_from_attrs = arm_asidx_from_attrs; cc->vmsd = &vmstate_arm_cpu; diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -XXX,XX +XXX,XX @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, deliver_fault(cpu, vaddr, access_type, fsr, fsc, &fi); } +/* arm_cpu_do_transaction_failed: handle a memory system error response + * (eg "no device/memory present at address") by raising an external abort + * exception + */ +void arm_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr, + vaddr addr, unsigned size, + MMUAccessType access_type, + int mmu_idx, MemTxAttrs attrs, + MemTxResult response, uintptr_t retaddr) +{ + ARMCPU *cpu = ARM_CPU(cs); + CPUARMState *env = &cpu->env; + uint32_t fsr, fsc; + ARMMMUFaultInfo fi = {}; + ARMMMUIdx arm_mmu_idx = core_to_arm_mmu_idx(env, mmu_idx); + + if (retaddr) { + /* now we have a real cpu fault */ + cpu_restore_state(cs, retaddr); + } + + /* The EA bit in syndromes and fault status registers is an + * IMPDEF classification of external aborts. ARM implementations + * usually use this to indicate AXI bus Decode error (0) or + * Slave error (1); in QEMU we follow that. + */ + fi.ea = (response != MEMTX_DECODE_ERROR); + + /* The fault status register format 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)) { + /* long descriptor form, STATUS 0b010000: synchronous ext abort */ + fsr = (fi.ea << 12) | (1 << 9) | 0x10; + } else { + /* short descriptor form, FSR 0b01000 : synchronous ext abort */ + fsr = (fi.ea << 12) | 0x8; + } + fsc = 0x10; + + deliver_fault(cpu, addr, access_type, fsr, fsc, &fi); +} + #endif /* !defined(CONFIG_USER_ONLY) */ uint32_t HELPER(add_setq)(CPUARMState *env, uint32_t a, uint32_t b) -- 2.7.4 From: Thomas Huth <thuth@redhat.com> QEMU currently aborts if the user is accidentially trying to do something like this: $ aarch64-softmmu/qemu-system-aarch64 -S -M integratorcp -nographic QEMU 2.9.93 monitor - type 'help' for more information (qemu) device_add ast2400 Unexpected error in error_set_from_qdev_prop_error() at hw/core/qdev-properties.c:1032: Aborted (core dumped) The ast2400 SoC devices are clearly not creatable by the user since they are using the serial_hds and nd_table arrays directly in their realize function, so mark them with user_creatable = false. Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/aspeed_soc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_class_init(ObjectClass *oc, void *data) sc->info = (AspeedSoCInfo *) data; dc->realize = aspeed_soc_realize; + /* Reason: Uses serial_hds and nd_table in realize() directly */ + dc->user_creatable = false; } static const TypeInfo aspeed_soc_type_info = { -- 2.7.4 From: Thomas Huth <thuth@redhat.com> QEMU currently shows some unexpected behavior when the user trys to do a "device_add digic" on an unrelated ARM machine like integratorcp in "-nographic" mode (the device_add command does not immediately return to the monitor prompt), and trying to "device_del" the device later results in a "qemu/qdev-monitor.c:872:qdev_unplug: assertion failed: (hotplug_ctrl)" error condition. Looking at the realize function of the device, it uses serial_hds directly and this means that the device can not be added a second time, so let's simply mark it with "user_creatable = false" now. Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/digic.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/arm/digic.c b/hw/arm/digic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/digic.c +++ b/hw/arm/digic.c @@ -XXX,XX +XXX,XX @@ static void digic_class_init(ObjectClass *oc, void *data) DeviceClass *dc = DEVICE_CLASS(oc); dc->realize = digic_realize; + /* Reason: Uses serial_hds in the realize function --> not usable twice */ + dc->user_creatable = false; } static const TypeInfo digic_type_info = { -- 2.7.4 From: Richard Henderson <richard.henderson@linaro.org> 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 <andrew@fubar.geek.nz> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20170825224833.4463-1-richard.henderson@linaro.org Fixes: https://bugs.launchpad.net/qemu/+bug/1713066 Signed-off-by: Richard Henderson <richard.henderson@linaro.org> [PMM: tweaked comment format] Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ 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); } -- 2.7.4 From: Pranith Kumar <bobby.prani@gmail.com> Fix the following warning: /home/pranith/qemu/hw/intc/arm_gicv3_kvm.c:296:17: warning: logical not is only applied to the left hand side of this bitwise operator [-Wlogical-not-parentheses] if (!c->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) { ^ ~ /home/pranith/qemu/hw/intc/arm_gicv3_kvm.c:296:17: note: add parentheses after the '!' to evaluate the bitwise operator first if (!c->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) { ^ /home/pranith/qemu/hw/intc/arm_gicv3_kvm.c:296:17: note: add parentheses around left hand side expression to silence this warning if (!c->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) { ^ This logic error meant we were not setting the PTZ bit when we should -- luckily as the comment suggests this wouldn't have had any effects beyond making GIC initialization take a little longer. Signed-off-by: Pranith Kumar <bobby.prani@gmail.com> Message-id: 20170829173226.7625-1-bobby.prani@gmail.com Cc: qemu-stable@nongnu.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gicv3_kvm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_kvm.c +++ b/hw/intc/arm_gicv3_kvm.c @@ -XXX,XX +XXX,XX @@ static void kvm_arm_gicv3_put(GICv3State *s) kvm_gicr_access(s, GICR_PROPBASER + 4, ncpu, &regh, true); reg64 = c->gicr_pendbaser; - if (!c->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) { + if (!(c->gicr_ctlr & GICR_CTLR_ENABLE_LPIS)) { /* Setting PTZ is advised if LPIs are disabled, to reduce * GIC initialization time. */ -- 2.7.4 Hi; here's a target-arm pullreq for rc0; these are all bugfixes and similar minor stuff. thanks -- PMM The following changes since commit 0462a32b4f63b2448b4a196381138afd50719dc4: Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into staging (2025-03-14 09:31:13 +0800) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20250314-1 for you to fetch changes up to a019e15edfd62beae1e2f6adc0fa7415ba20b14c: meson.build: Set RUST_BACKTRACE for all tests (2025-03-14 12:54:33 +0000) ---------------------------------------------------------------- target-arm queue: * Correctly handle corner cases of guest attempting an exception return to AArch32 when target EL is AArch64 only * MAINTAINERS: Fix status for Arm boards I "maintain" * tests/functional: Bump up arm_replay timeout * Revert "hw/char/pl011: Warn when using disabled receiver" * util/cacheflush: Make first DSB unconditional on aarch64 * target/arm: Fix SVE/SME access check logic * meson.build: Set RUST_BACKTRACE for all tests ---------------------------------------------------------------- Joe Komlodi (1): util/cacheflush: Make first DSB unconditional on aarch64 Paolo Bonzini (1): Revert "hw/char/pl011: Warn when using disabled receiver" Peter Maydell (13): target/arm: Move A32_BANKED_REG_{GET,SET} macros to cpregs.h target/arm: Un-inline access_secure_reg() linux-user/aarch64: Remove unused get/put_user macros linux-user/arm: Remove unused get_put_user macros target/arm: Move arm_cpu_data_is_big_endian() etc to internals.h target/arm: Move arm_current_el() and arm_el_is_aa64() to internals.h target/arm: SCR_EL3.RW should be treated as 1 if EL2 doesn't support AArch32 target/arm: HCR_EL2.RW should be RAO/WI if EL1 doesn't support AArch32 target/arm: Add cpu local variable to exception_return helper target/arm: Forbid return to AArch32 when CPU is AArch64-only MAINTAINERS: Fix status for Arm boards I "maintain" tests/functional: Bump up arm_replay timeout meson.build: Set RUST_BACKTRACE for all tests Richard Henderson (2): target/arm: Make DisasContext.{fp, sve}_access_checked tristate target/arm: Simplify pstate_sm check in sve_access_check MAINTAINERS | 14 ++-- meson.build | 9 ++- target/arm/cpregs.h | 28 +++++++ target/arm/cpu.h | 153 +----------------------------------- target/arm/internals.h | 135 +++++++++++++++++++++++++++++++ target/arm/tcg/translate-a64.h | 2 +- target/arm/tcg/translate.h | 10 ++- hw/char/pl011.c | 19 ++--- hw/intc/arm_gicv3_cpuif.c | 1 + linux-user/aarch64/cpu_loop.c | 48 ----------- linux-user/arm/cpu_loop.c | 43 +--------- target/arm/arch_dump.c | 1 + target/arm/helper.c | 16 +++- target/arm/tcg/helper-a64.c | 12 ++- target/arm/tcg/hflags.c | 9 +++ target/arm/tcg/translate-a64.c | 37 ++++----- util/cacheflush.c | 4 +- .gitlab-ci.d/buildtest-template.yml | 1 - 18 files changed, 257 insertions(+), 285 deletions(-) The A32_BANKED_REG_{GET,SET} macros are only used inside target/arm; move their definitions to cpregs.h. There's no need to have them defined in all the code that includes cpu.h. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/cpregs.h | 28 ++++++++++++++++++++++++++++ target/arm/cpu.h | 27 --------------------------- 2 files changed, 28 insertions(+), 27 deletions(-) diff --git a/target/arm/cpregs.h b/target/arm/cpregs.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpregs.h +++ b/target/arm/cpregs.h @@ -XXX,XX +XXX,XX @@ static inline bool arm_cpreg_traps_in_nv(const ARMCPRegInfo *ri) return ri->opc1 == 4 || ri->opc1 == 5; } +/* Macros for accessing a specified CP register bank */ +#define A32_BANKED_REG_GET(_env, _regname, _secure) \ + ((_secure) ? (_env)->cp15._regname##_s : (_env)->cp15._regname##_ns) + +#define A32_BANKED_REG_SET(_env, _regname, _secure, _val) \ + do { \ + if (_secure) { \ + (_env)->cp15._regname##_s = (_val); \ + } else { \ + (_env)->cp15._regname##_ns = (_val); \ + } \ + } while (0) + +/* + * Macros for automatically accessing a specific CP register bank depending on + * the current secure state of the system. These macros are not intended for + * supporting instruction translation reads/writes as these are dependent + * solely on the SCR.NS bit and not the mode. + */ +#define A32_BANKED_CURRENT_REG_GET(_env, _regname) \ + A32_BANKED_REG_GET((_env), _regname, \ + (arm_is_secure(_env) && !arm_el_is_aa64((_env), 3))) + +#define A32_BANKED_CURRENT_REG_SET(_env, _regname, _val) \ + A32_BANKED_REG_SET((_env), _regname, \ + (arm_is_secure(_env) && !arm_el_is_aa64((_env), 3)), \ + (_val)) + #endif /* TARGET_ARM_CPREGS_H */ diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline bool access_secure_reg(CPUARMState *env) return ret; } -/* Macros for accessing a specified CP register bank */ -#define A32_BANKED_REG_GET(_env, _regname, _secure) \ - ((_secure) ? (_env)->cp15._regname##_s : (_env)->cp15._regname##_ns) - -#define A32_BANKED_REG_SET(_env, _regname, _secure, _val) \ - do { \ - if (_secure) { \ - (_env)->cp15._regname##_s = (_val); \ - } else { \ - (_env)->cp15._regname##_ns = (_val); \ - } \ - } while (0) - -/* Macros for automatically accessing a specific CP register bank depending on - * the current secure state of the system. These macros are not intended for - * supporting instruction translation reads/writes as these are dependent - * solely on the SCR.NS bit and not the mode. - */ -#define A32_BANKED_CURRENT_REG_GET(_env, _regname) \ - A32_BANKED_REG_GET((_env), _regname, \ - (arm_is_secure(_env) && !arm_el_is_aa64((_env), 3))) - -#define A32_BANKED_CURRENT_REG_SET(_env, _regname, _val) \ - A32_BANKED_REG_SET((_env), _regname, \ - (arm_is_secure(_env) && !arm_el_is_aa64((_env), 3)), \ - (_val)) - uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx, uint32_t cur_el, bool secure); -- 2.43.0 We would like to move arm_el_is_aa64() to internals.h; however, it is used by access_secure_reg(). Make that function not be inline, so that it can stay in cpu.h. access_secure_reg() is used only in two places: * in hflags.c * in the user-mode arm emulators, to decide whether to store the TLS value in the secure or non-secure banked field The second of these is not on a super-hot path that would care about the inlining (and incidentally will always use the NS banked field because our user-mode CPUs never set ARM_FEATURE_EL3); put the definition of access_secure_reg() in hflags.c, near its only use inside target/arm. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/cpu.h | 12 +++--------- target/arm/tcg/hflags.c | 9 +++++++++ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline bool arm_el_is_aa64(CPUARMState *env, int el) return aa64; } -/* Function for determining whether guest cp register reads and writes should +/* + * Function for determining whether guest cp register reads and writes should * access the secure or non-secure bank of a cp register. When EL3 is * operating in AArch32 state, the NS-bit determines whether the secure * instance of a cp register should be used. When EL3 is AArch64 (or if * it doesn't exist at all) then there is no register banking, and all * accesses are to the non-secure version. */ -static inline bool access_secure_reg(CPUARMState *env) -{ - bool ret = (arm_feature(env, ARM_FEATURE_EL3) && - !arm_el_is_aa64(env, 3) && - !(env->cp15.scr_el3 & SCR_NS)); - - return ret; -} +bool access_secure_reg(CPUARMState *env); uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx, uint32_t cur_el, bool secure); diff --git a/target/arm/tcg/hflags.c b/target/arm/tcg/hflags.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/tcg/hflags.c +++ b/target/arm/tcg/hflags.c @@ -XXX,XX +XXX,XX @@ static bool aprofile_require_alignment(CPUARMState *env, int el, uint64_t sctlr) #endif } +bool access_secure_reg(CPUARMState *env) +{ + bool ret = (arm_feature(env, ARM_FEATURE_EL3) && + !arm_el_is_aa64(env, 3) && + !(env->cp15.scr_el3 & SCR_NS)); + + return ret; +} + static CPUARMTBFlags rebuild_hflags_common(CPUARMState *env, int fp_el, ARMMMUIdx mmu_idx, CPUARMTBFlags flags) -- 2.43.0 At the top of linux-user/aarch64/cpu_loop.c we define a set of macros for reading and writing data and code words, but we never use these macros. Delete them. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> --- linux-user/aarch64/cpu_loop.c | 48 ----------------------------------- 1 file changed, 48 deletions(-) diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c index XXXXXXX..XXXXXXX 100644 --- a/linux-user/aarch64/cpu_loop.c +++ b/linux-user/aarch64/cpu_loop.c @@ -XXX,XX +XXX,XX @@ #include "target/arm/syndrome.h" #include "target/arm/cpu-features.h" -#define get_user_code_u32(x, gaddr, env) \ - ({ abi_long __r = get_user_u32((x), (gaddr)); \ - if (!__r && bswap_code(arm_sctlr_b(env))) { \ - (x) = bswap32(x); \ - } \ - __r; \ - }) - -#define get_user_code_u16(x, gaddr, env) \ - ({ abi_long __r = get_user_u16((x), (gaddr)); \ - if (!__r && bswap_code(arm_sctlr_b(env))) { \ - (x) = bswap16(x); \ - } \ - __r; \ - }) - -#define get_user_data_u32(x, gaddr, env) \ - ({ abi_long __r = get_user_u32((x), (gaddr)); \ - if (!__r && arm_cpu_bswap_data(env)) { \ - (x) = bswap32(x); \ - } \ - __r; \ - }) - -#define get_user_data_u16(x, gaddr, env) \ - ({ abi_long __r = get_user_u16((x), (gaddr)); \ - if (!__r && arm_cpu_bswap_data(env)) { \ - (x) = bswap16(x); \ - } \ - __r; \ - }) - -#define put_user_data_u32(x, gaddr, env) \ - ({ typeof(x) __x = (x); \ - if (arm_cpu_bswap_data(env)) { \ - __x = bswap32(__x); \ - } \ - put_user_u32(__x, (gaddr)); \ - }) - -#define put_user_data_u16(x, gaddr, env) \ - ({ typeof(x) __x = (x); \ - if (arm_cpu_bswap_data(env)) { \ - __x = bswap16(__x); \ - } \ - put_user_u16(__x, (gaddr)); \ - }) - /* AArch64 main loop */ void cpu_loop(CPUARMState *env) { -- 2.43.0 In linux-user/arm/cpu_loop.c we define a full set of get/put macros for both code and data (since the endianness handling is different between the two). However the only one we actually use is get_user_code_u32(). Remove the rest. We leave a comment noting how data-side accesses should be handled for big-endian, because that's a subtle point and we just removed the macros that were effectively documenting it. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> --- linux-user/arm/cpu_loop.c | 43 ++++----------------------------------- 1 file changed, 4 insertions(+), 39 deletions(-) diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c index XXXXXXX..XXXXXXX 100644 --- a/linux-user/arm/cpu_loop.c +++ b/linux-user/arm/cpu_loop.c @@ -XXX,XX +XXX,XX @@ __r; \ }) -#define get_user_code_u16(x, gaddr, env) \ - ({ abi_long __r = get_user_u16((x), (gaddr)); \ - if (!__r && bswap_code(arm_sctlr_b(env))) { \ - (x) = bswap16(x); \ - } \ - __r; \ - }) - -#define get_user_data_u32(x, gaddr, env) \ - ({ abi_long __r = get_user_u32((x), (gaddr)); \ - if (!__r && arm_cpu_bswap_data(env)) { \ - (x) = bswap32(x); \ - } \ - __r; \ - }) - -#define get_user_data_u16(x, gaddr, env) \ - ({ abi_long __r = get_user_u16((x), (gaddr)); \ - if (!__r && arm_cpu_bswap_data(env)) { \ - (x) = bswap16(x); \ - } \ - __r; \ - }) - -#define put_user_data_u32(x, gaddr, env) \ - ({ typeof(x) __x = (x); \ - if (arm_cpu_bswap_data(env)) { \ - __x = bswap32(__x); \ - } \ - put_user_u32(__x, (gaddr)); \ - }) - -#define put_user_data_u16(x, gaddr, env) \ - ({ typeof(x) __x = (x); \ - if (arm_cpu_bswap_data(env)) { \ - __x = bswap16(__x); \ - } \ - put_user_u16(__x, (gaddr)); \ - }) +/* + * Note that if we need to do data accesses here, they should do a + * bswap if arm_cpu_bswap_data() returns true. + */ /* * Similar to code in accel/tcg/user-exec.c, but outside the execution loop. -- 2.43.0 The arm_cpu_data_is_big_endian() and related functions are now used only in target/arm; they can be moved to internals.h. The motivation here is that we would like to move arm_current_el() to internals.h. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/cpu.h | 48 ------------------------------------------ target/arm/internals.h | 48 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 48 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline bool arm_sctlr_b(CPUARMState *env) uint64_t arm_sctlr(CPUARMState *env, int el); -static inline bool arm_cpu_data_is_big_endian_a32(CPUARMState *env, - bool sctlr_b) -{ -#ifdef CONFIG_USER_ONLY - /* - * In system mode, BE32 is modelled in line with the - * architecture (as word-invariant big-endianness), where loads - * and stores are done little endian but from addresses which - * are adjusted by XORing with the appropriate constant. So the - * endianness to use for the raw data access is not affected by - * SCTLR.B. - * In user mode, however, we model BE32 as byte-invariant - * big-endianness (because user-only code cannot tell the - * difference), and so we need to use a data access endianness - * that depends on SCTLR.B. - */ - if (sctlr_b) { - return true; - } -#endif - /* In 32bit endianness is determined by looking at CPSR's E bit */ - return env->uncached_cpsr & CPSR_E; -} - -static inline bool arm_cpu_data_is_big_endian_a64(int el, uint64_t sctlr) -{ - return sctlr & (el ? SCTLR_EE : SCTLR_E0E); -} - -/* Return true if the processor is in big-endian mode. */ -static inline bool arm_cpu_data_is_big_endian(CPUARMState *env) -{ - if (!is_a64(env)) { - return arm_cpu_data_is_big_endian_a32(env, arm_sctlr_b(env)); - } else { - int cur_el = arm_current_el(env); - uint64_t sctlr = arm_sctlr(env, cur_el); - return arm_cpu_data_is_big_endian_a64(cur_el, sctlr); - } -} - #include "exec/cpu-all.h" /* @@ -XXX,XX +XXX,XX @@ static inline bool bswap_code(bool sctlr_b) #endif } -#ifdef CONFIG_USER_ONLY -static inline bool arm_cpu_bswap_data(CPUARMState *env) -{ - return TARGET_BIG_ENDIAN ^ arm_cpu_data_is_big_endian(env); -} -#endif - void cpu_get_tb_cpu_state(CPUARMState *env, vaddr *pc, uint64_t *cs_base, uint32_t *flags); diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ static inline FloatRoundMode arm_rmode_to_sf(ARMFPRounding rmode) return arm_rmode_to_sf_map[rmode]; } +static inline bool arm_cpu_data_is_big_endian_a32(CPUARMState *env, + bool sctlr_b) +{ +#ifdef CONFIG_USER_ONLY + /* + * In system mode, BE32 is modelled in line with the + * architecture (as word-invariant big-endianness), where loads + * and stores are done little endian but from addresses which + * are adjusted by XORing with the appropriate constant. So the + * endianness to use for the raw data access is not affected by + * SCTLR.B. + * In user mode, however, we model BE32 as byte-invariant + * big-endianness (because user-only code cannot tell the + * difference), and so we need to use a data access endianness + * that depends on SCTLR.B. + */ + if (sctlr_b) { + return true; + } +#endif + /* In 32bit endianness is determined by looking at CPSR's E bit */ + return env->uncached_cpsr & CPSR_E; +} + +static inline bool arm_cpu_data_is_big_endian_a64(int el, uint64_t sctlr) +{ + return sctlr & (el ? SCTLR_EE : SCTLR_E0E); +} + +/* Return true if the processor is in big-endian mode. */ +static inline bool arm_cpu_data_is_big_endian(CPUARMState *env) +{ + if (!is_a64(env)) { + return arm_cpu_data_is_big_endian_a32(env, arm_sctlr_b(env)); + } else { + int cur_el = arm_current_el(env); + uint64_t sctlr = arm_sctlr(env, cur_el); + return arm_cpu_data_is_big_endian_a64(cur_el, sctlr); + } +} + +#ifdef CONFIG_USER_ONLY +static inline bool arm_cpu_bswap_data(CPUARMState *env) +{ + return TARGET_BIG_ENDIAN ^ arm_cpu_data_is_big_endian(env); +} +#endif + static inline void aarch64_save_sp(CPUARMState *env, int el) { if (env->pstate & PSTATE_SP) { -- 2.43.0 The functions arm_current_el() and arm_el_is_aa64() are used only in target/arm and in hw/intc/arm_gicv3_cpuif.c. They're functions that query internal state of the CPU. Move them out of cpu.h and into internals.h. This means we need to include internals.h in arm_gicv3_cpuif.c, but this is justifiable because that file is implementing the GICv3 CPU interface, which really is part of the CPU proper; we just ended up implementing it in code in hw/intc/ for historical reasons. The motivation for this move is that we'd like to change arm_el_is_aa64() to add a condition that uses cpu_isar_feature(); but we don't want to include cpu-features.h in cpu.h. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/cpu.h | 66 -------------------------------------- target/arm/internals.h | 67 +++++++++++++++++++++++++++++++++++++++ hw/intc/arm_gicv3_cpuif.c | 1 + target/arm/arch_dump.c | 1 + 4 files changed, 69 insertions(+), 66 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ uint64_t arm_hcr_el2_eff_secstate(CPUARMState *env, ARMSecuritySpace space); uint64_t arm_hcr_el2_eff(CPUARMState *env); uint64_t arm_hcrx_el2_eff(CPUARMState *env); -/* Return true if the specified exception level is running in AArch64 state. */ -static inline bool arm_el_is_aa64(CPUARMState *env, int el) -{ - /* This isn't valid for EL0 (if we're in EL0, is_a64() is what you want, - * and if we're not in EL0 then the state of EL0 isn't well defined.) - */ - assert(el >= 1 && el <= 3); - bool aa64 = arm_feature(env, ARM_FEATURE_AARCH64); - - /* The highest exception level is always at the maximum supported - * register width, and then lower levels have a register width controlled - * by bits in the SCR or HCR registers. - */ - if (el == 3) { - return aa64; - } - - if (arm_feature(env, ARM_FEATURE_EL3) && - ((env->cp15.scr_el3 & SCR_NS) || !(env->cp15.scr_el3 & SCR_EEL2))) { - aa64 = aa64 && (env->cp15.scr_el3 & SCR_RW); - } - - if (el == 2) { - return aa64; - } - - if (arm_is_el2_enabled(env)) { - aa64 = aa64 && (env->cp15.hcr_el2 & HCR_RW); - } - - return aa64; -} - /* * Function for determining whether guest cp register reads and writes should * access the secure or non-secure bank of a cp register. When EL3 is @@ -XXX,XX +XXX,XX @@ 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 arm_v7m_is_handler_mode(env) || - !(env->v7m.control[env->v7m.secure] & 1); - } - - if (is_a64(env)) { - return extract32(env->pstate, 2, 2); - } - - switch (env->uncached_cpsr & 0x1f) { - case ARM_CPU_MODE_USR: - return 0; - case ARM_CPU_MODE_HYP: - return 2; - case ARM_CPU_MODE_MON: - return 3; - default: - if (arm_is_secure(env) && !arm_el_is_aa64(env, 3)) { - /* If EL3 is 32-bit then all secure privileged modes run in - * EL3 - */ - return 3; - } - - return 1; - } -} - /** * write_list_to_cpustate * @cpu: ARMCPU diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ static inline FloatRoundMode arm_rmode_to_sf(ARMFPRounding rmode) return arm_rmode_to_sf_map[rmode]; } +/* Return true if the specified exception level is running in AArch64 state. */ +static inline bool arm_el_is_aa64(CPUARMState *env, int el) +{ + /* + * This isn't valid for EL0 (if we're in EL0, is_a64() is what you want, + * and if we're not in EL0 then the state of EL0 isn't well defined.) + */ + assert(el >= 1 && el <= 3); + bool aa64 = arm_feature(env, ARM_FEATURE_AARCH64); + + /* + * The highest exception level is always at the maximum supported + * register width, and then lower levels have a register width controlled + * by bits in the SCR or HCR registers. + */ + if (el == 3) { + return aa64; + } + + if (arm_feature(env, ARM_FEATURE_EL3) && + ((env->cp15.scr_el3 & SCR_NS) || !(env->cp15.scr_el3 & SCR_EEL2))) { + aa64 = aa64 && (env->cp15.scr_el3 & SCR_RW); + } + + if (el == 2) { + return aa64; + } + + if (arm_is_el2_enabled(env)) { + aa64 = aa64 && (env->cp15.hcr_el2 & HCR_RW); + } + + return aa64; +} + +/* + * 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 arm_v7m_is_handler_mode(env) || + !(env->v7m.control[env->v7m.secure] & 1); + } + + if (is_a64(env)) { + return extract32(env->pstate, 2, 2); + } + + switch (env->uncached_cpsr & 0x1f) { + case ARM_CPU_MODE_USR: + return 0; + case ARM_CPU_MODE_HYP: + return 2; + case ARM_CPU_MODE_MON: + return 3; + default: + if (arm_is_secure(env) && !arm_el_is_aa64(env, 3)) { + /* If EL3 is 32-bit then all secure privileged modes run in EL3 */ + return 3; + } + + return 1; + } +} + static inline bool arm_cpu_data_is_big_endian_a32(CPUARMState *env, bool sctlr_b) { diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -XXX,XX +XXX,XX @@ #include "cpu.h" #include "target/arm/cpregs.h" #include "target/arm/cpu-features.h" +#include "target/arm/internals.h" #include "system/tcg.h" #include "system/qtest.h" diff --git a/target/arm/arch_dump.c b/target/arm/arch_dump.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/arch_dump.c +++ b/target/arm/arch_dump.c @@ -XXX,XX +XXX,XX @@ #include "elf.h" #include "system/dump.h" #include "cpu-features.h" +#include "internals.h" /* struct user_pt_regs from arch/arm64/include/uapi/asm/ptrace.h */ struct aarch64_user_regs { -- 2.43.0 The definition of SCR_EL3.RW says that its effective value is 1 if: - EL2 is implemented and does not support AArch32, and SCR_EL3.NS is 1 - the effective value of SCR_EL3.{EEL2,NS} is {1,0} (i.e. we are Secure and Secure EL2 is disabled) We implement the second of these in arm_el_is_aa64(), but forgot the first. Provide a new function arm_scr_rw_eff() to return the effective value of SCR_EL3.RW, and use it in arm_el_is_aa64() and the other places that currently look directly at the bit value. (scr_write() enforces that the RW bit is RAO/WI if neither EL1 nor EL2 have AArch32 support, but if EL1 does but EL2 does not then the bit must still be writeable.) This will mean that if code at EL3 attempts to perform an exception return to AArch32 EL2 when EL2 is AArch64-only we will correctly handle this as an illegal exception return: it will be caught by the "return to an EL which is configured for a different register width" check in HELPER(exception_return). We do already have some CPU types which don't implement AArch32 above EL0, so this is technically a bug; it doesn't seem worth backporting to stable because no sensible guest code will be deliberately attempting to set the RW bit to a value corresponding to an unimplemented execution state and then checking that we did the right thing. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/internals.h | 26 +++++++++++++++++++++++--- target/arm/helper.c | 4 ++-- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ static inline FloatRoundMode arm_rmode_to_sf(ARMFPRounding rmode) return arm_rmode_to_sf_map[rmode]; } +/* Return the effective value of SCR_EL3.RW */ +static inline bool arm_scr_rw_eff(CPUARMState *env) +{ + /* + * SCR_EL3.RW has an effective value of 1 if: + * - we are NS and EL2 is implemented but doesn't support AArch32 + * - we are S and EL2 is enabled (in which case it must be AArch64) + */ + ARMCPU *cpu = env_archcpu(env); + + if (env->cp15.scr_el3 & SCR_RW) { + return true; + } + if (env->cp15.scr_el3 & SCR_NS) { + return arm_feature(env, ARM_FEATURE_EL2) && + !cpu_isar_feature(aa64_aa32_el2, cpu); + } else { + return env->cp15.scr_el3 & SCR_EEL2; + } +} + /* Return true if the specified exception level is running in AArch64 state. */ static inline bool arm_el_is_aa64(CPUARMState *env, int el) { @@ -XXX,XX +XXX,XX @@ static inline bool arm_el_is_aa64(CPUARMState *env, int el) return aa64; } - if (arm_feature(env, ARM_FEATURE_EL3) && - ((env->cp15.scr_el3 & SCR_NS) || !(env->cp15.scr_el3 & SCR_EEL2))) { - aa64 = aa64 && (env->cp15.scr_el3 & SCR_RW); + if (arm_feature(env, ARM_FEATURE_EL3)) { + aa64 = aa64 && arm_scr_rw_eff(env); } if (el == 2) { 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 @@ uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx, uint64_t hcr_el2; if (arm_feature(env, ARM_FEATURE_EL3)) { - rw = ((env->cp15.scr_el3 & SCR_RW) == SCR_RW); + rw = arm_scr_rw_eff(env); } else { /* * Either EL2 is the highest EL (and so the EL2 register width @@ -XXX,XX +XXX,XX @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs) switch (new_el) { case 3: - is_aa64 = (env->cp15.scr_el3 & SCR_RW) != 0; + is_aa64 = arm_scr_rw_eff(env); break; case 2: hcr = arm_hcr_el2_eff(env); -- 2.43.0 When EL1 doesn't support AArch32, the HCR_EL2.RW bit is supposed to be RAO/WI. Enforce the RAO/WI behaviour. Note that we handle "reset value should honour RES1 bits" in the same way that SCR_EL3 does, via a reset function. We do already have some CPU types which don't implement AArch32 above EL0, so this is technically a bug; it doesn't seem worth backporting to stable because no sensible guest code will be deliberately attempting to set the RW bit to a value corresponding to an unimplemented execution state and then checking that we did the right thing. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/helper.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) 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 void do_hcr_write(CPUARMState *env, uint64_t value, uint64_t valid_mask) /* Clear RES0 bits. */ value &= valid_mask; + /* RW is RAO/WI if EL1 is AArch64 only */ + if (!cpu_isar_feature(aa64_aa32_el1, cpu)) { + value |= HCR_RW; + } + /* * These bits change the MMU setup: * HCR_VM enables stage 2 translation @@ -XXX,XX +XXX,XX @@ static void hcr_writelow(CPUARMState *env, const ARMCPRegInfo *ri, do_hcr_write(env, value, MAKE_64BIT_MASK(32, 32)); } +static void hcr_reset(CPUARMState *env, const ARMCPRegInfo *ri) +{ + /* hcr_write will set the RES1 bits on an AArch64-only CPU */ + hcr_write(env, ri, 0); +} + /* * Return the effective value of HCR_EL2, at the given security state. * Bits that are not included here: @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo el2_cp_reginfo[] = { .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0, .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.hcr_el2), .nv2_redirect_offset = 0x78, + .resetfn = hcr_reset, .writefn = hcr_write, .raw_writefn = raw_write }, { .name = "HCR", .state = ARM_CP_STATE_AA32, .type = ARM_CP_ALIAS | ARM_CP_IO, -- 2.43.0 We already call env_archcpu() multiple times within the exception_return helper function, and we're about to want to add another use of the ARMCPU pointer. Add a local variable cpu so we can call env_archcpu() just once. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/tcg/helper-a64.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/tcg/helper-a64.c +++ b/target/arm/tcg/helper-a64.c @@ -XXX,XX +XXX,XX @@ static void cpsr_write_from_spsr_elx(CPUARMState *env, void HELPER(exception_return)(CPUARMState *env, uint64_t new_pc) { + ARMCPU *cpu = env_archcpu(env); int cur_el = arm_current_el(env); unsigned int spsr_idx = aarch64_banked_spsr_index(cur_el); uint32_t spsr = env->banked_spsr[spsr_idx]; @@ -XXX,XX +XXX,XX @@ void HELPER(exception_return)(CPUARMState *env, uint64_t new_pc) } bql_lock(); - arm_call_pre_el_change_hook(env_archcpu(env)); + arm_call_pre_el_change_hook(cpu); bql_unlock(); if (!return_to_aa64) { @@ -XXX,XX +XXX,XX @@ void HELPER(exception_return)(CPUARMState *env, uint64_t new_pc) int tbii; env->aarch64 = true; - spsr &= aarch64_pstate_valid_mask(&env_archcpu(env)->isar); + spsr &= aarch64_pstate_valid_mask(&cpu->isar); pstate_write(env, spsr); if (!arm_singlestep_active(env)) { env->pstate &= ~PSTATE_SS; @@ -XXX,XX +XXX,XX @@ void HELPER(exception_return)(CPUARMState *env, uint64_t new_pc) aarch64_sve_change_el(env, cur_el, new_el, return_to_aa64); bql_lock(); - arm_call_el_change_hook(env_archcpu(env)); + arm_call_el_change_hook(cpu); bql_unlock(); return; -- 2.43.0 In the Arm ARM, rule R_TYTWB states that returning to AArch32 is an illegal exception return if: * AArch32 is not supported at any exception level * the target EL is configured for AArch64 via SCR_EL3.RW or HCR_EL2.RW or via CPU state at reset We check the second of these, but not the first (which can only be relevant for the case of a return to EL0, because if AArch32 is not supported at one of the higher ELs then the RW bits will have an effective value of 1 and the the "configured for AArch64" condition will hold also). Add the missing condition. Although this is technically a bug (because we have one AArch64-only CPU: a64fx) it isn't worth backporting to stable because no sensible guest code will deliberately try to return to a nonexistent execution state to check that it gets an illegal exception return. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/tcg/helper-a64.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/tcg/helper-a64.c +++ b/target/arm/tcg/helper-a64.c @@ -XXX,XX +XXX,XX @@ void HELPER(exception_return)(CPUARMState *env, uint64_t new_pc) goto illegal_return; } + if (!return_to_aa64 && !cpu_isar_feature(aa64_aa32, cpu)) { + /* Return to AArch32 when CPU is AArch64-only */ + goto illegal_return; + } + if (new_el == 1 && (arm_hcr_el2_eff(env) & HCR_TGE)) { goto illegal_return; } -- 2.43.0 I'm down as the only listed maintainer for quite a lot of Arm SoC and board types. In some cases this is only as the "maintainer of last resort" and I'm not in practice doing anything beyond patch review and the odd bit of tidyup. Move these entries in MAINTAINERS from "Maintained" to "Odd Fixes", to better represent reality. Entries for other boards and SoCs where I do more actively care (or where there is a listed co-maintainer) remain as they are. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-id: 20250307152838.3226398-1-peter.maydell@linaro.org --- MAINTAINERS | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index XXXXXXX..XXXXXXX 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -XXX,XX +XXX,XX @@ F: docs/system/arm/kzm.rst Integrator CP M: Peter Maydell <peter.maydell@linaro.org> L: qemu-arm@nongnu.org -S: Maintained +S: Odd Fixes F: hw/arm/integratorcp.c F: hw/misc/arm_integrator_debug.c F: include/hw/misc/arm_integrator_debug.h @@ -XXX,XX +XXX,XX @@ F: docs/system/arm/mps2.rst Musca M: Peter Maydell <peter.maydell@linaro.org> L: qemu-arm@nongnu.org -S: Maintained +S: Odd Fixes F: hw/arm/musca.c F: docs/system/arm/musca.rst @@ -XXX,XX +XXX,XX @@ F: tests/functional/test_aarch64_raspi4.py Real View M: Peter Maydell <peter.maydell@linaro.org> L: qemu-arm@nongnu.org -S: Maintained +S: Odd Fixes F: hw/arm/realview* F: hw/cpu/realview_mpcore.c F: hw/intc/realview_gic.c @@ -XXX,XX +XXX,XX @@ F: tests/functional/test_arm_collie.py Stellaris M: Peter Maydell <peter.maydell@linaro.org> L: qemu-arm@nongnu.org -S: Maintained +S: Odd Fixes F: hw/*/stellaris* F: hw/display/ssd03* F: include/hw/input/gamepad.h @@ -XXX,XX +XXX,XX @@ F: docs/system/arm/stm32.rst Versatile Express M: Peter Maydell <peter.maydell@linaro.org> L: qemu-arm@nongnu.org -S: Maintained +S: Odd Fixes F: hw/arm/vexpress.c F: hw/display/sii9022.c F: docs/system/arm/vexpress.rst @@ -XXX,XX +XXX,XX @@ F: tests/functional/test_arm_vexpress.py Versatile PB M: Peter Maydell <peter.maydell@linaro.org> L: qemu-arm@nongnu.org -S: Maintained +S: Odd Fixes F: hw/*/versatile* F: hw/i2c/arm_sbcon_i2c.c F: include/hw/i2c/arm_sbcon_i2c.h @@ -XXX,XX +XXX,XX @@ F: include/hw/hyperv/vmbus*.h OMAP M: Peter Maydell <peter.maydell@linaro.org> L: qemu-arm@nongnu.org -S: Maintained +S: Odd Fixes F: hw/*/omap* F: include/hw/arm/omap.h F: docs/system/arm/sx1.rst -- 2.43.0 From: Paolo Bonzini <pbonzini@redhat.com> The guest does not control whether characters are sent on the UART. Sending them before the guest happens to boot will now result in a "guest error" log entry that is only because of timing, even if the guest _would_ later setup the receiver correctly. This reverts the bulk of commit abf2b6a028670bd2890bb3aee7e103fe53e4b0df, and instead adds a comment about why we don't check the enable bits. Cc: Philippe Mathieu-Daudé <philmd@linaro.org> Cc: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-id: 20250311153717.206129-1-pbonzini@redhat.com [PMM: expanded comment] Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/char/pl011.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/hw/char/pl011.c b/hw/char/pl011.c index XXXXXXX..XXXXXXX 100644 --- a/hw/char/pl011.c +++ b/hw/char/pl011.c @@ -XXX,XX +XXX,XX @@ static int pl011_can_receive(void *opaque) unsigned fifo_depth = pl011_get_fifo_depth(s); unsigned fifo_available = fifo_depth - s->read_count; - if (!(s->cr & CR_UARTEN)) { - qemu_log_mask(LOG_GUEST_ERROR, - "PL011 receiving data on disabled UART\n"); - } - if (!(s->cr & CR_RXE)) { - qemu_log_mask(LOG_GUEST_ERROR, - "PL011 receiving data on disabled RX UART\n"); - } - trace_pl011_can_receive(s->lcr, s->read_count, fifo_depth, fifo_available); + /* + * In theory we should check the UART and RX enable bits here and + * return 0 if they are not set (so the guest can't receive data + * until you have enabled the UART). In practice we suspect there + * is at least some guest code out there which has been tested only + * on QEMU and which never bothers to enable the UART because we + * historically never enforced that. So we effectively keep the + * UART continuously enabled regardless of the enable bits. + */ + trace_pl011_can_receive(s->lcr, s->read_count, fifo_depth, fifo_available); return fifo_available; } -- 2.43.0 From: Joe Komlodi <komlodi@google.com> On ARM hosts with CTR_EL0.DIC and CTR_EL0.IDC set, this would only cause an ISB to be executed during cache maintenance, which could lead to QEMU executing TBs containing garbage instructions. This seems to be because the ISB finishes executing instructions and flushes the pipeline, but the ISB doesn't guarantee that writes from the executed instructions are committed. If a small enough TB is created, it's possible that the writes setting up the TB aren't committed by the time the TB is executed. This function is intended to be a port of the gcc implementation (https://github.com/gcc-mirror/gcc/blob/85b46d0795ac76bc192cb8f88b646a647acf98c1/libgcc/config/aarch64/sync-cache.c#L67) which makes the first DSB unconditional, so we can fix the synchronization issue by doing that as well. Cc: qemu-stable@nongnu.org Fixes: 664a79735e4deb1 ("util: Specialize flush_idcache_range for aarch64") Signed-off-by: Joe Komlodi <komlodi@google.com> Message-id: 20250310203622.1827940-2-komlodi@google.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- util/cacheflush.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/util/cacheflush.c b/util/cacheflush.c index XXXXXXX..XXXXXXX 100644 --- a/util/cacheflush.c +++ b/util/cacheflush.c @@ -XXX,XX +XXX,XX @@ void flush_idcache_range(uintptr_t rx, uintptr_t rw, size_t len) for (p = rw & -dcache_lsize; p < rw + len; p += dcache_lsize) { asm volatile("dc\tcvau, %0" : : "r" (p) : "memory"); } - asm volatile("dsb\tish" : : : "memory"); } + /* DSB unconditionally to ensure any outstanding writes are committed. */ + asm volatile("dsb\tish" : : : "memory"); + /* * If CTR_EL0.DIC is enabled, Instruction cache cleaning to the Point * of Unification is not required for instruction to data coherence. -- 2.43.0 From: Richard Henderson <richard.henderson@linaro.org> The check for fp_excp_el in assert_fp_access_checked is incorrect. For SME, with StreamingMode enabled, the access is really against the streaming mode vectors, and access to the normal fp registers is allowed to be disabled. C.f. sme_enabled_check. Convert sve_access_checked to match, even though we don't currently check the exception state. Cc: qemu-stable@nongnu.org Fixes: 3d74825f4d6 ("target/arm: Add SME enablement checks") Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20250307190415.982049-2-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/tcg/translate-a64.h | 2 +- target/arm/tcg/translate.h | 10 +++++++--- target/arm/tcg/translate-a64.c | 17 +++++++++-------- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/target/arm/tcg/translate-a64.h b/target/arm/tcg/translate-a64.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/tcg/translate-a64.h +++ b/target/arm/tcg/translate-a64.h @@ -XXX,XX +XXX,XX @@ TCGv_i64 gen_mte_checkN(DisasContext *s, TCGv_i64 addr, bool is_write, static inline void assert_fp_access_checked(DisasContext *s) { #ifdef CONFIG_DEBUG_TCG - if (unlikely(!s->fp_access_checked || s->fp_excp_el)) { + if (unlikely(s->fp_access_checked <= 0)) { fprintf(stderr, "target-arm: FP access check missing for " "instruction 0x%08x\n", s->insn); abort(); diff --git a/target/arm/tcg/translate.h b/target/arm/tcg/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/tcg/translate.h +++ b/target/arm/tcg/translate.h @@ -XXX,XX +XXX,XX @@ typedef struct DisasContext { bool aarch64; bool thumb; bool lse2; - /* Because unallocated encodings generate different exception syndrome + /* + * Because unallocated encodings generate different exception syndrome * information from traps due to FP being disabled, we can't do a single * "is fp access disabled" check at a high level in the decode tree. * To help in catching bugs where the access check was forgotten in some * code path, we set this flag when the access check is done, and assert * that it is set at the point where we actually touch the FP regs. + * 0: not checked, + * 1: checked, access ok + * -1: checked, access denied */ - bool fp_access_checked; - bool sve_access_checked; + int8_t fp_access_checked; + int8_t sve_access_checked; /* ARMv8 single-step state (this is distinct from the QEMU gdbstub * single-step support). */ diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/tcg/translate-a64.c +++ b/target/arm/tcg/translate-a64.c @@ -XXX,XX +XXX,XX @@ static bool fp_access_check_only(DisasContext *s) { if (s->fp_excp_el) { assert(!s->fp_access_checked); - s->fp_access_checked = true; + s->fp_access_checked = -1; gen_exception_insn_el(s, 0, EXCP_UDEF, syn_fp_access_trap(1, 0xe, false, 0), s->fp_excp_el); return false; } - s->fp_access_checked = true; + s->fp_access_checked = 1; return true; } @@ -XXX,XX +XXX,XX @@ bool sve_access_check(DisasContext *s) syn_sve_access_trap(), s->sve_excp_el); goto fail_exit; } - s->sve_access_checked = true; + s->sve_access_checked = 1; return fp_access_check(s); fail_exit: /* Assert that we only raise one exception per instruction. */ assert(!s->sve_access_checked); - s->sve_access_checked = true; + s->sve_access_checked = -1; return false; } @@ -XXX,XX +XXX,XX @@ bool sme_enabled_check(DisasContext *s) * sme_excp_el by itself for cpregs access checks. */ if (!s->fp_excp_el || s->sme_excp_el < s->fp_excp_el) { - s->fp_access_checked = true; - return sme_access_check(s); + bool ret = sme_access_check(s); + s->fp_access_checked = (ret ? 1 : -1); + return ret; } return fp_access_check_only(s); } @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) s->insn = insn; s->base.pc_next = pc + 4; - s->fp_access_checked = false; - s->sve_access_checked = false; + s->fp_access_checked = 0; + s->sve_access_checked = 0; if (s->pstate_il) { /* -- 2.43.0 From: Richard Henderson <richard.henderson@linaro.org> In StreamingMode, fp_access_checked is handled already. We cannot fall through to fp_access_check lest we fall foul of the double-check assertion. Cc: qemu-stable@nongnu.org Fixes: 285b1d5fcef ("target/arm: Handle SME in sve_access_check") Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20250307190415.982049-3-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> [PMM: move declaration of 'ret' to top of block] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/tcg/translate-a64.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/tcg/translate-a64.c +++ b/target/arm/tcg/translate-a64.c @@ -XXX,XX +XXX,XX @@ static int fp_access_check_vector_hsd(DisasContext *s, bool is_q, MemOp esz) bool sve_access_check(DisasContext *s) { if (s->pstate_sm || !dc_isar_feature(aa64_sve, s)) { + bool ret; + assert(dc_isar_feature(aa64_sme, s)); - if (!sme_sm_enabled_check(s)) { - goto fail_exit; - } - } else if (s->sve_excp_el) { + ret = sme_sm_enabled_check(s); + s->sve_access_checked = (ret ? 1 : -1); + return ret; + } + if (s->sve_excp_el) { + /* Assert that we only raise one exception per instruction. */ + assert(!s->sve_access_checked); gen_exception_insn_el(s, 0, EXCP_UDEF, syn_sve_access_trap(), s->sve_excp_el); - goto fail_exit; + s->sve_access_checked = -1; + return false; } s->sve_access_checked = 1; return fp_access_check(s); - - fail_exit: - /* Assert that we only raise one exception per instruction. */ - assert(!s->sve_access_checked); - s->sve_access_checked = -1; - return false; } /* -- 2.43.0 We want to capture potential Rust backtraces on panics in our test logs, which isn't Rust's default behaviour. Set RUST_BACKTRACE=1 in the add_test_setup environments, so that all our tests get run with this environment variable set. This makes the setting of that variable in the gitlab CI template redundant, so we can remove it. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-id: 20250310102950.3752908-1-peter.maydell@linaro.org --- meson.build | 9 ++++++--- .gitlab-ci.d/buildtest-template.yml | 1 - 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/meson.build b/meson.build index XXXXXXX..XXXXXXX 100644 --- a/meson.build +++ b/meson.build @@ -XXX,XX +XXX,XX @@ project('qemu', ['c'], meson_version: '>=1.5.0', meson.add_devenv({ 'MESON_BUILD_ROOT' : meson.project_build_root() }) -add_test_setup('quick', exclude_suites: ['slow', 'thorough'], is_default: true) -add_test_setup('slow', exclude_suites: ['thorough'], env: ['G_TEST_SLOW=1', 'SPEED=slow']) -add_test_setup('thorough', env: ['G_TEST_SLOW=1', 'SPEED=thorough']) +add_test_setup('quick', exclude_suites: ['slow', 'thorough'], is_default: true, + env: ['RUST_BACKTRACE=1']) +add_test_setup('slow', exclude_suites: ['thorough'], + env: ['G_TEST_SLOW=1', 'SPEED=slow', 'RUST_BACKTRACE=1']) +add_test_setup('thorough', + env: ['G_TEST_SLOW=1', 'SPEED=thorough', 'RUST_BACKTRACE=1']) meson.add_postconf_script(find_program('scripts/symlink-install-tree.py')) diff --git a/.gitlab-ci.d/buildtest-template.yml b/.gitlab-ci.d/buildtest-template.yml index XXXXXXX..XXXXXXX 100644 --- a/.gitlab-ci.d/buildtest-template.yml +++ b/.gitlab-ci.d/buildtest-template.yml @@ -XXX,XX +XXX,XX @@ stage: test image: $CI_REGISTRY_IMAGE/qemu/$IMAGE:$QEMU_CI_CONTAINER_TAG script: - - export RUST_BACKTRACE=1 - source scripts/ci/gitlab-ci-section - section_start buildenv "Setting up to run tests" - scripts/git-submodule.sh update roms/SLOF -- 2.43.0