The following changes since commit 4c41341af76cfc85b5a6c0f87de4838672ab9f89: Merge remote-tracking branch 'remotes/aperard/tags/pull-xen-20201020' into staging (2020-10-20 11:20:36 +0100) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20201020 for you to fetch changes up to 6358890cb939192f6169fdf7664d903bf9b1d338: tests/tcg/aarch64: Add bti smoke tests (2020-10-20 16:12:02 +0100) ---------------------------------------------------------------- target-arm queue: * Fix AArch32 SMLAD incorrect setting of Q bit * AArch32 VCVT fixed-point to float is always round-to-nearest * strongarm: Fix 'time to transmit a char' unit comment * Restrict APEI tables generation to the 'virt' machine * bcm2835: minor code cleanups * correctly flush TLBs when TBI is enabled * tests/qtest: Add npcm7xx timer test * loads-stores.rst: add footnote that clarifies GETPC usage * Fix reported EL for mte_check_fail * Ignore HCR_EL2.ATA when {E2H,TGE} != 11 * microbit_i2c: Fix coredump when dump-vmstate * nseries: Fix loading kernel image on n8x0 machines * Implement v8.1M low-overhead-loops * linux-user: Support AArch64 BTI ---------------------------------------------------------------- Emanuele Giuseppe Esposito (1): loads-stores.rst: add footnote that clarifies GETPC usage Havard Skinnemoen (1): tests/qtest: Add npcm7xx timer test Peng Liang (1): microbit_i2c: Fix coredump when dump-vmstate Peter Maydell (12): target/arm: Fix SMLAD incorrect setting of Q bit target/arm: AArch32 VCVT fixed-point to float is always round-to-nearest decodetree: Fix codegen for non-overlapping group inside overlapping group target/arm: Implement v8.1M NOCP handling target/arm: Implement v8.1M conditional-select insns target/arm: Make the t32 insn[25:23]=111 group non-overlapping target/arm: Don't allow BLX imm for M-profile target/arm: Implement v8.1M branch-future insns (as NOPs) target/arm: Implement v8.1M low-overhead-loop instructions target/arm: Fix has_vfp/has_neon ID reg squashing for M-profile target/arm: Allow M-profile CPUs with FP16 to set FPSCR.FP16 target/arm: Implement FPSCR.LTPSIZE for M-profile LOB extension Philippe Mathieu-Daudé (10): hw/arm/strongarm: Fix 'time to transmit a char' unit comment hw/arm: Restrict APEI tables generation to the 'virt' machine hw/timer/bcm2835: Introduce BCM2835_SYSTIMER_COUNT definition hw/timer/bcm2835: Rename variable holding CTRL_STATUS register hw/timer/bcm2835: Support the timer COMPARE registers hw/arm/bcm2835_peripherals: Correctly wire the SYS_timer IRQs hw/intc/bcm2835_ic: Trace GPU/CPU IRQ handlers hw/intc/bcm2836_control: Use IRQ definitions instead of magic numbers hw/arm/nseries: Fix loading kernel image on n8x0 machines linux-user/elfload: Avoid leaking interp_name using GLib memory API Richard Henderson (16): accel/tcg: Add tlb_flush_page_bits_by_mmuidx* target/arm: Use tlb_flush_page_bits_by_mmuidx* target/arm: Remove redundant mmu_idx lookup target/arm: Fix reported EL for mte_check_fail target/arm: Ignore HCR_EL2.ATA when {E2H,TGE} != 11 linux-user/aarch64: Reset btype for signals linux-user: Set PAGE_TARGET_1 for TARGET_PROT_BTI include/elf: Add defines related to GNU property notes for AArch64 linux-user/elfload: Fix coding style in load_elf_image linux-user/elfload: Adjust iteration over phdr linux-user/elfload: Move PT_INTERP detection to first loop linux-user/elfload: Use Error for load_elf_image linux-user/elfload: Use Error for load_elf_interp linux-user/elfload: Parse NT_GNU_PROPERTY_TYPE_0 notes linux-user/elfload: Parse GNU_PROPERTY_AARCH64_FEATURE_1_AND tests/tcg/aarch64: Add bti smoke tests docs/devel/loads-stores.rst | 8 +- default-configs/devices/arm-softmmu.mak | 1 - include/elf.h | 22 ++ include/exec/cpu-all.h | 2 + include/exec/exec-all.h | 36 ++ include/hw/timer/bcm2835_systmr.h | 17 +- linux-user/qemu.h | 4 + linux-user/syscall_defs.h | 4 + target/arm/cpu.h | 13 + target/arm/helper.h | 13 + target/arm/internals.h | 9 +- target/arm/m-nocp.decode | 10 +- target/arm/t32.decode | 50 ++- accel/tcg/cputlb.c | 275 +++++++++++++++- hw/arm/bcm2835_peripherals.c | 13 +- hw/arm/nseries.c | 1 + hw/arm/strongarm.c | 2 +- hw/i2c/microbit_i2c.c | 1 + hw/intc/bcm2835_ic.c | 4 +- hw/intc/bcm2836_control.c | 8 +- hw/timer/bcm2835_systmr.c | 57 ++-- linux-user/aarch64/signal.c | 10 +- linux-user/elfload.c | 326 ++++++++++++++---- linux-user/mmap.c | 16 + target/arm/cpu.c | 38 ++- target/arm/helper.c | 55 +++- target/arm/mte_helper.c | 13 +- target/arm/translate-a64.c | 6 +- target/arm/translate.c | 239 +++++++++++++- target/arm/vfp_helper.c | 76 +++-- tests/qtest/npcm7xx_timer-test.c | 562 ++++++++++++++++++++++++++++++++ tests/tcg/aarch64/bti-1.c | 62 ++++ tests/tcg/aarch64/bti-2.c | 108 ++++++ tests/tcg/aarch64/bti-crt.inc.c | 51 +++ hw/arm/Kconfig | 1 + hw/intc/trace-events | 4 + hw/timer/trace-events | 6 +- scripts/decodetree.py | 2 +- target/arm/translate-vfp.c.inc | 41 ++- tests/qtest/meson.build | 1 + tests/tcg/aarch64/Makefile.target | 10 + tests/tcg/configure.sh | 4 + 42 files changed, 1973 insertions(+), 208 deletions(-) create mode 100644 tests/qtest/npcm7xx_timer-test.c create mode 100644 tests/tcg/aarch64/bti-1.c create mode 100644 tests/tcg/aarch64/bti-2.c create mode 100644 tests/tcg/aarch64/bti-crt.inc.c The SMLAD instruction is supposed to: * signed multiply Rn[15:0] * Rm[15:0] * signed multiply Rn[31:16] * Rm[31:16] * perform a signed addition of the products and Ra * set Rd to the low 32 bits of the theoretical infinite-precision result * set the Q flag if the sign-extension of Rd would differ from the infinite-precision result (ie on overflow) Our current implementation doesn't quite do this, though: it performs an addition of the products setting Q on overflow, and then it adds Ra, again possibly setting Q. This sometimes incorrectly sets Q when the architecturally mandated only-check-for-overflow-once algorithm does not. For instance: r1 = 0x80008000; r2 = 0x80008000; r3 = 0xffffffff smlad r0, r1, r2, r3 This is (-32768 * -32768) + (-32768 * -32768) - 1 The products are both 0x4000_0000, so when added together as 32-bit signed numbers they overflow (and QEMU sets Q), but because the addition of Ra == -1 brings the total back down to 0x7fff_ffff there is no overflow for the complete operation and setting Q is incorrect. Fix this edge case by resorting to 64-bit arithmetic for the case where we need to add three values together. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201009144712.11187-1-peter.maydell@linaro.org --- target/arm/translate.c | 58 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 48 insertions(+), 10 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 bool op_smlad(DisasContext *s, arg_rrrr *a, bool m_swap, bool sub) gen_smul_dual(t1, t2); if (sub) { - /* This subtraction cannot overflow. */ + /* + * This subtraction cannot overflow, so we can do a simple + * 32-bit subtraction and then a possible 32-bit saturating + * addition of Ra. + */ tcg_gen_sub_i32(t1, t1, t2); + tcg_temp_free_i32(t2); + + if (a->ra != 15) { + t2 = load_reg(s, a->ra); + gen_helper_add_setq(t1, cpu_env, t1, t2); + tcg_temp_free_i32(t2); + } + } else if (a->ra == 15) { + /* Single saturation-checking addition */ + gen_helper_add_setq(t1, cpu_env, t1, t2); + tcg_temp_free_i32(t2); } else { /* - * This addition cannot overflow 32 bits; however it may - * overflow considered as a signed operation, in which case - * we must set the Q flag. + * We need to add the products and Ra together and then + * determine whether the final result overflowed. Doing + * this as two separate add-and-check-overflow steps incorrectly + * sets Q for cases like (-32768 * -32768) + (-32768 * -32768) + -1. + * Do all the arithmetic at 64-bits and then check for overflow. */ - gen_helper_add_setq(t1, cpu_env, t1, t2); - } - tcg_temp_free_i32(t2); + TCGv_i64 p64, q64; + TCGv_i32 t3, qf, one; - if (a->ra != 15) { - t2 = load_reg(s, a->ra); - gen_helper_add_setq(t1, cpu_env, t1, t2); + p64 = tcg_temp_new_i64(); + q64 = tcg_temp_new_i64(); + tcg_gen_ext_i32_i64(p64, t1); + tcg_gen_ext_i32_i64(q64, t2); + tcg_gen_add_i64(p64, p64, q64); + load_reg_var(s, t2, a->ra); + tcg_gen_ext_i32_i64(q64, t2); + tcg_gen_add_i64(p64, p64, q64); + tcg_temp_free_i64(q64); + + tcg_gen_extr_i64_i32(t1, t2, p64); + tcg_temp_free_i64(p64); + /* + * t1 is the low half of the result which goes into Rd. + * We have overflow and must set Q if the high half (t2) + * is different from the sign-extension of t1. + */ + t3 = tcg_temp_new_i32(); + tcg_gen_sari_i32(t3, t1, 31); + qf = load_cpu_field(QF); + one = tcg_const_i32(1); + tcg_gen_movcond_i32(TCG_COND_NE, qf, t2, t3, one, qf); + store_cpu_field(qf, QF); + tcg_temp_free_i32(one); + tcg_temp_free_i32(t3); tcg_temp_free_i32(t2); } store_reg(s, a->rd, t1); -- 2.20.1 For AArch32, unlike the VCVT of integer to float, which honours the rounding mode specified by the FPSCR, VCVT of fixed-point to float is always round-to-nearest. (AArch64 fixed-point-to-float conversions always honour the FPCR rounding mode.) Implement this by providing _round_to_nearest versions of the relevant helpers which set the rounding mode temporarily when making the call to the underlying softfloat function. We only need to change the VFP VCVT instructions, because the standard- FPSCR value used by the Neon VCVT is always set to round-to-nearest, so we don't need to do the extra work of saving and restoring the rounding mode. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201013103532.13391-1-peter.maydell@linaro.org --- target/arm/helper.h | 13 +++++++++++++ target/arm/vfp_helper.c | 23 ++++++++++++++++++++++- target/arm/translate-vfp.c.inc | 24 ++++++++++++------------ 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_3(vfp_ultoh, f16, i32, i32, ptr) DEF_HELPER_3(vfp_sqtoh, f16, i64, i32, ptr) DEF_HELPER_3(vfp_uqtoh, f16, i64, i32, ptr) +DEF_HELPER_3(vfp_shtos_round_to_nearest, f32, i32, i32, ptr) +DEF_HELPER_3(vfp_sltos_round_to_nearest, f32, i32, i32, ptr) +DEF_HELPER_3(vfp_uhtos_round_to_nearest, f32, i32, i32, ptr) +DEF_HELPER_3(vfp_ultos_round_to_nearest, f32, i32, i32, ptr) +DEF_HELPER_3(vfp_shtod_round_to_nearest, f64, i64, i32, ptr) +DEF_HELPER_3(vfp_sltod_round_to_nearest, f64, i64, i32, ptr) +DEF_HELPER_3(vfp_uhtod_round_to_nearest, f64, i64, i32, ptr) +DEF_HELPER_3(vfp_ultod_round_to_nearest, f64, i64, i32, ptr) +DEF_HELPER_3(vfp_shtoh_round_to_nearest, f16, i32, i32, ptr) +DEF_HELPER_3(vfp_uhtoh_round_to_nearest, f16, i32, i32, ptr) +DEF_HELPER_3(vfp_sltoh_round_to_nearest, f16, i32, i32, ptr) +DEF_HELPER_3(vfp_ultoh_round_to_nearest, f16, i32, i32, ptr) + DEF_HELPER_FLAGS_2(set_rmode, TCG_CALL_NO_RWG, i32, i32, ptr) DEF_HELPER_FLAGS_3(vfp_fcvt_f16_to_f32, TCG_CALL_NO_RWG, f32, f16, ptr, i32) diff --git a/target/arm/vfp_helper.c b/target/arm/vfp_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/vfp_helper.c +++ b/target/arm/vfp_helper.c @@ -XXX,XX +XXX,XX @@ float32 VFP_HELPER(fcvts, d)(float64 x, CPUARMState *env) return float64_to_float32(x, &env->vfp.fp_status); } -/* VFP3 fixed point conversion. */ +/* + * VFP3 fixed point conversion. The AArch32 versions of fix-to-float + * must always round-to-nearest; the AArch64 ones honour the FPSCR + * rounding mode. (For AArch32 Neon the standard-FPSCR is set to + * round-to-nearest so either helper will work.) AArch32 float-to-fix + * must round-to-zero. + */ #define VFP_CONV_FIX_FLOAT(name, p, fsz, ftype, isz, itype) \ ftype HELPER(vfp_##name##to##p)(uint##isz##_t x, uint32_t shift, \ void *fpstp) \ { return itype##_to_##float##fsz##_scalbn(x, -shift, fpstp); } +#define VFP_CONV_FIX_FLOAT_ROUND(name, p, fsz, ftype, isz, itype) \ + ftype HELPER(vfp_##name##to##p##_round_to_nearest)(uint##isz##_t x, \ + uint32_t shift, \ + void *fpstp) \ + { \ + ftype ret; \ + float_status *fpst = fpstp; \ + FloatRoundMode oldmode = fpst->float_rounding_mode; \ + fpst->float_rounding_mode = float_round_nearest_even; \ + ret = itype##_to_##float##fsz##_scalbn(x, -shift, fpstp); \ + fpst->float_rounding_mode = oldmode; \ + return ret; \ + } + #define VFP_CONV_FLOAT_FIX_ROUND(name, p, fsz, ftype, isz, itype, ROUND, suff) \ uint##isz##_t HELPER(vfp_to##name##p##suff)(ftype x, uint32_t shift, \ void *fpst) \ @@ -XXX,XX +XXX,XX @@ uint##isz##_t HELPER(vfp_to##name##p##suff)(ftype x, uint32_t shift, \ #define VFP_CONV_FIX(name, p, fsz, ftype, isz, itype) \ VFP_CONV_FIX_FLOAT(name, p, fsz, ftype, isz, itype) \ +VFP_CONV_FIX_FLOAT_ROUND(name, p, fsz, ftype, isz, itype) \ VFP_CONV_FLOAT_FIX_ROUND(name, p, fsz, ftype, isz, itype, \ float_round_to_zero, _round_to_zero) \ VFP_CONV_FLOAT_FIX_ROUND(name, p, fsz, ftype, isz, itype, \ diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-vfp.c.inc +++ b/target/arm/translate-vfp.c.inc @@ -XXX,XX +XXX,XX @@ static bool trans_VCVT_fix_hp(DisasContext *s, arg_VCVT_fix_sp *a) /* Switch on op:U:sx bits */ switch (a->opc) { case 0: - gen_helper_vfp_shtoh(vd, vd, shift, fpst); + gen_helper_vfp_shtoh_round_to_nearest(vd, vd, shift, fpst); break; case 1: - gen_helper_vfp_sltoh(vd, vd, shift, fpst); + gen_helper_vfp_sltoh_round_to_nearest(vd, vd, shift, fpst); break; case 2: - gen_helper_vfp_uhtoh(vd, vd, shift, fpst); + gen_helper_vfp_uhtoh_round_to_nearest(vd, vd, shift, fpst); break; case 3: - gen_helper_vfp_ultoh(vd, vd, shift, fpst); + gen_helper_vfp_ultoh_round_to_nearest(vd, vd, shift, fpst); break; case 4: gen_helper_vfp_toshh_round_to_zero(vd, vd, shift, fpst); @@ -XXX,XX +XXX,XX @@ static bool trans_VCVT_fix_sp(DisasContext *s, arg_VCVT_fix_sp *a) /* Switch on op:U:sx bits */ switch (a->opc) { case 0: - gen_helper_vfp_shtos(vd, vd, shift, fpst); + gen_helper_vfp_shtos_round_to_nearest(vd, vd, shift, fpst); break; case 1: - gen_helper_vfp_sltos(vd, vd, shift, fpst); + gen_helper_vfp_sltos_round_to_nearest(vd, vd, shift, fpst); break; case 2: - gen_helper_vfp_uhtos(vd, vd, shift, fpst); + gen_helper_vfp_uhtos_round_to_nearest(vd, vd, shift, fpst); break; case 3: - gen_helper_vfp_ultos(vd, vd, shift, fpst); + gen_helper_vfp_ultos_round_to_nearest(vd, vd, shift, fpst); break; case 4: gen_helper_vfp_toshs_round_to_zero(vd, vd, shift, fpst); @@ -XXX,XX +XXX,XX @@ static bool trans_VCVT_fix_dp(DisasContext *s, arg_VCVT_fix_dp *a) /* Switch on op:U:sx bits */ switch (a->opc) { case 0: - gen_helper_vfp_shtod(vd, vd, shift, fpst); + gen_helper_vfp_shtod_round_to_nearest(vd, vd, shift, fpst); break; case 1: - gen_helper_vfp_sltod(vd, vd, shift, fpst); + gen_helper_vfp_sltod_round_to_nearest(vd, vd, shift, fpst); break; case 2: - gen_helper_vfp_uhtod(vd, vd, shift, fpst); + gen_helper_vfp_uhtod_round_to_nearest(vd, vd, shift, fpst); break; case 3: - gen_helper_vfp_ultod(vd, vd, shift, fpst); + gen_helper_vfp_ultod_round_to_nearest(vd, vd, shift, fpst); break; case 4: gen_helper_vfp_toshd_round_to_zero(vd, vd, shift, fpst); -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> The time to transmit a char is expressed in nanoseconds, not in ticks. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201014213601.205222-1-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/strongarm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/strongarm.c +++ b/hw/arm/strongarm.c @@ -XXX,XX +XXX,XX @@ struct StrongARMUARTState { uint8_t rx_start; uint8_t rx_len; - uint64_t char_transmit_time; /* time to transmit a char in ticks*/ + uint64_t char_transmit_time; /* time to transmit a char in nanoseconds */ bool wait_break_end; QEMUTimer *rx_timeout_timer; QEMUTimer *tx_timer; -- 2.20.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> While APEI is a generic ACPI feature (usable by X86 and ARM64), only the 'virt' machine uses it, by enabling the RAS Virtualization. See commit 2afa8c8519: "hw/arm/virt: Introduce a RAS machine option"). Restrict the APEI tables generation code to the single user: the virt machine. If another machine wants to use it, it simply has to 'select ACPI_APEI' in its Kconfig. Fixes: aa16508f1d ("ACPI: Build related register address fields via hardware error fw_cfg blob") Acked-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Dongjiu Geng <gengdongjiu@huawei.com> Acked-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-id: 20201008161414.2672569-1-philmd@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- default-configs/devices/arm-softmmu.mak | 1 - hw/arm/Kconfig | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/default-configs/devices/arm-softmmu.mak b/default-configs/devices/arm-softmmu.mak index XXXXXXX..XXXXXXX 100644 --- a/default-configs/devices/arm-softmmu.mak +++ b/default-configs/devices/arm-softmmu.mak @@ -XXX,XX +XXX,XX @@ CONFIG_FSL_IMX7=y CONFIG_FSL_IMX6UL=y CONFIG_SEMIHOSTING=y CONFIG_ALLWINNER_H3=y -CONFIG_ACPI_APEI=y diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -XXX,XX +XXX,XX @@ config ARM_VIRT select ACPI_MEMORY_HOTPLUG select ACPI_HW_REDUCED select ACPI_NVDIMM + select ACPI_APEI config CHEETAH bool -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Use the BCM2835_SYSTIMER_COUNT definition instead of the magic '4' value. Reviewed-by: Luc Michel <luc.michel@greensocs.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201010203709.3116542-2-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/timer/bcm2835_systmr.h | 4 +++- hw/timer/bcm2835_systmr.c | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/include/hw/timer/bcm2835_systmr.h b/include/hw/timer/bcm2835_systmr.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/timer/bcm2835_systmr.h +++ b/include/hw/timer/bcm2835_systmr.h @@ -XXX,XX +XXX,XX @@ #define TYPE_BCM2835_SYSTIMER "bcm2835-sys-timer" OBJECT_DECLARE_SIMPLE_TYPE(BCM2835SystemTimerState, BCM2835_SYSTIMER) +#define BCM2835_SYSTIMER_COUNT 4 + struct BCM2835SystemTimerState { /*< private >*/ SysBusDevice parent_obj; @@ -XXX,XX +XXX,XX @@ struct BCM2835SystemTimerState { struct { uint32_t status; - uint32_t compare[4]; + uint32_t compare[BCM2835_SYSTIMER_COUNT]; } reg; }; diff --git a/hw/timer/bcm2835_systmr.c b/hw/timer/bcm2835_systmr.c index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/bcm2835_systmr.c +++ b/hw/timer/bcm2835_systmr.c @@ -XXX,XX +XXX,XX @@ static const VMStateDescription bcm2835_systmr_vmstate = { .minimum_version_id = 1, .fields = (VMStateField[]) { VMSTATE_UINT32(reg.status, BCM2835SystemTimerState), - VMSTATE_UINT32_ARRAY(reg.compare, BCM2835SystemTimerState, 4), + VMSTATE_UINT32_ARRAY(reg.compare, BCM2835SystemTimerState, + BCM2835_SYSTIMER_COUNT), VMSTATE_END_OF_LIST() } }; -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> The variable holding the CTRL_STATUS register is misnamed 'status'. Rename it 'ctrl_status' to make it more obvious this register is also used to control the peripheral. Reviewed-by: Luc Michel <luc.michel@greensocs.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201010203709.3116542-3-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/timer/bcm2835_systmr.h | 2 +- hw/timer/bcm2835_systmr.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/hw/timer/bcm2835_systmr.h b/include/hw/timer/bcm2835_systmr.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/timer/bcm2835_systmr.h +++ b/include/hw/timer/bcm2835_systmr.h @@ -XXX,XX +XXX,XX @@ struct BCM2835SystemTimerState { qemu_irq irq; struct { - uint32_t status; + uint32_t ctrl_status; uint32_t compare[BCM2835_SYSTIMER_COUNT]; } reg; }; diff --git a/hw/timer/bcm2835_systmr.c b/hw/timer/bcm2835_systmr.c index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/bcm2835_systmr.c +++ b/hw/timer/bcm2835_systmr.c @@ -XXX,XX +XXX,XX @@ REG32(COMPARE3, 0x18) static void bcm2835_systmr_update_irq(BCM2835SystemTimerState *s) { - bool enable = !!s->reg.status; + bool enable = !!s->reg.ctrl_status; trace_bcm2835_systmr_irq(enable); qemu_set_irq(s->irq, enable); @@ -XXX,XX +XXX,XX @@ static uint64_t bcm2835_systmr_read(void *opaque, hwaddr offset, switch (offset) { case A_CTRL_STATUS: - r = s->reg.status; + r = s->reg.ctrl_status; break; case A_COMPARE0 ... A_COMPARE3: r = s->reg.compare[(offset - A_COMPARE0) >> 2]; @@ -XXX,XX +XXX,XX @@ static void bcm2835_systmr_write(void *opaque, hwaddr offset, trace_bcm2835_systmr_write(offset, value); switch (offset) { case A_CTRL_STATUS: - s->reg.status &= ~value; /* Ack */ + s->reg.ctrl_status &= ~value; /* Ack */ bcm2835_systmr_update_irq(s); break; case A_COMPARE0 ... A_COMPARE3: @@ -XXX,XX +XXX,XX @@ static const VMStateDescription bcm2835_systmr_vmstate = { .version_id = 1, .minimum_version_id = 1, .fields = (VMStateField[]) { - VMSTATE_UINT32(reg.status, BCM2835SystemTimerState), + VMSTATE_UINT32(reg.ctrl_status, BCM2835SystemTimerState), VMSTATE_UINT32_ARRAY(reg.compare, BCM2835SystemTimerState, BCM2835_SYSTIMER_COUNT), VMSTATE_END_OF_LIST() -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> This peripheral has 1 free-running timer and 4 compare registers. Only the free-running timer is implemented. Add support the COMPARE registers (each register is wired to an IRQ). Reference: "BCM2835 ARM Peripherals" datasheet [*] chapter 12 "System Timer": The System Timer peripheral provides four 32-bit timer channels and a single 64-bit free running counter. Each channel has an output compare register, which is compared against the 32 least significant bits of the free running counter values. When the two values match, the system timer peripheral generates a signal to indicate a match for the appropriate channel. The match signal is then fed into the interrupt controller. This peripheral is used since Linux 3.7, commit ee4af5696720 ("ARM: bcm2835: add system timer"). [*] https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Message-id: 20201010203709.3116542-4-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/timer/bcm2835_systmr.h | 11 +++++-- hw/timer/bcm2835_systmr.c | 48 ++++++++++++++++++++----------- hw/timer/trace-events | 6 ++-- 3 files changed, 44 insertions(+), 21 deletions(-) diff --git a/include/hw/timer/bcm2835_systmr.h b/include/hw/timer/bcm2835_systmr.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/timer/bcm2835_systmr.h +++ b/include/hw/timer/bcm2835_systmr.h @@ -XXX,XX +XXX,XX @@ #include "hw/sysbus.h" #include "hw/irq.h" +#include "qemu/timer.h" #include "qom/object.h" #define TYPE_BCM2835_SYSTIMER "bcm2835-sys-timer" @@ -XXX,XX +XXX,XX @@ OBJECT_DECLARE_SIMPLE_TYPE(BCM2835SystemTimerState, BCM2835_SYSTIMER) #define BCM2835_SYSTIMER_COUNT 4 +typedef struct { + unsigned id; + QEMUTimer timer; + qemu_irq irq; + BCM2835SystemTimerState *state; +} BCM2835SystemTimerCompare; + struct BCM2835SystemTimerState { /*< private >*/ SysBusDevice parent_obj; /*< public >*/ MemoryRegion iomem; - qemu_irq irq; - struct { uint32_t ctrl_status; uint32_t compare[BCM2835_SYSTIMER_COUNT]; } reg; + BCM2835SystemTimerCompare tmr[BCM2835_SYSTIMER_COUNT]; }; #endif diff --git a/hw/timer/bcm2835_systmr.c b/hw/timer/bcm2835_systmr.c index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/bcm2835_systmr.c +++ b/hw/timer/bcm2835_systmr.c @@ -XXX,XX +XXX,XX @@ REG32(COMPARE1, 0x10) REG32(COMPARE2, 0x14) REG32(COMPARE3, 0x18) -static void bcm2835_systmr_update_irq(BCM2835SystemTimerState *s) +static void bcm2835_systmr_timer_expire(void *opaque) { - bool enable = !!s->reg.ctrl_status; + BCM2835SystemTimerCompare *tmr = opaque; - trace_bcm2835_systmr_irq(enable); - qemu_set_irq(s->irq, enable); -} - -static void bcm2835_systmr_update_compare(BCM2835SystemTimerState *s, - unsigned timer_index) -{ - /* TODO fow now, since neither Linux nor U-boot use these timers. */ - qemu_log_mask(LOG_UNIMP, "COMPARE register %u not implemented\n", - timer_index); + trace_bcm2835_systmr_timer_expired(tmr->id); + tmr->state->reg.ctrl_status |= 1 << tmr->id; + qemu_set_irq(tmr->irq, 1); } static uint64_t bcm2835_systmr_read(void *opaque, hwaddr offset, @@ -XXX,XX +XXX,XX @@ static uint64_t bcm2835_systmr_read(void *opaque, hwaddr offset, } static void bcm2835_systmr_write(void *opaque, hwaddr offset, - uint64_t value, unsigned size) + uint64_t value64, unsigned size) { BCM2835SystemTimerState *s = BCM2835_SYSTIMER(opaque); + int index; + uint32_t value = value64; + uint32_t triggers_delay_us; + uint64_t now; trace_bcm2835_systmr_write(offset, value); switch (offset) { case A_CTRL_STATUS: s->reg.ctrl_status &= ~value; /* Ack */ - bcm2835_systmr_update_irq(s); + for (index = 0; index < ARRAY_SIZE(s->tmr); index++) { + if (extract32(value, index, 1)) { + trace_bcm2835_systmr_irq_ack(index); + qemu_set_irq(s->tmr[index].irq, 0); + } + } break; case A_COMPARE0 ... A_COMPARE3: - s->reg.compare[(offset - A_COMPARE0) >> 2] = value; - bcm2835_systmr_update_compare(s, (offset - A_COMPARE0) >> 2); + index = (offset - A_COMPARE0) >> 2; + s->reg.compare[index] = value; + now = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL); + /* Compare lower 32-bits of the free-running counter. */ + triggers_delay_us = value - now; + trace_bcm2835_systmr_run(index, triggers_delay_us); + timer_mod(&s->tmr[index].timer, now + triggers_delay_us); break; case A_COUNTER_LOW: case A_COUNTER_HIGH: @@ -XXX,XX +XXX,XX @@ static void bcm2835_systmr_realize(DeviceState *dev, Error **errp) memory_region_init_io(&s->iomem, OBJECT(dev), &bcm2835_systmr_ops, s, "bcm2835-sys-timer", 0x20); sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem); - sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq); + + for (size_t i = 0; i < ARRAY_SIZE(s->tmr); i++) { + s->tmr[i].id = i; + s->tmr[i].state = s; + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->tmr[i].irq); + timer_init_us(&s->tmr[i].timer, QEMU_CLOCK_VIRTUAL, + bcm2835_systmr_timer_expire, &s->tmr[i]); + } } static const VMStateDescription bcm2835_systmr_vmstate = { diff --git a/hw/timer/trace-events b/hw/timer/trace-events index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/trace-events +++ b/hw/timer/trace-events @@ -XXX,XX +XXX,XX @@ nrf51_timer_write(uint8_t timer_id, uint64_t addr, uint32_t value, unsigned size nrf51_timer_set_count(uint8_t timer_id, uint8_t counter_id, uint32_t value) "timer %u counter %u count 0x%" PRIx32 # bcm2835_systmr.c -bcm2835_systmr_irq(bool enable) "timer irq state %u" +bcm2835_systmr_timer_expired(unsigned id) "timer #%u expired" +bcm2835_systmr_irq_ack(unsigned id) "timer #%u acked" bcm2835_systmr_read(uint64_t offset, uint64_t data) "timer read: offset 0x%" PRIx64 " data 0x%" PRIx64 -bcm2835_systmr_write(uint64_t offset, uint64_t data) "timer write: offset 0x%" PRIx64 " data 0x%" PRIx64 +bcm2835_systmr_write(uint64_t offset, uint32_t data) "timer write: offset 0x%" PRIx64 " data 0x%" PRIx32 +bcm2835_systmr_run(unsigned id, uint64_t delay_us) "timer #%u expiring in %"PRIu64" us" # avr_timer16.c avr_timer16_read(uint8_t addr, uint8_t value) "timer16 read addr:%u value:%u" -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> The SYS_timer is not directly wired to the ARM core, but to the SoC (peripheral) interrupt controller. Fixes: 0e5bbd74064 ("hw/arm/bcm2835_peripherals: Use the SYS_timer") Reviewed-by: Luc Michel <luc.michel@greensocs.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201010203709.3116542-5-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/bcm2835_peripherals.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/bcm2835_peripherals.c +++ b/hw/arm/bcm2835_peripherals.c @@ -XXX,XX +XXX,XX @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp) memory_region_add_subregion(&s->peri_mr, ST_OFFSET, sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->systmr), 0)); sysbus_connect_irq(SYS_BUS_DEVICE(&s->systmr), 0, - qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_ARM_IRQ, - INTERRUPT_ARM_TIMER)); + qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ, + INTERRUPT_TIMER0)); + sysbus_connect_irq(SYS_BUS_DEVICE(&s->systmr), 1, + qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ, + INTERRUPT_TIMER1)); + sysbus_connect_irq(SYS_BUS_DEVICE(&s->systmr), 2, + qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ, + INTERRUPT_TIMER2)); + sysbus_connect_irq(SYS_BUS_DEVICE(&s->systmr), 3, + qdev_get_gpio_in_named(DEVICE(&s->ic), BCM2835_IC_GPU_IRQ, + INTERRUPT_TIMER3)); /* UART0 */ qdev_prop_set_chr(DEVICE(&s->uart0), "chardev", serial_hd(0)); -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> On ARM, the Top Byte Ignore feature means that only 56 bits of the address are significant in the virtual address. We are required to give the entire 64-bit address to FAR_ELx on fault, which means that we do not "clean" the top byte early in TCG. This new interface allows us to flush all 256 possible aliases for a given page, currently missed by tlb_flush_page*. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20201016210754.818257-2-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/exec/exec-all.h | 36 ++++++ accel/tcg/cputlb.c | 275 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 302 insertions(+), 9 deletions(-) diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -XXX,XX +XXX,XX @@ void tlb_flush_by_mmuidx_all_cpus(CPUState *cpu, uint16_t idxmap); * depend on when the guests translation ends the TB. */ void tlb_flush_by_mmuidx_all_cpus_synced(CPUState *cpu, uint16_t idxmap); + +/** + * tlb_flush_page_bits_by_mmuidx + * @cpu: CPU whose TLB should be flushed + * @addr: virtual address of page to be flushed + * @idxmap: bitmap of mmu indexes to flush + * @bits: number of significant bits in address + * + * Similar to tlb_flush_page_mask, but with a bitmap of indexes. + */ +void tlb_flush_page_bits_by_mmuidx(CPUState *cpu, target_ulong addr, + uint16_t idxmap, unsigned bits); + +/* Similarly, with broadcast and syncing. */ +void tlb_flush_page_bits_by_mmuidx_all_cpus(CPUState *cpu, target_ulong addr, + uint16_t idxmap, unsigned bits); +void tlb_flush_page_bits_by_mmuidx_all_cpus_synced + (CPUState *cpu, target_ulong addr, uint16_t idxmap, unsigned bits); + /** * tlb_set_page_with_attrs: * @cpu: CPU to add this TLB entry for @@ -XXX,XX +XXX,XX @@ static inline void tlb_flush_by_mmuidx_all_cpus_synced(CPUState *cpu, uint16_t idxmap) { } +static inline void tlb_flush_page_bits_by_mmuidx(CPUState *cpu, + target_ulong addr, + uint16_t idxmap, + unsigned bits) +{ +} +static inline void tlb_flush_page_bits_by_mmuidx_all_cpus(CPUState *cpu, + target_ulong addr, + uint16_t idxmap, + unsigned bits) +{ +} +static inline void +tlb_flush_page_bits_by_mmuidx_all_cpus_synced(CPUState *cpu, target_ulong addr, + uint16_t idxmap, unsigned bits) +{ +} #endif /** * probe_access: diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c index XXXXXXX..XXXXXXX 100644 --- a/accel/tcg/cputlb.c +++ b/accel/tcg/cputlb.c @@ -XXX,XX +XXX,XX @@ void tlb_flush_all_cpus_synced(CPUState *src_cpu) tlb_flush_by_mmuidx_all_cpus_synced(src_cpu, ALL_MMUIDX_BITS); } +static bool tlb_hit_page_mask_anyprot(CPUTLBEntry *tlb_entry, + target_ulong page, target_ulong mask) +{ + page &= mask; + mask &= TARGET_PAGE_MASK | TLB_INVALID_MASK; + + return (page == (tlb_entry->addr_read & mask) || + page == (tlb_addr_write(tlb_entry) & mask) || + page == (tlb_entry->addr_code & mask)); +} + static inline bool tlb_hit_page_anyprot(CPUTLBEntry *tlb_entry, target_ulong page) { - return tlb_hit_page(tlb_entry->addr_read, page) || - tlb_hit_page(tlb_addr_write(tlb_entry), page) || - tlb_hit_page(tlb_entry->addr_code, page); + return tlb_hit_page_mask_anyprot(tlb_entry, page, -1); } /** @@ -XXX,XX +XXX,XX @@ static inline bool tlb_entry_is_empty(const CPUTLBEntry *te) } /* Called with tlb_c.lock held */ -static inline bool tlb_flush_entry_locked(CPUTLBEntry *tlb_entry, - target_ulong page) +static bool tlb_flush_entry_mask_locked(CPUTLBEntry *tlb_entry, + target_ulong page, + target_ulong mask) { - if (tlb_hit_page_anyprot(tlb_entry, page)) { + if (tlb_hit_page_mask_anyprot(tlb_entry, page, mask)) { memset(tlb_entry, -1, sizeof(*tlb_entry)); return true; } return false; } +static inline bool tlb_flush_entry_locked(CPUTLBEntry *tlb_entry, + target_ulong page) +{ + return tlb_flush_entry_mask_locked(tlb_entry, page, -1); +} + /* Called with tlb_c.lock held */ -static inline void tlb_flush_vtlb_page_locked(CPUArchState *env, int mmu_idx, - target_ulong page) +static void tlb_flush_vtlb_page_mask_locked(CPUArchState *env, int mmu_idx, + target_ulong page, + target_ulong mask) { CPUTLBDesc *d = &env_tlb(env)->d[mmu_idx]; int k; assert_cpu_is_self(env_cpu(env)); for (k = 0; k < CPU_VTLB_SIZE; k++) { - if (tlb_flush_entry_locked(&d->vtable[k], page)) { + if (tlb_flush_entry_mask_locked(&d->vtable[k], page, mask)) { tlb_n_used_entries_dec(env, mmu_idx); } } } +static inline void tlb_flush_vtlb_page_locked(CPUArchState *env, int mmu_idx, + target_ulong page) +{ + tlb_flush_vtlb_page_mask_locked(env, mmu_idx, page, -1); +} + static void tlb_flush_page_locked(CPUArchState *env, int midx, target_ulong page) { @@ -XXX,XX +XXX,XX @@ void tlb_flush_page_all_cpus_synced(CPUState *src, target_ulong addr) tlb_flush_page_by_mmuidx_all_cpus_synced(src, addr, ALL_MMUIDX_BITS); } +static void tlb_flush_page_bits_locked(CPUArchState *env, int midx, + target_ulong page, unsigned bits) +{ + CPUTLBDesc *d = &env_tlb(env)->d[midx]; + CPUTLBDescFast *f = &env_tlb(env)->f[midx]; + target_ulong mask = MAKE_64BIT_MASK(0, bits); + + /* + * If @bits is smaller than the tlb size, there may be multiple entries + * within the TLB; otherwise all addresses that match under @mask hit + * the same TLB entry. + * + * TODO: Perhaps allow bits to be a few bits less than the size. + * For now, just flush the entire TLB. + */ + if (mask < f->mask) { + tlb_debug("forcing full flush midx %d (" + TARGET_FMT_lx "/" TARGET_FMT_lx ")\n", + midx, page, mask); + tlb_flush_one_mmuidx_locked(env, midx, get_clock_realtime()); + return; + } + + /* Check if we need to flush due to large pages. */ + if ((page & d->large_page_mask) == d->large_page_addr) { + tlb_debug("forcing full flush midx %d (" + TARGET_FMT_lx "/" TARGET_FMT_lx ")\n", + midx, d->large_page_addr, d->large_page_mask); + tlb_flush_one_mmuidx_locked(env, midx, get_clock_realtime()); + return; + } + + if (tlb_flush_entry_mask_locked(tlb_entry(env, midx, page), page, mask)) { + tlb_n_used_entries_dec(env, midx); + } + tlb_flush_vtlb_page_mask_locked(env, midx, page, mask); +} + +typedef struct { + target_ulong addr; + uint16_t idxmap; + uint16_t bits; +} TLBFlushPageBitsByMMUIdxData; + +static void +tlb_flush_page_bits_by_mmuidx_async_0(CPUState *cpu, + TLBFlushPageBitsByMMUIdxData d) +{ + CPUArchState *env = cpu->env_ptr; + int mmu_idx; + + assert_cpu_is_self(cpu); + + tlb_debug("page addr:" TARGET_FMT_lx "/%u mmu_map:0x%x\n", + d.addr, d.bits, d.idxmap); + + qemu_spin_lock(&env_tlb(env)->c.lock); + for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) { + if ((d.idxmap >> mmu_idx) & 1) { + tlb_flush_page_bits_locked(env, mmu_idx, d.addr, d.bits); + } + } + qemu_spin_unlock(&env_tlb(env)->c.lock); + + tb_flush_jmp_cache(cpu, d.addr); +} + +static bool encode_pbm_to_runon(run_on_cpu_data *out, + TLBFlushPageBitsByMMUIdxData d) +{ + /* We need 6 bits to hold to hold @bits up to 63. */ + if (d.idxmap <= MAKE_64BIT_MASK(0, TARGET_PAGE_BITS - 6)) { + *out = RUN_ON_CPU_TARGET_PTR(d.addr | (d.idxmap << 6) | d.bits); + return true; + } + return false; +} + +static TLBFlushPageBitsByMMUIdxData +decode_runon_to_pbm(run_on_cpu_data data) +{ + target_ulong addr_map_bits = (target_ulong) data.target_ptr; + return (TLBFlushPageBitsByMMUIdxData){ + .addr = addr_map_bits & TARGET_PAGE_MASK, + .idxmap = (addr_map_bits & ~TARGET_PAGE_MASK) >> 6, + .bits = addr_map_bits & 0x3f + }; +} + +static void tlb_flush_page_bits_by_mmuidx_async_1(CPUState *cpu, + run_on_cpu_data runon) +{ + tlb_flush_page_bits_by_mmuidx_async_0(cpu, decode_runon_to_pbm(runon)); +} + +static void tlb_flush_page_bits_by_mmuidx_async_2(CPUState *cpu, + run_on_cpu_data data) +{ + TLBFlushPageBitsByMMUIdxData *d = data.host_ptr; + tlb_flush_page_bits_by_mmuidx_async_0(cpu, *d); + g_free(d); +} + +void tlb_flush_page_bits_by_mmuidx(CPUState *cpu, target_ulong addr, + uint16_t idxmap, unsigned bits) +{ + TLBFlushPageBitsByMMUIdxData d; + run_on_cpu_data runon; + + /* If all bits are significant, this devolves to tlb_flush_page. */ + if (bits >= TARGET_LONG_BITS) { + tlb_flush_page_by_mmuidx(cpu, addr, idxmap); + return; + } + /* If no page bits are significant, this devolves to tlb_flush. */ + if (bits < TARGET_PAGE_BITS) { + tlb_flush_by_mmuidx(cpu, idxmap); + return; + } + + /* This should already be page aligned */ + d.addr = addr & TARGET_PAGE_MASK; + d.idxmap = idxmap; + d.bits = bits; + + if (qemu_cpu_is_self(cpu)) { + tlb_flush_page_bits_by_mmuidx_async_0(cpu, d); + } else if (encode_pbm_to_runon(&runon, d)) { + async_run_on_cpu(cpu, tlb_flush_page_bits_by_mmuidx_async_1, runon); + } else { + TLBFlushPageBitsByMMUIdxData *p + = g_new(TLBFlushPageBitsByMMUIdxData, 1); + + /* Otherwise allocate a structure, freed by the worker. */ + *p = d; + async_run_on_cpu(cpu, tlb_flush_page_bits_by_mmuidx_async_2, + RUN_ON_CPU_HOST_PTR(p)); + } +} + +void tlb_flush_page_bits_by_mmuidx_all_cpus(CPUState *src_cpu, + target_ulong addr, + uint16_t idxmap, + unsigned bits) +{ + TLBFlushPageBitsByMMUIdxData d; + run_on_cpu_data runon; + + /* If all bits are significant, this devolves to tlb_flush_page. */ + if (bits >= TARGET_LONG_BITS) { + tlb_flush_page_by_mmuidx_all_cpus(src_cpu, addr, idxmap); + return; + } + /* If no page bits are significant, this devolves to tlb_flush. */ + if (bits < TARGET_PAGE_BITS) { + tlb_flush_by_mmuidx_all_cpus(src_cpu, idxmap); + return; + } + + /* This should already be page aligned */ + d.addr = addr & TARGET_PAGE_MASK; + d.idxmap = idxmap; + d.bits = bits; + + if (encode_pbm_to_runon(&runon, d)) { + flush_all_helper(src_cpu, tlb_flush_page_bits_by_mmuidx_async_1, runon); + } else { + CPUState *dst_cpu; + TLBFlushPageBitsByMMUIdxData *p; + + /* Allocate a separate data block for each destination cpu. */ + CPU_FOREACH(dst_cpu) { + if (dst_cpu != src_cpu) { + p = g_new(TLBFlushPageBitsByMMUIdxData, 1); + *p = d; + async_run_on_cpu(dst_cpu, + tlb_flush_page_bits_by_mmuidx_async_2, + RUN_ON_CPU_HOST_PTR(p)); + } + } + } + + tlb_flush_page_bits_by_mmuidx_async_0(src_cpu, d); +} + +void tlb_flush_page_bits_by_mmuidx_all_cpus_synced(CPUState *src_cpu, + target_ulong addr, + uint16_t idxmap, + unsigned bits) +{ + TLBFlushPageBitsByMMUIdxData d; + run_on_cpu_data runon; + + /* If all bits are significant, this devolves to tlb_flush_page. */ + if (bits >= TARGET_LONG_BITS) { + tlb_flush_page_by_mmuidx_all_cpus_synced(src_cpu, addr, idxmap); + return; + } + /* If no page bits are significant, this devolves to tlb_flush. */ + if (bits < TARGET_PAGE_BITS) { + tlb_flush_by_mmuidx_all_cpus_synced(src_cpu, idxmap); + return; + } + + /* This should already be page aligned */ + d.addr = addr & TARGET_PAGE_MASK; + d.idxmap = idxmap; + d.bits = bits; + + if (encode_pbm_to_runon(&runon, d)) { + flush_all_helper(src_cpu, tlb_flush_page_bits_by_mmuidx_async_1, runon); + async_safe_run_on_cpu(src_cpu, tlb_flush_page_bits_by_mmuidx_async_1, + runon); + } else { + CPUState *dst_cpu; + TLBFlushPageBitsByMMUIdxData *p; + + /* Allocate a separate data block for each destination cpu. */ + CPU_FOREACH(dst_cpu) { + if (dst_cpu != src_cpu) { + p = g_new(TLBFlushPageBitsByMMUIdxData, 1); + *p = d; + async_run_on_cpu(dst_cpu, tlb_flush_page_bits_by_mmuidx_async_2, + RUN_ON_CPU_HOST_PTR(p)); + } + } + + p = g_new(TLBFlushPageBitsByMMUIdxData, 1); + *p = d; + async_safe_run_on_cpu(src_cpu, tlb_flush_page_bits_by_mmuidx_async_2, + RUN_ON_CPU_HOST_PTR(p)); + } +} + /* update the TLBs so that writes to code in the virtual page 'addr' can be detected */ void tlb_protect_code(ram_addr_t ram_addr) -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> When TBI is enabled in a given regime, 56 bits of the address are significant and we need to clear out any other matching virtual addresses with differing tags. The other uses of tlb_flush_page (without mmuidx) in this file are only used by aarch32 mode. Fixes: 38d931687fa1 Reported-by: Jordan Frank <jordanfrank@fb.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201016210754.818257-3-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 46 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, #endif static void switch_mode(CPUARMState *env, int mode); +static int aa64_va_parameter_tbi(uint64_t tcr, ARMMMUIdx mmu_idx); static int vfp_gdb_get_reg(CPUARMState *env, GByteArray *buf, int reg) { @@ -XXX,XX +XXX,XX @@ static int vae1_tlbmask(CPUARMState *env) } } +/* Return 56 if TBI is enabled, 64 otherwise. */ +static int tlbbits_for_regime(CPUARMState *env, ARMMMUIdx mmu_idx, + uint64_t addr) +{ + uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr; + int tbi = aa64_va_parameter_tbi(tcr, mmu_idx); + int select = extract64(addr, 55, 1); + + return (tbi >> select) & 1 ? 56 : 64; +} + +static int vae1_tlbbits(CPUARMState *env, uint64_t addr) +{ + ARMMMUIdx mmu_idx; + + /* Only the regime of the mmu_idx below is significant. */ + if (arm_is_secure_below_el3(env)) { + mmu_idx = ARMMMUIdx_SE10_0; + } else if ((env->cp15.hcr_el2 & (HCR_E2H | HCR_TGE)) + == (HCR_E2H | HCR_TGE)) { + mmu_idx = ARMMMUIdx_E20_0; + } else { + mmu_idx = ARMMMUIdx_E10_0; + } + return tlbbits_for_regime(env, mmu_idx, addr); +} + static void tlbi_aa64_vmalle1is_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) { @@ -XXX,XX +XXX,XX @@ static void tlbi_aa64_vae1is_write(CPUARMState *env, const ARMCPRegInfo *ri, CPUState *cs = env_cpu(env); int mask = vae1_tlbmask(env); uint64_t pageaddr = sextract64(value << 12, 0, 56); + int bits = vae1_tlbbits(env, pageaddr); - tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, mask); + tlb_flush_page_bits_by_mmuidx_all_cpus_synced(cs, pageaddr, mask, bits); } static void tlbi_aa64_vae1_write(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static void tlbi_aa64_vae1_write(CPUARMState *env, const ARMCPRegInfo *ri, CPUState *cs = env_cpu(env); int mask = vae1_tlbmask(env); uint64_t pageaddr = sextract64(value << 12, 0, 56); + int bits = vae1_tlbbits(env, pageaddr); if (tlb_force_broadcast(env)) { - tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, mask); + tlb_flush_page_bits_by_mmuidx_all_cpus_synced(cs, pageaddr, mask, bits); } else { - tlb_flush_page_by_mmuidx(cs, pageaddr, mask); + tlb_flush_page_bits_by_mmuidx(cs, pageaddr, mask, bits); } } @@ -XXX,XX +XXX,XX @@ static void tlbi_aa64_vae2is_write(CPUARMState *env, const ARMCPRegInfo *ri, { CPUState *cs = env_cpu(env); uint64_t pageaddr = sextract64(value << 12, 0, 56); + int bits = tlbbits_for_regime(env, ARMMMUIdx_E2, pageaddr); - tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, - ARMMMUIdxBit_E2); + tlb_flush_page_bits_by_mmuidx_all_cpus_synced(cs, pageaddr, + ARMMMUIdxBit_E2, bits); } static void tlbi_aa64_vae3is_write(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ static void tlbi_aa64_vae3is_write(CPUARMState *env, const ARMCPRegInfo *ri, { CPUState *cs = env_cpu(env); uint64_t pageaddr = sextract64(value << 12, 0, 56); + int bits = tlbbits_for_regime(env, ARMMMUIdx_SE3, pageaddr); - tlb_flush_page_by_mmuidx_all_cpus_synced(cs, pageaddr, - ARMMMUIdxBit_SE3); + tlb_flush_page_bits_by_mmuidx_all_cpus_synced(cs, pageaddr, + ARMMMUIdxBit_SE3, bits); } static CPAccessResult aa64_zva_access(CPUARMState *env, const ARMCPRegInfo *ri, -- 2.20.1 From: Havard Skinnemoen <hskinnemoen@google.com> This test exercises the various modes of the npcm7xx timer. In particular, it triggers the bug found by the fuzzer, as reported here: https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg02992.html It also found several other bugs, especially related to interrupt handling. The test exercises all the timers in all the timer modules, which expands to 180 test cases in total. Reviewed-by: Tyrone Ting <kfting@nuvoton.com> Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com> Message-id: 20201008232154.94221-2-hskinnemoen@google.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/qtest/npcm7xx_timer-test.c | 562 +++++++++++++++++++++++++++++++ tests/qtest/meson.build | 1 + 2 files changed, 563 insertions(+) create mode 100644 tests/qtest/npcm7xx_timer-test.c diff --git a/tests/qtest/npcm7xx_timer-test.c b/tests/qtest/npcm7xx_timer-test.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/tests/qtest/npcm7xx_timer-test.c @@ -XXX,XX +XXX,XX @@ +/* + * QTest testcase for the Nuvoton NPCM7xx Timer + * + * Copyright 2020 Google LLC + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program 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 General Public License + * for more details. + */ + +#include "qemu/osdep.h" +#include "qemu/timer.h" +#include "libqtest-single.h" + +#define TIM_REF_HZ (25000000) + +/* Bits in TCSRx */ +#define CEN BIT(30) +#define IE BIT(29) +#define MODE_ONESHOT (0 << 27) +#define MODE_PERIODIC (1 << 27) +#define CRST BIT(26) +#define CACT BIT(25) +#define PRESCALE(x) (x) + +/* Registers shared between all timers in a module. */ +#define TISR 0x18 +#define WTCR 0x1c +# define WTCLK(x) ((x) << 10) + +/* Power-on default; used to re-initialize timers before each test. */ +#define TCSR_DEFAULT PRESCALE(5) + +/* Register offsets for a timer within a timer block. */ +typedef struct Timer { + unsigned int tcsr_offset; + unsigned int ticr_offset; + unsigned int tdr_offset; +} Timer; + +/* A timer block containing 5 timers. */ +typedef struct TimerBlock { + int irq_base; + uint64_t base_addr; +} TimerBlock; + +/* Testdata for testing a particular timer within a timer block. */ +typedef struct TestData { + const TimerBlock *tim; + const Timer *timer; +} TestData; + +const TimerBlock timer_block[] = { + { + .irq_base = 32, + .base_addr = 0xf0008000, + }, + { + .irq_base = 37, + .base_addr = 0xf0009000, + }, + { + .irq_base = 42, + .base_addr = 0xf000a000, + }, +}; + +const Timer timer[] = { + { + .tcsr_offset = 0x00, + .ticr_offset = 0x08, + .tdr_offset = 0x10, + }, { + .tcsr_offset = 0x04, + .ticr_offset = 0x0c, + .tdr_offset = 0x14, + }, { + .tcsr_offset = 0x20, + .ticr_offset = 0x28, + .tdr_offset = 0x30, + }, { + .tcsr_offset = 0x24, + .ticr_offset = 0x2c, + .tdr_offset = 0x34, + }, { + .tcsr_offset = 0x40, + .ticr_offset = 0x48, + .tdr_offset = 0x50, + }, +}; + +/* Returns the index of the timer block. */ +static int tim_index(const TimerBlock *tim) +{ + ptrdiff_t diff = tim - timer_block; + + g_assert(diff >= 0 && diff < ARRAY_SIZE(timer_block)); + + return diff; +} + +/* Returns the index of a timer within a timer block. */ +static int timer_index(const Timer *t) +{ + ptrdiff_t diff = t - timer; + + g_assert(diff >= 0 && diff < ARRAY_SIZE(timer)); + + return diff; +} + +/* Returns the irq line for a given timer. */ +static int tim_timer_irq(const TestData *td) +{ + return td->tim->irq_base + timer_index(td->timer); +} + +/* Register read/write accessors. */ + +static void tim_write(const TestData *td, + unsigned int offset, uint32_t value) +{ + writel(td->tim->base_addr + offset, value); +} + +static uint32_t tim_read(const TestData *td, unsigned int offset) +{ + return readl(td->tim->base_addr + offset); +} + +static void tim_write_tcsr(const TestData *td, uint32_t value) +{ + tim_write(td, td->timer->tcsr_offset, value); +} + +static uint32_t tim_read_tcsr(const TestData *td) +{ + return tim_read(td, td->timer->tcsr_offset); +} + +static void tim_write_ticr(const TestData *td, uint32_t value) +{ + tim_write(td, td->timer->ticr_offset, value); +} + +static uint32_t tim_read_ticr(const TestData *td) +{ + return tim_read(td, td->timer->ticr_offset); +} + +static uint32_t tim_read_tdr(const TestData *td) +{ + return tim_read(td, td->timer->tdr_offset); +} + +/* Returns the number of nanoseconds to count the given number of cycles. */ +static int64_t tim_calculate_step(uint32_t count, uint32_t prescale) +{ + return (1000000000LL / TIM_REF_HZ) * count * (prescale + 1); +} + +/* Returns a bitmask corresponding to the timer under test. */ +static uint32_t tim_timer_bit(const TestData *td) +{ + return BIT(timer_index(td->timer)); +} + +/* Resets all timers to power-on defaults. */ +static void tim_reset(const TestData *td) +{ + int i, j; + + /* Reset all the timers, in case a previous test left a timer running. */ + for (i = 0; i < ARRAY_SIZE(timer_block); i++) { + for (j = 0; j < ARRAY_SIZE(timer); j++) { + writel(timer_block[i].base_addr + timer[j].tcsr_offset, + CRST | TCSR_DEFAULT); + } + writel(timer_block[i].base_addr + TISR, -1); + } +} + +/* Verifies the reset state of a timer. */ +static void test_reset(gconstpointer test_data) +{ + const TestData *td = test_data; + + tim_reset(td); + + g_assert_cmphex(tim_read_tcsr(td), ==, TCSR_DEFAULT); + g_assert_cmphex(tim_read_ticr(td), ==, 0); + g_assert_cmphex(tim_read_tdr(td), ==, 0); + g_assert_cmphex(tim_read(td, TISR), ==, 0); + g_assert_cmphex(tim_read(td, WTCR), ==, WTCLK(1)); +} + +/* Verifies that CRST wins if both CEN and CRST are set. */ +static void test_reset_overrides_enable(gconstpointer test_data) +{ + const TestData *td = test_data; + + tim_reset(td); + + /* CRST should force CEN to 0 */ + tim_write_tcsr(td, CEN | CRST | TCSR_DEFAULT); + + g_assert_cmphex(tim_read_tcsr(td), ==, TCSR_DEFAULT); + g_assert_cmphex(tim_read_tdr(td), ==, 0); + g_assert_cmphex(tim_read(td, TISR), ==, 0); +} + +/* Verifies the behavior when CEN is set and then cleared. */ +static void test_oneshot_enable_then_disable(gconstpointer test_data) +{ + const TestData *td = test_data; + + tim_reset(td); + + /* Enable the timer with zero initial count, then disable it again. */ + tim_write_tcsr(td, CEN | TCSR_DEFAULT); + tim_write_tcsr(td, TCSR_DEFAULT); + + g_assert_cmphex(tim_read_tcsr(td), ==, TCSR_DEFAULT); + g_assert_cmphex(tim_read_tdr(td), ==, 0); + /* Timer interrupt flag should be set, but interrupts are not enabled. */ + g_assert_cmphex(tim_read(td, TISR), ==, tim_timer_bit(td)); + g_assert_false(qtest_get_irq(global_qtest, tim_timer_irq(td))); +} + +/* Verifies that a one-shot timer fires when expected with prescaler 5. */ +static void test_oneshot_ps5(gconstpointer test_data) +{ + const TestData *td = test_data; + unsigned int count = 256; + unsigned int ps = 5; + + tim_reset(td); + + tim_write_ticr(td, count); + tim_write_tcsr(td, CEN | PRESCALE(ps)); + g_assert_cmphex(tim_read_tcsr(td), ==, CEN | CACT | PRESCALE(ps)); + g_assert_cmpuint(tim_read_tdr(td), ==, count); + + clock_step(tim_calculate_step(count, ps) - 1); + + g_assert_cmphex(tim_read_tcsr(td), ==, CEN | CACT | PRESCALE(ps)); + g_assert_cmpuint(tim_read_tdr(td), <, count); + g_assert_cmphex(tim_read(td, TISR), ==, 0); + + clock_step(1); + + g_assert_cmphex(tim_read_tcsr(td), ==, PRESCALE(ps)); + g_assert_cmpuint(tim_read_tdr(td), ==, count); + g_assert_cmphex(tim_read(td, TISR), ==, tim_timer_bit(td)); + g_assert_false(qtest_get_irq(global_qtest, tim_timer_irq(td))); + + /* Clear the interrupt flag. */ + tim_write(td, TISR, tim_timer_bit(td)); + g_assert_cmphex(tim_read(td, TISR), ==, 0); + g_assert_false(qtest_get_irq(global_qtest, tim_timer_irq(td))); + + /* Verify that this isn't a periodic timer. */ + clock_step(2 * tim_calculate_step(count, ps)); + g_assert_cmphex(tim_read(td, TISR), ==, 0); + g_assert_false(qtest_get_irq(global_qtest, tim_timer_irq(td))); +} + +/* Verifies that a one-shot timer fires when expected with prescaler 0. */ +static void test_oneshot_ps0(gconstpointer test_data) +{ + const TestData *td = test_data; + unsigned int count = 1; + unsigned int ps = 0; + + tim_reset(td); + + tim_write_ticr(td, count); + tim_write_tcsr(td, CEN | PRESCALE(ps)); + g_assert_cmphex(tim_read_tcsr(td), ==, CEN | CACT | PRESCALE(ps)); + g_assert_cmpuint(tim_read_tdr(td), ==, count); + + clock_step(tim_calculate_step(count, ps) - 1); + + g_assert_cmphex(tim_read_tcsr(td), ==, CEN | CACT | PRESCALE(ps)); + g_assert_cmpuint(tim_read_tdr(td), <, count); + g_assert_cmphex(tim_read(td, TISR), ==, 0); + + clock_step(1); + + g_assert_cmphex(tim_read_tcsr(td), ==, PRESCALE(ps)); + g_assert_cmpuint(tim_read_tdr(td), ==, count); + g_assert_cmphex(tim_read(td, TISR), ==, tim_timer_bit(td)); + g_assert_false(qtest_get_irq(global_qtest, tim_timer_irq(td))); +} + +/* Verifies that a one-shot timer fires when expected with highest prescaler. */ +static void test_oneshot_ps255(gconstpointer test_data) +{ + const TestData *td = test_data; + unsigned int count = (1U << 24) - 1; + unsigned int ps = 255; + + tim_reset(td); + + tim_write_ticr(td, count); + tim_write_tcsr(td, CEN | PRESCALE(ps)); + g_assert_cmphex(tim_read_tcsr(td), ==, CEN | CACT | PRESCALE(ps)); + g_assert_cmpuint(tim_read_tdr(td), ==, count); + + clock_step(tim_calculate_step(count, ps) - 1); + + g_assert_cmphex(tim_read_tcsr(td), ==, CEN | CACT | PRESCALE(ps)); + g_assert_cmpuint(tim_read_tdr(td), <, count); + g_assert_cmphex(tim_read(td, TISR), ==, 0); + + clock_step(1); + + g_assert_cmphex(tim_read_tcsr(td), ==, PRESCALE(ps)); + g_assert_cmpuint(tim_read_tdr(td), ==, count); + g_assert_cmphex(tim_read(td, TISR), ==, tim_timer_bit(td)); + g_assert_false(qtest_get_irq(global_qtest, tim_timer_irq(td))); +} + +/* Verifies that a oneshot timer fires an interrupt when expected. */ +static void test_oneshot_interrupt(gconstpointer test_data) +{ + const TestData *td = test_data; + unsigned int count = 256; + unsigned int ps = 7; + + tim_reset(td); + + tim_write_ticr(td, count); + tim_write_tcsr(td, IE | CEN | MODE_ONESHOT | PRESCALE(ps)); + + clock_step_next(); + + g_assert_cmphex(tim_read(td, TISR), ==, tim_timer_bit(td)); + g_assert_true(qtest_get_irq(global_qtest, tim_timer_irq(td))); +} + +/* + * Verifies that the timer can be paused and later resumed, and it still fires + * at the right moment. + */ +static void test_pause_resume(gconstpointer test_data) +{ + const TestData *td = test_data; + unsigned int count = 256; + unsigned int ps = 1; + + tim_reset(td); + + tim_write_ticr(td, count); + tim_write_tcsr(td, IE | CEN | MODE_ONESHOT | PRESCALE(ps)); + + /* Pause the timer halfway to expiration. */ + clock_step(tim_calculate_step(count / 2, ps)); + tim_write_tcsr(td, IE | MODE_ONESHOT | PRESCALE(ps)); + g_assert_cmpuint(tim_read_tdr(td), ==, count / 2); + + /* Counter should not advance during the following step. */ + clock_step(2 * tim_calculate_step(count, ps)); + g_assert_cmpuint(tim_read_tdr(td), ==, count / 2); + g_assert_cmphex(tim_read(td, TISR), ==, 0); + g_assert_false(qtest_get_irq(global_qtest, tim_timer_irq(td))); + + /* Resume the timer and run _almost_ to expiration. */ + tim_write_tcsr(td, IE | CEN | MODE_ONESHOT | PRESCALE(ps)); + clock_step(tim_calculate_step(count / 2, ps) - 1); + g_assert_cmpuint(tim_read_tdr(td), <, count); + g_assert_cmphex(tim_read(td, TISR), ==, 0); + g_assert_false(qtest_get_irq(global_qtest, tim_timer_irq(td))); + + /* Now, run the rest of the way and verify that the interrupt fires. */ + clock_step(1); + g_assert_cmphex(tim_read(td, TISR), ==, tim_timer_bit(td)); + g_assert_true(qtest_get_irq(global_qtest, tim_timer_irq(td))); +} + +/* Verifies that the prescaler can be changed while the timer is runnin. */ +static void test_prescaler_change(gconstpointer test_data) +{ + const TestData *td = test_data; + unsigned int count = 256; + unsigned int ps = 5; + + tim_reset(td); + + tim_write_ticr(td, count); + tim_write_tcsr(td, CEN | MODE_ONESHOT | PRESCALE(ps)); + + /* Run a quarter of the way, and change the prescaler. */ + clock_step(tim_calculate_step(count / 4, ps)); + g_assert_cmpuint(tim_read_tdr(td), ==, 3 * count / 4); + ps = 2; + tim_write_tcsr(td, CEN | MODE_ONESHOT | PRESCALE(ps)); + /* The counter must not change. */ + g_assert_cmpuint(tim_read_tdr(td), ==, 3 * count / 4); + + /* Run another quarter of the way, and change the prescaler again. */ + clock_step(tim_calculate_step(count / 4, ps)); + g_assert_cmpuint(tim_read_tdr(td), ==, count / 2); + ps = 8; + tim_write_tcsr(td, CEN | MODE_ONESHOT | PRESCALE(ps)); + /* The counter must not change. */ + g_assert_cmpuint(tim_read_tdr(td), ==, count / 2); + + /* Run another quarter of the way, and change the prescaler again. */ + clock_step(tim_calculate_step(count / 4, ps)); + g_assert_cmpuint(tim_read_tdr(td), ==, count / 4); + ps = 0; + tim_write_tcsr(td, CEN | MODE_ONESHOT | PRESCALE(ps)); + /* The counter must not change. */ + g_assert_cmpuint(tim_read_tdr(td), ==, count / 4); + + /* Run almost to expiration, and verify the timer didn't fire yet. */ + clock_step(tim_calculate_step(count / 4, ps) - 1); + g_assert_cmpuint(tim_read_tdr(td), <, count); + g_assert_cmphex(tim_read(td, TISR), ==, 0); + + /* Now, run the rest of the way and verify that the timer fires. */ + clock_step(1); + g_assert_cmphex(tim_read(td, TISR), ==, tim_timer_bit(td)); +} + +/* Verifies that a periodic timer automatically restarts after expiration. */ +static void test_periodic_no_interrupt(gconstpointer test_data) +{ + const TestData *td = test_data; + unsigned int count = 2; + unsigned int ps = 3; + int i; + + tim_reset(td); + + tim_write_ticr(td, count); + tim_write_tcsr(td, CEN | MODE_PERIODIC | PRESCALE(ps)); + + for (i = 0; i < 4; i++) { + clock_step_next(); + + g_assert_cmphex(tim_read(td, TISR), ==, tim_timer_bit(td)); + g_assert_false(qtest_get_irq(global_qtest, tim_timer_irq(td))); + + tim_write(td, TISR, tim_timer_bit(td)); + + g_assert_cmphex(tim_read(td, TISR), ==, 0); + g_assert_false(qtest_get_irq(global_qtest, tim_timer_irq(td))); + } +} + +/* Verifies that a periodict timer fires an interrupt every time it expires. */ +static void test_periodic_interrupt(gconstpointer test_data) +{ + const TestData *td = test_data; + unsigned int count = 65535; + unsigned int ps = 2; + int i; + + tim_reset(td); + + tim_write_ticr(td, count); + tim_write_tcsr(td, CEN | IE | MODE_PERIODIC | PRESCALE(ps)); + + for (i = 0; i < 4; i++) { + clock_step_next(); + + g_assert_cmphex(tim_read(td, TISR), ==, tim_timer_bit(td)); + g_assert_true(qtest_get_irq(global_qtest, tim_timer_irq(td))); + + tim_write(td, TISR, tim_timer_bit(td)); + + g_assert_cmphex(tim_read(td, TISR), ==, 0); + g_assert_false(qtest_get_irq(global_qtest, tim_timer_irq(td))); + } +} + +/* + * Verifies that the timer behaves correctly when disabled right before and + * exactly when it's supposed to expire. + */ +static void test_disable_on_expiration(gconstpointer test_data) +{ + const TestData *td = test_data; + unsigned int count = 8; + unsigned int ps = 255; + + tim_reset(td); + + tim_write_ticr(td, count); + tim_write_tcsr(td, CEN | MODE_ONESHOT | PRESCALE(ps)); + + clock_step(tim_calculate_step(count, ps) - 1); + + tim_write_tcsr(td, MODE_ONESHOT | PRESCALE(ps)); + tim_write_tcsr(td, CEN | MODE_ONESHOT | PRESCALE(ps)); + clock_step(1); + tim_write_tcsr(td, MODE_ONESHOT | PRESCALE(ps)); + g_assert_cmphex(tim_read(td, TISR), ==, tim_timer_bit(td)); +} + +/* + * Constructs a name that includes the timer block, timer and testcase name, + * and adds the test to the test suite. + */ +static void tim_add_test(const char *name, const TestData *td, GTestDataFunc fn) +{ + g_autofree char *full_name; + + full_name = g_strdup_printf("npcm7xx_timer/tim[%d]/timer[%d]/%s", + tim_index(td->tim), timer_index(td->timer), + name); + qtest_add_data_func(full_name, td, fn); +} + +/* Convenience macro for adding a test with a predictable function name. */ +#define add_test(name, td) tim_add_test(#name, td, test_##name) + +int main(int argc, char **argv) +{ + TestData testdata[ARRAY_SIZE(timer_block) * ARRAY_SIZE(timer)]; + int ret; + int i, j; + + g_test_init(&argc, &argv, NULL); + g_test_set_nonfatal_assertions(); + + for (i = 0; i < ARRAY_SIZE(timer_block); i++) { + for (j = 0; j < ARRAY_SIZE(timer); j++) { + TestData *td = &testdata[i * ARRAY_SIZE(timer) + j]; + td->tim = &timer_block[i]; + td->timer = &timer[j]; + + add_test(reset, td); + add_test(reset_overrides_enable, td); + add_test(oneshot_enable_then_disable, td); + add_test(oneshot_ps5, td); + add_test(oneshot_ps0, td); + add_test(oneshot_ps255, td); + add_test(oneshot_interrupt, td); + add_test(pause_resume, td); + add_test(prescaler_change, td); + add_test(periodic_no_interrupt, td); + add_test(periodic_interrupt, td); + add_test(disable_on_expiration, td); + } + } + + qtest_start("-machine npcm750-evb"); + qtest_irq_intercept_in(global_qtest, "/machine/soc/a9mpcore/gic"); + ret = g_test_run(); + qtest_end(); + + return ret; +} diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -XXX,XX +XXX,XX @@ qtests_arm = \ ['arm-cpu-features', 'microbit-test', 'm25p80-test', + 'npcm7xx_timer-test', 'test-arm-mptimer', 'boot-serial-test', 'hexloader-test'] -- 2.20.1 From: Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com> Current documentation is not too clear on the GETPC usage. In particular, when used outside the top level helper function it causes unexpected behavior. Signed-off-by: Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com> Message-id: 20201015095147.1691-1-e.emanuelegiuseppe@gmail.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- docs/devel/loads-stores.rst | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/devel/loads-stores.rst b/docs/devel/loads-stores.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/devel/loads-stores.rst +++ b/docs/devel/loads-stores.rst @@ -XXX,XX +XXX,XX @@ guest CPU state in case of a guest CPU exception. This is passed to ``cpu_restore_state()``. Therefore the value should either be 0, to indicate that the guest CPU state is already synchronized, or the result of ``GETPC()`` from the top level ``HELPER(foo)`` -function, which is a return address into the generated code. +function, which is a return address into the generated code [#gpc]_. + +.. [#gpc] Note that ``GETPC()`` should be used with great care: calling + it in other functions that are *not* the top level + ``HELPER(foo)`` will cause unexpected behavior. Instead, the + value of ``GETPC()`` should be read from the helper and passed + if needed to the functions that the helper calls. Function names follow the pattern: -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Add trace events for GPU and CPU IRQs. Reviewed-by: Luc Michel <luc.michel@greensocs.com> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201017180731.1165871-2-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/bcm2835_ic.c | 4 +++- hw/intc/trace-events | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/hw/intc/bcm2835_ic.c b/hw/intc/bcm2835_ic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/bcm2835_ic.c +++ b/hw/intc/bcm2835_ic.c @@ -XXX,XX +XXX,XX @@ #include "migration/vmstate.h" #include "qemu/log.h" #include "qemu/module.h" +#include "trace.h" #define GPU_IRQS 64 #define ARM_IRQS 8 @@ -XXX,XX +XXX,XX @@ static void bcm2835_ic_update(BCM2835ICState *s) set = (s->gpu_irq_level & s->gpu_irq_enable) || (s->arm_irq_level & s->arm_irq_enable); qemu_set_irq(s->irq, set); - } static void bcm2835_ic_set_gpu_irq(void *opaque, int irq, int level) @@ -XXX,XX +XXX,XX @@ static void bcm2835_ic_set_gpu_irq(void *opaque, int irq, int level) BCM2835ICState *s = opaque; assert(irq >= 0 && irq < 64); + trace_bcm2835_ic_set_gpu_irq(irq, level); s->gpu_irq_level = deposit64(s->gpu_irq_level, irq, 1, level != 0); bcm2835_ic_update(s); } @@ -XXX,XX +XXX,XX @@ static void bcm2835_ic_set_arm_irq(void *opaque, int irq, int level) BCM2835ICState *s = opaque; assert(irq >= 0 && irq < 8); + trace_bcm2835_ic_set_cpu_irq(irq, level); s->arm_irq_level = deposit32(s->arm_irq_level, irq, 1, level != 0); bcm2835_ic_update(s); } diff --git a/hw/intc/trace-events b/hw/intc/trace-events index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/trace-events +++ b/hw/intc/trace-events @@ -XXX,XX +XXX,XX @@ nvic_sysreg_write(uint64_t addr, uint32_t value, unsigned size) "NVIC sysreg wri heathrow_write(uint64_t addr, unsigned int n, uint64_t value) "0x%"PRIx64" %u: 0x%"PRIx64 heathrow_read(uint64_t addr, unsigned int n, uint64_t value) "0x%"PRIx64" %u: 0x%"PRIx64 heathrow_set_irq(int num, int level) "set_irq: num=0x%02x level=%d" + +# bcm2835_ic.c +bcm2835_ic_set_gpu_irq(int irq, int level) "GPU irq #%d level %d" +bcm2835_ic_set_cpu_irq(int irq, int level) "CPU irq #%d level %d" -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> The IRQ values are defined few lines earlier, use them instead of the magic numbers. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201017180731.1165871-3-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/bcm2836_control.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/intc/bcm2836_control.c b/hw/intc/bcm2836_control.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/bcm2836_control.c +++ b/hw/intc/bcm2836_control.c @@ -XXX,XX +XXX,XX @@ static void bcm2836_control_set_local_irq(void *opaque, int core, int local_irq, static void bcm2836_control_set_local_irq0(void *opaque, int core, int level) { - bcm2836_control_set_local_irq(opaque, core, 0, level); + bcm2836_control_set_local_irq(opaque, core, IRQ_CNTPSIRQ, level); } static void bcm2836_control_set_local_irq1(void *opaque, int core, int level) { - bcm2836_control_set_local_irq(opaque, core, 1, level); + bcm2836_control_set_local_irq(opaque, core, IRQ_CNTPNSIRQ, level); } static void bcm2836_control_set_local_irq2(void *opaque, int core, int level) { - bcm2836_control_set_local_irq(opaque, core, 2, level); + bcm2836_control_set_local_irq(opaque, core, IRQ_CNTHPIRQ, level); } static void bcm2836_control_set_local_irq3(void *opaque, int core, int level) { - bcm2836_control_set_local_irq(opaque, core, 3, level); + bcm2836_control_set_local_irq(opaque, core, IRQ_CNTVIRQ, level); } static void bcm2836_control_set_gpu_irq(void *opaque, int irq, int level) -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> We already have the full ARMMMUIdx as computed from the function parameter. For the purpose of regime_has_2_ranges, we can ignore any difference between AccType_Normal and AccType_Unpriv, which would be the only difference between the passed mmu_idx and arm_mmu_idx_el. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com> Tested-by: Vincenzo Frascino <vincenzo.frascino@arm.com> Message-id: 20201008162155.161886-2-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/mte_helper.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/mte_helper.c +++ b/target/arm/mte_helper.c @@ -XXX,XX +XXX,XX @@ static void mte_check_fail(CPUARMState *env, uint32_t desc, case 2: /* Tag check fail causes asynchronous flag set. */ - mmu_idx = arm_mmu_idx_el(env, el); - if (regime_has_2_ranges(mmu_idx)) { + if (regime_has_2_ranges(arm_mmu_idx)) { select = extract64(dirty_ptr, 55, 1); } else { select = 0; -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> The reporting in AArch64.TagCheckFail only depends on PSTATE.EL, and not the AccType of the operation. There are two guest visible problems that affect LDTR and STTR because of this: (1) Selecting TCF0 vs TCF1 to decide on reporting, (2) Report "data abort same el" not "data abort lower el". Reported-by: Vincenzo Frascino <vincenzo.frascino@arm.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com> Tested-by: Vincenzo Frascino <vincenzo.frascino@arm.com> Message-id: 20201008162155.161886-3-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/mte_helper.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/mte_helper.c +++ b/target/arm/mte_helper.c @@ -XXX,XX +XXX,XX @@ static void mte_check_fail(CPUARMState *env, uint32_t desc, reg_el = regime_el(env, arm_mmu_idx); sctlr = env->cp15.sctlr_el[reg_el]; - switch (arm_mmu_idx) { - case ARMMMUIdx_E10_0: - case ARMMMUIdx_E20_0: - el = 0; + el = arm_current_el(env); + if (el == 0) { tcf = extract64(sctlr, 38, 2); - break; - default: - el = reg_el; + } else { tcf = extract64(sctlr, 40, 2); } -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> Unlike many other bits in HCR_EL2, the description for this bit does not contain the phrase "if ... this field behaves as 0 for all purposes other than", so do not squash the bit in arm_hcr_el2_eff. Instead, replicate the E2H+TGE test in the two places that require it. Reported-by: Vincenzo Frascino <vincenzo.frascino@arm.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com> Tested-by: Vincenzo Frascino <vincenzo.frascino@arm.com> Message-id: 20201008162155.161886-4-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/internals.h | 9 +++++---- target/arm/helper.c | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ static inline bool allocation_tag_access_enabled(CPUARMState *env, int el, && !(env->cp15.scr_el3 & SCR_ATA)) { return false; } - if (el < 2 - && arm_feature(env, ARM_FEATURE_EL2) - && !(arm_hcr_el2_eff(env) & HCR_ATA)) { - return false; + if (el < 2 && arm_feature(env, ARM_FEATURE_EL2)) { + uint64_t hcr = arm_hcr_el2_eff(env); + if (!(hcr & HCR_ATA) && (!(hcr & HCR_E2H) || !(hcr & HCR_TGE))) { + return false; + } } sctlr &= (el == 0 ? SCTLR_ATA0 : SCTLR_ATA); return sctlr != 0; diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static CPAccessResult access_mte(CPUARMState *env, const ARMCPRegInfo *ri, { int el = arm_current_el(env); - if (el < 2 && - arm_feature(env, ARM_FEATURE_EL2) && - !(arm_hcr_el2_eff(env) & HCR_ATA)) { - return CP_ACCESS_TRAP_EL2; + if (el < 2 && arm_feature(env, ARM_FEATURE_EL2)) { + uint64_t hcr = arm_hcr_el2_eff(env); + if (!(hcr & HCR_ATA) && (!(hcr & HCR_E2H) || !(hcr & HCR_TGE))) { + return CP_ACCESS_TRAP_EL2; + } } if (el < 3 && arm_feature(env, ARM_FEATURE_EL3) && -- 2.20.1 From: Peng Liang <liangpeng10@huawei.com> VMStateDescription.fields should be end with VMSTATE_END_OF_LIST(). However, microbit_i2c_vmstate doesn't follow it. Let's change it. Fixes: 9d68bf564e ("arm: Stub out NRF51 TWI magnetometer/accelerometer detection") Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Peng Liang <liangpeng10@huawei.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-id: 20201019093401.2993833-1-liangpeng10@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/i2c/microbit_i2c.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/i2c/microbit_i2c.c b/hw/i2c/microbit_i2c.c index XXXXXXX..XXXXXXX 100644 --- a/hw/i2c/microbit_i2c.c +++ b/hw/i2c/microbit_i2c.c @@ -XXX,XX +XXX,XX @@ static const VMStateDescription microbit_i2c_vmstate = { .fields = (VMStateField[]) { VMSTATE_UINT32_ARRAY(regs, MicrobitI2CState, MICROBIT_I2C_NREGS), VMSTATE_UINT32(read_idx, MicrobitI2CState), + VMSTATE_END_OF_LIST() }, }; -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Commit 7998beb9c2e removed the ram_size initialization in the arm_boot_info structure, however it is used by arm_load_kernel(). Initialize the field to fix: $ qemu-system-arm -M n800 -append 'console=ttyS1' \ -kernel meego-arm-n8x0-1.0.80.20100712.1431-vmlinuz-2.6.35~rc4-129.1-n8x0 qemu-system-arm: kernel 'meego-arm-n8x0-1.0.80.20100712.1431-vmlinuz-2.6.35~rc4-129.1-n8x0' is too large to fit in RAM (kernel size 1964608, RAM size 0) Noticed while running the test introduced in commit 050a82f0c5b ("tests/acceptance: Add a test for the N800 and N810 arm machines"). Fixes: 7998beb9c2e ("arm/nseries: use memdev for RAM") Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: Thomas Huth <thuth@redhat.com> Message-id: 20201019095148.1602119-1-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/nseries.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/nseries.c +++ b/hw/arm/nseries.c @@ -XXX,XX +XXX,XX @@ static void n8x0_init(MachineState *machine, g_free(sz); exit(EXIT_FAILURE); } + binfo->ram_size = machine->ram_size; memory_region_add_subregion(get_system_memory(), OMAP2_Q2_BASE, machine->ram); -- 2.20.1 For nested groups like: { [ pattern 1 pattern 2 ] pattern 3 } the intended behaviour is that patterns 1 and 2 must not overlap with each other; if the insn matches neither then we fall through to pattern 3 as the next thing in the outer overlapping group. Currently we generate incorrect code for this situation, because in the code path for a failed match inside the inner non-overlapping group we generate a "return" statement, which causes decode to stop entirely rather than continuing to the next thing in the outer group. Generate a "break" instead, so that decode flow behaves as required for this nested group case. Suggested-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20201019151301.2046-2-peter.maydell@linaro.org --- scripts/decodetree.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/decodetree.py b/scripts/decodetree.py index XXXXXXX..XXXXXXX 100644 --- a/scripts/decodetree.py +++ b/scripts/decodetree.py @@ -XXX,XX +XXX,XX @@ class Tree: output(ind, ' /* ', str_match_bits(innerbits, innermask), ' */\n') s.output_code(i + 4, extracted, innerbits, innermask) - output(ind, ' return false;\n') + output(ind, ' break;\n') output(ind, '}\n') # end Tree -- 2.20.1 From v8.1M, disabled-coprocessor handling changes slightly: * coprocessors 8, 9, 14 and 15 are also governed by the cp10 enable bit, like cp11 * an extra range of instruction patterns is considered to be inside the coprocessor space We previously marked these up with TODO comments; implement the correct behaviour. Unfortunately there is no ID register field which indicates this behaviour. We could in theory test an unrelated ID register which indicates guaranteed-to-be-in-v8.1M behaviour like ID_ISAR0.CmpBranch >= 3 (low-overhead-loops), but it seems better to simply define a new ARM_FEATURE_V8_1M feature flag and use it for this and other new-in-v8.1M behaviour that isn't identifiable from the ID registers. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201019151301.2046-3-peter.maydell@linaro.org --- target/arm/cpu.h | 1 + target/arm/m-nocp.decode | 10 ++++++---- target/arm/translate-vfp.c.inc | 17 +++++++++++++++-- 3 files changed, 22 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 @@ enum arm_features { ARM_FEATURE_VBAR, /* has cp15 VBAR */ ARM_FEATURE_M_SECURITY, /* M profile Security Extension */ ARM_FEATURE_M_MAIN, /* M profile Main Extension */ + ARM_FEATURE_V8_1M, /* M profile extras only in v8.1M and later */ }; static inline int arm_feature(CPUARMState *env, int feature) diff --git a/target/arm/m-nocp.decode b/target/arm/m-nocp.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/m-nocp.decode +++ b/target/arm/m-nocp.decode @@ -XXX,XX +XXX,XX @@ # If the coprocessor is not present or disabled then we will generate # the NOCP exception; otherwise we let the insn through to the main decode. +&nocp cp + { # Special cases which do not take an early NOCP: VLLDM and VLSTM VLLDM_VLSTM 1110 1100 001 l:1 rn:4 0000 1010 0000 0000 # TODO: VSCCLRM (new in v8.1M) is similar: #VSCCLRM 1110 1100 1-01 1111 ---- 1011 ---- ---0 - NOCP 111- 1110 ---- ---- ---- cp:4 ---- ---- - NOCP 111- 110- ---- ---- ---- cp:4 ---- ---- - # TODO: From v8.1M onwards we will also want this range to NOCP - #NOCP_8_1 111- 1111 ---- ---- ---- ---- ---- ---- cp=10 + NOCP 111- 1110 ---- ---- ---- cp:4 ---- ---- &nocp + NOCP 111- 110- ---- ---- ---- cp:4 ---- ---- &nocp + # From v8.1M onwards this range will also NOCP: + NOCP_8_1 111- 1111 ---- ---- ---- ---- ---- ---- &nocp cp=10 } diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-vfp.c.inc +++ b/target/arm/translate-vfp.c.inc @@ -XXX,XX +XXX,XX @@ static bool trans_VLLDM_VLSTM(DisasContext *s, arg_VLLDM_VLSTM *a) return true; } -static bool trans_NOCP(DisasContext *s, arg_NOCP *a) +static bool trans_NOCP(DisasContext *s, arg_nocp *a) { /* * Handle M-profile early check for disabled coprocessor: @@ -XXX,XX +XXX,XX @@ static bool trans_NOCP(DisasContext *s, arg_NOCP *a) if (a->cp == 11) { a->cp = 10; } - /* TODO: in v8.1M cp 8, 9, 14, 15 also are governed by the cp10 enable */ + if (arm_dc_feature(s, ARM_FEATURE_V8_1M) && + (a->cp == 8 || a->cp == 9 || a->cp == 14 || a->cp == 15)) { + /* in v8.1M cp 8, 9, 14, 15 also are governed by the cp10 enable */ + a->cp = 10; + } if (a->cp != 10) { gen_exception_insn(s, s->pc_curr, EXCP_NOCP, @@ -XXX,XX +XXX,XX @@ static bool trans_NOCP(DisasContext *s, arg_NOCP *a) return false; } +static bool trans_NOCP_8_1(DisasContext *s, arg_nocp *a) +{ + /* This range needs a coprocessor check for v8.1M and later only */ + if (!arm_dc_feature(s, ARM_FEATURE_V8_1M)) { + return false; + } + return trans_NOCP(s, a); +} + static bool trans_VINS(DisasContext *s, arg_VINS *a) { TCGv_i32 rd, rm; -- 2.20.1 v8.1M brings four new insns to M-profile: * CSEL : Rd = cond ? Rn : Rm * CSINC : Rd = cond ? Rn : Rm+1 * CSINV : Rd = cond ? Rn : ~Rm * CSNEG : Rd = cond ? Rn : -Rm Implement these. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20201019151301.2046-4-peter.maydell@linaro.org --- target/arm/t32.decode | 3 +++ target/arm/translate.c | 60 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/target/arm/t32.decode b/target/arm/t32.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/t32.decode +++ b/target/arm/t32.decode @@ -XXX,XX +XXX,XX @@ SBC_rrri 1110101 1011 . .... 0 ... .... .... .... @s_rrr_shi } RSB_rrri 1110101 1110 . .... 0 ... .... .... .... @s_rrr_shi +# v8.1M CSEL and friends +CSEL 1110101 0010 1 rn:4 10 op:2 rd:4 fcond:4 rm:4 + # Data-processing (register-shifted register) MOV_rxrr 1111 1010 0 shty:2 s:1 rm:4 1111 rd:4 0000 rs:4 \ diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static bool trans_IT(DisasContext *s, arg_IT *a) return true; } +/* v8.1M CSEL/CSINC/CSNEG/CSINV */ +static bool trans_CSEL(DisasContext *s, arg_CSEL *a) +{ + TCGv_i32 rn, rm, zero; + DisasCompare c; + + if (!arm_dc_feature(s, ARM_FEATURE_V8_1M)) { + return false; + } + + if (a->rm == 13) { + /* SEE "Related encodings" (MVE shifts) */ + return false; + } + + if (a->rd == 13 || a->rd == 15 || a->rn == 13 || a->fcond >= 14) { + /* CONSTRAINED UNPREDICTABLE: we choose to UNDEF */ + return false; + } + + /* In this insn input reg fields of 0b1111 mean "zero", not "PC" */ + if (a->rn == 15) { + rn = tcg_const_i32(0); + } else { + rn = load_reg(s, a->rn); + } + if (a->rm == 15) { + rm = tcg_const_i32(0); + } else { + rm = load_reg(s, a->rm); + } + + switch (a->op) { + case 0: /* CSEL */ + break; + case 1: /* CSINC */ + tcg_gen_addi_i32(rm, rm, 1); + break; + case 2: /* CSINV */ + tcg_gen_not_i32(rm, rm); + break; + case 3: /* CSNEG */ + tcg_gen_neg_i32(rm, rm); + break; + default: + g_assert_not_reached(); + } + + arm_test_cc(&c, a->fcond); + zero = tcg_const_i32(0); + tcg_gen_movcond_i32(c.cond, rn, c.value, zero, rn, rm); + arm_free_cc(&c); + tcg_temp_free_i32(zero); + + store_reg(s, a->rd, rn); + tcg_temp_free_i32(rm); + + return true; +} + /* * Legacy decoder. */ -- 2.20.1 The t32 decode has a group which represents a set of insns which overlap with B_cond_thumb because they have [25:23]=111 (which is an invalid condition code field for the branch insn). This group is currently defined using the {} overlap-OK syntax, but it is almost entirely non-overlapping patterns. Switch it over to use a non-overlapping group. For this to be valid syntactically, CPS must move into the same overlapping-group as the hint insns (CPS vs hints was the only actual use of the overlap facility for the group). The non-overlapping subgroup for CLREX/DSB/DMB/ISB/SB is no longer necessary and so we can remove it (promoting those insns to be members of the parent group). Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20201019151301.2046-5-peter.maydell@linaro.org --- target/arm/t32.decode | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/target/arm/t32.decode b/target/arm/t32.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/t32.decode +++ b/target/arm/t32.decode @@ -XXX,XX +XXX,XX @@ CLZ 1111 1010 1011 ---- 1111 .... 1000 .... @rdm { # Group insn[25:23] = 111, which is cond=111x for the branch below, # or unconditional, which would be illegal for the branch. - { - # Hints + [ + # Hints, and CPS { YIELD 1111 0011 1010 1111 1000 0000 0000 0001 WFE 1111 0011 1010 1111 1000 0000 0000 0010 @@ -XXX,XX +XXX,XX @@ CLZ 1111 1010 1011 ---- 1111 .... 1000 .... @rdm # The canonical nop ends in 0000 0000, but the whole rest # of the space is "reserved hint, behaves as nop". NOP 1111 0011 1010 1111 1000 0000 ---- ---- + + # If imod == '00' && M == '0' then SEE "Hint instructions", above. + CPS 1111 0011 1010 1111 1000 0 imod:2 M:1 A:1 I:1 F:1 mode:5 \ + &cps } - # If imod == '00' && M == '0' then SEE "Hint instructions", above. - CPS 1111 0011 1010 1111 1000 0 imod:2 M:1 A:1 I:1 F:1 mode:5 \ - &cps - # Miscellaneous control - [ - CLREX 1111 0011 1011 1111 1000 1111 0010 1111 - DSB 1111 0011 1011 1111 1000 1111 0100 ---- - DMB 1111 0011 1011 1111 1000 1111 0101 ---- - ISB 1111 0011 1011 1111 1000 1111 0110 ---- - SB 1111 0011 1011 1111 1000 1111 0111 0000 - ] + CLREX 1111 0011 1011 1111 1000 1111 0010 1111 + DSB 1111 0011 1011 1111 1000 1111 0100 ---- + DMB 1111 0011 1011 1111 1000 1111 0101 ---- + ISB 1111 0011 1011 1111 1000 1111 0110 ---- + SB 1111 0011 1011 1111 1000 1111 0111 0000 # Note that the v7m insn overlaps both the normal and banked insn. { @@ -XXX,XX +XXX,XX @@ CLZ 1111 1010 1011 ---- 1111 .... 1000 .... @rdm HVC 1111 0111 1110 .... 1000 .... .... .... \ &i imm=%imm16_16_0 UDF 1111 0111 1111 ---- 1010 ---- ---- ---- - } + ] B_cond_thumb 1111 0. cond:4 ...... 10.0 ............ &ci imm=%imm21 } -- 2.20.1 The BLX immediate insn in the Thumb encoding always performs a switch from Thumb to Arm state. This would be totally useless in M-profile which has no Arm decoder, and so the instruction does not exist at all there. Make the encoding UNDEF for M-profile. (This part of the encoding space is used for the branch-future and low-overhead-loop insns in v8.1M.) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20201019151301.2046-6-peter.maydell@linaro.org --- target/arm/translate.c | 8 ++++++++ 1 file changed, 8 insertions(+) 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 bool trans_BLX_i(DisasContext *s, arg_BLX_i *a) { TCGv_i32 tmp; + /* + * BLX <imm> would be useless on M-profile; the encoding space + * is used for other insns from v8.1M onward, and UNDEFs before that. + */ + if (arm_dc_feature(s, ARM_FEATURE_M)) { + return false; + } + /* For A32, ARM_FEATURE_V5 is checked near the start of the uncond block. */ if (s->thumb && (a->imm & 2)) { return false; -- 2.20.1 v8.1M implements a new 'branch future' feature, which is a set of instructions that request the CPU to perform a branch "in the future", when it reaches a particular execution address. In hardware, the expected implementation is that the information about the branch location and destination is cached and then acted upon when execution reaches the specified address. However the architecture permits an implementation to discard this cached information at any point, and so guest code must always include a normal branch insn at the branch point as a fallback. In particular, an implementation is specifically permitted to treat all BF insns as NOPs (which is equivalent to discarding the cached information immediately). For QEMU, implementing this caching of branch information would be complicated and would not improve the speed of execution at all, so we make the IMPDEF choice to implement all BF insns as NOPs. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20201019151301.2046-7-peter.maydell@linaro.org --- target/arm/cpu.h | 6 ++++++ target/arm/t32.decode | 13 ++++++++++++- target/arm/translate.c | 20 ++++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline bool isar_feature_aa32_arm_div(const ARMISARegisters *id) return FIELD_EX32(id->id_isar0, ID_ISAR0, DIVIDE) > 1; } +static inline bool isar_feature_aa32_lob(const ARMISARegisters *id) +{ + /* (M-profile) low-overhead loops and branch future */ + return FIELD_EX32(id->id_isar0, ID_ISAR0, CMPBRANCH) >= 3; +} + static inline bool isar_feature_aa32_jazelle(const ARMISARegisters *id) { return FIELD_EX32(id->id_isar1, ID_ISAR1, JAZELLE) != 0; diff --git a/target/arm/t32.decode b/target/arm/t32.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/t32.decode +++ b/target/arm/t32.decode @@ -XXX,XX +XXX,XX @@ MRC 1110 1110 ... 1 .... .... .... ... 1 .... @mcr B 1111 0. .......... 10.1 ............ @branch24 BL 1111 0. .......... 11.1 ............ @branch24 -BLX_i 1111 0. .......... 11.0 ............ @branch24 +{ + # BLX_i is non-M-profile only + BLX_i 1111 0. .......... 11.0 ............ @branch24 + # M-profile only: loop and branch insns + [ + # All these BF insns have boff != 0b0000; we NOP them all + BF 1111 0 boff:4 ------- 1100 - ---------- 1 # BFL + BF 1111 0 boff:4 0 ------ 1110 - ---------- 1 # BFCSEL + BF 1111 0 boff:4 10 ----- 1110 - ---------- 1 # BF + BF 1111 0 boff:4 11 ----- 1110 0 0000000000 1 # BFX, BFLX + ] +} 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 bool trans_BLX_suffix(DisasContext *s, arg_BLX_suffix *a) return true; } +static bool trans_BF(DisasContext *s, arg_BF *a) +{ + /* + * M-profile branch future insns. The architecture permits an + * implementation to implement these as NOPs (equivalent to + * discarding the LO_BRANCH_INFO cache immediately), and we + * take that IMPDEF option because for QEMU a "real" implementation + * would be complicated and wouldn't execute any faster. + */ + if (!dc_isar_feature(aa32_lob, s)) { + return false; + } + if (a->boff == 0) { + /* SEE "Related encodings" (loop insns) */ + return false; + } + /* Handle as NOP */ + return true; +} + static bool op_tbranch(DisasContext *s, arg_tbranch *a, bool half) { TCGv_i32 addr, tmp; -- 2.20.1 v8.1M's "low-overhead-loop" extension has three instructions for looping: * DLS (start of a do-loop) * WLS (start of a while-loop) * LE (end of a loop) The loop-start instructions are both simple operations to start a loop whose iteration count (if any) is in LR. The loop-end instruction handles "decrement iteration count and jump back to loop start"; it also caches the information about the branch back to the start of the loop to improve performance of the branch on subsequent iterations. As with the branch-future instructions, the architecture permits an implementation to discard the LO_BRANCH_INFO cache at any time, and QEMU takes the IMPDEF option to never set it in the first place (equivalent to discarding it immediately), because for us a "real" implementation would be unnecessary complexity. (This implementation only provides the simple looping constructs; the vector extension MVE (Helium) adds some extra variants to handle looping across vectors. We'll add those later when we implement MVE.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201019151301.2046-8-peter.maydell@linaro.org --- target/arm/t32.decode | 8 ++++ target/arm/translate.c | 93 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 99 insertions(+), 2 deletions(-) diff --git a/target/arm/t32.decode b/target/arm/t32.decode index XXXXXXX..XXXXXXX 100644 --- a/target/arm/t32.decode +++ b/target/arm/t32.decode @@ -XXX,XX +XXX,XX @@ BL 1111 0. .......... 11.1 ............ @branch24 BF 1111 0 boff:4 10 ----- 1110 - ---------- 1 # BF BF 1111 0 boff:4 11 ----- 1110 0 0000000000 1 # BFX, BFLX ] + [ + # LE and WLS immediate + %lob_imm 1:10 11:1 !function=times_2 + + DLS 1111 0 0000 100 rn:4 1110 0000 0000 0001 + WLS 1111 0 0000 100 rn:4 1100 . .......... 1 imm=%lob_imm + LE 1111 0 0000 0 f:1 0 1111 1100 . .......... 1 imm=%lob_imm + ] } 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_goto_tb(DisasContext *s, int n, target_ulong dest) s->base.is_jmp = DISAS_NORETURN; } -static inline void gen_jmp (DisasContext *s, uint32_t dest) +/* Jump, specifying which TB number to use if we gen_goto_tb() */ +static inline void gen_jmp_tb(DisasContext *s, uint32_t dest, int tbno) { if (unlikely(is_singlestepping(s))) { /* An indirect jump so that we still trigger the debug exception. */ gen_set_pc_im(s, dest); s->base.is_jmp = DISAS_JUMP; } else { - gen_goto_tb(s, 0, dest); + gen_goto_tb(s, tbno, dest); } } +static inline void gen_jmp(DisasContext *s, uint32_t dest) +{ + gen_jmp_tb(s, dest, 0); +} + static inline void gen_mulxy(TCGv_i32 t0, TCGv_i32 t1, int x, int y) { if (x) @@ -XXX,XX +XXX,XX @@ static bool trans_BF(DisasContext *s, arg_BF *a) return true; } +static bool trans_DLS(DisasContext *s, arg_DLS *a) +{ + /* M-profile low-overhead loop start */ + TCGv_i32 tmp; + + if (!dc_isar_feature(aa32_lob, s)) { + return false; + } + if (a->rn == 13 || a->rn == 15) { + /* CONSTRAINED UNPREDICTABLE: we choose to UNDEF */ + return false; + } + + /* Not a while loop, no tail predication: just set LR to the count */ + tmp = load_reg(s, a->rn); + store_reg(s, 14, tmp); + return true; +} + +static bool trans_WLS(DisasContext *s, arg_WLS *a) +{ + /* M-profile low-overhead while-loop start */ + TCGv_i32 tmp; + TCGLabel *nextlabel; + + if (!dc_isar_feature(aa32_lob, s)) { + return false; + } + if (a->rn == 13 || a->rn == 15) { + /* CONSTRAINED UNPREDICTABLE: we choose to UNDEF */ + return false; + } + if (s->condexec_mask) { + /* + * WLS in an IT block is CONSTRAINED UNPREDICTABLE; + * we choose to UNDEF, because otherwise our use of + * gen_goto_tb(1) would clash with the use of TB exit 1 + * in the dc->condjmp condition-failed codepath in + * arm_tr_tb_stop() and we'd get an assertion. + */ + return false; + } + nextlabel = gen_new_label(); + tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_R[a->rn], 0, nextlabel); + tmp = load_reg(s, a->rn); + store_reg(s, 14, tmp); + gen_jmp_tb(s, s->base.pc_next, 1); + + gen_set_label(nextlabel); + gen_jmp(s, read_pc(s) + a->imm); + return true; +} + +static bool trans_LE(DisasContext *s, arg_LE *a) +{ + /* + * M-profile low-overhead loop end. The architecture permits an + * implementation to discard the LO_BRANCH_INFO cache at any time, + * and we take the IMPDEF option to never set it in the first place + * (equivalent to always discarding it immediately), because for QEMU + * a "real" implementation would be complicated and wouldn't execute + * any faster. + */ + TCGv_i32 tmp; + + if (!dc_isar_feature(aa32_lob, s)) { + return false; + } + + if (!a->f) { + /* Not loop-forever. If LR <= 1 this is the last loop: do nothing. */ + arm_gen_condlabel(s); + tcg_gen_brcondi_i32(TCG_COND_LEU, cpu_R[14], 1, s->condlabel); + /* Decrement LR */ + tmp = load_reg(s, 14); + tcg_gen_addi_i32(tmp, tmp, -1); + store_reg(s, 14, tmp); + } + /* Jump back to the loop start */ + gen_jmp(s, read_pc(s) - a->imm); + return true; +} + static bool op_tbranch(DisasContext *s, arg_tbranch *a, bool half) { TCGv_i32 addr, tmp; -- 2.20.1 In arm_cpu_realizefn(), if the CPU has VFP or Neon disabled then we squash the ID register fields so that we don't advertise it to the guest. This code was written for A-profile and needs some tweaks to work correctly on M-profile: * A-profile only fields should not be zeroed on M-profile: - MVFR0.FPSHVEC,FPTRAP - MVFR1.SIMDLS,SIMDINT,SIMDSP,SIMDHP - MVFR2.SIMDMISC * M-profile only fields should be zeroed on M-profile: - MVFR1.FP16 In particular, because MVFR1.SIMDHP on A-profile is the same field as MVFR1.FP16 on M-profile this code was incorrectly disabling FP16 support on an M-profile CPU (where has_neon is always false). This isn't a visible bug yet because we don't have any M-profile CPUs with FP16 support, but the change is necessary before we introduce any. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20201019151301.2046-9-peter.maydell@linaro.org --- target/arm/cpu.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) 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) u = cpu->isar.mvfr0; u = FIELD_DP32(u, MVFR0, FPSP, 0); u = FIELD_DP32(u, MVFR0, FPDP, 0); - u = FIELD_DP32(u, MVFR0, FPTRAP, 0); u = FIELD_DP32(u, MVFR0, FPDIVIDE, 0); u = FIELD_DP32(u, MVFR0, FPSQRT, 0); - u = FIELD_DP32(u, MVFR0, FPSHVEC, 0); u = FIELD_DP32(u, MVFR0, FPROUND, 0); + if (!arm_feature(env, ARM_FEATURE_M)) { + u = FIELD_DP32(u, MVFR0, FPTRAP, 0); + u = FIELD_DP32(u, MVFR0, FPSHVEC, 0); + } cpu->isar.mvfr0 = u; u = cpu->isar.mvfr1; u = FIELD_DP32(u, MVFR1, FPFTZ, 0); u = FIELD_DP32(u, MVFR1, FPDNAN, 0); u = FIELD_DP32(u, MVFR1, FPHP, 0); + if (arm_feature(env, ARM_FEATURE_M)) { + u = FIELD_DP32(u, MVFR1, FP16, 0); + } cpu->isar.mvfr1 = u; u = cpu->isar.mvfr2; @@ -XXX,XX +XXX,XX @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) u = FIELD_DP32(u, ID_ISAR6, FHM, 0); cpu->isar.id_isar6 = u; - u = cpu->isar.mvfr1; - u = FIELD_DP32(u, MVFR1, SIMDLS, 0); - u = FIELD_DP32(u, MVFR1, SIMDINT, 0); - u = FIELD_DP32(u, MVFR1, SIMDSP, 0); - u = FIELD_DP32(u, MVFR1, SIMDHP, 0); - cpu->isar.mvfr1 = u; + if (!arm_feature(env, ARM_FEATURE_M)) { + u = cpu->isar.mvfr1; + u = FIELD_DP32(u, MVFR1, SIMDLS, 0); + u = FIELD_DP32(u, MVFR1, SIMDINT, 0); + u = FIELD_DP32(u, MVFR1, SIMDSP, 0); + u = FIELD_DP32(u, MVFR1, SIMDHP, 0); + cpu->isar.mvfr1 = u; - u = cpu->isar.mvfr2; - u = FIELD_DP32(u, MVFR2, SIMDMISC, 0); - cpu->isar.mvfr2 = u; + u = cpu->isar.mvfr2; + u = FIELD_DP32(u, MVFR2, SIMDMISC, 0); + cpu->isar.mvfr2 = u; + } } if (!cpu->has_neon && !cpu->has_vfp) { -- 2.20.1 M-profile CPUs with half-precision floating point support should be able to write to FPSCR.FZ16, but an M-profile specific masking of the value at the top of vfp_set_fpscr() currently prevents that. This is not yet an active bug because we have no M-profile FP16 CPUs, but needs to be fixed before we can add any. The bits that the masking is effectively preventing from being set are the A-profile only short-vector Len and Stride fields, plus the Neon QC bit. Rearrange the order of the function so that those fields are handled earlier and only under a suitable guard; this allows us to drop the M-profile specific masking, making FZ16 writeable. This change also makes the QC bit correctly RAZ/WI for older no-Neon A-profile cores. This refactoring also paves the way for the low-overhead-branch LTPSIZE field, which uses some of the bits that are used for A-profile Stride and Len. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201019151301.2046-10-peter.maydell@linaro.org --- target/arm/vfp_helper.c | 47 ++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/target/arm/vfp_helper.c b/target/arm/vfp_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/vfp_helper.c +++ b/target/arm/vfp_helper.c @@ -XXX,XX +XXX,XX @@ void HELPER(vfp_set_fpscr)(CPUARMState *env, uint32_t val) val &= ~FPCR_FZ16; } - if (arm_feature(env, ARM_FEATURE_M)) { + vfp_set_fpscr_to_host(env, val); + + if (!arm_feature(env, ARM_FEATURE_M)) { /* - * M profile FPSCR is RES0 for the QC, STRIDE, FZ16, LEN bits - * and also for the trapped-exception-handling bits IxE. + * Short-vector length and stride; on M-profile these bits + * are used for different purposes. + * We can't make this conditional be "if MVFR0.FPShVec != 0", + * because in v7A no-short-vector-support cores still had to + * allow Stride/Len to be written with the only effect that + * some insns are required to UNDEF if the guest sets them. + * + * TODO: if M-profile MVE implemented, set LTPSIZE. */ - val &= 0xf7c0009f; + env->vfp.vec_len = extract32(val, 16, 3); + env->vfp.vec_stride = extract32(val, 20, 2); } - vfp_set_fpscr_to_host(env, val); + if (arm_feature(env, ARM_FEATURE_NEON)) { + /* + * The bit we set within fpscr_q is arbitrary; the register as a + * whole being zero/non-zero is what counts. + * TODO: M-profile MVE also has a QC bit. + */ + env->vfp.qc[0] = val & FPCR_QC; + env->vfp.qc[1] = 0; + env->vfp.qc[2] = 0; + env->vfp.qc[3] = 0; + } /* * We don't implement trapped exception handling, so the * trap enable bits, IDE|IXE|UFE|OFE|DZE|IOE are all RAZ/WI (not RES0!) * - * If we exclude the exception flags, IOC|DZC|OFC|UFC|IXC|IDC - * (which are stored in fp_status), and the other RES0 bits - * in between, then we clear all of the low 16 bits. + * The exception flags IOC|DZC|OFC|UFC|IXC|IDC are stored in + * fp_status; QC, Len and Stride are stored separately earlier. + * Clear out all of those and the RES0 bits: only NZCV, AHP, DN, + * FZ, RMode and FZ16 are kept in vfp.xregs[FPSCR]. */ env->vfp.xregs[ARM_VFP_FPSCR] = val & 0xf7c80000; - env->vfp.vec_len = (val >> 16) & 7; - env->vfp.vec_stride = (val >> 20) & 3; - - /* - * The bit we set within fpscr_q is arbitrary; the register as a - * whole being zero/non-zero is what counts. - */ - env->vfp.qc[0] = val & FPCR_QC; - env->vfp.qc[1] = 0; - env->vfp.qc[2] = 0; - env->vfp.qc[3] = 0; } void vfp_set_fpscr(CPUARMState *env, uint32_t val) -- 2.20.1 If the M-profile low-overhead-branch extension is implemented, FPSCR bits [18:16] are a new field LTPSIZE. If MVE is not implemented (currently always true for us) then this field always reads as 4 and ignores writes. These bits used to be the vector-length field for the old short-vector extension, so we need to take care that they are not misinterpreted as setting vec_len. We do this with a rearrangement of the vfp_set_fpscr() code that deals with vec_len, vec_stride and also the QC bit; this obviates the need for the M-profile only masking step that we used to have at the start of the function. We provide a new field in CPUState for LTPSIZE, even though this will always be 4, in preparation for MVE, so we don't have to come back later and split it out of the vfp.xregs[FPSCR] value. (This state struct field will be saved and restored as part of the FPSCR value via the vmstate_fpscr in machine.c.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201019151301.2046-11-peter.maydell@linaro.org --- target/arm/cpu.h | 1 + target/arm/cpu.c | 9 +++++++++ target/arm/vfp_helper.c | 6 ++++++ 3 files changed, 16 insertions(+) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ typedef struct CPUARMState { uint32_t fpdscr[M_REG_NUM_BANKS]; uint32_t cpacr[M_REG_NUM_BANKS]; uint32_t nsacr; + int ltpsize; } v7m; /* Information associated with an exception about to be taken: diff --git a/target/arm/cpu.c b/target/arm/cpu.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -XXX,XX +XXX,XX @@ static void arm_cpu_reset(DeviceState *dev) uint8_t *rom; uint32_t vecbase; + if (cpu_isar_feature(aa32_lob, cpu)) { + /* + * LTPSIZE is constant 4 if MVE not implemented, and resets + * to an UNKNOWN value if MVE is implemented. We choose to + * always reset to 4. + */ + env->v7m.ltpsize = 4; + } + if (arm_feature(env, ARM_FEATURE_M_SECURITY)) { env->v7m.secure = true; } else { diff --git a/target/arm/vfp_helper.c b/target/arm/vfp_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/vfp_helper.c +++ b/target/arm/vfp_helper.c @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(vfp_get_fpscr)(CPUARMState *env) | (env->vfp.vec_len << 16) | (env->vfp.vec_stride << 20); + /* + * M-profile LTPSIZE overlaps A-profile Stride; whichever of the + * two is not applicable to this CPU will always be zero. + */ + fpscr |= env->v7m.ltpsize << 16; + fpscr |= vfp_get_fpscr_from_host(env); i = env->vfp.qc[0] | env->vfp.qc[1] | env->vfp.qc[2] | env->vfp.qc[3]; -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> The kernel sets btype for the signal handler as if for a call. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201016184207.786698-2-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- linux-user/aarch64/signal.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c index XXXXXXX..XXXXXXX 100644 --- a/linux-user/aarch64/signal.c +++ b/linux-user/aarch64/signal.c @@ -XXX,XX +XXX,XX @@ static void target_setup_frame(int usig, struct target_sigaction *ka, + offsetof(struct target_rt_frame_record, tramp); } env->xregs[0] = usig; - env->xregs[31] = frame_addr; env->xregs[29] = frame_addr + fr_ofs; - env->pc = ka->_sa_handler; env->xregs[30] = return_addr; + env->xregs[31] = frame_addr; + env->pc = ka->_sa_handler; + + /* Invoke the signal handler as if by indirect call. */ + if (cpu_isar_feature(aa64_bti, env_archcpu(env))) { + env->btype = 2; + } + if (info) { tswap_siginfo(&frame->info, info); env->xregs[1] = frame_addr + offsetof(struct target_rt_sigframe, info); -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> Transform the prot bit to a qemu internal page bit, and save it in the page tables. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201016184207.786698-3-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/exec/cpu-all.h | 2 ++ linux-user/syscall_defs.h | 4 ++++ target/arm/cpu.h | 5 +++++ linux-user/mmap.c | 16 ++++++++++++++++ target/arm/translate-a64.c | 6 +++--- 5 files changed, 30 insertions(+), 3 deletions(-) diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/cpu-all.h +++ b/include/exec/cpu-all.h @@ -XXX,XX +XXX,XX @@ extern intptr_t qemu_host_page_mask; /* FIXME: Code that sets/uses this is broken and needs to go away. */ #define PAGE_RESERVED 0x0020 #endif +/* Target-specific bits that will be used via page_get_flags(). */ +#define PAGE_TARGET_1 0x0080 #if defined(CONFIG_USER_ONLY) void page_dump(FILE *f); diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h index XXXXXXX..XXXXXXX 100644 --- a/linux-user/syscall_defs.h +++ b/linux-user/syscall_defs.h @@ -XXX,XX +XXX,XX @@ struct target_winsize { #define TARGET_PROT_SEM 0x08 #endif +#ifdef TARGET_AARCH64 +#define TARGET_PROT_BTI 0x10 +#endif + /* Common */ #define TARGET_MAP_SHARED 0x01 /* Share changes */ #define TARGET_MAP_PRIVATE 0x02 /* Changes are private */ 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 MemTxAttrs *typecheck_memtxattrs(MemTxAttrs *x) #define arm_tlb_bti_gp(x) (typecheck_memtxattrs(x)->target_tlb_bit0) #define arm_tlb_mte_tagged(x) (typecheck_memtxattrs(x)->target_tlb_bit1) +/* + * AArch64 usage of the PAGE_TARGET_* bits for linux-user. + */ +#define PAGE_BTI PAGE_TARGET_1 + /* * Naming convention for isar_feature functions: * Functions which test 32-bit ID registers should have _aa32_ in diff --git a/linux-user/mmap.c b/linux-user/mmap.c index XXXXXXX..XXXXXXX 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -XXX,XX +XXX,XX @@ static int validate_prot_to_pageflags(int *host_prot, int prot) *host_prot = (prot & (PROT_READ | PROT_WRITE)) | (prot & PROT_EXEC ? PROT_READ : 0); +#ifdef TARGET_AARCH64 + /* + * The PROT_BTI bit is only accepted if the cpu supports the feature. + * Since this is the unusual case, don't bother checking unless + * the bit has been requested. If set and valid, record the bit + * within QEMU's page_flags. + */ + if (prot & TARGET_PROT_BTI) { + ARMCPU *cpu = ARM_CPU(thread_cpu); + if (cpu_isar_feature(aa64_bti, cpu)) { + valid |= TARGET_PROT_BTI; + page_flags |= PAGE_BTI; + } + } +#endif + return prot & ~valid ? 0 : page_flags; } diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static void disas_data_proc_simd_fp(DisasContext *s, uint32_t insn) */ static bool is_guarded_page(CPUARMState *env, DisasContext *s) { -#ifdef CONFIG_USER_ONLY - return false; /* FIXME */ -#else uint64_t addr = s->base.pc_first; +#ifdef CONFIG_USER_ONLY + return page_get_flags(addr) & PAGE_BTI; +#else int mmu_idx = arm_to_core_mmu_idx(s->mmu_idx); unsigned int index = tlb_index(env, mmu_idx, addr); CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr); -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> These are all of the defines required to parse GNU_PROPERTY_AARCH64_FEATURE_1_AND, copied from binutils. Other missing defines related to other GNU program headers and notes are elided for now. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201016184207.786698-4-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/elf.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/include/elf.h b/include/elf.h index XXXXXXX..XXXXXXX 100644 --- a/include/elf.h +++ b/include/elf.h @@ -XXX,XX +XXX,XX @@ typedef int64_t Elf64_Sxword; #define PT_NOTE 4 #define PT_SHLIB 5 #define PT_PHDR 6 +#define PT_LOOS 0x60000000 +#define PT_HIOS 0x6fffffff #define PT_LOPROC 0x70000000 #define PT_HIPROC 0x7fffffff +#define PT_GNU_PROPERTY (PT_LOOS + 0x474e553) + #define PT_MIPS_REGINFO 0x70000000 #define PT_MIPS_RTPROC 0x70000001 #define PT_MIPS_OPTIONS 0x70000002 @@ -XXX,XX +XXX,XX @@ typedef struct elf64_shdr { #define NT_ARM_SYSTEM_CALL 0x404 /* ARM system call number */ #define NT_ARM_SVE 0x405 /* ARM Scalable Vector Extension regs */ +/* Defined note types for GNU systems. */ + +#define NT_GNU_PROPERTY_TYPE_0 5 /* Program property */ + +/* Values used in GNU .note.gnu.property notes (NT_GNU_PROPERTY_TYPE_0). */ + +#define GNU_PROPERTY_STACK_SIZE 1 +#define GNU_PROPERTY_NO_COPY_ON_PROTECTED 2 + +#define GNU_PROPERTY_LOPROC 0xc0000000 +#define GNU_PROPERTY_HIPROC 0xdfffffff +#define GNU_PROPERTY_LOUSER 0xe0000000 +#define GNU_PROPERTY_HIUSER 0xffffffff + +#define GNU_PROPERTY_AARCH64_FEATURE_1_AND 0xc0000000 +#define GNU_PROPERTY_AARCH64_FEATURE_1_BTI (1u << 0) +#define GNU_PROPERTY_AARCH64_FEATURE_1_PAC (1u << 1) + /* * Physical entry point into the kernel. * -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Fix an unlikely memory leak in load_elf_image(). Fixes: bf858897b7 ("linux-user: Re-use load_elf_image for the main binary.") Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201016184207.786698-5-richard.henderson@linaro.org Message-Id: <20201003174944.1972444-1-f4bug@amsat.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- linux-user/elfload.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index XXXXXXX..XXXXXXX 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, info->brk = vaddr_em; } } else if (eppnt->p_type == PT_INTERP && pinterp_name) { - char *interp_name; + g_autofree char *interp_name = NULL; if (*pinterp_name) { errmsg = "Multiple PT_INTERP entries"; goto exit_errmsg; } - interp_name = malloc(eppnt->p_filesz); + interp_name = g_malloc(eppnt->p_filesz); if (!interp_name) { goto exit_perror; } @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, errmsg = "Invalid PT_INTERP entry"; goto exit_errmsg; } - *pinterp_name = interp_name; + *pinterp_name = g_steal_pointer(&interp_name); #ifdef TARGET_MIPS } else if (eppnt->p_type == PT_MIPS_ABIFLAGS) { Mips_elf_abiflags_v0 abiflags; @@ -XXX,XX +XXX,XX @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info) if (elf_interpreter) { info->load_bias = interp_info.load_bias; info->entry = interp_info.entry; - free(elf_interpreter); + g_free(elf_interpreter); } #ifdef USE_ELF_CORE_DUMP -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> Fixing this now will clarify following patches. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201016184207.786698-6-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- linux-user/elfload.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index XXXXXXX..XXXXXXX 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, abi_ulong vaddr, vaddr_po, vaddr_ps, vaddr_ef, vaddr_em, vaddr_len; int elf_prot = 0; - if (eppnt->p_flags & PF_R) elf_prot = PROT_READ; - if (eppnt->p_flags & PF_W) elf_prot |= PROT_WRITE; - if (eppnt->p_flags & PF_X) elf_prot |= PROT_EXEC; + if (eppnt->p_flags & PF_R) { + elf_prot |= PROT_READ; + } + if (eppnt->p_flags & PF_W) { + elf_prot |= PROT_WRITE; + } + if (eppnt->p_flags & PF_X) { + elf_prot |= PROT_EXEC; + } vaddr = load_bias + eppnt->p_vaddr; vaddr_po = TARGET_ELF_PAGEOFFSET(vaddr); -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> The second loop uses a loop induction variable, and the first does not. Transform the first to match the second, to simplify a following patch moving code between them. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201016184207.786698-7-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- linux-user/elfload.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index XXXXXXX..XXXXXXX 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, loaddr = -1, hiaddr = 0; info->alignment = 0; for (i = 0; i < ehdr->e_phnum; ++i) { - if (phdr[i].p_type == PT_LOAD) { - abi_ulong a = phdr[i].p_vaddr - phdr[i].p_offset; + struct elf_phdr *eppnt = phdr + i; + if (eppnt->p_type == PT_LOAD) { + abi_ulong a = eppnt->p_vaddr - eppnt->p_offset; if (a < loaddr) { loaddr = a; } - a = phdr[i].p_vaddr + phdr[i].p_memsz; + a = eppnt->p_vaddr + eppnt->p_memsz; if (a > hiaddr) { hiaddr = a; } ++info->nsegs; - info->alignment |= phdr[i].p_align; + info->alignment |= eppnt->p_align; } } -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> For BTI, we need to know if the executable is static or dynamic, which means looking for PT_INTERP earlier. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201016184207.786698-8-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- linux-user/elfload.c | 60 +++++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index XXXXXXX..XXXXXXX 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, mmap_lock(); - /* Find the maximum size of the image and allocate an appropriate - amount of memory to handle that. */ + /* + * Find the maximum size of the image and allocate an appropriate + * amount of memory to handle that. Locate the interpreter, if any. + */ loaddr = -1, hiaddr = 0; info->alignment = 0; for (i = 0; i < ehdr->e_phnum; ++i) { @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, } ++info->nsegs; info->alignment |= eppnt->p_align; + } else if (eppnt->p_type == PT_INTERP && pinterp_name) { + g_autofree char *interp_name = NULL; + + if (*pinterp_name) { + errmsg = "Multiple PT_INTERP entries"; + goto exit_errmsg; + } + interp_name = g_malloc(eppnt->p_filesz); + if (!interp_name) { + goto exit_perror; + } + + if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) { + memcpy(interp_name, bprm_buf + eppnt->p_offset, + eppnt->p_filesz); + } else { + retval = pread(image_fd, interp_name, eppnt->p_filesz, + eppnt->p_offset); + if (retval != eppnt->p_filesz) { + goto exit_perror; + } + } + if (interp_name[eppnt->p_filesz - 1] != 0) { + errmsg = "Invalid PT_INTERP entry"; + goto exit_errmsg; + } + *pinterp_name = g_steal_pointer(&interp_name); } } @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, if (vaddr_em > info->brk) { info->brk = vaddr_em; } - } else if (eppnt->p_type == PT_INTERP && pinterp_name) { - g_autofree char *interp_name = NULL; - - if (*pinterp_name) { - errmsg = "Multiple PT_INTERP entries"; - goto exit_errmsg; - } - interp_name = g_malloc(eppnt->p_filesz); - if (!interp_name) { - goto exit_perror; - } - - if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) { - memcpy(interp_name, bprm_buf + eppnt->p_offset, - eppnt->p_filesz); - } else { - retval = pread(image_fd, interp_name, eppnt->p_filesz, - eppnt->p_offset); - if (retval != eppnt->p_filesz) { - goto exit_perror; - } - } - if (interp_name[eppnt->p_filesz - 1] != 0) { - errmsg = "Invalid PT_INTERP entry"; - goto exit_errmsg; - } - *pinterp_name = g_steal_pointer(&interp_name); #ifdef TARGET_MIPS } else if (eppnt->p_type == PT_MIPS_ABIFLAGS) { Mips_elf_abiflags_v0 abiflags; -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> This is a bit clearer than open-coding some of this with a bare c string. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201016184207.786698-9-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- linux-user/elfload.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index XXXXXXX..XXXXXXX 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -XXX,XX +XXX,XX @@ #include "qemu/guest-random.h" #include "qemu/units.h" #include "qemu/selfmap.h" +#include "qapi/error.h" #ifdef _ARCH_PPC64 #undef ARCH_DLINFO @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, struct elf_phdr *phdr; abi_ulong load_addr, load_bias, loaddr, hiaddr, error; int i, retval; - const char *errmsg; + Error *err = NULL; /* First of all, some simple consistency checks */ - errmsg = "Invalid ELF image for this architecture"; if (!elf_check_ident(ehdr)) { + error_setg(&err, "Invalid ELF image for this architecture"); goto exit_errmsg; } bswap_ehdr(ehdr); if (!elf_check_ehdr(ehdr)) { + error_setg(&err, "Invalid ELF image for this architecture"); goto exit_errmsg; } @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, g_autofree char *interp_name = NULL; if (*pinterp_name) { - errmsg = "Multiple PT_INTERP entries"; + error_setg(&err, "Multiple PT_INTERP entries"); goto exit_errmsg; } + interp_name = g_malloc(eppnt->p_filesz); - if (!interp_name) { - goto exit_perror; - } if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) { memcpy(interp_name, bprm_buf + eppnt->p_offset, @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, retval = pread(image_fd, interp_name, eppnt->p_filesz, eppnt->p_offset); if (retval != eppnt->p_filesz) { - goto exit_perror; + goto exit_read; } } if (interp_name[eppnt->p_filesz - 1] != 0) { - errmsg = "Invalid PT_INTERP entry"; + error_setg(&err, "Invalid PT_INTERP entry"); goto exit_errmsg; } *pinterp_name = g_steal_pointer(&interp_name); @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, (ehdr->e_type == ET_EXEC ? MAP_FIXED : 0), -1, 0); if (load_addr == -1) { - goto exit_perror; + goto exit_mmap; } load_bias = load_addr - loaddr; @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, image_fd, eppnt->p_offset - vaddr_po); if (error == -1) { - goto exit_perror; + goto exit_mmap; } } @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, } else if (eppnt->p_type == PT_MIPS_ABIFLAGS) { Mips_elf_abiflags_v0 abiflags; if (eppnt->p_filesz < sizeof(Mips_elf_abiflags_v0)) { - errmsg = "Invalid PT_MIPS_ABIFLAGS entry"; + error_setg(&err, "Invalid PT_MIPS_ABIFLAGS entry"); goto exit_errmsg; } if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) { @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, retval = pread(image_fd, &abiflags, sizeof(Mips_elf_abiflags_v0), eppnt->p_offset); if (retval != sizeof(Mips_elf_abiflags_v0)) { - goto exit_perror; + goto exit_read; } } bswap_mips_abiflags(&abiflags); @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, exit_read: if (retval >= 0) { - errmsg = "Incomplete read of file header"; - goto exit_errmsg; + error_setg(&err, "Incomplete read of file header"); + } else { + error_setg_errno(&err, errno, "Error reading file header"); } - exit_perror: - errmsg = strerror(errno); + goto exit_errmsg; + exit_mmap: + error_setg_errno(&err, errno, "Error mapping file"); + goto exit_errmsg; exit_errmsg: - fprintf(stderr, "%s: %s\n", image_name, errmsg); + error_reportf_err(err, "%s: ", image_name); exit(-1); } -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> This is slightly clearer than just using strerror, though the different forms produced by error_setg_file_open and error_setg_errno isn't entirely convenient. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20201016184207.786698-10-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- linux-user/elfload.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index XXXXXXX..XXXXXXX 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -XXX,XX +XXX,XX @@ static void load_elf_interp(const char *filename, struct image_info *info, char bprm_buf[BPRM_BUF_SIZE]) { int fd, retval; + Error *err = NULL; fd = open(path(filename), O_RDONLY); if (fd < 0) { - goto exit_perror; + error_setg_file_open(&err, errno, filename); + error_report_err(err); + exit(-1); } retval = read(fd, bprm_buf, BPRM_BUF_SIZE); if (retval < 0) { - goto exit_perror; + error_setg_errno(&err, errno, "Error reading file header"); + error_reportf_err(err, "%s: ", filename); + exit(-1); } + if (retval < BPRM_BUF_SIZE) { memset(bprm_buf + retval, 0, BPRM_BUF_SIZE - retval); } load_elf_image(filename, fd, info, NULL, bprm_buf); - return; - - exit_perror: - fprintf(stderr, "%s: %s\n", filename, strerror(errno)); - exit(-1); } static int symfind(const void *s0, const void *s1) -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> This is generic support, with the code disabled for all targets. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201016184207.786698-11-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- linux-user/qemu.h | 4 ++ linux-user/elfload.c | 157 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 161 insertions(+) diff --git a/linux-user/qemu.h b/linux-user/qemu.h index XXXXXXX..XXXXXXX 100644 --- a/linux-user/qemu.h +++ b/linux-user/qemu.h @@ -XXX,XX +XXX,XX @@ struct image_info { abi_ulong interpreter_loadmap_addr; abi_ulong interpreter_pt_dynamic_addr; struct image_info *other_info; + + /* For target-specific processing of NT_GNU_PROPERTY_TYPE_0. */ + uint32_t note_flags; + #ifdef TARGET_MIPS int fp_abi; int interp_fp_abi; diff --git a/linux-user/elfload.c b/linux-user/elfload.c index XXXXXXX..XXXXXXX 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -XXX,XX +XXX,XX @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, #include "elf.h" +static bool arch_parse_elf_property(uint32_t pr_type, uint32_t pr_datasz, + const uint32_t *data, + struct image_info *info, + Error **errp) +{ + g_assert_not_reached(); +} +#define ARCH_USE_GNU_PROPERTY 0 + struct exec { unsigned int a_info; /* Use macros N_MAGIC, etc for access */ @@ -XXX,XX +XXX,XX @@ void probe_guest_base(const char *image_name, abi_ulong guest_loaddr, "@ 0x%" PRIx64 "\n", (uint64_t)guest_base); } +enum { + /* The string "GNU\0" as a magic number. */ + GNU0_MAGIC = const_le32('G' | 'N' << 8 | 'U' << 16), + NOTE_DATA_SZ = 1 * KiB, + NOTE_NAME_SZ = 4, + ELF_GNU_PROPERTY_ALIGN = ELF_CLASS == ELFCLASS32 ? 4 : 8, +}; + +/* + * Process a single gnu_property entry. + * Return false for error. + */ +static bool parse_elf_property(const uint32_t *data, int *off, int datasz, + struct image_info *info, bool have_prev_type, + uint32_t *prev_type, Error **errp) +{ + uint32_t pr_type, pr_datasz, step; + + if (*off > datasz || !QEMU_IS_ALIGNED(*off, ELF_GNU_PROPERTY_ALIGN)) { + goto error_data; + } + datasz -= *off; + data += *off / sizeof(uint32_t); + + if (datasz < 2 * sizeof(uint32_t)) { + goto error_data; + } + pr_type = data[0]; + pr_datasz = data[1]; + data += 2; + datasz -= 2 * sizeof(uint32_t); + step = ROUND_UP(pr_datasz, ELF_GNU_PROPERTY_ALIGN); + if (step > datasz) { + goto error_data; + } + + /* Properties are supposed to be unique and sorted on pr_type. */ + if (have_prev_type && pr_type <= *prev_type) { + if (pr_type == *prev_type) { + error_setg(errp, "Duplicate property in PT_GNU_PROPERTY"); + } else { + error_setg(errp, "Unsorted property in PT_GNU_PROPERTY"); + } + return false; + } + *prev_type = pr_type; + + if (!arch_parse_elf_property(pr_type, pr_datasz, data, info, errp)) { + return false; + } + + *off += 2 * sizeof(uint32_t) + step; + return true; + + error_data: + error_setg(errp, "Ill-formed property in PT_GNU_PROPERTY"); + return false; +} + +/* Process NT_GNU_PROPERTY_TYPE_0. */ +static bool parse_elf_properties(int image_fd, + struct image_info *info, + const struct elf_phdr *phdr, + char bprm_buf[BPRM_BUF_SIZE], + Error **errp) +{ + union { + struct elf_note nhdr; + uint32_t data[NOTE_DATA_SZ / sizeof(uint32_t)]; + } note; + + int n, off, datasz; + bool have_prev_type; + uint32_t prev_type; + + /* Unless the arch requires properties, ignore them. */ + if (!ARCH_USE_GNU_PROPERTY) { + return true; + } + + /* If the properties are crazy large, that's too bad. */ + n = phdr->p_filesz; + if (n > sizeof(note)) { + error_setg(errp, "PT_GNU_PROPERTY too large"); + return false; + } + if (n < sizeof(note.nhdr)) { + error_setg(errp, "PT_GNU_PROPERTY too small"); + return false; + } + + if (phdr->p_offset + n <= BPRM_BUF_SIZE) { + memcpy(&note, bprm_buf + phdr->p_offset, n); + } else { + ssize_t len = pread(image_fd, &note, n, phdr->p_offset); + if (len != n) { + error_setg_errno(errp, errno, "Error reading file header"); + return false; + } + } + + /* + * The contents of a valid PT_GNU_PROPERTY is a sequence + * of uint32_t -- swap them all now. + */ +#ifdef BSWAP_NEEDED + for (int i = 0; i < n / 4; i++) { + bswap32s(note.data + i); + } +#endif + + /* + * Note that nhdr is 3 words, and that the "name" described by namesz + * immediately follows nhdr and is thus at the 4th word. Further, all + * of the inputs to the kernel's round_up are multiples of 4. + */ + if (note.nhdr.n_type != NT_GNU_PROPERTY_TYPE_0 || + note.nhdr.n_namesz != NOTE_NAME_SZ || + note.data[3] != GNU0_MAGIC) { + error_setg(errp, "Invalid note in PT_GNU_PROPERTY"); + return false; + } + off = sizeof(note.nhdr) + NOTE_NAME_SZ; + + datasz = note.nhdr.n_descsz + off; + if (datasz > n) { + error_setg(errp, "Invalid note size in PT_GNU_PROPERTY"); + return false; + } + + have_prev_type = false; + prev_type = 0; + while (1) { + if (off == datasz) { + return true; /* end, exit ok */ + } + if (!parse_elf_property(note.data, &off, datasz, info, + have_prev_type, &prev_type, errp)) { + return false; + } + have_prev_type = true; + } +} + /* Load an ELF image into the address space. IMAGE_NAME is the filename of the image, to use in error messages. @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, goto exit_errmsg; } *pinterp_name = g_steal_pointer(&interp_name); + } else if (eppnt->p_type == PT_GNU_PROPERTY) { + if (!parse_elf_properties(image_fd, info, eppnt, bprm_buf, &err)) { + goto exit_errmsg; + } } } -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> Use the new generic support for NT_GNU_PROPERTY_TYPE_0. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201016184207.786698-12-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- linux-user/elfload.c | 48 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index XXXXXXX..XXXXXXX 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -XXX,XX +XXX,XX @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, #include "elf.h" +/* We must delay the following stanzas until after "elf.h". */ +#if defined(TARGET_AARCH64) + +static bool arch_parse_elf_property(uint32_t pr_type, uint32_t pr_datasz, + const uint32_t *data, + struct image_info *info, + Error **errp) +{ + if (pr_type == GNU_PROPERTY_AARCH64_FEATURE_1_AND) { + if (pr_datasz != sizeof(uint32_t)) { + error_setg(errp, "Ill-formed GNU_PROPERTY_AARCH64_FEATURE_1_AND"); + return false; + } + /* We will extract GNU_PROPERTY_AARCH64_FEATURE_1_BTI later. */ + info->note_flags = *data; + } + return true; +} +#define ARCH_USE_GNU_PROPERTY 1 + +#else + static bool arch_parse_elf_property(uint32_t pr_type, uint32_t pr_datasz, const uint32_t *data, struct image_info *info, @@ -XXX,XX +XXX,XX @@ static bool arch_parse_elf_property(uint32_t pr_type, uint32_t pr_datasz, } #define ARCH_USE_GNU_PROPERTY 0 +#endif + struct exec { unsigned int a_info; /* Use macros N_MAGIC, etc for access */ @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, struct elfhdr *ehdr = (struct elfhdr *)bprm_buf; struct elf_phdr *phdr; abi_ulong load_addr, load_bias, loaddr, hiaddr, error; - int i, retval; + int i, retval, prot_exec; Error *err = NULL; /* First of all, some simple consistency checks */ @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, info->brk = 0; info->elf_flags = ehdr->e_flags; + prot_exec = PROT_EXEC; +#ifdef TARGET_AARCH64 + /* + * If the BTI feature is present, this indicates that the executable + * pages of the startup binary should be mapped with PROT_BTI, so that + * branch targets are enforced. + * + * The startup binary is either the interpreter or the static executable. + * The interpreter is responsible for all pages of a dynamic executable. + * + * Elf notes are backward compatible to older cpus. + * Do not enable BTI unless it is supported. + */ + if ((info->note_flags & GNU_PROPERTY_AARCH64_FEATURE_1_BTI) + && (pinterp_name == NULL || *pinterp_name == 0) + && cpu_isar_feature(aa64_bti, ARM_CPU(thread_cpu))) { + prot_exec |= TARGET_PROT_BTI; + } +#endif + for (i = 0; i < ehdr->e_phnum; i++) { struct elf_phdr *eppnt = phdr + i; if (eppnt->p_type == PT_LOAD) { @@ -XXX,XX +XXX,XX @@ static void load_elf_image(const char *image_name, int image_fd, elf_prot |= PROT_WRITE; } if (eppnt->p_flags & PF_X) { - elf_prot |= PROT_EXEC; + elf_prot |= prot_exec; } vaddr = load_bias + eppnt->p_vaddr; -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> The note test requires gcc 10 for -mbranch-protection=standard. The mmap test uses PROT_BTI and does not require special compiler support. Acked-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201016184207.786698-13-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/tcg/aarch64/bti-1.c | 62 +++++++++++++++++ tests/tcg/aarch64/bti-2.c | 108 ++++++++++++++++++++++++++++++ tests/tcg/aarch64/bti-crt.inc.c | 51 ++++++++++++++ tests/tcg/aarch64/Makefile.target | 10 +++ tests/tcg/configure.sh | 4 ++ 5 files changed, 235 insertions(+) create mode 100644 tests/tcg/aarch64/bti-1.c create mode 100644 tests/tcg/aarch64/bti-2.c create mode 100644 tests/tcg/aarch64/bti-crt.inc.c diff --git a/tests/tcg/aarch64/bti-1.c b/tests/tcg/aarch64/bti-1.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/tests/tcg/aarch64/bti-1.c @@ -XXX,XX +XXX,XX @@ +/* + * Branch target identification, basic notskip cases. + */ + +#include "bti-crt.inc.c" + +static void skip2_sigill(int sig, siginfo_t *info, ucontext_t *uc) +{ + uc->uc_mcontext.pc += 8; + uc->uc_mcontext.pstate = 1; +} + +#define NOP "nop" +#define BTI_N "hint #32" +#define BTI_C "hint #34" +#define BTI_J "hint #36" +#define BTI_JC "hint #38" + +#define BTYPE_1(DEST) \ + asm("mov %0,#1; adr x16, 1f; br x16; 1: " DEST "; mov %0,#0" \ + : "=r"(skipped) : : "x16") + +#define BTYPE_2(DEST) \ + asm("mov %0,#1; adr x16, 1f; blr x16; 1: " DEST "; mov %0,#0" \ + : "=r"(skipped) : : "x16", "x30") + +#define BTYPE_3(DEST) \ + asm("mov %0,#1; adr x15, 1f; br x15; 1: " DEST "; mov %0,#0" \ + : "=r"(skipped) : : "x15") + +#define TEST(WHICH, DEST, EXPECT) \ + do { WHICH(DEST); fail += skipped ^ EXPECT; } while (0) + + +int main() +{ + int fail = 0; + int skipped; + + /* Signal-like with SA_SIGINFO. */ + signal_info(SIGILL, skip2_sigill); + + TEST(BTYPE_1, NOP, 1); + TEST(BTYPE_1, BTI_N, 1); + TEST(BTYPE_1, BTI_C, 0); + TEST(BTYPE_1, BTI_J, 0); + TEST(BTYPE_1, BTI_JC, 0); + + TEST(BTYPE_2, NOP, 1); + TEST(BTYPE_2, BTI_N, 1); + TEST(BTYPE_2, BTI_C, 0); + TEST(BTYPE_2, BTI_J, 1); + TEST(BTYPE_2, BTI_JC, 0); + + TEST(BTYPE_3, NOP, 1); + TEST(BTYPE_3, BTI_N, 1); + TEST(BTYPE_3, BTI_C, 1); + TEST(BTYPE_3, BTI_J, 0); + TEST(BTYPE_3, BTI_JC, 0); + + return fail; +} diff --git a/tests/tcg/aarch64/bti-2.c b/tests/tcg/aarch64/bti-2.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/tests/tcg/aarch64/bti-2.c @@ -XXX,XX +XXX,XX @@ +/* + * Branch target identification, basic notskip cases. + */ + +#include <stdio.h> +#include <signal.h> +#include <string.h> +#include <unistd.h> +#include <sys/mman.h> + +#ifndef PROT_BTI +#define PROT_BTI 0x10 +#endif + +static void skip2_sigill(int sig, siginfo_t *info, void *vuc) +{ + ucontext_t *uc = vuc; + uc->uc_mcontext.pc += 8; + uc->uc_mcontext.pstate = 1; +} + +#define NOP "nop" +#define BTI_N "hint #32" +#define BTI_C "hint #34" +#define BTI_J "hint #36" +#define BTI_JC "hint #38" + +#define BTYPE_1(DEST) \ + "mov x1, #1\n\t" \ + "adr x16, 1f\n\t" \ + "br x16\n" \ +"1: " DEST "\n\t" \ + "mov x1, #0" + +#define BTYPE_2(DEST) \ + "mov x1, #1\n\t" \ + "adr x16, 1f\n\t" \ + "blr x16\n" \ +"1: " DEST "\n\t" \ + "mov x1, #0" + +#define BTYPE_3(DEST) \ + "mov x1, #1\n\t" \ + "adr x15, 1f\n\t" \ + "br x15\n" \ +"1: " DEST "\n\t" \ + "mov x1, #0" + +#define TEST(WHICH, DEST, EXPECT) \ + WHICH(DEST) "\n" \ + ".if " #EXPECT "\n\t" \ + "eor x1, x1," #EXPECT "\n" \ + ".endif\n\t" \ + "add x0, x0, x1\n\t" + +extern char test_begin[], test_end[]; + +asm("\n" +"test_begin:\n\t" + BTI_C "\n\t" + "mov x2, x30\n\t" + "mov x0, #0\n\t" + + TEST(BTYPE_1, NOP, 1) + TEST(BTYPE_1, BTI_N, 1) + TEST(BTYPE_1, BTI_C, 0) + TEST(BTYPE_1, BTI_J, 0) + TEST(BTYPE_1, BTI_JC, 0) + + TEST(BTYPE_2, NOP, 1) + TEST(BTYPE_2, BTI_N, 1) + TEST(BTYPE_2, BTI_C, 0) + TEST(BTYPE_2, BTI_J, 1) + TEST(BTYPE_2, BTI_JC, 0) + + TEST(BTYPE_3, NOP, 1) + TEST(BTYPE_3, BTI_N, 1) + TEST(BTYPE_3, BTI_C, 1) + TEST(BTYPE_3, BTI_J, 0) + TEST(BTYPE_3, BTI_JC, 0) + + "ret x2\n" +"test_end:" +); + +int main() +{ + struct sigaction sa; + + void *p = mmap(0, getpagesize(), + PROT_EXEC | PROT_READ | PROT_WRITE | PROT_BTI, + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + if (p == MAP_FAILED) { + perror("mmap"); + return 1; + } + + memset(&sa, 0, sizeof(sa)); + sa.sa_sigaction = skip2_sigill; + sa.sa_flags = SA_SIGINFO; + if (sigaction(SIGILL, &sa, NULL) < 0) { + perror("sigaction"); + return 1; + } + + memcpy(p, test_begin, test_end - test_begin); + return ((int (*)(void))p)(); +} diff --git a/tests/tcg/aarch64/bti-crt.inc.c b/tests/tcg/aarch64/bti-crt.inc.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/tests/tcg/aarch64/bti-crt.inc.c @@ -XXX,XX +XXX,XX @@ +/* + * Minimal user-environment for testing BTI. + * + * Normal libc is not (yet) built with BTI support enabled, + * and so could generate a BTI TRAP before ever reaching main. + */ + +#include <stdlib.h> +#include <signal.h> +#include <ucontext.h> +#include <asm/unistd.h> + +int main(void); + +void _start(void) +{ + exit(main()); +} + +void exit(int ret) +{ + register int x0 __asm__("x0") = ret; + register int x8 __asm__("x8") = __NR_exit; + + asm volatile("svc #0" : : "r"(x0), "r"(x8)); + __builtin_unreachable(); +} + +/* + * Irritatingly, the user API struct sigaction does not match the + * kernel API struct sigaction. So for simplicity, isolate the + * kernel ABI here, and make this act like signal. + */ +void signal_info(int sig, void (*fn)(int, siginfo_t *, ucontext_t *)) +{ + struct kernel_sigaction { + void (*handler)(int, siginfo_t *, ucontext_t *); + unsigned long flags; + unsigned long restorer; + unsigned long mask; + } sa = { fn, SA_SIGINFO, 0, 0 }; + + register int x0 __asm__("x0") = sig; + register void *x1 __asm__("x1") = &sa; + register void *x2 __asm__("x2") = 0; + register int x3 __asm__("x3") = sizeof(unsigned long); + register int x8 __asm__("x8") = __NR_rt_sigaction; + + asm volatile("svc #0" + : : "r"(x0), "r"(x1), "r"(x2), "r"(x3), "r"(x8) : "memory"); +} diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target index XXXXXXX..XXXXXXX 100644 --- a/tests/tcg/aarch64/Makefile.target +++ b/tests/tcg/aarch64/Makefile.target @@ -XXX,XX +XXX,XX @@ run-pauth-%: QEMU_OPTS += -cpu max run-plugin-pauth-%: QEMU_OPTS += -cpu max endif +# BTI Tests +# bti-1 tests the elf notes, so we require special compiler support. +ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_ARMV8_BTI),) +AARCH64_TESTS += bti-1 +bti-1: CFLAGS += -mbranch-protection=standard +bti-1: LDFLAGS += -nostdlib +endif +# bti-2 tests PROT_BTI, so no special compiler support required. +AARCH64_TESTS += bti-2 + # Semihosting smoke test for linux-user AARCH64_TESTS += semihosting run-semihosting: semihosting diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh index XXXXXXX..XXXXXXX 100755 --- a/tests/tcg/configure.sh +++ b/tests/tcg/configure.sh @@ -XXX,XX +XXX,XX @@ for target in $target_list; do -march=armv8.3-a -o $TMPE $TMPC; then echo "CROSS_CC_HAS_ARMV8_3=y" >> $config_target_mak fi + if do_compiler "$target_compiler" $target_compiler_cflags \ + -mbranch-protection=standard -o $TMPE $TMPC; then + echo "CROSS_CC_HAS_ARMV8_BTI=y" >> $config_target_mak + fi ;; esac -- 2.20.1 Nothing too exciting, but does include the last bits of v8.1M support work. -- PMM The following changes since commit e79de63ab1bd1f6550e7b915e433bec1ad1a870a: Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210107' into staging (2021-01-07 20:34:05 +0000) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20210108 for you to fetch changes up to c9f8511ea8d2b80723af0fea1f716d752c1b5208: docs/system: arm: Add sabrelite board description (2021-01-08 15:13:39 +0000) ---------------------------------------------------------------- target-arm queue: * intc/arm_gic: Fix gic_irq_signaling_enabled() for vCPUs * target/arm: Fix MTE0_ACTIVE * target/arm: Implement v8.1M and Cortex-M55 model * hw/arm/highbank: Drop dead KVM support code * util/qemu-timer: Make timer_free() imply timer_del() * various devices: Use ptimer_free() in finalize function * docs/system: arm: Add sabrelite board description * sabrelite: Minor fixes to allow booting U-Boot ---------------------------------------------------------------- Andrew Jones (1): hw/arm/virt: Remove virt machine state 'smp_cpus' Bin Meng (4): hw/misc: imx6_ccm: Update PMU_MISC0 reset value hw/msic: imx6_ccm: Correct register value for silicon type hw/arm: sabrelite: Connect the Ethernet PHY at address 6 docs/system: arm: Add sabrelite board description Edgar E. Iglesias (1): intc/arm_gic: Fix gic_irq_signaling_enabled() for vCPUs Gan Qixin (7): digic-timer: Use ptimer_free() in the finalize function to avoid memleaks allwinner-a10-pit: Use ptimer_free() in the finalize function to avoid memleaks exynos4210_rtc: Use ptimer_free() in the finalize function to avoid memleaks exynos4210_pwm: Use ptimer_free() in the finalize function to avoid memleaks mss-timer: Use ptimer_free() in the finalize function to avoid memleaks musicpal: Use ptimer_free() in the finalize function to avoid memleaks exynos4210_mct: Use ptimer_free() in the finalize function to avoid memleaks Peter Maydell (9): hw/intc/armv7m_nvic: Correct handling of CCR.BFHFNMIGN target/arm: Correct store of FPSCR value via FPCXT_S target/arm: Implement FPCXT_NS fp system register target/arm: Implement Cortex-M55 model hw/arm/highbank: Drop dead KVM support code util/qemu-timer: Make timer_free() imply timer_del() scripts/coccinelle: New script to remove unnecessary timer_del() calls Remove superfluous timer_del() calls target/arm: Remove timer_del()/timer_deinit() before timer_free() Richard Henderson (1): target/arm: Fix MTE0_ACTIVE docs/system/arm/sabrelite.rst | 119 ++++++++++++++++++++++++++ docs/system/target-arm.rst | 1 + scripts/coccinelle/timer-del-timer-free.cocci | 18 ++++ include/hw/arm/virt.h | 3 +- include/qemu/timer.h | 24 +++--- block/iscsi.c | 2 - block/nbd.c | 1 - block/qcow2.c | 1 - hw/arm/highbank.c | 14 +-- hw/arm/musicpal.c | 12 +++ hw/arm/sabrelite.c | 4 + hw/arm/virt-acpi-build.c | 9 +- hw/arm/virt.c | 21 +++-- hw/block/nvme.c | 2 - hw/char/serial.c | 2 - hw/char/virtio-serial-bus.c | 2 - hw/ide/core.c | 1 - hw/input/hid.c | 1 - hw/intc/apic.c | 1 - hw/intc/arm_gic.c | 4 +- hw/intc/armv7m_nvic.c | 15 ++++ hw/intc/ioapic.c | 1 - hw/ipmi/ipmi_bmc_extern.c | 1 - hw/misc/imx6_ccm.c | 4 +- hw/net/e1000.c | 3 - hw/net/e1000e_core.c | 8 -- hw/net/pcnet-pci.c | 1 - hw/net/rtl8139.c | 1 - hw/net/spapr_llan.c | 1 - hw/net/virtio-net.c | 2 - hw/rtc/exynos4210_rtc.c | 9 ++ hw/s390x/s390-pci-inst.c | 1 - hw/sd/sd.c | 1 - hw/sd/sdhci.c | 2 - hw/timer/allwinner-a10-pit.c | 11 +++ hw/timer/digic-timer.c | 8 ++ hw/timer/exynos4210_mct.c | 14 +++ hw/timer/exynos4210_pwm.c | 11 +++ hw/timer/mss-timer.c | 13 +++ hw/usb/dev-hub.c | 1 - hw/usb/hcd-ehci.c | 1 - hw/usb/hcd-ohci-pci.c | 1 - hw/usb/hcd-uhci.c | 1 - hw/usb/hcd-xhci.c | 1 - hw/usb/redirect.c | 1 - hw/vfio/display.c | 1 - hw/virtio/vhost-vsock-common.c | 1 - hw/virtio/virtio-balloon.c | 1 - hw/virtio/virtio-rng.c | 1 - hw/watchdog/wdt_diag288.c | 1 - hw/watchdog/wdt_i6300esb.c | 1 - migration/colo.c | 1 - monitor/hmp-cmds.c | 1 - net/announce.c | 1 - net/colo-compare.c | 1 - net/slirp.c | 1 - replay/replay-debugging.c | 1 - target/arm/cpu.c | 2 - target/arm/cpu_tcg.c | 42 +++++++++ target/arm/helper.c | 2 +- target/s390x/cpu.c | 2 - ui/console.c | 1 - ui/spice-core.c | 1 - util/throttle.c | 1 - target/arm/translate-vfp.c.inc | 114 ++++++++++++++++++++++-- 65 files changed, 421 insertions(+), 111 deletions(-) create mode 100644 docs/system/arm/sabrelite.rst create mode 100644 scripts/coccinelle/timer-del-timer-free.cocci From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> Correct the indexing into s->cpu_ctlr for vCPUs. Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc.michel@greensocs.com> Message-id: 20201214222154.3480243-2-edgar.iglesias@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/arm_gic.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/arm_gic.c +++ b/hw/intc/arm_gic.c @@ -XXX,XX +XXX,XX @@ static inline void gic_get_best_virq(GICState *s, int cpu, static inline bool gic_irq_signaling_enabled(GICState *s, int cpu, bool virt, int group_mask) { + int cpu_iface = virt ? (cpu + GIC_NCPU) : cpu; + if (!virt && !(s->ctlr & group_mask)) { return false; } @@ -XXX,XX +XXX,XX @@ static inline bool gic_irq_signaling_enabled(GICState *s, int cpu, bool virt, return false; } - if (!(s->cpu_ctlr[cpu] & group_mask)) { + if (!(s->cpu_ctlr[cpu_iface] & group_mask)) { return false; } -- 2.20.1 From: Andrew Jones <drjones@redhat.com> virt machine's 'smp_cpus' and machine->smp.cpus must always have the same value. And, anywhere we have virt machine state we have machine state. So let's remove the redundancy. Also, to make it easier to see that machine->smp is the true source for "smp_cpus" and "max_cpus", avoid passing them in function parameters, preferring instead to get them from the state. No functional change intended. Signed-off-by: Andrew Jones <drjones@redhat.com> Reviewed-by: David Edmondson <david.edmondson@oracle.com> Reviewed-by: Ying Fang <fangying1@huawei.com> Message-id: 20201215174815.51520-1-drjones@redhat.com [PMM: minor formatting tweak to smp_cpus variable declaration] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/virt.h | 3 +-- hw/arm/virt-acpi-build.c | 9 +++++---- hw/arm/virt.c | 21 ++++++++++----------- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -XXX,XX +XXX,XX @@ struct VirtMachineState { MemMapEntry *memmap; char *pciehb_nodename; const int *irqmap; - int smp_cpus; void *fdt; int fdt_size; uint32_t clock_phandle; @@ -XXX,XX +XXX,XX @@ static inline int virt_gicv3_redist_region_count(VirtMachineState *vms) assert(vms->gic_version == VIRT_GIC_VERSION_3); - return vms->smp_cpus > redist0_capacity ? 2 : 1; + return MACHINE(vms)->smp.cpus > redist0_capacity ? 2 : 1; } #endif /* QEMU_ARM_VIRT_H */ diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -XXX,XX +XXX,XX @@ #define ACPI_BUILD_TABLE_SIZE 0x20000 -static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus) +static void acpi_dsdt_add_cpus(Aml *scope, VirtMachineState *vms) { + MachineState *ms = MACHINE(vms); uint16_t i; - for (i = 0; i < smp_cpus; i++) { + for (i = 0; i < ms->smp.cpus; i++) { Aml *dev = aml_device("C%.03X", i); aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0007"))); aml_append(dev, aml_name_decl("_UID", aml_int(i))); @@ -XXX,XX +XXX,XX @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) gicd->base_address = cpu_to_le64(memmap[VIRT_GIC_DIST].base); gicd->version = vms->gic_version; - for (i = 0; i < vms->smp_cpus; i++) { + for (i = 0; i < MACHINE(vms)->smp.cpus; i++) { AcpiMadtGenericCpuInterface *gicc = acpi_data_push(table_data, sizeof(*gicc)); ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(i)); @@ -XXX,XX +XXX,XX @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) * the RTC ACPI device at all when using UEFI. */ scope = aml_scope("\\_SB"); - acpi_dsdt_add_cpus(scope, vms->smp_cpus); + acpi_dsdt_add_cpus(scope, vms); acpi_dsdt_add_uart(scope, &memmap[VIRT_UART], (irqmap[VIRT_UART] + ARM_SPI_BASE)); if (vmc->acpi_expose_flash) { diff --git a/hw/arm/virt.c b/hw/arm/virt.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -XXX,XX +XXX,XX @@ static void fdt_add_timer_nodes(const VirtMachineState *vms) if (vms->gic_version == VIRT_GIC_VERSION_2) { irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START, GIC_FDT_IRQ_PPI_CPU_WIDTH, - (1 << vms->smp_cpus) - 1); + (1 << MACHINE(vms)->smp.cpus) - 1); } qemu_fdt_add_subnode(vms->fdt, "/timer"); @@ -XXX,XX +XXX,XX @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms) int cpu; int addr_cells = 1; const MachineState *ms = MACHINE(vms); + int smp_cpus = ms->smp.cpus; /* * From Documentation/devicetree/bindings/arm/cpus.txt @@ -XXX,XX +XXX,XX @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms) * The simplest way to go is to examine affinity IDs of all our CPUs. If * at least one of them has Aff3 populated, we set #address-cells to 2. */ - for (cpu = 0; cpu < vms->smp_cpus; cpu++) { + for (cpu = 0; cpu < smp_cpus; cpu++) { ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(cpu)); if (armcpu->mp_affinity & ARM_AFF3_MASK) { @@ -XXX,XX +XXX,XX @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms) qemu_fdt_setprop_cell(vms->fdt, "/cpus", "#address-cells", addr_cells); qemu_fdt_setprop_cell(vms->fdt, "/cpus", "#size-cells", 0x0); - for (cpu = vms->smp_cpus - 1; cpu >= 0; cpu--) { + for (cpu = smp_cpus - 1; cpu >= 0; cpu--) { char *nodename = g_strdup_printf("/cpus/cpu@%d", cpu); ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(cpu)); CPUState *cs = CPU(armcpu); @@ -XXX,XX +XXX,XX @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms) qemu_fdt_setprop_string(vms->fdt, nodename, "compatible", armcpu->dtb_compatible); - if (vms->psci_conduit != QEMU_PSCI_CONDUIT_DISABLED - && vms->smp_cpus > 1) { + if (vms->psci_conduit != QEMU_PSCI_CONDUIT_DISABLED && smp_cpus > 1) { qemu_fdt_setprop_string(vms->fdt, nodename, "enable-method", "psci"); } @@ -XXX,XX +XXX,XX @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms) if (vms->gic_version == VIRT_GIC_VERSION_2) { irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START, GIC_FDT_IRQ_PPI_CPU_WIDTH, - (1 << vms->smp_cpus) - 1); + (1 << MACHINE(vms)->smp.cpus) - 1); } qemu_fdt_add_subnode(vms->fdt, "/pmu"); @@ -XXX,XX +XXX,XX @@ static void finalize_gic_version(VirtMachineState *vms) * virt_cpu_post_init() must be called after the CPUs have * been realized and the GIC has been created. */ -static void virt_cpu_post_init(VirtMachineState *vms, int max_cpus, - MemoryRegion *sysmem) +static void virt_cpu_post_init(VirtMachineState *vms, MemoryRegion *sysmem) { + int max_cpus = MACHINE(vms)->smp.max_cpus; bool aarch64, pmu, steal_time; CPUState *cpu; @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) exit(1); } - vms->smp_cpus = smp_cpus; - if (vms->virt && kvm_enabled()) { error_report("mach-virt: KVM does not support providing " "Virtualization extensions to the guest CPU"); @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) create_fdt(vms); possible_cpus = mc->possible_cpu_arch_ids(machine); + assert(possible_cpus->len == max_cpus); for (n = 0; n < possible_cpus->len; n++) { Object *cpuobj; CPUState *cs; @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) create_gic(vms); - virt_cpu_post_init(vms, possible_cpus->len, sysmem); + virt_cpu_post_init(vms, sysmem); fdt_add_pmu_nodes(vms); -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> In 50244cc76abc we updated mte_check_fail to match the ARM pseudocode, using the correct EL to select the TCF field. But we failed to update MTE0_ACTIVE the same way, which led to g_assert_not_reached(). Cc: qemu-stable@nongnu.org Buglink: https://bugs.launchpad.net/bugs/1907137 Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201221204426.88514-1-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static uint32_t rebuild_hflags_a64(CPUARMState *env, int el, int fp_el, if (FIELD_EX32(flags, TBFLAG_A64, UNPRIV) && tbid && !(env->pstate & PSTATE_TCO) - && (sctlr & SCTLR_TCF0) + && (sctlr & SCTLR_TCF) && allocation_tag_access_enabled(env, 0, sctlr)) { flags = FIELD_DP32(flags, TBFLAG_A64, MTE0_ACTIVE, 1); } -- 2.20.1 The CCR is a register most of whose bits are banked between security states but where BFHFNMIGN is not, and we keep it in the non-secure entry of the v7m.ccr[] array. The logic which tries to handle this bit fails to implement the "RAZ/WI from Nonsecure if AIRCR.BFHFNMINS is zero" requirement; correct the omission. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201210201433.26262-2-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) */ val = cpu->env.v7m.ccr[attrs.secure]; val |= cpu->env.v7m.ccr[M_REG_NS] & R_V7M_CCR_BFHFNMIGN_MASK; + /* BFHFNMIGN is RAZ/WI from NS if AIRCR.BFHFNMINS is 0 */ + if (!attrs.secure) { + if (!(cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) { + val &= ~R_V7M_CCR_BFHFNMIGN_MASK; + } + } return val; case 0xd24: /* System Handler Control and State (SHCSR) */ if (!arm_feature(&cpu->env, ARM_FEATURE_V7)) { @@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, (cpu->env.v7m.ccr[M_REG_NS] & ~R_V7M_CCR_BFHFNMIGN_MASK) | (value & R_V7M_CCR_BFHFNMIGN_MASK); value &= ~R_V7M_CCR_BFHFNMIGN_MASK; + } else { + /* + * BFHFNMIGN is RAZ/WI from NS if AIRCR.BFHFNMINS is 0, so + * preserve the state currently in the NS element of the array + */ + if (!(cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) { + value &= ~R_V7M_CCR_BFHFNMIGN_MASK; + value |= cpu->env.v7m.ccr[M_REG_NS] & R_V7M_CCR_BFHFNMIGN_MASK; + } } cpu->env.v7m.ccr[attrs.secure] = value; -- 2.20.1 In commit 64f863baeedc8659 we implemented the v8.1M FPCXT_S register, but we got the write behaviour wrong. On read, this register reads bits [27:0] of FPSCR plus the CONTROL.SFPA bit. On write, it doesn't just write back those bits -- it writes a value to the whole FPSCR, whose upper 4 bits are zeroes. We also incorrectly implemented the write-to-FPSCR as a simple store to vfp.xregs; this skips the "update the softfloat flags" part of the vfp_set_fpscr helper so the value would read back correctly but not actually take effect. Fix both of these things by doing a complete write to the FPSCR using the helper function. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201210201433.26262-3-peter.maydell@linaro.org --- target/arm/translate-vfp.c.inc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-vfp.c.inc +++ b/target/arm/translate-vfp.c.inc @@ -XXX,XX +XXX,XX @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int regno, } case ARM_VFP_FPCXT_S: { - TCGv_i32 sfpa, control, fpscr; - /* Set FPSCR[27:0] and CONTROL.SFPA from value */ + TCGv_i32 sfpa, control; + /* + * Set FPSCR and CONTROL.SFPA from value; the new FPSCR takes + * bits [27:0] from value and zeroes bits [31:28]. + */ tmp = loadfn(s, opaque); sfpa = tcg_temp_new_i32(); tcg_gen_shri_i32(sfpa, tmp, 31); @@ -XXX,XX +XXX,XX @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int regno, tcg_gen_deposit_i32(control, control, sfpa, R_V7M_CONTROL_SFPA_SHIFT, 1); store_cpu_field(control, v7m.control[M_REG_S]); - fpscr = load_cpu_field(vfp.xregs[ARM_VFP_FPSCR]); - tcg_gen_andi_i32(fpscr, fpscr, FPCR_NZCV_MASK); tcg_gen_andi_i32(tmp, tmp, ~FPCR_NZCV_MASK); - tcg_gen_or_i32(fpscr, fpscr, tmp); - store_cpu_field(fpscr, vfp.xregs[ARM_VFP_FPSCR]); + gen_helper_vfp_set_fpscr(cpu_env, tmp); tcg_temp_free_i32(tmp); tcg_temp_free_i32(sfpa); break; -- 2.20.1 Implement the v8.1M FPCXT_NS floating-point system register. This is a little more complicated than FPCXT_S, because it has specific handling for "current FP state is inactive", and it only wants to do PreserveFPState(), not the full set of actions done by ExecuteFPCheck() which vfp_access_check() implements. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201210201433.26262-4-peter.maydell@linaro.org --- target/arm/translate-vfp.c.inc | 102 ++++++++++++++++++++++++++++++++- 1 file changed, 99 insertions(+), 3 deletions(-) diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-vfp.c.inc +++ b/target/arm/translate-vfp.c.inc @@ -XXX,XX +XXX,XX @@ static FPSysRegCheckResult fp_sysreg_checks(DisasContext *s, int regno) } break; case ARM_VFP_FPCXT_S: + case ARM_VFP_FPCXT_NS: if (!arm_dc_feature(s, ARM_FEATURE_V8_1M)) { return false; } @@ -XXX,XX +XXX,XX @@ static FPSysRegCheckResult fp_sysreg_checks(DisasContext *s, int regno) return FPSysRegCheckFailed; } - if (!vfp_access_check(s)) { + /* + * FPCXT_NS is a special case: it has specific handling for + * "current FP state is inactive", and must do the PreserveFPState() + * but not the usual full set of actions done by ExecuteFPCheck(). + * So we don't call vfp_access_check() and the callers must handle this. + */ + if (regno != ARM_VFP_FPCXT_NS && !vfp_access_check(s)) { return FPSysRegCheckDone; } - return FPSysRegCheckContinue; } +static void gen_branch_fpInactive(DisasContext *s, TCGCond cond, + TCGLabel *label) +{ + /* + * FPCXT_NS is a special case: it has specific handling for + * "current FP state is inactive", and must do the PreserveFPState() + * but not the usual full set of actions done by ExecuteFPCheck(). + * We don't have a TB flag that matches the fpInactive check, so we + * do it at runtime as we don't expect FPCXT_NS accesses to be frequent. + * + * Emit code that checks fpInactive and does a conditional + * branch to label based on it: + * if cond is TCG_COND_NE then branch if fpInactive != 0 (ie if inactive) + * if cond is TCG_COND_EQ then branch if fpInactive == 0 (ie if active) + */ + assert(cond == TCG_COND_EQ || cond == TCG_COND_NE); + + /* fpInactive = FPCCR_NS.ASPEN == 1 && CONTROL.FPCA == 0 */ + TCGv_i32 aspen, fpca; + aspen = load_cpu_field(v7m.fpccr[M_REG_NS]); + fpca = load_cpu_field(v7m.control[M_REG_S]); + tcg_gen_andi_i32(aspen, aspen, R_V7M_FPCCR_ASPEN_MASK); + tcg_gen_xori_i32(aspen, aspen, R_V7M_FPCCR_ASPEN_MASK); + tcg_gen_andi_i32(fpca, fpca, R_V7M_CONTROL_FPCA_MASK); + tcg_gen_or_i32(fpca, fpca, aspen); + tcg_gen_brcondi_i32(tcg_invert_cond(cond), fpca, 0, label); + tcg_temp_free_i32(aspen); + tcg_temp_free_i32(fpca); +} + static bool gen_M_fp_sysreg_write(DisasContext *s, int regno, fp_sysreg_loadfn *loadfn, @@ -XXX,XX +XXX,XX @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int regno, { /* Do a write to an M-profile floating point system register */ TCGv_i32 tmp; + TCGLabel *lab_end = NULL; switch (fp_sysreg_checks(s, regno)) { case FPSysRegCheckFailed: @@ -XXX,XX +XXX,XX @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int regno, tcg_temp_free_i32(tmp); break; } + case ARM_VFP_FPCXT_NS: + lab_end = gen_new_label(); + /* fpInactive case: write is a NOP, so branch to end */ + gen_branch_fpInactive(s, TCG_COND_NE, lab_end); + /* !fpInactive: PreserveFPState(), and reads same as FPCXT_S */ + gen_preserve_fp_state(s); + /* fall through */ case ARM_VFP_FPCXT_S: { TCGv_i32 sfpa, control; @@ -XXX,XX +XXX,XX @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int regno, default: g_assert_not_reached(); } + if (lab_end) { + gen_set_label(lab_end); + } return true; } @@ -XXX,XX +XXX,XX @@ static bool gen_M_fp_sysreg_read(DisasContext *s, int regno, { /* Do a read from an M-profile floating point system register */ TCGv_i32 tmp; + TCGLabel *lab_end = NULL; + bool lookup_tb = false; switch (fp_sysreg_checks(s, regno)) { case FPSysRegCheckFailed: @@ -XXX,XX +XXX,XX @@ static bool gen_M_fp_sysreg_read(DisasContext *s, int regno, fpscr = load_cpu_field(v7m.fpdscr[M_REG_NS]); gen_helper_vfp_set_fpscr(cpu_env, fpscr); tcg_temp_free_i32(fpscr); - gen_lookup_tb(s); + lookup_tb = true; + break; + } + case ARM_VFP_FPCXT_NS: + { + TCGv_i32 control, sfpa, fpscr, fpdscr, zero; + TCGLabel *lab_active = gen_new_label(); + + lookup_tb = true; + + gen_branch_fpInactive(s, TCG_COND_EQ, lab_active); + /* fpInactive case: reads as FPDSCR_NS */ + TCGv_i32 tmp = load_cpu_field(v7m.fpdscr[M_REG_NS]); + storefn(s, opaque, tmp); + lab_end = gen_new_label(); + tcg_gen_br(lab_end); + + gen_set_label(lab_active); + /* !fpInactive: Reads the same as FPCXT_S, but side effects differ */ + gen_preserve_fp_state(s); + tmp = tcg_temp_new_i32(); + sfpa = tcg_temp_new_i32(); + fpscr = tcg_temp_new_i32(); + gen_helper_vfp_get_fpscr(fpscr, cpu_env); + tcg_gen_andi_i32(tmp, fpscr, ~FPCR_NZCV_MASK); + control = load_cpu_field(v7m.control[M_REG_S]); + tcg_gen_andi_i32(sfpa, control, R_V7M_CONTROL_SFPA_MASK); + tcg_gen_shli_i32(sfpa, sfpa, 31 - R_V7M_CONTROL_SFPA_SHIFT); + tcg_gen_or_i32(tmp, tmp, sfpa); + tcg_temp_free_i32(control); + /* Store result before updating FPSCR, in case it faults */ + storefn(s, opaque, tmp); + /* If SFPA is zero then set FPSCR from FPDSCR_NS */ + fpdscr = load_cpu_field(v7m.fpdscr[M_REG_NS]); + zero = tcg_const_i32(0); + tcg_gen_movcond_i32(TCG_COND_EQ, fpscr, sfpa, zero, fpdscr, fpscr); + gen_helper_vfp_set_fpscr(cpu_env, fpscr); + tcg_temp_free_i32(zero); + tcg_temp_free_i32(sfpa); + tcg_temp_free_i32(fpdscr); + tcg_temp_free_i32(fpscr); break; } default: g_assert_not_reached(); } + + if (lab_end) { + gen_set_label(lab_end); + } + if (lookup_tb) { + gen_lookup_tb(s); + } return true; } -- 2.20.1 Now that we have implemented all the features needed by the v8.1M architecture, we can add the model of the Cortex-M55. This is the configuration without MVE support; we'll add MVE later. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201210201433.26262-5-peter.maydell@linaro.org --- target/arm/cpu_tcg.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu_tcg.c +++ b/target/arm/cpu_tcg.c @@ -XXX,XX +XXX,XX @@ static void cortex_m33_initfn(Object *obj) cpu->ctr = 0x8000c000; } +static void cortex_m55_initfn(Object *obj) +{ + ARMCPU *cpu = ARM_CPU(obj); + + set_feature(&cpu->env, ARM_FEATURE_V8); + set_feature(&cpu->env, ARM_FEATURE_V8_1M); + set_feature(&cpu->env, ARM_FEATURE_M); + set_feature(&cpu->env, ARM_FEATURE_M_MAIN); + set_feature(&cpu->env, ARM_FEATURE_M_SECURITY); + set_feature(&cpu->env, ARM_FEATURE_THUMB_DSP); + cpu->midr = 0x410fd221; /* r0p1 */ + cpu->revidr = 0; + cpu->pmsav7_dregion = 16; + cpu->sau_sregion = 8; + /* + * These are the MVFR* values for the FPU, no MVE configuration; + * we will update them later when we implement MVE + */ + cpu->isar.mvfr0 = 0x10110221; + cpu->isar.mvfr1 = 0x12100011; + cpu->isar.mvfr2 = 0x00000040; + cpu->isar.id_pfr0 = 0x20000030; + cpu->isar.id_pfr1 = 0x00000230; + cpu->isar.id_dfr0 = 0x10200000; + cpu->id_afr0 = 0x00000000; + cpu->isar.id_mmfr0 = 0x00111040; + cpu->isar.id_mmfr1 = 0x00000000; + cpu->isar.id_mmfr2 = 0x01000000; + cpu->isar.id_mmfr3 = 0x00000011; + cpu->isar.id_isar0 = 0x01103110; + cpu->isar.id_isar1 = 0x02212000; + cpu->isar.id_isar2 = 0x20232232; + cpu->isar.id_isar3 = 0x01111131; + cpu->isar.id_isar4 = 0x01310132; + cpu->isar.id_isar5 = 0x00000000; + cpu->isar.id_isar6 = 0x00000000; + cpu->clidr = 0x00000000; /* caches not implemented */ + cpu->ctr = 0x8303c003; +} + static const ARMCPRegInfo cortexr5_cp_reginfo[] = { /* Dummy the TCM region regs for the moment */ { .name = "ATCM", .cp = 15, .opc1 = 0, .crn = 9, .crm = 1, .opc2 = 0, @@ -XXX,XX +XXX,XX @@ static const ARMCPUInfo arm_tcg_cpus[] = { .class_init = arm_v7m_class_init }, { .name = "cortex-m33", .initfn = cortex_m33_initfn, .class_init = arm_v7m_class_init }, + { .name = "cortex-m55", .initfn = cortex_m55_initfn, + .class_init = arm_v7m_class_init }, { .name = "cortex-r5", .initfn = cortex_r5_initfn }, { .name = "cortex-r5f", .initfn = cortex_r5f_initfn }, { .name = "ti925t", .initfn = ti925t_initfn }, -- 2.20.1 Support for running KVM on 32-bit Arm hosts was removed in commit 82bf7ae84ce739e. You can still run a 32-bit guest on a 64-bit Arm host CPU, but because Arm KVM requires the host and guest CPU types to match, it is not possible to run a guest that requires a Cortex-A9 or Cortex-A15 CPU there. That means that the code in the highbank/midway board models to support KVM is no longer used, and we can delete 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: 20201215144215.28482-1-peter.maydell@linaro.org --- hw/arm/highbank.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/highbank.c +++ b/hw/arm/highbank.c @@ -XXX,XX +XXX,XX @@ #include "hw/arm/boot.h" #include "hw/loader.h" #include "net/net.h" -#include "sysemu/kvm.h" #include "sysemu/runstate.h" #include "sysemu/sysemu.h" #include "hw/boards.h" @@ -XXX,XX +XXX,XX @@ #include "hw/cpu/a15mpcore.h" #include "qemu/log.h" #include "qom/object.h" +#include "cpu.h" #define SMP_BOOT_ADDR 0x100 #define SMP_BOOT_REG 0x40 @@ -XXX,XX +XXX,XX @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id) highbank_binfo.loader_start = 0; highbank_binfo.write_secondary_boot = hb_write_secondary; highbank_binfo.secondary_cpu_reset_hook = hb_reset_secondary; - if (!kvm_enabled()) { - highbank_binfo.board_setup_addr = BOARD_SETUP_ADDR; - highbank_binfo.write_board_setup = hb_write_board_setup; - highbank_binfo.secure_board_setup = true; - } else { - warn_report("cannot load built-in Monitor support " - "if KVM is enabled. Some guests (such as Linux) " - "may not boot."); - } + highbank_binfo.board_setup_addr = BOARD_SETUP_ADDR; + highbank_binfo.write_board_setup = hb_write_board_setup; + highbank_binfo.secure_board_setup = true; arm_load_kernel(ARM_CPU(first_cpu), machine, &highbank_binfo); } -- 2.20.1 Currently timer_free() is a simple wrapper for g_free(). This means that the timer being freed must not be currently active, as otherwise QEMU might crash later when the active list is processed and still has a pointer to freed memory on it. As a result almost all calls to timer_free() are preceded by a timer_del() call, as can be seen in the output of git grep -B1 '\<timer_free\>' This is unfortunate API design as it makes it easy to accidentally misuse (by forgetting the timer_del()), and the correct use is annoyingly verbose. Make timer_free() imply a timer_del(). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201215154107.3255-2-peter.maydell@linaro.org --- include/qemu/timer.h | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/include/qemu/timer.h b/include/qemu/timer.h index XXXXXXX..XXXXXXX 100644 --- a/include/qemu/timer.h +++ b/include/qemu/timer.h @@ -XXX,XX +XXX,XX @@ static inline QEMUTimer *timer_new_ms(QEMUClockType type, QEMUTimerCB *cb, */ void timer_deinit(QEMUTimer *ts); -/** - * timer_free: - * @ts: the timer - * - * Free a timer (it must not be on the active list) - */ -static inline void timer_free(QEMUTimer *ts) -{ - g_free(ts); -} - /** * timer_del: * @ts: the timer @@ -XXX,XX +XXX,XX @@ static inline void timer_free(QEMUTimer *ts) */ void timer_del(QEMUTimer *ts); +/** + * timer_free: + * @ts: the timer + * + * Free a timer. This will call timer_del() for you to remove + * the timer from the active list if it was still active. + */ +static inline void timer_free(QEMUTimer *ts) +{ + timer_del(ts); + g_free(ts); +} + /** * timer_mod_ns: * @ts: the timer -- 2.20.1 Now that timer_free() implicitly calls timer_del(), sequences timer_del(mytimer); timer_free(mytimer); can be simplified to just timer_free(mytimer); Add a Coccinelle script to do this transformation. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201215154107.3255-3-peter.maydell@linaro.org --- scripts/coccinelle/timer-del-timer-free.cocci | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 scripts/coccinelle/timer-del-timer-free.cocci diff --git a/scripts/coccinelle/timer-del-timer-free.cocci b/scripts/coccinelle/timer-del-timer-free.cocci new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/scripts/coccinelle/timer-del-timer-free.cocci @@ -XXX,XX +XXX,XX @@ +// Remove superfluous timer_del() calls +// +// Copyright Linaro Limited 2020 +// This work is licensed under the terms of the GNU GPLv2 or later. +// +// spatch --macro-file scripts/cocci-macro-file.h \ +// --sp-file scripts/coccinelle/timer-del-timer-free.cocci \ +// --in-place --dir . +// +// The timer_free() function now implicitly calls timer_del() +// for you, so calls to timer_del() immediately before the +// timer_free() of the same timer can be deleted. + +@@ +expression T; +@@ +-timer_del(T); + timer_free(T); -- 2.20.1 This commit is the result of running the timer-del-timer-free.cocci script on the whole source tree. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Acked-by: Corey Minyard <cminyard@mvista.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201215154107.3255-4-peter.maydell@linaro.org --- block/iscsi.c | 2 -- block/nbd.c | 1 - block/qcow2.c | 1 - hw/block/nvme.c | 2 -- hw/char/serial.c | 2 -- hw/char/virtio-serial-bus.c | 2 -- hw/ide/core.c | 1 - hw/input/hid.c | 1 - hw/intc/apic.c | 1 - hw/intc/ioapic.c | 1 - hw/ipmi/ipmi_bmc_extern.c | 1 - hw/net/e1000.c | 3 --- hw/net/e1000e_core.c | 8 -------- hw/net/pcnet-pci.c | 1 - hw/net/rtl8139.c | 1 - hw/net/spapr_llan.c | 1 - hw/net/virtio-net.c | 2 -- hw/s390x/s390-pci-inst.c | 1 - hw/sd/sd.c | 1 - hw/sd/sdhci.c | 2 -- hw/usb/dev-hub.c | 1 - hw/usb/hcd-ehci.c | 1 - hw/usb/hcd-ohci-pci.c | 1 - hw/usb/hcd-uhci.c | 1 - hw/usb/hcd-xhci.c | 1 - hw/usb/redirect.c | 1 - hw/vfio/display.c | 1 - hw/virtio/vhost-vsock-common.c | 1 - hw/virtio/virtio-balloon.c | 1 - hw/virtio/virtio-rng.c | 1 - hw/watchdog/wdt_diag288.c | 1 - hw/watchdog/wdt_i6300esb.c | 1 - migration/colo.c | 1 - monitor/hmp-cmds.c | 1 - net/announce.c | 1 - net/colo-compare.c | 1 - net/slirp.c | 1 - replay/replay-debugging.c | 1 - target/s390x/cpu.c | 2 -- ui/console.c | 1 - ui/spice-core.c | 1 - util/throttle.c | 1 - 42 files changed, 58 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index XXXXXXX..XXXXXXX 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -XXX,XX +XXX,XX @@ static void iscsi_detach_aio_context(BlockDriverState *bs) iscsilun->events = 0; if (iscsilun->nop_timer) { - timer_del(iscsilun->nop_timer); timer_free(iscsilun->nop_timer); iscsilun->nop_timer = NULL; } if (iscsilun->event_timer) { - timer_del(iscsilun->event_timer); timer_free(iscsilun->event_timer); iscsilun->event_timer = NULL; } diff --git a/block/nbd.c b/block/nbd.c index XXXXXXX..XXXXXXX 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -XXX,XX +XXX,XX @@ static void nbd_recv_coroutines_wake_all(BDRVNBDState *s) static void reconnect_delay_timer_del(BDRVNBDState *s) { if (s->reconnect_delay_timer) { - timer_del(s->reconnect_delay_timer); timer_free(s->reconnect_delay_timer); s->reconnect_delay_timer = NULL; } diff --git a/block/qcow2.c b/block/qcow2.c index XXXXXXX..XXXXXXX 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -XXX,XX +XXX,XX @@ static void cache_clean_timer_del(BlockDriverState *bs) { BDRVQcow2State *s = bs->opaque; if (s->cache_clean_timer) { - timer_del(s->cache_clean_timer); timer_free(s->cache_clean_timer); s->cache_clean_timer = NULL; } diff --git a/hw/block/nvme.c b/hw/block/nvme.c index XXXXXXX..XXXXXXX 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -XXX,XX +XXX,XX @@ static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeRequest *req) static void nvme_free_sq(NvmeSQueue *sq, NvmeCtrl *n) { n->sq[sq->sqid] = NULL; - timer_del(sq->timer); timer_free(sq->timer); g_free(sq->io_req); if (sq->sqid) { @@ -XXX,XX +XXX,XX @@ static uint16_t nvme_get_log(NvmeCtrl *n, NvmeRequest *req) static void nvme_free_cq(NvmeCQueue *cq, NvmeCtrl *n) { n->cq[cq->cqid] = NULL; - timer_del(cq->timer); timer_free(cq->timer); msix_vector_unuse(&n->parent_obj, cq->vector); if (cq->cqid) { diff --git a/hw/char/serial.c b/hw/char/serial.c index XXXXXXX..XXXXXXX 100644 --- a/hw/char/serial.c +++ b/hw/char/serial.c @@ -XXX,XX +XXX,XX @@ static void serial_unrealize(DeviceState *dev) qemu_chr_fe_deinit(&s->chr, false); - timer_del(s->modem_status_poll); timer_free(s->modem_status_poll); - timer_del(s->fifo_timeout_timer); timer_free(s->fifo_timeout_timer); fifo8_destroy(&s->recv_fifo); diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c index XXXXXXX..XXXXXXX 100644 --- a/hw/char/virtio-serial-bus.c +++ b/hw/char/virtio-serial-bus.c @@ -XXX,XX +XXX,XX @@ static void virtio_serial_post_load_timer_cb(void *opaque) } } g_free(s->post_load->connected); - timer_del(s->post_load->timer); timer_free(s->post_load->timer); g_free(s->post_load); s->post_load = NULL; @@ -XXX,XX +XXX,XX @@ static void virtio_serial_device_unrealize(DeviceState *dev) g_free(vser->ports_map); if (vser->post_load) { g_free(vser->post_load->connected); - timer_del(vser->post_load->timer); timer_free(vser->post_load->timer); g_free(vser->post_load); } diff --git a/hw/ide/core.c b/hw/ide/core.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -XXX,XX +XXX,XX @@ void ide_init2(IDEBus *bus, qemu_irq irq) void ide_exit(IDEState *s) { - timer_del(s->sector_write_timer); timer_free(s->sector_write_timer); qemu_vfree(s->smart_selftest_data); qemu_vfree(s->io_buffer); diff --git a/hw/input/hid.c b/hw/input/hid.c index XXXXXXX..XXXXXXX 100644 --- a/hw/input/hid.c +++ b/hw/input/hid.c @@ -XXX,XX +XXX,XX @@ static void hid_idle_timer(void *opaque) static void hid_del_idle_timer(HIDState *hs) { if (hs->idle_timer) { - timer_del(hs->idle_timer); timer_free(hs->idle_timer); hs->idle_timer = NULL; } diff --git a/hw/intc/apic.c b/hw/intc/apic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/apic.c +++ b/hw/intc/apic.c @@ -XXX,XX +XXX,XX @@ static void apic_unrealize(DeviceState *dev) { APICCommonState *s = APIC(dev); - timer_del(s->timer); timer_free(s->timer); local_apics[s->id] = NULL; } diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/ioapic.c +++ b/hw/intc/ioapic.c @@ -XXX,XX +XXX,XX @@ static void ioapic_unrealize(DeviceState *dev) { IOAPICCommonState *s = IOAPIC_COMMON(dev); - timer_del(s->delayed_ioapic_service_timer); timer_free(s->delayed_ioapic_service_timer); } diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ipmi/ipmi_bmc_extern.c +++ b/hw/ipmi/ipmi_bmc_extern.c @@ -XXX,XX +XXX,XX @@ static void ipmi_bmc_extern_finalize(Object *obj) { IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(obj); - timer_del(ibe->extern_timer); timer_free(ibe->extern_timer); } diff --git a/hw/net/e1000.c b/hw/net/e1000.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/e1000.c +++ b/hw/net/e1000.c @@ -XXX,XX +XXX,XX @@ pci_e1000_uninit(PCIDevice *dev) { E1000State *d = E1000(dev); - timer_del(d->autoneg_timer); timer_free(d->autoneg_timer); - timer_del(d->mit_timer); timer_free(d->mit_timer); - timer_del(d->flush_queue_timer); timer_free(d->flush_queue_timer); qemu_del_nic(d->nic); } diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/e1000e_core.c +++ b/hw/net/e1000e_core.c @@ -XXX,XX +XXX,XX @@ e1000e_intrmgr_pci_unint(E1000ECore *core) { int i; - timer_del(core->radv.timer); timer_free(core->radv.timer); - timer_del(core->rdtr.timer); timer_free(core->rdtr.timer); - timer_del(core->raid.timer); timer_free(core->raid.timer); - timer_del(core->tadv.timer); timer_free(core->tadv.timer); - timer_del(core->tidv.timer); timer_free(core->tidv.timer); - timer_del(core->itr.timer); timer_free(core->itr.timer); for (i = 0; i < E1000E_MSIX_VEC_NUM; i++) { - timer_del(core->eitr[i].timer); timer_free(core->eitr[i].timer); } } @@ -XXX,XX +XXX,XX @@ e1000e_core_pci_uninit(E1000ECore *core) { int i; - timer_del(core->autoneg_timer); timer_free(core->autoneg_timer); e1000e_intrmgr_pci_unint(core); diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/pcnet-pci.c +++ b/hw/net/pcnet-pci.c @@ -XXX,XX +XXX,XX @@ static void pci_pcnet_uninit(PCIDevice *dev) PCIPCNetState *d = PCI_PCNET(dev); qemu_free_irq(d->state.irq); - timer_del(d->state.poll_timer); timer_free(d->state.poll_timer); qemu_del_nic(d->state.nic); } diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -XXX,XX +XXX,XX @@ static void pci_rtl8139_uninit(PCIDevice *dev) g_free(s->cplus_txbuffer); s->cplus_txbuffer = NULL; - timer_del(s->timer); timer_free(s->timer); qemu_del_nic(s->nic); } diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/spapr_llan.c +++ b/hw/net/spapr_llan.c @@ -XXX,XX +XXX,XX @@ static void spapr_vlan_instance_finalize(Object *obj) } if (dev->rxp_timer) { - timer_del(dev->rxp_timer); timer_free(dev->rxp_timer); } } diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -XXX,XX +XXX,XX @@ static void virtio_net_rsc_cleanup(VirtIONet *n) g_free(seg); } - timer_del(chain->drain_timer); timer_free(chain->drain_timer); QTAILQ_REMOVE(&n->rsc_chains, chain, next); g_free(chain); @@ -XXX,XX +XXX,XX @@ static void virtio_net_del_queue(VirtIONet *n, int index) virtio_del_queue(vdev, index * 2); if (q->tx_timer) { - timer_del(q->tx_timer); timer_free(q->tx_timer); q->tx_timer = NULL; } else { diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c index XXXXXXX..XXXXXXX 100644 --- a/hw/s390x/s390-pci-inst.c +++ b/hw/s390x/s390-pci-inst.c @@ -XXX,XX +XXX,XX @@ void pci_dereg_ioat(S390PCIIOMMU *iommu) void fmb_timer_free(S390PCIBusDevice *pbdev) { if (pbdev->fmb_timer) { - timer_del(pbdev->fmb_timer); timer_free(pbdev->fmb_timer); pbdev->fmb_timer = NULL; } diff --git a/hw/sd/sd.c b/hw/sd/sd.c index XXXXXXX..XXXXXXX 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -XXX,XX +XXX,XX @@ static void sd_instance_finalize(Object *obj) { SDState *sd = SD_CARD(obj); - timer_del(sd->ocr_power_timer); timer_free(sd->ocr_power_timer); } diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index XXXXXXX..XXXXXXX 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -XXX,XX +XXX,XX @@ void sdhci_initfn(SDHCIState *s) void sdhci_uninitfn(SDHCIState *s) { - timer_del(s->insert_timer); timer_free(s->insert_timer); - timer_del(s->transfer_timer); timer_free(s->transfer_timer); g_free(s->fifo_buffer); diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c index XXXXXXX..XXXXXXX 100644 --- a/hw/usb/dev-hub.c +++ b/hw/usb/dev-hub.c @@ -XXX,XX +XXX,XX @@ static void usb_hub_unrealize(USBDevice *dev) &s->ports[i].port); } - timer_del(s->port_timer); timer_free(s->port_timer); } diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index XXXXXXX..XXXXXXX 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -XXX,XX +XXX,XX @@ void usb_ehci_unrealize(EHCIState *s, DeviceState *dev) trace_usb_ehci_unrealize(); if (s->frame_timer) { - timer_del(s->frame_timer); timer_free(s->frame_timer); s->frame_timer = NULL; } diff --git a/hw/usb/hcd-ohci-pci.c b/hw/usb/hcd-ohci-pci.c index XXXXXXX..XXXXXXX 100644 --- a/hw/usb/hcd-ohci-pci.c +++ b/hw/usb/hcd-ohci-pci.c @@ -XXX,XX +XXX,XX @@ static void usb_ohci_exit(PCIDevice *dev) usb_bus_release(&s->bus); } - timer_del(s->eof_timer); timer_free(s->eof_timer); } diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index XXXXXXX..XXXXXXX 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -XXX,XX +XXX,XX @@ static void usb_uhci_exit(PCIDevice *dev) trace_usb_uhci_exit(); if (s->frame_timer) { - timer_del(s->frame_timer); timer_free(s->frame_timer); s->frame_timer = NULL; } diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index XXXXXXX..XXXXXXX 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -XXX,XX +XXX,XX @@ static void usb_xhci_unrealize(DeviceState *dev) } if (xhci->mfwrap_timer) { - timer_del(xhci->mfwrap_timer); timer_free(xhci->mfwrap_timer); xhci->mfwrap_timer = NULL; } diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c index XXXXXXX..XXXXXXX 100644 --- a/hw/usb/redirect.c +++ b/hw/usb/redirect.c @@ -XXX,XX +XXX,XX @@ static void usbredir_unrealize(USBDevice *udev) qemu_bh_delete(dev->chardev_close_bh); qemu_bh_delete(dev->device_reject_bh); - timer_del(dev->attach_timer); timer_free(dev->attach_timer); usbredir_cleanup_device_queues(dev); diff --git a/hw/vfio/display.c b/hw/vfio/display.c index XXXXXXX..XXXXXXX 100644 --- a/hw/vfio/display.c +++ b/hw/vfio/display.c @@ -XXX,XX +XXX,XX @@ static void vfio_display_edid_exit(VFIODisplay *dpy) g_free(dpy->edid_regs); g_free(dpy->edid_blob); - timer_del(dpy->edid_link_timer); timer_free(dpy->edid_link_timer); } diff --git a/hw/virtio/vhost-vsock-common.c b/hw/virtio/vhost-vsock-common.c index XXXXXXX..XXXXXXX 100644 --- a/hw/virtio/vhost-vsock-common.c +++ b/hw/virtio/vhost-vsock-common.c @@ -XXX,XX +XXX,XX @@ static void vhost_vsock_common_post_load_timer_cleanup(VHostVSockCommon *vvc) return; } - timer_del(vvc->post_load_timer); timer_free(vvc->post_load_timer); vvc->post_load_timer = NULL; } diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index XXXXXXX..XXXXXXX 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -XXX,XX +XXX,XX @@ static bool balloon_stats_enabled(const VirtIOBalloon *s) static void balloon_stats_destroy_timer(VirtIOBalloon *s) { if (balloon_stats_enabled(s)) { - timer_del(s->stats_timer); timer_free(s->stats_timer); s->stats_timer = NULL; s->stats_poll_interval = 0; diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c index XXXXXXX..XXXXXXX 100644 --- a/hw/virtio/virtio-rng.c +++ b/hw/virtio/virtio-rng.c @@ -XXX,XX +XXX,XX @@ static void virtio_rng_device_unrealize(DeviceState *dev) VirtIORNG *vrng = VIRTIO_RNG(dev); qemu_del_vm_change_state_handler(vrng->vmstate); - timer_del(vrng->rate_limit_timer); timer_free(vrng->rate_limit_timer); virtio_del_queue(vdev, 0); virtio_cleanup(vdev); diff --git a/hw/watchdog/wdt_diag288.c b/hw/watchdog/wdt_diag288.c index XXXXXXX..XXXXXXX 100644 --- a/hw/watchdog/wdt_diag288.c +++ b/hw/watchdog/wdt_diag288.c @@ -XXX,XX +XXX,XX @@ static void wdt_diag288_unrealize(DeviceState *dev) { DIAG288State *diag288 = DIAG288(dev); - timer_del(diag288->timer); timer_free(diag288->timer); } diff --git a/hw/watchdog/wdt_i6300esb.c b/hw/watchdog/wdt_i6300esb.c index XXXXXXX..XXXXXXX 100644 --- a/hw/watchdog/wdt_i6300esb.c +++ b/hw/watchdog/wdt_i6300esb.c @@ -XXX,XX +XXX,XX @@ static void i6300esb_exit(PCIDevice *dev) { I6300State *d = WATCHDOG_I6300ESB_DEVICE(dev); - timer_del(d->timer); timer_free(d->timer); } diff --git a/migration/colo.c b/migration/colo.c index XXXXXXX..XXXXXXX 100644 --- a/migration/colo.c +++ b/migration/colo.c @@ -XXX,XX +XXX,XX @@ out: * error. */ colo_compare_unregister_notifier(&packets_compare_notifier); - timer_del(s->colo_delay_timer); timer_free(s->colo_delay_timer); qemu_event_destroy(&s->colo_checkpoint_event); diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index XXXXXXX..XXXXXXX 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -XXX,XX +XXX,XX @@ static void hmp_migrate_status_cb(void *opaque) error_report("%s", info->error_desc); } monitor_resume(status->mon); - timer_del(status->timer); timer_free(status->timer); g_free(status); } diff --git a/net/announce.c b/net/announce.c index XXXXXXX..XXXXXXX 100644 --- a/net/announce.c +++ b/net/announce.c @@ -XXX,XX +XXX,XX @@ void qemu_announce_timer_del(AnnounceTimer *timer, bool free_named) { bool free_timer = false; if (timer->tm) { - timer_del(timer->tm); timer_free(timer->tm); timer->tm = NULL; } diff --git a/net/colo-compare.c b/net/colo-compare.c index XXXXXXX..XXXXXXX 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -XXX,XX +XXX,XX @@ static void colo_compare_timer_init(CompareState *s) static void colo_compare_timer_del(CompareState *s) { if (s->packet_check_timer) { - timer_del(s->packet_check_timer); timer_free(s->packet_check_timer); s->packet_check_timer = NULL; } diff --git a/net/slirp.c b/net/slirp.c index XXXXXXX..XXXXXXX 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -XXX,XX +XXX,XX @@ static void *net_slirp_timer_new(SlirpTimerCb cb, static void net_slirp_timer_free(void *timer, void *opaque) { - timer_del(timer); timer_free(timer); } diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c index XXXXXXX..XXXXXXX 100644 --- a/replay/replay-debugging.c +++ b/replay/replay-debugging.c @@ -XXX,XX +XXX,XX @@ static void replay_delete_break(void) assert(replay_mutex_locked()); if (replay_break_timer) { - timer_del(replay_break_timer); timer_free(replay_break_timer); replay_break_timer = NULL; } diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c index XXXXXXX..XXXXXXX 100644 --- a/target/s390x/cpu.c +++ b/target/s390x/cpu.c @@ -XXX,XX +XXX,XX @@ static void s390_cpu_finalize(Object *obj) #if !defined(CONFIG_USER_ONLY) S390CPU *cpu = S390_CPU(obj); - timer_del(cpu->env.tod_timer); timer_free(cpu->env.tod_timer); - timer_del(cpu->env.cpu_timer); timer_free(cpu->env.cpu_timer); qemu_unregister_reset(s390_cpu_machine_reset_cb, cpu); diff --git a/ui/console.c b/ui/console.c index XXXXXXX..XXXXXXX 100644 --- a/ui/console.c +++ b/ui/console.c @@ -XXX,XX +XXX,XX @@ static void gui_setup_refresh(DisplayState *ds) timer_mod(ds->gui_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME)); } if (!need_timer && ds->gui_timer != NULL) { - timer_del(ds->gui_timer); timer_free(ds->gui_timer); ds->gui_timer = NULL; } diff --git a/ui/spice-core.c b/ui/spice-core.c index XXXXXXX..XXXXXXX 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -XXX,XX +XXX,XX @@ static void timer_cancel(SpiceTimer *timer) static void timer_remove(SpiceTimer *timer) { - timer_del(timer->timer); timer_free(timer->timer); g_free(timer); } diff --git a/util/throttle.c b/util/throttle.c index XXXXXXX..XXXXXXX 100644 --- a/util/throttle.c +++ b/util/throttle.c @@ -XXX,XX +XXX,XX @@ static void throttle_timer_destroy(QEMUTimer **timer) { assert(*timer != NULL); - timer_del(*timer); timer_free(*timer); *timer = NULL; } -- 2.20.1 The Arm CPU finalize function uses a sequence of timer_del(), timer_deinit(), timer_free() to free the timer. The timer_deinit() step in this was always unnecessary, and now the timer_del() is implied by timer_free(), so we can collapse this down to simply calling timer_free(). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20201215154107.3255-5-peter.maydell@linaro.org --- target/arm/cpu.c | 2 -- 1 file changed, 2 deletions(-) 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_finalizefn(Object *obj) } #ifndef CONFIG_USER_ONLY if (cpu->pmu_timer) { - timer_del(cpu->pmu_timer); - timer_deinit(cpu->pmu_timer); timer_free(cpu->pmu_timer); } #endif -- 2.20.1 From: Gan Qixin <ganqixin@huawei.com> When running device-introspect-test, a memory leak occurred in the digic_timer_init function, so use ptimer_free() in the finalize function to avoid it. ASAN shows memory leak stack: Indirect leak of 288 byte(s) in 3 object(s) allocated from: #0 0xffffab97e1f0 in __interceptor_calloc (/lib64/libasan.so.5+0xee1f0) #1 0xffffab256800 in g_malloc0 (/lib64/libglib-2.0.so.0+0x56800) #2 0xaaabf555db78 in ptimer_init /qemu/hw/core/ptimer.c:432 #3 0xaaabf5b04084 in digic_timer_init /qemu/hw/timer/digic-timer.c:142 #4 0xaaabf6339f6c in object_initialize_with_type /qemu/qom/object.c:515 #5 0xaaabf633ca04 in object_initialize_child_with_propsv /qemu/qom/object.c:564 #6 0xaaabf633cc08 in object_initialize_child_with_props /qemu/qom/object.c:547 #7 0xaaabf5b40e84 in digic_init /qemu/hw/arm/digic.c:46 #8 0xaaabf6339f6c in object_initialize_with_type /qemu/qom/object.c:515 #9 0xaaabf633a1e0 in object_new_with_type /qemu/qom/object.c:729 #10 0xaaabf6375e40 in qmp_device_list_properties /qemu/qom/qom-qmp-cmds.c:153 #11 0xaaabf653d8ec in qmp_marshal_device_list_properties /qemu/qapi/qapi-commands-qdev.c:59 #12 0xaaabf6587d08 in do_qmp_dispatch_bh /qemu/qapi/qmp-dispatch.c:110 Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Gan Qixin <ganqixin@huawei.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/timer/digic-timer.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/hw/timer/digic-timer.c b/hw/timer/digic-timer.c index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/digic-timer.c +++ b/hw/timer/digic-timer.c @@ -XXX,XX +XXX,XX @@ static void digic_timer_init(Object *obj) sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem); } +static void digic_timer_finalize(Object *obj) +{ + DigicTimerState *s = DIGIC_TIMER(obj); + + ptimer_free(s->ptimer); +} + static void digic_timer_class_init(ObjectClass *klass, void *class_data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static const TypeInfo digic_timer_info = { .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(DigicTimerState), .instance_init = digic_timer_init, + .instance_finalize = digic_timer_finalize, .class_init = digic_timer_class_init, }; -- 2.20.1 From: Gan Qixin <ganqixin@huawei.com> When running device-introspect-test, a memory leak occurred in the a10_pit_init function, so use ptimer_free() in the finalize function to avoid it. ASAN shows memory leak stack: Indirect leak of 288 byte(s) in 6 object(s) allocated from: #0 0xffffab97e1f0 in __interceptor_calloc (/lib64/libasan.so.5+0xee1f0) #1 0xffffab256800 in g_malloc0 (/lib64/libglib-2.0.so.0+0x56800) #2 0xaaabf555db84 in timer_new_full /qemu/include/qemu/timer.h:523 #3 0xaaabf555db84 in timer_new /qemu/include/qemu/timer.h:544 #4 0xaaabf555db84 in timer_new_ns /qemu/include/qemu/timer.h:562 #5 0xaaabf555db84 in ptimer_init /qemu/hw/core/ptimer.c:433 #6 0xaaabf57415e8 in a10_pit_init /qemu/hw/timer/allwinner-a10-pit.c:278 #7 0xaaabf6339f6c in object_initialize_with_type /qemu/qom/object.c:515 #8 0xaaabf633ca04 in object_initialize_child_with_propsv /qemu/qom/object.c:564 #9 0xaaabf633cc08 in object_initialize_child_with_props /qemu/qom/object.c:547 #10 0xaaabf5b94680 in aw_a10_init /qemu/hw/arm/allwinner-a10.c:49 #11 0xaaabf6339f6c in object_initialize_with_type /qemu/qom/object.c:515 #12 0xaaabf633a1e0 in object_new_with_type /qemu/qom/object.c:729 Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Gan Qixin <ganqixin@huawei.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/timer/allwinner-a10-pit.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/hw/timer/allwinner-a10-pit.c b/hw/timer/allwinner-a10-pit.c index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/allwinner-a10-pit.c +++ b/hw/timer/allwinner-a10-pit.c @@ -XXX,XX +XXX,XX @@ static void a10_pit_init(Object *obj) } } +static void a10_pit_finalize(Object *obj) +{ + AwA10PITState *s = AW_A10_PIT(obj); + int i; + + for (i = 0; i < AW_A10_PIT_TIMER_NR; i++) { + ptimer_free(s->timer[i]); + } +} + static void a10_pit_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static const TypeInfo a10_pit_info = { .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(AwA10PITState), .instance_init = a10_pit_init, + .instance_finalize = a10_pit_finalize, .class_init = a10_pit_class_init, }; -- 2.20.1 From: Gan Qixin <ganqixin@huawei.com> When running device-introspect-test, a memory leak occurred in the exynos4210_rtc_init function, so use ptimer_free() in the finalize function to avoid it. ASAN shows memory leak stack: Indirect leak of 96 byte(s) in 1 object(s) allocated from: #0 0xffffab97e1f0 in __interceptor_calloc (/lib64/libasan.so.5+0xee1f0) #1 0xffffab256800 in g_malloc0 (/lib64/libglib-2.0.so.0+0x56800) #2 0xaaabf555db78 in ptimer_init /qemu/hw/core/ptimer.c:432 #3 0xaaabf57b3934 in exynos4210_rtc_init /qemu/hw/rtc/exynos4210_rtc.c:567 #4 0xaaabf6339f6c in object_initialize_with_type /qemu/qom/object.c:515 #5 0xaaabf633a1e0 in object_new_with_type /qemu/qom/object.c:729 #6 0xaaabf6375e40 in qmp_device_list_properties /qemu/qom/qom-qmp-cmds.c:153 #7 0xaaabf653d8ec in qmp_marshal_device_list_properties /qemu/qapi/qapi-commands-qdev.c:59 #8 0xaaabf6587d08 in do_qmp_dispatch_bh /qemu/qapi/qmp-dispatch.c:110 #9 0xaaabf6552708 in aio_bh_call /qemu/util/async.c:136 #10 0xaaabf6552708 in aio_bh_poll /qemu/util/async.c:164 #11 0xaaabf655f19c in aio_dispatch /qemu/util/aio-posix.c:381 #12 0xaaabf65523f4 in aio_ctx_dispatch /qemu/util/async.c:306 Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Gan Qixin <ganqixin@huawei.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/rtc/exynos4210_rtc.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/hw/rtc/exynos4210_rtc.c b/hw/rtc/exynos4210_rtc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/rtc/exynos4210_rtc.c +++ b/hw/rtc/exynos4210_rtc.c @@ -XXX,XX +XXX,XX @@ static void exynos4210_rtc_init(Object *obj) sysbus_init_mmio(dev, &s->iomem); } +static void exynos4210_rtc_finalize(Object *obj) +{ + Exynos4210RTCState *s = EXYNOS4210_RTC(obj); + + ptimer_free(s->ptimer); + ptimer_free(s->ptimer_1Hz); +} + static void exynos4210_rtc_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static const TypeInfo exynos4210_rtc_info = { .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(Exynos4210RTCState), .instance_init = exynos4210_rtc_init, + .instance_finalize = exynos4210_rtc_finalize, .class_init = exynos4210_rtc_class_init, }; -- 2.20.1 From: Gan Qixin <ganqixin@huawei.com> When running device-introspect-test, a memory leak occurred in the exynos4210_pwm_init function, so use ptimer_free() in the finalize function to avoid it. ASAN shows memory leak stack: Indirect leak of 240 byte(s) in 5 object(s) allocated from: #0 0xffffab97e1f0 in __interceptor_calloc (/lib64/libasan.so.5+0xee1f0) #1 0xffffab256800 in g_malloc0 (/lib64/libglib-2.0.so.0+0x56800) #2 0xaaabf555db84 in timer_new_full /qemu/include/qemu/timer.h:523 #3 0xaaabf555db84 in timer_new /qemu/include/qemu/timer.h:544 #4 0xaaabf555db84 in timer_new_ns /qemu/include/qemu/timer.h:562 #5 0xaaabf555db84 in ptimer_init /qemu/hw/core/ptimer.c:433 #6 0xaaabf56a36cc in exynos4210_pwm_init /qemu/hw/timer/exynos4210_pwm.c:401 #7 0xaaabf6339f6c in object_initialize_with_type /qemu/qom/object.c:515 #8 0xaaabf633a1e0 in object_new_with_type /qemu/qom/object.c:729 #9 0xaaabf6375e40 in qmp_device_list_properties /qemu/qom/qom-qmp-cmds.c:153 #10 0xaaabf653d8ec in qmp_marshal_device_list_properties /qemu/qapi/qapi-commands-qdev.c:59 #11 0xaaabf6587d08 in do_qmp_dispatch_bh /qemu/qapi/qmp-dispatch.c:110 #12 0xaaabf6552708 in aio_bh_call /qemu/util/async.c:136 Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Gan Qixin <ganqixin@huawei.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/timer/exynos4210_pwm.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/hw/timer/exynos4210_pwm.c b/hw/timer/exynos4210_pwm.c index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/exynos4210_pwm.c +++ b/hw/timer/exynos4210_pwm.c @@ -XXX,XX +XXX,XX @@ static void exynos4210_pwm_init(Object *obj) sysbus_init_mmio(dev, &s->iomem); } +static void exynos4210_pwm_finalize(Object *obj) +{ + Exynos4210PWMState *s = EXYNOS4210_PWM(obj); + int i; + + for (i = 0; i < EXYNOS4210_PWM_TIMERS_NUM; i++) { + ptimer_free(s->timer[i].ptimer); + } +} + static void exynos4210_pwm_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static const TypeInfo exynos4210_pwm_info = { .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(Exynos4210PWMState), .instance_init = exynos4210_pwm_init, + .instance_finalize = exynos4210_pwm_finalize, .class_init = exynos4210_pwm_class_init, }; -- 2.20.1 From: Gan Qixin <ganqixin@huawei.com> When running device-introspect-test, a memory leak occurred in the mss_timer_init function, so use ptimer_free() in the finalize function to avoid it. ASAN shows memory leak stack: Indirect leak of 192 byte(s) in 2 object(s) allocated from: #0 0xffffab97e1f0 in __interceptor_calloc (/lib64/libasan.so.5+0xee1f0) #1 0xffffab256800 in g_malloc0 (/lib64/libglib-2.0.so.0+0x56800) #2 0xaaabf555db78 in ptimer_init /qemu/hw/core/ptimer.c:432 #3 0xaaabf58a0010 in mss_timer_init /qemu/hw/timer/mss-timer.c:235 #4 0xaaabf6339f6c in object_initialize_with_type /qemu/qom/object.c:515 #5 0xaaabf633ca04 in object_initialize_child_with_propsv /qemu/qom/object.c:564 #6 0xaaabf633cc08 in object_initialize_child_with_props /qemu/qom/object.c:547 #7 0xaaabf5b8316c in m2sxxx_soc_initfn /qemu/hw/arm/msf2-soc.c:70 #8 0xaaabf6339f6c in object_initialize_with_type /qemu/qom/object.c:515 #9 0xaaabf633a1e0 in object_new_with_type /qemu/qom/object.c:729 #10 0xaaabf6375e40 in qmp_device_list_properties /qemu/qom/qom-qmp-cmds.c:153 #11 0xaaabf653d8ec in qmp_marshal_device_list_properties /qemu/qapi/qapi-commands-qdev.c:59 #12 0xaaabf6587d08 in do_qmp_dispatch_bh /qemu/qapi/qmp-dispatch.c:110 Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Gan Qixin <ganqixin@huawei.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/timer/mss-timer.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/hw/timer/mss-timer.c b/hw/timer/mss-timer.c index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/mss-timer.c +++ b/hw/timer/mss-timer.c @@ -XXX,XX +XXX,XX @@ static void mss_timer_init(Object *obj) sysbus_init_mmio(SYS_BUS_DEVICE(obj), &t->mmio); } +static void mss_timer_finalize(Object *obj) +{ + MSSTimerState *t = MSS_TIMER(obj); + int i; + + for (i = 0; i < NUM_TIMERS; i++) { + struct Msf2Timer *st = &t->timers[i]; + + ptimer_free(st->ptimer); + } +} + static const VMStateDescription vmstate_timers = { .name = "mss-timer-block", .version_id = 1, @@ -XXX,XX +XXX,XX @@ static const TypeInfo mss_timer_info = { .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(MSSTimerState), .instance_init = mss_timer_init, + .instance_finalize = mss_timer_finalize, .class_init = mss_timer_class_init, }; -- 2.20.1 From: Gan Qixin <ganqixin@huawei.com> When running device-introspect-test, a memory leak occurred in the mv88w8618_pit_init function, so use ptimer_free() in the finalize function to avoid it. ASAN shows memory leak stack: Indirect leak of 192 byte(s) in 4 object(s) allocated from: #0 0xffffab97e1f0 in __interceptor_calloc (/lib64/libasan.so.5+0xee1f0) #1 0xffffab256800 in g_malloc0 (/lib64/libglib-2.0.so.0+0x56800) #2 0xaaabf555db84 in timer_new_full /qemu/include/qemu/timer.h:523 #3 0xaaabf555db84 in timer_new /qemu/include/qemu/timer.h:544 #4 0xaaabf555db84 in timer_new_ns /qemu/include/qemu/timer.h:562 #5 0xaaabf555db84 in ptimer_init /qemu/hw/core/ptimer.c:433 #6 0xaaabf5bb2290 in mv88w8618_timer_init /qemu/hw/arm/musicpal.c:862 #7 0xaaabf5bb2290 in mv88w8618_pit_init /qemu/hw/arm/musicpal.c:954 #8 0xaaabf6339f6c in object_initialize_with_type /qemu/qom/object.c:515 #9 0xaaabf633a1e0 in object_new_with_type /qemu/qom/object.c:729 #10 0xaaabf6375e40 in qmp_device_list_properties /qemu/qom/qom-qmp-cmds.c:153 #11 0xaaabf5a95540 in qdev_device_help /qemu/softmmu/qdev-monitor.c:283 #12 0xaaabf5a96940 in qmp_device_add /qemu/softmmu/qdev-monitor.c:801 Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Gan Qixin <ganqixin@huawei.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/musicpal.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/musicpal.c +++ b/hw/arm/musicpal.c @@ -XXX,XX +XXX,XX @@ static void mv88w8618_pit_init(Object *obj) sysbus_init_mmio(dev, &s->iomem); } +static void mv88w8618_pit_finalize(Object *obj) +{ + SysBusDevice *dev = SYS_BUS_DEVICE(obj); + mv88w8618_pit_state *s = MV88W8618_PIT(dev); + int i; + + for (i = 0; i < 4; i++) { + ptimer_free(s->timer[i].ptimer); + } +} + static const VMStateDescription mv88w8618_timer_vmsd = { .name = "timer", .version_id = 1, @@ -XXX,XX +XXX,XX @@ static const TypeInfo mv88w8618_pit_info = { .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(mv88w8618_pit_state), .instance_init = mv88w8618_pit_init, + .instance_finalize = mv88w8618_pit_finalize, .class_init = mv88w8618_pit_class_init, }; -- 2.20.1 From: Gan Qixin <ganqixin@huawei.com> When running device-introspect-test, a memory leak occurred in the exynos4210_mct_init function, so use ptimer_free() in the finalize function to avoid it. ASAN shows memory leak stack: Indirect leak of 96 byte(s) in 1 object(s) allocated from: #0 0xffffab97e1f0 in __interceptor_calloc (/lib64/libasan.so.5+0xee1f0) #1 0xffffab256800 in g_malloc0 (/lib64/libglib-2.0.so.0+0x56800) #2 0xaaabf555db78 in ptimer_init /qemu/hw/core/ptimer.c:432 #3 0xaaabf56b01a0 in exynos4210_mct_init /qemu/hw/timer/exynos4210_mct.c:1505 #4 0xaaabf6339f6c in object_initialize_with_type /qemu/qom/object.c:515 #5 0xaaabf633a1e0 in object_new_with_type /qemu/qom/object.c:729 #6 0xaaabf6375e40 in qmp_device_list_properties /qemu/qom/qom-qmp-cmds.c:153 #7 0xaaabf653d8ec in qmp_marshal_device_list_properties /qemu/qapi/qapi-commands-qdev.c:59 #8 0xaaabf6587d08 in do_qmp_dispatch_bh /qemu/qapi/qmp-dispatch.c:110 #9 0xaaabf6552708 in aio_bh_call /qemu/util/async.c:136 #10 0xaaabf6552708 in aio_bh_poll /qemu/util/async.c:164 #11 0xaaabf655f19c in aio_dispatch /qemu/util/aio-posix.c:381 #12 0xaaabf65523f4 in aio_ctx_dispatch /qemu/util/async.c:306 Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Gan Qixin <ganqixin@huawei.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/timer/exynos4210_mct.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/hw/timer/exynos4210_mct.c b/hw/timer/exynos4210_mct.c index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/exynos4210_mct.c +++ b/hw/timer/exynos4210_mct.c @@ -XXX,XX +XXX,XX @@ static void exynos4210_mct_init(Object *obj) sysbus_init_mmio(dev, &s->iomem); } +static void exynos4210_mct_finalize(Object *obj) +{ + int i; + Exynos4210MCTState *s = EXYNOS4210_MCT(obj); + + ptimer_free(s->g_timer.ptimer_frc); + + for (i = 0; i < 2; i++) { + ptimer_free(s->l_timer[i].tick_timer.ptimer_tick); + ptimer_free(s->l_timer[i].ptimer_frc); + } +} + static void exynos4210_mct_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static const TypeInfo exynos4210_mct_info = { .parent = TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(Exynos4210MCTState), .instance_init = exynos4210_mct_init, + .instance_finalize = exynos4210_mct_finalize, .class_init = exynos4210_mct_class_init, }; -- 2.20.1 From: Bin Meng <bin.meng@windriver.com> U-Boot expects PMU_MISC0 register bit 7 is set (see init_bandgap() in arch/arm/mach-imx/mx6/soc.c) during boot. This bit indicates the bandgap has stabilized. With this change, the latest upstream U-Boot (v2021.01-rc3) for imx6 sabrelite board (mx6qsabrelite_defconfig), with a slight change made by switching CONFIG_OF_SEPARATE to CONFIG_OF_EMBED, boots to U-Boot shell on QEMU with the following command: $ qemu-system-arm -M sabrelite -smp 4 -m 1G -kernel u-boot \ -display none -serial null -serial stdio Boot log below: U-Boot 2021.01-rc3 (Dec 12 2020 - 17:40:02 +0800) CPU: Freescale i.MX?? rev1.0 at 792 MHz Reset cause: POR Model: Freescale i.MX6 Quad SABRE Lite Board Board: SABRE Lite I2C: ready DRAM: 1 GiB force_idle_bus: sda=0 scl=0 sda.gp=0x5c scl.gp=0x55 force_idle_bus: failed to clear bus, sda=0 scl=0 force_idle_bus: sda=0 scl=0 sda.gp=0x6d scl.gp=0x6c force_idle_bus: failed to clear bus, sda=0 scl=0 force_idle_bus: sda=0 scl=0 sda.gp=0xcb scl.gp=0x5 force_idle_bus: failed to clear bus, sda=0 scl=0 MMC: FSL_SDHC: 0, FSL_SDHC: 1 Loading Environment from MMC... *** Warning - No block device, using default environment In: serial Out: serial Err: serial Net: Board Net Initialization Failed No ethernet found. starting USB... Bus usb@2184000: usb dr_mode not found USB EHCI 1.00 Bus usb@2184200: USB EHCI 1.00 scanning bus usb@2184000 for devices... 1 USB Device(s) found scanning bus usb@2184200 for devices... 1 USB Device(s) found scanning usb for storage devices... 0 Storage Device(s) found scanning usb for ethernet devices... 0 Ethernet Device(s) found Hit any key to stop autoboot: 0 => Signed-off-by: Bin Meng <bin.meng@windriver.com> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20210106063504.10841-2-bmeng.cn@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/misc/imx6_ccm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/misc/imx6_ccm.c b/hw/misc/imx6_ccm.c index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/imx6_ccm.c +++ b/hw/misc/imx6_ccm.c @@ -XXX,XX +XXX,XX @@ static void imx6_ccm_reset(DeviceState *dev) s->analog[PMU_REG_3P0] = 0x00000F74; s->analog[PMU_REG_2P5] = 0x00005071; s->analog[PMU_REG_CORE] = 0x00402010; - s->analog[PMU_MISC0] = 0x04000000; + s->analog[PMU_MISC0] = 0x04000080; s->analog[PMU_MISC1] = 0x00000000; s->analog[PMU_MISC2] = 0x00272727; -- 2.20.1 From: Bin Meng <bin.meng@windriver.com> Currently when U-Boot boots, it prints "??" for i.MX processor: CPU: Freescale i.MX?? rev1.0 at 792 MHz The register that was used to determine the silicon type is undocumented in the latest IMX6DQRM (Rev. 6, 05/2020), but we can refer to get_cpu_rev() in arch/arm/mach-imx/mx6/soc.c in the U-Boot source codes that USB_ANALOG_DIGPROG is used. Update its reset value to indicate i.MX6Q. Signed-off-by: Bin Meng <bin.meng@windriver.com> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20210106063504.10841-3-bmeng.cn@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/misc/imx6_ccm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/misc/imx6_ccm.c b/hw/misc/imx6_ccm.c index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/imx6_ccm.c +++ b/hw/misc/imx6_ccm.c @@ -XXX,XX +XXX,XX @@ static void imx6_ccm_reset(DeviceState *dev) s->analog[USB_ANALOG_USB2_VBUS_DETECT] = 0x00000004; s->analog[USB_ANALOG_USB2_CHRG_DETECT] = 0x00000000; s->analog[USB_ANALOG_USB2_MISC] = 0x00000002; - s->analog[USB_ANALOG_DIGPROG] = 0x00000000; + s->analog[USB_ANALOG_DIGPROG] = 0x00630000; /* all PLLs need to be locked */ s->analog[CCM_ANALOG_PLL_ARM] |= CCM_ANALOG_PLL_LOCK; -- 2.20.1 From: Bin Meng <bin.meng@windriver.com> At present, when booting U-Boot on QEMU sabrelite, we see: Net: Board Net Initialization Failed No ethernet found. U-Boot scans PHY at address 4/5/6/7 (see board_eth_init() in the U-Boot source: board/boundary/nitrogen6x/nitrogen6x.c). On the real board, the Ethernet PHY is at address 6. Adjust this by updating the "fec-phy-num" property of the fsl_imx6 SoC object. With this change, U-Boot sees the PHY but complains MAC address: Net: using phy at 6 FEC [PRIME] Error: FEC address not set. This is due to U-Boot tries to read the MAC address from the fuse, which QEMU does not have any valid content filled in. However this does not prevent the Ethernet from working in QEMU. We just need to set up the MAC address later in the U-Boot command shell, by: => setenv ethaddr 00:11:22:33:44:55 Signed-off-by: Bin Meng <bin.meng@windriver.com> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20210106063504.10841-4-bmeng.cn@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/sabrelite.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/arm/sabrelite.c b/hw/arm/sabrelite.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/sabrelite.c +++ b/hw/arm/sabrelite.c @@ -XXX,XX +XXX,XX @@ static void sabrelite_init(MachineState *machine) s = FSL_IMX6(object_new(TYPE_FSL_IMX6)); object_property_add_child(OBJECT(machine), "soc", OBJECT(s)); + + /* Ethernet PHY address is 6 */ + object_property_set_int(OBJECT(s), "fec-phy-num", 6, &error_fatal); + qdev_realize(DEVICE(s), NULL, &error_fatal); memory_region_add_subregion(get_system_memory(), FSL_IMX6_MMDC_ADDR, -- 2.20.1 From: Bin Meng <bin.meng@windriver.com> This adds the target guide for SABRE Lite board, and documents how to boot a Linux kernel and U-Boot bootloader. Signed-off-by: Bin Meng <bin.meng@windriver.com> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20210106063504.10841-5-bmeng.cn@gmail.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- docs/system/arm/sabrelite.rst | 119 ++++++++++++++++++++++++++++++++++ docs/system/target-arm.rst | 1 + 2 files changed, 120 insertions(+) create mode 100644 docs/system/arm/sabrelite.rst diff --git a/docs/system/arm/sabrelite.rst b/docs/system/arm/sabrelite.rst new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/docs/system/arm/sabrelite.rst @@ -XXX,XX +XXX,XX @@ +Boundary Devices SABRE Lite (``sabrelite``) +=========================================== + +Boundary Devices SABRE Lite i.MX6 Development Board is a low-cost development +platform featuring the powerful Freescale / NXP Semiconductor's i.MX 6 Quad +Applications Processor. + +Supported devices +----------------- + +The SABRE Lite machine supports the following devices: + + * Up to 4 Cortex A9 cores + * Generic Interrupt Controller + * 1 Clock Controller Module + * 1 System Reset Controller + * 5 UARTs + * 2 EPIC timers + * 1 GPT timer + * 2 Watchdog timers + * 1 FEC Ethernet controller + * 3 I2C controllers + * 7 GPIO controllers + * 4 SDHC storage controllers + * 4 USB 2.0 host controllers + * 5 ECSPI controllers + * 1 SST 25VF016B flash + +Please note above list is a complete superset the QEMU SABRE Lite machine can +support. For a normal use case, a device tree blob that represents a real world +SABRE Lite board, only exposes a subset of devices to the guest software. + +Boot options +------------ + +The SABRE Lite machine can start using the standard -kernel functionality +for loading a Linux kernel, U-Boot bootloader or ELF executable. + +Running Linux kernel +-------------------- + +Linux mainline v5.10 release is tested at the time of writing. To build a Linux +mainline kernel that can be booted by the SABRE Lite machine, simply configure +the kernel using the imx_v6_v7_defconfig configuration: + +.. code-block:: bash + + $ export ARCH=arm + $ export CROSS_COMPILE=arm-linux-gnueabihf- + $ make imx_v6_v7_defconfig + $ make + +To boot the newly built Linux kernel in QEMU with the SABRE Lite machine, use: + +.. code-block:: bash + + $ qemu-system-arm -M sabrelite -smp 4 -m 1G \ + -display none -serial null -serial stdio \ + -kernel arch/arm/boot/zImage \ + -dtb arch/arm/boot/dts/imx6q-sabrelite.dtb \ + -initrd /path/to/rootfs.ext4 \ + -append "root=/dev/ram" + +Running U-Boot +-------------- + +U-Boot mainline v2020.10 release is tested at the time of writing. To build a +U-Boot mainline bootloader that can be booted by the SABRE Lite machine, use +the mx6qsabrelite_defconfig with similar commands as described above for Linux: + +.. code-block:: bash + + $ export CROSS_COMPILE=arm-linux-gnueabihf- + $ make mx6qsabrelite_defconfig + +Note we need to adjust settings by: + +.. code-block:: bash + + $ make menuconfig + +then manually select the following configuration in U-Boot: + + Device Tree Control > Provider of DTB for DT Control > Embedded DTB + +To start U-Boot using the SABRE Lite machine, provide the u-boot binary to +the -kernel argument, along with an SD card image with rootfs: + +.. code-block:: bash + + $ qemu-system-arm -M sabrelite -smp 4 -m 1G \ + -display none -serial null -serial stdio \ + -kernel u-boot + +The following example shows booting Linux kernel from dhcp, and uses the +rootfs on an SD card. This requires some additional command line parameters +for QEMU: + +.. code-block:: none + + -nic user,tftp=/path/to/kernel/zImage \ + -drive file=sdcard.img,id=rootfs -device sd-card,drive=rootfs + +The directory for the built-in TFTP server should also contain the device tree +blob of the SABRE Lite board. The sample SD card image was populated with the +root file system with one single partition. You may adjust the kernel "root=" +boot parameter accordingly. + +After U-Boot boots, type the following commands in the U-Boot command shell to +boot the Linux kernel: + +.. code-block:: none + + => setenv ethaddr 00:11:22:33:44:55 + => setenv bootfile zImage + => dhcp + => tftpboot 14000000 imx6q-sabrelite.dtb + => setenv bootargs root=/dev/mmcblk3p1 + => bootz 12000000 - 14000000 diff --git a/docs/system/target-arm.rst b/docs/system/target-arm.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/system/target-arm.rst +++ b/docs/system/target-arm.rst @@ -XXX,XX +XXX,XX @@ undocumented; you can get a complete list by running arm/versatile arm/vexpress arm/aspeed + arm/sabrelite arm/digic arm/musicpal arm/gumstix -- 2.20.1