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 Hi; here's a target-arm pullreq to go in before softfreeze. This is actually pretty much entirely bugfixes (since the SEL2 timers we implement here are a missing part of a feature we claim to already implement). thanks -- PMM The following changes since commit 98c7362b1efe651327385a25874a73e008c6549e: Merge tag 'accel-cpus-20250306' of https://github.com/philmd/qemu into staging (2025-03-07 07:39:49 +0800) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20250307 for you to fetch changes up to 0ce0739d46983e5e88fa9c149cb305689c9d8c6f: target/rx: Remove TCG_CALL_NO_WG from helpers which write env (2025-03-07 15:03:20 +0000) ---------------------------------------------------------------- target-arm queue: * hw/arm/smmu-common: Remove the repeated ttb field * hw/gpio: npcm7xx: fixup out-of-bounds access * tests/functional/test_arm_sx1: Check whether the serial console is working * target/arm: Fix minor bugs in generic timer register handling * target/arm: Implement SEL2 physical and virtual timers * target/arm: Correct STRD, LDRD atomicity and fault behaviour * target/arm: Make dummy debug registers RAZ, not NOP * util/qemu-timer.c: Don't warp timer from timerlist_rearm() * include/exec/memop.h: Expand comment for MO_ATOM_SUBALIGN * hw/arm/smmu: Introduce smmu_configs_inv_sid_range() helper * target/rx: Set exception vector base to 0xffffff80 * target/rx: Remove TCG_CALL_NO_WG from helpers which write env ---------------------------------------------------------------- Alex Bennée (4): target/arm: Implement SEL2 physical and virtual timers target/arm: Document the architectural names of our GTIMERs hw/arm: enable secure EL2 timers for virt machine hw/arm: enable secure EL2 timers for sbsa machine JianChunfu (2): hw/arm/smmu-common: Remove the repeated ttb field hw/arm/smmu: Introduce smmu_configs_inv_sid_range() helper Keith Packard (2): target/rx: Set exception vector base to 0xffffff80 target/rx: Remove TCG_CALL_NO_WG from helpers which write env Patrick Venture (1): hw/gpio: npcm7xx: fixup out-of-bounds access Peter Maydell (11): target/arm: Apply correct timer offset when calculating deadlines target/arm: Don't apply CNTVOFF_EL2 for EL2_VIRT timer target/arm: Make CNTPS_* UNDEF from Secure EL1 when Secure EL2 is enabled target/arm: Always apply CNTVOFF_EL2 for CNTV_TVAL_EL02 accesses target/arm: Refactor handling of timer offset for direct register accesses target/arm: Correct LDRD atomicity and fault behaviour target/arm: Correct STRD atomicity target/arm: Drop unused address_offset from op_addr_{rr, ri}_post() target/arm: Make dummy debug registers RAZ, not NOP util/qemu-timer.c: Don't warp timer from timerlist_rearm() include/exec/memop.h: Expand comment for MO_ATOM_SUBALIGN Thomas Huth (1): tests/functional/test_arm_sx1: Check whether the serial console is working MAINTAINERS | 1 + hw/arm/smmu-internal.h | 5 - include/exec/memop.h | 8 +- include/hw/arm/bsa.h | 2 + include/hw/arm/smmu-common.h | 7 +- target/arm/cpu.h | 2 + target/arm/gtimer.h | 14 +- target/arm/internals.h | 5 +- target/rx/helper.h | 34 ++-- hw/arm/sbsa-ref.c | 2 + hw/arm/smmu-common.c | 21 +++ hw/arm/smmuv3.c | 19 +-- hw/arm/virt.c | 2 + hw/gpio/npcm7xx_gpio.c | 3 +- target/arm/cpu.c | 4 + target/arm/debug_helper.c | 7 +- target/arm/helper.c | 324 ++++++++++++++++++++++++++++++++------- target/arm/tcg/op_helper.c | 8 +- target/arm/tcg/translate.c | 147 +++++++++++------- target/rx/helper.c | 2 +- util/qemu-timer.c | 4 - hw/arm/trace-events | 3 +- tests/functional/test_arm_sx1.py | 7 +- 23 files changed, 455 insertions(+), 176 deletions(-) From: JianChunfu <jansef.jian@hj-micro.com> SMMUTransCfg->ttb is never used in QEMU, TT base address can be accessed by SMMUTransCfg->tt[i]->ttb. Signed-off-by: JianChunfu <jansef.jian@hj-micro.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Message-id: 20250221031034.69822-1-jansef.jian@hj-micro.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/smmu-common.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/smmu-common.h +++ b/include/hw/arm/smmu-common.h @@ -XXX,XX +XXX,XX @@ typedef struct SMMUTransCfg { /* Used by stage-1 only. */ bool aa64; /* arch64 or aarch32 translation table */ bool record_faults; /* record fault events */ - uint64_t ttb; /* TT base address */ uint8_t oas; /* output address width */ uint8_t tbi; /* Top Byte Ignore */ int asid; -- 2.43.0 From: Patrick Venture <venture@google.com> The reg isn't validated to be a possible register before it's dereferenced for one case. The mmio space registered for the gpio device is 4KiB but there aren't that many registers in the struct. Cc: qemu-stable@nongnu.org Fixes: 526dbbe0874 ("hw/gpio: Add GPIO model for Nuvoton NPCM7xx") Signed-off-by: Patrick Venture <venture@google.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-id: 20250226024603.493148-1-venture@google.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/gpio/npcm7xx_gpio.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/gpio/npcm7xx_gpio.c b/hw/gpio/npcm7xx_gpio.c index XXXXXXX..XXXXXXX 100644 --- a/hw/gpio/npcm7xx_gpio.c +++ b/hw/gpio/npcm7xx_gpio.c @@ -XXX,XX +XXX,XX @@ static void npcm7xx_gpio_regs_write(void *opaque, hwaddr addr, uint64_t v, return; } - diff = s->regs[reg] ^ value; - switch (reg) { case NPCM7XX_GPIO_TLOCK1: case NPCM7XX_GPIO_TLOCK2: @@ -XXX,XX +XXX,XX @@ static void npcm7xx_gpio_regs_write(void *opaque, hwaddr addr, uint64_t v, case NPCM7XX_GPIO_PU: case NPCM7XX_GPIO_PD: case NPCM7XX_GPIO_IEM: + diff = s->regs[reg] ^ value; s->regs[reg] = value; npcm7xx_gpio_update_pins(s, diff); break; -- 2.43.0 From: Thomas Huth <thuth@redhat.com> The kernel that is used in the sx1 test prints the usual Linux log onto the serial console, but this test currently ignores it. To make sure that the serial device is working properly, let's check for some strings in the output here. While we're at it, also add the test to the corresponding section in the MAINTAINERS file. Signed-off-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-id: 20250226104833.1176253-1-thuth@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- MAINTAINERS | 1 + tests/functional/test_arm_sx1.py | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index XXXXXXX..XXXXXXX 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -XXX,XX +XXX,XX @@ S: Maintained F: hw/*/omap* F: include/hw/arm/omap.h F: docs/system/arm/sx1.rst +F: tests/functional/test_arm_sx1.py IPack M: Alberto Garcia <berto@igalia.com> diff --git a/tests/functional/test_arm_sx1.py b/tests/functional/test_arm_sx1.py index XXXXXXX..XXXXXXX 100755 --- a/tests/functional/test_arm_sx1.py +++ b/tests/functional/test_arm_sx1.py @@ -XXX,XX +XXX,XX @@ def test_arm_sx1_initrd(self): self.vm.add_args('-append', f'kunit.enable=0 rdinit=/sbin/init {self.CONSOLE_ARGS}') self.vm.add_args('-no-reboot') self.launch_kernel(zimage_path, - initrd=initrd_path) + initrd=initrd_path, + wait_for='Boot successful') self.vm.wait(timeout=120) def test_arm_sx1_sd(self): @@ -XXX,XX +XXX,XX @@ def test_arm_sx1_sd(self): self.vm.add_args('-no-reboot') self.vm.add_args('-snapshot') self.vm.add_args('-drive', f'format=raw,if=sd,file={sd_fs_path}') - self.launch_kernel(zimage_path) + self.launch_kernel(zimage_path, wait_for='Boot successful') self.vm.wait(timeout=120) def test_arm_sx1_flash(self): @@ -XXX,XX +XXX,XX @@ def test_arm_sx1_flash(self): self.vm.add_args('-no-reboot') self.vm.add_args('-snapshot') self.vm.add_args('-drive', f'format=raw,if=pflash,file={flash_path}') - self.launch_kernel(zimage_path) + self.launch_kernel(zimage_path, wait_for='Boot successful') self.vm.wait(timeout=120) if __name__ == '__main__': -- 2.43.0 When we are calculating timer deadlines, the correct definition of whether or not to apply an offset to the physical count is described in the Arm ARM DDI4087 rev L.a section D12.2.4.1. This is different from when the offset should be applied for a direct read of the counter sysreg. We got this right for the EL1 physical timer and for the EL1 virtual timer, but got all the rest wrong: they should be using a zero offset always. Factor the offset calculation out into a function that has a comment documenting exactly which offset it is calculating and which gets the HYP, SEC, and HYPVIRT cases right. Cc: qemu-stable@nongnu.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20250204125009.2281315-2-peter.maydell@linaro.org --- target/arm/helper.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 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 uint64_t gt_phys_cnt_offset(CPUARMState *env) return gt_phys_raw_cnt_offset(env); } +static uint64_t gt_indirect_access_timer_offset(CPUARMState *env, int timeridx) +{ + /* + * Return the timer offset to use for indirect accesses to the timer. + * This is the Offset value as defined in D12.2.4.1 "Operation of the + * CompareValue views of the timers". + * + * The condition here is not always the same as the condition for + * whether to apply an offset register when doing a direct read of + * the counter sysreg; those conditions are described in the + * access pseudocode for each counter register. + */ + switch (timeridx) { + case GTIMER_PHYS: + return gt_phys_raw_cnt_offset(env); + case GTIMER_VIRT: + return env->cp15.cntvoff_el2; + case GTIMER_HYP: + case GTIMER_SEC: + case GTIMER_HYPVIRT: + return 0; + default: + g_assert_not_reached(); + } +} + static void gt_recalc_timer(ARMCPU *cpu, int timeridx) { ARMGenericTimer *gt = &cpu->env.cp15.c14_timer[timeridx]; @@ -XXX,XX +XXX,XX @@ static void gt_recalc_timer(ARMCPU *cpu, int timeridx) * Timer enabled: calculate and set current ISTATUS, irq, and * reset timer to when ISTATUS next has to change */ - uint64_t offset = timeridx == GTIMER_VIRT ? - cpu->env.cp15.cntvoff_el2 : gt_phys_raw_cnt_offset(&cpu->env); + uint64_t offset = gt_indirect_access_timer_offset(&cpu->env, timeridx); uint64_t count = gt_get_countervalue(&cpu->env); /* Note that this must be unsigned 64 bit arithmetic: */ int istatus = count - offset >= gt->cval; -- 2.43.0 The CNTVOFF_EL2 offset register should only be applied for accessses to CNTVCT_EL0 and for the EL1 virtual timer (CNTV_*). We were incorrectly applying it for the EL2 virtual timer (CNTHV_*). Cc: qemu-stable@nongnu.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20250204125009.2281315-3-peter.maydell@linaro.org --- target/arm/helper.c | 2 -- 1 file changed, 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 uint64_t gt_tval_read(CPUARMState *env, const ARMCPRegInfo *ri, switch (timeridx) { case GTIMER_VIRT: - case GTIMER_HYPVIRT: offset = gt_virt_cnt_offset(env); break; case GTIMER_PHYS: @@ -XXX,XX +XXX,XX @@ static void gt_tval_write(CPUARMState *env, const ARMCPRegInfo *ri, switch (timeridx) { case GTIMER_VIRT: - case GTIMER_HYPVIRT: offset = gt_virt_cnt_offset(env); break; case GTIMER_PHYS: -- 2.43.0 When we added Secure EL2 support, we missed that this needs an update to the access code for the EL3 physical timer registers. These are supposed to UNDEF from Secure EL1 when Secure EL2 is enabled. (Note for stable backporting: for backports to branches where CP_ACCESS_UNDEFINED is not defined, the old name to use instead is CP_ACCESS_TRAP_UNCATEGORIZED.) Cc: qemu-stable@nongnu.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20250204125009.2281315-4-peter.maydell@linaro.org --- target/arm/helper.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static CPAccessResult gt_stimer_access(CPUARMState *env, if (!arm_is_secure(env)) { return CP_ACCESS_UNDEFINED; } + if (arm_is_el2_enabled(env)) { + return CP_ACCESS_UNDEFINED; + } if (!(env->cp15.scr_el3 & SCR_ST)) { return CP_ACCESS_TRAP_EL3; } -- 2.43.0 Currently we handle CNTV_TVAL_EL02 by calling gt_tval_read() for the EL1 virt timer. This is almost correct, but the underlying CNTV_TVAL_EL0 register behaves slightly differently. CNTV_TVAL_EL02 always applies the CNTVOFF_EL2 offset; CNTV_TVAL_EL0 doesn't do so if we're at EL2 and HCR_EL2.E2H is 1. We were getting this wrong, because we ended up in gt_virt_cnt_offset() and did the E2H check. Factor out the tval read/write calculation from the selection of the offset, so that we can special case gt_virt_tval_read() and gt_virt_tval_write() to unconditionally pass CNTVOFF_EL2. Cc: qemu-stable@nongnu.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20250204125009.2281315-5-peter.maydell@linaro.org --- target/arm/helper.c | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 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 gt_cval_write(CPUARMState *env, const ARMCPRegInfo *ri, gt_recalc_timer(env_archcpu(env), timeridx); } +static uint64_t do_tval_read(CPUARMState *env, int timeridx, uint64_t offset) +{ + return (uint32_t)(env->cp15.c14_timer[timeridx].cval - + (gt_get_countervalue(env) - offset)); +} + static uint64_t gt_tval_read(CPUARMState *env, const ARMCPRegInfo *ri, int timeridx) { @@ -XXX,XX +XXX,XX @@ static uint64_t gt_tval_read(CPUARMState *env, const ARMCPRegInfo *ri, break; } - return (uint32_t)(env->cp15.c14_timer[timeridx].cval - - (gt_get_countervalue(env) - offset)); + return do_tval_read(env, timeridx, offset); +} + +static void do_tval_write(CPUARMState *env, int timeridx, uint64_t value, + uint64_t offset) +{ + trace_arm_gt_tval_write(timeridx, value); + env->cp15.c14_timer[timeridx].cval = gt_get_countervalue(env) - offset + + sextract64(value, 0, 32); + gt_recalc_timer(env_archcpu(env), timeridx); } static void gt_tval_write(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static void gt_tval_write(CPUARMState *env, const ARMCPRegInfo *ri, offset = gt_phys_cnt_offset(env); break; } - - trace_arm_gt_tval_write(timeridx, value); - env->cp15.c14_timer[timeridx].cval = gt_get_countervalue(env) - offset + - sextract64(value, 0, 32); - gt_recalc_timer(env_archcpu(env), timeridx); + do_tval_write(env, timeridx, value, offset); } static void gt_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static void gt_virt_cval_write(CPUARMState *env, const ARMCPRegInfo *ri, static uint64_t gt_virt_tval_read(CPUARMState *env, const ARMCPRegInfo *ri) { - return gt_tval_read(env, ri, GTIMER_VIRT); + /* + * This is CNTV_TVAL_EL02; unlike the underlying CNTV_TVAL_EL0 + * we always apply CNTVOFF_EL2. Special case that here rather + * than going into the generic gt_tval_read() and then having + * to re-detect that it's this register. + * Note that the accessfn/perms mean we know we're at EL2 or EL3 here. + */ + return do_tval_read(env, GTIMER_VIRT, env->cp15.cntvoff_el2); } static void gt_virt_tval_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) { - gt_tval_write(env, ri, GTIMER_VIRT, value); + /* Similarly for writes to CNTV_TVAL_EL02 */ + do_tval_write(env, GTIMER_VIRT, value, env->cp15.cntvoff_el2); } static void gt_virt_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri, -- 2.43.0 When reading or writing the timer registers, sometimes we need to apply one of the timer offsets. Specifically, this happens for direct reads of the counter registers CNTPCT_EL0 and CNTVCT_EL0 (and their self-synchronized variants CNTVCTSS_EL0 and CNTPCTSS_EL0). It also applies for direct reads and writes of the CNT*_TVAL_EL* registers that provide the 32-bit downcounting view of each timer. We currently do this with duplicated code in gt_tval_read() and gt_tval_write() and a special-case in gt_virt_cnt_read() and gt_cnt_read(). Refactor this so that we handle it all in a single function gt_direct_access_timer_offset(), to parallel how we handle the offset for indirect accesses. The call in the WFIT helper previously to gt_virt_cnt_offset() is now to gt_direct_access_timer_offset(); this is the correct behaviour, but it's not immediately obvious that it shouldn't be considered an indirect access, so we add an explanatory comment. This commit should make no behavioural changes. (Cc to stable because the following bugfix commit will depend on this one.) Cc: qemu-stable@nongnu.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20250204125009.2281315-6-peter.maydell@linaro.org --- target/arm/internals.h | 5 +- target/arm/helper.c | 103 +++++++++++++++++++------------------ target/arm/tcg/op_helper.c | 8 ++- 3 files changed, 62 insertions(+), 54 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 @@ int delete_hw_watchpoint(target_ulong addr, target_ulong len, int type); uint64_t gt_get_countervalue(CPUARMState *env); /* * Return the currently applicable offset between the system counter - * and CNTVCT_EL0 (this will be either 0 or the value of CNTVOFF_EL2). + * and the counter for the specified timer, as used for direct register + * accesses. */ -uint64_t gt_virt_cnt_offset(CPUARMState *env); +uint64_t gt_direct_access_timer_offset(CPUARMState *env, int timeridx); /* * Return mask of ARMMMUIdxBit values corresponding to an "invalidate 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 uint64_t gt_phys_raw_cnt_offset(CPUARMState *env) return 0; } -static uint64_t gt_phys_cnt_offset(CPUARMState *env) -{ - if (arm_current_el(env) >= 2) { - return 0; - } - return gt_phys_raw_cnt_offset(env); -} - static uint64_t gt_indirect_access_timer_offset(CPUARMState *env, int timeridx) { /* @@ -XXX,XX +XXX,XX @@ static uint64_t gt_indirect_access_timer_offset(CPUARMState *env, int timeridx) } } +uint64_t gt_direct_access_timer_offset(CPUARMState *env, int timeridx) +{ + /* + * Return the timer offset to use for direct accesses to the + * counter registers CNTPCT and CNTVCT, and for direct accesses + * to the CNT*_TVAL registers. + * + * This isn't exactly the same as the indirect-access offset, + * because here we also care about what EL the register access + * is being made from. + * + * This corresponds to the access pseudocode for the registers. + */ + uint64_t hcr; + + switch (timeridx) { + case GTIMER_PHYS: + if (arm_current_el(env) >= 2) { + return 0; + } + return gt_phys_raw_cnt_offset(env); + case GTIMER_VIRT: + switch (arm_current_el(env)) { + case 2: + hcr = arm_hcr_el2_eff(env); + if (hcr & HCR_E2H) { + return 0; + } + break; + case 0: + hcr = arm_hcr_el2_eff(env); + if ((hcr & (HCR_E2H | HCR_TGE)) == (HCR_E2H | HCR_TGE)) { + return 0; + } + break; + } + return env->cp15.cntvoff_el2; + case GTIMER_HYP: + case GTIMER_SEC: + case GTIMER_HYPVIRT: + return 0; + default: + g_assert_not_reached(); + } +} + static void gt_recalc_timer(ARMCPU *cpu, int timeridx) { ARMGenericTimer *gt = &cpu->env.cp15.c14_timer[timeridx]; @@ -XXX,XX +XXX,XX @@ static void gt_timer_reset(CPUARMState *env, const ARMCPRegInfo *ri, static uint64_t gt_cnt_read(CPUARMState *env, const ARMCPRegInfo *ri) { - return gt_get_countervalue(env) - gt_phys_cnt_offset(env); -} - -uint64_t gt_virt_cnt_offset(CPUARMState *env) -{ - uint64_t hcr; - - switch (arm_current_el(env)) { - case 2: - hcr = arm_hcr_el2_eff(env); - if (hcr & HCR_E2H) { - return 0; - } - break; - case 0: - hcr = arm_hcr_el2_eff(env); - if ((hcr & (HCR_E2H | HCR_TGE)) == (HCR_E2H | HCR_TGE)) { - return 0; - } - break; - } - - return env->cp15.cntvoff_el2; + uint64_t offset = gt_direct_access_timer_offset(env, GTIMER_PHYS); + return gt_get_countervalue(env) - offset; } static uint64_t gt_virt_cnt_read(CPUARMState *env, const ARMCPRegInfo *ri) { - return gt_get_countervalue(env) - gt_virt_cnt_offset(env); + uint64_t offset = gt_direct_access_timer_offset(env, GTIMER_VIRT); + return gt_get_countervalue(env) - offset; } static void gt_cval_write(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static uint64_t do_tval_read(CPUARMState *env, int timeridx, uint64_t offset) static uint64_t gt_tval_read(CPUARMState *env, const ARMCPRegInfo *ri, int timeridx) { - uint64_t offset = 0; - - switch (timeridx) { - case GTIMER_VIRT: - offset = gt_virt_cnt_offset(env); - break; - case GTIMER_PHYS: - offset = gt_phys_cnt_offset(env); - break; - } + uint64_t offset = gt_direct_access_timer_offset(env, timeridx); return do_tval_read(env, timeridx, offset); } @@ -XXX,XX +XXX,XX @@ static void gt_tval_write(CPUARMState *env, const ARMCPRegInfo *ri, int timeridx, uint64_t value) { - uint64_t offset = 0; + uint64_t offset = gt_direct_access_timer_offset(env, timeridx); - switch (timeridx) { - case GTIMER_VIRT: - offset = gt_virt_cnt_offset(env); - break; - case GTIMER_PHYS: - offset = gt_phys_cnt_offset(env); - break; - } do_tval_write(env, timeridx, value, offset); } diff --git a/target/arm/tcg/op_helper.c b/target/arm/tcg/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/tcg/op_helper.c +++ b/target/arm/tcg/op_helper.c @@ -XXX,XX +XXX,XX @@ void HELPER(wfit)(CPUARMState *env, uint64_t timeout) int target_el = check_wfx_trap(env, false, &excp); /* The WFIT should time out when CNTVCT_EL0 >= the specified value. */ uint64_t cntval = gt_get_countervalue(env); - uint64_t offset = gt_virt_cnt_offset(env); + /* + * We want the value that we would get if we read CNTVCT_EL0 from + * the current exception level, so the direct_access offset, not + * the indirect_access one. Compare the pseudocode LocalTimeoutEvent(), + * which calls VirtualCounterTimer(). + */ + uint64_t offset = gt_direct_access_timer_offset(env, GTIMER_VIRT); uint64_t cntvct = cntval - offset; uint64_t nexttick; -- 2.43.0 From: Alex Bennée <alex.bennee@linaro.org> When FEAT_SEL2 was implemented the SEL2 timers were missed. This shows up when building the latest Hafnium with SPMC_AT_EL=2. The actual implementation utilises the same logic as the rest of the timers so all we need to do is: - define the timers and their access functions - conditionally add the correct system registers - create a new accessfn as the rules are subtly different to the existing secure timer Fixes: e9152ee91c (target/arm: add ARMv8.4-SEL2 system registers) Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20250204125009.2281315-7-peter.maydell@linaro.org Cc: qemu-stable@nongnu.org Cc: Andrei Homescu <ahomescu@google.com> Cc: Arve Hjønnevåg <arve@google.com> Cc: Rémi Denis-Courmont <remi.denis.courmont@huawei.com> [PMM: CP_ACCESS_TRAP_UNCATEGORIZED -> CP_ACCESS_UNDEFINED; offset logic now in gt_{indirect,direct}_access_timer_offset() ] Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/bsa.h | 2 + target/arm/cpu.h | 2 + target/arm/gtimer.h | 4 +- target/arm/cpu.c | 4 ++ target/arm/helper.c | 163 +++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 174 insertions(+), 1 deletion(-) diff --git a/include/hw/arm/bsa.h b/include/hw/arm/bsa.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/bsa.h +++ b/include/hw/arm/bsa.h @@ -XXX,XX +XXX,XX @@ #define QEMU_ARM_BSA_H /* These are architectural INTID values */ +#define ARCH_TIMER_S_EL2_VIRT_IRQ 19 +#define ARCH_TIMER_S_EL2_IRQ 20 #define VIRTUAL_PMU_IRQ 23 #define ARCH_GIC_MAINT_IRQ 25 #define ARCH_TIMER_NS_EL2_IRQ 26 diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ void arm_gt_vtimer_cb(void *opaque); void arm_gt_htimer_cb(void *opaque); void arm_gt_stimer_cb(void *opaque); void arm_gt_hvtimer_cb(void *opaque); +void arm_gt_sel2timer_cb(void *opaque); +void arm_gt_sel2vtimer_cb(void *opaque); unsigned int gt_cntfrq_period_ns(ARMCPU *cpu); void gt_rme_post_el_change(ARMCPU *cpu, void *opaque); diff --git a/target/arm/gtimer.h b/target/arm/gtimer.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/gtimer.h +++ b/target/arm/gtimer.h @@ -XXX,XX +XXX,XX @@ enum { GTIMER_HYP = 2, GTIMER_SEC = 3, GTIMER_HYPVIRT = 4, -#define NUM_GTIMERS 5 + GTIMER_S_EL2_PHYS = 5, /* CNTHPS_* ; only if FEAT_SEL2 */ + GTIMER_S_EL2_VIRT = 6, /* CNTHVS_* ; only if FEAT_SEL2 */ +#define NUM_GTIMERS 7 }; #endif diff --git a/target/arm/cpu.c b/target/arm/cpu.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -XXX,XX +XXX,XX @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) arm_gt_stimer_cb, cpu); cpu->gt_timer[GTIMER_HYPVIRT] = timer_new(QEMU_CLOCK_VIRTUAL, scale, arm_gt_hvtimer_cb, cpu); + cpu->gt_timer[GTIMER_S_EL2_PHYS] = timer_new(QEMU_CLOCK_VIRTUAL, scale, + arm_gt_sel2timer_cb, cpu); + cpu->gt_timer[GTIMER_S_EL2_VIRT] = timer_new(QEMU_CLOCK_VIRTUAL, scale, + arm_gt_sel2vtimer_cb, cpu); } #endif 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 CPAccessResult gt_stimer_access(CPUARMState *env, } } +static CPAccessResult gt_sel2timer_access(CPUARMState *env, + const ARMCPRegInfo *ri, + bool isread) +{ + /* + * The AArch64 register view of the secure EL2 timers are mostly + * accessible from EL3 and EL2 although can also be trapped to EL2 + * from EL1 depending on nested virt config. + */ + switch (arm_current_el(env)) { + case 0: /* UNDEFINED */ + return CP_ACCESS_UNDEFINED; + case 1: + if (!arm_is_secure(env)) { + /* UNDEFINED */ + return CP_ACCESS_UNDEFINED; + } else if (arm_hcr_el2_eff(env) & HCR_NV) { + /* Aarch64.SystemAccessTrap(EL2, 0x18) */ + return CP_ACCESS_TRAP_EL2; + } + /* UNDEFINED */ + return CP_ACCESS_UNDEFINED; + case 2: + if (!arm_is_secure(env)) { + /* UNDEFINED */ + return CP_ACCESS_UNDEFINED; + } + return CP_ACCESS_OK; + case 3: + if (env->cp15.scr_el3 & SCR_EEL2) { + return CP_ACCESS_OK; + } else { + return CP_ACCESS_UNDEFINED; + } + default: + g_assert_not_reached(); + } +} + uint64_t gt_get_countervalue(CPUARMState *env) { ARMCPU *cpu = env_archcpu(env); @@ -XXX,XX +XXX,XX @@ static uint64_t gt_indirect_access_timer_offset(CPUARMState *env, int timeridx) case GTIMER_HYP: case GTIMER_SEC: case GTIMER_HYPVIRT: + case GTIMER_S_EL2_PHYS: + case GTIMER_S_EL2_VIRT: return 0; default: g_assert_not_reached(); @@ -XXX,XX +XXX,XX @@ uint64_t gt_direct_access_timer_offset(CPUARMState *env, int timeridx) case GTIMER_HYP: case GTIMER_SEC: case GTIMER_HYPVIRT: + case GTIMER_S_EL2_PHYS: + case GTIMER_S_EL2_VIRT: return 0; default: g_assert_not_reached(); @@ -XXX,XX +XXX,XX @@ static void gt_sec_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri, gt_ctl_write(env, ri, GTIMER_SEC, value); } +static void gt_sec_pel2_timer_reset(CPUARMState *env, const ARMCPRegInfo *ri) +{ + gt_timer_reset(env, ri, GTIMER_S_EL2_PHYS); +} + +static void gt_sec_pel2_cval_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ + gt_cval_write(env, ri, GTIMER_S_EL2_PHYS, value); +} + +static uint64_t gt_sec_pel2_tval_read(CPUARMState *env, const ARMCPRegInfo *ri) +{ + return gt_tval_read(env, ri, GTIMER_S_EL2_PHYS); +} + +static void gt_sec_pel2_tval_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ + gt_tval_write(env, ri, GTIMER_S_EL2_PHYS, value); +} + +static void gt_sec_pel2_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ + gt_ctl_write(env, ri, GTIMER_S_EL2_PHYS, value); +} + +static void gt_sec_vel2_timer_reset(CPUARMState *env, const ARMCPRegInfo *ri) +{ + gt_timer_reset(env, ri, GTIMER_S_EL2_VIRT); +} + +static void gt_sec_vel2_cval_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ + gt_cval_write(env, ri, GTIMER_S_EL2_VIRT, value); +} + +static uint64_t gt_sec_vel2_tval_read(CPUARMState *env, const ARMCPRegInfo *ri) +{ + return gt_tval_read(env, ri, GTIMER_S_EL2_VIRT); +} + +static void gt_sec_vel2_tval_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ + gt_tval_write(env, ri, GTIMER_S_EL2_VIRT, value); +} + +static void gt_sec_vel2_ctl_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ + gt_ctl_write(env, ri, GTIMER_S_EL2_VIRT, value); +} + static void gt_hv_timer_reset(CPUARMState *env, const ARMCPRegInfo *ri) { gt_timer_reset(env, ri, GTIMER_HYPVIRT); @@ -XXX,XX +XXX,XX @@ void arm_gt_stimer_cb(void *opaque) gt_recalc_timer(cpu, GTIMER_SEC); } +void arm_gt_sel2timer_cb(void *opaque) +{ + ARMCPU *cpu = opaque; + + gt_recalc_timer(cpu, GTIMER_S_EL2_PHYS); +} + +void arm_gt_sel2vtimer_cb(void *opaque) +{ + ARMCPU *cpu = opaque; + + gt_recalc_timer(cpu, GTIMER_S_EL2_VIRT); +} + void arm_gt_hvtimer_cb(void *opaque) { ARMCPU *cpu = opaque; @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo el2_sec_cp_reginfo[] = { .access = PL2_RW, .accessfn = sel2_access, .nv2_redirect_offset = 0x48, .fieldoffset = offsetof(CPUARMState, cp15.vstcr_el2) }, +#ifndef CONFIG_USER_ONLY + /* Secure EL2 Physical Timer */ + { .name = "CNTHPS_TVAL_EL2", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 5, .opc2 = 0, + .type = ARM_CP_NO_RAW | ARM_CP_IO, .access = PL2_RW, + .accessfn = gt_sel2timer_access, + .readfn = gt_sec_pel2_tval_read, + .writefn = gt_sec_pel2_tval_write, + .resetfn = gt_sec_pel2_timer_reset, + }, + { .name = "CNTHPS_CTL_EL2", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 5, .opc2 = 1, + .type = ARM_CP_IO, .access = PL2_RW, + .accessfn = gt_sel2timer_access, + .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_S_EL2_PHYS].ctl), + .resetvalue = 0, + .writefn = gt_sec_pel2_ctl_write, .raw_writefn = raw_write, + }, + { .name = "CNTHPS_CVAL_EL2", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 5, .opc2 = 2, + .type = ARM_CP_IO, .access = PL2_RW, + .accessfn = gt_sel2timer_access, + .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_S_EL2_PHYS].cval), + .writefn = gt_sec_pel2_cval_write, .raw_writefn = raw_write, + }, + /* Secure EL2 Virtual Timer */ + { .name = "CNTHVS_TVAL_EL2", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 4, .opc2 = 0, + .type = ARM_CP_NO_RAW | ARM_CP_IO, .access = PL2_RW, + .accessfn = gt_sel2timer_access, + .readfn = gt_sec_vel2_tval_read, + .writefn = gt_sec_vel2_tval_write, + .resetfn = gt_sec_vel2_timer_reset, + }, + { .name = "CNTHVS_CTL_EL2", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 4, .opc2 = 1, + .type = ARM_CP_IO, .access = PL2_RW, + .accessfn = gt_sel2timer_access, + .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_S_EL2_VIRT].ctl), + .resetvalue = 0, + .writefn = gt_sec_vel2_ctl_write, .raw_writefn = raw_write, + }, + { .name = "CNTHVS_CVAL_EL2", .state = ARM_CP_STATE_AA64, + .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 4, .opc2 = 2, + .type = ARM_CP_IO, .access = PL2_RW, + .accessfn = gt_sel2timer_access, + .fieldoffset = offsetof(CPUARMState, cp15.c14_timer[GTIMER_S_EL2_VIRT].cval), + .writefn = gt_sec_vel2_cval_write, .raw_writefn = raw_write, + }, +#endif }; static CPAccessResult nsacr_access(CPUARMState *env, const ARMCPRegInfo *ri, -- 2.43.0 From: Alex Bennée <alex.bennee@linaro.org> As we are about to add more physical and virtual timers let's make it clear what each timer does. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20250204125009.2281315-8-peter.maydell@linaro.org [PMM: Add timer register name prefix to each comment] Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/gtimer.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/target/arm/gtimer.h b/target/arm/gtimer.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/gtimer.h +++ b/target/arm/gtimer.h @@ -XXX,XX +XXX,XX @@ #define TARGET_ARM_GTIMER_H enum { - GTIMER_PHYS = 0, - GTIMER_VIRT = 1, - GTIMER_HYP = 2, - GTIMER_SEC = 3, - GTIMER_HYPVIRT = 4, + GTIMER_PHYS = 0, /* CNTP_* ; EL1 physical timer */ + GTIMER_VIRT = 1, /* CNTV_* ; EL1 virtual timer */ + GTIMER_HYP = 2, /* CNTHP_* ; EL2 physical timer */ + GTIMER_SEC = 3, /* CNTPS_* ; EL3 physical timer */ + GTIMER_HYPVIRT = 4, /* CNTHV_* ; EL2 virtual timer ; only if FEAT_VHE */ GTIMER_S_EL2_PHYS = 5, /* CNTHPS_* ; only if FEAT_SEL2 */ GTIMER_S_EL2_VIRT = 6, /* CNTHVS_* ; only if FEAT_SEL2 */ #define NUM_GTIMERS 7 -- 2.43.0 From: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20250204125009.2281315-9-peter.maydell@linaro.org Cc: qemu-stable@nongnu.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -XXX,XX +XXX,XX @@ static void create_gic(VirtMachineState *vms, MemoryRegion *mem) [GTIMER_HYP] = ARCH_TIMER_NS_EL2_IRQ, [GTIMER_SEC] = ARCH_TIMER_S_EL1_IRQ, [GTIMER_HYPVIRT] = ARCH_TIMER_NS_EL2_VIRT_IRQ, + [GTIMER_S_EL2_PHYS] = ARCH_TIMER_S_EL2_IRQ, + [GTIMER_S_EL2_VIRT] = ARCH_TIMER_S_EL2_VIRT_IRQ, }; for (unsigned irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) { -- 2.43.0 From: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20250204125009.2281315-10-peter.maydell@linaro.org Cc: qemu-stable@nongnu.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/sbsa-ref.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/sbsa-ref.c +++ b/hw/arm/sbsa-ref.c @@ -XXX,XX +XXX,XX @@ static void create_gic(SBSAMachineState *sms, MemoryRegion *mem) [GTIMER_HYP] = ARCH_TIMER_NS_EL2_IRQ, [GTIMER_SEC] = ARCH_TIMER_S_EL1_IRQ, [GTIMER_HYPVIRT] = ARCH_TIMER_NS_EL2_VIRT_IRQ, + [GTIMER_S_EL2_PHYS] = ARCH_TIMER_S_EL2_IRQ, + [GTIMER_S_EL2_VIRT] = ARCH_TIMER_S_EL2_VIRT_IRQ, }; for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) { -- 2.43.0 Our LDRD implementation is wrong in two respects: * if the address is 4-aligned and the load crosses a page boundary and the second load faults and the first load was to the base register (as in cases like "ldrd r2, r3, [r2]", then we must not update the base register before taking the fault * if the address is 8-aligned the access must be a 64-bit single-copy atomic access, not two 32-bit accesses Rewrite the handling of the loads in LDRD to use a single tcg_gen_qemu_ld_i64() and split the result into the destination registers. This allows us to get the atomicity requirements right, and also implicitly means that we won't update the base register too early for the page-crossing case. Note that because we no longer increment 'addr' by 4 in the course of performing the LDRD we must change the adjustment value we pass to op_addr_ri_post() and op_addr_rr_post(): it no longer needs to subtract 4 to get the correct value to use if doing base register writeback. STRD has the same problem with not getting the atomicity right; we will deal with that in the following commit. Cc: qemu-stable@nongnu.org Reported-by: Stu Grossman <stu.grossman@gmail.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20250227142746.1698904-2-peter.maydell@linaro.org --- target/arm/tcg/translate.c | 70 +++++++++++++++++++++++++------------- 1 file changed, 46 insertions(+), 24 deletions(-) diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/tcg/translate.c +++ b/target/arm/tcg/translate.c @@ -XXX,XX +XXX,XX @@ static bool op_store_rr(DisasContext *s, arg_ldst_rr *a, return true; } +static void do_ldrd_load(DisasContext *s, TCGv_i32 addr, int rt, int rt2) +{ + /* + * LDRD is required to be an atomic 64-bit access if the + * address is 8-aligned, two atomic 32-bit accesses if + * it's only 4-aligned, and to give an alignment fault + * if it's not 4-aligned. This is MO_ALIGN_4 | MO_ATOM_SUBALIGN. + * Rt is always the word from the lower address, and Rt2 the + * data from the higher address, regardless of endianness. + * So (like gen_load_exclusive) we avoid gen_aa32_ld_i64() + * so we don't get its SCTLR_B check, and instead do a 64-bit access + * using MO_BE if appropriate and then split the two halves. + * + * For M-profile, and for A-profile before LPAE, the 64-bit + * atomicity is not required. We could model that using + * the looser MO_ATOM_IFALIGN_PAIR, but providing a higher + * level of atomicity than required is harmless (we would not + * currently generate better code for IFALIGN_PAIR here). + * + * This also gives us the correct behaviour of not updating + * rt if the load of rt2 faults; this is required for cases + * like "ldrd r2, r3, [r2]" where rt is also the base register. + */ + int mem_idx = get_mem_index(s); + MemOp opc = MO_64 | MO_ALIGN_4 | MO_ATOM_SUBALIGN | s->be_data; + TCGv taddr = gen_aa32_addr(s, addr, opc); + TCGv_i64 t64 = tcg_temp_new_i64(); + TCGv_i32 tmp = tcg_temp_new_i32(); + TCGv_i32 tmp2 = tcg_temp_new_i32(); + + tcg_gen_qemu_ld_i64(t64, taddr, mem_idx, opc); + if (s->be_data == MO_BE) { + tcg_gen_extr_i64_i32(tmp2, tmp, t64); + } else { + tcg_gen_extr_i64_i32(tmp, tmp2, t64); + } + store_reg(s, rt, tmp); + store_reg(s, rt2, tmp2); +} + static bool trans_LDRD_rr(DisasContext *s, arg_ldst_rr *a) { - int mem_idx = get_mem_index(s); - TCGv_i32 addr, tmp; + TCGv_i32 addr; if (!ENABLE_ARCH_5TE) { return false; @@ -XXX,XX +XXX,XX @@ static bool trans_LDRD_rr(DisasContext *s, arg_ldst_rr *a) } addr = op_addr_rr_pre(s, a); - tmp = tcg_temp_new_i32(); - gen_aa32_ld_i32(s, tmp, addr, mem_idx, MO_UL | MO_ALIGN); - store_reg(s, a->rt, tmp); - - tcg_gen_addi_i32(addr, addr, 4); - - tmp = tcg_temp_new_i32(); - gen_aa32_ld_i32(s, tmp, addr, mem_idx, MO_UL | MO_ALIGN); - store_reg(s, a->rt + 1, tmp); + do_ldrd_load(s, addr, a->rt, a->rt + 1); /* LDRD w/ base writeback is undefined if the registers overlap. */ - op_addr_rr_post(s, a, addr, -4); + op_addr_rr_post(s, a, addr, 0); return true; } @@ -XXX,XX +XXX,XX @@ static bool op_store_ri(DisasContext *s, arg_ldst_ri *a, static bool op_ldrd_ri(DisasContext *s, arg_ldst_ri *a, int rt2) { - int mem_idx = get_mem_index(s); - TCGv_i32 addr, tmp; + TCGv_i32 addr; addr = op_addr_ri_pre(s, a); - tmp = tcg_temp_new_i32(); - gen_aa32_ld_i32(s, tmp, addr, mem_idx, MO_UL | MO_ALIGN); - store_reg(s, a->rt, tmp); - - tcg_gen_addi_i32(addr, addr, 4); - - tmp = tcg_temp_new_i32(); - gen_aa32_ld_i32(s, tmp, addr, mem_idx, MO_UL | MO_ALIGN); - store_reg(s, rt2, tmp); + do_ldrd_load(s, addr, a->rt, rt2); /* LDRD w/ base writeback is undefined if the registers overlap. */ - op_addr_ri_post(s, a, addr, -4); + op_addr_ri_post(s, a, addr, 0); return true; } -- 2.43.0 Our STRD implementation doesn't correctly implement the requirement: * if the address is 8-aligned the access must be a 64-bit single-copy atomic access, not two 32-bit accesses Rewrite the handling of STRD to use a single tcg_gen_qemu_st_i64() of a value produced by concatenating the two 32 bit source registers. This allows us to get the atomicity right. As with the LDRD change, now that we don't update 'addr' in the course of performing the store we need to adjust the offset we pass to op_addr_ri_post() and op_addr_rr_post(). Cc: qemu-stable@nongnu.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20250227142746.1698904-3-peter.maydell@linaro.org --- target/arm/tcg/translate.c | 59 +++++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 20 deletions(-) diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/tcg/translate.c +++ b/target/arm/tcg/translate.c @@ -XXX,XX +XXX,XX @@ static bool trans_LDRD_rr(DisasContext *s, arg_ldst_rr *a) return true; } +static void do_strd_store(DisasContext *s, TCGv_i32 addr, int rt, int rt2) +{ + /* + * STRD is required to be an atomic 64-bit access if the + * address is 8-aligned, two atomic 32-bit accesses if + * it's only 4-aligned, and to give an alignment fault + * if it's not 4-aligned. + * Rt is always the word from the lower address, and Rt2 the + * data from the higher address, regardless of endianness. + * So (like gen_store_exclusive) we avoid gen_aa32_ld_i64() + * so we don't get its SCTLR_B check, and instead do a 64-bit access + * using MO_BE if appropriate, using a value constructed + * by putting the two halves together in the right order. + * + * As with LDRD, the 64-bit atomicity is not required for + * M-profile, or for A-profile before LPAE, and we provide + * the higher guarantee always for simplicity. + */ + int mem_idx = get_mem_index(s); + MemOp opc = MO_64 | MO_ALIGN_4 | MO_ATOM_SUBALIGN | s->be_data; + TCGv taddr = gen_aa32_addr(s, addr, opc); + TCGv_i32 t1 = load_reg(s, rt); + TCGv_i32 t2 = load_reg(s, rt2); + TCGv_i64 t64 = tcg_temp_new_i64(); + + if (s->be_data == MO_BE) { + tcg_gen_concat_i32_i64(t64, t2, t1); + } else { + tcg_gen_concat_i32_i64(t64, t1, t2); + } + tcg_gen_qemu_st_i64(t64, taddr, mem_idx, opc); +} + static bool trans_STRD_rr(DisasContext *s, arg_ldst_rr *a) { - int mem_idx = get_mem_index(s); - TCGv_i32 addr, tmp; + TCGv_i32 addr; if (!ENABLE_ARCH_5TE) { return false; @@ -XXX,XX +XXX,XX @@ static bool trans_STRD_rr(DisasContext *s, arg_ldst_rr *a) } addr = op_addr_rr_pre(s, a); - tmp = load_reg(s, a->rt); - gen_aa32_st_i32(s, tmp, addr, mem_idx, MO_UL | MO_ALIGN); + do_strd_store(s, addr, a->rt, a->rt + 1); - tcg_gen_addi_i32(addr, addr, 4); - - tmp = load_reg(s, a->rt + 1); - gen_aa32_st_i32(s, tmp, addr, mem_idx, MO_UL | MO_ALIGN); - - op_addr_rr_post(s, a, addr, -4); + op_addr_rr_post(s, a, addr, 0); return true; } @@ -XXX,XX +XXX,XX @@ static bool trans_LDRD_ri_t32(DisasContext *s, arg_ldst_ri2 *a) static bool op_strd_ri(DisasContext *s, arg_ldst_ri *a, int rt2) { - int mem_idx = get_mem_index(s); - TCGv_i32 addr, tmp; + TCGv_i32 addr; addr = op_addr_ri_pre(s, a); - tmp = load_reg(s, a->rt); - gen_aa32_st_i32(s, tmp, addr, mem_idx, MO_UL | MO_ALIGN); + do_strd_store(s, addr, a->rt, rt2); - tcg_gen_addi_i32(addr, addr, 4); - - tmp = load_reg(s, rt2); - gen_aa32_st_i32(s, tmp, addr, mem_idx, MO_UL | MO_ALIGN); - - op_addr_ri_post(s, a, addr, -4); + op_addr_ri_post(s, a, addr, 0); return true; } -- 2.43.0 All the callers of op_addr_rr_post() and op_addr_ri_post() now pass in zero for the address_offset, so we can remove that argument. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-id: 20250227142746.1698904-4-peter.maydell@linaro.org --- target/arm/tcg/translate.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/tcg/translate.c +++ b/target/arm/tcg/translate.c @@ -XXX,XX +XXX,XX @@ static TCGv_i32 op_addr_rr_pre(DisasContext *s, arg_ldst_rr *a) } static void op_addr_rr_post(DisasContext *s, arg_ldst_rr *a, - TCGv_i32 addr, int address_offset) + TCGv_i32 addr) { if (!a->p) { TCGv_i32 ofs = load_reg(s, a->rm); @@ -XXX,XX +XXX,XX @@ static void op_addr_rr_post(DisasContext *s, arg_ldst_rr *a, } else if (!a->w) { return; } - tcg_gen_addi_i32(addr, addr, address_offset); store_reg(s, a->rn, addr); } @@ -XXX,XX +XXX,XX @@ static bool op_load_rr(DisasContext *s, arg_ldst_rr *a, * Perform base writeback before the loaded value to * ensure correct behavior with overlapping index registers. */ - op_addr_rr_post(s, a, addr, 0); + op_addr_rr_post(s, a, addr); store_reg_from_load(s, a->rt, tmp); return true; } @@ -XXX,XX +XXX,XX @@ static bool op_store_rr(DisasContext *s, arg_ldst_rr *a, gen_aa32_st_i32(s, tmp, addr, mem_idx, mop); disas_set_da_iss(s, mop, issinfo); - op_addr_rr_post(s, a, addr, 0); + op_addr_rr_post(s, a, addr); return true; } @@ -XXX,XX +XXX,XX @@ static bool trans_LDRD_rr(DisasContext *s, arg_ldst_rr *a) do_ldrd_load(s, addr, a->rt, a->rt + 1); /* LDRD w/ base writeback is undefined if the registers overlap. */ - op_addr_rr_post(s, a, addr, 0); + op_addr_rr_post(s, a, addr); return true; } @@ -XXX,XX +XXX,XX @@ static bool trans_STRD_rr(DisasContext *s, arg_ldst_rr *a) do_strd_store(s, addr, a->rt, a->rt + 1); - op_addr_rr_post(s, a, addr, 0); + op_addr_rr_post(s, a, addr); return true; } @@ -XXX,XX +XXX,XX @@ static TCGv_i32 op_addr_ri_pre(DisasContext *s, arg_ldst_ri *a) } static void op_addr_ri_post(DisasContext *s, arg_ldst_ri *a, - TCGv_i32 addr, int address_offset) + TCGv_i32 addr) { + int address_offset = 0; if (!a->p) { if (a->u) { - address_offset += a->imm; + address_offset = a->imm; } else { - address_offset -= a->imm; + address_offset = -a->imm; } } else if (!a->w) { return; @@ -XXX,XX +XXX,XX @@ static bool op_load_ri(DisasContext *s, arg_ldst_ri *a, * Perform base writeback before the loaded value to * ensure correct behavior with overlapping index registers. */ - op_addr_ri_post(s, a, addr, 0); + op_addr_ri_post(s, a, addr); store_reg_from_load(s, a->rt, tmp); return true; } @@ -XXX,XX +XXX,XX @@ static bool op_store_ri(DisasContext *s, arg_ldst_ri *a, gen_aa32_st_i32(s, tmp, addr, mem_idx, mop); disas_set_da_iss(s, mop, issinfo); - op_addr_ri_post(s, a, addr, 0); + op_addr_ri_post(s, a, addr); return true; } @@ -XXX,XX +XXX,XX @@ static bool op_ldrd_ri(DisasContext *s, arg_ldst_ri *a, int rt2) do_ldrd_load(s, addr, a->rt, rt2); /* LDRD w/ base writeback is undefined if the registers overlap. */ - op_addr_ri_post(s, a, addr, 0); + op_addr_ri_post(s, a, addr); return true; } @@ -XXX,XX +XXX,XX @@ static bool op_strd_ri(DisasContext *s, arg_ldst_ri *a, int rt2) do_strd_store(s, addr, a->rt, rt2); - op_addr_ri_post(s, a, addr, 0); + op_addr_ri_post(s, a, addr); return true; } -- 2.43.0 In debug_helper.c we provide a few dummy versions of debug registers: * DBGVCR (AArch32 only): enable bits for vector-catch debug events * MDCCINT_EL1: interrupt enable bits for the DCC debug communications channel * DBGVCR32_EL2: the AArch64 accessor for the state in DBGVCR We implemented these only to stop Linux crashing on startup, but we chose to implement them as ARM_CP_NOP. This worked for Linux where it only cares about trying to write to these registers, but is very confusing behaviour for anything that wants to read the registers (perhaps for context state switches), because the destination register will be left with whatever random value it happened to have before the read. Model these registers instead as RAZ. Fixes: 5e8b12ffbb8c68 ("target-arm: Implement minimal DBGVCR, OSDLR_EL1, MDCCSR_EL0") Fixes: 5dbdc4342f479d ("target-arm: Implement dummy MDCCINT_EL1") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2708 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20250228162424.1917269-1-peter.maydell@linaro.org --- target/arm/debug_helper.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 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 @@ static const ARMCPRegInfo debug_cp_reginfo[] = { { .name = "DBGVCR", .cp = 14, .opc1 = 0, .crn = 0, .crm = 7, .opc2 = 0, .access = PL1_RW, .accessfn = access_tda, - .type = ARM_CP_NOP }, + .type = ARM_CP_CONST, .resetvalue = 0 }, /* * Dummy MDCCINT_EL1, since we don't implement the Debug Communications * Channel but Linux may try to access this register. The 32-bit @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo debug_cp_reginfo[] = { { .name = "MDCCINT_EL1", .state = ARM_CP_STATE_BOTH, .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 2, .opc2 = 0, .access = PL1_RW, .accessfn = access_tdcc, - .type = ARM_CP_NOP }, + .type = ARM_CP_CONST, .resetvalue = 0 }, /* * Dummy DBGCLAIM registers. * "The architecture does not define any functionality for the CLAIM tag bits.", @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo debug_aa32_el1_reginfo[] = { { .name = "DBGVCR32_EL2", .state = ARM_CP_STATE_AA64, .opc0 = 2, .opc1 = 4, .crn = 0, .crm = 7, .opc2 = 0, .access = PL2_RW, .accessfn = access_dbgvcr32, - .type = ARM_CP_NOP | ARM_CP_EL3_NO_EL2_KEEP }, + .type = ARM_CP_CONST | ARM_CP_EL3_NO_EL2_KEEP, + .resetvalue = 0 }, }; static const ARMCPRegInfo debug_lpae_cp_reginfo[] = { -- 2.43.0 Currently we call icount_start_warp_timer() from timerlist_rearm(). This produces incorrect behaviour, because timerlist_rearm() is called, for instance, when a timer callback modifies its timer. We cannot decide here to warp the timer forwards to the next timer deadline merely because all_cpu_threads_idle() is true, because the timer callback we were called from (or some other callback later in the list of callbacks being invoked) may be about to raise a CPU interrupt and move a CPU from idle to ready. The only valid place to choose to warp the timer forward is from the main loop, when we know we have no outstanding IO or timer callbacks that might be about to wake up a CPU. For Arm guests, this bug was mostly latent until the refactoring commit f6fc36deef6abc ("target/arm/helper: Implement CNTHCTL_EL2.CNT[VP]MASK"), which exposed it because it refactored a timer callback so that it happened to call timer_mod() first and raise the interrupt second, when it had previously raised the interrupt first and called timer_mod() afterwards. This call seems to have originally derived from the pre-record-and-replay icount code, which (as of e.g. commit db1a49726c3c in 2010) in this location did a call to qemu_notify_event(), necessary to get the icount code in the vCPU round-robin thread to stop and recalculate the icount deadline when a timer was reprogrammed from the IO thread. In current QEMU, everything is done on the vCPU thread when we are in icount mode, so there's no need to try to notify another thread here. I suspect that the other reason why this call was doing icount timer warping is that it pre-dates commit efab87cf79077a from 2015, which added a call to icount_start_warp_timer() to main_loop_wait(). Once the call in timerlist_rearm() has been removed, if the timer callbacks don't cause any CPU to be woken up then we will end up calling icount_start_warp_timer() from main_loop_wait() when the rr main loop code calls rr_wait_io_event(). Remove the incorrect call from timerlist_rearm(). Cc: qemu-stable@nongnu.org Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2703 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20250210135804.3526943-1-peter.maydell@linaro.org --- util/qemu-timer.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/util/qemu-timer.c b/util/qemu-timer.c index XXXXXXX..XXXXXXX 100644 --- a/util/qemu-timer.c +++ b/util/qemu-timer.c @@ -XXX,XX +XXX,XX @@ static bool timer_mod_ns_locked(QEMUTimerList *timer_list, static void timerlist_rearm(QEMUTimerList *timer_list) { - /* Interrupt execution to force deadline recalculation. */ - if (icount_enabled() && timer_list->clock->type == QEMU_CLOCK_VIRTUAL) { - icount_start_warp_timer(); - } timerlist_notify(timer_list); } -- 2.43.0 Expand the example in the comment documenting MO_ATOM_SUBALIGN, to be clearer about the atomicity guarantees it represents. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20250228103222.1838913-1-peter.maydell@linaro.org --- include/exec/memop.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/include/exec/memop.h b/include/exec/memop.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/memop.h +++ b/include/exec/memop.h @@ -XXX,XX +XXX,XX @@ typedef enum MemOp { * Depending on alignment, one or both will be single-copy atomic. * This is the atomicity e.g. of Arm FEAT_LSE2 LDP. * MO_ATOM_SUBALIGN: the operation is single-copy atomic by parts - * by the alignment. E.g. if the address is 0 mod 4, then each - * 4-byte subobject is single-copy atomic. + * by the alignment. E.g. if an 8-byte value is accessed at an + * address which is 0 mod 8, then the whole 8-byte access is + * single-copy atomic; otherwise, if it is accessed at 0 mod 4 + * then each 4-byte subobject is single-copy atomic; otherwise + * if it is accessed at 0 mod 2 then the four 2-byte subobjects + * are single-copy atomic. * This is the atomicity e.g. of IBM Power. * MO_ATOM_NONE: the operation has no atomicity requirements. * -- 2.43.0 From: JianChunfu <jansef.jian@hj-micro.com> Use a similar terminology smmu_hash_remove_by_sid_range() as the one being used for other hash table matching functions since smmuv3_invalidate_ste() name is not self explanatory, and introduce a helper that invokes the g_hash_table_foreach_remove. No functional change intended. Signed-off-by: JianChunfu <jansef.jian@hj-micro.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Message-id: 20250228031438.3916-1-jansef.jian@hj-micro.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/smmu-internal.h | 5 ----- include/hw/arm/smmu-common.h | 6 ++++++ hw/arm/smmu-common.c | 21 +++++++++++++++++++++ hw/arm/smmuv3.c | 19 ++----------------- hw/arm/trace-events | 3 ++- 5 files changed, 31 insertions(+), 23 deletions(-) diff --git a/hw/arm/smmu-internal.h b/hw/arm/smmu-internal.h index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/smmu-internal.h +++ b/hw/arm/smmu-internal.h @@ -XXX,XX +XXX,XX @@ typedef struct SMMUIOTLBPageInvInfo { uint64_t mask; } SMMUIOTLBPageInvInfo; -typedef struct SMMUSIDRange { - uint32_t start; - uint32_t end; -} SMMUSIDRange; - #endif diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/smmu-common.h +++ b/include/hw/arm/smmu-common.h @@ -XXX,XX +XXX,XX @@ typedef struct SMMUIOTLBKey { uint8_t level; } SMMUIOTLBKey; +typedef struct SMMUSIDRange { + uint32_t start; + uint32_t end; +} SMMUSIDRange; + struct SMMUState { /* <private> */ SysBusDevice dev; @@ -XXX,XX +XXX,XX @@ void smmu_iotlb_inv_iova(SMMUState *s, int asid, int vmid, dma_addr_t iova, uint8_t tg, uint64_t num_pages, uint8_t ttl); void smmu_iotlb_inv_ipa(SMMUState *s, int vmid, dma_addr_t ipa, uint8_t tg, uint64_t num_pages, uint8_t ttl); +void smmu_configs_inv_sid_range(SMMUState *s, SMMUSIDRange sid_range); /* Unmap the range of all the notifiers registered to any IOMMU mr */ void smmu_inv_notifiers_all(SMMUState *s); diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/smmu-common.c +++ b/hw/arm/smmu-common.c @@ -XXX,XX +XXX,XX @@ static gboolean smmu_hash_remove_by_vmid_ipa(gpointer key, gpointer value, ((entry->iova & ~info->mask) == info->iova); } +static gboolean +smmu_hash_remove_by_sid_range(gpointer key, gpointer value, gpointer user_data) +{ + SMMUDevice *sdev = (SMMUDevice *)key; + uint32_t sid = smmu_get_sid(sdev); + SMMUSIDRange *sid_range = (SMMUSIDRange *)user_data; + + if (sid < sid_range->start || sid > sid_range->end) { + return false; + } + trace_smmu_config_cache_inv(sid); + return true; +} + +void smmu_configs_inv_sid_range(SMMUState *s, SMMUSIDRange sid_range) +{ + trace_smmu_configs_inv_sid_range(sid_range.start, sid_range.end); + g_hash_table_foreach_remove(s->configs, smmu_hash_remove_by_sid_range, + &sid_range); +} + void smmu_iotlb_inv_iova(SMMUState *s, int asid, int vmid, dma_addr_t iova, uint8_t tg, uint64_t num_pages, uint8_t ttl) { diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -XXX,XX +XXX,XX @@ static void smmuv3_flush_config(SMMUDevice *sdev) SMMUv3State *s = sdev->smmu; SMMUState *bc = &s->smmu_state; - trace_smmuv3_config_cache_inv(smmu_get_sid(sdev)); + trace_smmu_config_cache_inv(smmu_get_sid(sdev)); g_hash_table_remove(bc->configs, sdev); } @@ -XXX,XX +XXX,XX @@ static void smmuv3_range_inval(SMMUState *s, Cmd *cmd, SMMUStage stage) } } -static gboolean -smmuv3_invalidate_ste(gpointer key, gpointer value, gpointer user_data) -{ - SMMUDevice *sdev = (SMMUDevice *)key; - uint32_t sid = smmu_get_sid(sdev); - SMMUSIDRange *sid_range = (SMMUSIDRange *)user_data; - - if (sid < sid_range->start || sid > sid_range->end) { - return false; - } - trace_smmuv3_config_cache_inv(sid); - return true; -} - static int smmuv3_cmdq_consume(SMMUv3State *s) { SMMUState *bs = ARM_SMMU(s); @@ -XXX,XX +XXX,XX @@ static int smmuv3_cmdq_consume(SMMUv3State *s) sid_range.end = sid_range.start + mask; trace_smmuv3_cmdq_cfgi_ste_range(sid_range.start, sid_range.end); - g_hash_table_foreach_remove(bs->configs, smmuv3_invalidate_ste, - &sid_range); + smmu_configs_inv_sid_range(bs, sid_range); break; } case SMMU_CMD_CFGI_CD: diff --git a/hw/arm/trace-events b/hw/arm/trace-events index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/trace-events +++ b/hw/arm/trace-events @@ -XXX,XX +XXX,XX @@ smmu_iotlb_inv_asid_vmid(int asid, int vmid) "IOTLB invalidate asid=%d vmid=%d" smmu_iotlb_inv_vmid(int vmid) "IOTLB invalidate vmid=%d" smmu_iotlb_inv_vmid_s1(int vmid) "IOTLB invalidate vmid=%d" smmu_iotlb_inv_iova(int asid, uint64_t addr) "IOTLB invalidate asid=%d addr=0x%"PRIx64 +smmu_configs_inv_sid_range(uint32_t start, uint32_t end) "Config cache INV SID range from 0x%x to 0x%x" +smmu_config_cache_inv(uint32_t sid) "Config cache INV for sid=0x%x" smmu_inv_notifiers_mr(const char *name) "iommu mr=%s" smmu_iotlb_lookup_hit(int asid, int vmid, uint64_t addr, uint32_t hit, uint32_t miss, uint32_t p) "IOTLB cache HIT asid=%d vmid=%d addr=0x%"PRIx64" hit=%d miss=%d hit rate=%d" smmu_iotlb_lookup_miss(int asid, int vmid, uint64_t addr, uint32_t hit, uint32_t miss, uint32_t p) "IOTLB cache MISS asid=%d vmid=%d addr=0x%"PRIx64" hit=%d miss=%d hit rate=%d" @@ -XXX,XX +XXX,XX @@ smmuv3_cmdq_tlbi_nh(int vmid) "vmid=%d" smmuv3_cmdq_tlbi_nsnh(void) "" smmuv3_cmdq_tlbi_nh_asid(int asid) "asid=%d" smmuv3_cmdq_tlbi_s12_vmid(int vmid) "vmid=%d" -smmuv3_config_cache_inv(uint32_t sid) "Config cache INV for sid=0x%x" smmuv3_notify_flag_add(const char *iommu) "ADD SMMUNotifier node for iommu mr=%s" smmuv3_notify_flag_del(const char *iommu) "DEL SMMUNotifier node for iommu mr=%s" smmuv3_inv_notifiers_iova(const char *name, int asid, int vmid, uint64_t iova, uint8_t tg, uint64_t num_pages, int stage) "iommu mr=%s asid=%d vmid=%d iova=0x%"PRIx64" tg=%d num_pages=0x%"PRIx64" stage=%d" -- 2.43.0 From: Keith Packard <keithp@keithp.com> The documentation says the vector is at 0xffffff80, instead of the previous value of 0xffffffc0. That value must have been a bug because the standard vector values (20, 21, 23, 25, 30) were all past the end of the array. Signed-off-by: Keith Packard <keithp@keithp.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/rx/helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/rx/helper.c b/target/rx/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/rx/helper.c +++ b/target/rx/helper.c @@ -XXX,XX +XXX,XX @@ void rx_cpu_do_interrupt(CPUState *cs) cpu_stl_data(env, env->isp, env->pc); if (vec < 0x100) { - env->pc = cpu_ldl_data(env, 0xffffffc0 + vec * 4); + env->pc = cpu_ldl_data(env, 0xffffff80 + vec * 4); } else { env->pc = cpu_ldl_data(env, env->intb + (vec & 0xff) * 4); } -- 2.43.0 From: Keith Packard <keithp@keithp.com> Functions which modify TCG globals must not be marked TCG_CALL_NO_WG, as that tells the optimizer that TCG global values already loaded in machine registers are still valid, and so any changes which these helpers make to the CPU state may be ignored. The target/rx code chooses to put (among other things) all the PSW bits and also ACC into globals, so the NO_WG flag on various functions that touch the PSW or ACC is incorrect and must be removed. This includes all the floating point helper functions, because update_fpsw() will update PSW Z and S. Signed-off-by: Keith Packard <keithp@keithp.com> [PMM: Clarified commit message] Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/rx/helper.h | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/target/rx/helper.h b/target/rx/helper.h index XXXXXXX..XXXXXXX 100644 --- a/target/rx/helper.h +++ b/target/rx/helper.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_1(raise_privilege_violation, noreturn, env) DEF_HELPER_1(wait, noreturn, env) DEF_HELPER_2(rxint, noreturn, env, i32) DEF_HELPER_1(rxbrk, noreturn, env) -DEF_HELPER_FLAGS_3(fadd, TCG_CALL_NO_WG, f32, env, f32, f32) -DEF_HELPER_FLAGS_3(fsub, TCG_CALL_NO_WG, f32, env, f32, f32) -DEF_HELPER_FLAGS_3(fmul, TCG_CALL_NO_WG, f32, env, f32, f32) -DEF_HELPER_FLAGS_3(fdiv, TCG_CALL_NO_WG, f32, env, f32, f32) -DEF_HELPER_FLAGS_3(fcmp, TCG_CALL_NO_WG, void, env, f32, f32) -DEF_HELPER_FLAGS_2(ftoi, TCG_CALL_NO_WG, i32, env, f32) -DEF_HELPER_FLAGS_2(round, TCG_CALL_NO_WG, i32, env, f32) -DEF_HELPER_FLAGS_2(itof, TCG_CALL_NO_WG, f32, env, i32) +DEF_HELPER_3(fadd, f32, env, f32, f32) +DEF_HELPER_3(fsub, f32, env, f32, f32) +DEF_HELPER_3(fmul, f32, env, f32, f32) +DEF_HELPER_3(fdiv, f32, env, f32, f32) +DEF_HELPER_3(fcmp, void, env, f32, f32) +DEF_HELPER_2(ftoi, i32, env, f32) +DEF_HELPER_2(round, i32, env, f32) +DEF_HELPER_2(itof, f32, env, i32) DEF_HELPER_2(set_fpsw, void, env, i32) -DEF_HELPER_FLAGS_2(racw, TCG_CALL_NO_WG, void, env, i32) -DEF_HELPER_FLAGS_2(set_psw_rte, TCG_CALL_NO_WG, void, env, i32) -DEF_HELPER_FLAGS_2(set_psw, TCG_CALL_NO_WG, void, env, i32) +DEF_HELPER_2(racw, void, env, i32) +DEF_HELPER_2(set_psw_rte, void, env, i32) +DEF_HELPER_2(set_psw, void, env, i32) DEF_HELPER_1(pack_psw, i32, env) -DEF_HELPER_FLAGS_3(div, TCG_CALL_NO_WG, i32, env, i32, i32) -DEF_HELPER_FLAGS_3(divu, TCG_CALL_NO_WG, i32, env, i32, i32) -DEF_HELPER_FLAGS_1(scmpu, TCG_CALL_NO_WG, void, env) +DEF_HELPER_3(div, i32, env, i32, i32) +DEF_HELPER_3(divu, i32, env, i32, i32) +DEF_HELPER_1(scmpu, void, env) DEF_HELPER_1(smovu, void, env) DEF_HELPER_1(smovf, void, env) DEF_HELPER_1(smovb, void, env) DEF_HELPER_2(sstr, void, env, i32) -DEF_HELPER_FLAGS_2(swhile, TCG_CALL_NO_WG, void, env, i32) -DEF_HELPER_FLAGS_2(suntil, TCG_CALL_NO_WG, void, env, i32) -DEF_HELPER_FLAGS_2(rmpa, TCG_CALL_NO_WG, void, env, i32) +DEF_HELPER_2(swhile, void, env, i32) +DEF_HELPER_2(suntil, void, env, i32) +DEF_HELPER_2(rmpa, void, env, i32) DEF_HELPER_1(satr, void, env) -- 2.43.0