The following changes since commit ad1b4ec39caa5b3f17cbd8160283a03a3dcfe2ae: Merge remote-tracking branch 'remotes/kraxel/tags/input-20180515-pull-request' into staging (2018-05-15 12:50:06 +0100) are available in the Git repository at: git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20180515 for you to fetch changes up to ae7651804748c6b479d5ae09aeac4edb9c44f76e: tcg: Optionally log FPU state in TCG -d cpu logging (2018-05-15 14:58:44 +0100) ---------------------------------------------------------------- target-arm queue: * Fix coverity nit in int_to_float code * Don't set Invalid for float-to-int(MAXINT) * Fix fp_status_f16 tininess before rounding * Add various missing insns from the v8.2-FP16 extension * Fix sqrt_f16 exception raising * sdcard: Correct CRC16 offset in sd_function_switch() * tcg: Optionally log FPU state in TCG -d cpu logging ---------------------------------------------------------------- Alex Bennée (5): fpu/softfloat: int_to_float ensure r fully initialised target/arm: Implement FCMP for fp16 target/arm: Implement FCSEL for fp16 target/arm: Implement FMOV (immediate) for fp16 target/arm: Fix sqrt_f16 exception raising Peter Maydell (3): fpu/softfloat: Don't set Invalid for float-to-int(MAXINT) target/arm: Fix fp_status_f16 tininess before rounding tcg: Optionally log FPU state in TCG -d cpu logging Philippe Mathieu-Daudé (1): sdcard: Correct CRC16 offset in sd_function_switch() Richard Henderson (7): target/arm: Implement FMOV (general) for fp16 target/arm: Early exit after unallocated_encoding in disas_fp_int_conv target/arm: Implement FCVT (scalar, integer) for fp16 target/arm: Implement FCVT (scalar, fixed-point) for fp16 target/arm: Introduce and use read_fp_hreg target/arm: Implement FP data-processing (2 source) for fp16 target/arm: Implement FP data-processing (3 source) for fp16 include/qemu/log.h | 1 + target/arm/helper-a64.h | 2 + target/arm/helper.h | 6 + accel/tcg/cpu-exec.c | 9 +- fpu/softfloat.c | 6 +- hw/sd/sd.c | 2 +- target/arm/cpu.c | 2 + target/arm/helper-a64.c | 10 ++ target/arm/helper.c | 38 +++- target/arm/translate-a64.c | 421 ++++++++++++++++++++++++++++++++++++++------- util/log.c | 2 + 11 files changed, 428 insertions(+), 71 deletions(-) From: Alex Bennée <alex.bennee@linaro.org> Reported by Coverity (CID1390635). We ensure this for uint_to_float later on so we might as well mirror that. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- fpu/softfloat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fpu/softfloat.c b/fpu/softfloat.c index XXXXXXX..XXXXXXX 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -XXX,XX +XXX,XX @@ FLOAT_TO_UINT(64, 64) static FloatParts int_to_float(int64_t a, float_status *status) { - FloatParts r; + FloatParts r = {}; if (a == 0) { r.cls = float_class_zero; r.sign = false; -- 2.17.0 In float-to-integer conversion, if the floating point input converts exactly to the largest or smallest integer that fits in to the result type, this is not an overflow. In this situation we were producing the correct result value, but were incorrectly setting the Invalid flag. For example for Arm A64, "FCVTAS w0, d0" on an input of 0x41dfffffffc00000 should produce 0x7fffffff and set no flags. Fix the boundary case to take the right half of the if() statements. This fixes a regression from 2.11 introduced by the softfloat refactoring. Cc: qemu-stable@nongnu.org Fixes: ab52f973a50 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180510140141.12120-1-peter.maydell@linaro.org --- fpu/softfloat.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fpu/softfloat.c b/fpu/softfloat.c index XXXXXXX..XXXXXXX 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -XXX,XX +XXX,XX @@ static int64_t round_to_int_and_pack(FloatParts in, int rmode, r = UINT64_MAX; } if (p.sign) { - if (r < -(uint64_t) min) { + if (r <= -(uint64_t) min) { return -r; } else { s->float_exception_flags = orig_flags | float_flag_invalid; return min; } } else { - if (r < max) { + if (r <= max) { return r; } else { s->float_exception_flags = orig_flags | float_flag_invalid; -- 2.17.0 In commit d81ce0ef2c4f105 we added an extra float_status field fp_status_fp16 for Arm, but forgot to initialize it correctly by setting it to float_tininess_before_rounding. This currently will only cause problems for the new V8_FP16 feature, since the float-to-float conversion code doesn't use it yet. The effect would be that we failed to set the Underflow IEEE exception flag in all the cases where we should. Add the missing initialization. Fixes: d81ce0ef2c4f105 Cc: qemu-stable@nongnu.org Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20180512004311.9299-16-richard.henderson@linaro.org --- target/arm/cpu.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target/arm/cpu.c b/target/arm/cpu.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -XXX,XX +XXX,XX @@ static void arm_cpu_reset(CPUState *s) &env->vfp.fp_status); set_float_detect_tininess(float_tininess_before_rounding, &env->vfp.standard_fp_status); + set_float_detect_tininess(float_tininess_before_rounding, + &env->vfp.fp_status_f16); #ifndef CONFIG_USER_ONLY if (kvm_enabled()) { kvm_arm_reset_vcpu(cpu); -- 2.17.0 From: Richard Henderson <richard.henderson@linaro.org> Adding the fp16 moves to/from general registers. Cc: qemu-stable@nongnu.org Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20180512003217.9105-2-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) 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 handle_fmov(DisasContext *s, int rd, int rn, int type, bool itof) tcg_gen_st_i64(tcg_rn, cpu_env, fp_reg_hi_offset(s, rd)); clear_vec_high(s, true, rd); break; + case 3: + /* 16 bit */ + tmp = tcg_temp_new_i64(); + tcg_gen_ext16u_i64(tmp, tcg_rn); + write_fp_dreg(s, rd, tmp); + tcg_temp_free_i64(tmp); + break; + default: + g_assert_not_reached(); } } else { TCGv_i64 tcg_rd = cpu_reg(s, rd); @@ -XXX,XX +XXX,XX @@ static void handle_fmov(DisasContext *s, int rd, int rn, int type, bool itof) /* 64 bits from top half */ tcg_gen_ld_i64(tcg_rd, cpu_env, fp_reg_hi_offset(s, rn)); break; + case 3: + /* 16 bit */ + tcg_gen_ld16u_i64(tcg_rd, cpu_env, fp_reg_offset(s, rn, MO_16)); + break; + default: + g_assert_not_reached(); } } } @@ -XXX,XX +XXX,XX @@ static void disas_fp_int_conv(DisasContext *s, uint32_t insn) case 0xa: /* 64 bit */ case 0xd: /* 64 bit to top half of quad */ break; + case 0x6: /* 16-bit float, 32-bit int */ + case 0xe: /* 16-bit float, 64-bit int */ + if (arm_dc_feature(s, ARM_FEATURE_V8_FP16)) { + break; + } + /* fallthru */ default: /* all other sf/type/rmode combinations are invalid */ unallocated_encoding(s); -- 2.17.0 From: Richard Henderson <richard.henderson@linaro.org> No sense in emitting code after the exception. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20180512003217.9105-3-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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_fp_int_conv(DisasContext *s, uint32_t insn) default: /* all other sf/type/rmode combinations are invalid */ unallocated_encoding(s); - break; + return; } if (!fp_access_check(s)) { -- 2.17.0 From: Richard Henderson <richard.henderson@linaro.org> Cc: qemu-stable@nongnu.org Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20180512003217.9105-4-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.h | 6 +++ target/arm/helper.c | 38 ++++++++++++++- target/arm/translate-a64.c | 96 +++++++++++++++++++++++++++++++------- 3 files changed, 122 insertions(+), 18 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_touhd_round_to_zero, i64, f64, i32, ptr) DEF_HELPER_3(vfp_tould_round_to_zero, i64, f64, i32, ptr) DEF_HELPER_3(vfp_touhh, i32, f16, i32, ptr) DEF_HELPER_3(vfp_toshh, i32, f16, i32, ptr) +DEF_HELPER_3(vfp_toulh, i32, f16, i32, ptr) +DEF_HELPER_3(vfp_toslh, i32, f16, i32, ptr) +DEF_HELPER_3(vfp_touqh, i64, f16, i32, ptr) +DEF_HELPER_3(vfp_tosqh, i64, f16, i32, ptr) DEF_HELPER_3(vfp_toshs, i32, f32, i32, ptr) DEF_HELPER_3(vfp_tosls, i32, f32, i32, ptr) DEF_HELPER_3(vfp_tosqs, i64, f32, i32, ptr) @@ -XXX,XX +XXX,XX @@ DEF_HELPER_3(vfp_ultod, f64, i64, i32, ptr) DEF_HELPER_3(vfp_uqtod, f64, i64, i32, ptr) DEF_HELPER_3(vfp_sltoh, f16, i32, i32, ptr) 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_FLAGS_2(set_rmode, TCG_CALL_NO_RWG, i32, i32, ptr) DEF_HELPER_FLAGS_2(set_neon_rmode, TCG_CALL_NO_RWG, i32, i32, env) 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 @@ VFP_CONV_FIX_A64(uq, s, 32, 64, uint64) #undef VFP_CONV_FIX_A64 /* Conversion to/from f16 can overflow to infinity before/after scaling. - * Therefore we convert to f64 (which does not round), scale, - * and then convert f64 to f16 (which may round). + * Therefore we convert to f64, scale, and then convert f64 to f16; or + * vice versa for conversion to integer. + * + * For 16- and 32-bit integers, the conversion to f64 never rounds. + * For 64-bit integers, any integer that would cause rounding will also + * overflow to f16 infinity, so there is no double rounding problem. */ static float16 do_postscale_fp16(float64 f, int shift, float_status *fpst) @@ -XXX,XX +XXX,XX @@ float16 HELPER(vfp_ultoh)(uint32_t x, uint32_t shift, void *fpst) return do_postscale_fp16(uint32_to_float64(x, fpst), shift, fpst); } +float16 HELPER(vfp_sqtoh)(uint64_t x, uint32_t shift, void *fpst) +{ + return do_postscale_fp16(int64_to_float64(x, fpst), shift, fpst); +} + +float16 HELPER(vfp_uqtoh)(uint64_t x, uint32_t shift, void *fpst) +{ + return do_postscale_fp16(uint64_to_float64(x, fpst), shift, fpst); +} + static float64 do_prescale_fp16(float16 f, int shift, float_status *fpst) { if (unlikely(float16_is_any_nan(f))) { @@ -XXX,XX +XXX,XX @@ uint32_t HELPER(vfp_touhh)(float16 x, uint32_t shift, void *fpst) return float64_to_uint16(do_prescale_fp16(x, shift, fpst), fpst); } +uint32_t HELPER(vfp_toslh)(float16 x, uint32_t shift, void *fpst) +{ + return float64_to_int32(do_prescale_fp16(x, shift, fpst), fpst); +} + +uint32_t HELPER(vfp_toulh)(float16 x, uint32_t shift, void *fpst) +{ + return float64_to_uint32(do_prescale_fp16(x, shift, fpst), fpst); +} + +uint64_t HELPER(vfp_tosqh)(float16 x, uint32_t shift, void *fpst) +{ + return float64_to_int64(do_prescale_fp16(x, shift, fpst), fpst); +} + +uint64_t HELPER(vfp_touqh)(float16 x, uint32_t shift, void *fpst) +{ + return float64_to_uint64(do_prescale_fp16(x, shift, fpst), fpst); +} + /* Set the current fp rounding mode and return the old one. * The argument is a softfloat float_round_ value. */ 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 handle_fpfpcvt(DisasContext *s, int rd, int rn, int opcode, bool itof, int rmode, int scale, int sf, int type) { bool is_signed = !(opcode & 1); - bool is_double = type; TCGv_ptr tcg_fpstatus; - TCGv_i32 tcg_shift; + TCGv_i32 tcg_shift, tcg_single; + TCGv_i64 tcg_double; - tcg_fpstatus = get_fpstatus_ptr(false); + tcg_fpstatus = get_fpstatus_ptr(type == 3); tcg_shift = tcg_const_i32(64 - scale); @@ -XXX,XX +XXX,XX @@ static void handle_fpfpcvt(DisasContext *s, int rd, int rn, int opcode, tcg_int = tcg_extend; } - if (is_double) { - TCGv_i64 tcg_double = tcg_temp_new_i64(); + switch (type) { + case 1: /* float64 */ + tcg_double = tcg_temp_new_i64(); if (is_signed) { gen_helper_vfp_sqtod(tcg_double, tcg_int, tcg_shift, tcg_fpstatus); @@ -XXX,XX +XXX,XX @@ static void handle_fpfpcvt(DisasContext *s, int rd, int rn, int opcode, } write_fp_dreg(s, rd, tcg_double); tcg_temp_free_i64(tcg_double); - } else { - TCGv_i32 tcg_single = tcg_temp_new_i32(); + break; + + case 0: /* float32 */ + tcg_single = tcg_temp_new_i32(); if (is_signed) { gen_helper_vfp_sqtos(tcg_single, tcg_int, tcg_shift, tcg_fpstatus); @@ -XXX,XX +XXX,XX @@ static void handle_fpfpcvt(DisasContext *s, int rd, int rn, int opcode, } write_fp_sreg(s, rd, tcg_single); tcg_temp_free_i32(tcg_single); + break; + + case 3: /* float16 */ + tcg_single = tcg_temp_new_i32(); + if (is_signed) { + gen_helper_vfp_sqtoh(tcg_single, tcg_int, + tcg_shift, tcg_fpstatus); + } else { + gen_helper_vfp_uqtoh(tcg_single, tcg_int, + tcg_shift, tcg_fpstatus); + } + write_fp_sreg(s, rd, tcg_single); + tcg_temp_free_i32(tcg_single); + break; + + default: + g_assert_not_reached(); } } else { TCGv_i64 tcg_int = cpu_reg(s, rd); @@ -XXX,XX +XXX,XX @@ static void handle_fpfpcvt(DisasContext *s, int rd, int rn, int opcode, gen_helper_set_rmode(tcg_rmode, tcg_rmode, tcg_fpstatus); - if (is_double) { - TCGv_i64 tcg_double = read_fp_dreg(s, rn); + switch (type) { + case 1: /* float64 */ + tcg_double = read_fp_dreg(s, rn); if (is_signed) { if (!sf) { gen_helper_vfp_tosld(tcg_int, tcg_double, @@ -XXX,XX +XXX,XX @@ static void handle_fpfpcvt(DisasContext *s, int rd, int rn, int opcode, tcg_shift, tcg_fpstatus); } } + if (!sf) { + tcg_gen_ext32u_i64(tcg_int, tcg_int); + } tcg_temp_free_i64(tcg_double); - } else { - TCGv_i32 tcg_single = read_fp_sreg(s, rn); + break; + + case 0: /* float32 */ + tcg_single = read_fp_sreg(s, rn); if (sf) { if (is_signed) { gen_helper_vfp_tosqs(tcg_int, tcg_single, @@ -XXX,XX +XXX,XX @@ static void handle_fpfpcvt(DisasContext *s, int rd, int rn, int opcode, tcg_temp_free_i32(tcg_dest); } tcg_temp_free_i32(tcg_single); + break; + + case 3: /* float16 */ + tcg_single = read_fp_sreg(s, rn); + if (sf) { + if (is_signed) { + gen_helper_vfp_tosqh(tcg_int, tcg_single, + tcg_shift, tcg_fpstatus); + } else { + gen_helper_vfp_touqh(tcg_int, tcg_single, + tcg_shift, tcg_fpstatus); + } + } else { + TCGv_i32 tcg_dest = tcg_temp_new_i32(); + if (is_signed) { + gen_helper_vfp_toslh(tcg_dest, tcg_single, + tcg_shift, tcg_fpstatus); + } else { + gen_helper_vfp_toulh(tcg_dest, tcg_single, + tcg_shift, tcg_fpstatus); + } + tcg_gen_extu_i32_i64(tcg_int, tcg_dest); + tcg_temp_free_i32(tcg_dest); + } + tcg_temp_free_i32(tcg_single); + break; + + default: + g_assert_not_reached(); } gen_helper_set_rmode(tcg_rmode, tcg_rmode, tcg_fpstatus); tcg_temp_free_i32(tcg_rmode); - - if (!sf) { - tcg_gen_ext32u_i64(tcg_int, tcg_int); - } } tcg_temp_free_ptr(tcg_fpstatus); @@ -XXX,XX +XXX,XX @@ static void disas_fp_int_conv(DisasContext *s, uint32_t insn) /* actual FP conversions */ bool itof = extract32(opcode, 1, 1); - if (type > 1 || (rmode != 0 && opcode > 1)) { + if (rmode != 0 && opcode > 1) { + unallocated_encoding(s); + return; + } + switch (type) { + case 0: /* float32 */ + case 1: /* float64 */ + break; + case 3: /* float16 */ + if (arm_dc_feature(s, ARM_FEATURE_V8_FP16)) { + break; + } + /* fallthru */ + default: unallocated_encoding(s); return; } -- 2.17.0 From: Richard Henderson <richard.henderson@linaro.org> Cc: qemu-stable@nongnu.org Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20180512003217.9105-5-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static void disas_fp_fixed_conv(DisasContext *s, uint32_t insn) bool sf = extract32(insn, 31, 1); bool itof; - if (sbit || (type > 1) - || (!sf && scale < 32)) { + if (sbit || (!sf && scale < 32)) { + unallocated_encoding(s); + return; + } + + switch (type) { + case 0: /* float32 */ + case 1: /* float64 */ + break; + case 3: /* float16 */ + if (arm_dc_feature(s, ARM_FEATURE_V8_FP16)) { + break; + } + /* fallthru */ + default: unallocated_encoding(s); return; } -- 2.17.0 From: Richard Henderson <richard.henderson@linaro.org> Cc: qemu-stable@nongnu.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20180512003217.9105-6-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static TCGv_i32 read_fp_sreg(DisasContext *s, int reg) return v; } +static TCGv_i32 read_fp_hreg(DisasContext *s, int reg) +{ + TCGv_i32 v = tcg_temp_new_i32(); + + tcg_gen_ld16u_i32(v, cpu_env, fp_reg_offset(s, reg, MO_16)); + return v; +} + /* Clear the bits above an N-bit vector, for N = (is_q ? 128 : 64). * If SVE is not enabled, then there are only 128 bits in the vector. */ @@ -XXX,XX +XXX,XX @@ static void disas_fp_csel(DisasContext *s, uint32_t insn) static void handle_fp_1src_half(DisasContext *s, int opcode, int rd, int rn) { TCGv_ptr fpst = NULL; - TCGv_i32 tcg_op = tcg_temp_new_i32(); + TCGv_i32 tcg_op = read_fp_hreg(s, rn); TCGv_i32 tcg_res = tcg_temp_new_i32(); - read_vec_element_i32(s, tcg_op, rn, 0, MO_16); - switch (opcode) { case 0x0: /* FMOV */ tcg_gen_mov_i32(tcg_res, tcg_op); @@ -XXX,XX +XXX,XX @@ static void disas_simd_scalar_three_reg_diff(DisasContext *s, uint32_t insn) tcg_temp_free_i64(tcg_op2); tcg_temp_free_i64(tcg_res); } else { - TCGv_i32 tcg_op1 = tcg_temp_new_i32(); - TCGv_i32 tcg_op2 = tcg_temp_new_i32(); + TCGv_i32 tcg_op1 = read_fp_hreg(s, rn); + TCGv_i32 tcg_op2 = read_fp_hreg(s, rm); TCGv_i64 tcg_res = tcg_temp_new_i64(); - read_vec_element_i32(s, tcg_op1, rn, 0, MO_16); - read_vec_element_i32(s, tcg_op2, rm, 0, MO_16); - gen_helper_neon_mull_s16(tcg_res, tcg_op1, tcg_op2); gen_helper_neon_addl_saturate_s32(tcg_res, cpu_env, tcg_res, tcg_res); @@ -XXX,XX +XXX,XX @@ static void disas_simd_scalar_three_reg_same_fp16(DisasContext *s, fpst = get_fpstatus_ptr(true); - tcg_op1 = tcg_temp_new_i32(); - tcg_op2 = tcg_temp_new_i32(); + tcg_op1 = read_fp_hreg(s, rn); + tcg_op2 = read_fp_hreg(s, rm); tcg_res = tcg_temp_new_i32(); - read_vec_element_i32(s, tcg_op1, rn, 0, MO_16); - read_vec_element_i32(s, tcg_op2, rm, 0, MO_16); - switch (fpopcode) { case 0x03: /* FMULX */ gen_helper_advsimd_mulxh(tcg_res, tcg_op1, tcg_op2, fpst); @@ -XXX,XX +XXX,XX @@ static void disas_simd_two_reg_misc_fp16(DisasContext *s, uint32_t insn) } if (is_scalar) { - TCGv_i32 tcg_op = tcg_temp_new_i32(); + TCGv_i32 tcg_op = read_fp_hreg(s, rn); TCGv_i32 tcg_res = tcg_temp_new_i32(); - read_vec_element_i32(s, tcg_op, rn, 0, MO_16); - switch (fpop) { case 0x1a: /* FCVTNS */ case 0x1b: /* FCVTMS */ -- 2.17.0 From: Richard Henderson <richard.henderson@linaro.org> We missed all of the scalar fp16 binary operations. Cc: qemu-stable@nongnu.org Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20180512003217.9105-7-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 65 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) 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 handle_fp_2src_double(DisasContext *s, int opcode, tcg_temp_free_i64(tcg_res); } +/* Floating-point data-processing (2 source) - half precision */ +static void handle_fp_2src_half(DisasContext *s, int opcode, + int rd, int rn, int rm) +{ + TCGv_i32 tcg_op1; + TCGv_i32 tcg_op2; + TCGv_i32 tcg_res; + TCGv_ptr fpst; + + tcg_res = tcg_temp_new_i32(); + fpst = get_fpstatus_ptr(true); + tcg_op1 = read_fp_hreg(s, rn); + tcg_op2 = read_fp_hreg(s, rm); + + switch (opcode) { + case 0x0: /* FMUL */ + gen_helper_advsimd_mulh(tcg_res, tcg_op1, tcg_op2, fpst); + break; + case 0x1: /* FDIV */ + gen_helper_advsimd_divh(tcg_res, tcg_op1, tcg_op2, fpst); + break; + case 0x2: /* FADD */ + gen_helper_advsimd_addh(tcg_res, tcg_op1, tcg_op2, fpst); + break; + case 0x3: /* FSUB */ + gen_helper_advsimd_subh(tcg_res, tcg_op1, tcg_op2, fpst); + break; + case 0x4: /* FMAX */ + gen_helper_advsimd_maxh(tcg_res, tcg_op1, tcg_op2, fpst); + break; + case 0x5: /* FMIN */ + gen_helper_advsimd_minh(tcg_res, tcg_op1, tcg_op2, fpst); + break; + case 0x6: /* FMAXNM */ + gen_helper_advsimd_maxnumh(tcg_res, tcg_op1, tcg_op2, fpst); + break; + case 0x7: /* FMINNM */ + gen_helper_advsimd_minnumh(tcg_res, tcg_op1, tcg_op2, fpst); + break; + case 0x8: /* FNMUL */ + gen_helper_advsimd_mulh(tcg_res, tcg_op1, tcg_op2, fpst); + tcg_gen_xori_i32(tcg_res, tcg_res, 0x8000); + break; + default: + g_assert_not_reached(); + } + + write_fp_sreg(s, rd, tcg_res); + + tcg_temp_free_ptr(fpst); + tcg_temp_free_i32(tcg_op1); + tcg_temp_free_i32(tcg_op2); + tcg_temp_free_i32(tcg_res); +} + /* Floating point data-processing (2 source) * 31 30 29 28 24 23 22 21 20 16 15 12 11 10 9 5 4 0 * +---+---+---+-----------+------+---+------+--------+-----+------+------+ @@ -XXX,XX +XXX,XX @@ static void disas_fp_2src(DisasContext *s, uint32_t insn) } handle_fp_2src_double(s, opcode, rd, rn, rm); break; + case 3: + if (!arm_dc_feature(s, ARM_FEATURE_V8_FP16)) { + unallocated_encoding(s); + return; + } + if (!fp_access_check(s)) { + return; + } + handle_fp_2src_half(s, opcode, rd, rn, rm); + break; default: unallocated_encoding(s); } -- 2.17.0 From: Richard Henderson <richard.henderson@linaro.org> We missed all of the scalar fp16 fma operations. Cc: qemu-stable@nongnu.org Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20180512003217.9105-8-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 48 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) 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 handle_fp_3src_double(DisasContext *s, bool o0, bool o1, tcg_temp_free_i64(tcg_res); } +/* Floating-point data-processing (3 source) - half precision */ +static void handle_fp_3src_half(DisasContext *s, bool o0, bool o1, + int rd, int rn, int rm, int ra) +{ + TCGv_i32 tcg_op1, tcg_op2, tcg_op3; + TCGv_i32 tcg_res = tcg_temp_new_i32(); + TCGv_ptr fpst = get_fpstatus_ptr(true); + + tcg_op1 = read_fp_hreg(s, rn); + tcg_op2 = read_fp_hreg(s, rm); + tcg_op3 = read_fp_hreg(s, ra); + + /* These are fused multiply-add, and must be done as one + * floating point operation with no rounding between the + * multiplication and addition steps. + * NB that doing the negations here as separate steps is + * correct : an input NaN should come out with its sign bit + * flipped if it is a negated-input. + */ + if (o1 == true) { + tcg_gen_xori_i32(tcg_op3, tcg_op3, 0x8000); + } + + if (o0 != o1) { + tcg_gen_xori_i32(tcg_op1, tcg_op1, 0x8000); + } + + gen_helper_advsimd_muladdh(tcg_res, tcg_op1, tcg_op2, tcg_op3, fpst); + + write_fp_sreg(s, rd, tcg_res); + + tcg_temp_free_ptr(fpst); + tcg_temp_free_i32(tcg_op1); + tcg_temp_free_i32(tcg_op2); + tcg_temp_free_i32(tcg_op3); + tcg_temp_free_i32(tcg_res); +} + /* Floating point data-processing (3 source) * 31 30 29 28 24 23 22 21 20 16 15 14 10 9 5 4 0 * +---+---+---+-----------+------+----+------+----+------+------+------+ @@ -XXX,XX +XXX,XX @@ static void disas_fp_3src(DisasContext *s, uint32_t insn) } handle_fp_3src_double(s, o0, o1, rd, rn, rm, ra); break; + case 3: + if (!arm_dc_feature(s, ARM_FEATURE_V8_FP16)) { + unallocated_encoding(s); + return; + } + if (!fp_access_check(s)) { + return; + } + handle_fp_3src_half(s, o0, o1, rd, rn, rm, ra); + break; default: unallocated_encoding(s); } -- 2.17.0 From: Alex Bennée <alex.bennee@linaro.org> These where missed out from the rest of the half-precision work. Cc: qemu-stable@nongnu.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180512003217.9105-9-richard.henderson@linaro.org [rth: Diagnose lack of FP16 before fp_access_check] Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper-a64.h | 2 + target/arm/helper-a64.c | 10 +++++ target/arm/translate-a64.c | 88 ++++++++++++++++++++++++++++++-------- 3 files changed, 83 insertions(+), 17 deletions(-) diff --git a/target/arm/helper-a64.h b/target/arm/helper-a64.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper-a64.h +++ b/target/arm/helper-a64.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_FLAGS_2(udiv64, TCG_CALL_NO_RWG_SE, i64, i64, i64) DEF_HELPER_FLAGS_2(sdiv64, TCG_CALL_NO_RWG_SE, s64, s64, s64) DEF_HELPER_FLAGS_1(rbit64, TCG_CALL_NO_RWG_SE, i64, i64) +DEF_HELPER_3(vfp_cmph_a64, i64, f16, f16, ptr) +DEF_HELPER_3(vfp_cmpeh_a64, i64, f16, f16, ptr) DEF_HELPER_3(vfp_cmps_a64, i64, f32, f32, ptr) DEF_HELPER_3(vfp_cmpes_a64, i64, f32, f32, ptr) DEF_HELPER_3(vfp_cmpd_a64, i64, f64, f64, ptr) diff --git a/target/arm/helper-a64.c b/target/arm/helper-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper-a64.c +++ b/target/arm/helper-a64.c @@ -XXX,XX +XXX,XX @@ static inline uint32_t float_rel_to_flags(int res) return flags; } +uint64_t HELPER(vfp_cmph_a64)(float16 x, float16 y, void *fp_status) +{ + return float_rel_to_flags(float16_compare_quiet(x, y, fp_status)); +} + +uint64_t HELPER(vfp_cmpeh_a64)(float16 x, float16 y, void *fp_status) +{ + return float_rel_to_flags(float16_compare(x, y, fp_status)); +} + uint64_t HELPER(vfp_cmps_a64)(float32 x, float32 y, void *fp_status) { return float_rel_to_flags(float32_compare_quiet(x, y, fp_status)); 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_reg(DisasContext *s, uint32_t insn) } } -static void handle_fp_compare(DisasContext *s, bool is_double, +static void handle_fp_compare(DisasContext *s, int size, unsigned int rn, unsigned int rm, bool cmp_with_zero, bool signal_all_nans) { TCGv_i64 tcg_flags = tcg_temp_new_i64(); - TCGv_ptr fpst = get_fpstatus_ptr(false); + TCGv_ptr fpst = get_fpstatus_ptr(size == MO_16); - if (is_double) { + if (size == MO_64) { TCGv_i64 tcg_vn, tcg_vm; tcg_vn = read_fp_dreg(s, rn); @@ -XXX,XX +XXX,XX @@ static void handle_fp_compare(DisasContext *s, bool is_double, tcg_temp_free_i64(tcg_vn); tcg_temp_free_i64(tcg_vm); } else { - TCGv_i32 tcg_vn, tcg_vm; + TCGv_i32 tcg_vn = tcg_temp_new_i32(); + TCGv_i32 tcg_vm = tcg_temp_new_i32(); - tcg_vn = read_fp_sreg(s, rn); + read_vec_element_i32(s, tcg_vn, rn, 0, size); if (cmp_with_zero) { - tcg_vm = tcg_const_i32(0); + tcg_gen_movi_i32(tcg_vm, 0); } else { - tcg_vm = read_fp_sreg(s, rm); + read_vec_element_i32(s, tcg_vm, rm, 0, size); } - if (signal_all_nans) { - gen_helper_vfp_cmpes_a64(tcg_flags, tcg_vn, tcg_vm, fpst); - } else { - gen_helper_vfp_cmps_a64(tcg_flags, tcg_vn, tcg_vm, fpst); + + switch (size) { + case MO_32: + if (signal_all_nans) { + gen_helper_vfp_cmpes_a64(tcg_flags, tcg_vn, tcg_vm, fpst); + } else { + gen_helper_vfp_cmps_a64(tcg_flags, tcg_vn, tcg_vm, fpst); + } + break; + case MO_16: + if (signal_all_nans) { + gen_helper_vfp_cmpeh_a64(tcg_flags, tcg_vn, tcg_vm, fpst); + } else { + gen_helper_vfp_cmph_a64(tcg_flags, tcg_vn, tcg_vm, fpst); + } + break; + default: + g_assert_not_reached(); } + tcg_temp_free_i32(tcg_vn); tcg_temp_free_i32(tcg_vm); } @@ -XXX,XX +XXX,XX @@ static void handle_fp_compare(DisasContext *s, bool is_double, static void disas_fp_compare(DisasContext *s, uint32_t insn) { unsigned int mos, type, rm, op, rn, opc, op2r; + int size; mos = extract32(insn, 29, 3); - type = extract32(insn, 22, 2); /* 0 = single, 1 = double */ + type = extract32(insn, 22, 2); rm = extract32(insn, 16, 5); op = extract32(insn, 14, 2); rn = extract32(insn, 5, 5); opc = extract32(insn, 3, 2); op2r = extract32(insn, 0, 3); - if (mos || op || op2r || type > 1) { + if (mos || op || op2r) { + unallocated_encoding(s); + return; + } + + switch (type) { + case 0: + size = MO_32; + break; + case 1: + size = MO_64; + break; + case 3: + size = MO_16; + if (arm_dc_feature(s, ARM_FEATURE_V8_FP16)) { + break; + } + /* fallthru */ + default: unallocated_encoding(s); return; } @@ -XXX,XX +XXX,XX @@ static void disas_fp_compare(DisasContext *s, uint32_t insn) return; } - handle_fp_compare(s, type, rn, rm, opc & 1, opc & 2); + handle_fp_compare(s, size, rn, rm, opc & 1, opc & 2); } /* Floating point conditional compare @@ -XXX,XX +XXX,XX @@ static void disas_fp_ccomp(DisasContext *s, uint32_t insn) unsigned int mos, type, rm, cond, rn, op, nzcv; TCGv_i64 tcg_flags; TCGLabel *label_continue = NULL; + int size; mos = extract32(insn, 29, 3); - type = extract32(insn, 22, 2); /* 0 = single, 1 = double */ + type = extract32(insn, 22, 2); rm = extract32(insn, 16, 5); cond = extract32(insn, 12, 4); rn = extract32(insn, 5, 5); op = extract32(insn, 4, 1); nzcv = extract32(insn, 0, 4); - if (mos || type > 1) { + if (mos) { + unallocated_encoding(s); + return; + } + + switch (type) { + case 0: + size = MO_32; + break; + case 1: + size = MO_64; + break; + case 3: + size = MO_16; + if (arm_dc_feature(s, ARM_FEATURE_V8_FP16)) { + break; + } + /* fallthru */ + default: unallocated_encoding(s); return; } @@ -XXX,XX +XXX,XX @@ static void disas_fp_ccomp(DisasContext *s, uint32_t insn) gen_set_label(label_match); } - handle_fp_compare(s, type, rn, rm, false, op); + handle_fp_compare(s, size, rn, rm, false, op); if (cond < 0x0e) { gen_set_label(label_continue); -- 2.17.0 From: Alex Bennée <alex.bennee@linaro.org> These were missed out from the rest of the half-precision work. Cc: qemu-stable@nongnu.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180512003217.9105-10-richard.henderson@linaro.org [rth: Fix erroneous check vs type] Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static void disas_fp_csel(DisasContext *s, uint32_t insn) unsigned int mos, type, rm, cond, rn, rd; TCGv_i64 t_true, t_false, t_zero; DisasCompare64 c; + TCGMemOp sz; mos = extract32(insn, 29, 3); - type = extract32(insn, 22, 2); /* 0 = single, 1 = double */ + type = extract32(insn, 22, 2); rm = extract32(insn, 16, 5); cond = extract32(insn, 12, 4); rn = extract32(insn, 5, 5); rd = extract32(insn, 0, 5); - if (mos || type > 1) { + if (mos) { + unallocated_encoding(s); + return; + } + + switch (type) { + case 0: + sz = MO_32; + break; + case 1: + sz = MO_64; + break; + case 3: + sz = MO_16; + if (arm_dc_feature(s, ARM_FEATURE_V8_FP16)) { + break; + } + /* fallthru */ + default: unallocated_encoding(s); return; } @@ -XXX,XX +XXX,XX @@ static void disas_fp_csel(DisasContext *s, uint32_t insn) return; } - /* Zero extend sreg inputs to 64 bits now. */ + /* Zero extend sreg & hreg inputs to 64 bits now. */ t_true = tcg_temp_new_i64(); t_false = tcg_temp_new_i64(); - read_vec_element(s, t_true, rn, 0, type ? MO_64 : MO_32); - read_vec_element(s, t_false, rm, 0, type ? MO_64 : MO_32); + read_vec_element(s, t_true, rn, 0, sz); + read_vec_element(s, t_false, rm, 0, sz); a64_test_cc(&c, cond); t_zero = tcg_const_i64(0); @@ -XXX,XX +XXX,XX @@ static void disas_fp_csel(DisasContext *s, uint32_t insn) tcg_temp_free_i64(t_false); a64_free_cc(&c); - /* Note that sregs write back zeros to the high bits, + /* Note that sregs & hregs write back zeros to the high bits, and we've already done the zero-extension. */ write_fp_dreg(s, rd, t_true); tcg_temp_free_i64(t_true); -- 2.17.0 From: Alex Bennée <alex.bennee@linaro.org> All the hard work is already done by vfp_expand_imm, we just need to make sure we pick up the correct size. Cc: qemu-stable@nongnu.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180512003217.9105-11-richard.henderson@linaro.org [rth: Merge unallocated_encoding check with TCGMemOp conversion.] Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.c +++ b/target/arm/translate-a64.c @@ -XXX,XX +XXX,XX @@ static void disas_fp_imm(DisasContext *s, uint32_t insn) { int rd = extract32(insn, 0, 5); int imm8 = extract32(insn, 13, 8); - int is_double = extract32(insn, 22, 2); + int type = extract32(insn, 22, 2); uint64_t imm; TCGv_i64 tcg_res; + TCGMemOp sz; - if (is_double > 1) { + switch (type) { + case 0: + sz = MO_32; + break; + case 1: + sz = MO_64; + break; + case 3: + sz = MO_16; + if (arm_dc_feature(s, ARM_FEATURE_V8_FP16)) { + break; + } + /* fallthru */ + default: unallocated_encoding(s); return; } @@ -XXX,XX +XXX,XX @@ static void disas_fp_imm(DisasContext *s, uint32_t insn) return; } - imm = vfp_expand_imm(MO_32 + is_double, imm8); + imm = vfp_expand_imm(sz, imm8); tcg_res = tcg_const_i64(imm); write_fp_dreg(s, rd, tcg_res); -- 2.17.0 From: Alex Bennée <alex.bennee@linaro.org> We are meant to explicitly pass fpst, not cpu_env. Cc: qemu-stable@nongnu.org Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20180512003217.9105-12-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 handle_fp_1src_half(DisasContext *s, int opcode, int rd, int rn) tcg_gen_xori_i32(tcg_res, tcg_op, 0x8000); break; case 0x3: /* FSQRT */ - gen_helper_sqrt_f16(tcg_res, tcg_op, cpu_env); + fpst = get_fpstatus_ptr(true); + gen_helper_sqrt_f16(tcg_res, tcg_op, fpst); break; case 0x8: /* FRINTN */ case 0x9: /* FRINTP */ -- 2.17.0 From: Philippe Mathieu-Daudé <f4bug@amsat.org> Per the Physical Layer Simplified Spec. "4.3.10.4 Switch Function Status": The block length is predefined to 512 bits and "4.10.2 SD Status": The SD Status contains status bits that are related to the SD Memory Card proprietary features and may be used for future application-specific usage. The size of the SD Status is one data block of 512 bit. The content of this register is transmitted to the Host over the DAT bus along with a 16-bit CRC. Thus the 16-bit CRC goes at offset 64. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20180509060104.4458-3-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/sd/sd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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_function_switch(SDState *sd, uint32_t arg) sd->data[14 + (i >> 1)] = new_func << ((i * 4) & 4); } memset(&sd->data[17], 0, 47); - stw_be_p(sd->data + 65, sd_crc16(sd->data, 64)); + stw_be_p(sd->data + 64, sd_crc16(sd->data, 64)); } static inline bool sd_wp_addr(SDState *sd, uint64_t addr) -- 2.17.0 Usually the logging of the CPU state produced by -d cpu is sufficient to diagnose problems, but sometimes you want to see the state of the floating point registers as well. We don't want to enable that by default as it adds a lot of extra data to the log; instead, allow it to be optionally enabled via -d fpu. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20180510130024.31678-1-peter.maydell@linaro.org --- include/qemu/log.h | 1 + accel/tcg/cpu-exec.c | 9 ++++++--- util/log.c | 2 ++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/include/qemu/log.h b/include/qemu/log.h index XXXXXXX..XXXXXXX 100644 --- a/include/qemu/log.h +++ b/include/qemu/log.h @@ -XXX,XX +XXX,XX @@ static inline bool qemu_log_separate(void) #define CPU_LOG_PAGE (1 << 14) /* LOG_TRACE (1 << 15) is defined in log-for-trace.h */ #define CPU_LOG_TB_OP_IND (1 << 16) +#define CPU_LOG_TB_FPU (1 << 17) /* Lock output for a series of related logs. Since this is not needed * for a single qemu_log / qemu_log_mask / qemu_log_mask_and_addr, we diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index XXXXXXX..XXXXXXX 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -XXX,XX +XXX,XX @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, TranslationBlock *itb) if (qemu_loglevel_mask(CPU_LOG_TB_CPU) && qemu_log_in_addr_range(itb->pc)) { qemu_log_lock(); + int flags = 0; + if (qemu_loglevel_mask(CPU_LOG_TB_FPU)) { + flags |= CPU_DUMP_FPU; + } #if defined(TARGET_I386) - log_cpu_state(cpu, CPU_DUMP_CCOP); -#else - log_cpu_state(cpu, 0); + flags |= CPU_DUMP_CCOP; #endif + log_cpu_state(cpu, flags); qemu_log_unlock(); } #endif /* DEBUG_DISAS */ diff --git a/util/log.c b/util/log.c index XXXXXXX..XXXXXXX 100644 --- a/util/log.c +++ b/util/log.c @@ -XXX,XX +XXX,XX @@ const QEMULogItem qemu_log_items[] = { "show trace before each executed TB (lots of logs)" }, { CPU_LOG_TB_CPU, "cpu", "show CPU registers before entering a TB (lots of logs)" }, + { CPU_LOG_TB_FPU, "fpu", + "include FPU registers in the 'cpu' logging" }, { CPU_LOG_MMU, "mmu", "log MMU-related activities" }, { CPU_LOG_PCALL, "pcall", -- 2.17.0 First arm pullreq of 5.0! The following changes since commit 084a398bf8aa7634738e6c6c0103236ee1b3b72f: Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-12-13 18:14:07 +0000) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20191216-1 for you to fetch changes up to f80741d107673f162e3b097fc76a1590036cc9d1: target/arm: ensure we use current exception state after SCR update (2019-12-16 10:52:58 +0000) ---------------------------------------------------------------- target-arm queue: * Add support for Cortex-M7 CPU * exynos4210_gic: Suppress gcc9 format-truncation warnings * aspeed: Various minor bug fixes and improvements * aspeed: Add support for the tacoma-bmc board * Honour HCR_EL32.TID1 and .TID2 trapping requirements * Handle trapping to EL2 of AArch32 VMRS instructions * Handle AArch32 CP15 trapping via HSTR_EL2 * Add support for missing Jazelle system registers * arm/arm-powerctl: set NSACR.{CP11, CP10} bits in arm_set_cpu_on * Add support for DC CVAP & DC CVADP instructions * Fix assertion when SCR.NS is changed in Secure-SVC &c * enable SHPC native hot plug in arm ACPI ---------------------------------------------------------------- Alex Bennée (1): target/arm: ensure we use current exception state after SCR update Beata Michalska (4): tcg: cputlb: Add probe_read Memory: Enable writeback for given memory region migration: ram: Switch to ram block writeback target/arm: Add support for DC CVAP & DC CVADP ins Christophe Lyon (1): target/arm: Add support for cortex-m7 CPU Cédric Le Goater (12): aspeed/i2c: Add support for pool buffer transfers aspeed/i2c: Check SRAM enablement on AST2500 aspeed: Add a DRAM memory region at the SoC level aspeed/i2c: Add support for DMA transfers aspeed/i2c: Add trace events aspeed/smc: Restore default AHB window mapping at reset aspeed/smc: Do not map disabled segment on the AST2600 aspeed/smc: Add AST2600 timings registers aspeed: Remove AspeedBoardConfig array and use AspeedMachineClass aspeed: Add support for the tacoma-bmc board aspeed: Change the "scu" property definition aspeed: Change the "nic" property definition David Gibson (1): exynos4210_gic: Suppress gcc9 format-truncation warnings Heyi Guo (2): hw/arm/acpi: simplify AML bit and/or statement hw/arm/acpi: enable SHPC native hot plug Joel Stanley (4): aspeed/sdmc: Make ast2600 default 1G aspeed/scu: Fix W1C behavior watchdog/aspeed: Improve watchdog timeout message watchdog/aspeed: Fix AST2600 frequency behaviour Marc Zyngier (5): target/arm: Honor HCR_EL2.TID2 trapping requirements target/arm: Honor HCR_EL2.TID1 trapping requirements target/arm: Handle trapping to EL2 of AArch32 VMRS instructions target/arm: Handle AArch32 CP15 trapping via HSTR_EL2 target/arm: Add support for missing Jazelle system registers Niek Linnenbank (1): arm/arm-powerctl: set NSACR.{CP11, CP10} bits in arm_set_cpu_on() PanNengyuan (1): gpio: fix memory leak in aspeed_gpio_init() Philippe Mathieu-Daudé (2): hw/arm/sbsa-ref: Simplify by moving the gic in the machine state hw/arm/virt: Simplify by moving the gic in the machine state include/exec/exec-all.h | 6 + include/exec/memory.h | 6 + include/exec/ram_addr.h | 8 + include/hw/arm/aspeed.h | 24 +-- include/hw/arm/aspeed_soc.h | 1 + include/hw/arm/virt.h | 1 + include/hw/i2c/aspeed_i2c.h | 16 ++ include/hw/ssi/aspeed_smc.h | 1 + include/hw/watchdog/wdt_aspeed.h | 1 + include/qemu/cutils.h | 1 + target/arm/cpu.h | 20 +- target/arm/helper.h | 3 + target/arm/translate.h | 2 + exec.c | 36 ++++ hw/arm/aspeed.c | 271 +++++++++++++---------- hw/arm/aspeed_ast2600.c | 25 ++- hw/arm/aspeed_soc.c | 22 +- hw/arm/sbsa-ref.c | 86 ++++---- hw/arm/virt-acpi-build.c | 21 +- hw/arm/virt.c | 109 +++++----- hw/gpio/aspeed_gpio.c | 1 + hw/i2c/aspeed_i2c.c | 439 +++++++++++++++++++++++++++++++++++--- hw/intc/exynos4210_gic.c | 9 +- hw/misc/aspeed_scu.c | 19 +- hw/misc/aspeed_sdmc.c | 6 +- hw/net/ftgmac100.c | 19 +- hw/ssi/aspeed_smc.c | 63 ++++-- hw/timer/aspeed_timer.c | 17 +- hw/watchdog/wdt_aspeed.c | 41 ++-- linux-user/elfload.c | 2 + memory.c | 12 ++ migration/ram.c | 5 +- target/arm/arm-powerctl.c | 3 + target/arm/cpu.c | 33 +++ target/arm/cpu64.c | 1 + target/arm/helper.c | 170 ++++++++++++++- target/arm/op_helper.c | 22 ++ target/arm/translate-vfp.inc.c | 20 +- target/arm/translate.c | 9 +- target/arm/vfp_helper.c | 29 +++ util/cutils.c | 38 ++++ hw/i2c/trace-events | 9 + tests/data/acpi/virt/DSDT | Bin 18470 -> 18462 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19807 -> 19799 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18470 -> 18462 bytes 45 files changed, 1273 insertions(+), 354 deletions(-) From: Christophe Lyon <christophe.lyon@linaro.org> This is derived from cortex-m4 description, adding DP support and FPv5 instructions with the corresponding flags in isar and mvfr2. Checked that it could successfully execute vrinta.f32 s15, s15 while cortex-m4 emulation rejects it with "illegal instruction". Signed-off-by: Christophe Lyon <christophe.lyon@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20191025090841.10299-1-christophe.lyon@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) 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 cortex_m4_initfn(Object *obj) cpu->isar.id_isar6 = 0x00000000; } +static void cortex_m7_initfn(Object *obj) +{ + ARMCPU *cpu = ARM_CPU(obj); + + set_feature(&cpu->env, ARM_FEATURE_V7); + set_feature(&cpu->env, ARM_FEATURE_M); + set_feature(&cpu->env, ARM_FEATURE_M_MAIN); + set_feature(&cpu->env, ARM_FEATURE_THUMB_DSP); + set_feature(&cpu->env, ARM_FEATURE_VFP4); + cpu->midr = 0x411fc272; /* r1p2 */ + cpu->pmsav7_dregion = 8; + cpu->isar.mvfr0 = 0x10110221; + cpu->isar.mvfr1 = 0x12000011; + cpu->isar.mvfr2 = 0x00000040; + cpu->id_pfr0 = 0x00000030; + cpu->id_pfr1 = 0x00000200; + cpu->id_dfr0 = 0x00100000; + cpu->id_afr0 = 0x00000000; + cpu->id_mmfr0 = 0x00100030; + cpu->id_mmfr1 = 0x00000000; + cpu->id_mmfr2 = 0x01000000; + cpu->id_mmfr3 = 0x00000000; + cpu->isar.id_isar0 = 0x01101110; + cpu->isar.id_isar1 = 0x02112000; + cpu->isar.id_isar2 = 0x20232231; + cpu->isar.id_isar3 = 0x01111131; + cpu->isar.id_isar4 = 0x01310132; + cpu->isar.id_isar5 = 0x00000000; + cpu->isar.id_isar6 = 0x00000000; +} + static void cortex_m33_initfn(Object *obj) { ARMCPU *cpu = ARM_CPU(obj); @@ -XXX,XX +XXX,XX @@ static const ARMCPUInfo arm_cpus[] = { .class_init = arm_v7m_class_init }, { .name = "cortex-m4", .initfn = cortex_m4_initfn, .class_init = arm_v7m_class_init }, + { .name = "cortex-m7", .initfn = cortex_m7_initfn, + .class_init = arm_v7m_class_init }, { .name = "cortex-m33", .initfn = cortex_m33_initfn, .class_init = arm_v7m_class_init }, { .name = "cortex-r5", .initfn = cortex_r5_initfn }, -- 2.20.1 From: David Gibson <david@gibson.dropbear.id.au> exynos4210_gic_realize() prints the number of cpus into some temporary buffers, but it only allows 3 bytes space for it. That's plenty: existing machines will only ever set this value to EXYNOS4210_NCPUS (2). But the compiler can't always figure that out, so some[*] gcc9 versions emit -Wformat-truncation warnings. We can fix that by hinting the constraint to the compiler with a suitably placed assert(). [*] The bizarre thing here, is that I've long gotten these warnings compiling in a 32-bit x86 container as host - Fedora 30 with gcc-9.2.1-1.fc30.i686 - but it compiles just fine on my normal x86_64 host - Fedora 30 with and gcc-9.2.1-1.fc30.x86_64. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> [PMM: deleted stray blank line] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/intc/exynos4210_gic.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/exynos4210_gic.c +++ b/hw/intc/exynos4210_gic.c @@ -XXX,XX +XXX,XX @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp) char cpu_alias_name[sizeof(cpu_prefix) + 3]; char dist_alias_name[sizeof(cpu_prefix) + 3]; SysBusDevice *gicbusdev; + uint32_t n = s->num_cpu; uint32_t i; s->gic = qdev_create(NULL, "arm_gic"); @@ -XXX,XX +XXX,XX @@ static void exynos4210_gic_realize(DeviceState *dev, Error **errp) memory_region_init(&s->dist_container, obj, "exynos4210-dist-container", EXYNOS4210_EXT_GIC_DIST_REGION_SIZE); - for (i = 0; i < s->num_cpu; i++) { + /* + * This clues in gcc that our on-stack buffers do, in fact have + * enough room for the cpu numbers. gcc 9.2.1 on 32-bit x86 + * doesn't figure this out, otherwise and gives spurious warnings. + */ + assert(n <= EXYNOS4210_NCPUS); + for (i = 0; i < n; i++) { /* Map CPU interface per SMP Core */ sprintf(cpu_alias_name, "%s%x", cpu_prefix, i); memory_region_init_alias(&s->cpu_alias[i], obj, -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> The Aspeed I2C controller can operate in different transfer modes : - Byte Buffer mode, using a dedicated register to transfer a byte. This is what the model supports today. - Pool Buffer mode, using an internal SRAM to transfer multiple bytes in the same command sequence. Each SoC has different SRAM characteristics. On the AST2400, 2048 bytes of SRAM are available at offset 0x800 of the controller AHB window. The pool buffer can be configured from 1 to 256 bytes per bus. On the AST2500, the SRAM is at offset 0x200 and the pool buffer is of 16 bytes per bus. On the AST2600, the SRAM is at offset 0xC00 and the pool buffer is of 32 bytes per bus. It can be splitted in two for TX and RX but the current model does not add support for it as it it unused by known drivers. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Tested-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-2-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/i2c/aspeed_i2c.h | 8 ++ hw/i2c/aspeed_i2c.c | 197 ++++++++++++++++++++++++++++++++---- 2 files changed, 186 insertions(+), 19 deletions(-) diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/i2c/aspeed_i2c.h +++ b/include/hw/i2c/aspeed_i2c.h @@ -XXX,XX +XXX,XX @@ OBJECT_CHECK(AspeedI2CState, (obj), TYPE_ASPEED_I2C) #define ASPEED_I2C_NR_BUSSES 16 +#define ASPEED_I2C_MAX_POOL_SIZE 0x800 struct AspeedI2CState; @@ -XXX,XX +XXX,XX @@ typedef struct AspeedI2CBus { uint32_t intr_status; uint32_t cmd; uint32_t buf; + uint32_t pool_ctrl; } AspeedI2CBus; typedef struct AspeedI2CState { @@ -XXX,XX +XXX,XX @@ typedef struct AspeedI2CState { qemu_irq irq; uint32_t intr_status; + MemoryRegion pool_iomem; + uint8_t pool[ASPEED_I2C_MAX_POOL_SIZE]; AspeedI2CBus busses[ASPEED_I2C_NR_BUSSES]; } AspeedI2CState; @@ -XXX,XX +XXX,XX @@ typedef struct AspeedI2CClass { uint8_t reg_size; uint8_t gap; qemu_irq (*bus_get_irq)(AspeedI2CBus *); + + uint64_t pool_size; + hwaddr pool_base; + uint8_t *(*bus_pool_base)(AspeedI2CBus *); } AspeedI2CClass; I2CBus *aspeed_i2c_get_bus(DeviceState *dev, int busnr); diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index XXXXXXX..XXXXXXX 100644 --- a/hw/i2c/aspeed_i2c.c +++ b/hw/i2c/aspeed_i2c.c @@ -XXX,XX +XXX,XX @@ /* I2C Device (Bus) Register */ #define I2CD_FUN_CTRL_REG 0x00 /* I2CD Function Control */ -#define I2CD_BUFF_SEL_MASK (0x7 << 20) -#define I2CD_BUFF_SEL(x) (x << 20) +#define I2CD_POOL_PAGE_SEL(x) (((x) >> 20) & 0x7) /* AST2400 */ #define I2CD_M_SDA_LOCK_EN (0x1 << 16) #define I2CD_MULTI_MASTER_DIS (0x1 << 15) #define I2CD_M_SCL_DRIVE_EN (0x1 << 14) @@ -XXX,XX +XXX,XX @@ #define I2CD_SCL_O_OUT_DIR (0x1 << 12) #define I2CD_BUS_RECOVER_CMD_EN (0x1 << 11) #define I2CD_S_ALT_EN (0x1 << 10) -#define I2CD_RX_DMA_ENABLE (0x1 << 9) -#define I2CD_TX_DMA_ENABLE (0x1 << 8) /* Command Bit */ +#define I2CD_RX_DMA_ENABLE (0x1 << 9) +#define I2CD_TX_DMA_ENABLE (0x1 << 8) +#define I2CD_RX_BUFF_ENABLE (0x1 << 7) +#define I2CD_TX_BUFF_ENABLE (0x1 << 6) #define I2CD_M_STOP_CMD (0x1 << 5) #define I2CD_M_S_RX_CMD_LAST (0x1 << 4) #define I2CD_M_RX_CMD (0x1 << 3) @@ -XXX,XX +XXX,XX @@ #define I2CD_M_START_CMD (0x1) #define I2CD_DEV_ADDR_REG 0x18 /* Slave Device Address */ -#define I2CD_BUF_CTRL_REG 0x1c /* Pool Buffer Control */ +#define I2CD_POOL_CTRL_REG 0x1c /* Pool Buffer Control */ +#define I2CD_POOL_RX_COUNT(x) (((x) >> 24) & 0xff) +#define I2CD_POOL_RX_SIZE(x) ((((x) >> 16) & 0xff) + 1) +#define I2CD_POOL_TX_COUNT(x) ((((x) >> 8) & 0xff) + 1) +#define I2CD_POOL_OFFSET(x) (((x) & 0x3f) << 2) /* AST2400 */ #define I2CD_BYTE_BUF_REG 0x20 /* Transmit/Receive Byte Buffer */ #define I2CD_BYTE_BUF_TX_SHIFT 0 #define I2CD_BYTE_BUF_TX_MASK 0xff @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_i2c_bus_read(void *opaque, hwaddr offset, return bus->intr_ctrl; case I2CD_INTR_STS_REG: return bus->intr_status; + case I2CD_POOL_CTRL_REG: + return bus->pool_ctrl; case I2CD_BYTE_BUF_REG: return bus->buf; case I2CD_CMD_REG: @@ -XXX,XX +XXX,XX @@ static uint8_t aspeed_i2c_get_state(AspeedI2CBus *bus) return (bus->cmd >> I2CD_TX_STATE_SHIFT) & I2CD_TX_STATE_MASK; } +static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start) +{ + AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); + int ret = -1; + int i; + + if (bus->cmd & I2CD_TX_BUFF_ENABLE) { + for (i = pool_start; i < I2CD_POOL_TX_COUNT(bus->pool_ctrl); i++) { + uint8_t *pool_base = aic->bus_pool_base(bus); + + ret = i2c_send(bus->bus, pool_base[i]); + if (ret) { + break; + } + } + bus->cmd &= ~I2CD_TX_BUFF_ENABLE; + } else { + ret = i2c_send(bus->bus, bus->buf); + } + + return ret; +} + +static void aspeed_i2c_bus_recv(AspeedI2CBus *bus) +{ + AspeedI2CState *s = bus->controller; + AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(s); + uint8_t data; + int i; + + if (bus->cmd & I2CD_RX_BUFF_ENABLE) { + uint8_t *pool_base = aic->bus_pool_base(bus); + + for (i = 0; i < I2CD_POOL_RX_SIZE(bus->pool_ctrl); i++) { + pool_base[i] = i2c_recv(bus->bus); + } + + /* Update RX count */ + bus->pool_ctrl &= ~(0xff << 24); + bus->pool_ctrl |= (i & 0xff) << 24; + bus->cmd &= ~I2CD_RX_BUFF_ENABLE; + } else { + data = i2c_recv(bus->bus); + bus->buf = (data & I2CD_BYTE_BUF_RX_MASK) << I2CD_BYTE_BUF_RX_SHIFT; + } +} + static void aspeed_i2c_handle_rx_cmd(AspeedI2CBus *bus) { - uint8_t ret; - aspeed_i2c_set_state(bus, I2CD_MRXD); - ret = i2c_recv(bus->bus); + aspeed_i2c_bus_recv(bus); bus->intr_status |= I2CD_INTR_RX_DONE; - bus->buf = (ret & I2CD_BYTE_BUF_RX_MASK) << I2CD_BYTE_BUF_RX_SHIFT; if (bus->cmd & I2CD_M_S_RX_CMD_LAST) { i2c_nack(bus->bus); } @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_handle_rx_cmd(AspeedI2CBus *bus) aspeed_i2c_set_state(bus, I2CD_MACTIVE); } +static uint8_t aspeed_i2c_get_addr(AspeedI2CBus *bus) +{ + AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); + + if (bus->cmd & I2CD_TX_BUFF_ENABLE) { + uint8_t *pool_base = aic->bus_pool_base(bus); + + return pool_base[0]; + } else { + return bus->buf; + } +} + /* * The state machine needs some refinement. It is only used to track * invalid STOP commands for the moment. */ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value) { + uint8_t pool_start = 0; + bus->cmd &= ~0xFFFF; bus->cmd |= value & 0xFFFF; if (bus->cmd & I2CD_M_START_CMD) { uint8_t state = aspeed_i2c_get_state(bus) & I2CD_MACTIVE ? I2CD_MSTARTR : I2CD_MSTART; + uint8_t addr; aspeed_i2c_set_state(bus, state); - if (i2c_start_transfer(bus->bus, extract32(bus->buf, 1, 7), - extract32(bus->buf, 0, 1))) { + addr = aspeed_i2c_get_addr(bus); + + if (i2c_start_transfer(bus->bus, extract32(addr, 1, 7), + extract32(addr, 0, 1))) { bus->intr_status |= I2CD_INTR_TX_NAK; } else { bus->intr_status |= I2CD_INTR_TX_ACK; } - /* START command is also a TX command, as the slave address is - * sent on the bus */ - bus->cmd &= ~(I2CD_M_START_CMD | I2CD_M_TX_CMD); + bus->cmd &= ~I2CD_M_START_CMD; + + /* + * The START command is also a TX command, as the slave + * address is sent on the bus. Drop the TX flag if nothing + * else needs to be sent in this sequence. + */ + if (bus->cmd & I2CD_TX_BUFF_ENABLE) { + if (I2CD_POOL_TX_COUNT(bus->pool_ctrl) == 1) { + bus->cmd &= ~I2CD_M_TX_CMD; + } else { + /* + * Increase the start index in the TX pool buffer to + * skip the address byte. + */ + pool_start++; + } + } else { + bus->cmd &= ~I2CD_M_TX_CMD; + } /* No slave found */ if (!i2c_bus_busy(bus->bus)) { @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value) if (bus->cmd & I2CD_M_TX_CMD) { aspeed_i2c_set_state(bus, I2CD_MTXD); - if (i2c_send(bus->bus, bus->buf)) { + if (aspeed_i2c_bus_send(bus, pool_start)) { bus->intr_status |= (I2CD_INTR_TX_NAK); i2c_end_transfer(bus->bus); } else { @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_write(void *opaque, hwaddr offset, qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n", __func__); break; + case I2CD_POOL_CTRL_REG: + bus->pool_ctrl &= ~0xffffff; + bus->pool_ctrl |= (value & 0xffffff); + break; + case I2CD_BYTE_BUF_REG: bus->buf = (value & I2CD_BYTE_BUF_TX_MASK) << I2CD_BYTE_BUF_TX_SHIFT; break; @@ -XXX,XX +XXX,XX @@ static const MemoryRegionOps aspeed_i2c_ctrl_ops = { .endianness = DEVICE_LITTLE_ENDIAN, }; +static uint64_t aspeed_i2c_pool_read(void *opaque, hwaddr offset, + unsigned size) +{ + AspeedI2CState *s = opaque; + uint64_t ret = 0; + int i; + + for (i = 0; i < size; i++) { + ret |= (uint64_t) s->pool[offset + i] << (8 * i); + } + + return ret; +} + +static void aspeed_i2c_pool_write(void *opaque, hwaddr offset, + uint64_t value, unsigned size) +{ + AspeedI2CState *s = opaque; + int i; + + for (i = 0; i < size; i++) { + s->pool[offset + i] = (value >> (8 * i)) & 0xFF; + } +} + +static const MemoryRegionOps aspeed_i2c_pool_ops = { + .read = aspeed_i2c_pool_read, + .write = aspeed_i2c_pool_write, + .endianness = DEVICE_LITTLE_ENDIAN, + .valid = { + .min_access_size = 1, + .max_access_size = 4, + }, +}; + static const VMStateDescription aspeed_i2c_bus_vmstate = { .name = TYPE_ASPEED_I2C, - .version_id = 1, - .minimum_version_id = 1, + .version_id = 2, + .minimum_version_id = 2, .fields = (VMStateField[]) { VMSTATE_UINT8(id, AspeedI2CBus), VMSTATE_UINT32(ctrl, AspeedI2CBus), @@ -XXX,XX +XXX,XX @@ static const VMStateDescription aspeed_i2c_bus_vmstate = { VMSTATE_UINT32(intr_status, AspeedI2CBus), VMSTATE_UINT32(cmd, AspeedI2CBus), VMSTATE_UINT32(buf, AspeedI2CBus), + VMSTATE_UINT32(pool_ctrl, AspeedI2CBus), VMSTATE_END_OF_LIST() } }; static const VMStateDescription aspeed_i2c_vmstate = { .name = TYPE_ASPEED_I2C, - .version_id = 1, - .minimum_version_id = 1, + .version_id = 2, + .minimum_version_id = 2, .fields = (VMStateField[]) { VMSTATE_UINT32(intr_status, AspeedI2CState), VMSTATE_STRUCT_ARRAY(busses, AspeedI2CState, ASPEED_I2C_NR_BUSSES, 1, aspeed_i2c_bus_vmstate, AspeedI2CBus), + VMSTATE_UINT8_ARRAY(pool, AspeedI2CState, ASPEED_I2C_MAX_POOL_SIZE), VMSTATE_END_OF_LIST() } }; @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_realize(DeviceState *dev, Error **errp) memory_region_add_subregion(&s->iomem, aic->reg_size * (i + offset), &s->busses[i].mr); } + + memory_region_init_io(&s->pool_iomem, OBJECT(s), &aspeed_i2c_pool_ops, s, + "aspeed.i2c-pool", aic->pool_size); + memory_region_add_subregion(&s->iomem, aic->pool_base, &s->pool_iomem); } static void aspeed_i2c_class_init(ObjectClass *klass, void *data) @@ -XXX,XX +XXX,XX @@ static qemu_irq aspeed_2400_i2c_bus_get_irq(AspeedI2CBus *bus) return bus->controller->irq; } +static uint8_t *aspeed_2400_i2c_bus_pool_base(AspeedI2CBus *bus) +{ + uint8_t *pool_page = + &bus->controller->pool[I2CD_POOL_PAGE_SEL(bus->ctrl) * 0x100]; + + return &pool_page[I2CD_POOL_OFFSET(bus->pool_ctrl)]; +} + static void aspeed_2400_i2c_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static void aspeed_2400_i2c_class_init(ObjectClass *klass, void *data) aic->reg_size = 0x40; aic->gap = 7; aic->bus_get_irq = aspeed_2400_i2c_bus_get_irq; + aic->pool_size = 0x800; + aic->pool_base = 0x800; + aic->bus_pool_base = aspeed_2400_i2c_bus_pool_base; } static const TypeInfo aspeed_2400_i2c_info = { @@ -XXX,XX +XXX,XX @@ static qemu_irq aspeed_2500_i2c_bus_get_irq(AspeedI2CBus *bus) return bus->controller->irq; } +static uint8_t *aspeed_2500_i2c_bus_pool_base(AspeedI2CBus *bus) +{ + return &bus->controller->pool[bus->id * 0x10]; +} + static void aspeed_2500_i2c_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static void aspeed_2500_i2c_class_init(ObjectClass *klass, void *data) aic->reg_size = 0x40; aic->gap = 7; aic->bus_get_irq = aspeed_2500_i2c_bus_get_irq; + aic->pool_size = 0x100; + aic->pool_base = 0x200; + aic->bus_pool_base = aspeed_2500_i2c_bus_pool_base; } static const TypeInfo aspeed_2500_i2c_info = { @@ -XXX,XX +XXX,XX @@ static qemu_irq aspeed_2600_i2c_bus_get_irq(AspeedI2CBus *bus) return bus->irq; } +static uint8_t *aspeed_2600_i2c_bus_pool_base(AspeedI2CBus *bus) +{ + return &bus->controller->pool[bus->id * 0x20]; +} + static void aspeed_2600_i2c_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static void aspeed_2600_i2c_class_init(ObjectClass *klass, void *data) aic->reg_size = 0x80; aic->gap = -1; /* no gap */ aic->bus_get_irq = aspeed_2600_i2c_bus_get_irq; + aic->pool_size = 0x200; + aic->pool_base = 0xC00; + aic->bus_pool_base = aspeed_2600_i2c_bus_pool_base; } static const TypeInfo aspeed_2600_i2c_info = { -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> The SRAM must be enabled before using the Buffer Pool mode or the DMA mode. This is not required on other SoCs. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Tested-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-3-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/i2c/aspeed_i2c.h | 3 +++ hw/i2c/aspeed_i2c.c | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/i2c/aspeed_i2c.h +++ b/include/hw/i2c/aspeed_i2c.h @@ -XXX,XX +XXX,XX @@ typedef struct AspeedI2CState { qemu_irq irq; uint32_t intr_status; + uint32_t ctrl_global; MemoryRegion pool_iomem; uint8_t pool[ASPEED_I2C_MAX_POOL_SIZE]; @@ -XXX,XX +XXX,XX @@ typedef struct AspeedI2CClass { uint64_t pool_size; hwaddr pool_base; uint8_t *(*bus_pool_base)(AspeedI2CBus *); + bool check_sram; + } AspeedI2CClass; I2CBus *aspeed_i2c_get_bus(DeviceState *dev, int busnr); diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index XXXXXXX..XXXXXXX 100644 --- a/hw/i2c/aspeed_i2c.c +++ b/hw/i2c/aspeed_i2c.c @@ -XXX,XX +XXX,XX @@ #define I2C_CTRL_STATUS 0x00 /* Device Interrupt Status */ #define I2C_CTRL_ASSIGN 0x08 /* Device Interrupt Target Assignment */ +#define I2C_CTRL_GLOBAL 0x0C /* Global Control Register */ +#define I2C_CTRL_SRAM_EN BIT(0) /* I2C Device (Bus) Register */ @@ -XXX,XX +XXX,XX @@ static uint8_t aspeed_i2c_get_addr(AspeedI2CBus *bus) } } +static bool aspeed_i2c_check_sram(AspeedI2CBus *bus) +{ + AspeedI2CState *s = bus->controller; + AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(s); + + if (!aic->check_sram) { + return true; + } + + /* + * AST2500: SRAM must be enabled before using the Buffer Pool or + * DMA mode. + */ + if (!(s->ctrl_global & I2C_CTRL_SRAM_EN) && + (bus->cmd & (I2CD_RX_DMA_ENABLE | I2CD_TX_DMA_ENABLE | + I2CD_RX_BUFF_ENABLE | I2CD_TX_BUFF_ENABLE))) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: SRAM is not enabled\n", __func__); + return false; + } + + return true; +} + /* * The state machine needs some refinement. It is only used to track * invalid STOP commands for the moment. @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value) bus->cmd &= ~0xFFFF; bus->cmd |= value & 0xFFFF; + if (!aspeed_i2c_check_sram(bus)) { + return; + } + if (bus->cmd & I2CD_M_START_CMD) { uint8_t state = aspeed_i2c_get_state(bus) & I2CD_MACTIVE ? I2CD_MSTARTR : I2CD_MSTART; @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_i2c_ctrl_read(void *opaque, hwaddr offset, switch (offset) { case I2C_CTRL_STATUS: return s->intr_status; + case I2C_CTRL_GLOBAL: + return s->ctrl_global; default: qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__, offset); @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_i2c_ctrl_read(void *opaque, hwaddr offset, static void aspeed_i2c_ctrl_write(void *opaque, hwaddr offset, uint64_t value, unsigned size) { + AspeedI2CState *s = opaque; + switch (offset) { + case I2C_CTRL_GLOBAL: + s->ctrl_global = value; + break; case I2C_CTRL_STATUS: default: qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n", @@ -XXX,XX +XXX,XX @@ static void aspeed_2500_i2c_class_init(ObjectClass *klass, void *data) aic->pool_size = 0x100; aic->pool_base = 0x200; aic->bus_pool_base = aspeed_2500_i2c_bus_pool_base; + aic->check_sram = true; } static const TypeInfo aspeed_2500_i2c_info = { -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> Currently, we link the DRAM memory region to the FMC model (for DMAs) through a property alias at the SoC level. The I2C model will need a similar region for DMA support, add a DRAM region property at the SoC level for both model to use. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Tested-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-4-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/aspeed_soc.h | 1 + hw/arm/aspeed_ast2600.c | 7 +++++-- hw/arm/aspeed_soc.c | 9 +++++++-- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/aspeed_soc.h +++ b/include/hw/arm/aspeed_soc.h @@ -XXX,XX +XXX,XX @@ typedef struct AspeedSoCState { ARMCPU cpu[ASPEED_CPUS_NUM]; uint32_t num_cpus; A15MPPrivState a7mpcore; + MemoryRegion *dram_mr; MemoryRegion sram; AspeedVICState vic; AspeedRtcState rtc; diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_ast2600.c +++ b/hw/arm/aspeed_ast2600.c @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_init(Object *obj) typename); object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs", &error_abort); - object_property_add_alias(obj, "dram", OBJECT(&s->fmc), "dram", - &error_abort); for (i = 0; i < sc->spis_num; i++) { snprintf(typename, sizeof(typename), "aspeed.spi%d-%s", i + 1, socname); @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) } /* FMC, The number of CS is set at the board level */ + object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram", &err); + if (err) { + error_propagate(errp, err); + return; + } object_property_set_int(OBJECT(&s->fmc), sc->memmap[ASPEED_SDRAM], "sdram-base", &err); if (err) { diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_init(Object *obj) typename); object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs", &error_abort); - object_property_add_alias(obj, "dram", OBJECT(&s->fmc), "dram", - &error_abort); for (i = 0; i < sc->spis_num; i++) { snprintf(typename, sizeof(typename), "aspeed.spi%d-%s", i + 1, socname); @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) aspeed_soc_get_irq(s, ASPEED_I2C)); /* FMC, The number of CS is set at the board level */ + object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram", &err); + if (err) { + error_propagate(errp, err); + return; + } object_property_set_int(OBJECT(&s->fmc), sc->memmap[ASPEED_SDRAM], "sdram-base", &err); if (err) { @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) } static Property aspeed_soc_properties[] = { DEFINE_PROP_UINT32("num-cpus", AspeedSoCState, num_cpus, 0), + DEFINE_PROP_LINK("dram", AspeedSoCState, dram_mr, TYPE_MEMORY_REGION, + MemoryRegion *), DEFINE_PROP_END_OF_LIST(), }; -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> The I2C controller of the Aspeed AST2500 and AST2600 SoCs supports DMA transfers to and from DRAM. A pair of registers defines the buffer address and the length of the DMA transfer. The address should be aligned on 4 bytes and the maximum length should not exceed 4K. The receive or transmit DMA transfer can then be initiated with specific bits in the Command/Status register of the controller. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Tested-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-5-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/i2c/aspeed_i2c.h | 5 ++ hw/arm/aspeed_ast2600.c | 5 ++ hw/arm/aspeed_soc.c | 5 ++ hw/i2c/aspeed_i2c.c | 126 +++++++++++++++++++++++++++++++++++- 4 files changed, 138 insertions(+), 3 deletions(-) diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/i2c/aspeed_i2c.h +++ b/include/hw/i2c/aspeed_i2c.h @@ -XXX,XX +XXX,XX @@ typedef struct AspeedI2CBus { uint32_t cmd; uint32_t buf; uint32_t pool_ctrl; + uint32_t dma_addr; + uint32_t dma_len; } AspeedI2CBus; typedef struct AspeedI2CState { @@ -XXX,XX +XXX,XX @@ typedef struct AspeedI2CState { uint8_t pool[ASPEED_I2C_MAX_POOL_SIZE]; AspeedI2CBus busses[ASPEED_I2C_NR_BUSSES]; + MemoryRegion *dram_mr; + AddressSpace dram_as; } AspeedI2CState; #define ASPEED_I2C_CLASS(klass) \ @@ -XXX,XX +XXX,XX @@ typedef struct AspeedI2CClass { hwaddr pool_base; uint8_t *(*bus_pool_base)(AspeedI2CBus *); bool check_sram; + bool has_dma; } AspeedI2CClass; diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_ast2600.c +++ b/hw/arm/aspeed_ast2600.c @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) } /* I2C */ + object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram", &err); + if (err) { + error_propagate(errp, err); + return; + } object_property_set_bool(OBJECT(&s->i2c), true, "realized", &err); if (err) { error_propagate(errp, err); diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) } /* I2C */ + object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram", &err); + if (err) { + error_propagate(errp, err); + return; + } object_property_set_bool(OBJECT(&s->i2c), true, "realized", &err); if (err) { error_propagate(errp, err); diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index XXXXXXX..XXXXXXX 100644 --- a/hw/i2c/aspeed_i2c.c +++ b/hw/i2c/aspeed_i2c.c @@ -XXX,XX +XXX,XX @@ #include "migration/vmstate.h" #include "qemu/log.h" #include "qemu/module.h" +#include "qemu/error-report.h" +#include "qapi/error.h" #include "hw/i2c/aspeed_i2c.h" #include "hw/irq.h" +#include "hw/qdev-properties.h" /* I2C Global Register */ @@ -XXX,XX +XXX,XX @@ #define I2CD_BYTE_BUF_TX_MASK 0xff #define I2CD_BYTE_BUF_RX_SHIFT 8 #define I2CD_BYTE_BUF_RX_MASK 0xff - +#define I2CD_DMA_ADDR 0x24 /* DMA Buffer Address */ +#define I2CD_DMA_LEN 0x28 /* DMA Transfer Length < 4KB */ static inline bool aspeed_i2c_bus_is_master(AspeedI2CBus *bus) { @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_i2c_bus_read(void *opaque, hwaddr offset, unsigned size) { AspeedI2CBus *bus = opaque; + AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); switch (offset) { case I2CD_FUN_CTRL_REG: @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_i2c_bus_read(void *opaque, hwaddr offset, return bus->buf; case I2CD_CMD_REG: return bus->cmd | (i2c_bus_busy(bus->bus) << 16); + case I2CD_DMA_ADDR: + if (!aic->has_dma) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n", __func__); + return -1; + } + return bus->dma_addr; + case I2CD_DMA_LEN: + if (!aic->has_dma) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n", __func__); + return -1; + } + return bus->dma_len; default: qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__, offset); @@ -XXX,XX +XXX,XX @@ static uint8_t aspeed_i2c_get_state(AspeedI2CBus *bus) return (bus->cmd >> I2CD_TX_STATE_SHIFT) & I2CD_TX_STATE_MASK; } +static int aspeed_i2c_dma_read(AspeedI2CBus *bus, uint8_t *data) +{ + MemTxResult result; + AspeedI2CState *s = bus->controller; + + result = address_space_read(&s->dram_as, bus->dma_addr, + MEMTXATTRS_UNSPECIFIED, data, 1); + if (result != MEMTX_OK) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM read failed @%08x\n", + __func__, bus->dma_addr); + return -1; + } + + bus->dma_addr++; + bus->dma_len--; + return 0; +} + static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start) { AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); @@ -XXX,XX +XXX,XX @@ static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start) } } bus->cmd &= ~I2CD_TX_BUFF_ENABLE; + } else if (bus->cmd & I2CD_TX_DMA_ENABLE) { + while (bus->dma_len) { + uint8_t data; + aspeed_i2c_dma_read(bus, &data); + ret = i2c_send(bus->bus, data); + if (ret) { + break; + } + } + bus->cmd &= ~I2CD_TX_DMA_ENABLE; } else { ret = i2c_send(bus->bus, bus->buf); } @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus) bus->pool_ctrl &= ~(0xff << 24); bus->pool_ctrl |= (i & 0xff) << 24; bus->cmd &= ~I2CD_RX_BUFF_ENABLE; + } else if (bus->cmd & I2CD_RX_DMA_ENABLE) { + uint8_t data; + + while (bus->dma_len) { + MemTxResult result; + + data = i2c_recv(bus->bus); + result = address_space_write(&s->dram_as, bus->dma_addr, + MEMTXATTRS_UNSPECIFIED, &data, 1); + if (result != MEMTX_OK) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: DRAM write failed @%08x\n", + __func__, bus->dma_addr); + return; + } + bus->dma_addr++; + bus->dma_len--; + } + bus->cmd &= ~I2CD_RX_DMA_ENABLE; } else { data = i2c_recv(bus->bus); bus->buf = (data & I2CD_BYTE_BUF_RX_MASK) << I2CD_BYTE_BUF_RX_SHIFT; @@ -XXX,XX +XXX,XX @@ static uint8_t aspeed_i2c_get_addr(AspeedI2CBus *bus) uint8_t *pool_base = aic->bus_pool_base(bus); return pool_base[0]; + } else if (bus->cmd & I2CD_TX_DMA_ENABLE) { + uint8_t data; + + aspeed_i2c_dma_read(bus, &data); + return data; } else { return bus->buf; } @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value) */ pool_start++; } + } else if (bus->cmd & I2CD_TX_DMA_ENABLE) { + if (bus->dma_len == 0) { + bus->cmd &= ~I2CD_M_TX_CMD; + } } else { bus->cmd &= ~I2CD_M_TX_CMD; } @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_write(void *opaque, hwaddr offset, break; } + if (!aic->has_dma && + value & (I2CD_RX_DMA_ENABLE | I2CD_TX_DMA_ENABLE)) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n", __func__); + break; + } + aspeed_i2c_bus_handle_cmd(bus, value); aspeed_i2c_bus_raise_interrupt(bus); break; + case I2CD_DMA_ADDR: + if (!aic->has_dma) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n", __func__); + break; + } + + bus->dma_addr = value & 0xfffffffc; + break; + + case I2CD_DMA_LEN: + if (!aic->has_dma) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n", __func__); + break; + } + + bus->dma_len = value & 0xfff; + if (!bus->dma_len) { + qemu_log_mask(LOG_UNIMP, "%s: invalid DMA length\n", __func__); + } + break; default: qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n", @@ -XXX,XX +XXX,XX @@ static const MemoryRegionOps aspeed_i2c_pool_ops = { static const VMStateDescription aspeed_i2c_bus_vmstate = { .name = TYPE_ASPEED_I2C, - .version_id = 2, - .minimum_version_id = 2, + .version_id = 3, + .minimum_version_id = 3, .fields = (VMStateField[]) { VMSTATE_UINT8(id, AspeedI2CBus), VMSTATE_UINT32(ctrl, AspeedI2CBus), @@ -XXX,XX +XXX,XX @@ static const VMStateDescription aspeed_i2c_bus_vmstate = { VMSTATE_UINT32(cmd, AspeedI2CBus), VMSTATE_UINT32(buf, AspeedI2CBus), VMSTATE_UINT32(pool_ctrl, AspeedI2CBus), + VMSTATE_UINT32(dma_addr, AspeedI2CBus), + VMSTATE_UINT32(dma_len, AspeedI2CBus), VMSTATE_END_OF_LIST() } }; @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_reset(DeviceState *dev) s->busses[i].intr_status = 0; s->busses[i].cmd = 0; s->busses[i].buf = 0; + s->busses[i].dma_addr = 0; + s->busses[i].dma_len = 0; i2c_end_transfer(s->busses[i].bus); } } @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_realize(DeviceState *dev, Error **errp) memory_region_init_io(&s->pool_iomem, OBJECT(s), &aspeed_i2c_pool_ops, s, "aspeed.i2c-pool", aic->pool_size); memory_region_add_subregion(&s->iomem, aic->pool_base, &s->pool_iomem); + + if (aic->has_dma) { + if (!s->dram_mr) { + error_setg(errp, TYPE_ASPEED_I2C ": 'dram' link not set"); + return; + } + + address_space_init(&s->dram_as, s->dram_mr, "dma-dram"); + } } +static Property aspeed_i2c_properties[] = { + DEFINE_PROP_LINK("dram", AspeedI2CState, dram_mr, + TYPE_MEMORY_REGION, MemoryRegion *), + DEFINE_PROP_END_OF_LIST(), +}; + static void aspeed_i2c_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); dc->vmsd = &aspeed_i2c_vmstate; dc->reset = aspeed_i2c_reset; + dc->props = aspeed_i2c_properties; dc->realize = aspeed_i2c_realize; dc->desc = "Aspeed I2C Controller"; } @@ -XXX,XX +XXX,XX @@ static void aspeed_2500_i2c_class_init(ObjectClass *klass, void *data) aic->pool_base = 0x200; aic->bus_pool_base = aspeed_2500_i2c_bus_pool_base; aic->check_sram = true; + aic->has_dma = true; } static const TypeInfo aspeed_2500_i2c_info = { @@ -XXX,XX +XXX,XX @@ static void aspeed_2600_i2c_class_init(ObjectClass *klass, void *data) aic->pool_size = 0x200; aic->pool_base = 0xC00; aic->bus_pool_base = aspeed_2600_i2c_bus_pool_base; + aic->has_dma = true; } static const TypeInfo aspeed_2600_i2c_info = { -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Tested-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-6-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/i2c/aspeed_i2c.c | 93 ++++++++++++++++++++++++++++++++++++++------- hw/i2c/trace-events | 9 +++++ 2 files changed, 89 insertions(+), 13 deletions(-) diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index XXXXXXX..XXXXXXX 100644 --- a/hw/i2c/aspeed_i2c.c +++ b/hw/i2c/aspeed_i2c.c @@ -XXX,XX +XXX,XX @@ #include "hw/i2c/aspeed_i2c.h" #include "hw/irq.h" #include "hw/qdev-properties.h" +#include "trace.h" /* I2C Global Register */ @@ -XXX,XX +XXX,XX @@ static inline void aspeed_i2c_bus_raise_interrupt(AspeedI2CBus *bus) { AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); + trace_aspeed_i2c_bus_raise_interrupt(bus->intr_status, + bus->intr_status & I2CD_INTR_TX_NAK ? "nak|" : "", + bus->intr_status & I2CD_INTR_TX_ACK ? "ack|" : "", + bus->intr_status & I2CD_INTR_RX_DONE ? "done|" : "", + bus->intr_status & I2CD_INTR_NORMAL_STOP ? "normal|" : "", + bus->intr_status & I2CD_INTR_ABNORMAL ? "abnormal" : ""); + bus->intr_status &= bus->intr_ctrl; if (bus->intr_status) { bus->controller->intr_status |= 1 << bus->id; @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_i2c_bus_read(void *opaque, hwaddr offset, { AspeedI2CBus *bus = opaque; AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); + uint64_t value = -1; switch (offset) { case I2CD_FUN_CTRL_REG: - return bus->ctrl; + value = bus->ctrl; + break; case I2CD_AC_TIMING_REG1: - return bus->timing[0]; + value = bus->timing[0]; + break; case I2CD_AC_TIMING_REG2: - return bus->timing[1]; + value = bus->timing[1]; + break; case I2CD_INTR_CTRL_REG: - return bus->intr_ctrl; + value = bus->intr_ctrl; + break; case I2CD_INTR_STS_REG: - return bus->intr_status; + value = bus->intr_status; + break; case I2CD_POOL_CTRL_REG: - return bus->pool_ctrl; + value = bus->pool_ctrl; + break; case I2CD_BYTE_BUF_REG: - return bus->buf; + value = bus->buf; + break; case I2CD_CMD_REG: - return bus->cmd | (i2c_bus_busy(bus->bus) << 16); + value = bus->cmd | (i2c_bus_busy(bus->bus) << 16); + break; case I2CD_DMA_ADDR: if (!aic->has_dma) { qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n", __func__); - return -1; + break; } - return bus->dma_addr; + value = bus->dma_addr; + break; case I2CD_DMA_LEN: if (!aic->has_dma) { qemu_log_mask(LOG_GUEST_ERROR, "%s: No DMA support\n", __func__); - return -1; + break; } - return bus->dma_len; + value = bus->dma_len; + break; + default: qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__, offset); - return -1; + value = -1; + break; } + + trace_aspeed_i2c_bus_read(bus->id, offset, size, value); + return value; } static void aspeed_i2c_set_state(AspeedI2CBus *bus, uint8_t state) @@ -XXX,XX +XXX,XX @@ static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start) for (i = pool_start; i < I2CD_POOL_TX_COUNT(bus->pool_ctrl); i++) { uint8_t *pool_base = aic->bus_pool_base(bus); + trace_aspeed_i2c_bus_send("BUF", i + 1, + I2CD_POOL_TX_COUNT(bus->pool_ctrl), + pool_base[i]); ret = i2c_send(bus->bus, pool_base[i]); if (ret) { break; @@ -XXX,XX +XXX,XX @@ static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start) while (bus->dma_len) { uint8_t data; aspeed_i2c_dma_read(bus, &data); + trace_aspeed_i2c_bus_send("DMA", bus->dma_len, bus->dma_len, data); ret = i2c_send(bus->bus, data); if (ret) { break; @@ -XXX,XX +XXX,XX @@ static int aspeed_i2c_bus_send(AspeedI2CBus *bus, uint8_t pool_start) } bus->cmd &= ~I2CD_TX_DMA_ENABLE; } else { + trace_aspeed_i2c_bus_send("BYTE", pool_start, 1, bus->buf); ret = i2c_send(bus->bus, bus->buf); } @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus) for (i = 0; i < I2CD_POOL_RX_SIZE(bus->pool_ctrl); i++) { pool_base[i] = i2c_recv(bus->bus); + trace_aspeed_i2c_bus_recv("BUF", i + 1, + I2CD_POOL_RX_SIZE(bus->pool_ctrl), + pool_base[i]); } /* Update RX count */ @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus) MemTxResult result; data = i2c_recv(bus->bus); + trace_aspeed_i2c_bus_recv("DMA", bus->dma_len, bus->dma_len, data); result = address_space_write(&s->dram_as, bus->dma_addr, MEMTXATTRS_UNSPECIFIED, &data, 1); if (result != MEMTX_OK) { @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_recv(AspeedI2CBus *bus) bus->cmd &= ~I2CD_RX_DMA_ENABLE; } else { data = i2c_recv(bus->bus); + trace_aspeed_i2c_bus_recv("BYTE", 1, 1, bus->buf); bus->buf = (data & I2CD_BYTE_BUF_RX_MASK) << I2CD_BYTE_BUF_RX_SHIFT; } } @@ -XXX,XX +XXX,XX @@ static bool aspeed_i2c_check_sram(AspeedI2CBus *bus) return true; } +static void aspeed_i2c_bus_cmd_dump(AspeedI2CBus *bus) +{ + g_autofree char *cmd_flags; + uint32_t count; + + if (bus->cmd & (I2CD_RX_BUFF_ENABLE | I2CD_RX_BUFF_ENABLE)) { + count = I2CD_POOL_TX_COUNT(bus->pool_ctrl); + } else if (bus->cmd & (I2CD_RX_DMA_ENABLE | I2CD_RX_DMA_ENABLE)) { + count = bus->dma_len; + } else { /* BYTE mode */ + count = 1; + } + + cmd_flags = g_strdup_printf("%s%s%s%s%s%s%s%s%s", + bus->cmd & I2CD_M_START_CMD ? "start|" : "", + bus->cmd & I2CD_RX_DMA_ENABLE ? "rxdma|" : "", + bus->cmd & I2CD_TX_DMA_ENABLE ? "txdma|" : "", + bus->cmd & I2CD_RX_BUFF_ENABLE ? "rxbuf|" : "", + bus->cmd & I2CD_TX_BUFF_ENABLE ? "txbuf|" : "", + bus->cmd & I2CD_M_TX_CMD ? "tx|" : "", + bus->cmd & I2CD_M_RX_CMD ? "rx|" : "", + bus->cmd & I2CD_M_S_RX_CMD_LAST ? "last|" : "", + bus->cmd & I2CD_M_STOP_CMD ? "stop" : ""); + + trace_aspeed_i2c_bus_cmd(bus->cmd, cmd_flags, count, bus->intr_status); +} + /* * The state machine needs some refinement. It is only used to track * invalid STOP commands for the moment. @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_handle_cmd(AspeedI2CBus *bus, uint64_t value) return; } + if (trace_event_get_state_backends(TRACE_ASPEED_I2C_BUS_CMD)) { + aspeed_i2c_bus_cmd_dump(bus); + } + if (bus->cmd & I2CD_M_START_CMD) { uint8_t state = aspeed_i2c_get_state(bus) & I2CD_MACTIVE ? I2CD_MSTARTR : I2CD_MSTART; @@ -XXX,XX +XXX,XX @@ static void aspeed_i2c_bus_write(void *opaque, hwaddr offset, AspeedI2CClass *aic = ASPEED_I2C_GET_CLASS(bus->controller); bool handle_rx; + trace_aspeed_i2c_bus_write(bus->id, offset, size, value); + switch (offset) { case I2CD_FUN_CTRL_REG: if (value & I2CD_SLAVE_EN) { diff --git a/hw/i2c/trace-events b/hw/i2c/trace-events index XXXXXXX..XXXXXXX 100644 --- a/hw/i2c/trace-events +++ b/hw/i2c/trace-events @@ -XXX,XX +XXX,XX @@ i2c_event(const char *event, uint8_t address) "%s(addr:0x%02x)" i2c_send(uint8_t address, uint8_t data) "send(addr:0x%02x) data:0x%02x" i2c_recv(uint8_t address, uint8_t data) "recv(addr:0x%02x) data:0x%02x" + +# aspeed_i2c.c + +aspeed_i2c_bus_cmd(uint32_t cmd, const char *cmd_flags, uint32_t count, uint32_t intr_status) "handling cmd=0x%x %s count=%d intr=0x%x" +aspeed_i2c_bus_raise_interrupt(uint32_t intr_status, const char *str1, const char *str2, const char *str3, const char *str4, const char *str5) "handled intr=0x%x %s%s%s%s%s" +aspeed_i2c_bus_read(uint32_t busid, uint64_t offset, unsigned size, uint64_t value) "bus[%d]: To 0x%" PRIx64 " of size %u: 0x%" PRIx64 +aspeed_i2c_bus_write(uint32_t busid, uint64_t offset, unsigned size, uint64_t value) "bus[%d]: To 0x%" PRIx64 " of size %u: 0x%" PRIx64 +aspeed_i2c_bus_send(const char *mode, int i, int count, uint8_t byte) "%s send %d/%d 0x%02x" +aspeed_i2c_bus_recv(const char *mode, int i, int count, uint8_t byte) "%s recv %d/%d 0x%02x" -- 2.20.1 From: Joel Stanley <joel@jms.id.au> Most boards have this much. Reviewed-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-7-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/misc/aspeed_sdmc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/aspeed_sdmc.c +++ b/hw/misc/aspeed_sdmc.c @@ -XXX,XX +XXX,XX @@ static int ast2600_rambits(AspeedSDMCState *s) } /* use a common default */ - warn_report("Invalid RAM size 0x%" PRIx64 ". Using default 512M", + warn_report("Invalid RAM size 0x%" PRIx64 ". Using default 1024M", s->ram_size); - s->ram_size = 512 << 20; - return ASPEED_SDMC_AST2600_512MB; + s->ram_size = 1024 << 20; + return ASPEED_SDMC_AST2600_1024MB; } static void aspeed_sdmc_reset(DeviceState *dev) -- 2.20.1 From: Joel Stanley <joel@jms.id.au> This models the clock write one to clear registers, and fixes up some incorrect behavior in all of the write to clear registers. There was also a typo in one of the register definitions. Reviewed-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-8-clg@kaod.org [clg: checkpatch.pl fixes ] Signed-off-by: Cédric Le Goater <clg@kaod.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/misc/aspeed_scu.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/aspeed_scu.c +++ b/hw/misc/aspeed_scu.c @@ -XXX,XX +XXX,XX @@ #define AST2600_CLK_STOP_CTRL TO_REG(0x80) #define AST2600_CLK_STOP_CTRL_CLR TO_REG(0x84) #define AST2600_CLK_STOP_CTRL2 TO_REG(0x90) -#define AST2600_CLK_STOP_CTR2L_CLR TO_REG(0x94) +#define AST2600_CLK_STOP_CTRL2_CLR TO_REG(0x94) #define AST2600_SDRAM_HANDSHAKE TO_REG(0x100) #define AST2600_HPLL_PARAM TO_REG(0x200) #define AST2600_HPLL_EXT TO_REG(0x204) @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_ast2600_scu_read(void *opaque, hwaddr offset, return s->regs[reg]; } -static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset, uint64_t data, - unsigned size) +static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset, + uint64_t data64, unsigned size) { AspeedSCUState *s = ASPEED_SCU(opaque); int reg = TO_REG(offset); + /* Truncate here so bitwise operations below behave as expected */ + uint32_t data = data64; if (reg >= ASPEED_AST2600_SCU_NR_REGS) { qemu_log_mask(LOG_GUEST_ERROR, @@ -XXX,XX +XXX,XX @@ static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset, uint64_t data, /* fall through */ case AST2600_SYS_RST_CTRL: case AST2600_SYS_RST_CTRL2: + case AST2600_CLK_STOP_CTRL: + case AST2600_CLK_STOP_CTRL2: /* W1S (Write 1 to set) registers */ s->regs[reg] |= data; return; case AST2600_SYS_RST_CTRL_CLR: case AST2600_SYS_RST_CTRL2_CLR: + case AST2600_CLK_STOP_CTRL_CLR: + case AST2600_CLK_STOP_CTRL2_CLR: case AST2600_HW_STRAP1_CLR: case AST2600_HW_STRAP2_CLR: - /* W1C (Write 1 to clear) registers */ - s->regs[reg] &= ~data; + /* + * W1C (Write 1 to clear) registers are offset by one address from + * the data register + */ + s->regs[reg - 1] &= ~data; return; case AST2600_RNG_DATA: -- 2.20.1 From: Joel Stanley <joel@jms.id.au> Users benefit from knowing which watchdog timer has expired. The address of the watchdog's registers unambiguously indicates which has expired, so log that. Reviewed-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-9-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/watchdog/wdt_aspeed.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c index XXXXXXX..XXXXXXX 100644 --- a/hw/watchdog/wdt_aspeed.c +++ b/hw/watchdog/wdt_aspeed.c @@ -XXX,XX +XXX,XX @@ static void aspeed_wdt_timer_expired(void *dev) return; } - qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n"); + qemu_log_mask(CPU_LOG_RESET, "Watchdog timer %" HWADDR_PRIx " expired.\n", + s->iomem.addr); watchdog_perform_action(); timer_del(s->timer); } -- 2.20.1 From: Joel Stanley <joel@jms.id.au> The AST2600 control register sneakily changed the meaning of bit 4 without anyone noticing. It no longer controls the 1MHz vs APB clock select, and instead always runs at 1MHz. The AST2500 was always 1MHz too, but it retained bit 4, making it read only. We can model both using the same fixed 1MHz calculation. Fixes: 6b2b2a703cad ("hw: wdt_aspeed: Add AST2600 support") Reviewed-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-10-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/watchdog/wdt_aspeed.h | 1 + hw/watchdog/wdt_aspeed.c | 21 +++++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/include/hw/watchdog/wdt_aspeed.h b/include/hw/watchdog/wdt_aspeed.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/watchdog/wdt_aspeed.h +++ b/include/hw/watchdog/wdt_aspeed.h @@ -XXX,XX +XXX,XX @@ typedef struct AspeedWDTClass { uint32_t ext_pulse_width_mask; uint32_t reset_ctrl_reg; void (*reset_pulse)(AspeedWDTState *s, uint32_t property); + void (*wdt_reload)(AspeedWDTState *s); } AspeedWDTClass; #endif /* WDT_ASPEED_H */ diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c index XXXXXXX..XXXXXXX 100644 --- a/hw/watchdog/wdt_aspeed.c +++ b/hw/watchdog/wdt_aspeed.c @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_wdt_read(void *opaque, hwaddr offset, unsigned size) } -static void aspeed_wdt_reload(AspeedWDTState *s, bool pclk) +static void aspeed_wdt_reload(AspeedWDTState *s) { uint64_t reload; - if (pclk) { + if (!(s->regs[WDT_CTRL] & WDT_CTRL_1MHZ_CLK)) { reload = muldiv64(s->regs[WDT_RELOAD_VALUE], NANOSECONDS_PER_SECOND, s->pclk_freq); } else { @@ -XXX,XX +XXX,XX @@ static void aspeed_wdt_reload(AspeedWDTState *s, bool pclk) } } +static void aspeed_wdt_reload_1mhz(AspeedWDTState *s) +{ + uint64_t reload = s->regs[WDT_RELOAD_VALUE] * 1000ULL; + + if (aspeed_wdt_is_enabled(s)) { + timer_mod(s->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + reload); + } +} + + static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data, unsigned size) { @@ -XXX,XX +XXX,XX @@ static void aspeed_wdt_write(void *opaque, hwaddr offset, uint64_t data, case WDT_RESTART: if ((data & 0xFFFF) == WDT_RESTART_MAGIC) { s->regs[WDT_STATUS] = s->regs[WDT_RELOAD_VALUE]; - aspeed_wdt_reload(s, !(s->regs[WDT_CTRL] & WDT_CTRL_1MHZ_CLK)); + awc->wdt_reload(s); } break; case WDT_CTRL: if (enable && !aspeed_wdt_is_enabled(s)) { s->regs[WDT_CTRL] = data; - aspeed_wdt_reload(s, !(data & WDT_CTRL_1MHZ_CLK)); + awc->wdt_reload(s); } else if (!enable && aspeed_wdt_is_enabled(s)) { s->regs[WDT_CTRL] = data; timer_del(s->timer); @@ -XXX,XX +XXX,XX @@ static void aspeed_2400_wdt_class_init(ObjectClass *klass, void *data) awc->offset = 0x20; awc->ext_pulse_width_mask = 0xff; awc->reset_ctrl_reg = SCU_RESET_CONTROL1; + awc->wdt_reload = aspeed_wdt_reload; } static const TypeInfo aspeed_2400_wdt_info = { @@ -XXX,XX +XXX,XX @@ static void aspeed_2500_wdt_class_init(ObjectClass *klass, void *data) awc->ext_pulse_width_mask = 0xfffff; awc->reset_ctrl_reg = SCU_RESET_CONTROL1; awc->reset_pulse = aspeed_2500_wdt_reset_pulse; + awc->wdt_reload = aspeed_wdt_reload_1mhz; } static const TypeInfo aspeed_2500_wdt_info = { @@ -XXX,XX +XXX,XX @@ static void aspeed_2600_wdt_class_init(ObjectClass *klass, void *data) awc->ext_pulse_width_mask = 0xfffff; /* TODO */ awc->reset_ctrl_reg = AST2600_SCU_RESET_CONTROL1; awc->reset_pulse = aspeed_2500_wdt_reset_pulse; + awc->wdt_reload = aspeed_wdt_reload_1mhz; } static const TypeInfo aspeed_2600_wdt_info = { -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> The current model only restores the Segment Register values but leaves the previous CS mapping behind. Introduce a helper setting the register value and mapping the region at the requested address. Use this helper when a Segment register is set and at reset. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-11-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/ssi/aspeed_smc.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -XXX,XX +XXX,XX @@ static bool aspeed_smc_flash_overlap(const AspeedSMCState *s, return false; } +static void aspeed_smc_flash_set_segment_region(AspeedSMCState *s, int cs, + uint64_t regval) +{ + AspeedSMCFlash *fl = &s->flashes[cs]; + AspeedSegments seg; + + s->ctrl->reg_to_segment(s, regval, &seg); + + memory_region_transaction_begin(); + memory_region_set_size(&fl->mmio, seg.size); + memory_region_set_address(&fl->mmio, seg.addr - s->ctrl->flash_window_base); + memory_region_set_enabled(&fl->mmio, true); + memory_region_transaction_commit(); + + s->regs[R_SEG_ADDR0 + cs] = regval; +} + static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs, uint64_t new) { - AspeedSMCFlash *fl = &s->flashes[cs]; AspeedSegments seg; s->ctrl->reg_to_segment(s, new, &seg); @@ -XXX,XX +XXX,XX @@ static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs, aspeed_smc_flash_overlap(s, &seg, cs); /* All should be fine now to move the region */ - memory_region_transaction_begin(); - memory_region_set_size(&fl->mmio, seg.size); - memory_region_set_address(&fl->mmio, seg.addr - s->ctrl->flash_window_base); - memory_region_set_enabled(&fl->mmio, true); - memory_region_transaction_commit(); - - s->regs[R_SEG_ADDR0 + cs] = new; + aspeed_smc_flash_set_segment_region(s, cs, new); } static uint64_t aspeed_smc_flash_default_read(void *opaque, hwaddr addr, @@ -XXX,XX +XXX,XX @@ static void aspeed_smc_reset(DeviceState *d) qemu_set_irq(s->cs_lines[i], true); } - /* setup default segment register values for all */ + /* setup the default segment register values and regions for all */ for (i = 0; i < s->ctrl->max_slaves; ++i) { - s->regs[R_SEG_ADDR0 + i] = - s->ctrl->segment_to_reg(s, &s->ctrl->segments[i]); + aspeed_smc_flash_set_segment_region(s, i, + s->ctrl->segment_to_reg(s, &s->ctrl->segments[i])); } /* HW strapping flash type for the AST2600 controllers */ -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> The segments can be disabled on the AST2600 (zero register value). CS0 is open by default but not the other CS. This is closing the access to the flash device in user mode and forbids scanning. In the model, check the segment size and disable the associated region when the value is zero. Fixes: bcaa8ddd081c ("aspeed/smc: Add AST2600 support") Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-12-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/ssi/aspeed_smc.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -XXX,XX +XXX,XX @@ static void aspeed_2600_smc_reg_to_segment(const AspeedSMCState *s, uint32_t start_offset = (reg << 16) & AST2600_SEG_ADDR_MASK; uint32_t end_offset = reg & AST2600_SEG_ADDR_MASK; - seg->addr = s->ctrl->flash_window_base + start_offset; - seg->size = end_offset + MiB - start_offset; + if (reg) { + seg->addr = s->ctrl->flash_window_base + start_offset; + seg->size = end_offset + MiB - start_offset; + } else { + seg->addr = s->ctrl->flash_window_base; + seg->size = 0; + } } static bool aspeed_smc_flash_overlap(const AspeedSMCState *s, @@ -XXX,XX +XXX,XX @@ static void aspeed_smc_flash_set_segment_region(AspeedSMCState *s, int cs, memory_region_transaction_begin(); memory_region_set_size(&fl->mmio, seg.size); memory_region_set_address(&fl->mmio, seg.addr - s->ctrl->flash_window_base); - memory_region_set_enabled(&fl->mmio, true); + memory_region_set_enabled(&fl->mmio, !!seg.size); memory_region_transaction_commit(); s->regs[R_SEG_ADDR0 + cs] = regval; @@ -XXX,XX +XXX,XX @@ static void aspeed_smc_flash_set_segment(AspeedSMCState *s, int cs, } /* Keep the segment in the overall flash window */ - if (seg.addr + seg.size <= s->ctrl->flash_window_base || - seg.addr > s->ctrl->flash_window_base + s->ctrl->flash_window_size) { + if (seg.size && + (seg.addr + seg.size <= s->ctrl->flash_window_base || + seg.addr > s->ctrl->flash_window_base + s->ctrl->flash_window_size)) { qemu_log_mask(LOG_GUEST_ERROR, "%s: new segment for CS%d is invalid : " "[ 0x%"HWADDR_PRIx" - 0x%"HWADDR_PRIx" ]\n", s->ctrl->name, cs, seg.addr, seg.addr + seg.size); -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> Each CS has its own Read Timing Compensation Register on newer SoCs. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-13-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/ssi/aspeed_smc.h | 1 + hw/ssi/aspeed_smc.c | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/ssi/aspeed_smc.h +++ b/include/hw/ssi/aspeed_smc.h @@ -XXX,XX +XXX,XX @@ typedef struct AspeedSMCController { uint8_t r_ce_ctrl; uint8_t r_ctrl0; uint8_t r_timings; + uint8_t nregs_timings; uint8_t conf_enable_w0; uint8_t max_slaves; const AspeedSegments *segments; diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -XXX,XX +XXX,XX @@ /* Checksum Calculation Result */ #define R_DMA_CHECKSUM (0x90 / 4) -/* Misc Control Register #2 */ +/* Read Timing Compensation Register */ #define R_TIMINGS (0x94 / 4) /* SPI controller registers and bits (AST2400) */ @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = R_CE_CTRL, .r_ctrl0 = R_CTRL0, .r_timings = R_TIMINGS, + .nregs_timings = 1, .conf_enable_w0 = CONF_ENABLE_W0, .max_slaves = 5, .segments = aspeed_segments_legacy, @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = R_CE_CTRL, .r_ctrl0 = R_CTRL0, .r_timings = R_TIMINGS, + .nregs_timings = 1, .conf_enable_w0 = CONF_ENABLE_W0, .max_slaves = 5, .segments = aspeed_segments_fmc, @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = 0xff, .r_ctrl0 = R_SPI_CTRL0, .r_timings = R_SPI_TIMINGS, + .nregs_timings = 1, .conf_enable_w0 = SPI_CONF_ENABLE_W0, .max_slaves = 1, .segments = aspeed_segments_spi, @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = R_CE_CTRL, .r_ctrl0 = R_CTRL0, .r_timings = R_TIMINGS, + .nregs_timings = 1, .conf_enable_w0 = CONF_ENABLE_W0, .max_slaves = 3, .segments = aspeed_segments_ast2500_fmc, @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = R_CE_CTRL, .r_ctrl0 = R_CTRL0, .r_timings = R_TIMINGS, + .nregs_timings = 1, .conf_enable_w0 = CONF_ENABLE_W0, .max_slaves = 2, .segments = aspeed_segments_ast2500_spi1, @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = R_CE_CTRL, .r_ctrl0 = R_CTRL0, .r_timings = R_TIMINGS, + .nregs_timings = 1, .conf_enable_w0 = CONF_ENABLE_W0, .max_slaves = 2, .segments = aspeed_segments_ast2500_spi2, @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = R_CE_CTRL, .r_ctrl0 = R_CTRL0, .r_timings = R_TIMINGS, + .nregs_timings = 1, .conf_enable_w0 = CONF_ENABLE_W0, .max_slaves = 3, .segments = aspeed_segments_ast2600_fmc, @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = R_CE_CTRL, .r_ctrl0 = R_CTRL0, .r_timings = R_TIMINGS, + .nregs_timings = 2, .conf_enable_w0 = CONF_ENABLE_W0, .max_slaves = 2, .segments = aspeed_segments_ast2600_spi1, @@ -XXX,XX +XXX,XX @@ static const AspeedSMCController controllers[] = { .r_ce_ctrl = R_CE_CTRL, .r_ctrl0 = R_CTRL0, .r_timings = R_TIMINGS, + .nregs_timings = 3, .conf_enable_w0 = CONF_ENABLE_W0, .max_slaves = 3, .segments = aspeed_segments_ast2600_spi2, @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size) addr >>= 2; if (addr == s->r_conf || - addr == s->r_timings || + (addr >= s->r_timings && + addr < s->r_timings + s->ctrl->nregs_timings) || addr == s->r_ce_ctrl || addr == R_INTR_CTRL || addr == R_DUMMY_DATA || @@ -XXX,XX +XXX,XX @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data, addr >>= 2; if (addr == s->r_conf || - addr == s->r_timings || + (addr >= s->r_timings && + addr < s->r_timings + s->ctrl->nregs_timings) || addr == s->r_ce_ctrl) { s->regs[addr] = value; } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs) { -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> AspeedBoardConfig is a redundant way to define class attributes and it complexifies the machine definition and initialization. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-14-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/aspeed.h | 24 ++-- hw/arm/aspeed.c | 243 ++++++++++++++++++++++------------------ 2 files changed, 143 insertions(+), 124 deletions(-) diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/aspeed.h +++ b/include/hw/arm/aspeed.h @@ -XXX,XX +XXX,XX @@ typedef struct AspeedBoardState AspeedBoardState; -typedef struct AspeedBoardConfig { - const char *name; - const char *desc; - const char *soc_name; - uint32_t hw_strap1; - uint32_t hw_strap2; - const char *fmc_model; - const char *spi_model; - uint32_t num_cs; - void (*i2c_init)(AspeedBoardState *bmc); - uint32_t ram; -} AspeedBoardConfig; - #define TYPE_ASPEED_MACHINE MACHINE_TYPE_NAME("aspeed") #define ASPEED_MACHINE(obj) \ OBJECT_CHECK(AspeedMachine, (obj), TYPE_ASPEED_MACHINE) @@ -XXX,XX +XXX,XX @@ typedef struct AspeedMachine { typedef struct AspeedMachineClass { MachineClass parent_obj; - const AspeedBoardConfig *board; + + const char *name; + const char *desc; + const char *soc_name; + uint32_t hw_strap1; + uint32_t hw_strap2; + const char *fmc_model; + const char *spi_model; + uint32_t num_cs; + void (*i2c_init)(AspeedBoardState *bmc); } AspeedMachineClass; diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -XXX,XX +XXX,XX @@ static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype, } } -static void aspeed_board_init(MachineState *machine, - const AspeedBoardConfig *cfg) +static void aspeed_machine_init(MachineState *machine) { AspeedBoardState *bmc; + AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(machine); AspeedSoCClass *sc; DriveInfo *drive0 = drive_get(IF_MTD, 0, 0); ram_addr_t max_ram_size; @@ -XXX,XX +XXX,XX @@ static void aspeed_board_init(MachineState *machine, UINT32_MAX); object_initialize_child(OBJECT(machine), "soc", &bmc->soc, - (sizeof(bmc->soc)), cfg->soc_name, &error_abort, + (sizeof(bmc->soc)), amc->soc_name, &error_abort, NULL); sc = ASPEED_SOC_GET_CLASS(&bmc->soc); object_property_set_uint(OBJECT(&bmc->soc), ram_size, "ram-size", &error_abort); - object_property_set_int(OBJECT(&bmc->soc), cfg->hw_strap1, "hw-strap1", + object_property_set_int(OBJECT(&bmc->soc), amc->hw_strap1, "hw-strap1", &error_abort); - object_property_set_int(OBJECT(&bmc->soc), cfg->hw_strap2, "hw-strap2", + object_property_set_int(OBJECT(&bmc->soc), amc->hw_strap2, "hw-strap2", &error_abort); - object_property_set_int(OBJECT(&bmc->soc), cfg->num_cs, "num-cs", + object_property_set_int(OBJECT(&bmc->soc), amc->num_cs, "num-cs", &error_abort); object_property_set_int(OBJECT(&bmc->soc), machine->smp.cpus, "num-cpus", &error_abort); @@ -XXX,XX +XXX,XX @@ static void aspeed_board_init(MachineState *machine, "max_ram", max_ram_size - ram_size); memory_region_add_subregion(&bmc->ram_container, ram_size, &bmc->max_ram); - aspeed_board_init_flashes(&bmc->soc.fmc, cfg->fmc_model, &error_abort); - aspeed_board_init_flashes(&bmc->soc.spi[0], cfg->spi_model, &error_abort); + aspeed_board_init_flashes(&bmc->soc.fmc, amc->fmc_model, &error_abort); + aspeed_board_init_flashes(&bmc->soc.spi[0], amc->spi_model, &error_abort); /* Install first FMC flash content as a boot rom. */ if (drive0) { @@ -XXX,XX +XXX,XX @@ static void aspeed_board_init(MachineState *machine, aspeed_board_binfo.loader_start = sc->memmap[ASPEED_SDRAM]; aspeed_board_binfo.nb_cpus = bmc->soc.num_cpus; - if (cfg->i2c_init) { - cfg->i2c_init(bmc); + if (amc->i2c_init) { + amc->i2c_init(bmc); } for (i = 0; i < ARRAY_SIZE(bmc->soc.sdhci.slots); i++) { @@ -XXX,XX +XXX,XX @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc) 0x60); } -static void aspeed_machine_init(MachineState *machine) -{ - AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(machine); - - aspeed_board_init(machine, amc->board); -} - static void aspeed_machine_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); - AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); - const AspeedBoardConfig *board = data; - mc->desc = board->desc; mc->init = aspeed_machine_init; mc->max_cpus = ASPEED_CPUS_NUM; mc->no_floppy = 1; mc->no_cdrom = 1; mc->no_parallel = 1; - if (board->ram) { - mc->default_ram_size = board->ram; - } - amc->board = board; } -static const TypeInfo aspeed_machine_type = { - .name = TYPE_ASPEED_MACHINE, - .parent = TYPE_MACHINE, - .instance_size = sizeof(AspeedMachine), - .class_size = sizeof(AspeedMachineClass), - .abstract = true, -}; - -static const AspeedBoardConfig aspeed_boards[] = { - { - .name = MACHINE_TYPE_NAME("palmetto-bmc"), - .desc = "OpenPOWER Palmetto BMC (ARM926EJ-S)", - .soc_name = "ast2400-a1", - .hw_strap1 = PALMETTO_BMC_HW_STRAP1, - .fmc_model = "n25q256a", - .spi_model = "mx25l25635e", - .num_cs = 1, - .i2c_init = palmetto_bmc_i2c_init, - .ram = 256 * MiB, - }, { - .name = MACHINE_TYPE_NAME("ast2500-evb"), - .desc = "Aspeed AST2500 EVB (ARM1176)", - .soc_name = "ast2500-a1", - .hw_strap1 = AST2500_EVB_HW_STRAP1, - .fmc_model = "w25q256", - .spi_model = "mx25l25635e", - .num_cs = 1, - .i2c_init = ast2500_evb_i2c_init, - .ram = 512 * MiB, - }, { - .name = MACHINE_TYPE_NAME("romulus-bmc"), - .desc = "OpenPOWER Romulus BMC (ARM1176)", - .soc_name = "ast2500-a1", - .hw_strap1 = ROMULUS_BMC_HW_STRAP1, - .fmc_model = "n25q256a", - .spi_model = "mx66l1g45g", - .num_cs = 2, - .i2c_init = romulus_bmc_i2c_init, - .ram = 512 * MiB, - }, { - .name = MACHINE_TYPE_NAME("swift-bmc"), - .desc = "OpenPOWER Swift BMC (ARM1176)", - .soc_name = "ast2500-a1", - .hw_strap1 = SWIFT_BMC_HW_STRAP1, - .fmc_model = "mx66l1g45g", - .spi_model = "mx66l1g45g", - .num_cs = 2, - .i2c_init = swift_bmc_i2c_init, - .ram = 512 * MiB, - }, { - .name = MACHINE_TYPE_NAME("witherspoon-bmc"), - .desc = "OpenPOWER Witherspoon BMC (ARM1176)", - .soc_name = "ast2500-a1", - .hw_strap1 = WITHERSPOON_BMC_HW_STRAP1, - .fmc_model = "mx25l25635e", - .spi_model = "mx66l1g45g", - .num_cs = 2, - .i2c_init = witherspoon_bmc_i2c_init, - .ram = 512 * MiB, - }, { - .name = MACHINE_TYPE_NAME("ast2600-evb"), - .desc = "Aspeed AST2600 EVB (Cortex A7)", - .soc_name = "ast2600-a0", - .hw_strap1 = AST2600_EVB_HW_STRAP1, - .hw_strap2 = AST2600_EVB_HW_STRAP2, - .fmc_model = "w25q512jv", - .spi_model = "mx66u51235f", - .num_cs = 1, - .i2c_init = ast2600_evb_i2c_init, - .ram = 1 * GiB, - }, -}; - -static void aspeed_machine_types(void) +static void aspeed_machine_palmetto_class_init(ObjectClass *oc, void *data) { - int i; + MachineClass *mc = MACHINE_CLASS(oc); + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); - type_register_static(&aspeed_machine_type); - for (i = 0; i < ARRAY_SIZE(aspeed_boards); ++i) { - TypeInfo ti = { - .name = aspeed_boards[i].name, - .parent = TYPE_ASPEED_MACHINE, - .class_init = aspeed_machine_class_init, - .class_data = (void *)&aspeed_boards[i], - }; - type_register(&ti); + mc->desc = "OpenPOWER Palmetto BMC (ARM926EJ-S)"; + amc->soc_name = "ast2400-a1"; + amc->hw_strap1 = PALMETTO_BMC_HW_STRAP1; + amc->fmc_model = "n25q256a"; + amc->spi_model = "mx25l25635e"; + amc->num_cs = 1; + amc->i2c_init = palmetto_bmc_i2c_init; + mc->default_ram_size = 256 * MiB; +}; + +static void aspeed_machine_ast2500_evb_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); + + mc->desc = "Aspeed AST2500 EVB (ARM1176)"; + amc->soc_name = "ast2500-a1"; + amc->hw_strap1 = AST2500_EVB_HW_STRAP1; + amc->fmc_model = "w25q256"; + amc->spi_model = "mx25l25635e"; + amc->num_cs = 1; + amc->i2c_init = ast2500_evb_i2c_init; + mc->default_ram_size = 512 * MiB; +}; + +static void aspeed_machine_romulus_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); + + mc->desc = "OpenPOWER Romulus BMC (ARM1176)"; + amc->soc_name = "ast2500-a1"; + amc->hw_strap1 = ROMULUS_BMC_HW_STRAP1; + amc->fmc_model = "n25q256a"; + amc->spi_model = "mx66l1g45g"; + amc->num_cs = 2; + amc->i2c_init = romulus_bmc_i2c_init; + mc->default_ram_size = 512 * MiB; +}; + +static void aspeed_machine_swift_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); + + mc->desc = "OpenPOWER Swift BMC (ARM1176)"; + amc->soc_name = "ast2500-a1"; + amc->hw_strap1 = SWIFT_BMC_HW_STRAP1; + amc->fmc_model = "mx66l1g45g"; + amc->spi_model = "mx66l1g45g"; + amc->num_cs = 2; + amc->i2c_init = swift_bmc_i2c_init; + mc->default_ram_size = 512 * MiB; +}; + +static void aspeed_machine_witherspoon_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); + + mc->desc = "OpenPOWER Witherspoon BMC (ARM1176)"; + amc->soc_name = "ast2500-a1"; + amc->hw_strap1 = WITHERSPOON_BMC_HW_STRAP1; + amc->fmc_model = "mx25l25635e"; + amc->spi_model = "mx66l1g45g"; + amc->num_cs = 2; + amc->i2c_init = witherspoon_bmc_i2c_init; + mc->default_ram_size = 512 * MiB; +}; + +static void aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); + + mc->desc = "Aspeed AST2600 EVB (Cortex A7)"; + amc->soc_name = "ast2600-a0"; + amc->hw_strap1 = AST2600_EVB_HW_STRAP1; + amc->hw_strap2 = AST2600_EVB_HW_STRAP2; + amc->fmc_model = "w25q512jv"; + amc->spi_model = "mx66u51235f"; + amc->num_cs = 1; + amc->i2c_init = ast2600_evb_i2c_init; + mc->default_ram_size = 1 * GiB; +}; + +static const TypeInfo aspeed_machine_types[] = { + { + .name = MACHINE_TYPE_NAME("palmetto-bmc"), + .parent = TYPE_ASPEED_MACHINE, + .class_init = aspeed_machine_palmetto_class_init, + }, { + .name = MACHINE_TYPE_NAME("ast2500-evb"), + .parent = TYPE_ASPEED_MACHINE, + .class_init = aspeed_machine_ast2500_evb_class_init, + }, { + .name = MACHINE_TYPE_NAME("romulus-bmc"), + .parent = TYPE_ASPEED_MACHINE, + .class_init = aspeed_machine_romulus_class_init, + }, { + .name = MACHINE_TYPE_NAME("swift-bmc"), + .parent = TYPE_ASPEED_MACHINE, + .class_init = aspeed_machine_swift_class_init, + }, { + .name = MACHINE_TYPE_NAME("witherspoon-bmc"), + .parent = TYPE_ASPEED_MACHINE, + .class_init = aspeed_machine_witherspoon_class_init, + }, { + .name = MACHINE_TYPE_NAME("ast2600-evb"), + .parent = TYPE_ASPEED_MACHINE, + .class_init = aspeed_machine_ast2600_evb_class_init, + }, { + .name = TYPE_ASPEED_MACHINE, + .parent = TYPE_MACHINE, + .instance_size = sizeof(AspeedMachine), + .class_size = sizeof(AspeedMachineClass), + .class_init = aspeed_machine_class_init, + .abstract = true, } -} +}; -type_init(aspeed_machine_types) +DEFINE_TYPES(aspeed_machine_types) -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> The Tacoma BMC board is replacement board for the BMC of the OpenPOWER Witherspoon system. It uses a AST2600 SoC instead of a AST2500 and the I2C layout is the same as it controls the same main board. Used for HW bringup. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-15-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/aspeed.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -XXX,XX +XXX,XX @@ struct AspeedBoardState { #define AST2600_EVB_HW_STRAP1 0x000000C0 #define AST2600_EVB_HW_STRAP2 0x00000003 +/* Tacoma hardware value */ +#define TACOMA_BMC_HW_STRAP1 0x00000000 +#define TACOMA_BMC_HW_STRAP2 0x00000000 + /* * The max ram region is for firmwares that scan the address space * with load/store to guess how much RAM the SoC has. @@ -XXX,XX +XXX,XX @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc) AspeedSoCState *soc = &bmc->soc; uint8_t *eeprom_buf = g_malloc0(8 * 1024); + /* Bus 3: TODO bmp280@77 */ + /* Bus 3: TODO max31785@52 */ + /* Bus 3: TODO dps310@76 */ i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 3), TYPE_PCA9552, 0x60); @@ -XXX,XX +XXX,XX @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc) eeprom_buf); i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), TYPE_PCA9552, 0x60); + /* Bus 11: TODO ucd90160@64 */ } static void aspeed_machine_class_init(ObjectClass *oc, void *data) @@ -XXX,XX +XXX,XX @@ static void aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data) mc->default_ram_size = 1 * GiB; }; +static void aspeed_machine_tacoma_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc); + + mc->desc = "Aspeed AST2600 EVB (Cortex A7)"; + amc->soc_name = "ast2600-a0"; + amc->hw_strap1 = TACOMA_BMC_HW_STRAP1; + amc->hw_strap2 = TACOMA_BMC_HW_STRAP2; + amc->fmc_model = "mx66l1g45g"; + amc->spi_model = "mx66l1g45g"; + amc->num_cs = 2; + amc->i2c_init = witherspoon_bmc_i2c_init; /* Same board layout */ + mc->default_ram_size = 1 * GiB; +}; + static const TypeInfo aspeed_machine_types[] = { { .name = MACHINE_TYPE_NAME("palmetto-bmc"), @@ -XXX,XX +XXX,XX @@ static const TypeInfo aspeed_machine_types[] = { .name = MACHINE_TYPE_NAME("ast2600-evb"), .parent = TYPE_ASPEED_MACHINE, .class_init = aspeed_machine_ast2600_evb_class_init, + }, { + .name = MACHINE_TYPE_NAME("tacoma-bmc"), + .parent = TYPE_ASPEED_MACHINE, + .class_init = aspeed_machine_tacoma_class_init, }, { .name = TYPE_ASPEED_MACHINE, .parent = TYPE_MACHINE, -- 2.20.1 From: PanNengyuan <pannengyuan@huawei.com> Address Sanitizer shows memory leak in hw/gpio/aspeed_gpio.c:875 Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: PanNengyuan <pannengyuan@huawei.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-16-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/gpio/aspeed_gpio.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c index XXXXXXX..XXXXXXX 100644 --- a/hw/gpio/aspeed_gpio.c +++ b/hw/gpio/aspeed_gpio.c @@ -XXX,XX +XXX,XX @@ static void aspeed_gpio_init(Object *obj) pin_idx % GPIOS_PER_GROUP); object_property_add(obj, name, "bool", aspeed_gpio_get_pin, aspeed_gpio_set_pin, NULL, NULL, NULL); + g_free(name); } } -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> The Aspeed Watchdog and Timer models have a link pointing to the SCU controller model of the machine. Change the "scu" property definition so that it explicitly sets the pointer. The property isn't optional : not being able to set the link is a bug and QEMU should rather abort than exit in this case. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Greg Kurz <groug@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-17-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/aspeed_ast2600.c | 8 ++++---- hw/arm/aspeed_soc.c | 8 ++++---- hw/timer/aspeed_timer.c | 17 +++++++++-------- hw/watchdog/wdt_aspeed.c | 17 ++++++++--------- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_ast2600.c +++ b/hw/arm/aspeed_ast2600.c @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_init(Object *obj) snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname); sysbus_init_child_obj(obj, "timerctrl", OBJECT(&s->timerctrl), sizeof(s->timerctrl), typename); - object_property_add_const_link(OBJECT(&s->timerctrl), "scu", - OBJECT(&s->scu), &error_abort); snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname); sysbus_init_child_obj(obj, "i2c", OBJECT(&s->i2c), sizeof(s->i2c), @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_init(Object *obj) snprintf(typename, sizeof(typename), "aspeed.wdt-%s", socname); sysbus_init_child_obj(obj, "wdt[*]", OBJECT(&s->wdt[i]), sizeof(s->wdt[i]), typename); - object_property_add_const_link(OBJECT(&s->wdt[i]), "scu", - OBJECT(&s->scu), &error_abort); } for (i = 0; i < sc->macs_num; i++) { @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) aspeed_soc_get_irq(s, ASPEED_RTC)); /* Timer */ + object_property_set_link(OBJECT(&s->timerctrl), + OBJECT(&s->scu), "scu", &error_abort); object_property_set_bool(OBJECT(&s->timerctrl), true, "realized", &err); if (err) { error_propagate(errp, err); @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) for (i = 0; i < sc->wdts_num; i++) { AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(&s->wdt[i]); + object_property_set_link(OBJECT(&s->wdt[i]), + OBJECT(&s->scu), "scu", &error_abort); object_property_set_bool(OBJECT(&s->wdt[i]), true, "realized", &err); if (err) { error_propagate(errp, err); diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_soc.c +++ b/hw/arm/aspeed_soc.c @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_init(Object *obj) snprintf(typename, sizeof(typename), "aspeed.timer-%s", socname); sysbus_init_child_obj(obj, "timerctrl", OBJECT(&s->timerctrl), sizeof(s->timerctrl), typename); - object_property_add_const_link(OBJECT(&s->timerctrl), "scu", - OBJECT(&s->scu), &error_abort); snprintf(typename, sizeof(typename), "aspeed.i2c-%s", socname); sysbus_init_child_obj(obj, "i2c", OBJECT(&s->i2c), sizeof(s->i2c), @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_init(Object *obj) snprintf(typename, sizeof(typename), "aspeed.wdt-%s", socname); sysbus_init_child_obj(obj, "wdt[*]", OBJECT(&s->wdt[i]), sizeof(s->wdt[i]), typename); - object_property_add_const_link(OBJECT(&s->wdt[i]), "scu", - OBJECT(&s->scu), &error_abort); } for (i = 0; i < sc->macs_num; i++) { @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) aspeed_soc_get_irq(s, ASPEED_RTC)); /* Timer */ + object_property_set_link(OBJECT(&s->timerctrl), + OBJECT(&s->scu), "scu", &error_abort); object_property_set_bool(OBJECT(&s->timerctrl), true, "realized", &err); if (err) { error_propagate(errp, err); @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp) for (i = 0; i < sc->wdts_num; i++) { AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(&s->wdt[i]); + object_property_set_link(OBJECT(&s->wdt[i]), + OBJECT(&s->scu), "scu", &error_abort); object_property_set_bool(OBJECT(&s->wdt[i]), true, "realized", &err); if (err) { error_propagate(errp, err); diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/aspeed_timer.c +++ b/hw/timer/aspeed_timer.c @@ -XXX,XX +XXX,XX @@ #include "qemu/timer.h" #include "qemu/log.h" #include "qemu/module.h" +#include "hw/qdev-properties.h" #include "trace.h" #define TIMER_NR_REGS 4 @@ -XXX,XX +XXX,XX @@ static void aspeed_timer_realize(DeviceState *dev, Error **errp) int i; SysBusDevice *sbd = SYS_BUS_DEVICE(dev); AspeedTimerCtrlState *s = ASPEED_TIMER(dev); - Object *obj; - Error *err = NULL; - obj = object_property_get_link(OBJECT(dev), "scu", &err); - if (!obj) { - error_propagate_prepend(errp, err, "required link 'scu' not found: "); - return; - } - s->scu = ASPEED_SCU(obj); + assert(s->scu); for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) { aspeed_init_one_timer(s, i); @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_aspeed_timer_state = { } }; +static Property aspeed_timer_properties[] = { + DEFINE_PROP_LINK("scu", AspeedTimerCtrlState, scu, TYPE_ASPEED_SCU, + AspeedSCUState *), + DEFINE_PROP_END_OF_LIST(), +}; + static void timer_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static void timer_class_init(ObjectClass *klass, void *data) dc->reset = aspeed_timer_reset; dc->desc = "ASPEED Timer"; dc->vmsd = &vmstate_aspeed_timer_state; + dc->props = aspeed_timer_properties; } static const TypeInfo aspeed_timer_info = { diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c index XXXXXXX..XXXXXXX 100644 --- a/hw/watchdog/wdt_aspeed.c +++ b/hw/watchdog/wdt_aspeed.c @@ -XXX,XX +XXX,XX @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp) { SysBusDevice *sbd = SYS_BUS_DEVICE(dev); AspeedWDTState *s = ASPEED_WDT(dev); - Error *err = NULL; - Object *obj; - obj = object_property_get_link(OBJECT(dev), "scu", &err); - if (!obj) { - error_propagate(errp, err); - error_prepend(errp, "required link 'scu' not found: "); - return; - } - s->scu = ASPEED_SCU(obj); + assert(s->scu); s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, aspeed_wdt_timer_expired, dev); @@ -XXX,XX +XXX,XX @@ static void aspeed_wdt_realize(DeviceState *dev, Error **errp) sysbus_init_mmio(sbd, &s->iomem); } +static Property aspeed_wdt_properties[] = { + DEFINE_PROP_LINK("scu", AspeedWDTState, scu, TYPE_ASPEED_SCU, + AspeedSCUState *), + DEFINE_PROP_END_OF_LIST(), +}; + static void aspeed_wdt_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static void aspeed_wdt_class_init(ObjectClass *klass, void *data) dc->reset = aspeed_wdt_reset; set_bit(DEVICE_CATEGORY_MISC, dc->categories); dc->vmsd = &vmstate_aspeed_wdt; + dc->props = aspeed_wdt_properties; } static const TypeInfo aspeed_wdt_info = { -- 2.20.1 From: Cédric Le Goater <clg@kaod.org> The Aspeed MII model has a link pointing to its associated FTGMAC100 NIC in the machine. Change the "nic" property definition so that it explicitly sets the pointer. The property isn't optional : not being able to set the link is a bug and QEMU should rather abort than exit in this case. Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Greg Kurz <groug@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20191119141211.25716-18-clg@kaod.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/aspeed_ast2600.c | 5 ++--- hw/net/ftgmac100.c | 19 +++++++++---------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/aspeed_ast2600.c +++ b/hw/arm/aspeed_ast2600.c @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_init(Object *obj) sysbus_init_child_obj(obj, "mii[*]", &s->mii[i], sizeof(s->mii[i]), TYPE_ASPEED_MII); - object_property_add_const_link(OBJECT(&s->mii[i]), "nic", - OBJECT(&s->ftgmac100[i]), - &error_abort); } sysbus_init_child_obj(obj, "xdma", OBJECT(&s->xdma), sizeof(s->xdma), @@ -XXX,XX +XXX,XX @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0, aspeed_soc_get_irq(s, ASPEED_ETH1 + i)); + object_property_set_link(OBJECT(&s->mii[i]), OBJECT(&s->ftgmac100[i]), + "nic", &error_abort); object_property_set_bool(OBJECT(&s->mii[i]), true, "realized", &err); if (err) { diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/ftgmac100.c +++ b/hw/net/ftgmac100.c @@ -XXX,XX +XXX,XX @@ static void aspeed_mii_realize(DeviceState *dev, Error **errp) { AspeedMiiState *s = ASPEED_MII(dev); SysBusDevice *sbd = SYS_BUS_DEVICE(dev); - Object *obj; - Error *local_err = NULL; - obj = object_property_get_link(OBJECT(dev), "nic", &local_err); - if (!obj) { - error_propagate(errp, local_err); - error_prepend(errp, "required link 'nic' not found: "); - return; - } - - s->nic = FTGMAC100(obj); + assert(s->nic); memory_region_init_io(&s->iomem, OBJECT(dev), &aspeed_mii_ops, s, TYPE_ASPEED_MII, 0x8); @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_aspeed_mii = { VMSTATE_END_OF_LIST() } }; + +static Property aspeed_mii_properties[] = { + DEFINE_PROP_LINK("nic", AspeedMiiState, nic, TYPE_FTGMAC100, + FTGMAC100State *), + DEFINE_PROP_END_OF_LIST(), +}; + static void aspeed_mii_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static void aspeed_mii_class_init(ObjectClass *klass, void *data) dc->reset = aspeed_mii_reset; dc->realize = aspeed_mii_realize; dc->desc = "Aspeed MII controller"; + dc->props = aspeed_mii_properties; } static const TypeInfo aspeed_mii_info = { -- 2.20.1 From: Marc Zyngier <maz@kernel.org> HCR_EL2.TID2 mandates that access from EL1 to CTR_EL0, CCSIDR_EL1, CCSIDR2_EL1, CLIDR_EL1, CSSELR_EL1 are trapped to EL2, and QEMU completely ignores it, making it impossible for hypervisors to virtualize the cache hierarchy. Do the right thing by trapping to EL2 if HCR_EL2.TID2 is set. Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191201122018.25808-2-maz@kernel.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) raw_write(env, ri, value); } +static CPAccessResult access_aa64_tid2(CPUARMState *env, + const ARMCPRegInfo *ri, + bool isread) +{ + if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TID2)) { + return CP_ACCESS_TRAP_EL2; + } + + return CP_ACCESS_OK; +} + static uint64_t ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri) { ARMCPU *cpu = env_archcpu(env); @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v7_cp_reginfo[] = { .writefn = pmintenclr_write }, { .name = "CCSIDR", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0, - .access = PL1_R, .readfn = ccsidr_read, .type = ARM_CP_NO_RAW }, + .access = PL1_R, + .accessfn = access_aa64_tid2, + .readfn = ccsidr_read, .type = ARM_CP_NO_RAW }, { .name = "CSSELR", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 2, .opc2 = 0, - .access = PL1_RW, .writefn = csselr_write, .resetvalue = 0, + .access = PL1_RW, + .accessfn = access_aa64_tid2, + .writefn = csselr_write, .resetvalue = 0, .bank_fieldoffsets = { offsetof(CPUARMState, cp15.csselr_s), offsetof(CPUARMState, cp15.csselr_ns) } }, /* Auxiliary ID register: this actually has an IMPDEF value but for now @@ -XXX,XX +XXX,XX @@ static CPAccessResult ctr_el0_access(CPUARMState *env, const ARMCPRegInfo *ri, if (arm_current_el(env) == 0 && !(env->cp15.sctlr_el[1] & SCTLR_UCT)) { return CP_ACCESS_TRAP; } + + if (arm_current_el(env) < 2 && arm_hcr_el2_eff(env) & HCR_TID2) { + return CP_ACCESS_TRAP_EL2; + } + return CP_ACCESS_OK; } @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) ARMCPRegInfo clidr = { .name = "CLIDR", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 1, - .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->clidr + .access = PL1_R, .type = ARM_CP_CONST, + .accessfn = access_aa64_tid2, + .resetvalue = cpu->clidr }; define_one_arm_cp_reg(cpu, &clidr); define_arm_cp_regs(cpu, v7_cp_reginfo); @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) /* These are common to v8 and pre-v8 */ { .name = "CTR", .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 1, - .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->ctr }, + .access = PL1_R, .accessfn = ctr_el0_access, + .type = ARM_CP_CONST, .resetvalue = cpu->ctr }, { .name = "CTR_EL0", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 0, .crm = 0, .access = PL0_R, .accessfn = ctr_el0_access, -- 2.20.1 From: Marc Zyngier <maz@kernel.org> HCR_EL2.TID1 mandates that access from EL1 to REVIDR_EL1, AIDR_EL1 (and their 32bit equivalents) as well as TCMTR, TLBTR are trapped to EL2. QEMU ignores it, making it harder for a hypervisor to virtualize the HW (though to be fair, no known hypervisor actually cares). Do the right thing by trapping to EL2 if HCR_EL2.TID1 is set. Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191201122018.25808-3-maz@kernel.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static uint64_t isr_read(CPUARMState *env, const ARMCPRegInfo *ri) return ret; } +static CPAccessResult access_aa64_tid1(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) +{ + if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TID1)) { + return CP_ACCESS_TRAP_EL2; + } + + return CP_ACCESS_OK; +} + +static CPAccessResult access_aa32_tid1(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) +{ + if (arm_feature(env, ARM_FEATURE_V8)) { + return access_aa64_tid1(env, ri, isread); + } + + return CP_ACCESS_OK; +} + static const ARMCPRegInfo v7_cp_reginfo[] = { /* the old v6 WFI, UNPREDICTABLE in v7 but we choose to NOP */ { .name = "NOP", .cp = 15, .crn = 7, .crm = 0, .opc1 = 0, .opc2 = 4, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v7_cp_reginfo[] = { */ { .name = "AIDR", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .opc1 = 1, .crn = 0, .crm = 0, .opc2 = 7, - .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 }, + .access = PL1_R, .type = ARM_CP_CONST, + .accessfn = access_aa64_tid1, + .resetvalue = 0 }, /* Auxiliary fault status registers: these also are IMPDEF, and we * choose to RAZ/WI for all cores. */ @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) .access = PL1_R, .resetvalue = cpu->midr }, { .name = "REVIDR_EL1", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 0, .opc2 = 6, - .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->revidr }, + .access = PL1_R, + .accessfn = access_aa64_tid1, + .type = ARM_CP_CONST, .resetvalue = cpu->revidr }, REGINFO_SENTINEL }; ARMCPRegInfo id_cp_reginfo[] = { @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) /* TCMTR and TLBTR exist in v8 but have no 64-bit versions */ { .name = "TCMTR", .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 2, - .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 }, + .access = PL1_R, + .accessfn = access_aa32_tid1, + .type = ARM_CP_CONST, .resetvalue = 0 }, REGINFO_SENTINEL }; /* TLBTR is specific to VMSA */ ARMCPRegInfo id_tlbtr_reginfo = { .name = "TLBTR", .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 3, - .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0, + .access = PL1_R, + .accessfn = access_aa32_tid1, + .type = ARM_CP_CONST, .resetvalue = 0, }; /* MPUIR is specific to PMSA V6+ */ ARMCPRegInfo id_mpuir_reginfo = { -- 2.20.1 From: Marc Zyngier <maz@kernel.org> HCR_EL2.TID3 requires that AArch32 reads of MVFR[012] are trapped to EL2, and HCR_EL2.TID0 does the same for reads of FPSID. In order to handle this, introduce a new TCG helper function that checks for these control bits before executing the VMRC instruction. Tested with a hacked-up version of KVM/arm64 that sets the control bits for 32bit guests. Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191201122018.25808-4-maz@kernel.org [PMM: move helper declaration to helper.h; make it TCG_CALL_NO_WG] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.h | 2 ++ target/arm/translate-vfp.inc.c | 20 ++++++++++++++++---- target/arm/vfp_helper.c | 29 +++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_FLAGS_2(rintd, TCG_CALL_NO_RWG, f64, f64, ptr) DEF_HELPER_FLAGS_2(vjcvt, TCG_CALL_NO_RWG, i32, f64, env) DEF_HELPER_FLAGS_2(fjcvtzs, TCG_CALL_NO_RWG, i64, f64, ptr) +DEF_HELPER_FLAGS_3(check_hcr_el2_trap, TCG_CALL_NO_WG, void, env, i32, i32) + /* neon_helper.c */ DEF_HELPER_FLAGS_3(neon_qadd_u8, TCG_CALL_NO_RWG, i32, env, i32, i32) DEF_HELPER_FLAGS_3(neon_qadd_s8, TCG_CALL_NO_RWG, i32, env, i32, i32) diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-vfp.inc.c +++ b/target/arm/translate-vfp.inc.c @@ -XXX,XX +XXX,XX @@ static bool trans_VMSR_VMRS(DisasContext *s, arg_VMSR_VMRS *a) if (a->l) { /* VMRS, move VFP special register to gp register */ switch (a->reg) { - case ARM_VFP_FPSID: - case ARM_VFP_FPEXC: - case ARM_VFP_FPINST: - case ARM_VFP_FPINST2: case ARM_VFP_MVFR0: case ARM_VFP_MVFR1: case ARM_VFP_MVFR2: + case ARM_VFP_FPSID: + if (s->current_el == 1) { + TCGv_i32 tcg_reg, tcg_rt; + + gen_set_condexec(s); + gen_set_pc_im(s, s->pc_curr); + tcg_reg = tcg_const_i32(a->reg); + tcg_rt = tcg_const_i32(a->rt); + gen_helper_check_hcr_el2_trap(cpu_env, tcg_rt, tcg_reg); + tcg_temp_free_i32(tcg_reg); + tcg_temp_free_i32(tcg_rt); + } + /* fall through */ + case ARM_VFP_FPEXC: + case ARM_VFP_FPINST: + case ARM_VFP_FPINST2: tmp = load_cpu_field(vfp.xregs[a->reg]); break; case ARM_VFP_FPSCR: 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 @@ float64 HELPER(frint64_d)(float64 f, void *fpst) return frint_d(f, fpst, 64); } +void HELPER(check_hcr_el2_trap)(CPUARMState *env, uint32_t rt, uint32_t reg) +{ + uint32_t syndrome; + + switch (reg) { + case ARM_VFP_MVFR0: + case ARM_VFP_MVFR1: + case ARM_VFP_MVFR2: + if (!(arm_hcr_el2_eff(env) & HCR_TID3)) { + return; + } + break; + case ARM_VFP_FPSID: + if (!(arm_hcr_el2_eff(env) & HCR_TID0)) { + return; + } + break; + default: + g_assert_not_reached(); + } + + syndrome = ((EC_FPIDTRAP << ARM_EL_EC_SHIFT) + | ARM_EL_IL + | (1 << 24) | (0xe << 20) | (7 << 14) + | (reg << 10) | (rt << 5) | 1); + + raise_exception(env, EXCP_HYP_TRAP, syndrome, 2); +} + #endif -- 2.20.1 From: Marc Zyngier <maz@kernel.org> HSTR_EL2 offers a way to trap ranges of CP15 system register accesses to EL2, and it looks like this register is completely ignored by QEMU. To avoid adding extra .accessfn filters all over the place (which would have a direct performance impact), let's add a new TB flag that gets set whenever HSTR_EL2 is non-zero and that QEMU translates a context where this trap has a chance to apply, and only generate the extra access check if the hypervisor is actively using this feature. Tested with a hand-crafted KVM guest accessing CBAR. Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191201122018.25808-5-maz@kernel.org [PMM: use is_a64(); fix comment syntax] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 2 ++ target/arm/translate.h | 2 ++ target/arm/helper.c | 6 ++++++ target/arm/op_helper.c | 22 ++++++++++++++++++++++ target/arm/translate.c | 3 ++- 5 files changed, 34 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 @@ FIELD(TBFLAG_A32, NS, 6, 1) FIELD(TBFLAG_A32, VFPEN, 7, 1) /* Partially cached, minus FPEXC. */ FIELD(TBFLAG_A32, CONDEXEC, 8, 8) /* Not cached. */ FIELD(TBFLAG_A32, SCTLR_B, 16, 1) +FIELD(TBFLAG_A32, HSTR_ACTIVE, 17, 1) + /* For M profile only, set if FPCCR.LSPACT is set */ FIELD(TBFLAG_A32, LSPACT, 18, 1) /* Not cached. */ /* For M profile only, set if we must create a new FP context */ diff --git a/target/arm/translate.h b/target/arm/translate.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -XXX,XX +XXX,XX @@ typedef struct DisasContext { bool pauth_active; /* True with v8.5-BTI and SCTLR_ELx.BT* set. */ bool bt; + /* True if any CP15 access is trapped by HSTR_EL2 */ + bool hstr_active; /* * >= 0, a copy of PSTATE.BTYPE, which will be 0 without v8.5-BTI. * < 0, set by the current instruction. 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_a32(CPUARMState *env, int fp_el, if (arm_el_is_aa64(env, 1)) { flags = FIELD_DP32(flags, TBFLAG_A32, VFPEN, 1); } + + if (arm_current_el(env) < 2 && env->cp15.hstr_el2 && + (arm_hcr_el2_eff(env) & (HCR_E2H | HCR_TGE)) != (HCR_E2H | HCR_TGE)) { + flags = FIELD_DP32(flags, TBFLAG_A32, HSTR_ACTIVE, 1); + } + return rebuild_hflags_common_32(env, fp_el, mmu_idx, flags); } diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -XXX,XX +XXX,XX @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome, raise_exception(env, EXCP_UDEF, syndrome, exception_target_el(env)); } + /* + * Check for an EL2 trap due to HSTR_EL2. We expect EL0 accesses + * to sysregs non accessible at EL0 to have UNDEF-ed already. + */ + if (!is_a64(env) && arm_current_el(env) < 2 && ri->cp == 15 && + (arm_hcr_el2_eff(env) & (HCR_E2H | HCR_TGE)) != (HCR_E2H | HCR_TGE)) { + uint32_t mask = 1 << ri->crn; + + if (ri->type & ARM_CP_64BIT) { + mask = 1 << ri->crm; + } + + /* T4 and T14 are RES0 */ + mask &= ~((1 << 4) | (1 << 14)); + + if (env->cp15.hstr_el2 & mask) { + target_el = 2; + goto exept; + } + } + if (!ri->accessfn) { return; } @@ -XXX,XX +XXX,XX @@ void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t syndrome, g_assert_not_reached(); } +exept: raise_exception(env, EXCP_UDEF, syndrome, target_el); } diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static int disas_coproc_insn(DisasContext *s, uint32_t insn) return 1; } - if (ri->accessfn || + if (s->hstr_active || ri->accessfn || (arm_dc_feature(s, ARM_FEATURE_XSCALE) && cpnum < 14)) { /* Emit code to perform further access permissions checks at * runtime; this may result in an exception. @@ -XXX,XX +XXX,XX @@ static void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) !arm_el_is_aa64(env, 3); dc->thumb = FIELD_EX32(tb_flags, TBFLAG_A32, THUMB); dc->sctlr_b = FIELD_EX32(tb_flags, TBFLAG_A32, SCTLR_B); + dc->hstr_active = FIELD_EX32(tb_flags, TBFLAG_A32, HSTR_ACTIVE); dc->be_data = FIELD_EX32(tb_flags, TBFLAG_ANY, BE_DATA) ? MO_BE : MO_LE; condexec = FIELD_EX32(tb_flags, TBFLAG_A32, CONDEXEC); dc->condexec_mask = (condexec & 0xf) << 1; -- 2.20.1 From: Marc Zyngier <maz@kernel.org> QEMU lacks the minimum Jazelle implementation that is required by the architecture (everything is RAZ or RAZ/WI). Add it together with the HCR_EL2.TID0 trapping that goes with it. Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191201122018.25808-6-maz@kernel.org [PMM: moved ARMCPRegInfo array to file scope, marked it 'static global', moved new condition down in register_cp_regs_for_features() to go with other feature things rather than up with the v6/v7/v8 stuff] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static CPAccessResult access_aa32_tid3(CPUARMState *env, const ARMCPRegInfo *ri, return CP_ACCESS_OK; } +static CPAccessResult access_jazelle(CPUARMState *env, const ARMCPRegInfo *ri, + bool isread) +{ + if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TID0)) { + return CP_ACCESS_TRAP_EL2; + } + + return CP_ACCESS_OK; +} + +static const ARMCPRegInfo jazelle_regs[] = { + { .name = "JIDR", + .cp = 14, .crn = 0, .crm = 0, .opc1 = 7, .opc2 = 0, + .access = PL1_R, .accessfn = access_jazelle, + .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "JOSCR", + .cp = 14, .crn = 1, .crm = 0, .opc1 = 7, .opc2 = 0, + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + { .name = "JMCR", + .cp = 14, .crn = 2, .crm = 0, .opc1 = 7, .opc2 = 0, + .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, + REGINFO_SENTINEL +}; + void register_cp_regs_for_features(ARMCPU *cpu) { /* Register all the coprocessor registers based on feature bits */ @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) if (arm_feature(env, ARM_FEATURE_LPAE)) { define_arm_cp_regs(cpu, lpae_cp_reginfo); } + if (cpu_isar_feature(jazelle, cpu)) { + define_arm_cp_regs(cpu, jazelle_regs); + } /* Slightly awkwardly, the OMAP and StrongARM cores need all of * cp15 crn=0 to be writes-ignored, whereas for other cores they should * be read-only (ie write causes UNDEF exception). -- 2.20.1 From: Niek Linnenbank <nieklinnenbank@gmail.com> This change ensures that the FPU can be accessed in Non-Secure mode when the CPU core is reset using the arm_set_cpu_on() function call. The NSACR.{CP11,CP10} bits define the exception level required to access the FPU in Non-Secure mode. Without these bits set, the CPU will give an undefined exception trap on the first FPU access for the secondary cores under Linux. This is necessary because in this power-control codepath QEMU is effectively emulating a bit of EL3 firmware, and has to set the CPU up as the EL3 firmware would. Fixes: fc1120a7f5 Cc: qemu-stable@nongnu.org Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com> [PMM: added clarifying para to commit message] Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/arm-powerctl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/arm-powerctl.c +++ b/target/arm/arm-powerctl.c @@ -XXX,XX +XXX,XX @@ static void arm_set_cpu_on_async_work(CPUState *target_cpu_state, /* Processor is not in secure mode */ target_cpu->env.cp15.scr_el3 |= SCR_NS; + /* Set NSACR.{CP11,CP10} so NS can access the FPU */ + target_cpu->env.cp15.nsacr |= 3 << 10; + /* * If QEMU is providing the equivalent of EL3 firmware, then we need * to make sure a CPU targeting EL2 comes out of reset with a -- 2.20.1 From: Beata Michalska <beata.michalska@linaro.org> Add probe_read alongside the write probing equivalent. Signed-off-by: Beata Michalska <beata.michalska@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191121000843.24844-2-beata.michalska@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/exec/exec-all.h | 6 ++++++ 1 file changed, 6 insertions(+) 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 @@ static inline void *probe_write(CPUArchState *env, target_ulong addr, int size, return probe_access(env, addr, size, MMU_DATA_STORE, mmu_idx, retaddr); } +static inline void *probe_read(CPUArchState *env, target_ulong addr, int size, + int mmu_idx, uintptr_t retaddr) +{ + return probe_access(env, addr, size, MMU_DATA_LOAD, mmu_idx, retaddr); +} + #define CODE_GEN_ALIGN 16 /* must be >= of the size of a icache line */ /* Estimated block size for TB allocation. */ -- 2.20.1 From: Beata Michalska <beata.michalska@linaro.org> Add an option to trigger memory writeback to sync given memory region with the corresponding backing store, case one is available. This extends the support for persistent memory, allowing syncing on-demand. Signed-off-by: Beata Michalska <beata.michalska@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191121000843.24844-3-beata.michalska@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/exec/memory.h | 6 ++++++ include/exec/ram_addr.h | 8 ++++++++ include/qemu/cutils.h | 1 + exec.c | 36 ++++++++++++++++++++++++++++++++++++ memory.c | 12 ++++++++++++ util/cutils.c | 38 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 101 insertions(+) diff --git a/include/exec/memory.h b/include/exec/memory.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -XXX,XX +XXX,XX @@ void *memory_region_get_ram_ptr(MemoryRegion *mr); */ void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize, Error **errp); +/** + * memory_region_do_writeback: Trigger writeback for selected address range + * [addr, addr + size] + * + */ +void memory_region_do_writeback(MemoryRegion *mr, hwaddr addr, hwaddr size); /** * memory_region_set_log: Turn dirty logging on or off for a region. diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/ram_addr.h +++ b/include/exec/ram_addr.h @@ -XXX,XX +XXX,XX @@ void qemu_ram_free(RAMBlock *block); int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp); +void qemu_ram_writeback(RAMBlock *block, ram_addr_t start, ram_addr_t length); + +/* Clear whole block of mem */ +static inline void qemu_ram_block_writeback(RAMBlock *block) +{ + qemu_ram_writeback(block, 0, block->used_length); +} + #define DIRTY_CLIENTS_ALL ((1 << DIRTY_MEMORY_NUM) - 1) #define DIRTY_CLIENTS_NOCODE (DIRTY_CLIENTS_ALL & ~(1 << DIRTY_MEMORY_CODE)) diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h index XXXXXXX..XXXXXXX 100644 --- a/include/qemu/cutils.h +++ b/include/qemu/cutils.h @@ -XXX,XX +XXX,XX @@ const char *qemu_strchrnul(const char *s, int c); #endif time_t mktimegm(struct tm *tm); int qemu_fdatasync(int fd); +int qemu_msync(void *addr, size_t length, int fd); int fcntl_setfl(int fd, int flag); int qemu_parse_fd(const char *param); int qemu_strtoi(const char *nptr, const char **endptr, int base, diff --git a/exec.c b/exec.c index XXXXXXX..XXXXXXX 100644 --- a/exec.c +++ b/exec.c @@ -XXX,XX +XXX,XX @@ #include "exec/ram_addr.h" #include "exec/log.h" +#include "qemu/pmem.h" + #include "migration/vmstate.h" #include "qemu/range.h" @@ -XXX,XX +XXX,XX @@ int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp) return 0; } +/* + * Trigger sync on the given ram block for range [start, start + length] + * with the backing store if one is available. + * Otherwise no-op. + * @Note: this is supposed to be a synchronous op. + */ +void qemu_ram_writeback(RAMBlock *block, ram_addr_t start, ram_addr_t length) +{ + void *addr = ramblock_ptr(block, start); + + /* The requested range should fit in within the block range */ + g_assert((start + length) <= block->used_length); + +#ifdef CONFIG_LIBPMEM + /* The lack of support for pmem should not block the sync */ + if (ramblock_is_pmem(block)) { + pmem_persist(addr, length); + return; + } +#endif + if (block->fd >= 0) { + /** + * Case there is no support for PMEM or the memory has not been + * specified as persistent (or is not one) - use the msync. + * Less optimal but still achieves the same goal + */ + if (qemu_msync(addr, length, block->fd)) { + warn_report("%s: failed to sync memory range: start: " + RAM_ADDR_FMT " length: " RAM_ADDR_FMT, + __func__, start, length); + } + } +} + /* Called with ram_list.mutex held */ static void dirty_memory_extend(ram_addr_t old_ram_size, ram_addr_t new_ram_size) diff --git a/memory.c b/memory.c index XXXXXXX..XXXXXXX 100644 --- a/memory.c +++ b/memory.c @@ -XXX,XX +XXX,XX @@ void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize, Error **errp qemu_ram_resize(mr->ram_block, newsize, errp); } + +void memory_region_do_writeback(MemoryRegion *mr, hwaddr addr, hwaddr size) +{ + /* + * Might be extended case needed to cover + * different types of memory regions + */ + if (mr->ram_block && mr->dirty_log_mask) { + qemu_ram_writeback(mr->ram_block, addr, size); + } +} + /* * Call proper memory listeners about the change on the newly * added/removed CoalescedMemoryRange. diff --git a/util/cutils.c b/util/cutils.c index XXXXXXX..XXXXXXX 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -XXX,XX +XXX,XX @@ int qemu_fdatasync(int fd) #endif } +/** + * Sync changes made to the memory mapped file back to the backing + * storage. For POSIX compliant systems this will fallback + * to regular msync call. Otherwise it will trigger whole file sync + * (including the metadata case there is no support to skip that otherwise) + * + * @addr - start of the memory area to be synced + * @length - length of the are to be synced + * @fd - file descriptor for the file to be synced + * (mandatory only for POSIX non-compliant systems) + */ +int qemu_msync(void *addr, size_t length, int fd) +{ +#ifdef CONFIG_POSIX + size_t align_mask = ~(qemu_real_host_page_size - 1); + + /** + * There are no strict reqs as per the length of mapping + * to be synced. Still the length needs to follow the address + * alignment changes. Additionally - round the size to the multiple + * of PAGE_SIZE + */ + length += ((uintptr_t)addr & (qemu_real_host_page_size - 1)); + length = (length + ~align_mask) & align_mask; + + addr = (void *)((uintptr_t)addr & align_mask); + + return msync(addr, length, MS_SYNC); +#else /* CONFIG_POSIX */ + /** + * Perform the sync based on the file descriptor + * The sync range will most probably be wider than the one + * requested - but it will still get the job done + */ + return qemu_fdatasync(fd); +#endif /* CONFIG_POSIX */ +} + #ifndef _WIN32 /* Sets a specific flag */ int fcntl_setfl(int fd, int flag) -- 2.20.1 From: Beata Michalska <beata.michalska@linaro.org> Switch to ram block writeback for pmem migration. Signed-off-by: Beata Michalska <beata.michalska@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Message-id: 20191121000843.24844-4-beata.michalska@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- migration/ram.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index XXXXXXX..XXXXXXX 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -XXX,XX +XXX,XX @@ #include "qemu/bitops.h" #include "qemu/bitmap.h" #include "qemu/main-loop.h" -#include "qemu/pmem.h" #include "xbzrle.h" #include "ram.h" #include "migration.h" @@ -XXX,XX +XXX,XX @@ static int ram_load_cleanup(void *opaque) RAMBlock *rb; RAMBLOCK_FOREACH_NOT_IGNORED(rb) { - if (ramblock_is_pmem(rb)) { - pmem_persist(rb->host, rb->used_length); - } + qemu_ram_block_writeback(rb); } xbzrle_load_cleanup(); -- 2.20.1 From: Beata Michalska <beata.michalska@linaro.org> ARMv8.2 introduced support for Data Cache Clean instructions to PoP (point-of-persistence) - DC CVAP and PoDP (point-of-deep-persistence) - DV CVADP. Both specify conceptual points in a memory system where all writes that are to reach them are considered persistent. The support provided considers both to be actually the same so there is no distinction between the two. If none is available (there is no backing store for given memory) both will result in Data Cache Clean up to the point of coherency. Otherwise sync for the specified range shall be performed. Signed-off-by: Beata Michalska <beata.michalska@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191121000843.24844-5-beata.michalska@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 10 ++++++++ linux-user/elfload.c | 2 ++ target/arm/cpu64.c | 1 + target/arm/helper.c | 56 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline bool isar_feature_aa64_frint(const ARMISARegisters *id) return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, FRINTTS) != 0; } +static inline bool isar_feature_aa64_dcpop(const ARMISARegisters *id) +{ + return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, DPB) != 0; +} + +static inline bool isar_feature_aa64_dcpodp(const ARMISARegisters *id) +{ + return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, DPB) >= 2; +} + static inline bool isar_feature_aa64_fp16(const ARMISARegisters *id) { /* We always set the AdvSIMD and FP fields identically wrt FP16. */ 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 uint32_t get_elf_hwcap(void) GET_FEATURE_ID(aa64_jscvt, ARM_HWCAP_A64_JSCVT); GET_FEATURE_ID(aa64_sb, ARM_HWCAP_A64_SB); GET_FEATURE_ID(aa64_condm_4, ARM_HWCAP_A64_FLAGM); + GET_FEATURE_ID(aa64_dcpop, ARM_HWCAP_A64_DCPOP); return hwcaps; } @@ -XXX,XX +XXX,XX @@ static uint32_t get_elf_hwcap2(void) ARMCPU *cpu = ARM_CPU(thread_cpu); uint32_t hwcaps = 0; + GET_FEATURE_ID(aa64_dcpodp, ARM_HWCAP2_A64_DCPODP); GET_FEATURE_ID(aa64_condm_5, ARM_HWCAP2_A64_FLAGM2); GET_FEATURE_ID(aa64_frint, ARM_HWCAP2_A64_FRINT); diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -XXX,XX +XXX,XX @@ static void aarch64_max_initfn(Object *obj) cpu->isar.id_aa64isar0 = t; t = cpu->isar.id_aa64isar1; + t = FIELD_DP64(t, ID_AA64ISAR1, DPB, 2); t = FIELD_DP64(t, ID_AA64ISAR1, JSCVT, 1); t = FIELD_DP64(t, ID_AA64ISAR1, FCMA, 1); t = FIELD_DP64(t, ID_AA64ISAR1, APA, 1); /* PAuth, architected only */ 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 const ARMCPRegInfo rndr_reginfo[] = { .access = PL0_R, .readfn = rndr_readfn }, REGINFO_SENTINEL }; + +#ifndef CONFIG_USER_ONLY +static void dccvap_writefn(CPUARMState *env, const ARMCPRegInfo *opaque, + uint64_t value) +{ + ARMCPU *cpu = env_archcpu(env); + /* CTR_EL0 System register -> DminLine, bits [19:16] */ + uint64_t dline_size = 4 << ((cpu->ctr >> 16) & 0xF); + uint64_t vaddr_in = (uint64_t) value; + uint64_t vaddr = vaddr_in & ~(dline_size - 1); + void *haddr; + int mem_idx = cpu_mmu_index(env, false); + + /* This won't be crossing page boundaries */ + haddr = probe_read(env, vaddr, dline_size, mem_idx, GETPC()); + if (haddr) { + + ram_addr_t offset; + MemoryRegion *mr; + + /* RCU lock is already being held */ + mr = memory_region_from_host(haddr, &offset); + + if (mr) { + memory_region_do_writeback(mr, offset, dline_size); + } + } +} + +static const ARMCPRegInfo dcpop_reg[] = { + { .name = "DC_CVAP", .state = ARM_CP_STATE_AA64, + .opc0 = 1, .opc1 = 3, .crn = 7, .crm = 12, .opc2 = 1, + .access = PL0_W, .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END, + .accessfn = aa64_cacheop_access, .writefn = dccvap_writefn }, + REGINFO_SENTINEL +}; + +static const ARMCPRegInfo dcpodp_reg[] = { + { .name = "DC_CVADP", .state = ARM_CP_STATE_AA64, + .opc0 = 1, .opc1 = 3, .crn = 7, .crm = 13, .opc2 = 1, + .access = PL0_W, .type = ARM_CP_NO_RAW | ARM_CP_SUPPRESS_TB_END, + .accessfn = aa64_cacheop_access, .writefn = dccvap_writefn }, + REGINFO_SENTINEL +}; +#endif /*CONFIG_USER_ONLY*/ + #endif static CPAccessResult access_predinv(CPUARMState *env, const ARMCPRegInfo *ri, @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) if (cpu_isar_feature(aa64_rndr, cpu)) { define_arm_cp_regs(cpu, rndr_reginfo); } +#ifndef CONFIG_USER_ONLY + /* Data Cache clean instructions up to PoP */ + if (cpu_isar_feature(aa64_dcpop, cpu)) { + define_one_arm_cp_reg(cpu, dcpop_reg); + + if (cpu_isar_feature(aa64_dcpodp, cpu)) { + define_one_arm_cp_reg(cpu, dcpodp_reg); + } + } +#endif /*CONFIG_USER_ONLY*/ #endif /* -- 2.20.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> Make the gic a field in the machine state, and instead of filling an array of qemu_irq and passing it around, directly call qdev_get_gpio_in() on the gic field. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-id: 20191206162303.30338-1-philmd@redhat.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/sbsa-ref.c | 86 +++++++++++++++++++++++------------------------ 1 file changed, 42 insertions(+), 44 deletions(-) diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/sbsa-ref.c +++ b/hw/arm/sbsa-ref.c @@ -XXX,XX +XXX,XX @@ typedef struct { void *fdt; int fdt_size; int psci_conduit; + DeviceState *gic; PFlashCFI01 *flash[2]; } SBSAMachineState; @@ -XXX,XX +XXX,XX @@ static void create_secure_ram(SBSAMachineState *sms, memory_region_add_subregion(secure_sysmem, base, secram); } -static void create_gic(SBSAMachineState *sms, qemu_irq *pic) +static void create_gic(SBSAMachineState *sms) { unsigned int smp_cpus = MACHINE(sms)->smp.cpus; - DeviceState *gicdev; SysBusDevice *gicbusdev; const char *gictype; uint32_t redist0_capacity, redist0_count; @@ -XXX,XX +XXX,XX @@ static void create_gic(SBSAMachineState *sms, qemu_irq *pic) gictype = gicv3_class_name(); - gicdev = qdev_create(NULL, gictype); - qdev_prop_set_uint32(gicdev, "revision", 3); - qdev_prop_set_uint32(gicdev, "num-cpu", smp_cpus); + sms->gic = qdev_create(NULL, gictype); + qdev_prop_set_uint32(sms->gic, "revision", 3); + qdev_prop_set_uint32(sms->gic, "num-cpu", smp_cpus); /* * Note that the num-irq property counts both internal and external * interrupts; there are always 32 of the former (mandated by GIC spec). */ - qdev_prop_set_uint32(gicdev, "num-irq", NUM_IRQS + 32); - qdev_prop_set_bit(gicdev, "has-security-extensions", true); + qdev_prop_set_uint32(sms->gic, "num-irq", NUM_IRQS + 32); + qdev_prop_set_bit(sms->gic, "has-security-extensions", true); redist0_capacity = sbsa_ref_memmap[SBSA_GIC_REDIST].size / GICV3_REDIST_SIZE; redist0_count = MIN(smp_cpus, redist0_capacity); - qdev_prop_set_uint32(gicdev, "len-redist-region-count", 1); - qdev_prop_set_uint32(gicdev, "redist-region-count[0]", redist0_count); + qdev_prop_set_uint32(sms->gic, "len-redist-region-count", 1); + qdev_prop_set_uint32(sms->gic, "redist-region-count[0]", redist0_count); - qdev_init_nofail(gicdev); - gicbusdev = SYS_BUS_DEVICE(gicdev); + qdev_init_nofail(sms->gic); + gicbusdev = SYS_BUS_DEVICE(sms->gic); sysbus_mmio_map(gicbusdev, 0, sbsa_ref_memmap[SBSA_GIC_DIST].base); sysbus_mmio_map(gicbusdev, 1, sbsa_ref_memmap[SBSA_GIC_REDIST].base); @@ -XXX,XX +XXX,XX @@ static void create_gic(SBSAMachineState *sms, qemu_irq *pic) for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) { qdev_connect_gpio_out(cpudev, irq, - qdev_get_gpio_in(gicdev, + qdev_get_gpio_in(sms->gic, ppibase + timer_irq[irq])); } qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", 0, - qdev_get_gpio_in(gicdev, ppibase + qdev_get_gpio_in(sms->gic, ppibase + ARCH_GIC_MAINT_IRQ)); qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0, - qdev_get_gpio_in(gicdev, ppibase + qdev_get_gpio_in(sms->gic, ppibase + VIRTUAL_PMU_IRQ)); sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ)); @@ -XXX,XX +XXX,XX @@ static void create_gic(SBSAMachineState *sms, qemu_irq *pic) sysbus_connect_irq(gicbusdev, i + 3 * smp_cpus, qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ)); } - - for (i = 0; i < NUM_IRQS; i++) { - pic[i] = qdev_get_gpio_in(gicdev, i); - } } -static void create_uart(const SBSAMachineState *sms, qemu_irq *pic, int uart, +static void create_uart(const SBSAMachineState *sms, int uart, MemoryRegion *mem, Chardev *chr) { hwaddr base = sbsa_ref_memmap[uart].base; @@ -XXX,XX +XXX,XX @@ static void create_uart(const SBSAMachineState *sms, qemu_irq *pic, int uart, qdev_init_nofail(dev); memory_region_add_subregion(mem, base, sysbus_mmio_get_region(s, 0)); - sysbus_connect_irq(s, 0, pic[irq]); + sysbus_connect_irq(s, 0, qdev_get_gpio_in(sms->gic, irq)); } -static void create_rtc(const SBSAMachineState *sms, qemu_irq *pic) +static void create_rtc(const SBSAMachineState *sms) { hwaddr base = sbsa_ref_memmap[SBSA_RTC].base; int irq = sbsa_ref_irqmap[SBSA_RTC]; - sysbus_create_simple("pl031", base, pic[irq]); + sysbus_create_simple("pl031", base, qdev_get_gpio_in(sms->gic, irq)); } static DeviceState *gpio_key_dev; @@ -XXX,XX +XXX,XX @@ static Notifier sbsa_ref_powerdown_notifier = { .notify = sbsa_ref_powerdown_req }; -static void create_gpio(const SBSAMachineState *sms, qemu_irq *pic) +static void create_gpio(const SBSAMachineState *sms) { DeviceState *pl061_dev; hwaddr base = sbsa_ref_memmap[SBSA_GPIO].base; int irq = sbsa_ref_irqmap[SBSA_GPIO]; - pl061_dev = sysbus_create_simple("pl061", base, pic[irq]); + pl061_dev = sysbus_create_simple("pl061", base, + qdev_get_gpio_in(sms->gic, irq)); gpio_key_dev = sysbus_create_simple("gpio-key", -1, qdev_get_gpio_in(pl061_dev, 3)); @@ -XXX,XX +XXX,XX @@ static void create_gpio(const SBSAMachineState *sms, qemu_irq *pic) qemu_register_powerdown_notifier(&sbsa_ref_powerdown_notifier); } -static void create_ahci(const SBSAMachineState *sms, qemu_irq *pic) +static void create_ahci(const SBSAMachineState *sms) { hwaddr base = sbsa_ref_memmap[SBSA_AHCI].base; int irq = sbsa_ref_irqmap[SBSA_AHCI]; @@ -XXX,XX +XXX,XX @@ static void create_ahci(const SBSAMachineState *sms, qemu_irq *pic) qdev_prop_set_uint32(dev, "num-ports", NUM_SATA_PORTS); qdev_init_nofail(dev); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); - sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, qdev_get_gpio_in(sms->gic, irq)); sysahci = SYSBUS_AHCI(dev); ahci = &sysahci->ahci; @@ -XXX,XX +XXX,XX @@ static void create_ahci(const SBSAMachineState *sms, qemu_irq *pic) } } -static void create_ehci(const SBSAMachineState *sms, qemu_irq *pic) +static void create_ehci(const SBSAMachineState *sms) { hwaddr base = sbsa_ref_memmap[SBSA_EHCI].base; int irq = sbsa_ref_irqmap[SBSA_EHCI]; - sysbus_create_simple("platform-ehci-usb", base, pic[irq]); + sysbus_create_simple("platform-ehci-usb", base, + qdev_get_gpio_in(sms->gic, irq)); } -static void create_smmu(const SBSAMachineState *sms, qemu_irq *pic, - PCIBus *bus) +static void create_smmu(const SBSAMachineState *sms, PCIBus *bus) { hwaddr base = sbsa_ref_memmap[SBSA_SMMU].base; int irq = sbsa_ref_irqmap[SBSA_SMMU]; @@ -XXX,XX +XXX,XX @@ static void create_smmu(const SBSAMachineState *sms, qemu_irq *pic, qdev_init_nofail(dev); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); for (i = 0; i < NUM_SMMU_IRQS; i++) { - sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, + qdev_get_gpio_in(sms->gic, irq + 1)); } } -static void create_pcie(SBSAMachineState *sms, qemu_irq *pic) +static void create_pcie(SBSAMachineState *sms) { hwaddr base_ecam = sbsa_ref_memmap[SBSA_PCIE_ECAM].base; hwaddr size_ecam = sbsa_ref_memmap[SBSA_PCIE_ECAM].size; @@ -XXX,XX +XXX,XX @@ static void create_pcie(SBSAMachineState *sms, qemu_irq *pic) sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_pio); for (i = 0; i < GPEX_NUM_IRQS; i++) { - sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, + qdev_get_gpio_in(sms->gic, irq + 1)); gpex_set_irq_num(GPEX_HOST(dev), i, irq + i); } @@ -XXX,XX +XXX,XX @@ static void create_pcie(SBSAMachineState *sms, qemu_irq *pic) pci_create_simple(pci->bus, -1, "VGA"); - create_smmu(sms, pic, pci->bus); + create_smmu(sms, pci->bus); } static void *sbsa_ref_dtb(const struct arm_boot_info *binfo, int *fdt_size) @@ -XXX,XX +XXX,XX @@ static void sbsa_ref_init(MachineState *machine) bool firmware_loaded; const CPUArchIdList *possible_cpus; int n, sbsa_max_cpus; - qemu_irq pic[NUM_IRQS]; if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) { error_report("sbsa-ref: CPU type other than the built-in " @@ -XXX,XX +XXX,XX @@ static void sbsa_ref_init(MachineState *machine) create_secure_ram(sms, secure_sysmem); - create_gic(sms, pic); + create_gic(sms); - create_uart(sms, pic, SBSA_UART, sysmem, serial_hd(0)); - create_uart(sms, pic, SBSA_SECURE_UART, secure_sysmem, serial_hd(1)); + create_uart(sms, SBSA_UART, sysmem, serial_hd(0)); + create_uart(sms, SBSA_SECURE_UART, secure_sysmem, serial_hd(1)); /* Second secure UART for RAS and MM from EL0 */ - create_uart(sms, pic, SBSA_SECURE_UART_MM, secure_sysmem, serial_hd(2)); + create_uart(sms, SBSA_SECURE_UART_MM, secure_sysmem, serial_hd(2)); - create_rtc(sms, pic); + create_rtc(sms); - create_gpio(sms, pic); + create_gpio(sms); - create_ahci(sms, pic); + create_ahci(sms); - create_ehci(sms, pic); + create_ehci(sms); - create_pcie(sms, pic); + create_pcie(sms); sms->bootinfo.ram_size = machine->ram_size; sms->bootinfo.nb_cpus = smp_cpus; -- 2.20.1 From: Heyi Guo <guoheyi@huawei.com> The last argument of AML bit and/or statement is the target variable, so we don't need to use a NULL target and then an additional store operation; using just aml_and() or aml_or() statement is enough. Also update tests/data/acpi/virt/DSDT* to pass "make check". Cc: Shannon Zhao <shannon.zhaosl@gmail.com> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> Suggested-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Heyi Guo <guoheyi@huawei.com> Message-id: 20191209063719.23086-2-guoheyi@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt-acpi-build.c | 16 ++++++++-------- tests/data/acpi/virt/DSDT | Bin 18470 -> 18462 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19807 -> 19799 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18470 -> 18462 bytes 4 files changed, 8 insertions(+), 8 deletions(-) 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 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3")); aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP"))); aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL"))); - aml_append(ifctx, aml_store(aml_and(aml_name("CTRL"), aml_int(0x1D), NULL), - aml_name("CTRL"))); + aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), + aml_name("CTRL"))); ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1)))); - aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x08), NULL), - aml_name("CDW1"))); + aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x08), + aml_name("CDW1"))); aml_append(ifctx, ifctx1); ifctx1 = aml_if(aml_lnot(aml_equal(aml_name("CDW3"), aml_name("CTRL")))); - aml_append(ifctx1, aml_store(aml_or(aml_name("CDW1"), aml_int(0x10), NULL), - aml_name("CDW1"))); + aml_append(ifctx1, aml_or(aml_name("CDW1"), aml_int(0x10), + aml_name("CDW1"))); aml_append(ifctx, ifctx1); aml_append(ifctx, aml_store(aml_name("CTRL"), aml_name("CDW3"))); @@ -XXX,XX +XXX,XX @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_append(method, ifctx); elsectx = aml_else(); - aml_append(elsectx, aml_store(aml_or(aml_name("CDW1"), aml_int(4), NULL), - aml_name("CDW1"))); + aml_append(elsectx, aml_or(aml_name("CDW1"), aml_int(4), + aml_name("CDW1"))); aml_append(elsectx, aml_return(aml_arg(3))); aml_append(method, elsectx); aml_append(dev, method); diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT index XXXXXXX..XXXXXXX 100644 GIT binary patch delta 133 zcmZ2BfpOjhMlP3Nmk>D*1_q|2iCof5o%I{lJ2{y;?{412x!p#<jWgaq*qNm(o59&7 z+;D-%<VrV7_iE>mARjJS5V=5L(&S9WT970c2Uv;Nq{%?q7$gZ1761tsfcPNsCD{x4 MAmS{W8QoPG0j8@bzW@LL delta 141 zcmbO?fpOUcMlP3Nmk>1%1_q`n6S<_B8XGpMcXBc{-rKy1bGwazA7{LOuro_nHiNTE zxZwi7$(3%F{sq;}AwfP|vJ4<<fzYJMnT!RsAbBnhh%$*ulYv}gkTg_604z}e5&_99 R$zCV`m0@An{L@X95dZ+BD!u>! diff --git a/tests/data/acpi/virt/DSDT.memhp b/tests/data/acpi/virt/DSDT.memhp index XXXXXXX..XXXXXXX 100644 GIT binary patch delta 132 zcmcaVi}Cs_MlP3NmymE@1_ma@iCof*O&is^IGH-{Zr;SX-A2HTGu}VgnWZb6!PzC; zaDm6<N;gaQYUhw3A1+xCxj<mj<V?m|kR%reSc%xA$w1l|Bnc4~00|d>_#p8m*$ep~ L;w+mP-Q(B*s{AMU delta 140 zcmcaUi}C&}MlP3Nmymd01_maViCof*T^rT9IGGynZQjJW-A2HVGu}VgnWZb6!PzC; zaDm_CN;gaYf@<fGARjJS1`xGCXwu|N#)4XqJQoK<nZ%^YK&~-J8Y&?GmM8#;fMk|r QFBE{vurO@?=@!QZ00dYn_y7O^ diff --git a/tests/data/acpi/virt/DSDT.numamem b/tests/data/acpi/virt/DSDT.numamem index XXXXXXX..XXXXXXX 100644 GIT binary patch delta 133 zcmZ2BfpOjhMlP3Nmk>D*1_q|2iCof5o%I{lJ2{y;?{412x!p#<jWgaq*qNm(o59&7 z+;D-%<VrV7_iE>mARjJS5V=5L(&S9WT970c2Uv;Nq{%?q7$gZ1761tsfcPNsCD{x4 MAmS{W8QoPG0j8@bzW@LL delta 141 zcmbO?fpOUcMlP3Nmk>1%1_q`n6S<_B8XGpMcXBc{-rKy1bGwazA7{LOuro_nHiNTE zxZwi7$(3%F{sq;}AwfP|vJ4<<fzYJMnT!RsAbBnhh%$*ulYv}gkTg_604z}e5&_99 R$zCV`m0@An{L@X95dZ+BD!u>! -- 2.20.1 From: Heyi Guo <guoheyi@huawei.com> After the introduction of generic PCIe root port and PCIe-PCI bridge, we will also have SHPC controller on ARM, so just enable SHPC native hot plug. Also update tests/data/acpi/virt/DSDT* to pass "make check". Cc: Shannon Zhao <shannon.zhaosl@gmail.com> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Heyi Guo <guoheyi@huawei.com> Message-id: 20191209063719.23086-3-guoheyi@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt-acpi-build.c | 7 ++++++- tests/data/acpi/virt/DSDT | Bin 18462 -> 18462 bytes tests/data/acpi/virt/DSDT.memhp | Bin 19799 -> 19799 bytes tests/data/acpi/virt/DSDT.numamem | Bin 18462 -> 18462 bytes 4 files changed, 6 insertions(+), 1 deletion(-) 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 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, aml_create_dword_field(aml_arg(3), aml_int(8), "CDW3")); aml_append(ifctx, aml_store(aml_name("CDW2"), aml_name("SUPP"))); aml_append(ifctx, aml_store(aml_name("CDW3"), aml_name("CTRL"))); - aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1D), + + /* + * Allow OS control for all 5 features: + * PCIeHotplug SHPCHotplug PME AER PCIeCapability. + */ + aml_append(ifctx, aml_and(aml_name("CTRL"), aml_int(0x1F), aml_name("CTRL"))); ifctx1 = aml_if(aml_lnot(aml_equal(aml_arg(1), aml_int(0x1)))); diff --git a/tests/data/acpi/virt/DSDT b/tests/data/acpi/virt/DSDT index XXXXXXX..XXXXXXX 100644 GIT binary patch delta 28 kcmbO?fpOjhMlP3Nmk>D*1_q{tja=*8809zbbW3Ff0C~9xM*si- delta 28 kcmbO?fpOjhMlP3Nmk>D*1_q|2ja=*87-cu_bW3Ff0C~j-M*si- diff --git a/tests/data/acpi/virt/DSDT.memhp b/tests/data/acpi/virt/DSDT.memhp index XXXXXXX..XXXXXXX 100644 GIT binary patch delta 28 kcmcaUi}Cs_MlP3NmymE@1_mbija=*8809zbbeqQp0Eq|*2mk;8 delta 28 kcmcaUi}Cs_MlP3NmymE@1_ma@ja=*87-cu_beqQp0ErX{2mk;8 diff --git a/tests/data/acpi/virt/DSDT.numamem b/tests/data/acpi/virt/DSDT.numamem index XXXXXXX..XXXXXXX 100644 GIT binary patch delta 28 kcmbO?fpOjhMlP3Nmk>D*1_q{tja=*8809zbbW3Ff0C~9xM*si- delta 28 kcmbO?fpOjhMlP3Nmk>D*1_q|2ja=*87-cu_bW3Ff0C~j-M*si- -- 2.20.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> Make the gic a field in the machine state, and instead of filling an array of qemu_irq and passing it around, directly call qdev_get_gpio_in() on the gic field. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Luc Michel <luc.michel@greensocs.com> Message-id: 20191209090306.20433-1-philmd@redhat.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/virt.h | 1 + hw/arm/virt.c | 109 +++++++++++++++++++++--------------------- 2 files changed, 55 insertions(+), 55 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 @@ typedef struct { uint32_t iommu_phandle; int psci_conduit; hwaddr highest_gpa; + DeviceState *gic; DeviceState *acpi_dev; Notifier powerdown_notifier; } VirtMachineState; diff --git a/hw/arm/virt.c b/hw/arm/virt.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -XXX,XX +XXX,XX @@ static void fdt_add_pmu_nodes(const VirtMachineState *vms) } } -static inline DeviceState *create_acpi_ged(VirtMachineState *vms, qemu_irq *pic) +static inline DeviceState *create_acpi_ged(VirtMachineState *vms) { DeviceState *dev; MachineState *ms = MACHINE(vms); @@ -XXX,XX +XXX,XX @@ static inline DeviceState *create_acpi_ged(VirtMachineState *vms, qemu_irq *pic) sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vms->memmap[VIRT_ACPI_GED].base); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, vms->memmap[VIRT_PCDIMM_ACPI].base); - sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, qdev_get_gpio_in(vms->gic, irq)); qdev_init_nofail(dev); return dev; } -static void create_its(VirtMachineState *vms, DeviceState *gicdev) +static void create_its(VirtMachineState *vms) { const char *itsclass = its_class_name(); DeviceState *dev; @@ -XXX,XX +XXX,XX @@ static void create_its(VirtMachineState *vms, DeviceState *gicdev) dev = qdev_create(NULL, itsclass); - object_property_set_link(OBJECT(dev), OBJECT(gicdev), "parent-gicv3", + object_property_set_link(OBJECT(dev), OBJECT(vms->gic), "parent-gicv3", &error_abort); qdev_init_nofail(dev); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vms->memmap[VIRT_GIC_ITS].base); @@ -XXX,XX +XXX,XX @@ static void create_its(VirtMachineState *vms, DeviceState *gicdev) fdt_add_its_gic_node(vms); } -static void create_v2m(VirtMachineState *vms, qemu_irq *pic) +static void create_v2m(VirtMachineState *vms) { int i; int irq = vms->irqmap[VIRT_GIC_V2M]; @@ -XXX,XX +XXX,XX @@ static void create_v2m(VirtMachineState *vms, qemu_irq *pic) qdev_init_nofail(dev); for (i = 0; i < NUM_GICV2M_SPIS; i++) { - sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, + qdev_get_gpio_in(vms->gic, irq + i)); } fdt_add_v2m_gic_node(vms); } -static void create_gic(VirtMachineState *vms, qemu_irq *pic) +static void create_gic(VirtMachineState *vms) { MachineState *ms = MACHINE(vms); /* We create a standalone GIC */ - DeviceState *gicdev; SysBusDevice *gicbusdev; const char *gictype; int type = vms->gic_version, i; @@ -XXX,XX +XXX,XX @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic) gictype = (type == 3) ? gicv3_class_name() : gic_class_name(); - gicdev = qdev_create(NULL, gictype); - qdev_prop_set_uint32(gicdev, "revision", type); - qdev_prop_set_uint32(gicdev, "num-cpu", smp_cpus); + vms->gic = qdev_create(NULL, gictype); + qdev_prop_set_uint32(vms->gic, "revision", type); + qdev_prop_set_uint32(vms->gic, "num-cpu", smp_cpus); /* Note that the num-irq property counts both internal and external * interrupts; there are always 32 of the former (mandated by GIC spec). */ - qdev_prop_set_uint32(gicdev, "num-irq", NUM_IRQS + 32); + qdev_prop_set_uint32(vms->gic, "num-irq", NUM_IRQS + 32); if (!kvm_irqchip_in_kernel()) { - qdev_prop_set_bit(gicdev, "has-security-extensions", vms->secure); + qdev_prop_set_bit(vms->gic, "has-security-extensions", vms->secure); } if (type == 3) { @@ -XXX,XX +XXX,XX @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic) nb_redist_regions = virt_gicv3_redist_region_count(vms); - qdev_prop_set_uint32(gicdev, "len-redist-region-count", + qdev_prop_set_uint32(vms->gic, "len-redist-region-count", nb_redist_regions); - qdev_prop_set_uint32(gicdev, "redist-region-count[0]", redist0_count); + qdev_prop_set_uint32(vms->gic, "redist-region-count[0]", redist0_count); if (nb_redist_regions == 2) { uint32_t redist1_capacity = vms->memmap[VIRT_HIGH_GIC_REDIST2].size / GICV3_REDIST_SIZE; - qdev_prop_set_uint32(gicdev, "redist-region-count[1]", + qdev_prop_set_uint32(vms->gic, "redist-region-count[1]", MIN(smp_cpus - redist0_count, redist1_capacity)); } } else { if (!kvm_irqchip_in_kernel()) { - qdev_prop_set_bit(gicdev, "has-virtualization-extensions", + qdev_prop_set_bit(vms->gic, "has-virtualization-extensions", vms->virt); } } - qdev_init_nofail(gicdev); - gicbusdev = SYS_BUS_DEVICE(gicdev); + qdev_init_nofail(vms->gic); + gicbusdev = SYS_BUS_DEVICE(vms->gic); sysbus_mmio_map(gicbusdev, 0, vms->memmap[VIRT_GIC_DIST].base); if (type == 3) { sysbus_mmio_map(gicbusdev, 1, vms->memmap[VIRT_GIC_REDIST].base); @@ -XXX,XX +XXX,XX @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic) for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) { qdev_connect_gpio_out(cpudev, irq, - qdev_get_gpio_in(gicdev, + qdev_get_gpio_in(vms->gic, ppibase + timer_irq[irq])); } if (type == 3) { - qemu_irq irq = qdev_get_gpio_in(gicdev, + qemu_irq irq = qdev_get_gpio_in(vms->gic, ppibase + ARCH_GIC_MAINT_IRQ); qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", 0, irq); } else if (vms->virt) { - qemu_irq irq = qdev_get_gpio_in(gicdev, + qemu_irq irq = qdev_get_gpio_in(vms->gic, ppibase + ARCH_GIC_MAINT_IRQ); sysbus_connect_irq(gicbusdev, i + 4 * smp_cpus, irq); } qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0, - qdev_get_gpio_in(gicdev, ppibase + qdev_get_gpio_in(vms->gic, ppibase + VIRTUAL_PMU_IRQ)); sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ)); @@ -XXX,XX +XXX,XX @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic) qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ)); } - for (i = 0; i < NUM_IRQS; i++) { - pic[i] = qdev_get_gpio_in(gicdev, i); - } - fdt_add_gic_node(vms); if (type == 3 && vms->its) { - create_its(vms, gicdev); + create_its(vms); } else if (type == 2) { - create_v2m(vms, pic); + create_v2m(vms); } } -static void create_uart(const VirtMachineState *vms, qemu_irq *pic, int uart, +static void create_uart(const VirtMachineState *vms, int uart, MemoryRegion *mem, Chardev *chr) { char *nodename; @@ -XXX,XX +XXX,XX @@ static void create_uart(const VirtMachineState *vms, qemu_irq *pic, int uart, qdev_init_nofail(dev); memory_region_add_subregion(mem, base, sysbus_mmio_get_region(s, 0)); - sysbus_connect_irq(s, 0, pic[irq]); + sysbus_connect_irq(s, 0, qdev_get_gpio_in(vms->gic, irq)); nodename = g_strdup_printf("/pl011@%" PRIx64, base); qemu_fdt_add_subnode(vms->fdt, nodename); @@ -XXX,XX +XXX,XX @@ static void create_uart(const VirtMachineState *vms, qemu_irq *pic, int uart, g_free(nodename); } -static void create_rtc(const VirtMachineState *vms, qemu_irq *pic) +static void create_rtc(const VirtMachineState *vms) { char *nodename; hwaddr base = vms->memmap[VIRT_RTC].base; @@ -XXX,XX +XXX,XX @@ static void create_rtc(const VirtMachineState *vms, qemu_irq *pic) int irq = vms->irqmap[VIRT_RTC]; const char compat[] = "arm,pl031\0arm,primecell"; - sysbus_create_simple("pl031", base, pic[irq]); + sysbus_create_simple("pl031", base, qdev_get_gpio_in(vms->gic, irq)); nodename = g_strdup_printf("/pl031@%" PRIx64, base); qemu_fdt_add_subnode(vms->fdt, nodename); @@ -XXX,XX +XXX,XX @@ static void virt_powerdown_req(Notifier *n, void *opaque) } } -static void create_gpio(const VirtMachineState *vms, qemu_irq *pic) +static void create_gpio(const VirtMachineState *vms) { char *nodename; DeviceState *pl061_dev; @@ -XXX,XX +XXX,XX @@ static void create_gpio(const VirtMachineState *vms, qemu_irq *pic) int irq = vms->irqmap[VIRT_GPIO]; const char compat[] = "arm,pl061\0arm,primecell"; - pl061_dev = sysbus_create_simple("pl061", base, pic[irq]); + pl061_dev = sysbus_create_simple("pl061", base, + qdev_get_gpio_in(vms->gic, irq)); uint32_t phandle = qemu_fdt_alloc_phandle(vms->fdt); nodename = g_strdup_printf("/pl061@%" PRIx64, base); @@ -XXX,XX +XXX,XX @@ static void create_gpio(const VirtMachineState *vms, qemu_irq *pic) g_free(nodename); } -static void create_virtio_devices(const VirtMachineState *vms, qemu_irq *pic) +static void create_virtio_devices(const VirtMachineState *vms) { int i; hwaddr size = vms->memmap[VIRT_MMIO].size; @@ -XXX,XX +XXX,XX @@ static void create_virtio_devices(const VirtMachineState *vms, qemu_irq *pic) int irq = vms->irqmap[VIRT_MMIO] + i; hwaddr base = vms->memmap[VIRT_MMIO].base + i * size; - sysbus_create_simple("virtio-mmio", base, pic[irq]); + sysbus_create_simple("virtio-mmio", base, + qdev_get_gpio_in(vms->gic, irq)); } /* We add dtb nodes in reverse order so that they appear in the finished @@ -XXX,XX +XXX,XX @@ static void create_pcie_irq_map(const VirtMachineState *vms, 0x7 /* PCI irq */); } -static void create_smmu(const VirtMachineState *vms, qemu_irq *pic, +static void create_smmu(const VirtMachineState *vms, PCIBus *bus) { char *node; @@ -XXX,XX +XXX,XX @@ static void create_smmu(const VirtMachineState *vms, qemu_irq *pic, qdev_init_nofail(dev); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); for (i = 0; i < NUM_SMMU_IRQS; i++) { - sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, + qdev_get_gpio_in(vms->gic, irq + i)); } node = g_strdup_printf("/smmuv3@%" PRIx64, base); @@ -XXX,XX +XXX,XX @@ static void create_smmu(const VirtMachineState *vms, qemu_irq *pic, g_free(node); } -static void create_pcie(VirtMachineState *vms, qemu_irq *pic) +static void create_pcie(VirtMachineState *vms) { hwaddr base_mmio = vms->memmap[VIRT_PCIE_MMIO].base; hwaddr size_mmio = vms->memmap[VIRT_PCIE_MMIO].size; @@ -XXX,XX +XXX,XX @@ static void create_pcie(VirtMachineState *vms, qemu_irq *pic) sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_pio); for (i = 0; i < GPEX_NUM_IRQS; i++) { - sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, pic[irq + i]); + sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, + qdev_get_gpio_in(vms->gic, irq + i)); gpex_set_irq_num(GPEX_HOST(dev), i, irq + i); } @@ -XXX,XX +XXX,XX @@ static void create_pcie(VirtMachineState *vms, qemu_irq *pic) if (vms->iommu) { vms->iommu_phandle = qemu_fdt_alloc_phandle(vms->fdt); - create_smmu(vms, pic, pci->bus); + create_smmu(vms, pci->bus); qemu_fdt_setprop_cells(vms->fdt, nodename, "iommu-map", 0x0, vms->iommu_phandle, 0x0, 0x10000); @@ -XXX,XX +XXX,XX @@ static void create_pcie(VirtMachineState *vms, qemu_irq *pic) g_free(nodename); } -static void create_platform_bus(VirtMachineState *vms, qemu_irq *pic) +static void create_platform_bus(VirtMachineState *vms) { DeviceState *dev; SysBusDevice *s; @@ -XXX,XX +XXX,XX @@ static void create_platform_bus(VirtMachineState *vms, qemu_irq *pic) s = SYS_BUS_DEVICE(dev); for (i = 0; i < PLATFORM_BUS_NUM_IRQS; i++) { - int irqn = vms->irqmap[VIRT_PLATFORM_BUS] + i; - sysbus_connect_irq(s, i, pic[irqn]); + int irq = vms->irqmap[VIRT_PLATFORM_BUS] + i; + sysbus_connect_irq(s, i, qdev_get_gpio_in(vms->gic, irq)); } memory_region_add_subregion(sysmem, @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(machine); MachineClass *mc = MACHINE_GET_CLASS(machine); const CPUArchIdList *possible_cpus; - qemu_irq pic[NUM_IRQS]; MemoryRegion *sysmem = get_system_memory(); MemoryRegion *secure_sysmem = NULL; int n, virt_max_cpus; @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) virt_flash_fdt(vms, sysmem, secure_sysmem ?: sysmem); - create_gic(vms, pic); + create_gic(vms); fdt_add_pmu_nodes(vms); - create_uart(vms, pic, VIRT_UART, sysmem, serial_hd(0)); + create_uart(vms, VIRT_UART, sysmem, serial_hd(0)); if (vms->secure) { create_secure_ram(vms, secure_sysmem); - create_uart(vms, pic, VIRT_SECURE_UART, secure_sysmem, serial_hd(1)); + create_uart(vms, VIRT_SECURE_UART, secure_sysmem, serial_hd(1)); } vms->highmem_ecam &= vms->highmem && (!firmware_loaded || aarch64); - create_rtc(vms, pic); + create_rtc(vms); - create_pcie(vms, pic); + create_pcie(vms); if (has_ged && aarch64 && firmware_loaded && acpi_enabled) { - vms->acpi_dev = create_acpi_ged(vms, pic); + vms->acpi_dev = create_acpi_ged(vms); } else { - create_gpio(vms, pic); + create_gpio(vms); } /* connect powerdown request */ @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) * (which will be automatically plugged in to the transports). If * no backend is created the transport will just sit harmlessly idle. */ - create_virtio_devices(vms, pic); + create_virtio_devices(vms); vms->fw_cfg = create_fw_cfg(vms, &address_space_memory); rom_set_fw(vms->fw_cfg); - create_platform_bus(vms, pic); + create_platform_bus(vms); vms->bootinfo.ram_size = machine->ram_size; vms->bootinfo.nb_cpus = smp_cpus; -- 2.20.1 From: Alex Bennée <alex.bennee@linaro.org> A write to the SCR can change the effective EL by droppping the system from secure to non-secure mode. However if we use a cached current_el from before the change we'll rebuild the flags incorrectly. To fix this we introduce the ARM_CP_NEWEL CP flag to indicate the new EL should be used when recomputing the flags. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20191212114734.6962-1-alex.bennee@linaro.org Cc: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20191209143723.6368-1-alex.bennee@linaro.org> Cc: qemu-stable@nongnu.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 8 ++++++-- target/arm/helper.h | 1 + target/arm/helper.c | 14 +++++++++++++- target/arm/translate.c | 6 +++++- 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) * RAISES_EXC is for when the read or write hook might raise an exception; * the generated code will synchronize the CPU state before calling the hook * so that it is safe for the hook to call raise_exception(). + * NEWEL is for writes to registers that might change the exception + * level - typically on older ARM chips. For those cases we need to + * re-read the new el when recomputing the translation flags. */ #define ARM_CP_SPECIAL 0x0001 #define ARM_CP_CONST 0x0002 @@ -XXX,XX +XXX,XX @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) #define ARM_CP_SVE 0x2000 #define ARM_CP_NO_GDB 0x4000 #define ARM_CP_RAISES_EXC 0x8000 +#define ARM_CP_NEWEL 0x10000 /* Used only as a terminator for ARMCPRegInfo lists */ -#define ARM_CP_SENTINEL 0xffff +#define ARM_CP_SENTINEL 0xfffff /* Mask of only the flag bits in a type field */ -#define ARM_CP_FLAG_MASK 0xf0ff +#define ARM_CP_FLAG_MASK 0x1f0ff /* Valid values for ARMCPRegInfo state field, indicating which of * the AArch32 and AArch64 execution states this register is visible in. 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_2(get_user_reg, i32, env, i32) DEF_HELPER_3(set_user_reg, void, env, i32, i32) DEF_HELPER_FLAGS_2(rebuild_hflags_m32, TCG_CALL_NO_RWG, void, env, int) +DEF_HELPER_FLAGS_1(rebuild_hflags_a32_newel, TCG_CALL_NO_RWG, void, env) DEF_HELPER_FLAGS_2(rebuild_hflags_a32, TCG_CALL_NO_RWG, void, env, int) DEF_HELPER_FLAGS_2(rebuild_hflags_a64, TCG_CALL_NO_RWG, void, env, int) 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 const ARMCPRegInfo el3_cp_reginfo[] = { .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 0, .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3), .resetvalue = 0, .writefn = scr_write }, - { .name = "SCR", .type = ARM_CP_ALIAS, + { .name = "SCR", .type = ARM_CP_ALIAS | ARM_CP_NEWEL, .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 0, .access = PL1_RW, .accessfn = access_trap_aa32s_el1, .fieldoffset = offsetoflow32(CPUARMState, cp15.scr_el3), @@ -XXX,XX +XXX,XX @@ void HELPER(rebuild_hflags_m32)(CPUARMState *env, int el) env->hflags = rebuild_hflags_m32(env, fp_el, mmu_idx); } +/* + * If we have triggered a EL state change we can't rely on the + * translator having passed it too us, we need to recompute. + */ +void HELPER(rebuild_hflags_a32_newel)(CPUARMState *env) +{ + int el = arm_current_el(env); + int fp_el = fp_exception_el(env, el); + ARMMMUIdx mmu_idx = arm_mmu_idx_el(env, el); + env->hflags = rebuild_hflags_a32(env, fp_el, mmu_idx); +} + void HELPER(rebuild_hflags_a32)(CPUARMState *env, int el) { int fp_el = fp_exception_el(env, el); diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static int disas_coproc_insn(DisasContext *s, uint32_t insn) if (arm_dc_feature(s, ARM_FEATURE_M)) { gen_helper_rebuild_hflags_m32(cpu_env, tcg_el); } else { - gen_helper_rebuild_hflags_a32(cpu_env, tcg_el); + if (ri->type & ARM_CP_NEWEL) { + gen_helper_rebuild_hflags_a32_newel(cpu_env); + } else { + gen_helper_rebuild_hflags_a32(cpu_env, tcg_el); + } } tcg_temp_free_i32(tcg_el); /* -- 2.20.1