Most of this is the Neon decodetree patches, followed by Edgar's versal cleanups. thanks -- PMM The following changes since commit 2ef486e76d64436be90f7359a3071fb2a56ce835: Merge remote-tracking branch 'remotes/marcel/tags/rdma-pull-request' into staging (2020-05-03 14:12:56 +0100) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20200504 for you to fetch changes up to 9aefc6cf9b73f66062d2f914a0136756e7a28211: target/arm: Move gen_ function typedefs to translate.h (2020-05-04 12:59:26 +0100) ---------------------------------------------------------------- target-arm queue: * Start of conversion of Neon insns to decodetree * versal board: support SD and RTC * Implement ARMv8.2-TTS2UXN * Make VQDMULL undefined when U=1 * Some minor code cleanups ---------------------------------------------------------------- Edgar E. Iglesias (11): hw/arm: versal: Remove inclusion of arm_gicv3_common.h hw/arm: versal: Move misplaced comment hw/arm: versal-virt: Fix typo xlnx-ve -> xlnx-versal hw/arm: versal: Embed the UARTs into the SoC type hw/arm: versal: Embed the GEMs into the SoC type hw/arm: versal: Embed the ADMAs into the SoC type hw/arm: versal: Embed the APUs into the SoC type hw/arm: versal: Add support for SD hw/arm: versal: Add support for the RTC hw/arm: versal-virt: Add support for SD hw/arm: versal-virt: Add support for the RTC Fredrik Strupe (1): target/arm: Make VQDMULL undefined when U=1 Peter Maydell (25): target/arm: Don't use a TLB for ARMMMUIdx_Stage2 target/arm: Use enum constant in get_phys_addr_lpae() call target/arm: Add new 's1_is_el0' argument to get_phys_addr_lpae() target/arm: Implement ARMv8.2-TTS2UXN target/arm: Use correct variable for setting 'max' cpu's ID_AA64DFR0 target/arm/translate-vfp.inc.c: Remove duplicate simd_r32 check target/arm: Don't allow Thumb Neon insns without FEATURE_NEON target/arm: Add stubs for AArch32 Neon decodetree target/arm: Convert VCMLA (vector) to decodetree target/arm: Convert VCADD (vector) to decodetree target/arm: Convert V[US]DOT (vector) to decodetree target/arm: Convert VFM[AS]L (vector) to decodetree target/arm: Convert VCMLA (scalar) to decodetree target/arm: Convert V[US]DOT (scalar) to decodetree target/arm: Convert VFM[AS]L (scalar) to decodetree target/arm: Convert Neon load/store multiple structures to decodetree target/arm: Convert Neon 'load single structure to all lanes' to decodetree target/arm: Convert Neon 'load/store single structure' to decodetree target/arm: Convert Neon 3-reg-same VADD/VSUB to decodetree target/arm: Convert Neon 3-reg-same logic ops to decodetree target/arm: Convert Neon 3-reg-same VMAX/VMIN to decodetree target/arm: Convert Neon 3-reg-same comparisons to decodetree target/arm: Convert Neon 3-reg-same VQADD/VQSUB to decodetree target/arm: Convert Neon 3-reg-same VMUL, VMLA, VMLS, VSHL to decodetree target/arm: Move gen_ function typedefs to translate.h Philippe Mathieu-Daudé (2): hw/arm/mps2-tz: Use TYPE_IOTKIT instead of hardcoded string target/arm: Use uint64_t for midr field in CPU state struct include/hw/arm/xlnx-versal.h | 31 +- target/arm/cpu-param.h | 2 +- target/arm/cpu.h | 38 ++- target/arm/translate-a64.h | 9 - target/arm/translate.h | 26 ++ target/arm/neon-dp.decode | 86 +++++ target/arm/neon-ls.decode | 52 +++ target/arm/neon-shared.decode | 66 ++++ hw/arm/mps2-tz.c | 2 +- hw/arm/xlnx-versal-virt.c | 74 ++++- hw/arm/xlnx-versal.c | 115 +++++-- target/arm/cpu.c | 3 +- target/arm/cpu64.c | 8 +- target/arm/helper.c | 183 ++++------ target/arm/translate-a64.c | 17 - target/arm/translate-neon.inc.c | 714 +++++++++++++++++++++++++++++++++++++++ target/arm/translate-vfp.inc.c | 6 - target/arm/translate.c | 716 +++------------------------------------- target/arm/Makefile.objs | 18 + 19 files changed, 1302 insertions(+), 864 deletions(-) create mode 100644 target/arm/neon-dp.decode create mode 100644 target/arm/neon-ls.decode create mode 100644 target/arm/neon-shared.decode create mode 100644 target/arm/translate-neon.inc.c From: Fredrik Strupe <fredrik@strupe.net> According to Arm ARM, VQDMULL is only valid when U=0, while having U=1 is unallocated. Signed-off-by: Fredrik Strupe <fredrik@strupe.net> Fixes: 695272dcb976 ("target-arm: Handle UNDEF cases for Neon 3-regs-different-widths") Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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_neon_data_insn(DisasContext *s, uint32_t insn) {0, 0, 0, 0}, /* VMLSL */ {0, 0, 0, 9}, /* VQDMLSL */ {0, 0, 0, 0}, /* Integer VMULL */ - {0, 0, 0, 1}, /* VQDMULL */ + {0, 0, 0, 9}, /* VQDMULL */ {0, 0, 0, 0xa}, /* Polynomial VMULL */ {0, 0, 0, 7}, /* Reserved: always UNDEF */ }; -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> By using the TYPE_* definitions for devices, we can: - quickly find where devices are used with 'git-grep' - easily rename a device (one-line change). Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20200428154650.21991-1-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/mps2-tz.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/mps2-tz.c +++ b/hw/arm/mps2-tz.c @@ -XXX,XX +XXX,XX @@ static void mps2tz_common_init(MachineState *machine) exit(EXIT_FAILURE); } - sysbus_init_child_obj(OBJECT(machine), "iotkit", &mms->iotkit, + sysbus_init_child_obj(OBJECT(machine), TYPE_IOTKIT, &mms->iotkit, sizeof(mms->iotkit), mmc->armsse_type); iotkitdev = DEVICE(&mms->iotkit); object_property_set_link(OBJECT(&mms->iotkit), OBJECT(system_memory), -- 2.20.1 We define ARMMMUIdx_Stage2 as being an MMU index which uses a QEMU TLB. However we never actually use the TLB -- all stage 2 lookups are done by direct calls to get_phys_addr_lpae() followed by a physical address load via address_space_ld*(). Remove Stage2 from the list of ARM MMU indexes which correspond to real core MMU indexes, and instead put it in the set of "NOTLB" ARM MMU indexes. This allows us to drop NB_MMU_MODES to 11. It also means we can safely add support for the ARMv8.3-TTS2UXN extension, which adds permission bits to the stage 2 descriptors which define execute permission separatel for EL0 and EL1; supporting that while keeping Stage2 in a QEMU TLB would require us to use separate TLBs for "Stage2 for an EL0 access" and "Stage2 for an EL1 access", which is a lot of extra complication given we aren't even using the QEMU TLB. In the process of updating the comment on our MMU index use, fix a couple of other minor errors: * NS EL2 EL2&0 was missing from the list in the comment * some text hadn't been updated from when we bumped NB_MMU_MODES above 8 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: 20200330210400.11724-2-peter.maydell@linaro.org --- target/arm/cpu-param.h | 2 +- target/arm/cpu.h | 21 +++++--- target/arm/helper.c | 112 ++++------------------------------------- 3 files changed, 27 insertions(+), 108 deletions(-) diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu-param.h +++ b/target/arm/cpu-param.h @@ -XXX,XX +XXX,XX @@ # define TARGET_PAGE_BITS_MIN 10 #endif -#define NB_MMU_MODES 12 +#define NB_MMU_MODES 11 #endif 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 @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync); * handling via the TLB. The only way to do a stage 1 translation without * the immediate stage 2 translation is via the ATS or AT system insns, * which can be slow-pathed and always do a page table walk. + * The only use of stage 2 translations is either as part of an s1+2 + * lookup or when loading the descriptors during a stage 1 page table walk, + * and in both those cases we don't use the TLB. * 4. we can also safely fold together the "32 bit EL3" and "64 bit EL3" * translation regimes, because they map reasonably well to each other * and they can't both be active at the same time. @@ -XXX,XX +XXX,XX @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync); * NS EL1 EL1&0 stage 1+2 (aka NS PL1) * NS EL1 EL1&0 stage 1+2 +PAN * NS EL0 EL2&0 + * NS EL2 EL2&0 * NS EL2 EL2&0 +PAN * NS EL2 (aka NS PL2) * S EL0 EL1&0 (aka S PL0) * S EL1 EL1&0 (not used if EL3 is 32 bit) * S EL1 EL1&0 +PAN * S EL3 (aka S PL1) - * NS EL1&0 stage 2 * - * for a total of 12 different mmu_idx. + * for a total of 11 different mmu_idx. * * R profile CPUs have an MPU, but can use the same set of MMU indexes * as A profile. They only need to distinguish NS EL0 and NS EL1 (and @@ -XXX,XX +XXX,XX @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync); * are not quite the same -- different CPU types (most notably M profile * vs A/R profile) would like to use MMU indexes with different semantics, * but since we don't ever need to use all of those in a single CPU we - * can avoid setting NB_MMU_MODES to more than 8. The lower bits of + * can avoid having to set NB_MMU_MODES to "total number of A profile MMU + * modes + total number of M profile MMU modes". The lower bits of * ARMMMUIdx are the core TLB mmu index, and the higher bits are always * the same for any particular CPU. * Variables of type ARMMUIdx are always full values, and the core @@ -XXX,XX +XXX,XX @@ typedef enum ARMMMUIdx { ARMMMUIdx_SE10_1_PAN = 9 | ARM_MMU_IDX_A, ARMMMUIdx_SE3 = 10 | ARM_MMU_IDX_A, - ARMMMUIdx_Stage2 = 11 | ARM_MMU_IDX_A, - /* * These are not allocated TLBs and are used only for AT system * instructions or for the first stage of an S12 page table walk. @@ -XXX,XX +XXX,XX @@ typedef enum ARMMMUIdx { ARMMMUIdx_Stage1_E0 = 0 | ARM_MMU_IDX_NOTLB, ARMMMUIdx_Stage1_E1 = 1 | ARM_MMU_IDX_NOTLB, ARMMMUIdx_Stage1_E1_PAN = 2 | ARM_MMU_IDX_NOTLB, + /* + * Not allocated a TLB: used only for second stage of an S12 page + * table walk, or for descriptor loads during first stage of an S1 + * page table walk. Note that if we ever want to have a TLB for this + * then various TLB flush insns which currently are no-ops or flush + * only stage 1 MMU indexes will need to change to flush stage 2. + */ + ARMMMUIdx_Stage2 = 3 | ARM_MMU_IDX_NOTLB, /* * M-profile. @@ -XXX,XX +XXX,XX @@ typedef enum ARMMMUIdxBit { TO_CORE_BIT(SE10_1), TO_CORE_BIT(SE10_1_PAN), TO_CORE_BIT(SE3), - TO_CORE_BIT(Stage2), TO_CORE_BIT(MUser), TO_CORE_BIT(MPriv), 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 tlbiall_nsnh_write(CPUARMState *env, const ARMCPRegInfo *ri, tlb_flush_by_mmuidx(cs, ARMMMUIdxBit_E10_1 | ARMMMUIdxBit_E10_1_PAN | - ARMMMUIdxBit_E10_0 | - ARMMMUIdxBit_Stage2); + ARMMMUIdxBit_E10_0); } static void tlbiall_nsnh_is_write(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static void tlbiall_nsnh_is_write(CPUARMState *env, const ARMCPRegInfo *ri, tlb_flush_by_mmuidx_all_cpus_synced(cs, ARMMMUIdxBit_E10_1 | ARMMMUIdxBit_E10_1_PAN | - ARMMMUIdxBit_E10_0 | - ARMMMUIdxBit_Stage2); + ARMMMUIdxBit_E10_0); } -static void tlbiipas2_write(CPUARMState *env, const ARMCPRegInfo *ri, - uint64_t value) -{ - /* Invalidate by IPA. This has to invalidate any structures that - * contain only stage 2 translation information, but does not need - * to apply to structures that contain combined stage 1 and stage 2 - * translation information. - * This must NOP if EL2 isn't implemented or SCR_EL3.NS is zero. - */ - CPUState *cs = env_cpu(env); - uint64_t pageaddr; - - if (!arm_feature(env, ARM_FEATURE_EL2) || !(env->cp15.scr_el3 & SCR_NS)) { - return; - } - - pageaddr = sextract64(value << 12, 0, 40); - - tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdxBit_Stage2); -} - -static void tlbiipas2_is_write(CPUARMState *env, const ARMCPRegInfo *ri, - uint64_t value) -{ - CPUState *cs = env_cpu(env); - uint64_t pageaddr; - - if (!arm_feature(env, ARM_FEATURE_EL2) || !(env->cp15.scr_el3 & SCR_NS)) { - return; - } - - pageaddr = sextract64(value << 12, 0, 40); - - tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, - ARMMMUIdxBit_Stage2); -} static void tlbiall_hyp_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) @@ -XXX,XX +XXX,XX @@ static void vttbr_write(CPUARMState *env, const ARMCPRegInfo *ri, tlb_flush_by_mmuidx(cs, ARMMMUIdxBit_E10_1 | ARMMMUIdxBit_E10_1_PAN | - ARMMMUIdxBit_E10_0 | - ARMMMUIdxBit_Stage2); + ARMMMUIdxBit_E10_0); raw_write(env, ri, value); } } @@ -XXX,XX +XXX,XX @@ static int alle1_tlbmask(CPUARMState *env) return ARMMMUIdxBit_SE10_1 | ARMMMUIdxBit_SE10_1_PAN | ARMMMUIdxBit_SE10_0; - } else if (arm_feature(env, ARM_FEATURE_EL2)) { - return ARMMMUIdxBit_E10_1 | - ARMMMUIdxBit_E10_1_PAN | - ARMMMUIdxBit_E10_0 | - ARMMMUIdxBit_Stage2; } else { return ARMMMUIdxBit_E10_1 | ARMMMUIdxBit_E10_1_PAN | @@ -XXX,XX +XXX,XX @@ static void tlbi_aa64_vae3is_write(CPUARMState *env, const ARMCPRegInfo *ri, ARMMMUIdxBit_SE3); } -static void tlbi_aa64_ipas2e1_write(CPUARMState *env, const ARMCPRegInfo *ri, - uint64_t value) -{ - /* Invalidate by IPA. This has to invalidate any structures that - * contain only stage 2 translation information, but does not need - * to apply to structures that contain combined stage 1 and stage 2 - * translation information. - * This must NOP if EL2 isn't implemented or SCR_EL3.NS is zero. - */ - ARMCPU *cpu = env_archcpu(env); - CPUState *cs = CPU(cpu); - uint64_t pageaddr; - - if (!arm_feature(env, ARM_FEATURE_EL2) || !(env->cp15.scr_el3 & SCR_NS)) { - return; - } - - pageaddr = sextract64(value << 12, 0, 48); - - tlb_flush_page_by_mmuidx(cs, pageaddr, ARMMMUIdxBit_Stage2); -} - -static void tlbi_aa64_ipas2e1is_write(CPUARMState *env, const ARMCPRegInfo *ri, - uint64_t value) -{ - CPUState *cs = env_cpu(env); - uint64_t pageaddr; - - if (!arm_feature(env, ARM_FEATURE_EL2) || !(env->cp15.scr_el3 & SCR_NS)) { - return; - } - - pageaddr = sextract64(value << 12, 0, 48); - - tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, - ARMMMUIdxBit_Stage2); -} - static CPAccessResult aa64_zva_access(CPUARMState *env, const ARMCPRegInfo *ri, bool isread) { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v8_cp_reginfo[] = { .writefn = tlbi_aa64_vae1_write }, { .name = "TLBI_IPAS2E1IS", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 1, - .access = PL2_W, .type = ARM_CP_NO_RAW, - .writefn = tlbi_aa64_ipas2e1is_write }, + .access = PL2_W, .type = ARM_CP_NOP }, { .name = "TLBI_IPAS2LE1IS", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 5, - .access = PL2_W, .type = ARM_CP_NO_RAW, - .writefn = tlbi_aa64_ipas2e1is_write }, + .access = PL2_W, .type = ARM_CP_NOP }, { .name = "TLBI_ALLE1IS", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 3, .opc2 = 4, .access = PL2_W, .type = ARM_CP_NO_RAW, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v8_cp_reginfo[] = { .writefn = tlbi_aa64_alle1is_write }, { .name = "TLBI_IPAS2E1", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 1, - .access = PL2_W, .type = ARM_CP_NO_RAW, - .writefn = tlbi_aa64_ipas2e1_write }, + .access = PL2_W, .type = ARM_CP_NOP }, { .name = "TLBI_IPAS2LE1", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 5, - .access = PL2_W, .type = ARM_CP_NO_RAW, - .writefn = tlbi_aa64_ipas2e1_write }, + .access = PL2_W, .type = ARM_CP_NOP }, { .name = "TLBI_ALLE1", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 7, .opc2 = 4, .access = PL2_W, .type = ARM_CP_NO_RAW, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v8_cp_reginfo[] = { .writefn = tlbimva_hyp_is_write }, { .name = "TLBIIPAS2", .cp = 15, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 1, - .type = ARM_CP_NO_RAW, .access = PL2_W, - .writefn = tlbiipas2_write }, + .type = ARM_CP_NOP, .access = PL2_W }, { .name = "TLBIIPAS2IS", .cp = 15, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 1, - .type = ARM_CP_NO_RAW, .access = PL2_W, - .writefn = tlbiipas2_is_write }, + .type = ARM_CP_NOP, .access = PL2_W }, { .name = "TLBIIPAS2L", .cp = 15, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 5, - .type = ARM_CP_NO_RAW, .access = PL2_W, - .writefn = tlbiipas2_write }, + .type = ARM_CP_NOP, .access = PL2_W }, { .name = "TLBIIPAS2LIS", .cp = 15, .opc1 = 4, .crn = 8, .crm = 0, .opc2 = 5, - .type = ARM_CP_NO_RAW, .access = PL2_W, - .writefn = tlbiipas2_is_write }, + .type = ARM_CP_NOP, .access = PL2_W }, /* 32 bit cache operations */ { .name = "ICIALLUIS", .cp = 15, .opc1 = 0, .crn = 7, .crm = 1, .opc2 = 0, .type = ARM_CP_NOP, .access = PL1_W, .accessfn = aa64_cacheop_pou_access }, -- 2.20.1 The access_type argument to get_phys_addr_lpae() is an MMUAccessType; use the enum constant MMU_DATA_LOAD rather than a literal 0 when we call it in S1_ptw_translate(). 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: 20200330210400.11724-3-peter.maydell@linaro.org --- target/arm/helper.c | 5 +++-- 1 file changed, 3 insertions(+), 2 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 hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, pcacheattrs = &cacheattrs; } - ret = get_phys_addr_lpae(env, addr, 0, ARMMMUIdx_Stage2, &s2pa, - &txattrs, &s2prot, &s2size, fi, pcacheattrs); + ret = get_phys_addr_lpae(env, addr, MMU_DATA_LOAD, ARMMMUIdx_Stage2, + &s2pa, &txattrs, &s2prot, &s2size, fi, + pcacheattrs); if (ret) { assert(fi->type != ARMFault_None); fi->s2addr = addr; -- 2.20.1 For ARMv8.2-TTS2UXN, the stage 2 page table walk wants to know whether the stage 1 access is for EL0 or not, because whether exec permission is given can depend on whether this is an EL0 or EL1 access. Add a new argument to get_phys_addr_lpae() so the call sites can pass this information in. Since get_phys_addr_lpae() doesn't already have a doc comment, add one so we have a place to put the documentation of the semantics of the new s1_is_el0 argument. 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: 20200330210400.11724-4-peter.maydell@linaro.org --- target/arm/helper.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 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 bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, MMUAccessType access_type, ARMMMUIdx mmu_idx, + bool s1_is_el0, hwaddr *phys_ptr, MemTxAttrs *txattrs, int *prot, target_ulong *page_size_ptr, ARMMMUFaultInfo *fi, ARMCacheAttrs *cacheattrs); @@ -XXX,XX +XXX,XX @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, } ret = get_phys_addr_lpae(env, addr, MMU_DATA_LOAD, ARMMMUIdx_Stage2, + false, &s2pa, &txattrs, &s2prot, &s2size, fi, pcacheattrs); if (ret) { @@ -XXX,XX +XXX,XX @@ static ARMVAParameters aa32_va_parameters(CPUARMState *env, uint32_t va, }; } +/** + * get_phys_addr_lpae: perform one stage of page table walk, LPAE format + * + * Returns false if the translation was successful. Otherwise, phys_ptr, attrs, + * prot and page_size may not be filled in, and the populated fsr value provides + * information on why the translation aborted, in the format of a long-format + * DFSR/IFSR fault register, with the following caveats: + * * the WnR bit is never set (the caller must do this). + * + * @env: CPUARMState + * @address: virtual address to get physical address for + * @access_type: MMU_DATA_LOAD, MMU_DATA_STORE or MMU_INST_FETCH + * @mmu_idx: MMU index indicating required translation regime + * @s1_is_el0: if @mmu_idx is ARMMMUIdx_Stage2 (so this is a stage 2 page table + * walk), must be true if this is stage 2 of a stage 1+2 walk for an + * EL0 access). If @mmu_idx is anything else, @s1_is_el0 is ignored. + * @phys_ptr: set to the physical address corresponding to the virtual address + * @attrs: set to the memory transaction attributes to use + * @prot: set to the permissions for the page containing phys_ptr + * @page_size_ptr: set to the size of the page containing phys_ptr + * @fi: set to fault info if the translation fails + * @cacheattrs: (if non-NULL) set to the cacheability/shareability attributes + */ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, MMUAccessType access_type, ARMMMUIdx mmu_idx, + bool s1_is_el0, hwaddr *phys_ptr, MemTxAttrs *txattrs, int *prot, target_ulong *page_size_ptr, ARMMMUFaultInfo *fi, ARMCacheAttrs *cacheattrs) @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, /* S1 is done. Now do S2 translation. */ ret = get_phys_addr_lpae(env, ipa, access_type, ARMMMUIdx_Stage2, + mmu_idx == ARMMMUIdx_E10_0, phys_ptr, attrs, &s2_prot, page_size, fi, cacheattrs != NULL ? &cacheattrs2 : NULL); @@ -XXX,XX +XXX,XX @@ bool get_phys_addr(CPUARMState *env, target_ulong address, } if (regime_using_lpae_format(env, mmu_idx)) { - return get_phys_addr_lpae(env, address, access_type, mmu_idx, + return get_phys_addr_lpae(env, address, access_type, mmu_idx, false, phys_ptr, attrs, prot, page_size, fi, cacheattrs); } else if (regime_sctlr(env, mmu_idx) & SCTLR_XP) { -- 2.20.1 The ARMv8.2-TTS2UXN feature extends the XN field in stage 2 translation table descriptors from just bit [54] to bits [54:53], allowing stage 2 to control execution permissions separately for EL0 and EL1. Implement the new semantics of the XN field and enable the feature for our 'max' CPU. 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: 20200330210400.11724-5-peter.maydell@linaro.org --- target/arm/cpu.h | 15 +++++++++++++++ target/arm/cpu.c | 1 + target/arm/cpu64.c | 2 ++ target/arm/helper.c | 37 +++++++++++++++++++++++++++++++------ 4 files changed, 49 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 bool isar_feature_aa32_ccidx(const ARMISARegisters *id) return FIELD_EX32(id->id_mmfr4, ID_MMFR4, CCIDX) != 0; } +static inline bool isar_feature_aa32_tts2uxn(const ARMISARegisters *id) +{ + return FIELD_EX32(id->id_mmfr4, ID_MMFR4, XNX) != 0; +} + /* * 64-bit feature tests via id registers. */ @@ -XXX,XX +XXX,XX @@ static inline bool isar_feature_aa64_ccidx(const ARMISARegisters *id) return FIELD_EX64(id->id_aa64mmfr2, ID_AA64MMFR2, CCIDX) != 0; } +static inline bool isar_feature_aa64_tts2uxn(const ARMISARegisters *id) +{ + return FIELD_EX64(id->id_aa64mmfr1, ID_AA64MMFR1, XNX) != 0; +} + /* * Feature tests for "does this exist in either 32-bit or 64-bit?" */ @@ -XXX,XX +XXX,XX @@ static inline bool isar_feature_any_ccidx(const ARMISARegisters *id) return isar_feature_aa64_ccidx(id) || isar_feature_aa32_ccidx(id); } +static inline bool isar_feature_any_tts2uxn(const ARMISARegisters *id) +{ + return isar_feature_aa64_tts2uxn(id) || isar_feature_aa32_tts2uxn(id); +} + /* * Forward to the above feature tests given an ARMCPU pointer. */ 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_max_initfn(Object *obj) t = FIELD_DP32(t, ID_MMFR4, HPDS, 1); /* AA32HPD */ t = FIELD_DP32(t, ID_MMFR4, AC2, 1); /* ACTLR2, HACTLR2 */ t = FIELD_DP32(t, ID_MMFR4, CNP, 1); /* TTCNP */ + t = FIELD_DP32(t, ID_MMFR4, XNX, 1); /* TTS2UXN */ cpu->isar.id_mmfr4 = t; } #endif diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -XXX,XX +XXX,XX @@ static void aarch64_max_initfn(Object *obj) t = FIELD_DP64(t, ID_AA64MMFR1, VH, 1); t = FIELD_DP64(t, ID_AA64MMFR1, PAN, 2); /* ATS1E1 */ t = FIELD_DP64(t, ID_AA64MMFR1, VMIDBITS, 2); /* VMID16 */ + t = FIELD_DP64(t, ID_AA64MMFR1, XNX, 1); /* TTS2UXN */ cpu->isar.id_aa64mmfr1 = t; t = cpu->isar.id_aa64mmfr2; @@ -XXX,XX +XXX,XX @@ static void aarch64_max_initfn(Object *obj) u = FIELD_DP32(u, ID_MMFR4, HPDS, 1); /* AA32HPD */ u = FIELD_DP32(u, ID_MMFR4, AC2, 1); /* ACTLR2, HACTLR2 */ u = FIELD_DP32(u, ID_MMFR4, CNP, 1); /* TTCNP */ + u = FIELD_DP32(u, ID_MMFR4, XNX, 1); /* TTS2UXN */ cpu->isar.id_mmfr4 = u; u = cpu->isar.id_aa64dfr0; 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 @@ simple_ap_to_rw_prot(CPUARMState *env, ARMMMUIdx mmu_idx, int ap) * * @env: CPUARMState * @s2ap: The 2-bit stage2 access permissions (S2AP) - * @xn: XN (execute-never) bit + * @xn: XN (execute-never) bits + * @s1_is_el0: true if this is S2 of an S1+2 walk for EL0 */ -static int get_S2prot(CPUARMState *env, int s2ap, int xn) +static int get_S2prot(CPUARMState *env, int s2ap, int xn, bool s1_is_el0) { int prot = 0; @@ -XXX,XX +XXX,XX @@ static int get_S2prot(CPUARMState *env, int s2ap, int xn) if (s2ap & 2) { prot |= PAGE_WRITE; } - if (!xn) { - if (arm_el_is_aa64(env, 2) || prot & PAGE_READ) { + + if (cpu_isar_feature(any_tts2uxn, env_archcpu(env))) { + switch (xn) { + case 0: prot |= PAGE_EXEC; + break; + case 1: + if (s1_is_el0) { + prot |= PAGE_EXEC; + } + break; + case 2: + break; + case 3: + if (!s1_is_el0) { + prot |= PAGE_EXEC; + } + break; + default: + g_assert_not_reached(); + } + } else { + if (!extract32(xn, 1, 1)) { + if (arm_el_is_aa64(env, 2) || prot & PAGE_READ) { + prot |= PAGE_EXEC; + } } } return prot; @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, } ap = extract32(attrs, 4, 2); - xn = extract32(attrs, 12, 1); if (mmu_idx == ARMMMUIdx_Stage2) { ns = true; - *prot = get_S2prot(env, ap, xn); + xn = extract32(attrs, 11, 2); + *prot = get_S2prot(env, ap, xn, s1_is_el0); } else { ns = extract32(attrs, 3, 1); + xn = extract32(attrs, 12, 1); pxn = extract32(attrs, 11, 1); *prot = get_S1prot(env, mmu_idx, aarch64, ap, ns, xn, pxn); } -- 2.20.1 In aarch64_max_initfn() we update both 32-bit and 64-bit ID registers. The intended pattern is that for 64-bit ID registers we use FIELD_DP64 and the uint64_t 't' register, while 32-bit ID registers use FIELD_DP32 and the uint32_t 'u' register. For ID_AA64DFR0 we accidentally used 'u', meaning that the top 32 bits of this 64-bit ID register would end up always zero. Luckily at the moment that's what they should be anyway, so this bug has no visible effects. Use the right-sized variable. Fixes: 3bec78447a958d481991 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20200423110915.10527-1-peter.maydell@linaro.org --- target/arm/cpu64.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -XXX,XX +XXX,XX @@ static void aarch64_max_initfn(Object *obj) u = FIELD_DP32(u, ID_MMFR4, XNX, 1); /* TTS2UXN */ cpu->isar.id_mmfr4 = u; - u = cpu->isar.id_aa64dfr0; - u = FIELD_DP64(u, ID_AA64DFR0, PMUVER, 5); /* v8.4-PMU */ - cpu->isar.id_aa64dfr0 = u; + t = cpu->isar.id_aa64dfr0; + t = FIELD_DP64(t, ID_AA64DFR0, PMUVER, 5); /* v8.4-PMU */ + cpu->isar.id_aa64dfr0 = t; u = cpu->isar.id_dfr0; u = FIELD_DP32(u, ID_DFR0, PERFMON, 5); /* v8.4-PMU */ -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> MIDR_EL1 is a 64-bit system register with the top 32-bit being RES0. Represent it in QEMU's ARMCPU struct with a uint64_t, not a uint32_t. This fixes an error when compiling with -Werror=conversion because we were manipulating the register value using a local uint64_t variable: target/arm/cpu64.c: In function ‘aarch64_max_initfn’: target/arm/cpu64.c:628:21: error: conversion from ‘uint64_t’ {aka ‘long unsigned int’} to ‘uint32_t’ {aka ‘unsigned int’} may change value [-Werror=conversion] 628 | cpu->midr = t; | ^ and future-proofs us against a possible future architecture change using some of the top 32 bits. Suggested-by: Laurent Desnogues <laurent.desnogues@gmail.com> Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com> Message-id: 20200428172634.29707-1-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 2 +- target/arm/cpu.c | 2 +- 2 files changed, 2 insertions(+), 2 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 @@ struct ARMCPU { uint64_t id_aa64dfr0; uint64_t id_aa64dfr1; } isar; - uint32_t midr; + uint64_t midr; uint32_t revidr; uint32_t reset_fpsid; uint32_t ctr; 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 const ARMCPUInfo arm_cpus[] = { static Property arm_cpu_properties[] = { DEFINE_PROP_BOOL("start-powered-off", ARMCPU, start_powered_off, false), DEFINE_PROP_UINT32("psci-conduit", ARMCPU, psci_conduit, 0), - DEFINE_PROP_UINT32("midr", ARMCPU, midr, 0), + DEFINE_PROP_UINT64("midr", ARMCPU, midr, 0), DEFINE_PROP_UINT64("mp-affinity", ARMCPU, mp_affinity, ARM64_AFFINITY_INVALID), DEFINE_PROP_INT32("node-id", ARMCPU, node_id, CPU_UNSET_NUMA_NODE_ID), -- 2.20.1 From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> Remove inclusion of arm_gicv3_common.h, this already gets included via xlnx-versal.h. Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Luc Michel <luc.michel@greensocs.com> Message-id: 20200427181649.26851-2-edgar.iglesias@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/xlnx-versal.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/xlnx-versal.c +++ b/hw/arm/xlnx-versal.c @@ -XXX,XX +XXX,XX @@ #include "hw/arm/boot.h" #include "kvm_arm.h" #include "hw/misc/unimp.h" -#include "hw/intc/arm_gicv3_common.h" #include "hw/arm/xlnx-versal.h" #include "hw/char/pl011.h" -- 2.20.1 From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> Move misplaced comment. Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc.michel@greensocs.com> Message-id: 20200427181649.26851-3-edgar.iglesias@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/xlnx-versal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/xlnx-versal.c +++ b/hw/arm/xlnx-versal.c @@ -XXX,XX +XXX,XX @@ static void versal_create_apu_cpus(Versal *s) obj = object_new(XLNX_VERSAL_ACPU_TYPE); if (!obj) { - /* Secondary CPUs start in PSCI powered-down state */ error_report("Unable to create apu.cpu[%d] of type %s", i, XLNX_VERSAL_ACPU_TYPE); exit(EXIT_FAILURE); @@ -XXX,XX +XXX,XX @@ static void versal_create_apu_cpus(Versal *s) object_property_set_int(obj, s->cfg.psci_conduit, "psci-conduit", &error_abort); if (i) { + /* Secondary CPUs start in PSCI powered-down state */ object_property_set_bool(obj, true, "start-powered-off", &error_abort); } -- 2.20.1 From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> Fix typo xlnx-ve -> xlnx-versal. Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc.michel@greensocs.com> Message-id: 20200427181649.26851-4-edgar.iglesias@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/xlnx-versal-virt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/xlnx-versal-virt.c +++ b/hw/arm/xlnx-versal-virt.c @@ -XXX,XX +XXX,XX @@ static void versal_virt_init(MachineState *machine) psci_conduit = QEMU_PSCI_CONDUIT_SMC; } - sysbus_init_child_obj(OBJECT(machine), "xlnx-ve", &s->soc, + sysbus_init_child_obj(OBJECT(machine), "xlnx-versal", &s->soc, sizeof(s->soc), TYPE_XLNX_VERSAL); object_property_set_link(OBJECT(&s->soc), OBJECT(machine->ram), "ddr", &error_abort); -- 2.20.1 From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> Embed the UARTs into the SoC type. Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc.michel@greensocs.com> Message-id: 20200427181649.26851-5-edgar.iglesias@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/xlnx-versal.h | 3 ++- hw/arm/xlnx-versal.c | 12 ++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/xlnx-versal.h +++ b/include/hw/arm/xlnx-versal.h @@ -XXX,XX +XXX,XX @@ #include "hw/sysbus.h" #include "hw/arm/boot.h" #include "hw/intc/arm_gicv3.h" +#include "hw/char/pl011.h" #define TYPE_XLNX_VERSAL "xlnx-versal" #define XLNX_VERSAL(obj) OBJECT_CHECK(Versal, (obj), TYPE_XLNX_VERSAL) @@ -XXX,XX +XXX,XX @@ typedef struct Versal { MemoryRegion mr_ocm; struct { - SysBusDevice *uart[XLNX_VERSAL_NR_UARTS]; + PL011State uart[XLNX_VERSAL_NR_UARTS]; SysBusDevice *gem[XLNX_VERSAL_NR_GEMS]; SysBusDevice *adma[XLNX_VERSAL_NR_ADMAS]; } iou; diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/xlnx-versal.c +++ b/hw/arm/xlnx-versal.c @@ -XXX,XX +XXX,XX @@ #include "kvm_arm.h" #include "hw/misc/unimp.h" #include "hw/arm/xlnx-versal.h" -#include "hw/char/pl011.h" #define XLNX_VERSAL_ACPU_TYPE ARM_CPU_TYPE_NAME("cortex-a72") #define GEM_REVISION 0x40070106 @@ -XXX,XX +XXX,XX @@ static void versal_create_uarts(Versal *s, qemu_irq *pic) DeviceState *dev; MemoryRegion *mr; - dev = qdev_create(NULL, TYPE_PL011); - s->lpd.iou.uart[i] = SYS_BUS_DEVICE(dev); + sysbus_init_child_obj(OBJECT(s), name, + &s->lpd.iou.uart[i], sizeof(s->lpd.iou.uart[i]), + TYPE_PL011); + dev = DEVICE(&s->lpd.iou.uart[i]); qdev_prop_set_chr(dev, "chardev", serial_hd(i)); - object_property_add_child(OBJECT(s), name, OBJECT(dev), &error_fatal); qdev_init_nofail(dev); - mr = sysbus_mmio_get_region(s->lpd.iou.uart[i], 0); + mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0); memory_region_add_subregion(&s->mr_ps, addrs[i], mr); - sysbus_connect_irq(s->lpd.iou.uart[i], 0, pic[irqs[i]]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irqs[i]]); g_free(name); } } -- 2.20.1 From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> Embed the GEMs into the SoC type. Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc.michel@greensocs.com> Message-id: 20200427181649.26851-6-edgar.iglesias@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/xlnx-versal.h | 3 ++- hw/arm/xlnx-versal.c | 15 ++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/xlnx-versal.h +++ b/include/hw/arm/xlnx-versal.h @@ -XXX,XX +XXX,XX @@ #include "hw/arm/boot.h" #include "hw/intc/arm_gicv3.h" #include "hw/char/pl011.h" +#include "hw/net/cadence_gem.h" #define TYPE_XLNX_VERSAL "xlnx-versal" #define XLNX_VERSAL(obj) OBJECT_CHECK(Versal, (obj), TYPE_XLNX_VERSAL) @@ -XXX,XX +XXX,XX @@ typedef struct Versal { struct { PL011State uart[XLNX_VERSAL_NR_UARTS]; - SysBusDevice *gem[XLNX_VERSAL_NR_GEMS]; + CadenceGEMState gem[XLNX_VERSAL_NR_GEMS]; SysBusDevice *adma[XLNX_VERSAL_NR_ADMAS]; } iou; } lpd; diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/xlnx-versal.c +++ b/hw/arm/xlnx-versal.c @@ -XXX,XX +XXX,XX @@ static void versal_create_gems(Versal *s, qemu_irq *pic) DeviceState *dev; MemoryRegion *mr; - dev = qdev_create(NULL, "cadence_gem"); - s->lpd.iou.gem[i] = SYS_BUS_DEVICE(dev); - object_property_add_child(OBJECT(s), name, OBJECT(dev), &error_fatal); + sysbus_init_child_obj(OBJECT(s), name, + &s->lpd.iou.gem[i], sizeof(s->lpd.iou.gem[i]), + TYPE_CADENCE_GEM); + dev = DEVICE(&s->lpd.iou.gem[i]); if (nd->used) { qemu_check_nic_model(nd, "cadence_gem"); qdev_set_nic_properties(dev, nd); } - object_property_set_int(OBJECT(s->lpd.iou.gem[i]), + object_property_set_int(OBJECT(dev), 2, "num-priority-queues", &error_abort); - object_property_set_link(OBJECT(s->lpd.iou.gem[i]), + object_property_set_link(OBJECT(dev), OBJECT(&s->mr_ps), "dma", &error_abort); qdev_init_nofail(dev); - mr = sysbus_mmio_get_region(s->lpd.iou.gem[i], 0); + mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0); memory_region_add_subregion(&s->mr_ps, addrs[i], mr); - sysbus_connect_irq(s->lpd.iou.gem[i], 0, pic[irqs[i]]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irqs[i]]); g_free(name); } } -- 2.20.1 From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> Embed the ADMAs into the SoC type. Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc.michel@greensocs.com> Message-id: 20200427181649.26851-7-edgar.iglesias@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/xlnx-versal.h | 3 ++- hw/arm/xlnx-versal.c | 14 +++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/xlnx-versal.h +++ b/include/hw/arm/xlnx-versal.h @@ -XXX,XX +XXX,XX @@ #include "hw/arm/boot.h" #include "hw/intc/arm_gicv3.h" #include "hw/char/pl011.h" +#include "hw/dma/xlnx-zdma.h" #include "hw/net/cadence_gem.h" #define TYPE_XLNX_VERSAL "xlnx-versal" @@ -XXX,XX +XXX,XX @@ typedef struct Versal { struct { PL011State uart[XLNX_VERSAL_NR_UARTS]; CadenceGEMState gem[XLNX_VERSAL_NR_GEMS]; - SysBusDevice *adma[XLNX_VERSAL_NR_ADMAS]; + XlnxZDMA adma[XLNX_VERSAL_NR_ADMAS]; } iou; } lpd; diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/xlnx-versal.c +++ b/hw/arm/xlnx-versal.c @@ -XXX,XX +XXX,XX @@ static void versal_create_admas(Versal *s, qemu_irq *pic) DeviceState *dev; MemoryRegion *mr; - dev = qdev_create(NULL, "xlnx.zdma"); - s->lpd.iou.adma[i] = SYS_BUS_DEVICE(dev); - object_property_set_int(OBJECT(s->lpd.iou.adma[i]), 128, "bus-width", - &error_abort); - object_property_add_child(OBJECT(s), name, OBJECT(dev), &error_fatal); + sysbus_init_child_obj(OBJECT(s), name, + &s->lpd.iou.adma[i], sizeof(s->lpd.iou.adma[i]), + TYPE_XLNX_ZDMA); + dev = DEVICE(&s->lpd.iou.adma[i]); + object_property_set_int(OBJECT(dev), 128, "bus-width", &error_abort); qdev_init_nofail(dev); - mr = sysbus_mmio_get_region(s->lpd.iou.adma[i], 0); + mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0); memory_region_add_subregion(&s->mr_ps, MM_ADMA_CH0 + i * MM_ADMA_CH0_SIZE, mr); - sysbus_connect_irq(s->lpd.iou.adma[i], 0, pic[VERSAL_ADMA_IRQ_0 + i]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[VERSAL_ADMA_IRQ_0 + i]); g_free(name); } } -- 2.20.1 From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> Embed the APUs into the SoC type. Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc.michel@greensocs.com> Message-id: 20200427181649.26851-8-edgar.iglesias@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/xlnx-versal.h | 2 +- hw/arm/xlnx-versal-virt.c | 4 ++-- hw/arm/xlnx-versal.c | 19 +++++-------------- 3 files changed, 8 insertions(+), 17 deletions(-) diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/xlnx-versal.h +++ b/include/hw/arm/xlnx-versal.h @@ -XXX,XX +XXX,XX @@ typedef struct Versal { struct { struct { MemoryRegion mr; - ARMCPU *cpu[XLNX_VERSAL_NR_ACPUS]; + ARMCPU cpu[XLNX_VERSAL_NR_ACPUS]; GICv3State gic; } apu; } fpd; diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/xlnx-versal-virt.c +++ b/hw/arm/xlnx-versal-virt.c @@ -XXX,XX +XXX,XX @@ static void versal_virt_init(MachineState *machine) s->binfo.get_dtb = versal_virt_get_dtb; s->binfo.modify_dtb = versal_virt_modify_dtb; if (machine->kernel_filename) { - arm_load_kernel(s->soc.fpd.apu.cpu[0], machine, &s->binfo); + arm_load_kernel(&s->soc.fpd.apu.cpu[0], machine, &s->binfo); } else { - AddressSpace *as = arm_boot_address_space(s->soc.fpd.apu.cpu[0], + AddressSpace *as = arm_boot_address_space(&s->soc.fpd.apu.cpu[0], &s->binfo); /* Some boot-loaders (e.g u-boot) don't like blobs at address 0 (NULL). * Offset things by 4K. */ diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/xlnx-versal.c +++ b/hw/arm/xlnx-versal.c @@ -XXX,XX +XXX,XX @@ static void versal_create_apu_cpus(Versal *s) for (i = 0; i < ARRAY_SIZE(s->fpd.apu.cpu); i++) { Object *obj; - char *name; - - obj = object_new(XLNX_VERSAL_ACPU_TYPE); - if (!obj) { - error_report("Unable to create apu.cpu[%d] of type %s", - i, XLNX_VERSAL_ACPU_TYPE); - exit(EXIT_FAILURE); - } - - name = g_strdup_printf("apu-cpu[%d]", i); - object_property_add_child(OBJECT(s), name, obj, &error_fatal); - g_free(name); + object_initialize_child(OBJECT(s), "apu-cpu[*]", + &s->fpd.apu.cpu[i], sizeof(s->fpd.apu.cpu[i]), + XLNX_VERSAL_ACPU_TYPE, &error_abort, NULL); + obj = OBJECT(&s->fpd.apu.cpu[i]); object_property_set_int(obj, s->cfg.psci_conduit, "psci-conduit", &error_abort); if (i) { @@ -XXX,XX +XXX,XX @@ static void versal_create_apu_cpus(Versal *s) object_property_set_link(obj, OBJECT(&s->fpd.apu.mr), "memory", &error_abort); object_property_set_bool(obj, true, "realized", &error_fatal); - s->fpd.apu.cpu[i] = ARM_CPU(obj); } } @@ -XXX,XX +XXX,XX @@ static void versal_create_apu_gic(Versal *s, qemu_irq *pic) } for (i = 0; i < nr_apu_cpus; i++) { - DeviceState *cpudev = DEVICE(s->fpd.apu.cpu[i]); + DeviceState *cpudev = DEVICE(&s->fpd.apu.cpu[i]); int ppibase = XLNX_VERSAL_NR_IRQS + i * GIC_INTERNAL + GIC_NR_SGIS; qemu_irq maint_irq; int ti; -- 2.20.1 From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> Add support for SD. Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc.michel@greensocs.com> Message-id: 20200427181649.26851-9-edgar.iglesias@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/xlnx-versal.h | 12 ++++++++++++ hw/arm/xlnx-versal.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/xlnx-versal.h +++ b/include/hw/arm/xlnx-versal.h @@ -XXX,XX +XXX,XX @@ #include "hw/sysbus.h" #include "hw/arm/boot.h" +#include "hw/sd/sdhci.h" #include "hw/intc/arm_gicv3.h" #include "hw/char/pl011.h" #include "hw/dma/xlnx-zdma.h" @@ -XXX,XX +XXX,XX @@ #define XLNX_VERSAL_NR_UARTS 2 #define XLNX_VERSAL_NR_GEMS 2 #define XLNX_VERSAL_NR_ADMAS 8 +#define XLNX_VERSAL_NR_SDS 2 #define XLNX_VERSAL_NR_IRQS 192 typedef struct Versal { @@ -XXX,XX +XXX,XX @@ typedef struct Versal { } iou; } lpd; + /* The Platform Management Controller subsystem. */ + struct { + struct { + SDHCIState sd[XLNX_VERSAL_NR_SDS]; + } iou; + } pmc; + struct { MemoryRegion *mr_ddr; uint32_t psci_conduit; @@ -XXX,XX +XXX,XX @@ typedef struct Versal { #define VERSAL_GEM1_IRQ_0 58 #define VERSAL_GEM1_WAKE_IRQ_0 59 #define VERSAL_ADMA_IRQ_0 60 +#define VERSAL_SD0_IRQ_0 126 /* Architecturally reserved IRQs suitable for virtualization. */ #define VERSAL_RSVD_IRQ_FIRST 111 @@ -XXX,XX +XXX,XX @@ typedef struct Versal { #define MM_FPD_CRF 0xfd1a0000U #define MM_FPD_CRF_SIZE 0x140000 +#define MM_PMC_SD0 0xf1040000U +#define MM_PMC_SD0_SIZE 0x10000 #define MM_PMC_CRP 0xf1260000U #define MM_PMC_CRP_SIZE 0x10000 #endif diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/xlnx-versal.c +++ b/hw/arm/xlnx-versal.c @@ -XXX,XX +XXX,XX @@ static void versal_create_admas(Versal *s, qemu_irq *pic) } } +#define SDHCI_CAPABILITIES 0x280737ec6481 /* Same as on ZynqMP. */ +static void versal_create_sds(Versal *s, qemu_irq *pic) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(s->pmc.iou.sd); i++) { + DeviceState *dev; + MemoryRegion *mr; + + sysbus_init_child_obj(OBJECT(s), "sd[*]", + &s->pmc.iou.sd[i], sizeof(s->pmc.iou.sd[i]), + TYPE_SYSBUS_SDHCI); + dev = DEVICE(&s->pmc.iou.sd[i]); + + object_property_set_uint(OBJECT(dev), + 3, "sd-spec-version", &error_fatal); + object_property_set_uint(OBJECT(dev), SDHCI_CAPABILITIES, "capareg", + &error_fatal); + object_property_set_uint(OBJECT(dev), UHS_I, "uhs", &error_fatal); + qdev_init_nofail(dev); + + mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0); + memory_region_add_subregion(&s->mr_ps, + MM_PMC_SD0 + i * MM_PMC_SD0_SIZE, mr); + + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, + pic[VERSAL_SD0_IRQ_0 + i * 2]); + } +} + /* This takes the board allocated linear DDR memory and creates aliases * for each split DDR range/aperture on the Versal address map. */ @@ -XXX,XX +XXX,XX @@ static void versal_realize(DeviceState *dev, Error **errp) versal_create_uarts(s, pic); versal_create_gems(s, pic); versal_create_admas(s, pic); + versal_create_sds(s, pic); versal_map_ddr(s); versal_unimp(s); -- 2.20.1 From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> hw/arm: versal: Add support for the RTC. Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Luc Michel <luc.michel@greensocs.com> Message-id: 20200427181649.26851-10-edgar.iglesias@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/xlnx-versal.h | 8 ++++++++ hw/arm/xlnx-versal.c | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/include/hw/arm/xlnx-versal.h b/include/hw/arm/xlnx-versal.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/xlnx-versal.h +++ b/include/hw/arm/xlnx-versal.h @@ -XXX,XX +XXX,XX @@ #include "hw/char/pl011.h" #include "hw/dma/xlnx-zdma.h" #include "hw/net/cadence_gem.h" +#include "hw/rtc/xlnx-zynqmp-rtc.h" #define TYPE_XLNX_VERSAL "xlnx-versal" #define XLNX_VERSAL(obj) OBJECT_CHECK(Versal, (obj), TYPE_XLNX_VERSAL) @@ -XXX,XX +XXX,XX @@ typedef struct Versal { struct { SDHCIState sd[XLNX_VERSAL_NR_SDS]; } iou; + + XlnxZynqMPRTC rtc; } pmc; struct { @@ -XXX,XX +XXX,XX @@ typedef struct Versal { #define VERSAL_GEM1_IRQ_0 58 #define VERSAL_GEM1_WAKE_IRQ_0 59 #define VERSAL_ADMA_IRQ_0 60 +#define VERSAL_RTC_APB_ERR_IRQ 121 #define VERSAL_SD0_IRQ_0 126 +#define VERSAL_RTC_ALARM_IRQ 142 +#define VERSAL_RTC_SECONDS_IRQ 143 /* Architecturally reserved IRQs suitable for virtualization. */ #define VERSAL_RSVD_IRQ_FIRST 111 @@ -XXX,XX +XXX,XX @@ typedef struct Versal { #define MM_PMC_SD0_SIZE 0x10000 #define MM_PMC_CRP 0xf1260000U #define MM_PMC_CRP_SIZE 0x10000 +#define MM_PMC_RTC 0xf12a0000 +#define MM_PMC_RTC_SIZE 0x10000 #endif diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/xlnx-versal.c +++ b/hw/arm/xlnx-versal.c @@ -XXX,XX +XXX,XX @@ static void versal_create_sds(Versal *s, qemu_irq *pic) } } +static void versal_create_rtc(Versal *s, qemu_irq *pic) +{ + SysBusDevice *sbd; + MemoryRegion *mr; + + sysbus_init_child_obj(OBJECT(s), "rtc", &s->pmc.rtc, sizeof(s->pmc.rtc), + TYPE_XLNX_ZYNQMP_RTC); + sbd = SYS_BUS_DEVICE(&s->pmc.rtc); + qdev_init_nofail(DEVICE(sbd)); + + mr = sysbus_mmio_get_region(sbd, 0); + memory_region_add_subregion(&s->mr_ps, MM_PMC_RTC, mr); + + /* + * TODO: Connect the ALARM and SECONDS interrupts once our RTC model + * supports them. + */ + sysbus_connect_irq(sbd, 1, pic[VERSAL_RTC_APB_ERR_IRQ]); +} + /* This takes the board allocated linear DDR memory and creates aliases * for each split DDR range/aperture on the Versal address map. */ @@ -XXX,XX +XXX,XX @@ static void versal_realize(DeviceState *dev, Error **errp) versal_create_gems(s, pic); versal_create_admas(s, pic); versal_create_sds(s, pic); + versal_create_rtc(s, pic); versal_map_ddr(s); versal_unimp(s); -- 2.20.1 From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> Add support for SD. Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Luc Michel <luc.michel@greensocs.com> Message-id: 20200427181649.26851-11-edgar.iglesias@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/xlnx-versal-virt.c | 46 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/xlnx-versal-virt.c +++ b/hw/arm/xlnx-versal-virt.c @@ -XXX,XX +XXX,XX @@ #include "hw/arm/sysbus-fdt.h" #include "hw/arm/fdt.h" #include "cpu.h" +#include "hw/qdev-properties.h" #include "hw/arm/xlnx-versal.h" #define TYPE_XLNX_VERSAL_VIRT_MACHINE MACHINE_TYPE_NAME("xlnx-versal-virt") @@ -XXX,XX +XXX,XX @@ static void fdt_add_zdma_nodes(VersalVirt *s) } } +static void fdt_add_sd_nodes(VersalVirt *s) +{ + const char clocknames[] = "clk_xin\0clk_ahb"; + const char compat[] = "arasan,sdhci-8.9a"; + int i; + + for (i = ARRAY_SIZE(s->soc.pmc.iou.sd) - 1; i >= 0; i--) { + uint64_t addr = MM_PMC_SD0 + MM_PMC_SD0_SIZE * i; + char *name = g_strdup_printf("/sdhci@%" PRIx64, addr); + + qemu_fdt_add_subnode(s->fdt, name); + + qemu_fdt_setprop_cells(s->fdt, name, "clocks", + s->phandle.clk_25Mhz, s->phandle.clk_25Mhz); + qemu_fdt_setprop(s->fdt, name, "clock-names", + clocknames, sizeof(clocknames)); + qemu_fdt_setprop_cells(s->fdt, name, "interrupts", + GIC_FDT_IRQ_TYPE_SPI, VERSAL_SD0_IRQ_0 + i * 2, + GIC_FDT_IRQ_FLAGS_LEVEL_HI); + qemu_fdt_setprop_sized_cells(s->fdt, name, "reg", + 2, addr, 2, MM_PMC_SD0_SIZE); + qemu_fdt_setprop(s->fdt, name, "compatible", compat, sizeof(compat)); + g_free(name); + } +} + static void fdt_nop_memory_nodes(void *fdt, Error **errp) { Error *err = NULL; @@ -XXX,XX +XXX,XX @@ static void create_virtio_regions(VersalVirt *s) } } +static void sd_plugin_card(SDHCIState *sd, DriveInfo *di) +{ + BlockBackend *blk = di ? blk_by_legacy_dinfo(di) : NULL; + DeviceState *card; + + card = qdev_create(qdev_get_child_bus(DEVICE(sd), "sd-bus"), TYPE_SD_CARD); + object_property_add_child(OBJECT(sd), "card[*]", OBJECT(card), + &error_fatal); + qdev_prop_set_drive(card, "drive", blk, &error_fatal); + object_property_set_bool(OBJECT(card), true, "realized", &error_fatal); +} + static void versal_virt_init(MachineState *machine) { VersalVirt *s = XLNX_VERSAL_VIRT_MACHINE(machine); int psci_conduit = QEMU_PSCI_CONDUIT_DISABLED; + int i; /* * If the user provides an Operating System to be loaded, we expect them @@ -XXX,XX +XXX,XX @@ static void versal_virt_init(MachineState *machine) fdt_add_gic_nodes(s); fdt_add_timer_nodes(s); fdt_add_zdma_nodes(s); + fdt_add_sd_nodes(s); fdt_add_cpu_nodes(s, psci_conduit); fdt_add_clk_node(s, "/clk125", 125000000, s->phandle.clk_125Mhz); fdt_add_clk_node(s, "/clk25", 25000000, s->phandle.clk_25Mhz); @@ -XXX,XX +XXX,XX @@ static void versal_virt_init(MachineState *machine) memory_region_add_subregion_overlap(get_system_memory(), 0, &s->soc.fpd.apu.mr, 0); + /* Plugin SD cards. */ + for (i = 0; i < ARRAY_SIZE(s->soc.pmc.iou.sd); i++) { + sd_plugin_card(&s->soc.pmc.iou.sd[i], drive_get_next(IF_SD)); + } + s->binfo.ram_size = machine->ram_size; s->binfo.loader_start = 0x0; s->binfo.get_dtb = versal_virt_get_dtb; -- 2.20.1 From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> Add support for the RTC. Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Luc Michel <luc.michel@greensocs.com> Message-id: 20200427181649.26851-12-edgar.iglesias@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/xlnx-versal-virt.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/xlnx-versal-virt.c +++ b/hw/arm/xlnx-versal-virt.c @@ -XXX,XX +XXX,XX @@ static void fdt_add_sd_nodes(VersalVirt *s) } } +static void fdt_add_rtc_node(VersalVirt *s) +{ + const char compat[] = "xlnx,zynqmp-rtc"; + const char interrupt_names[] = "alarm\0sec"; + char *name = g_strdup_printf("/rtc@%x", MM_PMC_RTC); + + qemu_fdt_add_subnode(s->fdt, name); + + qemu_fdt_setprop_cells(s->fdt, name, "interrupts", + GIC_FDT_IRQ_TYPE_SPI, VERSAL_RTC_ALARM_IRQ, + GIC_FDT_IRQ_FLAGS_LEVEL_HI, + GIC_FDT_IRQ_TYPE_SPI, VERSAL_RTC_SECONDS_IRQ, + GIC_FDT_IRQ_FLAGS_LEVEL_HI); + qemu_fdt_setprop(s->fdt, name, "interrupt-names", + interrupt_names, sizeof(interrupt_names)); + qemu_fdt_setprop_sized_cells(s->fdt, name, "reg", + 2, MM_PMC_RTC, 2, MM_PMC_RTC_SIZE); + qemu_fdt_setprop(s->fdt, name, "compatible", compat, sizeof(compat)); + g_free(name); +} + static void fdt_nop_memory_nodes(void *fdt, Error **errp) { Error *err = NULL; @@ -XXX,XX +XXX,XX @@ static void versal_virt_init(MachineState *machine) fdt_add_timer_nodes(s); fdt_add_zdma_nodes(s); fdt_add_sd_nodes(s); + fdt_add_rtc_node(s); fdt_add_cpu_nodes(s, psci_conduit); fdt_add_clk_node(s, "/clk125", 125000000, s->phandle.clk_125Mhz); fdt_add_clk_node(s, "/clk25", 25000000, s->phandle.clk_25Mhz); -- 2.20.1 Somewhere along theline we accidentally added a duplicate "using D16-D31 when they don't exist" check to do_vfm_dp() (probably an artifact of a patchseries rebase). Remove it. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20200430181003.21682-2-peter.maydell@linaro.org --- target/arm/translate-vfp.inc.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-vfp.inc.c +++ b/target/arm/translate-vfp.inc.c @@ -XXX,XX +XXX,XX @@ static bool do_vfm_dp(DisasContext *s, arg_VFMA_dp *a, bool neg_n, bool neg_d) return false; } - /* UNDEF accesses to D16-D31 if they don't exist. */ - if (!dc_isar_feature(aa32_simd_r32, s) && - ((a->vd | a->vn | a->vm) & 0x10)) { - return false; - } - if (!vfp_access_check(s)) { return true; } -- 2.20.1 We were accidentally permitting decode of Thumb Neon insns even if the CPU didn't have the FEATURE_NEON bit set, because the feature check was being done before the call to disas_neon_data_insn() and disas_neon_ls_insn() in the Arm decoder but was omitted from the Thumb decoder. Push the feature bit check down into the called functions so it is done for both Arm and Thumb encodings. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20200430181003.21682-3-peter.maydell@linaro.org --- target/arm/translate.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 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_neon_ls_insn(DisasContext *s, uint32_t insn) TCGv_i32 tmp2; TCGv_i64 tmp64; + if (!arm_dc_feature(s, ARM_FEATURE_NEON)) { + return 1; + } + /* FIXME: this access check should not take precedence over UNDEF * for invalid encodings; we will generate incorrect syndrome information * for attempts to execute invalid vfp/neon encodings with FP disabled. @@ -XXX,XX +XXX,XX @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn) TCGv_ptr ptr1, ptr2, ptr3; TCGv_i64 tmp64; + if (!arm_dc_feature(s, ARM_FEATURE_NEON)) { + return 1; + } + /* FIXME: this access check should not take precedence over UNDEF * for invalid encodings; we will generate incorrect syndrome information * for attempts to execute invalid vfp/neon encodings with FP disabled. @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) if (((insn >> 25) & 7) == 1) { /* NEON Data processing. */ - if (!arm_dc_feature(s, ARM_FEATURE_NEON)) { - goto illegal_op; - } - if (disas_neon_data_insn(s, insn)) { goto illegal_op; } @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) } if ((insn & 0x0f100000) == 0x04000000) { /* NEON load/store. */ - if (!arm_dc_feature(s, ARM_FEATURE_NEON)) { - goto illegal_op; - } - if (disas_neon_ls_insn(s, insn)) { goto illegal_op; } -- 2.20.1 Add the infrastructure for building and invoking a decodetree decoder for the AArch32 Neon encodings. At the moment the new decoder covers nothing, so we always fall back to the existing hand-written decode. We follow the same pattern we did for the VFP decodetree conversion (commit 78e138bc1f672c145ef6ace74617d and following): code that deals with Neon will be moving gradually out to translate-neon.vfp.inc, which we #include into translate.c. In order to share the decode files between A32 and T32, we split Neon into 3 parts: * data-processing * load-store * 'shared' encodings The first two groups of instructions have similar but not identical A32 and T32 encodings, so we need to manually transform the T32 encoding into the A32 one before calling the decoder; the third group covers the Neon instructions which are identical in A32 and T32. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-4-peter.maydell@linaro.org --- target/arm/neon-dp.decode | 29 ++++++++++++++++++++++++++ target/arm/neon-ls.decode | 29 ++++++++++++++++++++++++++ target/arm/neon-shared.decode | 27 +++++++++++++++++++++++++ target/arm/translate-neon.inc.c | 32 +++++++++++++++++++++++++++++ target/arm/translate.c | 36 +++++++++++++++++++++++++++++++-- target/arm/Makefile.objs | 18 +++++++++++++++++ 6 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 target/arm/neon-dp.decode create mode 100644 target/arm/neon-ls.decode create mode 100644 target/arm/neon-shared.decode create mode 100644 target/arm/translate-neon.inc.c diff --git a/target/arm/neon-dp.decode b/target/arm/neon-dp.decode new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/target/arm/neon-dp.decode @@ -XXX,XX +XXX,XX @@ +# AArch32 Neon data-processing instruction descriptions +# +# Copyright (c) 2020 Linaro, Ltd +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, see <http://www.gnu.org/licenses/>. + +# +# This file is processed by scripts/decodetree.py +# + +# Encodings for Neon data processing instructions where the T32 encoding +# is a simple transformation of the A32 encoding. +# More specifically, this file covers instructions where the A32 encoding is +# 0b1111_001p_qqqq_qqqq_qqqq_qqqq_qqqq_qqqq +# and the T32 encoding is +# 0b111p_1111_qqqq_qqqq_qqqq_qqqq_qqqq_qqqq +# This file works on the A32 encoding only; calling code for T32 has to +# transform the insn into the A32 version first. diff --git a/target/arm/neon-ls.decode b/target/arm/neon-ls.decode new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/target/arm/neon-ls.decode @@ -XXX,XX +XXX,XX @@ +# AArch32 Neon load/store instruction descriptions +# +# Copyright (c) 2020 Linaro, Ltd +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, see <http://www.gnu.org/licenses/>. + +# +# This file is processed by scripts/decodetree.py +# + +# Encodings for Neon load/store instructions where the T32 encoding +# is a simple transformation of the A32 encoding. +# More specifically, this file covers instructions where the A32 encoding is +# 0b1111_0100_xxx0_xxxx_xxxx_xxxx_xxxx_xxxx +# and the T32 encoding is +# 0b1111_1001_xxx0_xxxx_xxxx_xxxx_xxxx_xxxx +# This file works on the A32 encoding only; calling code for T32 has to +# transform the insn into the A32 version first. diff --git a/target/arm/neon-shared.decode b/target/arm/neon-shared.decode new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/target/arm/neon-shared.decode @@ -XXX,XX +XXX,XX @@ +# AArch32 Neon instruction descriptions +# +# Copyright (c) 2020 Linaro, Ltd +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, see <http://www.gnu.org/licenses/>. + +# +# This file is processed by scripts/decodetree.py +# + +# Encodings for Neon instructions whose encoding is the same for +# both A32 and T32. + +# More specifically, this covers: +# 2reg scalar ext: 0b1111_1110_xxxx_xxxx_xxxx_1x0x_xxxx_xxxx +# 3same ext: 0b1111_110x_xxxx_xxxx_xxxx_1x0x_xxxx_xxxx diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/target/arm/translate-neon.inc.c @@ -XXX,XX +XXX,XX @@ +/* + * ARM translation: AArch32 Neon instructions + * + * Copyright (c) 2003 Fabrice Bellard + * Copyright (c) 2005-2007 CodeSourcery + * Copyright (c) 2007 OpenedHand, Ltd. + * Copyright (c) 2020 Linaro, Ltd. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see <http://www.gnu.org/licenses/>. + */ + +/* + * This file is intended to be included from translate.c; it uses + * some macros and definitions provided by that file. + * It might be possible to convert it to a standalone .c file eventually. + */ + +/* Include the generated Neon decoder */ +#include "decode-neon-dp.inc.c" +#include "decode-neon-ls.inc.c" +#include "decode-neon-shared.inc.c" 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 TCGv_ptr vfp_reg_ptr(bool dp, int reg) #define ARM_CP_RW_BIT (1 << 20) -/* Include the VFP decoder */ +/* Include the VFP and Neon decoders */ #include "translate-vfp.inc.c" +#include "translate-neon.inc.c" static inline void iwmmxt_load_reg(TCGv_i64 var, int reg) { @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) /* Unconditional instructions. */ /* TODO: Perhaps merge these into one decodetree output file. */ if (disas_a32_uncond(s, insn) || - disas_vfp_uncond(s, insn)) { + disas_vfp_uncond(s, insn) || + disas_neon_dp(s, insn) || + disas_neon_ls(s, insn) || + disas_neon_shared(s, insn)) { return; } /* fall back to legacy decoder */ @@ -XXX,XX +XXX,XX @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) ARCH(6T2); } + if ((insn & 0xef000000) == 0xef000000) { + /* + * T32 encodings 0b111p_1111_qqqq_qqqq_qqqq_qqqq_qqqq_qqqq + * transform into + * A32 encodings 0b1111_001p_qqqq_qqqq_qqqq_qqqq_qqqq_qqqq + */ + uint32_t a32_insn = (insn & 0xe2ffffff) | + ((insn & (1 << 28)) >> 4) | (1 << 28); + + if (disas_neon_dp(s, a32_insn)) { + return; + } + } + + if ((insn & 0xff100000) == 0xf9000000) { + /* + * T32 encodings 0b1111_1001_ppp0_qqqq_qqqq_qqqq_qqqq_qqqq + * transform into + * A32 encodings 0b1111_0100_ppp0_qqqq_qqqq_qqqq_qqqq_qqqq + */ + uint32_t a32_insn = (insn & 0x00ffffff) | 0xf4000000; + + if (disas_neon_ls(s, a32_insn)) { + return; + } + } + /* * TODO: Perhaps merge these into one decodetree output file. * Note disas_vfp is written for a32 with cond field in the @@ -XXX,XX +XXX,XX @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) */ if (disas_t32(s, insn) || disas_vfp_uncond(s, insn) || + disas_neon_shared(s, insn) || ((insn >> 28) == 0xe && disas_vfp(s, insn))) { return; } diff --git a/target/arm/Makefile.objs b/target/arm/Makefile.objs index XXXXXXX..XXXXXXX 100644 --- a/target/arm/Makefile.objs +++ b/target/arm/Makefile.objs @@ -XXX,XX +XXX,XX @@ target/arm/decode-sve.inc.c: $(SRC_PATH)/target/arm/sve.decode $(DECODETREE) $(PYTHON) $(DECODETREE) --decode disas_sve -o $@ $<,\ "GEN", $(TARGET_DIR)$@) +target/arm/decode-neon-shared.inc.c: $(SRC_PATH)/target/arm/neon-shared.decode $(DECODETREE) + $(call quiet-command,\ + $(PYTHON) $(DECODETREE) --static-decode disas_neon_shared -o $@ $<,\ + "GEN", $(TARGET_DIR)$@) + +target/arm/decode-neon-dp.inc.c: $(SRC_PATH)/target/arm/neon-dp.decode $(DECODETREE) + $(call quiet-command,\ + $(PYTHON) $(DECODETREE) --static-decode disas_neon_dp -o $@ $<,\ + "GEN", $(TARGET_DIR)$@) + +target/arm/decode-neon-ls.inc.c: $(SRC_PATH)/target/arm/neon-ls.decode $(DECODETREE) + $(call quiet-command,\ + $(PYTHON) $(DECODETREE) --static-decode disas_neon_ls -o $@ $<,\ + "GEN", $(TARGET_DIR)$@) + target/arm/decode-vfp.inc.c: $(SRC_PATH)/target/arm/vfp.decode $(DECODETREE) $(call quiet-command,\ $(PYTHON) $(DECODETREE) --static-decode disas_vfp -o $@ $<,\ @@ -XXX,XX +XXX,XX @@ target/arm/decode-t16.inc.c: $(SRC_PATH)/target/arm/t16.decode $(DECODETREE) "GEN", $(TARGET_DIR)$@) target/arm/translate-sve.o: target/arm/decode-sve.inc.c +target/arm/translate.o: target/arm/decode-neon-shared.inc.c +target/arm/translate.o: target/arm/decode-neon-dp.inc.c +target/arm/translate.o: target/arm/decode-neon-ls.inc.c target/arm/translate.o: target/arm/decode-vfp.inc.c target/arm/translate.o: target/arm/decode-vfp-uncond.inc.c target/arm/translate.o: target/arm/decode-a32.inc.c -- 2.20.1 Convert the VCMLA (vector) insns in the 3same extension group to decodetree. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-5-peter.maydell@linaro.org --- target/arm/neon-shared.decode | 11 ++++++++++ target/arm/translate-neon.inc.c | 37 +++++++++++++++++++++++++++++++++ target/arm/translate.c | 11 +--------- 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/target/arm/neon-shared.decode b/target/arm/neon-shared.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/neon-shared.decode +++ b/target/arm/neon-shared.decode @@ -XXX,XX +XXX,XX @@ # More specifically, this covers: # 2reg scalar ext: 0b1111_1110_xxxx_xxxx_xxxx_1x0x_xxxx_xxxx # 3same ext: 0b1111_110x_xxxx_xxxx_xxxx_1x0x_xxxx_xxxx + +# VFP/Neon register fields; same as vfp.decode +%vm_dp 5:1 0:4 +%vm_sp 0:4 5:1 +%vn_dp 7:1 16:4 +%vn_sp 16:4 7:1 +%vd_dp 22:1 12:4 +%vd_sp 12:4 22:1 + +VCMLA 1111 110 rot:2 . 1 size:1 .... .... 1000 . q:1 . 0 .... \ + vm=%vm_dp vn=%vn_dp vd=%vd_dp diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.inc.c +++ b/target/arm/translate-neon.inc.c @@ -XXX,XX +XXX,XX @@ #include "decode-neon-dp.inc.c" #include "decode-neon-ls.inc.c" #include "decode-neon-shared.inc.c" + +static bool trans_VCMLA(DisasContext *s, arg_VCMLA *a) +{ + int opr_sz; + TCGv_ptr fpst; + gen_helper_gvec_3_ptr *fn_gvec_ptr; + + if (!dc_isar_feature(aa32_vcma, s) + || (!a->size && !dc_isar_feature(aa32_fp16_arith, s))) { + return false; + } + + /* UNDEF accesses to D16-D31 if they don't exist. */ + if (!dc_isar_feature(aa32_simd_r32, s) && + ((a->vd | a->vn | a->vm) & 0x10)) { + return false; + } + + if ((a->vn | a->vm | a->vd) & a->q) { + return false; + } + + if (!vfp_access_check(s)) { + return true; + } + + opr_sz = (1 + a->q) * 8; + fpst = get_fpstatus_ptr(1); + fn_gvec_ptr = a->size ? gen_helper_gvec_fcmlas : gen_helper_gvec_fcmlah; + tcg_gen_gvec_3_ptr(vfp_reg_offset(1, a->vd), + vfp_reg_offset(1, a->vn), + vfp_reg_offset(1, a->vm), + fpst, opr_sz, opr_sz, a->rot, + fn_gvec_ptr); + tcg_temp_free_ptr(fpst); + return true; +} 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_neon_insn_3same_ext(DisasContext *s, uint32_t insn) bool is_long = false, q = extract32(insn, 6, 1); bool ptr_is_env = false; - if ((insn & 0xfe200f10) == 0xfc200800) { - /* VCMLA -- 1111 110R R.1S .... .... 1000 ...0 .... */ - int size = extract32(insn, 20, 1); - data = extract32(insn, 23, 2); /* rot */ - if (!dc_isar_feature(aa32_vcma, s) - || (!size && !dc_isar_feature(aa32_fp16_arith, s))) { - return 1; - } - fn_gvec_ptr = size ? gen_helper_gvec_fcmlas : gen_helper_gvec_fcmlah; - } else if ((insn & 0xfea00f10) == 0xfc800800) { + if ((insn & 0xfea00f10) == 0xfc800800) { /* VCADD -- 1111 110R 1.0S .... .... 1000 ...0 .... */ int size = extract32(insn, 20, 1); data = extract32(insn, 24, 1); /* rot */ -- 2.20.1 Convert the VCADD (vector) insns to decodetree. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-6-peter.maydell@linaro.org --- target/arm/neon-shared.decode | 3 +++ target/arm/translate-neon.inc.c | 37 +++++++++++++++++++++++++++++++++ target/arm/translate.c | 11 +--------- 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/target/arm/neon-shared.decode b/target/arm/neon-shared.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/neon-shared.decode +++ b/target/arm/neon-shared.decode @@ -XXX,XX +XXX,XX @@ VCMLA 1111 110 rot:2 . 1 size:1 .... .... 1000 . q:1 . 0 .... \ vm=%vm_dp vn=%vn_dp vd=%vd_dp + +VCADD 1111 110 rot:1 1 . 0 size:1 .... .... 1000 . q:1 . 0 .... \ + vm=%vm_dp vn=%vn_dp vd=%vd_dp diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.inc.c +++ b/target/arm/translate-neon.inc.c @@ -XXX,XX +XXX,XX @@ static bool trans_VCMLA(DisasContext *s, arg_VCMLA *a) tcg_temp_free_ptr(fpst); return true; } + +static bool trans_VCADD(DisasContext *s, arg_VCADD *a) +{ + int opr_sz; + TCGv_ptr fpst; + gen_helper_gvec_3_ptr *fn_gvec_ptr; + + if (!dc_isar_feature(aa32_vcma, s) + || (!a->size && !dc_isar_feature(aa32_fp16_arith, s))) { + return false; + } + + /* UNDEF accesses to D16-D31 if they don't exist. */ + if (!dc_isar_feature(aa32_simd_r32, s) && + ((a->vd | a->vn | a->vm) & 0x10)) { + return false; + } + + if ((a->vn | a->vm | a->vd) & a->q) { + return false; + } + + if (!vfp_access_check(s)) { + return true; + } + + opr_sz = (1 + a->q) * 8; + fpst = get_fpstatus_ptr(1); + fn_gvec_ptr = a->size ? gen_helper_gvec_fcadds : gen_helper_gvec_fcaddh; + tcg_gen_gvec_3_ptr(vfp_reg_offset(1, a->vd), + vfp_reg_offset(1, a->vn), + vfp_reg_offset(1, a->vm), + fpst, opr_sz, opr_sz, a->rot, + fn_gvec_ptr); + tcg_temp_free_ptr(fpst); + return true; +} 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_neon_insn_3same_ext(DisasContext *s, uint32_t insn) bool is_long = false, q = extract32(insn, 6, 1); bool ptr_is_env = false; - if ((insn & 0xfea00f10) == 0xfc800800) { - /* VCADD -- 1111 110R 1.0S .... .... 1000 ...0 .... */ - int size = extract32(insn, 20, 1); - data = extract32(insn, 24, 1); /* rot */ - if (!dc_isar_feature(aa32_vcma, s) - || (!size && !dc_isar_feature(aa32_fp16_arith, s))) { - return 1; - } - fn_gvec_ptr = size ? gen_helper_gvec_fcadds : gen_helper_gvec_fcaddh; - } else if ((insn & 0xfeb00f00) == 0xfc200d00) { + if ((insn & 0xfeb00f00) == 0xfc200d00) { /* V[US]DOT -- 1111 1100 0.10 .... .... 1101 .Q.U .... */ bool u = extract32(insn, 4, 1); if (!dc_isar_feature(aa32_dp, s)) { -- 2.20.1 Convert the V[US]DOT (vector) insns to decodetree. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-7-peter.maydell@linaro.org --- target/arm/neon-shared.decode | 4 ++++ target/arm/translate-neon.inc.c | 32 ++++++++++++++++++++++++++++++++ target/arm/translate.c | 9 +-------- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/target/arm/neon-shared.decode b/target/arm/neon-shared.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/neon-shared.decode +++ b/target/arm/neon-shared.decode @@ -XXX,XX +XXX,XX @@ VCMLA 1111 110 rot:2 . 1 size:1 .... .... 1000 . q:1 . 0 .... \ VCADD 1111 110 rot:1 1 . 0 size:1 .... .... 1000 . q:1 . 0 .... \ vm=%vm_dp vn=%vn_dp vd=%vd_dp + +# VUDOT and VSDOT +VDOT 1111 110 00 . 10 .... .... 1101 . q:1 . u:1 .... \ + vm=%vm_dp vn=%vn_dp vd=%vd_dp diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.inc.c +++ b/target/arm/translate-neon.inc.c @@ -XXX,XX +XXX,XX @@ static bool trans_VCADD(DisasContext *s, arg_VCADD *a) tcg_temp_free_ptr(fpst); return true; } + +static bool trans_VDOT(DisasContext *s, arg_VDOT *a) +{ + int opr_sz; + gen_helper_gvec_3 *fn_gvec; + + if (!dc_isar_feature(aa32_dp, s)) { + return false; + } + + /* UNDEF accesses to D16-D31 if they don't exist. */ + if (!dc_isar_feature(aa32_simd_r32, s) && + ((a->vd | a->vn | a->vm) & 0x10)) { + return false; + } + + if ((a->vn | a->vm | a->vd) & a->q) { + return false; + } + + if (!vfp_access_check(s)) { + return true; + } + + opr_sz = (1 + a->q) * 8; + fn_gvec = a->u ? gen_helper_gvec_udot_b : gen_helper_gvec_sdot_b; + tcg_gen_gvec_3_ool(vfp_reg_offset(1, a->vd), + vfp_reg_offset(1, a->vn), + vfp_reg_offset(1, a->vm), + opr_sz, opr_sz, 0, fn_gvec); + return true; +} 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_neon_insn_3same_ext(DisasContext *s, uint32_t insn) bool is_long = false, q = extract32(insn, 6, 1); bool ptr_is_env = false; - if ((insn & 0xfeb00f00) == 0xfc200d00) { - /* V[US]DOT -- 1111 1100 0.10 .... .... 1101 .Q.U .... */ - bool u = extract32(insn, 4, 1); - if (!dc_isar_feature(aa32_dp, s)) { - return 1; - } - fn_gvec = u ? gen_helper_gvec_udot_b : gen_helper_gvec_sdot_b; - } else if ((insn & 0xff300f10) == 0xfc200810) { + if ((insn & 0xff300f10) == 0xfc200810) { /* VFM[AS]L -- 1111 1100 S.10 .... .... 1000 .Q.1 .... */ int is_s = extract32(insn, 23, 1); if (!dc_isar_feature(aa32_fhm, s)) { -- 2.20.1 Convert the VFM[AS]L (vector) insns to decodetree. This is the last insn in the legacy decoder for the 3same_ext group, so we can delete the legacy decoder function for the group entirely. Note that in disas_thumb2_insn() the parts of this encoding space where the decodetree decoder returns false will correctly be directed to illegal_op by the "(insn & (1 << 28))" check so they won't fall into disas_coproc_insn() by mistake. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-8-peter.maydell@linaro.org --- target/arm/neon-shared.decode | 6 +++ target/arm/translate-neon.inc.c | 31 +++++++++++ target/arm/translate.c | 92 +-------------------------------- 3 files changed, 38 insertions(+), 91 deletions(-) diff --git a/target/arm/neon-shared.decode b/target/arm/neon-shared.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/neon-shared.decode +++ b/target/arm/neon-shared.decode @@ -XXX,XX +XXX,XX @@ VCADD 1111 110 rot:1 1 . 0 size:1 .... .... 1000 . q:1 . 0 .... \ # VUDOT and VSDOT VDOT 1111 110 00 . 10 .... .... 1101 . q:1 . u:1 .... \ vm=%vm_dp vn=%vn_dp vd=%vd_dp + +# VFM[AS]L +VFML 1111 110 0 s:1 . 10 .... .... 1000 . 0 . 1 .... \ + vm=%vm_sp vn=%vn_sp vd=%vd_dp q=0 +VFML 1111 110 0 s:1 . 10 .... .... 1000 . 1 . 1 .... \ + vm=%vm_dp vn=%vn_dp vd=%vd_dp q=1 diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.inc.c +++ b/target/arm/translate-neon.inc.c @@ -XXX,XX +XXX,XX @@ static bool trans_VDOT(DisasContext *s, arg_VDOT *a) opr_sz, opr_sz, 0, fn_gvec); return true; } + +static bool trans_VFML(DisasContext *s, arg_VFML *a) +{ + int opr_sz; + + if (!dc_isar_feature(aa32_fhm, s)) { + return false; + } + + /* UNDEF accesses to D16-D31 if they don't exist. */ + if (!dc_isar_feature(aa32_simd_r32, s) && + (a->vd & 0x10)) { + return false; + } + + if (a->vd & a->q) { + return false; + } + + if (!vfp_access_check(s)) { + return true; + } + + opr_sz = (1 + a->q) * 8; + tcg_gen_gvec_3_ptr(vfp_reg_offset(1, a->vd), + vfp_reg_offset(a->q, a->vn), + vfp_reg_offset(a->q, a->vm), + cpu_env, opr_sz, opr_sz, a->s, /* is_2 == 0 */ + gen_helper_gvec_fmlal_a32); + return true; +} 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_neon_data_insn(DisasContext *s, uint32_t insn) return 0; } -/* Advanced SIMD three registers of the same length extension. - * 31 25 23 22 20 16 12 11 10 9 8 3 0 - * +---------------+-----+---+-----+----+----+---+----+---+----+---------+----+ - * | 1 1 1 1 1 1 0 | op1 | D | op2 | Vn | Vd | 1 | o3 | 0 | o4 | N Q M U | Vm | - * +---------------+-----+---+-----+----+----+---+----+---+----+---------+----+ - */ -static int disas_neon_insn_3same_ext(DisasContext *s, uint32_t insn) -{ - gen_helper_gvec_3 *fn_gvec = NULL; - gen_helper_gvec_3_ptr *fn_gvec_ptr = NULL; - int rd, rn, rm, opr_sz; - int data = 0; - int off_rn, off_rm; - bool is_long = false, q = extract32(insn, 6, 1); - bool ptr_is_env = false; - - if ((insn & 0xff300f10) == 0xfc200810) { - /* VFM[AS]L -- 1111 1100 S.10 .... .... 1000 .Q.1 .... */ - int is_s = extract32(insn, 23, 1); - if (!dc_isar_feature(aa32_fhm, s)) { - return 1; - } - is_long = true; - data = is_s; /* is_2 == 0 */ - fn_gvec_ptr = gen_helper_gvec_fmlal_a32; - ptr_is_env = true; - } else { - return 1; - } - - VFP_DREG_D(rd, insn); - if (rd & q) { - return 1; - } - if (q || !is_long) { - VFP_DREG_N(rn, insn); - VFP_DREG_M(rm, insn); - if ((rn | rm) & q & !is_long) { - return 1; - } - off_rn = vfp_reg_offset(1, rn); - off_rm = vfp_reg_offset(1, rm); - } else { - rn = VFP_SREG_N(insn); - rm = VFP_SREG_M(insn); - off_rn = vfp_reg_offset(0, rn); - off_rm = vfp_reg_offset(0, rm); - } - - if (s->fp_excp_el) { - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_simd_access_trap(1, 0xe, false), s->fp_excp_el); - return 0; - } - if (!s->vfp_enabled) { - return 1; - } - - opr_sz = (1 + q) * 8; - if (fn_gvec_ptr) { - TCGv_ptr ptr; - if (ptr_is_env) { - ptr = cpu_env; - } else { - ptr = get_fpstatus_ptr(1); - } - tcg_gen_gvec_3_ptr(vfp_reg_offset(1, rd), off_rn, off_rm, ptr, - opr_sz, opr_sz, data, fn_gvec_ptr); - if (!ptr_is_env) { - tcg_temp_free_ptr(ptr); - } - } else { - tcg_gen_gvec_3_ool(vfp_reg_offset(1, rd), off_rn, off_rm, - opr_sz, opr_sz, data, fn_gvec); - } - return 0; -} - /* Advanced SIMD two registers and a scalar extension. * 31 24 23 22 20 16 12 11 10 9 8 3 0 * +-----------------+----+---+----+----+----+---+----+---+----+---------+----+ @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) } } } - } else if ((insn & 0x0e000a00) == 0x0c000800 - && arm_dc_feature(s, ARM_FEATURE_V8)) { - if (disas_neon_insn_3same_ext(s, insn)) { - goto illegal_op; - } - return; } else if ((insn & 0x0f000a00) == 0x0e000800 && arm_dc_feature(s, ARM_FEATURE_V8)) { if (disas_neon_insn_2reg_scalar_ext(s, insn)) { @@ -XXX,XX +XXX,XX @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) } break; } - if ((insn & 0xfe000a00) == 0xfc000800 + if ((insn & 0xff000a00) == 0xfe000800 && arm_dc_feature(s, ARM_FEATURE_V8)) { /* The Thumb2 and ARM encodings are identical. */ - if (disas_neon_insn_3same_ext(s, insn)) { - goto illegal_op; - } - } else if ((insn & 0xff000a00) == 0xfe000800 - && arm_dc_feature(s, ARM_FEATURE_V8)) { - /* The Thumb2 and ARM encodings are identical. */ if (disas_neon_insn_2reg_scalar_ext(s, insn)) { goto illegal_op; } -- 2.20.1 Convert VCMLA (scalar) in the 2reg-scalar-ext group to decodetree. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-9-peter.maydell@linaro.org --- target/arm/neon-shared.decode | 5 +++++ target/arm/translate-neon.inc.c | 40 +++++++++++++++++++++++++++++++++ target/arm/translate.c | 26 +-------------------- 3 files changed, 46 insertions(+), 25 deletions(-) diff --git a/target/arm/neon-shared.decode b/target/arm/neon-shared.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/neon-shared.decode +++ b/target/arm/neon-shared.decode @@ -XXX,XX +XXX,XX @@ VFML 1111 110 0 s:1 . 10 .... .... 1000 . 0 . 1 .... \ vm=%vm_sp vn=%vn_sp vd=%vd_dp q=0 VFML 1111 110 0 s:1 . 10 .... .... 1000 . 1 . 1 .... \ vm=%vm_dp vn=%vn_dp vd=%vd_dp q=1 + +VCMLA_scalar 1111 1110 0 . rot:2 .... .... 1000 . q:1 index:1 0 vm:4 \ + vn=%vn_dp vd=%vd_dp size=0 +VCMLA_scalar 1111 1110 1 . rot:2 .... .... 1000 . q:1 . 0 .... \ + vm=%vm_dp vn=%vn_dp vd=%vd_dp size=1 index=0 diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.inc.c +++ b/target/arm/translate-neon.inc.c @@ -XXX,XX +XXX,XX @@ static bool trans_VFML(DisasContext *s, arg_VFML *a) gen_helper_gvec_fmlal_a32); return true; } + +static bool trans_VCMLA_scalar(DisasContext *s, arg_VCMLA_scalar *a) +{ + gen_helper_gvec_3_ptr *fn_gvec_ptr; + int opr_sz; + TCGv_ptr fpst; + + if (!dc_isar_feature(aa32_vcma, s)) { + return false; + } + if (a->size == 0 && !dc_isar_feature(aa32_fp16_arith, s)) { + return false; + } + + /* UNDEF accesses to D16-D31 if they don't exist. */ + if (!dc_isar_feature(aa32_simd_r32, s) && + ((a->vd | a->vn | a->vm) & 0x10)) { + return false; + } + + if ((a->vd | a->vn) & a->q) { + return false; + } + + if (!vfp_access_check(s)) { + return true; + } + + fn_gvec_ptr = (a->size ? gen_helper_gvec_fcmlas_idx + : gen_helper_gvec_fcmlah_idx); + opr_sz = (1 + a->q) * 8; + fpst = get_fpstatus_ptr(1); + tcg_gen_gvec_3_ptr(vfp_reg_offset(1, a->vd), + vfp_reg_offset(1, a->vn), + vfp_reg_offset(1, a->vm), + fpst, opr_sz, opr_sz, + (a->index << 2) | a->rot, fn_gvec_ptr); + tcg_temp_free_ptr(fpst); + return true; +} 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_neon_insn_2reg_scalar_ext(DisasContext *s, uint32_t insn) bool is_long = false, q = extract32(insn, 6, 1); bool ptr_is_env = false; - if ((insn & 0xff000f10) == 0xfe000800) { - /* VCMLA (indexed) -- 1111 1110 S.RR .... .... 1000 ...0 .... */ - int rot = extract32(insn, 20, 2); - int size = extract32(insn, 23, 1); - int index; - - if (!dc_isar_feature(aa32_vcma, s)) { - return 1; - } - if (size == 0) { - if (!dc_isar_feature(aa32_fp16_arith, s)) { - return 1; - } - /* For fp16, rm is just Vm, and index is M. */ - rm = extract32(insn, 0, 4); - index = extract32(insn, 5, 1); - } else { - /* For fp32, rm is the usual M:Vm, and index is 0. */ - VFP_DREG_M(rm, insn); - index = 0; - } - data = (index << 2) | rot; - fn_gvec_ptr = (size ? gen_helper_gvec_fcmlas_idx - : gen_helper_gvec_fcmlah_idx); - } else if ((insn & 0xffb00f00) == 0xfe200d00) { + if ((insn & 0xffb00f00) == 0xfe200d00) { /* V[US]DOT -- 1111 1110 0.10 .... .... 1101 .Q.U .... */ int u = extract32(insn, 4, 1); -- 2.20.1 Convert the V[US]DOT (scalar) insns in the 2reg-scalar-ext group to decodetree. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-10-peter.maydell@linaro.org --- target/arm/neon-shared.decode | 3 +++ target/arm/translate-neon.inc.c | 35 +++++++++++++++++++++++++++++++++ target/arm/translate.c | 13 +----------- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/target/arm/neon-shared.decode b/target/arm/neon-shared.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/neon-shared.decode +++ b/target/arm/neon-shared.decode @@ -XXX,XX +XXX,XX @@ VCMLA_scalar 1111 1110 0 . rot:2 .... .... 1000 . q:1 index:1 0 vm:4 \ vn=%vn_dp vd=%vd_dp size=0 VCMLA_scalar 1111 1110 1 . rot:2 .... .... 1000 . q:1 . 0 .... \ vm=%vm_dp vn=%vn_dp vd=%vd_dp size=1 index=0 + +VDOT_scalar 1111 1110 0 . 10 .... .... 1101 . q:1 index:1 u:1 rm:4 \ + vm=%vm_dp vn=%vn_dp vd=%vd_dp diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.inc.c +++ b/target/arm/translate-neon.inc.c @@ -XXX,XX +XXX,XX @@ static bool trans_VCMLA_scalar(DisasContext *s, arg_VCMLA_scalar *a) tcg_temp_free_ptr(fpst); return true; } + +static bool trans_VDOT_scalar(DisasContext *s, arg_VDOT_scalar *a) +{ + gen_helper_gvec_3 *fn_gvec; + int opr_sz; + TCGv_ptr fpst; + + if (!dc_isar_feature(aa32_dp, s)) { + return false; + } + + /* UNDEF accesses to D16-D31 if they don't exist. */ + if (!dc_isar_feature(aa32_simd_r32, s) && + ((a->vd | a->vn) & 0x10)) { + return false; + } + + if ((a->vd | a->vn) & a->q) { + return false; + } + + if (!vfp_access_check(s)) { + return true; + } + + fn_gvec = a->u ? gen_helper_gvec_udot_idx_b : gen_helper_gvec_sdot_idx_b; + opr_sz = (1 + a->q) * 8; + fpst = get_fpstatus_ptr(1); + tcg_gen_gvec_3_ool(vfp_reg_offset(1, a->vd), + vfp_reg_offset(1, a->vn), + vfp_reg_offset(1, a->rm), + opr_sz, opr_sz, a->index, fn_gvec); + tcg_temp_free_ptr(fpst); + return true; +} 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_neon_insn_2reg_scalar_ext(DisasContext *s, uint32_t insn) bool is_long = false, q = extract32(insn, 6, 1); bool ptr_is_env = false; - if ((insn & 0xffb00f00) == 0xfe200d00) { - /* V[US]DOT -- 1111 1110 0.10 .... .... 1101 .Q.U .... */ - int u = extract32(insn, 4, 1); - - if (!dc_isar_feature(aa32_dp, s)) { - return 1; - } - fn_gvec = u ? gen_helper_gvec_udot_idx_b : gen_helper_gvec_sdot_idx_b; - /* rm is just Vm, and index is M. */ - data = extract32(insn, 5, 1); /* index */ - rm = extract32(insn, 0, 4); - } else if ((insn & 0xffa00f10) == 0xfe000810) { + if ((insn & 0xffa00f10) == 0xfe000810) { /* VFM[AS]L -- 1111 1110 0.0S .... .... 1000 .Q.1 .... */ int is_s = extract32(insn, 20, 1); int vm20 = extract32(insn, 0, 3); -- 2.20.1 Convert the VFM[AS]L (scalar) insns in the 2reg-scalar-ext group to decodetree. These are the last ones in the group so we can remove all the legacy decode for the group. Note that in disas_thumb2_insn() the parts of this encoding space where the decodetree decoder returns false will correctly be directed to illegal_op by the "(insn & (1 << 28))" check so they won't fall into disas_coproc_insn() by mistake. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-11-peter.maydell@linaro.org --- target/arm/neon-shared.decode | 7 +++ target/arm/translate-neon.inc.c | 32 ++++++++++ target/arm/translate.c | 107 +------------------------------- 3 files changed, 40 insertions(+), 106 deletions(-) diff --git a/target/arm/neon-shared.decode b/target/arm/neon-shared.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/neon-shared.decode +++ b/target/arm/neon-shared.decode @@ -XXX,XX +XXX,XX @@ VCMLA_scalar 1111 1110 1 . rot:2 .... .... 1000 . q:1 . 0 .... \ VDOT_scalar 1111 1110 0 . 10 .... .... 1101 . q:1 index:1 u:1 rm:4 \ vm=%vm_dp vn=%vn_dp vd=%vd_dp + +%vfml_scalar_q0_rm 0:3 5:1 +%vfml_scalar_q1_index 5:1 3:1 +VFML_scalar 1111 1110 0 . 0 s:1 .... .... 1000 . 0 . 1 index:1 ... \ + rm=%vfml_scalar_q0_rm vn=%vn_sp vd=%vd_dp q=0 +VFML_scalar 1111 1110 0 . 0 s:1 .... .... 1000 . 1 . 1 . rm:3 \ + index=%vfml_scalar_q1_index vn=%vn_dp vd=%vd_dp q=1 diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.inc.c +++ b/target/arm/translate-neon.inc.c @@ -XXX,XX +XXX,XX @@ static bool trans_VDOT_scalar(DisasContext *s, arg_VDOT_scalar *a) tcg_temp_free_ptr(fpst); return true; } + +static bool trans_VFML_scalar(DisasContext *s, arg_VFML_scalar *a) +{ + int opr_sz; + + if (!dc_isar_feature(aa32_fhm, s)) { + return false; + } + + /* UNDEF accesses to D16-D31 if they don't exist. */ + if (!dc_isar_feature(aa32_simd_r32, s) && + ((a->vd & 0x10) || (a->q && (a->vn & 0x10)))) { + return false; + } + + if (a->vd & a->q) { + return false; + } + + if (!vfp_access_check(s)) { + return true; + } + + opr_sz = (1 + a->q) * 8; + tcg_gen_gvec_3_ptr(vfp_reg_offset(1, a->vd), + vfp_reg_offset(a->q, a->vn), + vfp_reg_offset(a->q, a->rm), + cpu_env, opr_sz, opr_sz, + (a->index << 2) | a->s, /* is_2 == 0 */ + gen_helper_gvec_fmlal_idx_a32); + return true; +} 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_dsp_insn(DisasContext *s, uint32_t insn) } #define VFP_REG_SHR(x, n) (((n) > 0) ? (x) >> (n) : (x) << -(n)) -#define VFP_SREG(insn, bigbit, smallbit) \ - ((VFP_REG_SHR(insn, bigbit - 1) & 0x1e) | (((insn) >> (smallbit)) & 1)) #define VFP_DREG(reg, insn, bigbit, smallbit) do { \ if (dc_isar_feature(aa32_simd_r32, s)) { \ reg = (((insn) >> (bigbit)) & 0x0f) \ @@ -XXX,XX +XXX,XX @@ static int disas_dsp_insn(DisasContext *s, uint32_t insn) reg = ((insn) >> (bigbit)) & 0x0f; \ }} while (0) -#define VFP_SREG_D(insn) VFP_SREG(insn, 12, 22) #define VFP_DREG_D(reg, insn) VFP_DREG(reg, insn, 12, 22) -#define VFP_SREG_N(insn) VFP_SREG(insn, 16, 7) #define VFP_DREG_N(reg, insn) VFP_DREG(reg, insn, 16, 7) -#define VFP_SREG_M(insn) VFP_SREG(insn, 0, 5) #define VFP_DREG_M(reg, insn) VFP_DREG(reg, insn, 0, 5) static void gen_neon_dup_low16(TCGv_i32 var) @@ -XXX,XX +XXX,XX @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn) return 0; } -/* Advanced SIMD two registers and a scalar extension. - * 31 24 23 22 20 16 12 11 10 9 8 3 0 - * +-----------------+----+---+----+----+----+---+----+---+----+---------+----+ - * | 1 1 1 1 1 1 1 0 | o1 | D | o2 | Vn | Vd | 1 | o3 | 0 | o4 | N Q M U | Vm | - * +-----------------+----+---+----+----+----+---+----+---+----+---------+----+ - * - */ - -static int disas_neon_insn_2reg_scalar_ext(DisasContext *s, uint32_t insn) -{ - gen_helper_gvec_3 *fn_gvec = NULL; - gen_helper_gvec_3_ptr *fn_gvec_ptr = NULL; - int rd, rn, rm, opr_sz, data; - int off_rn, off_rm; - bool is_long = false, q = extract32(insn, 6, 1); - bool ptr_is_env = false; - - if ((insn & 0xffa00f10) == 0xfe000810) { - /* VFM[AS]L -- 1111 1110 0.0S .... .... 1000 .Q.1 .... */ - int is_s = extract32(insn, 20, 1); - int vm20 = extract32(insn, 0, 3); - int vm3 = extract32(insn, 3, 1); - int m = extract32(insn, 5, 1); - int index; - - if (!dc_isar_feature(aa32_fhm, s)) { - return 1; - } - if (q) { - rm = vm20; - index = m * 2 + vm3; - } else { - rm = vm20 * 2 + m; - index = vm3; - } - is_long = true; - data = (index << 2) | is_s; /* is_2 == 0 */ - fn_gvec_ptr = gen_helper_gvec_fmlal_idx_a32; - ptr_is_env = true; - } else { - return 1; - } - - VFP_DREG_D(rd, insn); - if (rd & q) { - return 1; - } - if (q || !is_long) { - VFP_DREG_N(rn, insn); - if (rn & q & !is_long) { - return 1; - } - off_rn = vfp_reg_offset(1, rn); - off_rm = vfp_reg_offset(1, rm); - } else { - rn = VFP_SREG_N(insn); - off_rn = vfp_reg_offset(0, rn); - off_rm = vfp_reg_offset(0, rm); - } - if (s->fp_excp_el) { - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_simd_access_trap(1, 0xe, false), s->fp_excp_el); - return 0; - } - if (!s->vfp_enabled) { - return 1; - } - - opr_sz = (1 + q) * 8; - if (fn_gvec_ptr) { - TCGv_ptr ptr; - if (ptr_is_env) { - ptr = cpu_env; - } else { - ptr = get_fpstatus_ptr(1); - } - tcg_gen_gvec_3_ptr(vfp_reg_offset(1, rd), off_rn, off_rm, ptr, - opr_sz, opr_sz, data, fn_gvec_ptr); - if (!ptr_is_env) { - tcg_temp_free_ptr(ptr); - } - } else { - tcg_gen_gvec_3_ool(vfp_reg_offset(1, rd), off_rn, off_rm, - opr_sz, opr_sz, data, fn_gvec); - } - return 0; -} - static int disas_coproc_insn(DisasContext *s, uint32_t insn) { int cpnum, is64, crn, crm, opc1, opc2, isread, rt, rt2; @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) } } } - } else if ((insn & 0x0f000a00) == 0x0e000800 - && arm_dc_feature(s, ARM_FEATURE_V8)) { - if (disas_neon_insn_2reg_scalar_ext(s, insn)) { - goto illegal_op; - } - return; } goto illegal_op; } @@ -XXX,XX +XXX,XX @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) } break; } - if ((insn & 0xff000a00) == 0xfe000800 - && arm_dc_feature(s, ARM_FEATURE_V8)) { - /* The Thumb2 and ARM encodings are identical. */ - if (disas_neon_insn_2reg_scalar_ext(s, insn)) { - goto illegal_op; - } - } else if (((insn >> 24) & 3) == 3) { + if (((insn >> 24) & 3) == 3) { /* Translate into the equivalent ARM encoding. */ insn = (insn & 0xe2ffffff) | ((insn & (1 << 28)) >> 4) | (1 << 28); if (disas_neon_data_insn(s, insn)) { -- 2.20.1 Convert the Neon "load/store multiple structures" insns to decodetree. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-12-peter.maydell@linaro.org --- target/arm/neon-ls.decode | 7 ++ target/arm/translate-neon.inc.c | 124 ++++++++++++++++++++++++++++++++ target/arm/translate.c | 91 +---------------------- 3 files changed, 133 insertions(+), 89 deletions(-) diff --git a/target/arm/neon-ls.decode b/target/arm/neon-ls.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/neon-ls.decode +++ b/target/arm/neon-ls.decode @@ -XXX,XX +XXX,XX @@ # 0b1111_1001_xxx0_xxxx_xxxx_xxxx_xxxx_xxxx # This file works on the A32 encoding only; calling code for T32 has to # transform the insn into the A32 version first. + +%vd_dp 22:1 12:4 + +# Neon load/store multiple structures + +VLDST_multiple 1111 0100 0 . l:1 0 rn:4 .... itype:4 size:2 align:2 rm:4 \ + vd=%vd_dp diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.inc.c +++ b/target/arm/translate-neon.inc.c @@ -XXX,XX +XXX,XX @@ static bool trans_VFML_scalar(DisasContext *s, arg_VFML_scalar *a) gen_helper_gvec_fmlal_idx_a32); return true; } + +static struct { + int nregs; + int interleave; + int spacing; +} const neon_ls_element_type[11] = { + {1, 4, 1}, + {1, 4, 2}, + {4, 1, 1}, + {2, 2, 2}, + {1, 3, 1}, + {1, 3, 2}, + {3, 1, 1}, + {1, 1, 1}, + {1, 2, 1}, + {1, 2, 2}, + {2, 1, 1} +}; + +static void gen_neon_ldst_base_update(DisasContext *s, int rm, int rn, + int stride) +{ + if (rm != 15) { + TCGv_i32 base; + + base = load_reg(s, rn); + if (rm == 13) { + tcg_gen_addi_i32(base, base, stride); + } else { + TCGv_i32 index; + index = load_reg(s, rm); + tcg_gen_add_i32(base, base, index); + tcg_temp_free_i32(index); + } + store_reg(s, rn, base); + } +} + +static bool trans_VLDST_multiple(DisasContext *s, arg_VLDST_multiple *a) +{ + /* Neon load/store multiple structures */ + int nregs, interleave, spacing, reg, n; + MemOp endian = s->be_data; + int mmu_idx = get_mem_index(s); + int size = a->size; + TCGv_i64 tmp64; + TCGv_i32 addr, tmp; + + if (!arm_dc_feature(s, ARM_FEATURE_NEON)) { + return false; + } + + /* UNDEF accesses to D16-D31 if they don't exist */ + if (!dc_isar_feature(aa32_simd_r32, s) && (a->vd & 0x10)) { + return false; + } + if (a->itype > 10) { + return false; + } + /* Catch UNDEF cases for bad values of align field */ + switch (a->itype & 0xc) { + case 4: + if (a->align >= 2) { + return false; + } + break; + case 8: + if (a->align == 3) { + return false; + } + break; + default: + break; + } + nregs = neon_ls_element_type[a->itype].nregs; + interleave = neon_ls_element_type[a->itype].interleave; + spacing = neon_ls_element_type[a->itype].spacing; + if (size == 3 && (interleave | spacing) != 1) { + return false; + } + + if (!vfp_access_check(s)) { + return true; + } + + /* For our purposes, bytes are always little-endian. */ + if (size == 0) { + endian = MO_LE; + } + /* + * Consecutive little-endian elements from a single register + * can be promoted to a larger little-endian operation. + */ + if (interleave == 1 && endian == MO_LE) { + size = 3; + } + tmp64 = tcg_temp_new_i64(); + addr = tcg_temp_new_i32(); + tmp = tcg_const_i32(1 << size); + load_reg_var(s, addr, a->rn); + for (reg = 0; reg < nregs; reg++) { + for (n = 0; n < 8 >> size; n++) { + int xs; + for (xs = 0; xs < interleave; xs++) { + int tt = a->vd + reg + spacing * xs; + + if (a->l) { + gen_aa32_ld_i64(s, tmp64, addr, mmu_idx, endian | size); + neon_store_element64(tt, n, size, tmp64); + } else { + neon_load_element64(tmp64, tt, n, size); + gen_aa32_st_i64(s, tmp64, addr, mmu_idx, endian | size); + } + tcg_gen_add_i32(addr, addr, tmp); + } + } + } + tcg_temp_free_i32(addr); + tcg_temp_free_i32(tmp); + tcg_temp_free_i64(tmp64); + + gen_neon_ldst_base_update(s, a->rm, a->rn, nregs * interleave * 8); + return true; +} 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 void gen_neon_trn_u16(TCGv_i32 t0, TCGv_i32 t1) } -static struct { - int nregs; - int interleave; - int spacing; -} const neon_ls_element_type[11] = { - {1, 4, 1}, - {1, 4, 2}, - {4, 1, 1}, - {2, 2, 2}, - {1, 3, 1}, - {1, 3, 2}, - {3, 1, 1}, - {1, 1, 1}, - {1, 2, 1}, - {1, 2, 2}, - {2, 1, 1} -}; - /* Translate a NEON load/store element instruction. Return nonzero if the instruction is invalid. */ static int disas_neon_ls_insn(DisasContext *s, uint32_t insn) { int rd, rn, rm; - int op; int nregs; - int interleave; - int spacing; int stride; int size; int reg; int load; - int n; int vec_size; - int mmu_idx; - MemOp endian; TCGv_i32 addr; TCGv_i32 tmp; - TCGv_i32 tmp2; - TCGv_i64 tmp64; if (!arm_dc_feature(s, ARM_FEATURE_NEON)) { return 1; @@ -XXX,XX +XXX,XX @@ static int disas_neon_ls_insn(DisasContext *s, uint32_t insn) rn = (insn >> 16) & 0xf; rm = insn & 0xf; load = (insn & (1 << 21)) != 0; - endian = s->be_data; - mmu_idx = get_mem_index(s); if ((insn & (1 << 23)) == 0) { - /* Load store all elements. */ - op = (insn >> 8) & 0xf; - size = (insn >> 6) & 3; - if (op > 10) - return 1; - /* Catch UNDEF cases for bad values of align field */ - switch (op & 0xc) { - case 4: - if (((insn >> 5) & 1) == 1) { - return 1; - } - break; - case 8: - if (((insn >> 4) & 3) == 3) { - return 1; - } - break; - default: - break; - } - nregs = neon_ls_element_type[op].nregs; - interleave = neon_ls_element_type[op].interleave; - spacing = neon_ls_element_type[op].spacing; - if (size == 3 && (interleave | spacing) != 1) { - return 1; - } - /* For our purposes, bytes are always little-endian. */ - if (size == 0) { - endian = MO_LE; - } - /* Consecutive little-endian elements from a single register - * can be promoted to a larger little-endian operation. - */ - if (interleave == 1 && endian == MO_LE) { - size = 3; - } - tmp64 = tcg_temp_new_i64(); - addr = tcg_temp_new_i32(); - tmp2 = tcg_const_i32(1 << size); - load_reg_var(s, addr, rn); - for (reg = 0; reg < nregs; reg++) { - for (n = 0; n < 8 >> size; n++) { - int xs; - for (xs = 0; xs < interleave; xs++) { - int tt = rd + reg + spacing * xs; - - if (load) { - gen_aa32_ld_i64(s, tmp64, addr, mmu_idx, endian | size); - neon_store_element64(tt, n, size, tmp64); - } else { - neon_load_element64(tmp64, tt, n, size); - gen_aa32_st_i64(s, tmp64, addr, mmu_idx, endian | size); - } - tcg_gen_add_i32(addr, addr, tmp2); - } - } - } - tcg_temp_free_i32(addr); - tcg_temp_free_i32(tmp2); - tcg_temp_free_i64(tmp64); - stride = nregs * interleave * 8; + /* Load store all elements -- handled already by decodetree */ + return 1; } else { size = (insn >> 10) & 3; if (size == 3) { -- 2.20.1 Convert the Neon "load single structure to all lanes" insns to decodetree. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-13-peter.maydell@linaro.org --- target/arm/neon-ls.decode | 5 +++ target/arm/translate-neon.inc.c | 73 +++++++++++++++++++++++++++++++++ target/arm/translate.c | 55 +------------------------ 3 files changed, 80 insertions(+), 53 deletions(-) diff --git a/target/arm/neon-ls.decode b/target/arm/neon-ls.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/neon-ls.decode +++ b/target/arm/neon-ls.decode @@ -XXX,XX +XXX,XX @@ VLDST_multiple 1111 0100 0 . l:1 0 rn:4 .... itype:4 size:2 align:2 rm:4 \ vd=%vd_dp + +# Neon load single element to all lanes + +VLD_all_lanes 1111 0100 1 . 1 0 rn:4 .... 11 n:2 size:2 t:1 a:1 rm:4 \ + vd=%vd_dp diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.inc.c +++ b/target/arm/translate-neon.inc.c @@ -XXX,XX +XXX,XX @@ static bool trans_VLDST_multiple(DisasContext *s, arg_VLDST_multiple *a) gen_neon_ldst_base_update(s, a->rm, a->rn, nregs * interleave * 8); return true; } + +static bool trans_VLD_all_lanes(DisasContext *s, arg_VLD_all_lanes *a) +{ + /* Neon load single structure to all lanes */ + int reg, stride, vec_size; + int vd = a->vd; + int size = a->size; + int nregs = a->n + 1; + TCGv_i32 addr, tmp; + + if (!arm_dc_feature(s, ARM_FEATURE_NEON)) { + return false; + } + + /* UNDEF accesses to D16-D31 if they don't exist */ + if (!dc_isar_feature(aa32_simd_r32, s) && (a->vd & 0x10)) { + return false; + } + + if (size == 3) { + if (nregs != 4 || a->a == 0) { + return false; + } + /* For VLD4 size == 3 a == 1 means 32 bits at 16 byte alignment */ + size = 2; + } + if (nregs == 1 && a->a == 1 && size == 0) { + return false; + } + if (nregs == 3 && a->a == 1) { + return false; + } + + if (!vfp_access_check(s)) { + return true; + } + + /* + * VLD1 to all lanes: T bit indicates how many Dregs to write. + * VLD2/3/4 to all lanes: T bit indicates register stride. + */ + stride = a->t ? 2 : 1; + vec_size = nregs == 1 ? stride * 8 : 8; + + tmp = tcg_temp_new_i32(); + addr = tcg_temp_new_i32(); + load_reg_var(s, addr, a->rn); + for (reg = 0; reg < nregs; reg++) { + gen_aa32_ld_i32(s, tmp, addr, get_mem_index(s), + s->be_data | size); + if ((vd & 1) && vec_size == 16) { + /* + * We cannot write 16 bytes at once because the + * destination is unaligned. + */ + tcg_gen_gvec_dup_i32(size, neon_reg_offset(vd, 0), + 8, 8, tmp); + tcg_gen_gvec_mov(0, neon_reg_offset(vd + 1, 0), + neon_reg_offset(vd, 0), 8, 8); + } else { + tcg_gen_gvec_dup_i32(size, neon_reg_offset(vd, 0), + vec_size, vec_size, tmp); + } + tcg_gen_addi_i32(addr, addr, 1 << size); + vd += stride; + } + tcg_temp_free_i32(tmp); + tcg_temp_free_i32(addr); + + gen_neon_ldst_base_update(s, a->rm, a->rn, (1 << size) * nregs); + + return true; +} 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_neon_ls_insn(DisasContext *s, uint32_t insn) int size; int reg; int load; - int vec_size; TCGv_i32 addr; TCGv_i32 tmp; @@ -XXX,XX +XXX,XX @@ static int disas_neon_ls_insn(DisasContext *s, uint32_t insn) } else { size = (insn >> 10) & 3; if (size == 3) { - /* Load single element to all lanes. */ - int a = (insn >> 4) & 1; - if (!load) { - return 1; - } - size = (insn >> 6) & 3; - nregs = ((insn >> 8) & 3) + 1; - - if (size == 3) { - if (nregs != 4 || a == 0) { - return 1; - } - /* For VLD4 size==3 a == 1 means 32 bits at 16 byte alignment */ - size = 2; - } - if (nregs == 1 && a == 1 && size == 0) { - return 1; - } - if (nregs == 3 && a == 1) { - return 1; - } - addr = tcg_temp_new_i32(); - load_reg_var(s, addr, rn); - - /* VLD1 to all lanes: bit 5 indicates how many Dregs to write. - * VLD2/3/4 to all lanes: bit 5 indicates register stride. - */ - stride = (insn & (1 << 5)) ? 2 : 1; - vec_size = nregs == 1 ? stride * 8 : 8; - - tmp = tcg_temp_new_i32(); - for (reg = 0; reg < nregs; reg++) { - gen_aa32_ld_i32(s, tmp, addr, get_mem_index(s), - s->be_data | size); - if ((rd & 1) && vec_size == 16) { - /* We cannot write 16 bytes at once because the - * destination is unaligned. - */ - tcg_gen_gvec_dup_i32(size, neon_reg_offset(rd, 0), - 8, 8, tmp); - tcg_gen_gvec_mov(0, neon_reg_offset(rd + 1, 0), - neon_reg_offset(rd, 0), 8, 8); - } else { - tcg_gen_gvec_dup_i32(size, neon_reg_offset(rd, 0), - vec_size, vec_size, tmp); - } - tcg_gen_addi_i32(addr, addr, 1 << size); - rd += stride; - } - tcg_temp_free_i32(tmp); - tcg_temp_free_i32(addr); - stride = (1 << size) * nregs; + /* Load single element to all lanes -- handled by decodetree */ + return 1; } else { /* Single element. */ int idx = (insn >> 4) & 0xf; -- 2.20.1 Convert the Neon "load/store single structure to one lane" insns to decodetree. As this is the last set of insns in the neon load/store group, we can remove the whole disas_neon_ls_insn() function. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-14-peter.maydell@linaro.org --- target/arm/neon-ls.decode | 11 +++ target/arm/translate-neon.inc.c | 89 +++++++++++++++++++ target/arm/translate.c | 147 -------------------------------- 3 files changed, 100 insertions(+), 147 deletions(-) diff --git a/target/arm/neon-ls.decode b/target/arm/neon-ls.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/neon-ls.decode +++ b/target/arm/neon-ls.decode @@ -XXX,XX +XXX,XX @@ VLDST_multiple 1111 0100 0 . l:1 0 rn:4 .... itype:4 size:2 align:2 rm:4 \ VLD_all_lanes 1111 0100 1 . 1 0 rn:4 .... 11 n:2 size:2 t:1 a:1 rm:4 \ vd=%vd_dp + +# Neon load/store single structure to one lane +%imm1_5_p1 5:1 !function=plus1 +%imm1_6_p1 6:1 !function=plus1 + +VLDST_single 1111 0100 1 . l:1 0 rn:4 .... 00 n:2 reg_idx:3 align:1 rm:4 \ + vd=%vd_dp size=0 stride=1 +VLDST_single 1111 0100 1 . l:1 0 rn:4 .... 01 n:2 reg_idx:2 align:2 rm:4 \ + vd=%vd_dp size=1 stride=%imm1_5_p1 +VLDST_single 1111 0100 1 . l:1 0 rn:4 .... 10 n:2 reg_idx:1 align:3 rm:4 \ + vd=%vd_dp size=2 stride=%imm1_6_p1 diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.inc.c +++ b/target/arm/translate-neon.inc.c @@ -XXX,XX +XXX,XX @@ * It might be possible to convert it to a standalone .c file eventually. */ +static inline int plus1(DisasContext *s, int x) +{ + return x + 1; +} + /* Include the generated Neon decoder */ #include "decode-neon-dp.inc.c" #include "decode-neon-ls.inc.c" @@ -XXX,XX +XXX,XX @@ static bool trans_VLD_all_lanes(DisasContext *s, arg_VLD_all_lanes *a) return true; } + +static bool trans_VLDST_single(DisasContext *s, arg_VLDST_single *a) +{ + /* Neon load/store single structure to one lane */ + int reg; + int nregs = a->n + 1; + int vd = a->vd; + TCGv_i32 addr, tmp; + + if (!arm_dc_feature(s, ARM_FEATURE_NEON)) { + return false; + } + + /* UNDEF accesses to D16-D31 if they don't exist */ + if (!dc_isar_feature(aa32_simd_r32, s) && (a->vd & 0x10)) { + return false; + } + + /* Catch the UNDEF cases. This is unavoidably a bit messy. */ + switch (nregs) { + case 1: + if (((a->align & (1 << a->size)) != 0) || + (a->size == 2 && ((a->align & 3) == 1 || (a->align & 3) == 2))) { + return false; + } + break; + case 3: + if ((a->align & 1) != 0) { + return false; + } + /* fall through */ + case 2: + if (a->size == 2 && (a->align & 2) != 0) { + return false; + } + break; + case 4: + if ((a->size == 2) && ((a->align & 3) == 3)) { + return false; + } + break; + default: + abort(); + } + if ((vd + a->stride * (nregs - 1)) > 31) { + /* + * Attempts to write off the end of the register file are + * UNPREDICTABLE; we choose to UNDEF because otherwise we would + * access off the end of the array that holds the register data. + */ + return false; + } + + if (!vfp_access_check(s)) { + return true; + } + + tmp = tcg_temp_new_i32(); + addr = tcg_temp_new_i32(); + load_reg_var(s, addr, a->rn); + /* + * TODO: if we implemented alignment exceptions, we should check + * addr against the alignment encoded in a->align here. + */ + for (reg = 0; reg < nregs; reg++) { + if (a->l) { + gen_aa32_ld_i32(s, tmp, addr, get_mem_index(s), + s->be_data | a->size); + neon_store_element(vd, a->reg_idx, a->size, tmp); + } else { /* Store */ + neon_load_element(tmp, vd, a->reg_idx, a->size); + gen_aa32_st_i32(s, tmp, addr, get_mem_index(s), + s->be_data | a->size); + } + vd += a->stride; + tcg_gen_addi_i32(addr, addr, 1 << a->size); + } + tcg_temp_free_i32(addr); + tcg_temp_free_i32(tmp); + + gen_neon_ldst_base_update(s, a->rm, a->rn, (1 << a->size) * nregs); + + return true; +} 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 void gen_neon_trn_u16(TCGv_i32 t0, TCGv_i32 t1) tcg_temp_free_i32(rd); } - -/* Translate a NEON load/store element instruction. Return nonzero if the - instruction is invalid. */ -static int disas_neon_ls_insn(DisasContext *s, uint32_t insn) -{ - int rd, rn, rm; - int nregs; - int stride; - int size; - int reg; - int load; - TCGv_i32 addr; - TCGv_i32 tmp; - - if (!arm_dc_feature(s, ARM_FEATURE_NEON)) { - return 1; - } - - /* FIXME: this access check should not take precedence over UNDEF - * for invalid encodings; we will generate incorrect syndrome information - * for attempts to execute invalid vfp/neon encodings with FP disabled. - */ - if (s->fp_excp_el) { - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_simd_access_trap(1, 0xe, false), s->fp_excp_el); - return 0; - } - - if (!s->vfp_enabled) - return 1; - VFP_DREG_D(rd, insn); - rn = (insn >> 16) & 0xf; - rm = insn & 0xf; - load = (insn & (1 << 21)) != 0; - if ((insn & (1 << 23)) == 0) { - /* Load store all elements -- handled already by decodetree */ - return 1; - } else { - size = (insn >> 10) & 3; - if (size == 3) { - /* Load single element to all lanes -- handled by decodetree */ - return 1; - } else { - /* Single element. */ - int idx = (insn >> 4) & 0xf; - int reg_idx; - switch (size) { - case 0: - reg_idx = (insn >> 5) & 7; - stride = 1; - break; - case 1: - reg_idx = (insn >> 6) & 3; - stride = (insn & (1 << 5)) ? 2 : 1; - break; - case 2: - reg_idx = (insn >> 7) & 1; - stride = (insn & (1 << 6)) ? 2 : 1; - break; - default: - abort(); - } - nregs = ((insn >> 8) & 3) + 1; - /* Catch the UNDEF cases. This is unavoidably a bit messy. */ - switch (nregs) { - case 1: - if (((idx & (1 << size)) != 0) || - (size == 2 && ((idx & 3) == 1 || (idx & 3) == 2))) { - return 1; - } - break; - case 3: - if ((idx & 1) != 0) { - return 1; - } - /* fall through */ - case 2: - if (size == 2 && (idx & 2) != 0) { - return 1; - } - break; - case 4: - if ((size == 2) && ((idx & 3) == 3)) { - return 1; - } - break; - default: - abort(); - } - if ((rd + stride * (nregs - 1)) > 31) { - /* Attempts to write off the end of the register file - * are UNPREDICTABLE; we choose to UNDEF because otherwise - * the neon_load_reg() would write off the end of the array. - */ - return 1; - } - tmp = tcg_temp_new_i32(); - addr = tcg_temp_new_i32(); - load_reg_var(s, addr, rn); - for (reg = 0; reg < nregs; reg++) { - if (load) { - gen_aa32_ld_i32(s, tmp, addr, get_mem_index(s), - s->be_data | size); - neon_store_element(rd, reg_idx, size, tmp); - } else { /* Store */ - neon_load_element(tmp, rd, reg_idx, size); - gen_aa32_st_i32(s, tmp, addr, get_mem_index(s), - s->be_data | size); - } - rd += stride; - tcg_gen_addi_i32(addr, addr, 1 << size); - } - tcg_temp_free_i32(addr); - tcg_temp_free_i32(tmp); - stride = nregs * (1 << size); - } - } - if (rm != 15) { - TCGv_i32 base; - - base = load_reg(s, rn); - if (rm == 13) { - tcg_gen_addi_i32(base, base, stride); - } else { - TCGv_i32 index; - index = load_reg(s, rm); - tcg_gen_add_i32(base, base, index); - tcg_temp_free_i32(index); - } - store_reg(s, rn, base); - } - return 0; -} - static inline void gen_neon_narrow(int size, TCGv_i32 dest, TCGv_i64 src) { switch (size) { @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) } return; } - if ((insn & 0x0f100000) == 0x04000000) { - /* NEON load/store. */ - if (disas_neon_ls_insn(s, insn)) { - goto illegal_op; - } - return; - } if ((insn & 0x0e000f00) == 0x0c000100) { if (arm_dc_feature(s, ARM_FEATURE_IWMMXT)) { /* iWMMXt register transfer. */ @@ -XXX,XX +XXX,XX @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) } break; case 12: - if ((insn & 0x01100000) == 0x01000000) { - if (disas_neon_ls_insn(s, insn)) { - goto illegal_op; - } - break; - } goto illegal_op; default: illegal_op: -- 2.20.1 Convert the Neon 3-reg-same VADD and VSUB insns to decodetree. Note that we don't need the neon_3r_sizes[op] check here because all size values are OK for VADD and VSUB; we'll add this when we convert the first insn that has size restrictions. For this we need one of the GVecGen*Fn typedefs currently in translate-a64.h; move them all to translate.h as a block so they are visible to the 32-bit decoder. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-15-peter.maydell@linaro.org --- target/arm/translate-a64.h | 9 -------- target/arm/translate.h | 9 ++++++++ target/arm/neon-dp.decode | 17 +++++++++++++++ target/arm/translate-neon.inc.c | 38 +++++++++++++++++++++++++++++++++ target/arm/translate.c | 14 ++++-------- 5 files changed, 68 insertions(+), 19 deletions(-) diff --git a/target/arm/translate-a64.h b/target/arm/translate-a64.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.h +++ b/target/arm/translate-a64.h @@ -XXX,XX +XXX,XX @@ static inline int vec_full_reg_size(DisasContext *s) bool disas_sve(DisasContext *, uint32_t); -/* Note that the gvec expanders operate on offsets + sizes. */ -typedef void GVecGen2Fn(unsigned, uint32_t, uint32_t, uint32_t, uint32_t); -typedef void GVecGen2iFn(unsigned, uint32_t, uint32_t, int64_t, - uint32_t, uint32_t); -typedef void GVecGen3Fn(unsigned, uint32_t, uint32_t, - uint32_t, uint32_t, uint32_t); -typedef void GVecGen4Fn(unsigned, uint32_t, uint32_t, uint32_t, - uint32_t, uint32_t, uint32_t); - #endif /* TARGET_ARM_TRANSLATE_A64_H */ diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ void gen_sshl_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b); #define dc_isar_feature(name, ctx) \ ({ DisasContext *ctx_ = (ctx); isar_feature_##name(ctx_->isar); }) +/* Note that the gvec expanders operate on offsets + sizes. */ +typedef void GVecGen2Fn(unsigned, uint32_t, uint32_t, uint32_t, uint32_t); +typedef void GVecGen2iFn(unsigned, uint32_t, uint32_t, int64_t, + uint32_t, uint32_t); +typedef void GVecGen3Fn(unsigned, uint32_t, uint32_t, + uint32_t, uint32_t, uint32_t); +typedef void GVecGen4Fn(unsigned, uint32_t, uint32_t, uint32_t, + uint32_t, uint32_t, uint32_t); + #endif /* TARGET_ARM_TRANSLATE_H */ diff --git a/target/arm/neon-dp.decode b/target/arm/neon-dp.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/neon-dp.decode +++ b/target/arm/neon-dp.decode @@ -XXX,XX +XXX,XX @@ # # This file is processed by scripts/decodetree.py # +# VFP/Neon register fields; same as vfp.decode +%vm_dp 5:1 0:4 +%vn_dp 7:1 16:4 +%vd_dp 22:1 12:4 # Encodings for Neon data processing instructions where the T32 encoding # is a simple transformation of the A32 encoding. @@ -XXX,XX +XXX,XX @@ # 0b111p_1111_qqqq_qqqq_qqqq_qqqq_qqqq_qqqq # This file works on the A32 encoding only; calling code for T32 has to # transform the insn into the A32 version first. + +###################################################################### +# 3-reg-same grouping: +# 1111 001 U 0 D sz:2 Vn:4 Vd:4 opc:4 N Q M op Vm:4 +###################################################################### + +&3same vm vn vd q size + +@3same .... ... . . . size:2 .... .... .... . q:1 . . .... \ + &3same vm=%vm_dp vn=%vn_dp vd=%vd_dp + +VADD_3s 1111 001 0 0 . .. .... .... 1000 . . . 0 .... @3same +VSUB_3s 1111 001 1 0 . .. .... .... 1000 . . . 0 .... @3same diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.inc.c +++ b/target/arm/translate-neon.inc.c @@ -XXX,XX +XXX,XX @@ static bool trans_VLDST_single(DisasContext *s, arg_VLDST_single *a) return true; } + +static bool do_3same(DisasContext *s, arg_3same *a, GVecGen3Fn fn) +{ + int vec_size = a->q ? 16 : 8; + int rd_ofs = neon_reg_offset(a->vd, 0); + int rn_ofs = neon_reg_offset(a->vn, 0); + int rm_ofs = neon_reg_offset(a->vm, 0); + + if (!arm_dc_feature(s, ARM_FEATURE_NEON)) { + return false; + } + + /* UNDEF accesses to D16-D31 if they don't exist. */ + if (!dc_isar_feature(aa32_simd_r32, s) && + ((a->vd | a->vn | a->vm) & 0x10)) { + return false; + } + + if ((a->vn | a->vm | a->vd) & a->q) { + return false; + } + + if (!vfp_access_check(s)) { + return true; + } + + fn(a->size, rd_ofs, rn_ofs, rm_ofs, vec_size, vec_size); + return true; +} + +#define DO_3SAME(INSN, FUNC) \ + static bool trans_##INSN##_3s(DisasContext *s, arg_3same *a) \ + { \ + return do_3same(s, a, FUNC); \ + } + +DO_3SAME(VADD, tcg_gen_gvec_add) +DO_3SAME(VSUB, tcg_gen_gvec_sub) 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_neon_data_insn(DisasContext *s, uint32_t insn) } return 0; - case NEON_3R_VADD_VSUB: - if (u) { - tcg_gen_gvec_sub(size, rd_ofs, rn_ofs, rm_ofs, - vec_size, vec_size); - } else { - tcg_gen_gvec_add(size, rd_ofs, rn_ofs, rm_ofs, - vec_size, vec_size); - } - return 0; - case NEON_3R_VQADD: tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc), rn_ofs, rm_ofs, vec_size, vec_size, @@ -XXX,XX +XXX,XX @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn) tcg_gen_gvec_3(rd_ofs, rm_ofs, rn_ofs, vec_size, vec_size, u ? &ushl_op[size] : &sshl_op[size]); return 0; + + case NEON_3R_VADD_VSUB: + /* Already handled by decodetree */ + return 1; } if (size == 3) { -- 2.20.1 Convert the Neon logic ops in the 3-reg-same grouping to decodetree. Note that for the logic ops the 'size' field forms part of their decode and the actual operations are always bitwise. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-16-peter.maydell@linaro.org --- target/arm/neon-dp.decode | 12 +++++++++++ target/arm/translate-neon.inc.c | 19 +++++++++++++++++ target/arm/translate.c | 38 +-------------------------------- 3 files changed, 32 insertions(+), 37 deletions(-) diff --git a/target/arm/neon-dp.decode b/target/arm/neon-dp.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/neon-dp.decode +++ b/target/arm/neon-dp.decode @@ -XXX,XX +XXX,XX @@ @3same .... ... . . . size:2 .... .... .... . q:1 . . .... \ &3same vm=%vm_dp vn=%vn_dp vd=%vd_dp +@3same_logic .... ... . . . .. .... .... .... . q:1 .. .... \ + &3same vm=%vm_dp vn=%vn_dp vd=%vd_dp size=0 + +VAND_3s 1111 001 0 0 . 00 .... .... 0001 ... 1 .... @3same_logic +VBIC_3s 1111 001 0 0 . 01 .... .... 0001 ... 1 .... @3same_logic +VORR_3s 1111 001 0 0 . 10 .... .... 0001 ... 1 .... @3same_logic +VORN_3s 1111 001 0 0 . 11 .... .... 0001 ... 1 .... @3same_logic +VEOR_3s 1111 001 1 0 . 00 .... .... 0001 ... 1 .... @3same_logic +VBSL_3s 1111 001 1 0 . 01 .... .... 0001 ... 1 .... @3same_logic +VBIT_3s 1111 001 1 0 . 10 .... .... 0001 ... 1 .... @3same_logic +VBIF_3s 1111 001 1 0 . 11 .... .... 0001 ... 1 .... @3same_logic + VADD_3s 1111 001 0 0 . .. .... .... 1000 . . . 0 .... @3same VSUB_3s 1111 001 1 0 . .. .... .... 1000 . . . 0 .... @3same diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.inc.c +++ b/target/arm/translate-neon.inc.c @@ -XXX,XX +XXX,XX @@ static bool do_3same(DisasContext *s, arg_3same *a, GVecGen3Fn fn) DO_3SAME(VADD, tcg_gen_gvec_add) DO_3SAME(VSUB, tcg_gen_gvec_sub) +DO_3SAME(VAND, tcg_gen_gvec_and) +DO_3SAME(VBIC, tcg_gen_gvec_andc) +DO_3SAME(VORR, tcg_gen_gvec_or) +DO_3SAME(VORN, tcg_gen_gvec_orc) +DO_3SAME(VEOR, tcg_gen_gvec_xor) + +/* These insns are all gvec_bitsel but with the inputs in various orders. */ +#define DO_3SAME_BITSEL(INSN, O1, O2, O3) \ + static void gen_##INSN##_3s(unsigned vece, uint32_t rd_ofs, \ + uint32_t rn_ofs, uint32_t rm_ofs, \ + uint32_t oprsz, uint32_t maxsz) \ + { \ + tcg_gen_gvec_bitsel(vece, rd_ofs, O1, O2, O3, oprsz, maxsz); \ + } \ + DO_3SAME(INSN, gen_##INSN##_3s) + +DO_3SAME_BITSEL(VBSL, rd_ofs, rn_ofs, rm_ofs) +DO_3SAME_BITSEL(VBIT, rm_ofs, rn_ofs, rd_ofs) +DO_3SAME_BITSEL(VBIF, rm_ofs, rd_ofs, rn_ofs) 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_neon_data_insn(DisasContext *s, uint32_t insn) } return 1; - case NEON_3R_LOGIC: /* Logic ops. */ - switch ((u << 2) | size) { - case 0: /* VAND */ - tcg_gen_gvec_and(0, rd_ofs, rn_ofs, rm_ofs, - vec_size, vec_size); - break; - case 1: /* VBIC */ - tcg_gen_gvec_andc(0, rd_ofs, rn_ofs, rm_ofs, - vec_size, vec_size); - break; - case 2: /* VORR */ - tcg_gen_gvec_or(0, rd_ofs, rn_ofs, rm_ofs, - vec_size, vec_size); - break; - case 3: /* VORN */ - tcg_gen_gvec_orc(0, rd_ofs, rn_ofs, rm_ofs, - vec_size, vec_size); - break; - case 4: /* VEOR */ - tcg_gen_gvec_xor(0, rd_ofs, rn_ofs, rm_ofs, - vec_size, vec_size); - break; - case 5: /* VBSL */ - tcg_gen_gvec_bitsel(MO_8, rd_ofs, rd_ofs, rn_ofs, rm_ofs, - vec_size, vec_size); - break; - case 6: /* VBIT */ - tcg_gen_gvec_bitsel(MO_8, rd_ofs, rm_ofs, rn_ofs, rd_ofs, - vec_size, vec_size); - break; - case 7: /* VBIF */ - tcg_gen_gvec_bitsel(MO_8, rd_ofs, rm_ofs, rd_ofs, rn_ofs, - vec_size, vec_size); - break; - } - return 0; - case NEON_3R_VQADD: tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc), rn_ofs, rm_ofs, vec_size, vec_size, @@ -XXX,XX +XXX,XX @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn) return 0; case NEON_3R_VADD_VSUB: + case NEON_3R_LOGIC: /* Already handled by decodetree */ return 1; } -- 2.20.1 Convert the Neon 3-reg-same VMAX and VMIN insns to decodetree. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-17-peter.maydell@linaro.org --- target/arm/neon-dp.decode | 5 +++++ target/arm/translate-neon.inc.c | 14 ++++++++++++++ target/arm/translate.c | 21 ++------------------- 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/target/arm/neon-dp.decode b/target/arm/neon-dp.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/neon-dp.decode +++ b/target/arm/neon-dp.decode @@ -XXX,XX +XXX,XX @@ VBSL_3s 1111 001 1 0 . 01 .... .... 0001 ... 1 .... @3same_logic VBIT_3s 1111 001 1 0 . 10 .... .... 0001 ... 1 .... @3same_logic VBIF_3s 1111 001 1 0 . 11 .... .... 0001 ... 1 .... @3same_logic +VMAX_S_3s 1111 001 0 0 . .. .... .... 0110 . . . 0 .... @3same +VMAX_U_3s 1111 001 1 0 . .. .... .... 0110 . . . 0 .... @3same +VMIN_S_3s 1111 001 0 0 . .. .... .... 0110 . . . 1 .... @3same +VMIN_U_3s 1111 001 1 0 . .. .... .... 0110 . . . 1 .... @3same + VADD_3s 1111 001 0 0 . .. .... .... 1000 . . . 0 .... @3same VSUB_3s 1111 001 1 0 . .. .... .... 1000 . . . 0 .... @3same diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.inc.c +++ b/target/arm/translate-neon.inc.c @@ -XXX,XX +XXX,XX @@ DO_3SAME(VEOR, tcg_gen_gvec_xor) DO_3SAME_BITSEL(VBSL, rd_ofs, rn_ofs, rm_ofs) DO_3SAME_BITSEL(VBIT, rm_ofs, rn_ofs, rd_ofs) DO_3SAME_BITSEL(VBIF, rm_ofs, rd_ofs, rn_ofs) + +#define DO_3SAME_NO_SZ_3(INSN, FUNC) \ + static bool trans_##INSN##_3s(DisasContext *s, arg_3same *a) \ + { \ + if (a->size == 3) { \ + return false; \ + } \ + return do_3same(s, a, FUNC); \ + } + +DO_3SAME_NO_SZ_3(VMAX_S, tcg_gen_gvec_smax) +DO_3SAME_NO_SZ_3(VMAX_U, tcg_gen_gvec_umax) +DO_3SAME_NO_SZ_3(VMIN_S, tcg_gen_gvec_smin) +DO_3SAME_NO_SZ_3(VMIN_U, tcg_gen_gvec_umin) 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_neon_data_insn(DisasContext *s, uint32_t insn) rd_ofs, rn_ofs, rm_ofs, vec_size, vec_size); return 0; - case NEON_3R_VMAX: - if (u) { - tcg_gen_gvec_umax(size, rd_ofs, rn_ofs, rm_ofs, - vec_size, vec_size); - } else { - tcg_gen_gvec_smax(size, rd_ofs, rn_ofs, rm_ofs, - vec_size, vec_size); - } - return 0; - case NEON_3R_VMIN: - if (u) { - tcg_gen_gvec_umin(size, rd_ofs, rn_ofs, rm_ofs, - vec_size, vec_size); - } else { - tcg_gen_gvec_smin(size, rd_ofs, rn_ofs, rm_ofs, - vec_size, vec_size); - } - return 0; - case NEON_3R_VSHL: /* Note the operation is vshl vd,vm,vn */ tcg_gen_gvec_3(rd_ofs, rm_ofs, rn_ofs, vec_size, vec_size, @@ -XXX,XX +XXX,XX @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn) case NEON_3R_VADD_VSUB: case NEON_3R_LOGIC: + case NEON_3R_VMAX: + case NEON_3R_VMIN: /* Already handled by decodetree */ return 1; } -- 2.20.1 Convert the Neon comparison ops in the 3-reg-same grouping to decodetree. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-18-peter.maydell@linaro.org --- target/arm/neon-dp.decode | 8 ++++++++ target/arm/translate-neon.inc.c | 22 ++++++++++++++++++++++ target/arm/translate.c | 23 +++-------------------- 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/target/arm/neon-dp.decode b/target/arm/neon-dp.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/neon-dp.decode +++ b/target/arm/neon-dp.decode @@ -XXX,XX +XXX,XX @@ VBSL_3s 1111 001 1 0 . 01 .... .... 0001 ... 1 .... @3same_logic VBIT_3s 1111 001 1 0 . 10 .... .... 0001 ... 1 .... @3same_logic VBIF_3s 1111 001 1 0 . 11 .... .... 0001 ... 1 .... @3same_logic +VCGT_S_3s 1111 001 0 0 . .. .... .... 0011 . . . 0 .... @3same +VCGT_U_3s 1111 001 1 0 . .. .... .... 0011 . . . 0 .... @3same +VCGE_S_3s 1111 001 0 0 . .. .... .... 0011 . . . 1 .... @3same +VCGE_U_3s 1111 001 1 0 . .. .... .... 0011 . . . 1 .... @3same + VMAX_S_3s 1111 001 0 0 . .. .... .... 0110 . . . 0 .... @3same VMAX_U_3s 1111 001 1 0 . .. .... .... 0110 . . . 0 .... @3same VMIN_S_3s 1111 001 0 0 . .. .... .... 0110 . . . 1 .... @3same @@ -XXX,XX +XXX,XX @@ VMIN_U_3s 1111 001 1 0 . .. .... .... 0110 . . . 1 .... @3same VADD_3s 1111 001 0 0 . .. .... .... 1000 . . . 0 .... @3same VSUB_3s 1111 001 1 0 . .. .... .... 1000 . . . 0 .... @3same + +VTST_3s 1111 001 0 0 . .. .... .... 1000 . . . 1 .... @3same +VCEQ_3s 1111 001 1 0 . .. .... .... 1000 . . . 1 .... @3same diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.inc.c +++ b/target/arm/translate-neon.inc.c @@ -XXX,XX +XXX,XX @@ DO_3SAME_NO_SZ_3(VMAX_S, tcg_gen_gvec_smax) DO_3SAME_NO_SZ_3(VMAX_U, tcg_gen_gvec_umax) DO_3SAME_NO_SZ_3(VMIN_S, tcg_gen_gvec_smin) DO_3SAME_NO_SZ_3(VMIN_U, tcg_gen_gvec_umin) + +#define DO_3SAME_CMP(INSN, COND) \ + static void gen_##INSN##_3s(unsigned vece, uint32_t rd_ofs, \ + uint32_t rn_ofs, uint32_t rm_ofs, \ + uint32_t oprsz, uint32_t maxsz) \ + { \ + tcg_gen_gvec_cmp(COND, vece, rd_ofs, rn_ofs, rm_ofs, oprsz, maxsz); \ + } \ + DO_3SAME_NO_SZ_3(INSN, gen_##INSN##_3s) + +DO_3SAME_CMP(VCGT_S, TCG_COND_GT) +DO_3SAME_CMP(VCGT_U, TCG_COND_GTU) +DO_3SAME_CMP(VCGE_S, TCG_COND_GE) +DO_3SAME_CMP(VCGE_U, TCG_COND_GEU) +DO_3SAME_CMP(VCEQ, TCG_COND_EQ) + +static void gen_VTST_3s(unsigned vece, uint32_t rd_ofs, uint32_t rn_ofs, + uint32_t rm_ofs, uint32_t oprsz, uint32_t maxsz) +{ + tcg_gen_gvec_3(rd_ofs, rn_ofs, rm_ofs, oprsz, maxsz, &cmtst_op[vece]); +} +DO_3SAME_NO_SZ_3(VTST, gen_VTST_3s) 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_neon_data_insn(DisasContext *s, uint32_t insn) u ? &mls_op[size] : &mla_op[size]); return 0; - case NEON_3R_VTST_VCEQ: - if (u) { /* VCEQ */ - tcg_gen_gvec_cmp(TCG_COND_EQ, size, rd_ofs, rn_ofs, rm_ofs, - vec_size, vec_size); - } else { /* VTST */ - tcg_gen_gvec_3(rd_ofs, rn_ofs, rm_ofs, - vec_size, vec_size, &cmtst_op[size]); - } - return 0; - - case NEON_3R_VCGT: - tcg_gen_gvec_cmp(u ? TCG_COND_GTU : TCG_COND_GT, size, - rd_ofs, rn_ofs, rm_ofs, vec_size, vec_size); - return 0; - - case NEON_3R_VCGE: - tcg_gen_gvec_cmp(u ? TCG_COND_GEU : TCG_COND_GE, size, - rd_ofs, rn_ofs, rm_ofs, vec_size, vec_size); - return 0; - case NEON_3R_VSHL: /* Note the operation is vshl vd,vm,vn */ tcg_gen_gvec_3(rd_ofs, rm_ofs, rn_ofs, vec_size, vec_size, @@ -XXX,XX +XXX,XX @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn) case NEON_3R_LOGIC: case NEON_3R_VMAX: case NEON_3R_VMIN: + case NEON_3R_VTST_VCEQ: + case NEON_3R_VCGT: + case NEON_3R_VCGE: /* Already handled by decodetree */ return 1; } -- 2.20.1 Convert the Neon VQADD/VQSUB insns in the 3-reg-same grouping to decodetree. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-19-peter.maydell@linaro.org --- target/arm/neon-dp.decode | 6 ++++++ target/arm/translate-neon.inc.c | 15 +++++++++++++++ target/arm/translate.c | 14 ++------------ 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/target/arm/neon-dp.decode b/target/arm/neon-dp.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/neon-dp.decode +++ b/target/arm/neon-dp.decode @@ -XXX,XX +XXX,XX @@ @3same .... ... . . . size:2 .... .... .... . q:1 . . .... \ &3same vm=%vm_dp vn=%vn_dp vd=%vd_dp +VQADD_S_3s 1111 001 0 0 . .. .... .... 0000 . . . 1 .... @3same +VQADD_U_3s 1111 001 1 0 . .. .... .... 0000 . . . 1 .... @3same + @3same_logic .... ... . . . .. .... .... .... . q:1 .. .... \ &3same vm=%vm_dp vn=%vn_dp vd=%vd_dp size=0 @@ -XXX,XX +XXX,XX @@ VBSL_3s 1111 001 1 0 . 01 .... .... 0001 ... 1 .... @3same_logic VBIT_3s 1111 001 1 0 . 10 .... .... 0001 ... 1 .... @3same_logic VBIF_3s 1111 001 1 0 . 11 .... .... 0001 ... 1 .... @3same_logic +VQSUB_S_3s 1111 001 0 0 . .. .... .... 0010 . . . 1 .... @3same +VQSUB_U_3s 1111 001 1 0 . .. .... .... 0010 . . . 1 .... @3same + VCGT_S_3s 1111 001 0 0 . .. .... .... 0011 . . . 0 .... @3same VCGT_U_3s 1111 001 1 0 . .. .... .... 0011 . . . 0 .... @3same VCGE_S_3s 1111 001 0 0 . .. .... .... 0011 . . . 1 .... @3same diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.inc.c +++ b/target/arm/translate-neon.inc.c @@ -XXX,XX +XXX,XX @@ static void gen_VTST_3s(unsigned vece, uint32_t rd_ofs, uint32_t rn_ofs, tcg_gen_gvec_3(rd_ofs, rn_ofs, rm_ofs, oprsz, maxsz, &cmtst_op[vece]); } DO_3SAME_NO_SZ_3(VTST, gen_VTST_3s) + +#define DO_3SAME_GVEC4(INSN, OPARRAY) \ + static void gen_##INSN##_3s(unsigned vece, uint32_t rd_ofs, \ + uint32_t rn_ofs, uint32_t rm_ofs, \ + uint32_t oprsz, uint32_t maxsz) \ + { \ + tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc), \ + rn_ofs, rm_ofs, oprsz, maxsz, &OPARRAY[vece]); \ + } \ + DO_3SAME(INSN, gen_##INSN##_3s) + +DO_3SAME_GVEC4(VQADD_S, sqadd_op) +DO_3SAME_GVEC4(VQADD_U, uqadd_op) +DO_3SAME_GVEC4(VQSUB_S, sqsub_op) +DO_3SAME_GVEC4(VQSUB_U, uqsub_op) 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_neon_data_insn(DisasContext *s, uint32_t insn) } return 1; - case NEON_3R_VQADD: - tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc), - rn_ofs, rm_ofs, vec_size, vec_size, - (u ? uqadd_op : sqadd_op) + size); - return 0; - - case NEON_3R_VQSUB: - tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc), - rn_ofs, rm_ofs, vec_size, vec_size, - (u ? uqsub_op : sqsub_op) + size); - return 0; - case NEON_3R_VMUL: /* VMUL */ if (u) { /* Polynomial case allows only P8. */ @@ -XXX,XX +XXX,XX @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn) case NEON_3R_VTST_VCEQ: case NEON_3R_VCGT: case NEON_3R_VCGE: + case NEON_3R_VQADD: + case NEON_3R_VQSUB: /* Already handled by decodetree */ return 1; } -- 2.20.1 Convert the Neon VMUL, VMLA, VMLS and VSHL insns in the 3-reg-same grouping to decodetree. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-20-peter.maydell@linaro.org --- target/arm/neon-dp.decode | 9 +++++++ target/arm/translate-neon.inc.c | 44 +++++++++++++++++++++++++++++++++ target/arm/translate.c | 28 +++------------------ 3 files changed, 56 insertions(+), 25 deletions(-) diff --git a/target/arm/neon-dp.decode b/target/arm/neon-dp.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/neon-dp.decode +++ b/target/arm/neon-dp.decode @@ -XXX,XX +XXX,XX @@ VCGT_U_3s 1111 001 1 0 . .. .... .... 0011 . . . 0 .... @3same VCGE_S_3s 1111 001 0 0 . .. .... .... 0011 . . . 1 .... @3same VCGE_U_3s 1111 001 1 0 . .. .... .... 0011 . . . 1 .... @3same +VSHL_S_3s 1111 001 0 0 . .. .... .... 0100 . . . 0 .... @3same +VSHL_U_3s 1111 001 1 0 . .. .... .... 0100 . . . 0 .... @3same + VMAX_S_3s 1111 001 0 0 . .. .... .... 0110 . . . 0 .... @3same VMAX_U_3s 1111 001 1 0 . .. .... .... 0110 . . . 0 .... @3same VMIN_S_3s 1111 001 0 0 . .. .... .... 0110 . . . 1 .... @3same @@ -XXX,XX +XXX,XX @@ VSUB_3s 1111 001 1 0 . .. .... .... 1000 . . . 0 .... @3same VTST_3s 1111 001 0 0 . .. .... .... 1000 . . . 1 .... @3same VCEQ_3s 1111 001 1 0 . .. .... .... 1000 . . . 1 .... @3same + +VMLA_3s 1111 001 0 0 . .. .... .... 1001 . . . 0 .... @3same +VMLS_3s 1111 001 1 0 . .. .... .... 1001 . . . 0 .... @3same + +VMUL_3s 1111 001 0 0 . .. .... .... 1001 . . . 1 .... @3same +VMUL_p_3s 1111 001 1 0 . .. .... .... 1001 . . . 1 .... @3same diff --git a/target/arm/translate-neon.inc.c b/target/arm/translate-neon.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-neon.inc.c +++ b/target/arm/translate-neon.inc.c @@ -XXX,XX +XXX,XX @@ DO_3SAME_NO_SZ_3(VMAX_S, tcg_gen_gvec_smax) DO_3SAME_NO_SZ_3(VMAX_U, tcg_gen_gvec_umax) DO_3SAME_NO_SZ_3(VMIN_S, tcg_gen_gvec_smin) DO_3SAME_NO_SZ_3(VMIN_U, tcg_gen_gvec_umin) +DO_3SAME_NO_SZ_3(VMUL, tcg_gen_gvec_mul) #define DO_3SAME_CMP(INSN, COND) \ static void gen_##INSN##_3s(unsigned vece, uint32_t rd_ofs, \ @@ -XXX,XX +XXX,XX @@ DO_3SAME_GVEC4(VQADD_S, sqadd_op) DO_3SAME_GVEC4(VQADD_U, uqadd_op) DO_3SAME_GVEC4(VQSUB_S, sqsub_op) DO_3SAME_GVEC4(VQSUB_U, uqsub_op) + +static void gen_VMUL_p_3s(unsigned vece, uint32_t rd_ofs, uint32_t rn_ofs, + uint32_t rm_ofs, uint32_t oprsz, uint32_t maxsz) +{ + tcg_gen_gvec_3_ool(rd_ofs, rn_ofs, rm_ofs, oprsz, maxsz, + 0, gen_helper_gvec_pmul_b); +} + +static bool trans_VMUL_p_3s(DisasContext *s, arg_3same *a) +{ + if (a->size != 0) { + return false; + } + return do_3same(s, a, gen_VMUL_p_3s); +} + +#define DO_3SAME_GVEC3_NO_SZ_3(INSN, OPARRAY) \ + static void gen_##INSN##_3s(unsigned vece, uint32_t rd_ofs, \ + uint32_t rn_ofs, uint32_t rm_ofs, \ + uint32_t oprsz, uint32_t maxsz) \ + { \ + tcg_gen_gvec_3(rd_ofs, rn_ofs, rm_ofs, \ + oprsz, maxsz, &OPARRAY[vece]); \ + } \ + DO_3SAME_NO_SZ_3(INSN, gen_##INSN##_3s) + + +DO_3SAME_GVEC3_NO_SZ_3(VMLA, mla_op) +DO_3SAME_GVEC3_NO_SZ_3(VMLS, mls_op) + +#define DO_3SAME_GVEC3_SHIFT(INSN, OPARRAY) \ + static void gen_##INSN##_3s(unsigned vece, uint32_t rd_ofs, \ + uint32_t rn_ofs, uint32_t rm_ofs, \ + uint32_t oprsz, uint32_t maxsz) \ + { \ + /* Note the operation is vshl vd,vm,vn */ \ + tcg_gen_gvec_3(rd_ofs, rm_ofs, rn_ofs, \ + oprsz, maxsz, &OPARRAY[vece]); \ + } \ + DO_3SAME(INSN, gen_##INSN##_3s) + +DO_3SAME_GVEC3_SHIFT(VSHL_S, sshl_op) +DO_3SAME_GVEC3_SHIFT(VSHL_U, ushl_op) 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_neon_data_insn(DisasContext *s, uint32_t insn) } return 1; - case NEON_3R_VMUL: /* VMUL */ - if (u) { - /* Polynomial case allows only P8. */ - if (size != 0) { - return 1; - } - tcg_gen_gvec_3_ool(rd_ofs, rn_ofs, rm_ofs, vec_size, vec_size, - 0, gen_helper_gvec_pmul_b); - } else { - tcg_gen_gvec_mul(size, rd_ofs, rn_ofs, rm_ofs, - vec_size, vec_size); - } - return 0; - - case NEON_3R_VML: /* VMLA, VMLS */ - tcg_gen_gvec_3(rd_ofs, rn_ofs, rm_ofs, vec_size, vec_size, - u ? &mls_op[size] : &mla_op[size]); - return 0; - - case NEON_3R_VSHL: - /* Note the operation is vshl vd,vm,vn */ - tcg_gen_gvec_3(rd_ofs, rm_ofs, rn_ofs, vec_size, vec_size, - u ? &ushl_op[size] : &sshl_op[size]); - return 0; - case NEON_3R_VADD_VSUB: case NEON_3R_LOGIC: case NEON_3R_VMAX: @@ -XXX,XX +XXX,XX @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn) case NEON_3R_VCGE: case NEON_3R_VQADD: case NEON_3R_VQSUB: + case NEON_3R_VMUL: + case NEON_3R_VML: + case NEON_3R_VSHL: /* Already handled by decodetree */ return 1; } -- 2.20.1 We're going to want at least some of the NeonGen* typedefs for the refactored 32-bit Neon decoder, so move them all to translate.h since it makes more sense to keep them in one group. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20200430181003.21682-23-peter.maydell@linaro.org --- target/arm/translate.h | 17 +++++++++++++++++ target/arm/translate-a64.c | 17 ----------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ typedef void GVecGen3Fn(unsigned, uint32_t, uint32_t, typedef void GVecGen4Fn(unsigned, uint32_t, uint32_t, uint32_t, uint32_t, uint32_t, uint32_t); +/* Function prototype for gen_ functions for calling Neon helpers */ +typedef void NeonGenOneOpEnvFn(TCGv_i32, TCGv_ptr, TCGv_i32); +typedef void NeonGenTwoOpFn(TCGv_i32, TCGv_i32, TCGv_i32); +typedef void NeonGenTwoOpEnvFn(TCGv_i32, TCGv_ptr, TCGv_i32, TCGv_i32); +typedef void NeonGenTwo64OpFn(TCGv_i64, TCGv_i64, TCGv_i64); +typedef void NeonGenTwo64OpEnvFn(TCGv_i64, TCGv_ptr, TCGv_i64, TCGv_i64); +typedef void NeonGenNarrowFn(TCGv_i32, TCGv_i64); +typedef void NeonGenNarrowEnvFn(TCGv_i32, TCGv_ptr, TCGv_i64); +typedef void NeonGenWidenFn(TCGv_i64, TCGv_i32); +typedef void NeonGenTwoSingleOPFn(TCGv_i32, TCGv_i32, TCGv_i32, TCGv_ptr); +typedef void NeonGenTwoDoubleOPFn(TCGv_i64, TCGv_i64, TCGv_i64, TCGv_ptr); +typedef void NeonGenOneOpFn(TCGv_i64, TCGv_i64); +typedef void CryptoTwoOpFn(TCGv_ptr, TCGv_ptr); +typedef void CryptoThreeOpIntFn(TCGv_ptr, TCGv_ptr, TCGv_i32); +typedef void CryptoThreeOpFn(TCGv_ptr, TCGv_ptr, TCGv_ptr); +typedef void AtomicThreeOpFn(TCGv_i64, TCGv_i64, TCGv_i64, TCGArg, MemOp); + #endif /* TARGET_ARM_TRANSLATE_H */ 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 @@ typedef struct AArch64DecodeTable { AArch64DecodeFn *disas_fn; } AArch64DecodeTable; -/* Function prototype for gen_ functions for calling Neon helpers */ -typedef void NeonGenOneOpEnvFn(TCGv_i32, TCGv_ptr, TCGv_i32); -typedef void NeonGenTwoOpFn(TCGv_i32, TCGv_i32, TCGv_i32); -typedef void NeonGenTwoOpEnvFn(TCGv_i32, TCGv_ptr, TCGv_i32, TCGv_i32); -typedef void NeonGenTwo64OpFn(TCGv_i64, TCGv_i64, TCGv_i64); -typedef void NeonGenTwo64OpEnvFn(TCGv_i64, TCGv_ptr, TCGv_i64, TCGv_i64); -typedef void NeonGenNarrowFn(TCGv_i32, TCGv_i64); -typedef void NeonGenNarrowEnvFn(TCGv_i32, TCGv_ptr, TCGv_i64); -typedef void NeonGenWidenFn(TCGv_i64, TCGv_i32); -typedef void NeonGenTwoSingleOPFn(TCGv_i32, TCGv_i32, TCGv_i32, TCGv_ptr); -typedef void NeonGenTwoDoubleOPFn(TCGv_i64, TCGv_i64, TCGv_i64, TCGv_ptr); -typedef void NeonGenOneOpFn(TCGv_i64, TCGv_i64); -typedef void CryptoTwoOpFn(TCGv_ptr, TCGv_ptr); -typedef void CryptoThreeOpIntFn(TCGv_ptr, TCGv_ptr, TCGv_i32); -typedef void CryptoThreeOpFn(TCGv_ptr, TCGv_ptr, TCGv_ptr); -typedef void AtomicThreeOpFn(TCGv_i64, TCGv_i64, TCGv_i64, TCGArg, MemOp); - /* initialize TCG globals. */ void a64_translate_init(void) { -- 2.20.1 Just flushing my target-arm queue since I won't be working next week :-) -- PMM The following changes since commit b3cd3b5a66f0dddfe3d5ba2bef13cd4f5b89cde9: Merge tag 'pull-riscv-to-apply-20220610' of github.com:alistair23/qemu into staging (2022-06-09 22:08:27 -0700) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20220610 for you to fetch changes up to 90c072e063737e9e8f431489bbd334452f89056e: semihosting/config: Merge --semihosting-config option groups (2022-06-10 14:32:36 +0100) ---------------------------------------------------------------- * refactor exception routing code * fix SCR_EL3 RAO/RAZ bits * gdbstub: Don't use GDB syscalls if no GDB is attached * semihosting/config: Merge --semihosting-config option groups * tests/qtest: Reduce npcm7xx_sdhci test image size ---------------------------------------------------------------- Hao Wu (1): tests/qtest: Reduce npcm7xx_sdhci test image size Peter Maydell (2): gdbstub: Don't use GDB syscalls if no GDB is attached semihosting/config: Merge --semihosting-config option groups Richard Henderson (25): target/arm: Mark exception helpers as noreturn target/arm: Add coproc parameter to syn_fp_access_trap target/arm: Move exception_target_el out of line target/arm: Move arm_singlestep_active out of line target/arm: Move arm_generate_debug_exceptions out of line target/arm: Use is_a64 in arm_generate_debug_exceptions target/arm: Move exception_bkpt_insn to debug_helper.c target/arm: Move arm_debug_exception_fsr to debug_helper.c target/arm: Rename helper_exception_with_syndrome target/arm: Introduce gen_exception_insn_el_v target/arm: Rename gen_exception_insn to gen_exception_insn_el target/arm: Introduce gen_exception_insn target/arm: Create helper_exception_swstep target/arm: Remove TBFLAG_ANY.DEBUG_TARGET_EL target/arm: Move gen_exception to translate.c target/arm: Rename gen_exception to gen_exception_el target/arm: Introduce gen_exception target/arm: Introduce gen_exception_el_v target/arm: Introduce helper_exception_with_syndrome target/arm: Remove default_exception_el target/arm: Create raise_exception_debug target/arm: Move arm_debug_target_el to debug_helper.c target/arm: Fix Secure PL1 tests in fp_exception_el target/arm: Adjust format test in scr_write target/arm: SCR_EL3.RW is RAO/WI without AArch32 EL[12] target/arm/cpu.h | 133 ++--------------------- target/arm/helper.h | 8 +- target/arm/internals.h | 43 +------- target/arm/syndrome.h | 7 +- target/arm/translate.h | 43 ++------ gdbstub.c | 14 ++- semihosting/config.c | 1 + target/arm/debug_helper.c | 220 +++++++++++++++++++++++++++++++++++++-- target/arm/helper.c | 53 ++++------ target/arm/op_helper.c | 52 +++++---- target/arm/translate-a64.c | 34 +++--- target/arm/translate-m-nocp.c | 15 ++- target/arm/translate-mve.c | 3 +- target/arm/translate-vfp.c | 18 +++- target/arm/translate.c | 106 ++++++++++--------- tests/qtest/npcm7xx_sdhci-test.c | 2 +- 16 files changed, 390 insertions(+), 362 deletions(-) From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-2-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_FLAGS_2(usad8, TCG_CALL_NO_RWG_SE, i32, i32, i32) DEF_HELPER_FLAGS_3(sel_flags, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32) -DEF_HELPER_2(exception_internal, void, env, i32) -DEF_HELPER_4(exception_with_syndrome, void, env, i32, i32, i32) -DEF_HELPER_2(exception_bkpt_insn, void, env, i32) +DEF_HELPER_2(exception_internal, noreturn, env, i32) +DEF_HELPER_4(exception_with_syndrome, noreturn, env, i32, i32, i32) +DEF_HELPER_2(exception_bkpt_insn, noreturn, env, i32) DEF_HELPER_2(exception_pc_alignment, noreturn, env, tl) DEF_HELPER_1(setend, void, env) DEF_HELPER_2(wfi, void, env, i32) -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> With ARMv8, this field is always RES0. With ARMv7, targeting EL2 and TA=0, it is always 0xA. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-3-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/syndrome.h | 7 ++++--- target/arm/translate-a64.c | 3 ++- target/arm/translate-vfp.c | 14 ++++++++++++-- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/target/arm/syndrome.h b/target/arm/syndrome.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/syndrome.h +++ b/target/arm/syndrome.h @@ -XXX,XX +XXX,XX @@ static inline uint32_t syn_cp15_rrt_trap(int cv, int cond, int opc1, int crm, | (rt2 << 10) | (rt << 5) | (crm << 1) | isread; } -static inline uint32_t syn_fp_access_trap(int cv, int cond, bool is_16bit) +static inline uint32_t syn_fp_access_trap(int cv, int cond, bool is_16bit, + int coproc) { - /* AArch32 FP trap or any AArch64 FP/SIMD trap: TA == 0 coproc == 0xa */ + /* AArch32 FP trap or any AArch64 FP/SIMD trap: TA == 0 */ return (EC_ADVSIMDFPACCESSTRAP << ARM_EL_EC_SHIFT) | (is_16bit ? 0 : ARM_EL_IL) - | (cv << 24) | (cond << 20) | 0xa; + | (cv << 24) | (cond << 20) | coproc; } static inline uint32_t syn_simd_access_trap(int cv, int cond, bool is_16bit) 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 bool fp_access_check(DisasContext *s) s->fp_access_checked = true; gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_fp_access_trap(1, 0xe, false), s->fp_excp_el); + syn_fp_access_trap(1, 0xe, false, 0), + s->fp_excp_el); return false; } s->fp_access_checked = true; diff --git a/target/arm/translate-vfp.c b/target/arm/translate-vfp.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-vfp.c +++ b/target/arm/translate-vfp.c @@ -XXX,XX +XXX,XX @@ static void gen_update_fp_context(DisasContext *s) static bool vfp_access_check_a(DisasContext *s, bool ignore_vfp_enabled) { if (s->fp_excp_el) { - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_fp_access_trap(1, 0xe, false), s->fp_excp_el); + /* + * The full syndrome is only used for HSR when HCPTR traps: + * For v8, when TA==0, coproc is RES0. + * For v7, any use of a Floating-point instruction or access + * to a Floating-point Extension register that is trapped to + * Hyp mode because of a trap configured in the HCPTR sets + * this field to 0xA. + */ + int coproc = arm_dc_feature(s, ARM_FEATURE_V8) ? 0 : 0xa; + uint32_t syn = syn_fp_access_trap(1, 0xe, false, coproc); + + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn, s->fp_excp_el); return false; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Move the function to op_helper.c, near raise_exception. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-4-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/internals.h | 16 +--------------- target/arm/op_helper.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 15 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 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, uint64_t va, int aa64_va_parameter_tbi(uint64_t tcr, ARMMMUIdx mmu_idx); int aa64_va_parameter_tbid(uint64_t tcr, ARMMMUIdx mmu_idx); -static inline int exception_target_el(CPUARMState *env) -{ - int target_el = MAX(1, arm_current_el(env)); - - /* - * No such thing as secure EL1 if EL3 is aarch32, - * so update the target EL to EL3 in this case. - */ - if (arm_is_secure(env) && !arm_el_is_aa64(env, 3) && target_el == 1) { - target_el = 3; - } - - return target_el; -} - /* Determine if allocation tags are available. */ static inline bool allocation_tag_access_enabled(CPUARMState *env, int el, uint64_t sctlr) @@ -XXX,XX +XXX,XX @@ void define_cortex_a72_a57_a53_cp_reginfo(ARMCPU *cpu); bool el_is_in_host(CPUARMState *env, int el); void aa32_max_features(ARMCPU *cpu); +int exception_target_el(CPUARMState *env); /* Powers of 2 for sve_vq_map et al. */ #define SVE_VQ_POW2_MAP \ 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 @@ #define SIGNBIT (uint32_t)0x80000000 #define SIGNBIT64 ((uint64_t)1 << 63) +int exception_target_el(CPUARMState *env) +{ + int target_el = MAX(1, arm_current_el(env)); + + /* + * No such thing as secure EL1 if EL3 is aarch32, + * so update the target EL to EL3 in this case. + */ + if (arm_is_secure(env) && !arm_el_is_aa64(env, 3) && target_el == 1) { + target_el = 3; + } + + return target_el; +} + void raise_exception(CPUARMState *env, uint32_t excp, uint32_t syndrome, uint32_t target_el) { -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Move the function to debug_helper.c, and the declaration to internals.h. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-5-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 10 ---------- target/arm/internals.h | 1 + target/arm/debug_helper.c | 12 ++++++++++++ 3 files changed, 13 insertions(+), 10 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_generate_debug_exceptions(CPUARMState *env) } } -/* Is single-stepping active? (Note that the "is EL_D AArch64?" check - * implicitly means this always returns false in pre-v8 CPUs.) - */ -static inline bool arm_singlestep_active(CPUARMState *env) -{ - return extract32(env->cp15.mdscr_el1, 0, 1) - && arm_el_is_aa64(env, arm_debug_target_el(env)) - && arm_generate_debug_exceptions(env); -} - static inline bool arm_sctlr_b(CPUARMState *env) { return 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 @@ bool el_is_in_host(CPUARMState *env, int el); void aa32_max_features(ARMCPU *cpu); int exception_target_el(CPUARMState *env); +bool arm_singlestep_active(CPUARMState *env); /* Powers of 2 for sve_vq_map et al. */ #define SVE_VQ_POW2_MAP \ diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -XXX,XX +XXX,XX @@ #include "exec/exec-all.h" #include "exec/helper-proto.h" + +/* + * Is single-stepping active? (Note that the "is EL_D AArch64?" check + * implicitly means this always returns false in pre-v8 CPUs.) + */ +bool arm_singlestep_active(CPUARMState *env) +{ + return extract32(env->cp15.mdscr_el1, 0, 1) + && arm_el_is_aa64(env, arm_debug_target_el(env)) + && arm_generate_debug_exceptions(env); +} + /* Return true if the linked breakpoint entry lbn passes its checks */ static bool linked_bp_matches(ARMCPU *cpu, int lbn) { -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Move arm_generate_debug_exceptions and its two subroutines, {aa32,aa64}_generate_debug_exceptions into debug_helper.c, and the one interface declaration to internals.h. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-6-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 91 ------------------------------------- target/arm/internals.h | 1 + target/arm/debug_helper.c | 94 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 91 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_v7m_csselr_razwi(ARMCPU *cpu) return (cpu->clidr & R_V7M_CLIDR_CTYPE_ALL_MASK) != 0; } -/* See AArch64.GenerateDebugExceptionsFrom() in ARM ARM pseudocode */ -static inline bool aa64_generate_debug_exceptions(CPUARMState *env) -{ - int cur_el = arm_current_el(env); - int debug_el; - - if (cur_el == 3) { - return false; - } - - /* MDCR_EL3.SDD disables debug events from Secure state */ - if (arm_is_secure_below_el3(env) - && extract32(env->cp15.mdcr_el3, 16, 1)) { - return false; - } - - /* - * Same EL to same EL debug exceptions need MDSCR_KDE enabled - * while not masking the (D)ebug bit in DAIF. - */ - debug_el = arm_debug_target_el(env); - - if (cur_el == debug_el) { - return extract32(env->cp15.mdscr_el1, 13, 1) - && !(env->daif & PSTATE_D); - } - - /* Otherwise the debug target needs to be a higher EL */ - return debug_el > cur_el; -} - -static inline bool aa32_generate_debug_exceptions(CPUARMState *env) -{ - int el = arm_current_el(env); - - if (el == 0 && arm_el_is_aa64(env, 1)) { - return aa64_generate_debug_exceptions(env); - } - - if (arm_is_secure(env)) { - int spd; - - if (el == 0 && (env->cp15.sder & 1)) { - /* SDER.SUIDEN means debug exceptions from Secure EL0 - * are always enabled. Otherwise they are controlled by - * SDCR.SPD like those from other Secure ELs. - */ - return true; - } - - spd = extract32(env->cp15.mdcr_el3, 14, 2); - switch (spd) { - case 1: - /* SPD == 0b01 is reserved, but behaves as 0b00. */ - case 0: - /* For 0b00 we return true if external secure invasive debug - * is enabled. On real hardware this is controlled by external - * signals to the core. QEMU always permits debug, and behaves - * as if DBGEN, SPIDEN, NIDEN and SPNIDEN are all tied high. - */ - return true; - case 2: - return false; - case 3: - return true; - } - } - - return el != 2; -} - -/* Return true if debugging exceptions are currently enabled. - * This corresponds to what in ARM ARM pseudocode would be - * if UsingAArch32() then - * return AArch32.GenerateDebugExceptions() - * else - * return AArch64.GenerateDebugExceptions() - * We choose to push the if() down into this function for clarity, - * since the pseudocode has it at all callsites except for the one in - * CheckSoftwareStep(), where it is elided because both branches would - * always return the same value. - */ -static inline bool arm_generate_debug_exceptions(CPUARMState *env) -{ - if (env->aarch64) { - return aa64_generate_debug_exceptions(env); - } else { - return aa32_generate_debug_exceptions(env); - } -} - static inline bool arm_sctlr_b(CPUARMState *env) { return 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 @@ bool el_is_in_host(CPUARMState *env, int el); void aa32_max_features(ARMCPU *cpu); int exception_target_el(CPUARMState *env); bool arm_singlestep_active(CPUARMState *env); +bool arm_generate_debug_exceptions(CPUARMState *env); /* Powers of 2 for sve_vq_map et al. */ #define SVE_VQ_POW2_MAP \ diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -XXX,XX +XXX,XX @@ #include "exec/helper-proto.h" +/* See AArch64.GenerateDebugExceptionsFrom() in ARM ARM pseudocode */ +static bool aa64_generate_debug_exceptions(CPUARMState *env) +{ + int cur_el = arm_current_el(env); + int debug_el; + + if (cur_el == 3) { + return false; + } + + /* MDCR_EL3.SDD disables debug events from Secure state */ + if (arm_is_secure_below_el3(env) + && extract32(env->cp15.mdcr_el3, 16, 1)) { + return false; + } + + /* + * Same EL to same EL debug exceptions need MDSCR_KDE enabled + * while not masking the (D)ebug bit in DAIF. + */ + debug_el = arm_debug_target_el(env); + + if (cur_el == debug_el) { + return extract32(env->cp15.mdscr_el1, 13, 1) + && !(env->daif & PSTATE_D); + } + + /* Otherwise the debug target needs to be a higher EL */ + return debug_el > cur_el; +} + +static bool aa32_generate_debug_exceptions(CPUARMState *env) +{ + int el = arm_current_el(env); + + if (el == 0 && arm_el_is_aa64(env, 1)) { + return aa64_generate_debug_exceptions(env); + } + + if (arm_is_secure(env)) { + int spd; + + if (el == 0 && (env->cp15.sder & 1)) { + /* + * SDER.SUIDEN means debug exceptions from Secure EL0 + * are always enabled. Otherwise they are controlled by + * SDCR.SPD like those from other Secure ELs. + */ + return true; + } + + spd = extract32(env->cp15.mdcr_el3, 14, 2); + switch (spd) { + case 1: + /* SPD == 0b01 is reserved, but behaves as 0b00. */ + case 0: + /* + * For 0b00 we return true if external secure invasive debug + * is enabled. On real hardware this is controlled by external + * signals to the core. QEMU always permits debug, and behaves + * as if DBGEN, SPIDEN, NIDEN and SPNIDEN are all tied high. + */ + return true; + case 2: + return false; + case 3: + return true; + } + } + + return el != 2; +} + +/* + * Return true if debugging exceptions are currently enabled. + * This corresponds to what in ARM ARM pseudocode would be + * if UsingAArch32() then + * return AArch32.GenerateDebugExceptions() + * else + * return AArch64.GenerateDebugExceptions() + * We choose to push the if() down into this function for clarity, + * since the pseudocode has it at all callsites except for the one in + * CheckSoftwareStep(), where it is elided because both branches would + * always return the same value. + */ +bool arm_generate_debug_exceptions(CPUARMState *env) +{ + if (env->aarch64) { + return aa64_generate_debug_exceptions(env); + } else { + return aa32_generate_debug_exceptions(env); + } +} + /* * Is single-stepping active? (Note that the "is EL_D AArch64?" check * implicitly means this always returns false in pre-v8 CPUs.) -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Use the accessor rather than the raw structure member. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-7-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/debug_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -XXX,XX +XXX,XX @@ static bool aa32_generate_debug_exceptions(CPUARMState *env) */ bool arm_generate_debug_exceptions(CPUARMState *env) { - if (env->aarch64) { + if (is_a64(env)) { return aa64_generate_debug_exceptions(env); } else { return aa32_generate_debug_exceptions(env); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-8-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/debug_helper.c | 31 +++++++++++++++++++++++++++++++ target/arm/op_helper.c | 29 ----------------------------- 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -XXX,XX +XXX,XX @@ void arm_debug_excp_handler(CPUState *cs) } } +/* + * Raise an EXCP_BKPT with the specified syndrome register value, + * targeting the correct exception level for debug exceptions. + */ +void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome) +{ + int debug_el = arm_debug_target_el(env); + int cur_el = arm_current_el(env); + + /* FSR will only be used if the debug target EL is AArch32. */ + env->exception.fsr = arm_debug_exception_fsr(env); + /* + * FAR is UNKNOWN: clear vaddress to avoid potentially exposing + * values to the guest that it shouldn't be able to see at its + * exception/security level. + */ + env->exception.vaddress = 0; + /* + * Other kinds of architectural debug exception are ignored if + * they target an exception level below the current one (in QEMU + * this is checked by arm_generate_debug_exceptions()). Breakpoint + * instructions are special because they always generate an exception + * to somewhere: if they can't go to the configured debug exception + * level they are taken to the current exception level. + */ + if (debug_el < cur_el) { + debug_el = cur_el; + } + raise_exception(env, EXCP_BKPT, syndrome, debug_el); +} + #if !defined(CONFIG_USER_ONLY) vaddr arm_adjust_watchpoint_address(CPUState *cs, vaddr addr, int len) diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -XXX,XX +XXX,XX @@ void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp, raise_exception(env, excp, syndrome, target_el); } -/* Raise an EXCP_BKPT with the specified syndrome register value, - * targeting the correct exception level for debug exceptions. - */ -void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome) -{ - int debug_el = arm_debug_target_el(env); - int cur_el = arm_current_el(env); - - /* FSR will only be used if the debug target EL is AArch32. */ - env->exception.fsr = arm_debug_exception_fsr(env); - /* FAR is UNKNOWN: clear vaddress to avoid potentially exposing - * values to the guest that it shouldn't be able to see at its - * exception/security level. - */ - env->exception.vaddress = 0; - /* - * Other kinds of architectural debug exception are ignored if - * they target an exception level below the current one (in QEMU - * this is checked by arm_generate_debug_exceptions()). Breakpoint - * instructions are special because they always generate an exception - * to somewhere: if they can't go to the configured debug exception - * level they are taken to the current exception level. - */ - if (debug_el < cur_el) { - debug_el = cur_el; - } - raise_exception(env, EXCP_BKPT, syndrome, debug_el); -} - uint32_t HELPER(cpsr_read)(CPUARMState *env) { return cpsr_read(env) & ~CPSR_EXEC; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> This function now now only used in debug_helper.c, so there is no reason to have a declaration in a header. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-9-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/internals.h | 25 ------------------------- target/arm/debug_helper.c | 26 ++++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 25 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 TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx) return &env->cp15.tcr_el[regime_el(env, mmu_idx)]; } -/* Return the FSR value for a debug exception (watchpoint, hardware - * breakpoint or BKPT insn) targeting the specified exception level. - */ -static inline uint32_t arm_debug_exception_fsr(CPUARMState *env) -{ - ARMMMUFaultInfo fi = { .type = ARMFault_Debug }; - int target_el = arm_debug_target_el(env); - bool using_lpae = false; - - if (target_el == 2 || arm_el_is_aa64(env, target_el)) { - using_lpae = true; - } else { - if (arm_feature(env, ARM_FEATURE_LPAE) && - (env->cp15.tcr_el[target_el].raw_tcr & TTBCR_EAE)) { - using_lpae = true; - } - } - - if (using_lpae) { - return arm_fi_to_lfsc(&fi); - } else { - return arm_fi_to_sfsc(&fi); - } -} - /** * arm_num_brps: Return number of implemented breakpoints. * Note that the ID register BRPS field is "number of bps - 1", diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -XXX,XX +XXX,XX @@ bool arm_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp) return check_watchpoints(cpu); } +/* + * Return the FSR value for a debug exception (watchpoint, hardware + * breakpoint or BKPT insn) targeting the specified exception level. + */ +static uint32_t arm_debug_exception_fsr(CPUARMState *env) +{ + ARMMMUFaultInfo fi = { .type = ARMFault_Debug }; + int target_el = arm_debug_target_el(env); + bool using_lpae = false; + + if (target_el == 2 || arm_el_is_aa64(env, target_el)) { + using_lpae = true; + } else { + if (arm_feature(env, ARM_FEATURE_LPAE) && + (env->cp15.tcr_el[target_el].raw_tcr & TTBCR_EAE)) { + using_lpae = true; + } + } + + if (using_lpae) { + return arm_fi_to_lfsc(&fi); + } else { + return arm_fi_to_sfsc(&fi); + } +} + void arm_debug_excp_handler(CPUState *cs) { /* -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Rename to helper_exception_with_syndrome_el, to emphasize that the target el is a parameter. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-10-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.h | 2 +- target/arm/translate.h | 6 +++--- target/arm/op_helper.c | 6 +++--- target/arm/translate.c | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_FLAGS_2(usad8, TCG_CALL_NO_RWG_SE, i32, i32, i32) DEF_HELPER_FLAGS_3(sel_flags, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32) DEF_HELPER_2(exception_internal, noreturn, env, i32) -DEF_HELPER_4(exception_with_syndrome, noreturn, env, i32, i32, i32) +DEF_HELPER_4(exception_with_syndrome_el, noreturn, env, i32, i32, i32) DEF_HELPER_2(exception_bkpt_insn, noreturn, env, i32) DEF_HELPER_2(exception_pc_alignment, noreturn, env, tl) DEF_HELPER_1(setend, void, env) diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ static inline void gen_ss_advance(DisasContext *s) static inline void gen_exception(int excp, uint32_t syndrome, uint32_t target_el) { - gen_helper_exception_with_syndrome(cpu_env, tcg_constant_i32(excp), - tcg_constant_i32(syndrome), - tcg_constant_i32(target_el)); + gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp), + tcg_constant_i32(syndrome), + tcg_constant_i32(target_el)); } /* Generate an architectural singlestep exception */ diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -XXX,XX +XXX,XX @@ void HELPER(yield)(CPUARMState *env) * those EXCP values which are special cases for QEMU to interrupt * execution and not to be used for exceptions which are passed to * the guest (those must all have syndrome information and thus should - * use exception_with_syndrome). + * use exception_with_syndrome*). */ void HELPER(exception_internal)(CPUARMState *env, uint32_t excp) { @@ -XXX,XX +XXX,XX @@ void HELPER(exception_internal)(CPUARMState *env, uint32_t excp) } /* Raise an exception with the specified syndrome register value */ -void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp, - uint32_t syndrome, uint32_t target_el) +void HELPER(exception_with_syndrome_el)(CPUARMState *env, uint32_t excp, + uint32_t syndrome, uint32_t target_el) { raise_exception(env, excp, syndrome, target_el); } 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 void gen_exception_el(DisasContext *s, int excp, uint32_t syn, { gen_set_condexec(s); gen_set_pc_im(s, s->pc_curr); - gen_helper_exception_with_syndrome(cpu_env, - tcg_constant_i32(excp), - tcg_constant_i32(syn), tcg_el); + gen_helper_exception_with_syndrome_el(cpu_env, + tcg_constant_i32(excp), + tcg_constant_i32(syn), tcg_el); s->base.is_jmp = DISAS_NORETURN; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Create a function below gen_exception_insn that takes the target_el as a TCGv_i32, replacing gen_exception_el. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-11-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 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 void gen_exception_internal_insn(DisasContext *s, uint32_t pc, int excp) s->base.is_jmp = DISAS_NORETURN; } -void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, - uint32_t syn, uint32_t target_el) +static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp, + uint32_t syn, TCGv_i32 tcg_el) { if (s->aarch64) { gen_a64_set_pc_im(pc); @@ -XXX,XX +XXX,XX @@ void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, gen_set_condexec(s); gen_set_pc_im(s, pc); } - gen_exception(excp, syn, target_el); + gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp), + tcg_constant_i32(syn), tcg_el); s->base.is_jmp = DISAS_NORETURN; } +void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, + uint32_t syn, uint32_t target_el) +{ + gen_exception_insn_el_v(s, pc, excp, syn, tcg_constant_i32(target_el)); +} + static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn) { gen_set_condexec(s); @@ -XXX,XX +XXX,XX @@ void unallocated_encoding(DisasContext *s) default_exception_el(s)); } -static void gen_exception_el(DisasContext *s, int excp, uint32_t syn, - TCGv_i32 tcg_el) -{ - gen_set_condexec(s); - gen_set_pc_im(s, s->pc_curr); - gen_helper_exception_with_syndrome_el(cpu_env, - tcg_constant_i32(excp), - tcg_constant_i32(syn), tcg_el); - s->base.is_jmp = DISAS_NORETURN; -} - /* Force a TB lookup after an instruction that changes the CPU state. */ void gen_lookup_tb(DisasContext *s) { @@ -XXX,XX +XXX,XX @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn, tcg_el = tcg_constant_i32(3); } - gen_exception_el(s, EXCP_UDEF, syn_uncategorized(), tcg_el); + gen_exception_insn_el_v(s, s->pc_curr, EXCP_UDEF, + syn_uncategorized(), tcg_el); tcg_temp_free_i32(tcg_el); return false; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-12-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.h | 4 ++-- target/arm/translate-a64.c | 36 ++++++++++++++++---------------- target/arm/translate-m-nocp.c | 16 +++++++------- target/arm/translate-mve.c | 4 ++-- target/arm/translate-vfp.c | 6 +++--- target/arm/translate.c | 39 ++++++++++++++++++----------------- 6 files changed, 53 insertions(+), 52 deletions(-) diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ void arm_jump_cc(DisasCompare *cmp, TCGLabel *label); void arm_gen_test_cc(int cc, TCGLabel *label); MemOp pow2_align(unsigned i); void unallocated_encoding(DisasContext *s); -void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, - uint32_t syn, uint32_t target_el); +void gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp, + uint32_t syn, uint32_t target_el); /* Return state of Alternate Half-precision flag, caller frees result */ static inline TCGv_i32 get_ahp_flag(void) 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 bool fp_access_check(DisasContext *s) assert(!s->fp_access_checked); s->fp_access_checked = true; - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_fp_access_trap(1, 0xe, false, 0), - s->fp_excp_el); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, + syn_fp_access_trap(1, 0xe, false, 0), + s->fp_excp_el); return false; } s->fp_access_checked = true; @@ -XXX,XX +XXX,XX @@ bool sve_access_check(DisasContext *s) assert(!s->sve_access_checked); s->sve_access_checked = true; - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_sve_access_trap(), s->sve_excp_el); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, + syn_sve_access_trap(), s->sve_excp_el); return false; } s->sve_access_checked = true; @@ -XXX,XX +XXX,XX @@ static void gen_sysreg_undef(DisasContext *s, bool isread, } else { syndrome = syn_uncategorized(); } - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syndrome, - default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, syndrome, + default_exception_el(s)); } /* MRS - move from system register @@ -XXX,XX +XXX,XX @@ static void disas_exc(DisasContext *s, uint32_t insn) switch (op2_ll) { case 1: /* SVC */ gen_ss_advance(s); - gen_exception_insn(s, s->base.pc_next, EXCP_SWI, - syn_aa64_svc(imm16), default_exception_el(s)); + gen_exception_insn_el(s, s->base.pc_next, EXCP_SWI, + syn_aa64_svc(imm16), default_exception_el(s)); break; case 2: /* HVC */ if (s->current_el == 0) { @@ -XXX,XX +XXX,XX @@ static void disas_exc(DisasContext *s, uint32_t insn) gen_a64_set_pc_im(s->pc_curr); gen_helper_pre_hvc(cpu_env); gen_ss_advance(s); - gen_exception_insn(s, s->base.pc_next, EXCP_HVC, - syn_aa64_hvc(imm16), 2); + gen_exception_insn_el(s, s->base.pc_next, EXCP_HVC, + syn_aa64_hvc(imm16), 2); break; case 3: /* SMC */ if (s->current_el == 0) { @@ -XXX,XX +XXX,XX @@ static void disas_exc(DisasContext *s, uint32_t insn) gen_a64_set_pc_im(s->pc_curr); gen_helper_pre_smc(cpu_env, tcg_constant_i32(syn_aa64_smc(imm16))); gen_ss_advance(s); - gen_exception_insn(s, s->base.pc_next, EXCP_SMC, - syn_aa64_smc(imm16), 3); + gen_exception_insn_el(s, s->base.pc_next, EXCP_SMC, + syn_aa64_smc(imm16), 3); break; default: unallocated_encoding(s); @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) * Illegal execution state. This has priority over BTI * exceptions, but comes after instruction abort exceptions. */ - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_illegalstate(), default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, + syn_illegalstate(), default_exception_el(s)); return; } @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) if (s->btype != 0 && s->guarded_page && !btype_destination_ok(insn, s->bt, s->btype)) { - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_btitrap(s->btype), - default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, + syn_btitrap(s->btype), + default_exception_el(s)); return; } } else { diff --git a/target/arm/translate-m-nocp.c b/target/arm/translate-m-nocp.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-m-nocp.c +++ b/target/arm/translate-m-nocp.c @@ -XXX,XX +XXX,XX @@ static bool trans_VSCCLRM(DisasContext *s, arg_VSCCLRM *a) tcg_gen_brcondi_i32(TCG_COND_EQ, sfpa, 0, s->condlabel); if (s->fp_excp_el != 0) { - gen_exception_insn(s, s->pc_curr, EXCP_NOCP, - syn_uncategorized(), s->fp_excp_el); + gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP, + syn_uncategorized(), s->fp_excp_el); return true; } @@ -XXX,XX +XXX,XX @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int regno, if (!vfp_access_check_m(s, true)) { /* * This was only a conditional exception, so override - * gen_exception_insn()'s default to DISAS_NORETURN + * gen_exception_insn_el()'s default to DISAS_NORETURN */ s->base.is_jmp = DISAS_NEXT; break; @@ -XXX,XX +XXX,XX @@ static bool gen_M_fp_sysreg_read(DisasContext *s, int regno, if (!vfp_access_check_m(s, true)) { /* * This was only a conditional exception, so override - * gen_exception_insn()'s default to DISAS_NORETURN + * gen_exception_insn_el()'s default to DISAS_NORETURN */ s->base.is_jmp = DISAS_NEXT; break; @@ -XXX,XX +XXX,XX @@ static bool trans_NOCP(DisasContext *s, arg_nocp *a) } if (a->cp != 10) { - gen_exception_insn(s, s->pc_curr, EXCP_NOCP, - syn_uncategorized(), default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP, + syn_uncategorized(), default_exception_el(s)); return true; } if (s->fp_excp_el != 0) { - gen_exception_insn(s, s->pc_curr, EXCP_NOCP, - syn_uncategorized(), s->fp_excp_el); + gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP, + syn_uncategorized(), s->fp_excp_el); return true; } diff --git a/target/arm/translate-mve.c b/target/arm/translate-mve.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-mve.c +++ b/target/arm/translate-mve.c @@ -XXX,XX +XXX,XX @@ bool mve_eci_check(DisasContext *s) return true; default: /* Reserved value: INVSTATE UsageFault */ - gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), - default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), + default_exception_el(s)); return false; } } diff --git a/target/arm/translate-vfp.c b/target/arm/translate-vfp.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-vfp.c +++ b/target/arm/translate-vfp.c @@ -XXX,XX +XXX,XX @@ static bool vfp_access_check_a(DisasContext *s, bool ignore_vfp_enabled) int coproc = arm_dc_feature(s, ARM_FEATURE_V8) ? 0 : 0xa; uint32_t syn = syn_fp_access_trap(1, 0xe, false, coproc); - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn, s->fp_excp_el); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, syn, s->fp_excp_el); return false; } @@ -XXX,XX +XXX,XX @@ bool vfp_access_check_m(DisasContext *s, bool skip_context_update) * the encoding space handled by the patterns in m-nocp.decode, * and for them we may need to raise NOCP here. */ - gen_exception_insn(s, s->pc_curr, EXCP_NOCP, - syn_uncategorized(), s->fp_excp_el); + gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP, + syn_uncategorized(), s->fp_excp_el); return false; } 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 void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp, s->base.is_jmp = DISAS_NORETURN; } -void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, - uint32_t syn, uint32_t target_el) +void gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp, + uint32_t syn, uint32_t target_el) { gen_exception_insn_el_v(s, pc, excp, syn, tcg_constant_i32(target_el)); } @@ -XXX,XX +XXX,XX @@ static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn) void unallocated_encoding(DisasContext *s) { /* Unallocated and reserved encodings are uncategorized */ - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), + default_exception_el(s)); } /* Force a TB lookup after an instruction that changes the CPU state. */ @@ -XXX,XX +XXX,XX @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn, undef: /* If we get here then some access check did not pass */ - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_uncategorized(), exc_target); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, + syn_uncategorized(), exc_target); return false; } @@ -XXX,XX +XXX,XX @@ static void gen_srs(DisasContext *s, * For the UNPREDICTABLE cases we choose to UNDEF. */ if (s->current_el == 1 && !s->ns && mode == ARM_CPU_MODE_MON) { - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), 3); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, + syn_uncategorized(), 3); return; } @@ -XXX,XX +XXX,XX @@ static bool trans_WLS(DisasContext *s, arg_WLS *a) * Do the check-and-raise-exception by hand. */ if (s->fp_excp_el) { - gen_exception_insn(s, s->pc_curr, EXCP_NOCP, - syn_uncategorized(), s->fp_excp_el); + gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP, + syn_uncategorized(), s->fp_excp_el); return true; } } @@ -XXX,XX +XXX,XX @@ static bool trans_LE(DisasContext *s, arg_LE *a) tmp = load_cpu_field(v7m.ltpsize); tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 4, skipexc); tcg_temp_free_i32(tmp); - gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), - default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), + default_exception_el(s)); gen_set_label(skipexc); } @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) * UsageFault exception. */ if (arm_dc_feature(s, ARM_FEATURE_M)) { - gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), - default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), + default_exception_el(s)); return; } @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) * Illegal execution state. This has priority over BTI * exceptions, but comes after instruction abort exceptions. */ - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, - syn_illegalstate(), default_exception_el(s)); + gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, + syn_illegalstate(), default_exception_el(s)); return; } @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) * Illegal execution state. This has priority over BTI * exceptions, but comes after instruction abort exceptions. */ - gen_exception_insn(dc, dc->pc_curr, EXCP_UDEF, - syn_illegalstate(), default_exception_el(dc)); + gen_exception_insn_el(dc, dc->pc_curr, EXCP_UDEF, + syn_illegalstate(), default_exception_el(dc)); return; } @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) */ tcg_remove_ops_after(dc->insn_eci_rewind); dc->condjmp = 0; - gen_exception_insn(dc, dc->pc_curr, EXCP_INVSTATE, syn_uncategorized(), - default_exception_el(dc)); + gen_exception_insn_el(dc, dc->pc_curr, EXCP_INVSTATE, syn_uncategorized(), + default_exception_el(dc)); } arm_post_translate_insn(dc); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Create a new wrapper function that passes the default exception target to gen_exception_insn_el. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-13-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.h | 1 + target/arm/translate-a64.c | 15 ++++++--------- target/arm/translate-m-nocp.c | 3 +-- target/arm/translate-mve.c | 3 +-- target/arm/translate.c | 29 +++++++++++++---------------- 5 files changed, 22 insertions(+), 29 deletions(-) diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ MemOp pow2_align(unsigned i); void unallocated_encoding(DisasContext *s); void gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp, uint32_t syn, uint32_t target_el); +void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, uint32_t syn); /* Return state of Alternate Half-precision flag, caller frees result */ static inline TCGv_i32 get_ahp_flag(void) 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 gen_sysreg_undef(DisasContext *s, bool isread, } else { syndrome = syn_uncategorized(); } - gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, syndrome, - default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syndrome); } /* MRS - move from system register @@ -XXX,XX +XXX,XX @@ static void disas_exc(DisasContext *s, uint32_t insn) switch (op2_ll) { case 1: /* SVC */ gen_ss_advance(s); - gen_exception_insn_el(s, s->base.pc_next, EXCP_SWI, - syn_aa64_svc(imm16), default_exception_el(s)); + gen_exception_insn(s, s->base.pc_next, EXCP_SWI, + syn_aa64_svc(imm16)); break; case 2: /* HVC */ if (s->current_el == 0) { @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) * Illegal execution state. This has priority over BTI * exceptions, but comes after instruction abort exceptions. */ - gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, - syn_illegalstate(), default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_illegalstate()); return; } @@ -XXX,XX +XXX,XX @@ static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) if (s->btype != 0 && s->guarded_page && !btype_destination_ok(insn, s->bt, s->btype)) { - gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, - syn_btitrap(s->btype), - default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, + syn_btitrap(s->btype)); return; } } else { diff --git a/target/arm/translate-m-nocp.c b/target/arm/translate-m-nocp.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-m-nocp.c +++ b/target/arm/translate-m-nocp.c @@ -XXX,XX +XXX,XX @@ static bool trans_NOCP(DisasContext *s, arg_nocp *a) } if (a->cp != 10) { - gen_exception_insn_el(s, s->pc_curr, EXCP_NOCP, - syn_uncategorized(), default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_NOCP, syn_uncategorized()); return true; } diff --git a/target/arm/translate-mve.c b/target/arm/translate-mve.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-mve.c +++ b/target/arm/translate-mve.c @@ -XXX,XX +XXX,XX @@ bool mve_eci_check(DisasContext *s) return true; default: /* Reserved value: INVSTATE UsageFault */ - gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), - default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized()); return false; } } 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 gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp, gen_exception_insn_el_v(s, pc, excp, syn, tcg_constant_i32(target_el)); } +void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, uint32_t syn) +{ + gen_exception_insn_el(s, pc, excp, syn, default_exception_el(s)); +} + static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn) { gen_set_condexec(s); @@ -XXX,XX +XXX,XX @@ static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn) void unallocated_encoding(DisasContext *s) { /* Unallocated and reserved encodings are uncategorized */ - gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized()); } /* Force a TB lookup after an instruction that changes the CPU state. */ @@ -XXX,XX +XXX,XX @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn, * an exception and return false. Otherwise it will return true, * and set *tgtmode and *regno appropriately. */ - int exc_target = default_exception_el(s); - /* These instructions are present only in ARMv8, or in ARMv7 with the * Virtualization Extensions. */ @@ -XXX,XX +XXX,XX @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn, undef: /* If we get here then some access check did not pass */ - gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, - syn_uncategorized(), exc_target); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized()); return false; } @@ -XXX,XX +XXX,XX @@ static bool trans_LE(DisasContext *s, arg_LE *a) tmp = load_cpu_field(v7m.ltpsize); tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 4, skipexc); tcg_temp_free_i32(tmp); - gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), - default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized()); gen_set_label(skipexc); } @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) * UsageFault exception. */ if (arm_dc_feature(s, ARM_FEATURE_M)) { - gen_exception_insn_el(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized(), - default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_INVSTATE, syn_uncategorized()); return; } @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) * Illegal execution state. This has priority over BTI * exceptions, but comes after instruction abort exceptions. */ - gen_exception_insn_el(s, s->pc_curr, EXCP_UDEF, - syn_illegalstate(), default_exception_el(s)); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_illegalstate()); return; } @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) * Illegal execution state. This has priority over BTI * exceptions, but comes after instruction abort exceptions. */ - gen_exception_insn_el(dc, dc->pc_curr, EXCP_UDEF, - syn_illegalstate(), default_exception_el(dc)); + gen_exception_insn(dc, dc->pc_curr, EXCP_UDEF, syn_illegalstate()); return; } @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) */ tcg_remove_ops_after(dc->insn_eci_rewind); dc->condjmp = 0; - gen_exception_insn_el(dc, dc->pc_curr, EXCP_INVSTATE, syn_uncategorized(), - default_exception_el(dc)); + gen_exception_insn(dc, dc->pc_curr, EXCP_INVSTATE, + syn_uncategorized()); } arm_post_translate_insn(dc); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Move the computation from gen_swstep_exception into a helper. This fixes a bug when: - MDSCR_EL1.KDE == 1 to enable debug exceptions within EL_D itself - we singlestep an ERET from EL_D to some lower EL Previously we were computing 'same el' based on the EL which executed the ERET instruction, whereas it ought to be computed based on the EL to which ERET returned. This happens naturally with the new helper, which runs after EL has been changed. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-14-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.h | 1 + target/arm/translate.h | 12 +++--------- target/arm/debug_helper.c | 16 ++++++++++++++++ 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_FLAGS_3(sel_flags, TCG_CALL_NO_RWG_SE, DEF_HELPER_2(exception_internal, noreturn, env, i32) DEF_HELPER_4(exception_with_syndrome_el, noreturn, env, i32, i32, i32) DEF_HELPER_2(exception_bkpt_insn, noreturn, env, i32) +DEF_HELPER_2(exception_swstep, noreturn, env, i32) DEF_HELPER_2(exception_pc_alignment, noreturn, env, tl) DEF_HELPER_1(setend, void, env) DEF_HELPER_2(wfi, void, env, i32) diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ static inline void gen_exception(int excp, uint32_t syndrome, /* Generate an architectural singlestep exception */ static inline void gen_swstep_exception(DisasContext *s, int isv, int ex) { - bool same_el = (s->debug_target_el == s->current_el); - - /* - * If singlestep is targeting a lower EL than the current one, - * then s->ss_active must be false and we can never get here. - */ - assert(s->debug_target_el >= s->current_el); - - gen_exception(EXCP_UDEF, syn_swstep(same_el, isv, ex), s->debug_target_el); + /* Fill in the same_el field of the syndrome in the helper. */ + uint32_t syn = syn_swstep(false, isv, ex); + gen_helper_exception_swstep(cpu_env, tcg_constant_i32(syn)); } /* diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -XXX,XX +XXX,XX @@ void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome) raise_exception(env, EXCP_BKPT, syndrome, debug_el); } +void HELPER(exception_swstep)(CPUARMState *env, uint32_t syndrome) +{ + int debug_el = arm_debug_target_el(env); + int cur_el = arm_current_el(env); + + /* + * If singlestep is targeting a lower EL than the current one, then + * DisasContext.ss_active must be false and we can never get here. + */ + assert(debug_el >= cur_el); + if (debug_el == cur_el) { + syndrome |= 1 << ARM_EL_EC_SHIFT; + } + raise_exception(env, EXCP_UDEF, syndrome, debug_el); +} + #if !defined(CONFIG_USER_ONLY) vaddr arm_adjust_watchpoint_address(CPUState *cs, vaddr addr, int len) -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> We no longer need this value during translation, as it is now handled within the helpers. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-15-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 6 ++---- target/arm/translate.h | 2 -- target/arm/helper.c | 12 ++---------- target/arm/translate-a64.c | 1 - target/arm/translate.c | 1 - 5 files changed, 4 insertions(+), 18 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 @@ FIELD(TBFLAG_ANY, BE_DATA, 3, 1) FIELD(TBFLAG_ANY, MMUIDX, 4, 4) /* Target EL if we take a floating-point-disabled exception */ FIELD(TBFLAG_ANY, FPEXC_EL, 8, 2) -/* For A-profile only, target EL for debug exceptions. */ -FIELD(TBFLAG_ANY, DEBUG_TARGET_EL, 10, 2) /* Memory operations require alignment: SCTLR_ELx.A or CCR.UNALIGN_TRP */ -FIELD(TBFLAG_ANY, ALIGN_MEM, 12, 1) -FIELD(TBFLAG_ANY, PSTATE__IL, 13, 1) +FIELD(TBFLAG_ANY, ALIGN_MEM, 10, 1) +FIELD(TBFLAG_ANY, PSTATE__IL, 11, 1) /* * Bit usage when in AArch32 state, both A- and M-profile. diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ typedef struct DisasContext { */ uint32_t svc_imm; int current_el; - /* Debug target exception level for single-step exceptions */ - int debug_target_el; GHashTable *cp_regs; uint64_t features; /* CPU features bits */ bool aarch64; 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 CPUARMTBFlags rebuild_hflags_m32(CPUARMState *env, int fp_el, return rebuild_hflags_common_32(env, fp_el, mmu_idx, flags); } -static CPUARMTBFlags rebuild_hflags_aprofile(CPUARMState *env) -{ - CPUARMTBFlags flags = {}; - - DP_TBFLAG_ANY(flags, DEBUG_TARGET_EL, arm_debug_target_el(env)); - return flags; -} - static CPUARMTBFlags rebuild_hflags_a32(CPUARMState *env, int fp_el, ARMMMUIdx mmu_idx) { - CPUARMTBFlags flags = rebuild_hflags_aprofile(env); + CPUARMTBFlags flags = {}; int el = arm_current_el(env); if (arm_sctlr(env, el) & SCTLR_A) { @@ -XXX,XX +XXX,XX @@ static CPUARMTBFlags rebuild_hflags_a32(CPUARMState *env, int fp_el, static CPUARMTBFlags rebuild_hflags_a64(CPUARMState *env, int el, int fp_el, ARMMMUIdx mmu_idx) { - CPUARMTBFlags flags = rebuild_hflags_aprofile(env); + CPUARMTBFlags flags = {}; ARMMMUIdx stage1 = stage_1_mmu_idx(mmu_idx); uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr; uint64_t sctlr; 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 aarch64_tr_init_disas_context(DisasContextBase *dcbase, dc->ss_active = EX_TBFLAG_ANY(tb_flags, SS_ACTIVE); dc->pstate_ss = EX_TBFLAG_ANY(tb_flags, PSTATE__SS); dc->is_ldex = false; - dc->debug_target_el = EX_TBFLAG_ANY(tb_flags, DEBUG_TARGET_EL); /* Bound the number of insns to execute to those left on the page. */ bound = -(dc->base.pc_first | TARGET_PAGE_MASK) / 4; 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 void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) dc->v7m_lspact = EX_TBFLAG_M32(tb_flags, LSPACT); dc->mve_no_pred = EX_TBFLAG_M32(tb_flags, MVE_NO_PRED); } else { - dc->debug_target_el = EX_TBFLAG_ANY(tb_flags, DEBUG_TARGET_EL); dc->sctlr_b = EX_TBFLAG_A32(tb_flags, SCTLR__B); dc->hstr_active = EX_TBFLAG_A32(tb_flags, HSTR_ACTIVE); dc->ns = EX_TBFLAG_A32(tb_flags, NS); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> This function is not required by any other translation file. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-16-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.h | 8 -------- target/arm/translate.c | 7 +++++++ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ static inline void gen_ss_advance(DisasContext *s) } } -static inline void gen_exception(int excp, uint32_t syndrome, - uint32_t target_el) -{ - gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp), - tcg_constant_i32(syndrome), - tcg_constant_i32(target_el)); -} - /* Generate an architectural singlestep exception */ static inline void gen_swstep_exception(DisasContext *s, int isv, int ex) { 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 void gen_exception_internal_insn(DisasContext *s, uint32_t pc, int excp) s->base.is_jmp = DISAS_NORETURN; } +static void gen_exception(int excp, uint32_t syndrome, uint32_t target_el) +{ + gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp), + tcg_constant_i32(syndrome), + tcg_constant_i32(target_el)); +} + static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp, uint32_t syn, TCGv_i32 tcg_el) { -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-17-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 18 +++++++++--------- 1 file changed, 9 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 void gen_exception_internal_insn(DisasContext *s, uint32_t pc, int excp) s->base.is_jmp = DISAS_NORETURN; } -static void gen_exception(int excp, uint32_t syndrome, uint32_t target_el) +static void gen_exception_el(int excp, uint32_t syndrome, uint32_t target_el) { gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp), tcg_constant_i32(syndrome), @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) switch (dc->base.is_jmp) { case DISAS_SWI: gen_ss_advance(dc); - gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb), - default_exception_el(dc)); + gen_exception_el(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb), + default_exception_el(dc)); break; case DISAS_HVC: gen_ss_advance(dc); - gen_exception(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2); + gen_exception_el(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2); break; case DISAS_SMC: gen_ss_advance(dc); - gen_exception(EXCP_SMC, syn_aa32_smc(), 3); + gen_exception_el(EXCP_SMC, syn_aa32_smc(), 3); break; case DISAS_NEXT: case DISAS_TOO_MANY: @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) gen_helper_yield(cpu_env); break; case DISAS_SWI: - gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb), - default_exception_el(dc)); + gen_exception_el(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb), + default_exception_el(dc)); break; case DISAS_HVC: - gen_exception(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2); + gen_exception_el(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2); break; case DISAS_SMC: - gen_exception(EXCP_SMC, syn_aa32_smc(), 3); + gen_exception_el(EXCP_SMC, syn_aa32_smc(), 3); break; } } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Create a new wrapper function that passes the default exception target to gen_exception_el. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-18-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 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 void gen_exception_el(int excp, uint32_t syndrome, uint32_t target_el) tcg_constant_i32(target_el)); } +static void gen_exception(DisasContext *s, int excp, uint32_t syndrome) +{ + gen_exception_el(excp, syndrome, default_exception_el(s)); +} + static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp, uint32_t syn, TCGv_i32 tcg_el) { @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) switch (dc->base.is_jmp) { case DISAS_SWI: gen_ss_advance(dc); - gen_exception_el(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb), - default_exception_el(dc)); + gen_exception(dc, EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb)); break; case DISAS_HVC: gen_ss_advance(dc); @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) gen_helper_yield(cpu_env); break; case DISAS_SWI: - gen_exception_el(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb), - default_exception_el(dc)); + gen_exception(dc, EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb)); break; case DISAS_HVC: gen_exception_el(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Split out a common helper function for gen_exception_el and gen_exception_insn_el_v. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-19-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 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 void gen_exception_internal_insn(DisasContext *s, uint32_t pc, int excp) s->base.is_jmp = DISAS_NORETURN; } -static void gen_exception_el(int excp, uint32_t syndrome, uint32_t target_el) +static void gen_exception_el_v(int excp, uint32_t syndrome, TCGv_i32 tcg_el) { gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp), - tcg_constant_i32(syndrome), - tcg_constant_i32(target_el)); + tcg_constant_i32(syndrome), tcg_el); +} + +static void gen_exception_el(int excp, uint32_t syndrome, uint32_t target_el) +{ + gen_exception_el_v(excp, syndrome, tcg_constant_i32(target_el)); } static void gen_exception(DisasContext *s, int excp, uint32_t syndrome) @@ -XXX,XX +XXX,XX @@ static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp, gen_set_condexec(s); gen_set_pc_im(s, pc); } - gen_helper_exception_with_syndrome_el(cpu_env, tcg_constant_i32(excp), - tcg_constant_i32(syn), tcg_el); + gen_exception_el_v(excp, syn, tcg_el); s->base.is_jmp = DISAS_NORETURN; } -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> With the helper we can use exception_target_el at runtime, instead of default_exception_el at translate time. While we're at it, remove the DisasContext parameter from gen_exception, as it is no longer used. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-20-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.h | 1 + target/arm/op_helper.c | 10 ++++++++++ target/arm/translate.c | 18 +++++++++++++----- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_FLAGS_2(usad8, TCG_CALL_NO_RWG_SE, i32, i32, i32) DEF_HELPER_FLAGS_3(sel_flags, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32) DEF_HELPER_2(exception_internal, noreturn, env, i32) +DEF_HELPER_3(exception_with_syndrome, noreturn, env, i32, i32) DEF_HELPER_4(exception_with_syndrome_el, noreturn, env, i32, i32, i32) DEF_HELPER_2(exception_bkpt_insn, noreturn, env, i32) DEF_HELPER_2(exception_swstep, noreturn, env, i32) diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -XXX,XX +XXX,XX @@ void HELPER(exception_with_syndrome_el)(CPUARMState *env, uint32_t excp, raise_exception(env, excp, syndrome, target_el); } +/* + * Raise an exception with the specified syndrome register value + * to the default target el. + */ +void HELPER(exception_with_syndrome)(CPUARMState *env, uint32_t excp, + uint32_t syndrome) +{ + raise_exception(env, excp, syndrome, exception_target_el(env)); +} + uint32_t HELPER(cpsr_read)(CPUARMState *env) { return cpsr_read(env) & ~CPSR_EXEC; 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 void gen_exception_el(int excp, uint32_t syndrome, uint32_t target_el) gen_exception_el_v(excp, syndrome, tcg_constant_i32(target_el)); } -static void gen_exception(DisasContext *s, int excp, uint32_t syndrome) +static void gen_exception(int excp, uint32_t syndrome) { - gen_exception_el(excp, syndrome, default_exception_el(s)); + gen_helper_exception_with_syndrome(cpu_env, tcg_constant_i32(excp), + tcg_constant_i32(syndrome)); } static void gen_exception_insn_el_v(DisasContext *s, uint64_t pc, int excp, @@ -XXX,XX +XXX,XX @@ void gen_exception_insn_el(DisasContext *s, uint64_t pc, int excp, void gen_exception_insn(DisasContext *s, uint64_t pc, int excp, uint32_t syn) { - gen_exception_insn_el(s, pc, excp, syn, default_exception_el(s)); + if (s->aarch64) { + gen_a64_set_pc_im(pc); + } else { + gen_set_condexec(s); + gen_set_pc_im(s, pc); + } + gen_exception(excp, syn); + s->base.is_jmp = DISAS_NORETURN; } static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn) @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) switch (dc->base.is_jmp) { case DISAS_SWI: gen_ss_advance(dc); - gen_exception(dc, EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb)); + gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb)); break; case DISAS_HVC: gen_ss_advance(dc); @@ -XXX,XX +XXX,XX @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) gen_helper_yield(cpu_env); break; case DISAS_SWI: - gen_exception(dc, EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb)); + gen_exception(EXCP_SWI, syn_aa32_svc(dc->svc_imm, dc->thumb)); break; case DISAS_HVC: gen_exception_el(EXCP_HVC, syn_aa32_hvc(dc->svc_imm), 2); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> This function is no longer used. At the same time, remove DisasContext.secure_routed_to_el3, as it in turn becomes unused. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-21-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.h | 16 ---------------- target/arm/translate-a64.c | 5 ----- target/arm/translate.c | 5 ----- 3 files changed, 26 deletions(-) diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ typedef struct DisasContext { int fp_excp_el; /* FP exception EL or 0 if enabled */ int sve_excp_el; /* SVE exception EL or 0 if enabled */ int vl; /* current vector length in bytes */ - /* Flag indicating that exceptions from secure mode are routed to EL3. */ - bool secure_routed_to_el3; bool vfp_enabled; /* FP enabled via FPSCR.EN */ int vec_len; int vec_stride; @@ -XXX,XX +XXX,XX @@ static inline int get_mem_index(DisasContext *s) return arm_to_core_mmu_idx(s->mmu_idx); } -/* Function used to determine the target exception EL when otherwise not known - * or default. - */ -static inline int default_exception_el(DisasContext *s) -{ - /* If we are coming from secure EL0 in a system with a 32-bit EL3, then - * there is no secure EL1, so we route exceptions to EL3. Otherwise, - * exceptions can only be routed to ELs above 1, so we target the higher of - * 1 or the current EL. - */ - return (s->mmu_idx == ARMMMUIdx_SE10_0 && s->secure_routed_to_el3) - ? 3 : MAX(1, s->current_el); -} - static inline void disas_set_insn_syndrome(DisasContext *s, uint32_t syn) { /* We don't need to save all of the syndrome so we mask and shift 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 aarch64_tr_init_disas_context(DisasContextBase *dcbase, dc->condjmp = 0; dc->aarch64 = true; - /* If we are coming from secure EL0 in a system with a 32-bit EL3, then - * there is no secure EL1, so we route exceptions to EL3. - */ - dc->secure_routed_to_el3 = arm_feature(env, ARM_FEATURE_EL3) && - !arm_el_is_aa64(env, 3); dc->thumb = false; dc->sctlr_b = 0; dc->be_data = EX_TBFLAG_ANY(tb_flags, BE_DATA) ? MO_BE : MO_LE; 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 void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) dc->condjmp = 0; dc->aarch64 = false; - /* If we are coming from secure EL0 in a system with a 32-bit EL3, then - * there is no secure EL1, so we route exceptions to EL3. - */ - dc->secure_routed_to_el3 = arm_feature(env, ARM_FEATURE_EL3) && - !arm_el_is_aa64(env, 3); dc->thumb = EX_TBFLAG_AM32(tb_flags, THUMB); dc->be_data = EX_TBFLAG_ANY(tb_flags, BE_DATA) ? MO_BE : MO_LE; condexec = EX_TBFLAG_AM32(tb_flags, CONDEXEC); -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Handle the debug vs current el exception test in one place. Leave EXCP_BKPT alone, since that treats debug < current differently. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-22-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/debug_helper.c | 44 +++++++++++++++++++++------------------ 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -XXX,XX +XXX,XX @@ #include "exec/helper-proto.h" +/* + * Raise an exception to the debug target el. + * Modify syndrome to indicate when origin and target EL are the same. + */ +G_NORETURN static void +raise_exception_debug(CPUARMState *env, uint32_t excp, uint32_t syndrome) +{ + int debug_el = arm_debug_target_el(env); + int cur_el = arm_current_el(env); + + /* + * If singlestep is targeting a lower EL than the current one, then + * DisasContext.ss_active must be false and we can never get here. + * Similarly for watchpoint and breakpoint matches. + */ + assert(debug_el >= cur_el); + syndrome |= (debug_el == cur_el) << ARM_EL_EC_SHIFT; + raise_exception(env, excp, syndrome, debug_el); +} + /* See AArch64.GenerateDebugExceptionsFrom() in ARM ARM pseudocode */ static bool aa64_generate_debug_exceptions(CPUARMState *env) { @@ -XXX,XX +XXX,XX @@ void arm_debug_excp_handler(CPUState *cs) if (wp_hit) { if (wp_hit->flags & BP_CPU) { bool wnr = (wp_hit->flags & BP_WATCHPOINT_HIT_WRITE) != 0; - bool same_el = arm_debug_target_el(env) == arm_current_el(env); cs->watchpoint_hit = NULL; env->exception.fsr = arm_debug_exception_fsr(env); env->exception.vaddress = wp_hit->hitaddr; - raise_exception(env, EXCP_DATA_ABORT, - syn_watchpoint(same_el, 0, wnr), - arm_debug_target_el(env)); + raise_exception_debug(env, EXCP_DATA_ABORT, + syn_watchpoint(0, 0, wnr)); } } else { uint64_t pc = is_a64(env) ? env->pc : env->regs[15]; - bool same_el = (arm_debug_target_el(env) == arm_current_el(env)); /* * (1) GDB breakpoints should be handled first. @@ -XXX,XX +XXX,XX @@ void arm_debug_excp_handler(CPUState *cs) * exception/security level. */ env->exception.vaddress = 0; - raise_exception(env, EXCP_PREFETCH_ABORT, - syn_breakpoint(same_el), - arm_debug_target_el(env)); + raise_exception_debug(env, EXCP_PREFETCH_ABORT, syn_breakpoint(0)); } } @@ -XXX,XX +XXX,XX @@ void HELPER(exception_bkpt_insn)(CPUARMState *env, uint32_t syndrome) void HELPER(exception_swstep)(CPUARMState *env, uint32_t syndrome) { - int debug_el = arm_debug_target_el(env); - int cur_el = arm_current_el(env); - - /* - * If singlestep is targeting a lower EL than the current one, then - * DisasContext.ss_active must be false and we can never get here. - */ - assert(debug_el >= cur_el); - if (debug_el == cur_el) { - syndrome |= 1 << ARM_EL_EC_SHIFT; - } - raise_exception(env, EXCP_UDEF, syndrome, debug_el); + raise_exception_debug(env, EXCP_UDEF, syndrome); } #if !defined(CONFIG_USER_ONLY) -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> This function is no longer used outside debug_helper.c. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-23-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 21 --------------------- target/arm/debug_helper.c | 21 +++++++++++++++++++++ 2 files changed, 21 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 enum ARMASIdx { ARMASIdx_TagS = 3, } ARMASIdx; -/* Return the Exception Level targeted by debug exceptions. */ -static inline int arm_debug_target_el(CPUARMState *env) -{ - bool secure = arm_is_secure(env); - bool route_to_el2 = false; - - if (arm_is_el2_enabled(env)) { - route_to_el2 = env->cp15.hcr_el2 & HCR_TGE || - env->cp15.mdcr_el2 & MDCR_TDE; - } - - if (route_to_el2) { - return 2; - } else if (arm_feature(env, ARM_FEATURE_EL3) && - !arm_el_is_aa64(env, 3) && secure) { - return 3; - } else { - return 1; - } -} - static inline bool arm_v7m_csselr_razwi(ARMCPU *cpu) { /* If all the CLIDR.Ctypem bits are 0 there are no caches, and diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/debug_helper.c +++ b/target/arm/debug_helper.c @@ -XXX,XX +XXX,XX @@ #include "exec/helper-proto.h" +/* Return the Exception Level targeted by debug exceptions. */ +static int arm_debug_target_el(CPUARMState *env) +{ + bool secure = arm_is_secure(env); + bool route_to_el2 = false; + + if (arm_is_el2_enabled(env)) { + route_to_el2 = env->cp15.hcr_el2 & HCR_TGE || + env->cp15.mdcr_el2 & MDCR_TDE; + } + + if (route_to_el2) { + return 2; + } else if (arm_feature(env, ARM_FEATURE_EL3) && + !arm_el_is_aa64(env, 3) && secure) { + return 3; + } else { + return 1; + } +} + /* * Raise an exception to the debug target el. * Modify syndrome to indicate when origin and target EL are the same. -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> We were using arm_is_secure and is_a64, which are tests against the current EL, as opposed to arm_el_is_aa64 and arm_is_secure_below_el3, which can be applied to a different EL than current. Consolidate the two tests. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609202901.1177572-24-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 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 @@ int fp_exception_el(CPUARMState *env, int cur_el) int fpen = FIELD_EX64(env->cp15.cpacr_el1, CPACR_EL1, FPEN); switch (fpen) { + case 1: + if (cur_el != 0) { + break; + } + /* fall through */ case 0: case 2: - if (cur_el == 0 || cur_el == 1) { - /* Trap to PL1, which might be EL1 or EL3 */ - if (arm_is_secure(env) && !arm_el_is_aa64(env, 3)) { - return 3; - } - return 1; - } - if (cur_el == 3 && !is_a64(env)) { - /* Secure PL1 running at EL3 */ + /* Trap from Secure PL0 or PL1 to Secure PL1. */ + if (!arm_el_is_aa64(env, 3) + && (cur_el == 3 || arm_is_secure_below_el3(env))) { return 3; } - break; - case 1: - if (cur_el == 0) { + if (cur_el <= 1) { return 1; } break; - case 3: - break; } } -- 2.25.1 From: Hao Wu <wuhaotsh@google.com> Creating 1GB image for a simple qtest is unnecessary and could lead to failures. We reduce the image size to 1MB to reduce the test overhead. Signed-off-by: Hao Wu <wuhaotsh@google.com> Message-id: 20220609214125.4192212-1-wuhaotsh@google.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/qtest/npcm7xx_sdhci-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qtest/npcm7xx_sdhci-test.c b/tests/qtest/npcm7xx_sdhci-test.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/npcm7xx_sdhci-test.c +++ b/tests/qtest/npcm7xx_sdhci-test.c @@ -XXX,XX +XXX,XX @@ #define NPCM7XX_REG_SIZE 0x100 #define NPCM7XX_MMC_BA 0xF0842000 #define NPCM7XX_BLK_SIZE 512 -#define NPCM7XX_TEST_IMAGE_SIZE (1 << 30) +#define NPCM7XX_TEST_IMAGE_SIZE (1 << 20) char *sd_path; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Because reset always initializes the AA64 version, SCR_EL3, test the mode of EL3 instead of the type of the cpreg. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609214657.1217913-2-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 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 void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) uint32_t valid_mask = 0x3fff; ARMCPU *cpu = env_archcpu(env); - if (ri->state == ARM_CP_STATE_AA64) { - if (arm_feature(env, ARM_FEATURE_AARCH64) && - !cpu_isar_feature(aa64_aa32_el1, cpu)) { - value |= SCR_FW | SCR_AW; /* these two bits are RES1. */ - } - valid_mask &= ~SCR_NET; + /* + * Because SCR_EL3 is the "real" cpreg and SCR is the alias, reset always + * passes the reginfo for SCR_EL3, which has type ARM_CP_STATE_AA64. + * Instead, choose the format based on the mode of EL3. + */ + if (arm_el_is_aa64(env, 3)) { + value |= SCR_FW | SCR_AW; /* RES1 */ + valid_mask &= ~SCR_NET; /* RES0 */ if (cpu_isar_feature(aa64_ras, cpu)) { valid_mask |= SCR_TERR; -- 2.25.1 From: Richard Henderson <richard.henderson@linaro.org> Since DDI0487F.a, the RW bit is RAO/WI. When specifically targeting such a cpu, e.g. cortex-a76, it is legitimate to ignore the bit within the secure monitor. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1062 Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220609214657.1217913-3-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 5 +++++ target/arm/helper.c | 4 ++++ 2 files changed, 9 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 @@ static inline bool isar_feature_aa64_aa32_el1(const ARMISARegisters *id) return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL1) >= 2; } +static inline bool isar_feature_aa64_aa32_el2(const ARMISARegisters *id) +{ + return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) >= 2; +} + static inline bool isar_feature_aa64_ras(const ARMISARegisters *id) { return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, RAS) != 0; 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 scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) value |= SCR_FW | SCR_AW; /* RES1 */ valid_mask &= ~SCR_NET; /* RES0 */ + if (!cpu_isar_feature(aa64_aa32_el1, cpu) && + !cpu_isar_feature(aa64_aa32_el2, cpu)) { + value |= SCR_RW; /* RAO/WI */ + } if (cpu_isar_feature(aa64_ras, cpu)) { valid_mask |= SCR_TERR; } -- 2.25.1 In two places in gdbstub.c we look at gdbserver_state.init to decide whether we're going to do a semihosting syscall via the gdb remote protocol: * when setting up, if the user didn't explicitly select either native semihosting or gdb semihosting, we autoselect, with the intended behaviour "use gdb if gdb is connected" * when the semihosting layer attempts to do a syscall via gdb, we silently ignore it if the gdbstub wasn't actually set up However, if the user's commandline sets up the gdbstub but tells QEMU to start rather than waiting for a GDB to connect (eg using '-s' but not '-S'), then we will have gdbserver_state.init true but no actual connection; an attempt to use gdb syscalls will then crash because we try to use gdbserver_state.c_cpu when it hasn't been set up: #0 0x00007ffff6803ba8 in qemu_cpu_kick (cpu=0x0) at ../../softmmu/cpus.c:457 #1 0x00007ffff6c03913 in gdb_do_syscallv (cb=0x7ffff6c19944 <common_semi_cb>, fmt=0x7ffff7573b7e "", va=0x7ffff56294c0) at ../../gdbstub.c:2946 #2 0x00007ffff6c19c3a in common_semi_gdb_syscall (cs=0x7ffff83fe060, cb=0x7ffff6c19944 <common_semi_cb>, fmt=0x7ffff7573b75 "isatty,%x") at ../../semihosting/arm-compat-semi.c:494 #3 0x00007ffff6c1a064 in gdb_isattyfn (cs=0x7ffff83fe060, gf=0x7ffff86a3690) at ../../semihosting/arm-compat-semi.c:636 #4 0x00007ffff6c1b20f in do_common_semihosting (cs=0x7ffff83fe060) at ../../semihosting/arm-compat-semi.c:967 #5 0x00007ffff693a037 in handle_semihosting (cs=0x7ffff83fe060) at ../../target/arm/helper.c:10316 You can probably also get into this state via some odd corner cases involving connecting a GDB and then telling it to detach from all the vCPUs. Abstract out the test into a new gdb_attached() function which returns true only if there's actually a GDB connected to the debug stub and attached to at least one vCPU. Reported-by: Liviu Ionescu <ilg@livius.net> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Message-id: 20220526190053.521505-2-peter.maydell@linaro.org --- gdbstub.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index XXXXXXX..XXXXXXX 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -XXX,XX +XXX,XX @@ static int get_char(void) } #endif +/* + * Return true if there is a GDB currently connected to the stub + * and attached to a CPU + */ +static bool gdb_attached(void) +{ + return gdbserver_state.init && gdbserver_state.c_cpu; +} + static enum { GDB_SYS_UNKNOWN, GDB_SYS_ENABLED, @@ -XXX,XX +XXX,XX @@ int use_gdb_syscalls(void) /* -semihosting-config target=auto */ /* On the first call check if gdb is connected and remember. */ if (gdb_syscall_mode == GDB_SYS_UNKNOWN) { - gdb_syscall_mode = gdbserver_state.init ? - GDB_SYS_ENABLED : GDB_SYS_DISABLED; + gdb_syscall_mode = gdb_attached() ? GDB_SYS_ENABLED : GDB_SYS_DISABLED; } return gdb_syscall_mode == GDB_SYS_ENABLED; } @@ -XXX,XX +XXX,XX @@ void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va) target_ulong addr; uint64_t i64; - if (!gdbserver_state.init) { + if (!gdb_attached()) { return; } -- 2.25.1 Currently we mishandle the --semihosting-config option if the user specifies it on the command line more than once. For example with: --semihosting-config target=gdb --semihosting-config arg=foo,arg=bar the function qemu_semihosting_config_options() is called twice, once for each argument. But that function expects to be called only once, and it always unconditionally sets the semihosting.enabled, semihost_chardev and semihosting.target variables. This means that if any of those options were set anywhere except the last --semihosting-config option on the command line, those settings are ignored. In the example above, 'target=gdb' in the first option is overridden by an implied default 'target=auto' in the second. The QemuOptsList machinery has a flag for handling this kind of "option group is setting global state": by setting .merge_lists = true; we make the machinery merge all the --semihosting-config arguments the user passes into a single set of options and call our qemu_semihosting_config_options() just once. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Message-id: 20220526190053.521505-3-peter.maydell@linaro.org --- semihosting/config.c | 1 + 1 file changed, 1 insertion(+) diff --git a/semihosting/config.c b/semihosting/config.c index XXXXXXX..XXXXXXX 100644 --- a/semihosting/config.c +++ b/semihosting/config.c @@ -XXX,XX +XXX,XX @@ QemuOptsList qemu_semihosting_config_opts = { .name = "semihosting-config", + .merge_lists = true, .implied_opt_name = "enable", .head = QTAILQ_HEAD_INITIALIZER(qemu_semihosting_config_opts.head), .desc = { -- 2.25.1