target-arm queue: this time around is all small fixes and changes. thanks -- PMM The following changes since commit fec105c2abda8567ec15230429c41429b5ee307c: Merge remote-tracking branch 'remotes/kraxel/tags/audio-20190828-pull-request' into staging (2019-09-03 14:03:15 +0100) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20190903 for you to fetch changes up to 5e5584c89f36b302c666bc6db535fd3f7ff35ad2: target/arm: Don't abort on M-profile exception return in linux-user mode (2019-09-03 16:20:35 +0100) ---------------------------------------------------------------- target-arm queue: * Revert and correctly fix refactoring of unallocated_encoding() * Take exceptions on ATS instructions when needed * aspeed/timer: Provide back-pressure information for short periods * memory: Remove unused memory_region_iommu_replay_all() * hw/arm/smmuv3: Log a guest error when decoding an invalid STE * hw/arm/smmuv3: Remove spurious error messages on IOVA invalidations * target/arm: Fix SMMLS argument order * hw/arm: Use ARM_CPU_TYPE_NAME() macro when appropriate * hw/arm: Correct reference counting for creation of various objects * includes: remove stale [smp|max]_cpus externs * tcg/README: fix typo * atomic_template: fix indentation in GEN_ATOMIC_HELPER * include/exec/cpu-defs.h: fix typo * target/arm: Free TCG temps in trans_VMOV_64_sp() * target/arm: Don't abort on M-profile exception return in linux-user mode ---------------------------------------------------------------- Alex Bennée (2): includes: remove stale [smp|max]_cpus externs include/exec/cpu-defs.h: fix typo Andrew Jeffery (1): aspeed/timer: Provide back-pressure information for short periods Emilio G. Cota (2): tcg/README: fix typo s/afterwise/afterwards/ atomic_template: fix indentation in GEN_ATOMIC_HELPER Eric Auger (3): memory: Remove unused memory_region_iommu_replay_all() hw/arm/smmuv3: Log a guest error when decoding an invalid STE hw/arm/smmuv3: Remove spurious error messages on IOVA invalidations Peter Maydell (4): target/arm: Allow ARMCPRegInfo read/write functions to throw exceptions target/arm: Take exceptions on ATS instructions when needed target/arm: Free TCG temps in trans_VMOV_64_sp() target/arm: Don't abort on M-profile exception return in linux-user mode Philippe Mathieu-Daudé (6): hw/arm: Use ARM_CPU_TYPE_NAME() macro when appropriate hw/arm: Use object_initialize_child for correct reference counting hw/arm: Use sysbus_init_child_obj for correct reference counting hw/arm/fsl-imx: Add the cpu as child of the SoC object hw/dma/xilinx_axi: Use object_initialize_child for correct ref. counting hw/net/xilinx_axi: Use object_initialize_child for correct ref. counting Richard Henderson (3): Revert "target/arm: Use unallocated_encoding for aarch32" target/arm: Factor out unallocated_encoding for aarch32 target/arm: Fix SMMLS argument order accel/tcg/atomic_template.h | 2 +- hw/arm/smmuv3-internal.h | 1 + include/exec/cpu-defs.h | 2 +- include/exec/memory.h | 10 ---- include/sysemu/sysemu.h | 2 - target/arm/cpu.h | 6 ++- target/arm/translate-a64.h | 2 + target/arm/translate.h | 2 - hw/arm/allwinner-a10.c | 3 +- hw/arm/cubieboard.c | 3 +- hw/arm/digic.c | 3 +- hw/arm/exynos4_boards.c | 4 +- hw/arm/fsl-imx25.c | 4 +- hw/arm/fsl-imx31.c | 4 +- hw/arm/fsl-imx6.c | 3 +- hw/arm/fsl-imx6ul.c | 2 +- hw/arm/mcimx7d-sabre.c | 9 ++-- hw/arm/mps2-tz.c | 15 +++--- hw/arm/musca.c | 9 ++-- hw/arm/smmuv3.c | 18 ++++--- hw/arm/xlnx-zynqmp.c | 8 +-- hw/dma/xilinx_axidma.c | 16 +++--- hw/net/xilinx_axienet.c | 17 +++---- hw/timer/aspeed_timer.c | 17 ++++++- memory.c | 9 ---- target/arm/helper.c | 107 +++++++++++++++++++++++++++++++++++------ target/arm/translate-a64.c | 13 +++++ target/arm/translate-vfp.inc.c | 2 + target/arm/translate.c | 50 +++++++++++++++++-- tcg/README | 2 +- 30 files changed, 244 insertions(+), 101 deletions(-) From: Richard Henderson <richard.henderson@linaro.org> This reverts commit 3cb36637157088892e9e33ddb1034bffd1251d3b. Despite the fact that the text for the call to gen_exception_insn is identical for aarch64 and aarch32, the implementation inside gen_exception_insn is totally different. This fixes exceptions raised from aarch64. Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com> Message-id: 20190826151536.6771-2-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-a64.h | 2 ++ target/arm/translate.h | 2 -- target/arm/translate-a64.c | 7 +++++++ target/arm/translate-vfp.inc.c | 3 ++- target/arm/translate.c | 22 ++++++++++------------ 5 files changed, 21 insertions(+), 15 deletions(-) diff --git a/target/arm/translate-a64.h b/target/arm/translate-a64.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-a64.h +++ b/target/arm/translate-a64.h @@ -XXX,XX +XXX,XX @@ #ifndef TARGET_ARM_TRANSLATE_A64_H #define TARGET_ARM_TRANSLATE_A64_H +void unallocated_encoding(DisasContext *s); + #define unsupported_encoding(s, insn) \ do { \ qemu_log_mask(LOG_UNIMP, \ 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 DisasCompare { bool value_global; } DisasCompare; -void unallocated_encoding(DisasContext *s); - /* Share the TCG temporaries common between 32 and 64 bit modes. */ extern TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF; extern TCGv_i64 cpu_exclusive_addr; 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 inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest) } } +void unallocated_encoding(DisasContext *s) +{ + /* Unallocated and reserved encodings are uncategorized */ + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), + default_exception_el(s)); +} + static void init_tmp_a64_array(DisasContext *s) { #ifdef CONFIG_DEBUG_TCG 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 full_vfp_access_check(DisasContext *s, bool ignore_vfp_enabled) if (!s->vfp_enabled && !ignore_vfp_enabled) { assert(!arm_dc_feature(s, ARM_FEATURE_M)); - unallocated_encoding(s); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), + default_exception_el(s)); return false; } diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn) s->base.is_jmp = DISAS_NORETURN; } -void unallocated_encoding(DisasContext *s) -{ - /* Unallocated and reserved encodings are uncategorized */ - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); -} - /* Force a TB lookup after an instruction that changes the CPU state. */ static inline void gen_lookup_tb(DisasContext *s) { @@ -XXX,XX +XXX,XX @@ static inline void gen_hlt(DisasContext *s, int imm) return; } - unallocated_encoding(s); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), + default_exception_el(s)); } static inline void gen_add_data_offset(DisasContext *s, unsigned int insn, @@ -XXX,XX +XXX,XX @@ static void gen_srs(DisasContext *s, } if (undef) { - unallocated_encoding(s); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), + default_exception_el(s)); return; } @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) break; default: illegal_op: - unallocated_encoding(s); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), + default_exception_el(s)); break; } } @@ -XXX,XX +XXX,XX @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) } return; illegal_op: - unallocated_encoding(s); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), + default_exception_el(s)); } static void disas_thumb_insn(DisasContext *s, uint32_t insn) @@ -XXX,XX +XXX,XX @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn) return; illegal_op: undef: - unallocated_encoding(s); + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), + default_exception_el(s)); } static bool insn_crosses_page(CPUARMState *env, DisasContext *s) -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> Make this a static function private to translate.c. Thus we can use the same idiom between aarch64 and aarch32 without actually sharing function implementations. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com> Message-id: 20190826151536.6771-3-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate-vfp.inc.c | 3 +-- target/arm/translate.c | 22 ++++++++++++---------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate-vfp.inc.c +++ b/target/arm/translate-vfp.inc.c @@ -XXX,XX +XXX,XX @@ static bool full_vfp_access_check(DisasContext *s, bool ignore_vfp_enabled) if (!s->vfp_enabled && !ignore_vfp_enabled) { assert(!arm_dc_feature(s, ARM_FEATURE_M)); - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); + unallocated_encoding(s); return false; } diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn) s->base.is_jmp = DISAS_NORETURN; } +static void unallocated_encoding(DisasContext *s) +{ + /* Unallocated and reserved encodings are uncategorized */ + gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), + default_exception_el(s)); +} + /* Force a TB lookup after an instruction that changes the CPU state. */ static inline void gen_lookup_tb(DisasContext *s) { @@ -XXX,XX +XXX,XX @@ static inline void gen_hlt(DisasContext *s, int imm) return; } - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); + unallocated_encoding(s); } static inline void gen_add_data_offset(DisasContext *s, unsigned int insn, @@ -XXX,XX +XXX,XX @@ static void gen_srs(DisasContext *s, } if (undef) { - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); + unallocated_encoding(s); return; } @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) break; default: illegal_op: - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); + unallocated_encoding(s); break; } } @@ -XXX,XX +XXX,XX @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) } return; illegal_op: - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); + unallocated_encoding(s); } static void disas_thumb_insn(DisasContext *s, uint32_t insn) @@ -XXX,XX +XXX,XX @@ static void disas_thumb_insn(DisasContext *s, uint32_t insn) return; illegal_op: undef: - gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); + unallocated_encoding(s); } static bool insn_crosses_page(CPUARMState *env, DisasContext *s) -- 2.20.1 Currently the only part of an ARMCPRegInfo which is allowed to cause a CPU exception is the access function, which returns a value indicating that some flavour of UNDEF should be generated. For the ATS system instructions, we would like to conditionally generate exceptions as part of the writefn, because some faults during the page table walk (like external aborts) should cause an exception to be raised rather than returning a value. There are several ways we could do this: * plumb the GETPC() value from the top level set_cp_reg/get_cp_reg helper functions through into the readfn and writefn hooks * add extra readfn_with_ra/writefn_with_ra hooks that take the GETPC() value * require the ATS instructions to provide a dummy accessfn, which serves no purpose except to cause the code generation to emit TCG ops to sync the CPU state * add an ARM_CP_ flag to mark the ARMCPRegInfo as possibly throwing an exception in its read/write hooks, and make the codegen sync the CPU state before calling the hooks if the flag is set This patch opts for the last of these, as it is fairly simple to implement and doesn't require invasive changes like updating the readfn/writefn hook function prototype signature. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Message-id: 20190816125802.25877-2-peter.maydell@linaro.org --- target/arm/cpu.h | 6 +++++- target/arm/translate-a64.c | 6 ++++++ target/arm/translate.c | 7 +++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) * IO indicates that this register does I/O and therefore its accesses * need to be surrounded by gen_io_start()/gen_io_end(). In particular, * registers which implement clocks or timers require this. + * 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(). */ #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_FPU 0x1000 #define ARM_CP_SVE 0x2000 #define ARM_CP_NO_GDB 0x4000 +#define ARM_CP_RAISES_EXC 0x8000 /* Used only as a terminator for ARMCPRegInfo lists */ #define ARM_CP_SENTINEL 0xffff /* Mask of only the flag bits in a type field */ -#define ARM_CP_FLAG_MASK 0x70ff +#define ARM_CP_FLAG_MASK 0xf0ff /* 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/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_sys(DisasContext *s, uint32_t insn, bool isread, tcg_temp_free_ptr(tmpptr); tcg_temp_free_i32(tcg_syn); tcg_temp_free_i32(tcg_isread); + } else if (ri->type & ARM_CP_RAISES_EXC) { + /* + * The readfn or writefn might raise an exception; + * synchronize the CPU state in case it does. + */ + gen_a64_set_pc_im(s->pc_curr); } /* Handle special cases first */ 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) tcg_temp_free_ptr(tmpptr); tcg_temp_free_i32(tcg_syn); tcg_temp_free_i32(tcg_isread); + } else if (ri->type & ARM_CP_RAISES_EXC) { + /* + * The readfn or writefn might raise an exception; + * synchronize the CPU state in case it does. + */ + gen_set_condexec(s); + gen_set_pc_im(s, s->pc_curr); } /* Handle special cases first */ -- 2.20.1 The translation table walk for an ATS instruction can result in various faults. In general these are just reported back via the PAR_EL1 fault status fields, but in some cases the architecture requires that the fault is turned into an exception: * synchronous stage 2 faults of any kind during AT S1E0* and AT S1E1* instructions executed from NS EL1 fault to EL2 or EL3 * synchronous external aborts are taken as Data Abort exceptions (This is documented in the v8A Arm ARM DDI0487A.e D5.2.11 and G5.13.4.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Message-id: 20190816125802.25877-3-peter.maydell@linaro.org --- target/arm/helper.c | 107 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 92 insertions(+), 15 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 do_ats_write(CPUARMState *env, uint64_t value, ret = get_phys_addr(env, value, access_type, mmu_idx, &phys_addr, &attrs, &prot, &page_size, &fi, &cacheattrs); + if (ret) { + /* + * Some kinds of translation fault must cause exceptions rather + * than being reported in the PAR. + */ + int current_el = arm_current_el(env); + int target_el; + uint32_t syn, fsr, fsc; + bool take_exc = false; + + if (fi.s1ptw && current_el == 1 && !arm_is_secure(env) + && (mmu_idx == ARMMMUIdx_S1NSE1 || mmu_idx == ARMMMUIdx_S1NSE0)) { + /* + * Synchronous stage 2 fault on an access made as part of the + * translation table walk for AT S1E0* or AT S1E1* insn + * executed from NS EL1. If this is a synchronous external abort + * and SCR_EL3.EA == 1, then we take a synchronous external abort + * to EL3. Otherwise the fault is taken as an exception to EL2, + * and HPFAR_EL2 holds the faulting IPA. + */ + if (fi.type == ARMFault_SyncExternalOnWalk && + (env->cp15.scr_el3 & SCR_EA)) { + target_el = 3; + } else { + env->cp15.hpfar_el2 = extract64(fi.s2addr, 12, 47) << 4; + target_el = 2; + } + take_exc = true; + } else if (fi.type == ARMFault_SyncExternalOnWalk) { + /* + * Synchronous external aborts during a translation table walk + * are taken as Data Abort exceptions. + */ + if (fi.stage2) { + if (current_el == 3) { + target_el = 3; + } else { + target_el = 2; + } + } else { + target_el = exception_target_el(env); + } + take_exc = true; + } + + if (take_exc) { + /* Construct FSR and FSC using same logic as arm_deliver_fault() */ + if (target_el == 2 || arm_el_is_aa64(env, target_el) || + arm_s1_regime_using_lpae_format(env, mmu_idx)) { + fsr = arm_fi_to_lfsc(&fi); + fsc = extract32(fsr, 0, 6); + } else { + fsr = arm_fi_to_sfsc(&fi); + fsc = 0x3f; + } + /* + * Report exception with ESR indicating a fault due to a + * translation table walk for a cache maintenance instruction. + */ + syn = syn_data_abort_no_iss(current_el == target_el, + fi.ea, 1, fi.s1ptw, 1, fsc); + env->exception.vaddress = value; + env->exception.fsr = fsr; + raise_exception(env, EXCP_DATA_ABORT, syn, target_el); + } + } + if (is_a64(env)) { format64 = true; } else if (arm_feature(env, ARM_FEATURE_LPAE)) { @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo vapa_cp_reginfo[] = { /* This underdecoding is safe because the reginfo is NO_RAW. */ { .name = "ATS", .cp = 15, .crn = 7, .crm = 8, .opc1 = 0, .opc2 = CP_ANY, .access = PL1_W, .accessfn = ats_access, - .writefn = ats_write, .type = ARM_CP_NO_RAW }, + .writefn = ats_write, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC }, #endif REGINFO_SENTINEL }; @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v8_cp_reginfo[] = { /* 64 bit address translation operations */ { .name = "AT_S1E1R", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 0, - .access = PL1_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "AT_S1E1W", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 1, - .access = PL1_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "AT_S1E0R", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 2, - .access = PL1_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "AT_S1E0W", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 3, - .access = PL1_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "AT_S12E1R", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 4, - .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "AT_S12E1W", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 5, - .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "AT_S12E0R", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 6, - .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "AT_S12E0W", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 7, - .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, /* AT S1E2* are elsewhere as they UNDEF from EL3 if EL2 is not present */ { .name = "AT_S1E3R", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 6, .crn = 7, .crm = 8, .opc2 = 0, - .access = PL3_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL3_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "AT_S1E3W", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 6, .crn = 7, .crm = 8, .opc2 = 1, - .access = PL3_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .access = PL3_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, + .writefn = ats_write64 }, { .name = "PAR_EL1", .state = ARM_CP_STATE_AA64, .type = ARM_CP_ALIAS, .opc0 = 3, .opc1 = 0, .crn = 7, .crm = 4, .opc2 = 0, @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo el2_cp_reginfo[] = { { .name = "AT_S1E2R", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 0, .access = PL2_W, .accessfn = at_s1e2_access, - .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, .writefn = ats_write64 }, { .name = "AT_S1E2W", .state = ARM_CP_STATE_AA64, .opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 1, .access = PL2_W, .accessfn = at_s1e2_access, - .type = ARM_CP_NO_RAW, .writefn = ats_write64 }, + .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, .writefn = ats_write64 }, /* The AArch32 ATS1H* operations are CONSTRAINED UNPREDICTABLE * if EL2 is not implemented; we choose to UNDEF. Behaviour at EL3 * with SCR.NS == 0 outside Monitor mode is UNPREDICTABLE; we choose @@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo el2_cp_reginfo[] = { */ { .name = "ATS1HR", .cp = 15, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 0, .access = PL2_W, - .writefn = ats1h_write, .type = ARM_CP_NO_RAW }, + .writefn = ats1h_write, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC }, { .name = "ATS1HW", .cp = 15, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 1, .access = PL2_W, - .writefn = ats1h_write, .type = ARM_CP_NO_RAW }, + .writefn = ats1h_write, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC }, { .name = "CNTHCTL_EL2", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .opc1 = 4, .crn = 14, .crm = 1, .opc2 = 0, /* ARMv7 requires bit 0 and 1 to reset to 1. ARMv8 defines the -- 2.20.1 From: Andrew Jeffery <andrew@aj.id.au> First up: This is not the way the hardware behaves. However, it helps resolve real-world problems with short periods being used under Linux. Commit 4451d3f59f2a ("clocksource/drivers/fttmr010: Fix set_next_event handler") in Linux fixed the timer driver to correctly schedule the next event for the Aspeed controller, and in combination with 5daa8212c08e ("ARM: dts: aspeed: Describe random number device") Linux will now set a timer with a period as low as 1us. Configuring a qemu timer with such a short period results in spending time handling the interrupt in the model rather than executing guest code, leading to noticeable "sticky" behaviour in the guest. The behaviour of Linux is correct with respect to the hardware, so we need to improve our handling under emulation. The approach chosen is to provide back-pressure information by calculating an acceptable minimum number of ticks to be set on the model. Under Linux an additional read is added in the timer configuration path to detect back-pressure, which will never occur on hardware. However if back-pressure is observed, the driver alerts the clock event subsystem, which then performs its own next event dilation via a config option - d1748302f70b ("clockevents: Make minimum delay adjustments configurable") A minimum period of 5us was experimentally determined on a Lenovo T480s, which I've increased to 20us for "safety". Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Reviewed-by: Joel Stanley <joel@jms.id.au> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Cédric Le Goater <clg@kaod.org> Message-id: 20190704055150.4899-1-clg@kaod.org [clg: - changed the computation of min_ticks to be done each time the timer value is reloaded. It removes the ordering issue of the timer and scu reset handlers but is slightly slower ] - introduced TIMER_MIN_NS - introduced calculate_min_ticks() ] Signed-off-by: Cédric Le Goater <clg@kaod.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/timer/aspeed_timer.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) 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 @@ enum timer_ctrl_op { op_pulse_enable }; +/* + * Minimum value of the reload register to filter out short period + * timers which have a noticeable impact in emulation. 5us should be + * enough, use 20us for "safety". + */ +#define TIMER_MIN_NS (20 * SCALE_US) + /** * Avoid mutual references between AspeedTimerCtrlState and AspeedTimer * structs, as it's a waste of memory. The ptimer BH callback needs to know @@ -XXX,XX +XXX,XX @@ static inline uint32_t calculate_ticks(struct AspeedTimer *t, uint64_t now_ns) return t->reload - MIN(t->reload, ticks); } +static uint32_t calculate_min_ticks(AspeedTimer *t, uint32_t value) +{ + uint32_t rate = calculate_rate(t); + uint32_t min_ticks = muldiv64(TIMER_MIN_NS, rate, NANOSECONDS_PER_SECOND); + + return value < min_ticks ? min_ticks : value; +} + static inline uint64_t calculate_time(struct AspeedTimer *t, uint32_t ticks) { uint64_t delta_ns; @@ -XXX,XX +XXX,XX @@ static void aspeed_timer_set_value(AspeedTimerCtrlState *s, int timer, int reg, switch (reg) { case TIMER_REG_RELOAD: old_reload = t->reload; - t->reload = value; + t->reload = calculate_min_ticks(t, value); /* If the reload value was not previously set, or zero, and * the current value is valid, try to start the timer if it is -- 2.20.1 From: Eric Auger <eric.auger@redhat.com> memory_region_iommu_replay_all is not used. Remove it. Signed-off-by: Eric Auger <eric.auger@redhat.com> Reported-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com> Message-id: 20190822172350.12008-2-eric.auger@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/exec/memory.h | 10 ---------- memory.c | 9 --------- 2 files changed, 19 deletions(-) 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_register_iommu_notifier(MemoryRegion *mr, */ void memory_region_iommu_replay(IOMMUMemoryRegion *iommu_mr, IOMMUNotifier *n); -/** - * memory_region_iommu_replay_all: replay existing IOMMU translations - * to all the notifiers registered. - * - * Note: this is not related to record-and-replay functionality. - * - * @iommu_mr: the memory region to observe - */ -void memory_region_iommu_replay_all(IOMMUMemoryRegion *iommu_mr); - /** * memory_region_unregister_iommu_notifier: unregister a notifier for * changes to IOMMU translation entries. 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_iommu_replay(IOMMUMemoryRegion *iommu_mr, IOMMUNotifier *n) } } -void memory_region_iommu_replay_all(IOMMUMemoryRegion *iommu_mr) -{ - IOMMUNotifier *notifier; - - IOMMU_NOTIFIER_FOREACH(notifier, iommu_mr) { - memory_region_iommu_replay(iommu_mr, notifier); - } -} - void memory_region_unregister_iommu_notifier(MemoryRegion *mr, IOMMUNotifier *n) { -- 2.20.1 From: Eric Auger <eric.auger@redhat.com> Log a guest error when encountering an invalid STE. Signed-off-by: Eric Auger <eric.auger@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-id: 20190822172350.12008-5-eric.auger@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/smmuv3.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -XXX,XX +XXX,XX @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg, uint32_t config; if (!STE_VALID(ste)) { + qemu_log_mask(LOG_GUEST_ERROR, "invalid STE\n"); goto bad_ste; } -- 2.20.1 From: Eric Auger <eric.auger@redhat.com> An IOVA/ASID invalidation is notified to all IOMMU Memory Regions through smmuv3_inv_notifiers_iova/smmuv3_notify_iova. When the notification occurs it is possible that some of the PCIe devices associated to the notified regions do not have a valid stream table entry. In that case we output a LOG_GUEST_ERROR message, for example: invalid sid=<SID> (L1STD span=0) "smmuv3_notify_iova error decoding the configuration for iommu mr=<MR> This is unfortunate as the user gets the impression that there are some translation decoding errors whereas there are not. This patch adds a new field in SMMUEventInfo that tells whether the detection of an invalid STE must lead to an error report. invalid_ste_allowed is set before doing the invalidations and kept unset on actual translation. The other configuration decoding error messages are kept since if the STE is valid then the rest of the config must be correct. Signed-off-by: Eric Auger <eric.auger@redhat.com> Message-id: 20190822172350.12008-6-eric.auger@redhat.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/smmuv3-internal.h | 1 + hw/arm/smmuv3.c | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/smmuv3-internal.h +++ b/hw/arm/smmuv3-internal.h @@ -XXX,XX +XXX,XX @@ typedef struct SMMUEventInfo { uint32_t sid; bool recorded; bool record_trans_faults; + bool inval_ste_allowed; union { struct { uint32_t ssid; diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -XXX,XX +XXX,XX @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg, uint32_t config; if (!STE_VALID(ste)) { - qemu_log_mask(LOG_GUEST_ERROR, "invalid STE\n"); + if (!event->inval_ste_allowed) { + qemu_log_mask(LOG_GUEST_ERROR, "invalid STE\n"); + } goto bad_ste; } @@ -XXX,XX +XXX,XX @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste, if (!span) { /* l2ptr is not valid */ - qemu_log_mask(LOG_GUEST_ERROR, - "invalid sid=%d (L1STD span=0)\n", sid); + if (!event->inval_ste_allowed) { + qemu_log_mask(LOG_GUEST_ERROR, + "invalid sid=%d (L1STD span=0)\n", sid); + } event->type = SMMU_EVT_C_BAD_STREAMID; return -EINVAL; } @@ -XXX,XX +XXX,XX @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr, SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu); SMMUv3State *s = sdev->smmu; uint32_t sid = smmu_get_sid(sdev); - SMMUEventInfo event = {.type = SMMU_EVT_NONE, .sid = sid}; + SMMUEventInfo event = {.type = SMMU_EVT_NONE, + .sid = sid, + .inval_ste_allowed = false}; SMMUPTWEventInfo ptw_info = {}; SMMUTranslationStatus status; SMMUState *bs = ARM_SMMU(s); @@ -XXX,XX +XXX,XX @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr, dma_addr_t iova) { SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu); - SMMUEventInfo event = {}; + SMMUEventInfo event = {.inval_ste_allowed = true}; SMMUTransTableInfo *tt; SMMUTransCfg *cfg; IOMMUTLBEntry entry; cfg = smmuv3_get_config(sdev, &event); if (!cfg) { - qemu_log_mask(LOG_GUEST_ERROR, - "%s error decoding the configuration for iommu mr=%s\n", - __func__, mr->parent_obj.name); return; } -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> The previous simplification got the order of operands to the subtraction wrong. Since the 64-bit product is the subtrahend, we must use a 64-bit subtract to properly compute the borrow from the low-part of the product. Fixes: 5f8cd06ebcf5 ("target/arm: Simplify SMMLA, SMMLAR, SMMLS, SMMLSR") Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com> Message-id: 20190829013258.16102-1-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) if (rd != 15) { tmp3 = load_reg(s, rd); if (insn & (1 << 6)) { - tcg_gen_sub_i32(tmp, tmp, tmp3); + /* + * For SMMLS, we need a 64-bit subtract. + * Borrow caused by a non-zero multiplicand + * lowpart, and the correct result lowpart + * for rounding. + */ + TCGv_i32 zero = tcg_const_i32(0); + tcg_gen_sub2_i32(tmp2, tmp, zero, tmp3, + tmp2, tmp); + tcg_temp_free_i32(zero); } else { tcg_gen_add_i32(tmp, tmp, tmp3); } @@ -XXX,XX +XXX,XX @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn) if (insn & (1 << 20)) { tcg_gen_add_i32(tmp, tmp, tmp3); } else { - tcg_gen_sub_i32(tmp, tmp, tmp3); + /* + * For SMMLS, we need a 64-bit subtract. + * Borrow caused by a non-zero multiplicand lowpart, + * and the correct result lowpart for rounding. + */ + TCGv_i32 zero = tcg_const_i32(0); + tcg_gen_sub2_i32(tmp2, tmp, zero, tmp3, tmp2, tmp); + tcg_temp_free_i32(zero); } tcg_temp_free_i32(tmp3); } -- 2.20.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> Commit ba1ba5cca introduce the ARM_CPU_TYPE_NAME() macro. Unify the code base by use it in all places. Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20190823143249.8096-2-philmd@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/allwinner-a10.c | 3 ++- hw/arm/cubieboard.c | 3 ++- hw/arm/digic.c | 3 ++- hw/arm/fsl-imx25.c | 2 +- hw/arm/fsl-imx31.c | 2 +- hw/arm/fsl-imx6.c | 3 ++- hw/arm/fsl-imx6ul.c | 2 +- hw/arm/xlnx-zynqmp.c | 8 ++++---- 8 files changed, 15 insertions(+), 11 deletions(-) diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/allwinner-a10.c +++ b/hw/arm/allwinner-a10.c @@ -XXX,XX +XXX,XX @@ static void aw_a10_init(Object *obj) AwA10State *s = AW_A10(obj); object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu), - "cortex-a8-" TYPE_ARM_CPU, &error_abort, NULL); + ARM_CPU_TYPE_NAME("cortex-a8"), + &error_abort, NULL); sysbus_init_child_obj(obj, "intc", &s->intc, sizeof(s->intc), TYPE_AW_A10_PIC); diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/cubieboard.c +++ b/hw/arm/cubieboard.c @@ -XXX,XX +XXX,XX @@ static void cubieboard_init(MachineState *machine) static void cubieboard_machine_init(MachineClass *mc) { - mc->desc = "cubietech cubieboard"; + mc->desc = "cubietech cubieboard (Cortex-A9)"; + mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a9"); mc->init = cubieboard_init; mc->block_default_type = IF_IDE; mc->units_per_default_bus = 1; diff --git a/hw/arm/digic.c b/hw/arm/digic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/digic.c +++ b/hw/arm/digic.c @@ -XXX,XX +XXX,XX @@ static void digic_init(Object *obj) int i; object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu), - "arm946-" TYPE_ARM_CPU, &error_abort, NULL); + ARM_CPU_TYPE_NAME("arm946"), + &error_abort, NULL); for (i = 0; i < DIGIC4_NB_TIMERS; i++) { #define DIGIC_TIMER_NAME_MLEN 11 diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/fsl-imx25.c +++ b/hw/arm/fsl-imx25.c @@ -XXX,XX +XXX,XX @@ static void fsl_imx25_init(Object *obj) FslIMX25State *s = FSL_IMX25(obj); int i; - object_initialize(&s->cpu, sizeof(s->cpu), "arm926-" TYPE_ARM_CPU); + object_initialize(&s->cpu, sizeof(s->cpu), ARM_CPU_TYPE_NAME("arm926")); sysbus_init_child_obj(obj, "avic", &s->avic, sizeof(s->avic), TYPE_IMX_AVIC); diff --git a/hw/arm/fsl-imx31.c b/hw/arm/fsl-imx31.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/fsl-imx31.c +++ b/hw/arm/fsl-imx31.c @@ -XXX,XX +XXX,XX @@ static void fsl_imx31_init(Object *obj) FslIMX31State *s = FSL_IMX31(obj); int i; - object_initialize(&s->cpu, sizeof(s->cpu), "arm1136-" TYPE_ARM_CPU); + object_initialize(&s->cpu, sizeof(s->cpu), ARM_CPU_TYPE_NAME("arm1136")); sysbus_init_child_obj(obj, "avic", &s->avic, sizeof(s->avic), TYPE_IMX_AVIC); diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/fsl-imx6.c +++ b/hw/arm/fsl-imx6.c @@ -XXX,XX +XXX,XX @@ static void fsl_imx6_init(Object *obj) for (i = 0; i < MIN(ms->smp.cpus, FSL_IMX6_NUM_CPUS); i++) { snprintf(name, NAME_SIZE, "cpu%d", i); object_initialize_child(obj, name, &s->cpu[i], sizeof(s->cpu[i]), - "cortex-a9-" TYPE_ARM_CPU, &error_abort, NULL); + ARM_CPU_TYPE_NAME("cortex-a9"), + &error_abort, NULL); } sysbus_init_child_obj(obj, "a9mpcore", &s->a9mpcore, sizeof(s->a9mpcore), diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/fsl-imx6ul.c +++ b/hw/arm/fsl-imx6ul.c @@ -XXX,XX +XXX,XX @@ static void fsl_imx6ul_init(Object *obj) int i; object_initialize_child(obj, "cpu0", &s->cpu, sizeof(s->cpu), - "cortex-a7-" TYPE_ARM_CPU, &error_abort, NULL); + ARM_CPU_TYPE_NAME("cortex-a7"), &error_abort, NULL); /* * A7MPCORE diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/xlnx-zynqmp.c +++ b/hw/arm/xlnx-zynqmp.c @@ -XXX,XX +XXX,XX @@ static void xlnx_zynqmp_create_rpu(MachineState *ms, XlnxZynqMPState *s, object_initialize_child(OBJECT(&s->rpu_cluster), "rpu-cpu[*]", &s->rpu_cpu[i], sizeof(s->rpu_cpu[i]), - "cortex-r5f-" TYPE_ARM_CPU, &error_abort, - NULL); + ARM_CPU_TYPE_NAME("cortex-r5f"), + &error_abort, NULL); name = object_get_canonical_path_component(OBJECT(&s->rpu_cpu[i])); if (strcmp(name, boot_cpu)) { @@ -XXX,XX +XXX,XX @@ static void xlnx_zynqmp_init(Object *obj) for (i = 0; i < num_apus; i++) { object_initialize_child(OBJECT(&s->apu_cluster), "apu-cpu[*]", &s->apu_cpu[i], sizeof(s->apu_cpu[i]), - "cortex-a53-" TYPE_ARM_CPU, &error_abort, - NULL); + ARM_CPU_TYPE_NAME("cortex-a53"), + &error_abort, NULL); } sysbus_init_child_obj(obj, "gic", &s->gic, sizeof(s->gic), -- 2.20.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> As explained in commit aff39be0ed97: Both functions, object_initialize() and object_property_add_child() increase the reference counter of the new object, so one of the references has to be dropped afterwards to get the reference counting right. Otherwise the child object will not be properly cleaned up when the parent gets destroyed. Thus let's use now object_initialize_child() instead to get the reference counting here right. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20190823143249.8096-3-philmd@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/mcimx7d-sabre.c | 9 ++++----- hw/arm/mps2-tz.c | 15 +++++++-------- hw/arm/musca.c | 9 +++++---- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/hw/arm/mcimx7d-sabre.c b/hw/arm/mcimx7d-sabre.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/mcimx7d-sabre.c +++ b/hw/arm/mcimx7d-sabre.c @@ -XXX,XX +XXX,XX @@ static void mcimx7d_sabre_init(MachineState *machine) { static struct arm_boot_info boot_info; MCIMX7Sabre *s = g_new0(MCIMX7Sabre, 1); - Object *soc; int i; if (machine->ram_size > FSL_IMX7_MMDC_SIZE) { @@ -XXX,XX +XXX,XX @@ static void mcimx7d_sabre_init(MachineState *machine) .nb_cpus = machine->smp.cpus, }; - object_initialize(&s->soc, sizeof(s->soc), TYPE_FSL_IMX7); - soc = OBJECT(&s->soc); - object_property_add_child(OBJECT(machine), "soc", soc, &error_fatal); - object_property_set_bool(soc, true, "realized", &error_fatal); + object_initialize_child(OBJECT(machine), "soc", + &s->soc, sizeof(s->soc), + TYPE_FSL_IMX7, &error_fatal, NULL); + object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal); memory_region_allocate_system_memory(&s->ram, NULL, "mcimx7d-sabre.ram", machine->ram_size); diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/mps2-tz.c +++ b/hw/arm/mps2-tz.c @@ -XXX,XX +XXX,XX @@ static void mps2tz_common_init(MachineState *machine) /* The sec_resp_cfg output from the IoTKit must be split into multiple * lines, one for each of the PPCs we create here, plus one per MSC. */ - object_initialize(&mms->sec_resp_splitter, sizeof(mms->sec_resp_splitter), - TYPE_SPLIT_IRQ); - object_property_add_child(OBJECT(machine), "sec-resp-splitter", - OBJECT(&mms->sec_resp_splitter), &error_abort); + object_initialize_child(OBJECT(machine), "sec-resp-splitter", + &mms->sec_resp_splitter, + sizeof(mms->sec_resp_splitter), + TYPE_SPLIT_IRQ, &error_abort, NULL); object_property_set_int(OBJECT(&mms->sec_resp_splitter), ARRAY_SIZE(mms->ppc) + ARRAY_SIZE(mms->msc), "num-lines", &error_fatal); @@ -XXX,XX +XXX,XX @@ static void mps2tz_common_init(MachineState *machine) * Tx, Rx and "combined" IRQs are sent to the NVIC separately. * Create the OR gate for this. */ - object_initialize(&mms->uart_irq_orgate, sizeof(mms->uart_irq_orgate), - TYPE_OR_IRQ); - object_property_add_child(OBJECT(mms), "uart-irq-orgate", - OBJECT(&mms->uart_irq_orgate), &error_abort); + object_initialize_child(OBJECT(mms), "uart-irq-orgate", + &mms->uart_irq_orgate, sizeof(mms->uart_irq_orgate), + TYPE_OR_IRQ, &error_abort, NULL); object_property_set_int(OBJECT(&mms->uart_irq_orgate), 10, "num-lines", &error_fatal); object_property_set_bool(OBJECT(&mms->uart_irq_orgate), true, diff --git a/hw/arm/musca.c b/hw/arm/musca.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/musca.c +++ b/hw/arm/musca.c @@ -XXX,XX +XXX,XX @@ static void musca_init(MachineState *machine) * The sec_resp_cfg output from the SSE-200 must be split into multiple * lines, one for each of the PPCs we create here. */ - object_initialize(&mms->sec_resp_splitter, sizeof(mms->sec_resp_splitter), - TYPE_SPLIT_IRQ); - object_property_add_child(OBJECT(machine), "sec-resp-splitter", - OBJECT(&mms->sec_resp_splitter), &error_fatal); + object_initialize_child(OBJECT(machine), "sec-resp-splitter", + &mms->sec_resp_splitter, + sizeof(mms->sec_resp_splitter), + TYPE_SPLIT_IRQ, &error_fatal, NULL); + object_property_set_int(OBJECT(&mms->sec_resp_splitter), ARRAY_SIZE(mms->ppc), "num-lines", &error_fatal); object_property_set_bool(OBJECT(&mms->sec_resp_splitter), true, -- 2.20.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> Both object_initialize() and qdev_set_parent_bus() increase the reference counter of the new object, so one of the references has to be dropped afterwards to get the reference counting right. In machine model code this refcount leak is not particularly problematic because (unlike devices) machines will never be created on demand via QMP, and they are never destroyed. But in any case let's use the new sysbus_init_child_obj() instead to get the reference counting here right. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20190823143249.8096-4-philmd@redhat.com [PMM: rewrote commit message] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/exynos4_boards.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/arm/exynos4_boards.c b/hw/arm/exynos4_boards.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/exynos4_boards.c +++ b/hw/arm/exynos4_boards.c @@ -XXX,XX +XXX,XX @@ exynos4_boards_init_common(MachineState *machine, exynos4_boards_init_ram(s, get_system_memory(), exynos4_board_ram_size[board_type]); - object_initialize(&s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC); - qdev_set_parent_bus(DEVICE(&s->soc), sysbus_get_default()); + sysbus_init_child_obj(OBJECT(machine), "soc", + &s->soc, sizeof(s->soc), TYPE_EXYNOS4210_SOC); object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal); -- 2.20.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> Child properties form the composition tree. All objects need to be a child of another object. Objects can only be a child of one object. Respect this with the i.MX SoC, to get a cleaner composition tree. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20190823143249.8096-5-philmd@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/fsl-imx25.c | 4 +++- hw/arm/fsl-imx31.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/fsl-imx25.c +++ b/hw/arm/fsl-imx25.c @@ -XXX,XX +XXX,XX @@ static void fsl_imx25_init(Object *obj) FslIMX25State *s = FSL_IMX25(obj); int i; - object_initialize(&s->cpu, sizeof(s->cpu), ARM_CPU_TYPE_NAME("arm926")); + object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu), + ARM_CPU_TYPE_NAME("arm926"), + &error_abort, NULL); sysbus_init_child_obj(obj, "avic", &s->avic, sizeof(s->avic), TYPE_IMX_AVIC); diff --git a/hw/arm/fsl-imx31.c b/hw/arm/fsl-imx31.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/fsl-imx31.c +++ b/hw/arm/fsl-imx31.c @@ -XXX,XX +XXX,XX @@ static void fsl_imx31_init(Object *obj) FslIMX31State *s = FSL_IMX31(obj); int i; - object_initialize(&s->cpu, sizeof(s->cpu), ARM_CPU_TYPE_NAME("arm1136")); + object_initialize_child(obj, "cpu", &s->cpu, sizeof(s->cpu), + ARM_CPU_TYPE_NAME("arm1136"), + &error_abort, NULL); sysbus_init_child_obj(obj, "avic", &s->avic, sizeof(s->avic), TYPE_IMX_AVIC); -- 2.20.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> As explained in commit aff39be0ed97: Both functions, object_initialize() and object_property_add_child() increase the reference counter of the new object, so one of the references has to be dropped afterwards to get the reference counting right. Otherwise the child object will not be properly cleaned up when the parent gets destroyed. Thus let's use now object_initialize_child() instead to get the reference counting here right. Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20190823143249.8096-6-philmd@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/dma/xilinx_axidma.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c index XXXXXXX..XXXXXXX 100644 --- a/hw/dma/xilinx_axidma.c +++ b/hw/dma/xilinx_axidma.c @@ -XXX,XX +XXX,XX @@ static void xilinx_axidma_init(Object *obj) XilinxAXIDMA *s = XILINX_AXI_DMA(obj); SysBusDevice *sbd = SYS_BUS_DEVICE(obj); - object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev), - TYPE_XILINX_AXI_DMA_DATA_STREAM); - object_initialize(&s->rx_control_dev, sizeof(s->rx_control_dev), - TYPE_XILINX_AXI_DMA_CONTROL_STREAM); - object_property_add_child(OBJECT(s), "axistream-connected-target", - (Object *)&s->rx_data_dev, &error_abort); - object_property_add_child(OBJECT(s), "axistream-control-connected-target", - (Object *)&s->rx_control_dev, &error_abort); + object_initialize_child(OBJECT(s), "axistream-connected-target", + &s->rx_data_dev, sizeof(s->rx_data_dev), + TYPE_XILINX_AXI_DMA_DATA_STREAM, &error_abort, + NULL); + object_initialize_child(OBJECT(s), "axistream-control-connected-target", + &s->rx_control_dev, sizeof(s->rx_control_dev), + TYPE_XILINX_AXI_DMA_CONTROL_STREAM, &error_abort, + NULL); sysbus_init_irq(sbd, &s->streams[0].irq); sysbus_init_irq(sbd, &s->streams[1].irq); -- 2.20.1 From: Philippe Mathieu-Daudé <philmd@redhat.com> As explained in commit aff39be0ed97: Both functions, object_initialize() and object_property_add_child() increase the reference counter of the new object, so one of the references has to be dropped afterwards to get the reference counting right. Otherwise the child object will not be properly cleaned up when the parent gets destroyed. Thus let's use now object_initialize_child() instead to get the reference counting here right. Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20190823143249.8096-7-philmd@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/net/xilinx_axienet.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c index XXXXXXX..XXXXXXX 100644 --- a/hw/net/xilinx_axienet.c +++ b/hw/net/xilinx_axienet.c @@ -XXX,XX +XXX,XX @@ static void xilinx_enet_init(Object *obj) XilinxAXIEnet *s = XILINX_AXI_ENET(obj); SysBusDevice *sbd = SYS_BUS_DEVICE(obj); - object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev), - TYPE_XILINX_AXI_ENET_DATA_STREAM); - object_initialize(&s->rx_control_dev, sizeof(s->rx_control_dev), - TYPE_XILINX_AXI_ENET_CONTROL_STREAM); - object_property_add_child(OBJECT(s), "axistream-connected-target", - (Object *)&s->rx_data_dev, &error_abort); - object_property_add_child(OBJECT(s), "axistream-control-connected-target", - (Object *)&s->rx_control_dev, &error_abort); - + object_initialize_child(OBJECT(s), "axistream-connected-target", + &s->rx_data_dev, sizeof(s->rx_data_dev), + TYPE_XILINX_AXI_ENET_DATA_STREAM, &error_abort, + NULL); + object_initialize_child(OBJECT(s), "axistream-control-connected-target", + &s->rx_control_dev, sizeof(s->rx_control_dev), + TYPE_XILINX_AXI_ENET_CONTROL_STREAM, &error_abort, + NULL); sysbus_init_irq(sbd, &s->irq); memory_region_init_io(&s->iomem, OBJECT(s), &enet_ops, s, "enet", 0x40000); -- 2.20.1 From: Alex Bennée <alex.bennee@linaro.org> Commit a5e0b3311 removed these in favour of querying machine properties. Remove the extern declarations as well. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20190828165307.18321-6-alex.bennee@linaro.org Cc: Like Xu <like.xu@linux.intel.com> Message-Id: <20190711130546.18578-1-alex.bennee@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/sysemu/sysemu.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index XXXXXXX..XXXXXXX 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -XXX,XX +XXX,XX @@ extern const char *keyboard_layout; extern int win2k_install_hack; extern int alt_grab; extern int ctrl_grab; -extern int smp_cpus; -extern unsigned int max_cpus; extern int cursor_hide; extern int graphic_rotate; extern int no_quit; -- 2.20.1 From: "Emilio G. Cota" <cota@braap.org> Afterwise is "wise after the fact", as in "hindsight". Here we meant "afterwards" (as in "subsequently"). Fix it. Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Emilio G. Cota <cota@braap.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-id: 20190828165307.18321-7-alex.bennee@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tcg/README | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tcg/README b/tcg/README index XXXXXXX..XXXXXXX 100644 --- a/tcg/README +++ b/tcg/README @@ -XXX,XX +XXX,XX @@ This can be overridden using the following function modifiers: canonical locations before calling the helper. - TCG_CALL_NO_WRITE_GLOBALS means that the helper does not modify any globals. They will only be saved to their canonical location before calling helpers, - but they won't be reloaded afterwise. + but they won't be reloaded afterwards. - TCG_CALL_NO_SIDE_EFFECTS means that the call to the function is removed if the return value is not used. -- 2.20.1 From: "Emilio G. Cota" <cota@braap.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Emilio G. Cota <cota@braap.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-id: 20190828165307.18321-8-alex.bennee@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- accel/tcg/atomic_template.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h index XXXXXXX..XXXXXXX 100644 --- a/accel/tcg/atomic_template.h +++ b/accel/tcg/atomic_template.h @@ -XXX,XX +XXX,XX @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, #define GEN_ATOMIC_HELPER(X) \ ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \ - ABI_TYPE val EXTRA_ARGS) \ + ABI_TYPE val EXTRA_ARGS) \ { \ ATOMIC_MMU_DECLS; \ DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP; \ -- 2.20.1 From: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-id: 20190828165307.18321-10-alex.bennee@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/exec/cpu-defs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/cpu-defs.h +++ b/include/exec/cpu-defs.h @@ -XXX,XX +XXX,XX @@ typedef struct CPUTLB { } CPUTLB; #endif /* !CONFIG_USER_ONLY && CONFIG_TCG */ /* - * This structure must be placed in ArchCPU immedately + * This structure must be placed in ArchCPU immediately * before CPUArchState, as a field named "neg". */ typedef struct CPUNegativeOffsetState { -- 2.20.1 The function neon_store_reg32() doesn't free the TCG temp that it is passed, so the caller must do that. We got this right in most places but forgot to free the TCG temps in trans_VMOV_64_sp(). Cc: qemu-stable@nongnu.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-id: 20190827121931.26836-1-peter.maydell@linaro.org --- target/arm/translate-vfp.inc.c | 2 ++ 1 file changed, 2 insertions(+) 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_VMOV_64_sp(DisasContext *s, arg_VMOV_64_sp *a) /* gpreg to fpreg */ tmp = load_reg(s, a->rt); neon_store_reg32(tmp, a->vm); + tcg_temp_free_i32(tmp); tmp = load_reg(s, a->rt2); neon_store_reg32(tmp, a->vm + 1); + tcg_temp_free_i32(tmp); } return true; -- 2.20.1 An attempt to do an exception-return (branch to one of the magic addresses) in linux-user mode for M-profile should behave like a normal branch, because linux-user mode is always going to be in 'handler' mode. This used to work, but we broke it when we added support for the M-profile security extension in commit d02a8698d7ae2bfed. In that commit we allowed even handler-mode calls to magic return values to be checked for and dealt with by causing an EXCP_EXCEPTION_EXIT exception to be taken, because this is needed for the FNC_RETURN return-from-non-secure-function-call handling. For system mode we added a check in do_v7m_exception_exit() to make any spurious calls from Handler mode behave correctly, but forgot that linux-user mode would also be affected. How an attempted return-from-non-secure-function-call in linux-user mode should be handled is not clear -- on real hardware it would result in return to secure code (not to the Linux kernel) which could then handle the error in any way it chose. For QEMU we take the simple approach of treating this erroneous return the same way it would be handled on a CPU without the security extensions -- treat it as a normal branch. The upshot of all this is that for linux-user mode we should never do any of the bx_excret magic, so the code change is simple. This ought to be a weird corner case that only affects broken guest code (because Linux user processes should never be attempting to do exception returns or NS function returns), except that the code that assigns addresses in RAM for the process and stack in our linux-user code does not attempt to avoid this magic address range, so legitimate code attempting to return to a trampoline routine on the stack can fall into this case. This change fixes those programs, but we should also look at restricting the range of memory we use for M-profile linux-user guests to the area that would be real RAM in hardware. Cc: qemu-stable@nongnu.org Reported-by: Christophe Lyon <christophe.lyon@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 20190822131534.16602-1-peter.maydell@linaro.org Fixes: https://bugs.launchpad.net/qemu/+bug/1840922 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/translate.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static inline void gen_bx(DisasContext *s, TCGv_i32 var) store_cpu_field(var, thumb); } -/* Set PC and Thumb state from var. var is marked as dead. +/* + * Set PC and Thumb state from var. var is marked as dead. * For M-profile CPUs, include logic to detect exception-return * branches and handle them. This is needed for Thumb POP/LDM to PC, LDR to PC, * and BX reg, and no others, and happens only for code in Handler mode. + * The Security Extension also requires us to check for the FNC_RETURN + * which signals a function return from non-secure state; this can happen + * in both Handler and Thread mode. + * To avoid having to do multiple comparisons in inline generated code, + * we make the check we do here loose, so it will match for EXC_RETURN + * in Thread mode. For system emulation do_v7m_exception_exit() checks + * for these spurious cases and returns without doing anything (giving + * the same behaviour as for a branch to a non-magic address). + * + * In linux-user mode it is unclear what the right behaviour for an + * attempted FNC_RETURN should be, because in real hardware this will go + * directly to Secure code (ie not the Linux kernel) which will then treat + * the error in any way it chooses. For QEMU we opt to make the FNC_RETURN + * attempt behave the way it would on a CPU without the security extension, + * which is to say "like a normal branch". That means we can simply treat + * all branches as normal with no magic address behaviour. */ static inline void gen_bx_excret(DisasContext *s, TCGv_i32 var) { @@ -XXX,XX +XXX,XX @@ static inline void gen_bx_excret(DisasContext *s, TCGv_i32 var) * s->base.is_jmp that we need to do the rest of the work later. */ gen_bx(s, var); +#ifndef CONFIG_USER_ONLY if (arm_dc_feature(s, ARM_FEATURE_M_SECURITY) || (s->v7m_handler_mode && arm_dc_feature(s, ARM_FEATURE_M))) { s->base.is_jmp = DISAS_BX_EXCRET; } +#endif } static inline void gen_bx_excret_final_code(DisasContext *s) -- 2.20.1 The following changes since commit 7e7eb9f852a46b51a71ae9d82590b2e4d28827ee: Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2021-01-28' into staging (2021-01-28 22:43:18 +0000) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20210129 for you to fetch changes up to 11749122e1a86866591306d43603d2795a3dea1a: hw/arm/stellaris: Remove board-creation reset of STELLARIS_SYS (2021-01-29 10:47:29 +0000) ---------------------------------------------------------------- target-arm queue: * Implement ID_PFR2 * Conditionalize DBGDIDR * rename xlnx-zcu102.canbusN properties * provide powerdown/reset mechanism for secure firmware on 'virt' board * hw/misc: Fix arith overflow in NPCM7XX PWM module * target/arm: Replace magic value by MMU_DATA_LOAD definition * configure: fix preadv errors on Catalina macOS with new XCode * Various configure and other cleanups in preparation for iOS support * hvf: Add hypervisor entitlement to output binaries (needed for Big Sur) * Implement pvpanic-pci device * Convert the CMSDK timer devices to the Clock framework ---------------------------------------------------------------- Alexander Graf (1): hvf: Add hypervisor entitlement to output binaries Hao Wu (1): hw/misc: Fix arith overflow in NPCM7XX PWM module Joelle van Dyne (7): configure: cross-compiling with empty cross_prefix osdep: build with non-working system() function darwin: remove redundant dependency declaration darwin: fix cross-compiling for Darwin configure: cross compile should use x86_64 cpu_family darwin: detect CoreAudio for build darwin: remove 64-bit build detection on 32-bit OS Maxim Uvarov (3): hw: gpio: implement gpio-pwr driver for qemu reset/poweroff arm-virt: refactor gpios creation arm-virt: add secure pl061 for reset/power down Mihai Carabas (4): hw/misc/pvpanic: split-out generic and bus dependent code hw/misc/pvpanic: add PCI interface support pvpanic : update pvpanic spec document tests/qtest: add a test case for pvpanic-pci Paolo Bonzini (1): arm: rename xlnx-zcu102.canbusN properties Peter Maydell (26): configure: Move preadv check to meson.build ptimer: Add new ptimer_set_period_from_clock() function clock: Add new clock_has_source() function tests: Add a simple test of the CMSDK APB timer tests: Add a simple test of the CMSDK APB watchdog tests: Add a simple test of the CMSDK APB dual timer hw/timer/cmsdk-apb-timer: Rename CMSDKAPBTIMER struct to CMSDKAPBTimer hw/timer/cmsdk-apb-timer: Add Clock input hw/timer/cmsdk-apb-dualtimer: Add Clock input hw/watchdog/cmsdk-apb-watchdog: Add Clock input hw/arm/armsse: Rename "MAINCLK" property to "MAINCLK_FRQ" hw/arm/armsse: Wire up clocks hw/arm/mps2: Inline CMSDK_APB_TIMER creation hw/arm/mps2: Create and connect SYSCLK Clock hw/arm/mps2-tz: Create and connect ARMSSE Clocks hw/arm/musca: Create and connect ARMSSE Clocks hw/arm/stellaris: Convert SSYS to QOM device hw/arm/stellaris: Create Clock input for watchdog hw/timer/cmsdk-apb-timer: Convert to use Clock input hw/timer/cmsdk-apb-dualtimer: Convert to use Clock input hw/watchdog/cmsdk-apb-watchdog: Convert to use Clock input tests/qtest/cmsdk-apb-watchdog-test: Test clock changes hw/arm/armsse: Use Clock to set system_clock_scale arm: Don't set freq properties on CMSDK timer, dualtimer, watchdog, ARMSSE arm: Remove frq properties on CMSDK timer, dualtimer, watchdog, ARMSSE hw/arm/stellaris: Remove board-creation reset of STELLARIS_SYS Philippe Mathieu-Daudé (1): target/arm: Replace magic value by MMU_DATA_LOAD definition Richard Henderson (2): target/arm: Implement ID_PFR2 target/arm: Conditionalize DBGDIDR docs/devel/clocks.rst | 16 +++ docs/specs/pci-ids.txt | 1 + docs/specs/pvpanic.txt | 13 ++- docs/system/arm/virt.rst | 2 + configure | 78 ++++++++------ meson.build | 34 ++++++- include/hw/arm/armsse.h | 14 ++- include/hw/arm/virt.h | 2 + include/hw/clock.h | 15 +++ include/hw/misc/pvpanic.h | 24 ++++- include/hw/pci/pci.h | 1 + include/hw/ptimer.h | 22 ++++ include/hw/timer/cmsdk-apb-dualtimer.h | 5 +- include/hw/timer/cmsdk-apb-timer.h | 34 ++----- include/hw/watchdog/cmsdk-apb-watchdog.h | 5 +- include/qemu/osdep.h | 12 +++ include/qemu/typedefs.h | 1 + target/arm/cpu.h | 1 + hw/arm/armsse.c | 48 ++++++--- hw/arm/mps2-tz.c | 14 ++- hw/arm/mps2.c | 28 ++++- hw/arm/musca.c | 13 ++- hw/arm/stellaris.c | 170 +++++++++++++++++++++++-------- hw/arm/virt.c | 111 ++++++++++++++++---- hw/arm/xlnx-zcu102.c | 4 +- hw/core/ptimer.c | 34 +++++++ hw/gpio/gpio_pwr.c | 70 +++++++++++++ hw/misc/npcm7xx_pwm.c | 23 ++++- hw/misc/pvpanic-isa.c | 94 +++++++++++++++++ hw/misc/pvpanic-pci.c | 94 +++++++++++++++++ hw/misc/pvpanic.c | 85 ++-------------- hw/timer/cmsdk-apb-dualtimer.c | 53 +++++++--- hw/timer/cmsdk-apb-timer.c | 55 +++++----- hw/watchdog/cmsdk-apb-watchdog.c | 29 ++++-- target/arm/helper.c | 27 +++-- target/arm/kvm64.c | 2 + tests/qtest/cmsdk-apb-dualtimer-test.c | 130 +++++++++++++++++++++++ tests/qtest/cmsdk-apb-timer-test.c | 75 ++++++++++++++ tests/qtest/cmsdk-apb-watchdog-test.c | 131 ++++++++++++++++++++++++ tests/qtest/npcm7xx_pwm-test.c | 4 +- tests/qtest/pvpanic-pci-test.c | 94 +++++++++++++++++ tests/qtest/xlnx-can-test.c | 30 +++--- MAINTAINERS | 3 + accel/hvf/entitlements.plist | 8 ++ hw/arm/Kconfig | 1 + hw/gpio/Kconfig | 3 + hw/gpio/meson.build | 1 + hw/i386/Kconfig | 2 +- hw/misc/Kconfig | 12 ++- hw/misc/meson.build | 4 +- scripts/entitlement.sh | 13 +++ tests/qtest/meson.build | 6 +- 52 files changed, 1432 insertions(+), 319 deletions(-) create mode 100644 hw/gpio/gpio_pwr.c create mode 100644 hw/misc/pvpanic-isa.c create mode 100644 hw/misc/pvpanic-pci.c create mode 100644 tests/qtest/cmsdk-apb-dualtimer-test.c create mode 100644 tests/qtest/cmsdk-apb-timer-test.c create mode 100644 tests/qtest/cmsdk-apb-watchdog-test.c create mode 100644 tests/qtest/pvpanic-pci-test.c create mode 100644 accel/hvf/entitlements.plist create mode 100755 scripts/entitlement.sh From: Richard Henderson <richard.henderson@linaro.org> This was defined at some point before ARMv8.4, and will shortly be used by new processor descriptions. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20210120204400.1056582-1-richard.henderson@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 1 + target/arm/helper.c | 4 ++-- target/arm/kvm64.c | 2 ++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -XXX,XX +XXX,XX @@ struct ARMCPU { uint32_t id_mmfr4; uint32_t id_pfr0; uint32_t id_pfr1; + uint32_t id_pfr2; uint32_t mvfr0; uint32_t mvfr1; uint32_t mvfr2; diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ void register_cp_regs_for_features(ARMCPU *cpu) .access = PL1_R, .type = ARM_CP_CONST, .accessfn = access_aa64_tid3, .resetvalue = 0 }, - { .name = "MVFR4_EL1_RESERVED", .state = ARM_CP_STATE_AA64, + { .name = "ID_PFR2", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 3, .opc2 = 4, .access = PL1_R, .type = ARM_CP_CONST, .accessfn = access_aa64_tid3, - .resetvalue = 0 }, + .resetvalue = cpu->isar.id_pfr2 }, { .name = "MVFR5_EL1_RESERVED", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 3, .opc2 = 5, .access = PL1_R, .type = ARM_CP_CONST, diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -XXX,XX +XXX,XX @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) ARM64_SYS_REG(3, 0, 0, 1, 0)); err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_pfr1, ARM64_SYS_REG(3, 0, 0, 1, 1)); + err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_pfr2, + ARM64_SYS_REG(3, 0, 0, 3, 4)); err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_dfr0, ARM64_SYS_REG(3, 0, 0, 1, 2)); err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_mmfr0, -- 2.20.1 From: Richard Henderson <richard.henderson@linaro.org> Only define the register if it exists for the cpu. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20210120031656.737646-1-richard.henderson@linaro.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ static void define_debug_regs(ARMCPU *cpu) */ int i; int wrps, brps, ctx_cmps; - ARMCPRegInfo dbgdidr = { - .name = "DBGDIDR", .cp = 14, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 0, - .access = PL0_R, .accessfn = access_tda, - .type = ARM_CP_CONST, .resetvalue = cpu->isar.dbgdidr, - }; + + /* + * The Arm ARM says DBGDIDR is optional and deprecated if EL1 cannot + * use AArch32. Given that bit 15 is RES1, if the value is 0 then + * the register must not exist for this cpu. + */ + if (cpu->isar.dbgdidr != 0) { + ARMCPRegInfo dbgdidr = { + .name = "DBGDIDR", .cp = 14, .crn = 0, .crm = 0, + .opc1 = 0, .opc2 = 0, + .access = PL0_R, .accessfn = access_tda, + .type = ARM_CP_CONST, .resetvalue = cpu->isar.dbgdidr, + }; + define_one_arm_cp_reg(cpu, &dbgdidr); + } /* Note that all these register fields hold "number of Xs minus 1". */ brps = arm_num_brps(cpu); @@ -XXX,XX +XXX,XX @@ static void define_debug_regs(ARMCPU *cpu) assert(ctx_cmps <= brps); - define_one_arm_cp_reg(cpu, &dbgdidr); define_arm_cp_regs(cpu, debug_cp_reginfo); if (arm_feature(&cpu->env, ARM_FEATURE_LPAE)) { -- 2.20.1 From: Paolo Bonzini <pbonzini@redhat.com> The properties to attach a CANBUS object to the xlnx-zcu102 machine have a period in them. We want to use periods in properties for compound QAPI types, and besides the "xlnx-zcu102." prefix is both unnecessary and different from any other machine property name. Remove it. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-id: 20210118162537.779542-1-pbonzini@redhat.com Reviewed-by: Vikram Garhwal <fnu.vikram@xilinx.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/xlnx-zcu102.c | 4 ++-- tests/qtest/xlnx-can-test.c | 30 +++++++++++++++--------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/xlnx-zcu102.c +++ b/hw/arm/xlnx-zcu102.c @@ -XXX,XX +XXX,XX @@ static void xlnx_zcu102_machine_instance_init(Object *obj) s->secure = false; /* Default to virt (EL2) being disabled */ s->virt = false; - object_property_add_link(obj, "xlnx-zcu102.canbus0", TYPE_CAN_BUS, + object_property_add_link(obj, "canbus0", TYPE_CAN_BUS, (Object **)&s->canbus[0], object_property_allow_set_link, 0); - object_property_add_link(obj, "xlnx-zcu102.canbus1", TYPE_CAN_BUS, + object_property_add_link(obj, "canbus1", TYPE_CAN_BUS, (Object **)&s->canbus[1], object_property_allow_set_link, 0); diff --git a/tests/qtest/xlnx-can-test.c b/tests/qtest/xlnx-can-test.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/xlnx-can-test.c +++ b/tests/qtest/xlnx-can-test.c @@ -XXX,XX +XXX,XX @@ static void test_can_bus(void) uint8_t can_timestamp = 1; QTestState *qts = qtest_init("-machine xlnx-zcu102" - " -object can-bus,id=canbus0" - " -machine xlnx-zcu102.canbus0=canbus0" - " -machine xlnx-zcu102.canbus1=canbus0" + " -object can-bus,id=canbus" + " -machine canbus0=canbus" + " -machine canbus1=canbus" ); /* Configure the CAN0 and CAN1. */ @@ -XXX,XX +XXX,XX @@ static void test_can_loopback(void) uint32_t status = 0; QTestState *qts = qtest_init("-machine xlnx-zcu102" - " -object can-bus,id=canbus0" - " -machine xlnx-zcu102.canbus0=canbus0" - " -machine xlnx-zcu102.canbus1=canbus0" + " -object can-bus,id=canbus" + " -machine canbus0=canbus" + " -machine canbus1=canbus" ); /* Configure the CAN0 in loopback mode. */ @@ -XXX,XX +XXX,XX @@ static void test_can_filter(void) uint8_t can_timestamp = 1; QTestState *qts = qtest_init("-machine xlnx-zcu102" - " -object can-bus,id=canbus0" - " -machine xlnx-zcu102.canbus0=canbus0" - " -machine xlnx-zcu102.canbus1=canbus0" + " -object can-bus,id=canbus" + " -machine canbus0=canbus" + " -machine canbus1=canbus" ); /* Configure the CAN0 and CAN1. */ @@ -XXX,XX +XXX,XX @@ static void test_can_sleepmode(void) uint8_t can_timestamp = 1; QTestState *qts = qtest_init("-machine xlnx-zcu102" - " -object can-bus,id=canbus0" - " -machine xlnx-zcu102.canbus0=canbus0" - " -machine xlnx-zcu102.canbus1=canbus0" + " -object can-bus,id=canbus" + " -machine canbus0=canbus" + " -machine canbus1=canbus" ); /* Configure the CAN0. */ @@ -XXX,XX +XXX,XX @@ static void test_can_snoopmode(void) uint8_t can_timestamp = 1; QTestState *qts = qtest_init("-machine xlnx-zcu102" - " -object can-bus,id=canbus0" - " -machine xlnx-zcu102.canbus0=canbus0" - " -machine xlnx-zcu102.canbus1=canbus0" + " -object can-bus,id=canbus" + " -machine canbus0=canbus" + " -machine canbus1=canbus" ); /* Configure the CAN0. */ -- 2.20.1 From: Maxim Uvarov <maxim.uvarov@linaro.org> Implement gpio-pwr driver to allow reboot and poweroff machine. This is simple driver with just 2 gpios lines. Current use case is to reboot and poweroff virt machine in secure mode. Secure pl066 gpio chip is needed for that. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> Reviewed-by: Hao Wu <wuhaotsh@google.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/gpio/gpio_pwr.c | 70 +++++++++++++++++++++++++++++++++++++++++++++ hw/gpio/Kconfig | 3 ++ hw/gpio/meson.build | 1 + 3 files changed, 74 insertions(+) create mode 100644 hw/gpio/gpio_pwr.c diff --git a/hw/gpio/gpio_pwr.c b/hw/gpio/gpio_pwr.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/hw/gpio/gpio_pwr.c @@ -XXX,XX +XXX,XX @@ +/* + * GPIO qemu power controller + * + * Copyright (c) 2020 Linaro Limited + * + * Author: Maxim Uvarov <maxim.uvarov@linaro.org> + * + * Virtual gpio driver which can be used on top of pl061 + * to reboot and shutdown qemu virtual machine. One of use + * case is gpio driver for secure world application (ARM + * Trusted Firmware.). + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +/* + * QEMU interface: + * two named input GPIO lines: + * 'reset' : when asserted, trigger system reset + * 'shutdown' : when asserted, trigger system shutdown + */ + +#include "qemu/osdep.h" +#include "hw/sysbus.h" +#include "sysemu/runstate.h" + +#define TYPE_GPIOPWR "gpio-pwr" +OBJECT_DECLARE_SIMPLE_TYPE(GPIO_PWR_State, GPIOPWR) + +struct GPIO_PWR_State { + SysBusDevice parent_obj; +}; + +static void gpio_pwr_reset(void *opaque, int n, int level) +{ + if (level) { + qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET); + } +} + +static void gpio_pwr_shutdown(void *opaque, int n, int level) +{ + if (level) { + qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); + } +} + +static void gpio_pwr_init(Object *obj) +{ + DeviceState *dev = DEVICE(obj); + + qdev_init_gpio_in_named(dev, gpio_pwr_reset, "reset", 1); + qdev_init_gpio_in_named(dev, gpio_pwr_shutdown, "shutdown", 1); +} + +static const TypeInfo gpio_pwr_info = { + .name = TYPE_GPIOPWR, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(GPIO_PWR_State), + .instance_init = gpio_pwr_init, +}; + +static void gpio_pwr_register_types(void) +{ + type_register_static(&gpio_pwr_info); +} + +type_init(gpio_pwr_register_types) diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/hw/gpio/Kconfig +++ b/hw/gpio/Kconfig @@ -XXX,XX +XXX,XX @@ config PL061 config GPIO_KEY bool +config GPIO_PWR + bool + config SIFIVE_GPIO bool diff --git a/hw/gpio/meson.build b/hw/gpio/meson.build index XXXXXXX..XXXXXXX 100644 --- a/hw/gpio/meson.build +++ b/hw/gpio/meson.build @@ -XXX,XX +XXX,XX @@ softmmu_ss.add(when: 'CONFIG_E500', if_true: files('mpc8xxx.c')) softmmu_ss.add(when: 'CONFIG_GPIO_KEY', if_true: files('gpio_key.c')) +softmmu_ss.add(when: 'CONFIG_GPIO_PWR', if_true: files('gpio_pwr.c')) softmmu_ss.add(when: 'CONFIG_MAX7310', if_true: files('max7310.c')) softmmu_ss.add(when: 'CONFIG_PL061', if_true: files('pl061.c')) softmmu_ss.add(when: 'CONFIG_PUV3', if_true: files('puv3_gpio.c')) -- 2.20.1 From: Maxim Uvarov <maxim.uvarov@linaro.org> No functional change. Just refactor code to better support secure and normal world gpios. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> Reviewed-by: Andrew Jones <drjones@redhat.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm/virt.c | 57 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -XXX,XX +XXX,XX @@ static void virt_powerdown_req(Notifier *n, void *opaque) } } -static void create_gpio(const VirtMachineState *vms) +static void create_gpio_keys(const VirtMachineState *vms, + DeviceState *pl061_dev, + uint32_t phandle) +{ + gpio_key_dev = sysbus_create_simple("gpio-key", -1, + qdev_get_gpio_in(pl061_dev, 3)); + + qemu_fdt_add_subnode(vms->fdt, "/gpio-keys"); + qemu_fdt_setprop_string(vms->fdt, "/gpio-keys", "compatible", "gpio-keys"); + qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#size-cells", 0); + qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#address-cells", 1); + + qemu_fdt_add_subnode(vms->fdt, "/gpio-keys/poweroff"); + qemu_fdt_setprop_string(vms->fdt, "/gpio-keys/poweroff", + "label", "GPIO Key Poweroff"); + qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys/poweroff", "linux,code", + KEY_POWER); + qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff", + "gpios", phandle, 3, 0); +} + +static void create_gpio_devices(const VirtMachineState *vms, int gpio, + MemoryRegion *mem) { char *nodename; DeviceState *pl061_dev; - hwaddr base = vms->memmap[VIRT_GPIO].base; - hwaddr size = vms->memmap[VIRT_GPIO].size; - int irq = vms->irqmap[VIRT_GPIO]; + hwaddr base = vms->memmap[gpio].base; + hwaddr size = vms->memmap[gpio].size; + int irq = vms->irqmap[gpio]; const char compat[] = "arm,pl061\0arm,primecell"; + SysBusDevice *s; - pl061_dev = sysbus_create_simple("pl061", base, - qdev_get_gpio_in(vms->gic, irq)); + pl061_dev = qdev_new("pl061"); + s = SYS_BUS_DEVICE(pl061_dev); + sysbus_realize_and_unref(s, &error_fatal); + memory_region_add_subregion(mem, base, sysbus_mmio_get_region(s, 0)); + sysbus_connect_irq(s, 0, 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_fdt_setprop_string(vms->fdt, nodename, "clock-names", "apb_pclk"); qemu_fdt_setprop_cell(vms->fdt, nodename, "phandle", phandle); - gpio_key_dev = sysbus_create_simple("gpio-key", -1, - qdev_get_gpio_in(pl061_dev, 3)); - qemu_fdt_add_subnode(vms->fdt, "/gpio-keys"); - qemu_fdt_setprop_string(vms->fdt, "/gpio-keys", "compatible", "gpio-keys"); - qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#size-cells", 0); - qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys", "#address-cells", 1); - - qemu_fdt_add_subnode(vms->fdt, "/gpio-keys/poweroff"); - qemu_fdt_setprop_string(vms->fdt, "/gpio-keys/poweroff", - "label", "GPIO Key Poweroff"); - qemu_fdt_setprop_cell(vms->fdt, "/gpio-keys/poweroff", "linux,code", - KEY_POWER); - qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff", - "gpios", phandle, 3, 0); g_free(nodename); + + /* Child gpio devices */ + create_gpio_keys(vms, pl061_dev, phandle); } static void create_virtio_devices(const VirtMachineState *vms) @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) if (has_ged && aarch64 && firmware_loaded && virt_is_acpi_enabled(vms)) { vms->acpi_dev = create_acpi_ged(vms); } else { - create_gpio(vms); + create_gpio_devices(vms, VIRT_GPIO, sysmem); } /* connect powerdown request */ -- 2.20.1 From: Maxim Uvarov <maxim.uvarov@linaro.org> Add secure pl061 for reset/power down machine from the secure world (Arm Trusted Firmware). Connect it with gpio-pwr driver. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> Reviewed-by: Andrew Jones <drjones@redhat.com> [PMM: Added mention of the new device to the documentation] Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- docs/system/arm/virt.rst | 2 ++ include/hw/arm/virt.h | 2 ++ hw/arm/virt.c | 56 +++++++++++++++++++++++++++++++++++++++- hw/arm/Kconfig | 1 + 4 files changed, 60 insertions(+), 1 deletion(-) diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/system/arm/virt.rst +++ b/docs/system/arm/virt.rst @@ -XXX,XX +XXX,XX @@ The virt board supports: - Secure-World-only devices if the CPU has TrustZone: - A second PL011 UART + - A second PL061 GPIO controller, with GPIO lines for triggering + a system reset or system poweroff - A secure flash memory - 16MB of secure RAM 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 @@ enum { VIRT_GPIO, VIRT_SECURE_UART, VIRT_SECURE_MEM, + VIRT_SECURE_GPIO, VIRT_PCDIMM_ACPI, VIRT_ACPI_GED, VIRT_NVDIMM_ACPI, @@ -XXX,XX +XXX,XX @@ struct VirtMachineClass { bool kvm_no_adjvtime; bool no_kvm_steal_time; bool acpi_expose_flash; + bool no_secure_gpio; }; struct 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 const MemMapEntry base_memmap[] = { [VIRT_ACPI_GED] = { 0x09080000, ACPI_GED_EVT_SEL_LEN }, [VIRT_NVDIMM_ACPI] = { 0x09090000, NVDIMM_ACPI_IO_LEN}, [VIRT_PVTIME] = { 0x090a0000, 0x00010000 }, + [VIRT_SECURE_GPIO] = { 0x090b0000, 0x00001000 }, [VIRT_MMIO] = { 0x0a000000, 0x00000200 }, /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ [VIRT_PLATFORM_BUS] = { 0x0c000000, 0x02000000 }, @@ -XXX,XX +XXX,XX @@ static void create_gpio_keys(const VirtMachineState *vms, "gpios", phandle, 3, 0); } +#define SECURE_GPIO_POWEROFF 0 +#define SECURE_GPIO_RESET 1 + +static void create_secure_gpio_pwr(const VirtMachineState *vms, + DeviceState *pl061_dev, + uint32_t phandle) +{ + DeviceState *gpio_pwr_dev; + + /* gpio-pwr */ + gpio_pwr_dev = sysbus_create_simple("gpio-pwr", -1, NULL); + + /* connect secure pl061 to gpio-pwr */ + qdev_connect_gpio_out(pl061_dev, SECURE_GPIO_RESET, + qdev_get_gpio_in_named(gpio_pwr_dev, "reset", 0)); + qdev_connect_gpio_out(pl061_dev, SECURE_GPIO_POWEROFF, + qdev_get_gpio_in_named(gpio_pwr_dev, "shutdown", 0)); + + qemu_fdt_add_subnode(vms->fdt, "/gpio-poweroff"); + qemu_fdt_setprop_string(vms->fdt, "/gpio-poweroff", "compatible", + "gpio-poweroff"); + qemu_fdt_setprop_cells(vms->fdt, "/gpio-poweroff", + "gpios", phandle, SECURE_GPIO_POWEROFF, 0); + qemu_fdt_setprop_string(vms->fdt, "/gpio-poweroff", "status", "disabled"); + qemu_fdt_setprop_string(vms->fdt, "/gpio-poweroff", "secure-status", + "okay"); + + qemu_fdt_add_subnode(vms->fdt, "/gpio-restart"); + qemu_fdt_setprop_string(vms->fdt, "/gpio-restart", "compatible", + "gpio-restart"); + qemu_fdt_setprop_cells(vms->fdt, "/gpio-restart", + "gpios", phandle, SECURE_GPIO_RESET, 0); + qemu_fdt_setprop_string(vms->fdt, "/gpio-restart", "status", "disabled"); + qemu_fdt_setprop_string(vms->fdt, "/gpio-restart", "secure-status", + "okay"); +} + static void create_gpio_devices(const VirtMachineState *vms, int gpio, MemoryRegion *mem) { @@ -XXX,XX +XXX,XX @@ static void create_gpio_devices(const VirtMachineState *vms, int gpio, qemu_fdt_setprop_string(vms->fdt, nodename, "clock-names", "apb_pclk"); qemu_fdt_setprop_cell(vms->fdt, nodename, "phandle", phandle); + if (gpio != VIRT_GPIO) { + /* Mark as not usable by the normal world */ + qemu_fdt_setprop_string(vms->fdt, nodename, "status", "disabled"); + qemu_fdt_setprop_string(vms->fdt, nodename, "secure-status", "okay"); + } g_free(nodename); /* Child gpio devices */ - create_gpio_keys(vms, pl061_dev, phandle); + if (gpio == VIRT_GPIO) { + create_gpio_keys(vms, pl061_dev, phandle); + } else { + create_secure_gpio_pwr(vms, pl061_dev, phandle); + } } static void create_virtio_devices(const VirtMachineState *vms) @@ -XXX,XX +XXX,XX @@ static void machvirt_init(MachineState *machine) create_gpio_devices(vms, VIRT_GPIO, sysmem); } + if (vms->secure && !vmc->no_secure_gpio) { + create_gpio_devices(vms, VIRT_SECURE_GPIO, secure_sysmem); + } + /* connect powerdown request */ vms->powerdown_notifier.notify = virt_powerdown_req; qemu_register_powerdown_notifier(&vms->powerdown_notifier); @@ -XXX,XX +XXX,XX @@ DEFINE_VIRT_MACHINE_AS_LATEST(6, 0) static void virt_machine_5_2_options(MachineClass *mc) { + VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc)); + virt_machine_6_0_options(mc); compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len); + vmc->no_secure_gpio = true; } DEFINE_VIRT_MACHINE(5, 2) diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -XXX,XX +XXX,XX @@ config ARM_VIRT select PL011 # UART select PL031 # RTC select PL061 # GPIO + select GPIO_PWR select PLATFORM_BUS select SMBIOS select VIRTIO_MMIO -- 2.20.1 From: Hao Wu <wuhaotsh@google.com> Fix potential overflow problem when calculating pwm_duty. 1. Ensure p->cmr and p->cnr to be from [0,65535], according to the hardware specification. 2. Changed duty to uint32_t. However, since MAX_DUTY * (p->cmr+1) can excceed UINT32_MAX, we convert them to uint64_t in computation and converted them back to uint32_t. (duty is guaranteed to be <= MAX_DUTY so it won't overflow.) Fixes: CID 1442342 Suggested-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Doug Evans <dje@google.com> Signed-off-by: Hao Wu <wuhaotsh@google.com> Message-id: 20210127011142.2122790-1-wuhaotsh@google.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/misc/npcm7xx_pwm.c | 23 +++++++++++++++++++---- tests/qtest/npcm7xx_pwm-test.c | 4 ++-- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/hw/misc/npcm7xx_pwm.c b/hw/misc/npcm7xx_pwm.c index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/npcm7xx_pwm.c +++ b/hw/misc/npcm7xx_pwm.c @@ -XXX,XX +XXX,XX @@ REG32(NPCM7XX_PWM_PWDR3, 0x50); #define NPCM7XX_CH_INV BIT(2) #define NPCM7XX_CH_MOD BIT(3) +#define NPCM7XX_MAX_CMR 65535 +#define NPCM7XX_MAX_CNR 65535 + /* Offset of each PWM channel's prescaler in the PPR register. */ static const int npcm7xx_ppr_base[] = { 0, 0, 8, 8 }; /* Offset of each PWM channel's clock selector in the CSR register. */ @@ -XXX,XX +XXX,XX @@ static uint32_t npcm7xx_pwm_calculate_freq(NPCM7xxPWM *p) static uint32_t npcm7xx_pwm_calculate_duty(NPCM7xxPWM *p) { - uint64_t duty; + uint32_t duty; if (p->running) { if (p->cnr == 0) { @@ -XXX,XX +XXX,XX @@ static uint32_t npcm7xx_pwm_calculate_duty(NPCM7xxPWM *p) } else if (p->cmr >= p->cnr) { duty = NPCM7XX_PWM_MAX_DUTY; } else { - duty = NPCM7XX_PWM_MAX_DUTY * (p->cmr + 1) / (p->cnr + 1); + duty = (uint64_t)NPCM7XX_PWM_MAX_DUTY * (p->cmr + 1) / (p->cnr + 1); } } else { duty = 0; @@ -XXX,XX +XXX,XX @@ static void npcm7xx_pwm_write(void *opaque, hwaddr offset, case A_NPCM7XX_PWM_CNR2: case A_NPCM7XX_PWM_CNR3: p = &s->pwm[npcm7xx_cnr_index(offset)]; - p->cnr = value; + if (value > NPCM7XX_MAX_CNR) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid cnr value: %u", __func__, value); + p->cnr = NPCM7XX_MAX_CNR; + } else { + p->cnr = value; + } npcm7xx_pwm_update_output(p); break; @@ -XXX,XX +XXX,XX @@ static void npcm7xx_pwm_write(void *opaque, hwaddr offset, case A_NPCM7XX_PWM_CMR2: case A_NPCM7XX_PWM_CMR3: p = &s->pwm[npcm7xx_cmr_index(offset)]; - p->cmr = value; + if (value > NPCM7XX_MAX_CMR) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid cmr value: %u", __func__, value); + p->cmr = NPCM7XX_MAX_CMR; + } else { + p->cmr = value; + } npcm7xx_pwm_update_output(p); break; diff --git a/tests/qtest/npcm7xx_pwm-test.c b/tests/qtest/npcm7xx_pwm-test.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/npcm7xx_pwm-test.c +++ b/tests/qtest/npcm7xx_pwm-test.c @@ -XXX,XX +XXX,XX @@ static uint64_t pwm_compute_freq(QTestState *qts, uint32_t ppr, uint32_t csr, static uint64_t pwm_compute_duty(uint32_t cnr, uint32_t cmr, bool inverted) { - uint64_t duty; + uint32_t duty; if (cnr == 0) { /* PWM is stopped. */ @@ -XXX,XX +XXX,XX @@ static uint64_t pwm_compute_duty(uint32_t cnr, uint32_t cmr, bool inverted) } else if (cmr >= cnr) { duty = MAX_DUTY; } else { - duty = MAX_DUTY * (cmr + 1) / (cnr + 1); + duty = (uint64_t)MAX_DUTY * (cmr + 1) / (cnr + 1); } if (inverted) { -- 2.20.1 From: Philippe Mathieu-Daudé <f4bug@amsat.org> cpu_get_phys_page_debug() uses 'DATA LOAD' MMU access type. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210127232822.3530782-1-f4bug@amsat.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -XXX,XX +XXX,XX @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr, *attrs = (MemTxAttrs) {}; - ret = get_phys_addr(env, addr, 0, mmu_idx, &phys_addr, + ret = get_phys_addr(env, addr, MMU_DATA_LOAD, mmu_idx, &phys_addr, attrs, &prot, &page_size, &fi, &cacheattrs); if (ret) { -- 2.20.1 Move the preadv availability check to meson.build. This is what we want to be doing for host-OS-feature-checks anyway, but it also fixes a problem with building for macOS with the most recent XCode SDK on a Catalina host. On that configuration, 'preadv()' is provided as a weak symbol, so that programs can be built with optional support for it and make a runtime availability check to see whether the preadv() they have is a working one or one which they must not call because it will runtime-assert. QEMU's configure test passes (unless you're building with --enable-werror) because the test program using preadv() compiles, but then QEMU crashes at runtime when preadv() is called, with errors like: dyld: lazy symbol binding failed: Symbol not found: _preadv Referenced from: /Users/pm215/src/qemu/./build/x86/tests/test-replication Expected in: /usr/lib/libSystem.B.dylib dyld: Symbol not found: _preadv Referenced from: /Users/pm215/src/qemu/./build/x86/tests/test-replication Expected in: /usr/lib/libSystem.B.dylib Meson's own function availability check has a special case for macOS which adds '-Wl,-no_weak_imports' to the compiler flags, which forces the test to require the real function, not the macOS-version-too-old stub. So this commit fixes the bug where macOS builds on Catalina currently require --disable-werror. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-id: 20210126155846.17109-1-peter.maydell@linaro.org --- configure | 16 ---------------- meson.build | 4 +++- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/configure b/configure index XXXXXXX..XXXXXXX 100755 --- a/configure +++ b/configure @@ -XXX,XX +XXX,XX @@ if compile_prog "" "" ; then iovec=yes fi -########################################## -# preadv probe -cat > $TMPC <<EOF -#include <sys/types.h> -#include <sys/uio.h> -#include <unistd.h> -int main(void) { return preadv(0, 0, 0, 0); } -EOF -preadv=no -if compile_prog "" "" ; then - preadv=yes -fi - ########################################## # fdt probe @@ -XXX,XX +XXX,XX @@ fi if test "$iovec" = "yes" ; then echo "CONFIG_IOVEC=y" >> $config_host_mak fi -if test "$preadv" = "yes" ; then - echo "CONFIG_PREADV=y" >> $config_host_mak -fi if test "$membarrier" = "yes" ; then echo "CONFIG_MEMBARRIER=y" >> $config_host_mak fi diff --git a/meson.build b/meson.build index XXXXXXX..XXXXXXX 100644 --- a/meson.build +++ b/meson.build @@ -XXX,XX +XXX,XX @@ config_host_data.set('HAVE_PTY_H', cc.has_header('pty.h')) config_host_data.set('HAVE_SYS_IOCCOM_H', cc.has_header('sys/ioccom.h')) config_host_data.set('HAVE_SYS_KCOV_H', cc.has_header('sys/kcov.h')) +config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix: '#include <sys/uio.h>')) + ignored = ['CONFIG_QEMU_INTERP_PREFIX'] # actually per-target arrays = ['CONFIG_AUDIO_DRIVERS', 'CONFIG_BDRV_RW_WHITELIST', 'CONFIG_BDRV_RO_WHITELIST'] strings = ['HOST_DSOSUF', 'CONFIG_IASL'] @@ -XXX,XX +XXX,XX @@ summary_info += {'PIE': get_option('b_pie')} summary_info += {'static build': config_host.has_key('CONFIG_STATIC')} summary_info += {'malloc trim support': has_malloc_trim} summary_info += {'membarrier': config_host.has_key('CONFIG_MEMBARRIER')} -summary_info += {'preadv support': config_host.has_key('CONFIG_PREADV')} +summary_info += {'preadv support': config_host_data.get('CONFIG_PREADV')} summary_info += {'fdatasync': config_host.has_key('CONFIG_FDATASYNC')} summary_info += {'madvise': config_host.has_key('CONFIG_MADVISE')} summary_info += {'posix_madvise': config_host.has_key('CONFIG_POSIX_MADVISE')} -- 2.20.1 From: Joelle van Dyne <j@getutm.app> The iOS toolchain does not use the host prefix naming convention. So we need to enable cross-compile options while allowing the PREFIX to be blank. Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Joelle van Dyne <j@getutm.app> Message-id: 20210126012457.39046-3-j@getutm.app Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- configure | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/configure b/configure index XXXXXXX..XXXXXXX 100755 --- a/configure +++ b/configure @@ -XXX,XX +XXX,XX @@ cpu="" iasl="iasl" interp_prefix="/usr/gnemul/qemu-%M" static="no" +cross_compile="no" cross_prefix="" audio_drv_list="" block_drv_rw_whitelist="" @@ -XXX,XX +XXX,XX @@ for opt do optarg=$(expr "x$opt" : 'x[^=]*=\(.*\)') case "$opt" in --cross-prefix=*) cross_prefix="$optarg" + cross_compile="yes" ;; --cc=*) CC="$optarg" ;; @@ -XXX,XX +XXX,XX @@ $(echo Deprecated targets: $deprecated_targets_list | \ --target-list-exclude=LIST exclude a set of targets from the default target-list Advanced options (experts only): - --cross-prefix=PREFIX use PREFIX for compile tools [$cross_prefix] + --cross-prefix=PREFIX use PREFIX for compile tools, PREFIX can be blank [$cross_prefix] --cc=CC use C compiler CC [$cc] --iasl=IASL use ACPI compiler IASL [$iasl] --host-cc=CC use C compiler CC [$host_cc] for code run at @@ -XXX,XX +XXX,XX @@ if has $sdl2_config; then fi echo "strip = [$(meson_quote $strip)]" >> $cross echo "windres = [$(meson_quote $windres)]" >> $cross -if test -n "$cross_prefix"; then +if test "$cross_compile" = "yes"; then cross_arg="--cross-file config-meson.cross" echo "[host_machine]" >> $cross if test "$mingw32" = "yes" ; then -- 2.20.1 From: Joelle van Dyne <j@getutm.app> Build without error on hosts without a working system(). If system() is called, return -1 with ENOSYS. Signed-off-by: Joelle van Dyne <j@getutm.app> Message-id: 20210126012457.39046-6-j@getutm.app Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- meson.build | 1 + include/qemu/osdep.h | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/meson.build b/meson.build index XXXXXXX..XXXXXXX 100644 --- a/meson.build +++ b/meson.build @@ -XXX,XX +XXX,XX @@ config_host_data.set('HAVE_DRM_H', cc.has_header('libdrm/drm.h')) config_host_data.set('HAVE_PTY_H', cc.has_header('pty.h')) config_host_data.set('HAVE_SYS_IOCCOM_H', cc.has_header('sys/ioccom.h')) config_host_data.set('HAVE_SYS_KCOV_H', cc.has_header('sys/kcov.h')) +config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>')) config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix: '#include <sys/uio.h>')) diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index XXXXXXX..XXXXXXX 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -XXX,XX +XXX,XX @@ static inline void qemu_thread_jit_write(void) {} static inline void qemu_thread_jit_execute(void) {} #endif +/** + * Platforms which do not support system() return ENOSYS + */ +#ifndef HAVE_SYSTEM_FUNCTION +#define system platform_does_not_support_system +static inline int platform_does_not_support_system(const char *command) +{ + errno = ENOSYS; + return -1; +} +#endif /* !HAVE_SYSTEM_FUNCTION */ + #endif -- 2.20.1 From: Joelle van Dyne <j@getutm.app> Meson will find CoreFoundation, IOKit, and Cocoa as needed. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Joelle van Dyne <j@getutm.app> Message-id: 20210126012457.39046-7-j@getutm.app Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- configure | 1 - 1 file changed, 1 deletion(-) diff --git a/configure b/configure index XXXXXXX..XXXXXXX 100755 --- a/configure +++ b/configure @@ -XXX,XX +XXX,XX @@ Darwin) fi audio_drv_list="coreaudio try-sdl" audio_possible_drivers="coreaudio sdl" - QEMU_LDFLAGS="-framework CoreFoundation -framework IOKit $QEMU_LDFLAGS" # Disable attempts to use ObjectiveC features in os/object.h since they # won't work when we're compiling with gcc as a C compiler. QEMU_CFLAGS="-DOS_OBJECT_USE_OBJC=0 $QEMU_CFLAGS" -- 2.20.1 From: Joelle van Dyne <j@getutm.app> Add objc to the Meson cross file as well as detection of Darwin. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Joelle van Dyne <j@getutm.app> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210126012457.39046-8-j@getutm.app Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- configure | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/configure b/configure index XXXXXXX..XXXXXXX 100755 --- a/configure +++ b/configure @@ -XXX,XX +XXX,XX @@ echo "cpp_link_args = [${LDFLAGS:+$(meson_quote $LDFLAGS)}]" >> $cross echo "[binaries]" >> $cross echo "c = [$(meson_quote $cc)]" >> $cross test -n "$cxx" && echo "cpp = [$(meson_quote $cxx)]" >> $cross +test -n "$objcc" && echo "objc = [$(meson_quote $objcc)]" >> $cross echo "ar = [$(meson_quote $ar)]" >> $cross echo "nm = [$(meson_quote $nm)]" >> $cross echo "pkgconfig = [$(meson_quote $pkg_config_exe)]" >> $cross @@ -XXX,XX +XXX,XX @@ if test "$cross_compile" = "yes"; then if test "$linux" = "yes" ; then echo "system = 'linux'" >> $cross fi + if test "$darwin" = "yes" ; then + echo "system = 'darwin'" >> $cross + fi case "$ARCH" in i386|x86_64) echo "cpu_family = 'x86'" >> $cross -- 2.20.1 From: Joelle van Dyne <j@getutm.app> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Joelle van Dyne <j@getutm.app> Message-id: 20210126012457.39046-9-j@getutm.app Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- configure | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/configure b/configure index XXXXXXX..XXXXXXX 100755 --- a/configure +++ b/configure @@ -XXX,XX +XXX,XX @@ if test "$cross_compile" = "yes"; then echo "system = 'darwin'" >> $cross fi case "$ARCH" in - i386|x86_64) + i386) echo "cpu_family = 'x86'" >> $cross ;; + x86_64) + echo "cpu_family = 'x86_64'" >> $cross + ;; ppc64le) echo "cpu_family = 'ppc64'" >> $cross ;; -- 2.20.1 From: Joelle van Dyne <j@getutm.app> On iOS there is no CoreAudio, so we should not assume Darwin always has it. Signed-off-by: Joelle van Dyne <j@getutm.app> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210126012457.39046-11-j@getutm.app Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- configure | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/configure b/configure index XXXXXXX..XXXXXXX 100755 --- a/configure +++ b/configure @@ -XXX,XX +XXX,XX @@ fdt="auto" netmap="no" sdl="auto" sdl_image="auto" +coreaudio="auto" virtiofsd="auto" virtfs="auto" libudev="auto" @@ -XXX,XX +XXX,XX @@ Darwin) QEMU_CFLAGS="-arch x86_64 $QEMU_CFLAGS" QEMU_LDFLAGS="-arch x86_64 $QEMU_LDFLAGS" fi - audio_drv_list="coreaudio try-sdl" + audio_drv_list="try-coreaudio try-sdl" audio_possible_drivers="coreaudio sdl" # Disable attempts to use ObjectiveC features in os/object.h since they # won't work when we're compiling with gcc as a C compiler. @@ -XXX,XX +XXX,XX @@ EOF fi fi +########################################## +# detect CoreAudio +if test "$coreaudio" != "no" ; then + coreaudio_libs="-framework CoreAudio" + cat > $TMPC << EOF +#include <CoreAudio/CoreAudio.h> +int main(void) +{ + return (int)AudioGetCurrentHostTime(); +} +EOF + if compile_prog "" "$coreaudio_libs" ; then + coreaudio=yes + else + coreaudio=no + fi +fi + ########################################## # Sound support libraries probe @@ -XXX,XX +XXX,XX @@ for drv in $audio_drv_list; do fi ;; - coreaudio) + coreaudio | try-coreaudio) + if test "$coreaudio" = "no"; then + if test "$drv" = "try-coreaudio"; then + audio_drv_list=$(echo "$audio_drv_list" | sed -e 's/try-coreaudio//') + else + error_exit "$drv check failed" \ + "Make sure to have the $drv is available." + fi + else coreaudio_libs="-framework CoreAudio" + if test "$drv" = "try-coreaudio"; then + audio_drv_list=$(echo "$audio_drv_list" | sed -e 's/try-coreaudio/coreaudio/') + fi + fi ;; dsound) -- 2.20.1 From: Joelle van Dyne <j@getutm.app> A workaround added in early days of 64-bit OSX forced x86_64 if the host machine had 64-bit support. This creates issues when cross- compiling for ARM64. Additionally, the user can always use --cpu=* to manually set the host CPU and therefore this workaround should be removed. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Joelle van Dyne <j@getutm.app> Message-id: 20210126012457.39046-12-j@getutm.app Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- configure | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/configure b/configure index XXXXXXX..XXXXXXX 100755 --- a/configure +++ b/configure @@ -XXX,XX +XXX,XX @@ fi # the correct CPU with the --cpu option. case $targetos in Darwin) - # on Leopard most of the system is 32-bit, so we have to ask the kernel if we can - # run 64-bit userspace code. - # If the user didn't specify a CPU explicitly and the kernel says this is - # 64 bit hw, then assume x86_64. Otherwise fall through to the usual detection code. - if test -z "$cpu" && test "$(sysctl -n hw.optional.x86_64)" = "1"; then - cpu="x86_64" - fi HOST_DSOSUF=".dylib" ;; SunOS) @@ -XXX,XX +XXX,XX @@ OpenBSD) Darwin) bsd="yes" darwin="yes" - if [ "$cpu" = "x86_64" ] ; then - QEMU_CFLAGS="-arch x86_64 $QEMU_CFLAGS" - QEMU_LDFLAGS="-arch x86_64 $QEMU_LDFLAGS" - fi audio_drv_list="try-coreaudio try-sdl" audio_possible_drivers="coreaudio sdl" # Disable attempts to use ObjectiveC features in os/object.h since they -- 2.20.1 From: Alexander Graf <agraf@csgraf.de> In macOS 11, QEMU only gets access to Hypervisor.framework if it has the respective entitlement. Add an entitlement template and automatically self sign and apply the entitlement in the build. Signed-off-by: Alexander Graf <agraf@csgraf.de> Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com> Tested-by: Roman Bolshakov <r.bolshakov@yadro.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- meson.build | 29 +++++++++++++++++++++++++---- accel/hvf/entitlements.plist | 8 ++++++++ scripts/entitlement.sh | 13 +++++++++++++ 3 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 accel/hvf/entitlements.plist create mode 100755 scripts/entitlement.sh diff --git a/meson.build b/meson.build index XXXXXXX..XXXXXXX 100644 --- a/meson.build +++ b/meson.build @@ -XXX,XX +XXX,XX @@ foreach target : target_dirs }] endif foreach exe: execs - emulators += {exe['name']: - executable(exe['name'], exe['sources'], - install: true, + exe_name = exe['name'] + exe_sign = 'CONFIG_HVF' in config_target + if exe_sign + exe_name += '-unsigned' + endif + + emulator = executable(exe_name, exe['sources'], + install: not exe_sign, c_args: c_args, dependencies: arch_deps + deps + exe['dependencies'], objects: lib.extract_all_objects(recursive: true), @@ -XXX,XX +XXX,XX @@ foreach target : target_dirs link_depends: [block_syms, qemu_syms] + exe.get('link_depends', []), link_args: link_args, gui_app: exe['gui']) - } + + if exe_sign + emulators += {exe['name'] : custom_target(exe['name'], + install: true, + install_dir: get_option('bindir'), + depends: emulator, + output: exe['name'], + command: [ + meson.current_source_dir() / 'scripts/entitlement.sh', + meson.current_build_dir() / exe_name, + meson.current_build_dir() / exe['name'], + meson.current_source_dir() / 'accel/hvf/entitlements.plist' + ]) + } + else + emulators += {exe['name']: emulator} + endif if 'CONFIG_TRACE_SYSTEMTAP' in config_host foreach stp: [ diff --git a/accel/hvf/entitlements.plist b/accel/hvf/entitlements.plist new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/accel/hvf/entitlements.plist @@ -XXX,XX +XXX,XX @@ +<?xml version="1.0" encoding="UTF-8"?> +<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"> +<plist version="1.0"> +<dict> + <key>com.apple.security.hypervisor</key> + <true/> +</dict> +</plist> diff --git a/scripts/entitlement.sh b/scripts/entitlement.sh new file mode 100755 index XXXXXXX..XXXXXXX --- /dev/null +++ b/scripts/entitlement.sh @@ -XXX,XX +XXX,XX @@ +#!/bin/sh -e +# +# Helper script for the build process to apply entitlements + +SRC="$1" +DST="$2" +ENTITLEMENT="$3" + +trap 'rm "$DST.tmp"' exit +cp -af "$SRC" "$DST.tmp" +codesign --entitlements "$ENTITLEMENT" --force -s - "$DST.tmp" +mv "$DST.tmp" "$DST" +trap '' exit -- 2.20.1 From: Mihai Carabas <mihai.carabas@oracle.com> To ease the PCI device addition in next patches, split the code as follows: - generic code (read/write/setup) is being kept in pvpanic.c - ISA dependent code moved to pvpanic-isa.c Also, rename: - ISA_PVPANIC_DEVICE -> PVPANIC_ISA_DEVICE. - TYPE_PVPANIC -> TYPE_PVPANIC_ISA. - MemoryRegion io -> mr. - pvpanic_ioport_* in pvpanic_*. Update the build system with the new files and config structure. Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/misc/pvpanic.h | 23 +++++++++- hw/misc/pvpanic-isa.c | 94 +++++++++++++++++++++++++++++++++++++++ hw/misc/pvpanic.c | 85 +++-------------------------------- hw/i386/Kconfig | 2 +- hw/misc/Kconfig | 6 ++- hw/misc/meson.build | 3 +- tests/qtest/meson.build | 2 +- 7 files changed, 130 insertions(+), 85 deletions(-) create mode 100644 hw/misc/pvpanic-isa.c diff --git a/include/hw/misc/pvpanic.h b/include/hw/misc/pvpanic.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/misc/pvpanic.h +++ b/include/hw/misc/pvpanic.h @@ -XXX,XX +XXX,XX @@ #include "qom/object.h" -#define TYPE_PVPANIC "pvpanic" +#define TYPE_PVPANIC_ISA_DEVICE "pvpanic" #define PVPANIC_IOPORT_PROP "ioport" +/* The bit of supported pv event, TODO: include uapi header and remove this */ +#define PVPANIC_F_PANICKED 0 +#define PVPANIC_F_CRASHLOADED 1 + +/* The pv event value */ +#define PVPANIC_PANICKED (1 << PVPANIC_F_PANICKED) +#define PVPANIC_CRASHLOADED (1 << PVPANIC_F_CRASHLOADED) + +/* + * PVPanicState for any device type + */ +typedef struct PVPanicState PVPanicState; +struct PVPanicState { + MemoryRegion mr; + uint8_t events; +}; + +void pvpanic_setup_io(PVPanicState *s, DeviceState *dev, unsigned size); + static inline uint16_t pvpanic_port(void) { - Object *o = object_resolve_path_type("", TYPE_PVPANIC, NULL); + Object *o = object_resolve_path_type("", TYPE_PVPANIC_ISA_DEVICE, NULL); if (!o) { return 0; } diff --git a/hw/misc/pvpanic-isa.c b/hw/misc/pvpanic-isa.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/hw/misc/pvpanic-isa.c @@ -XXX,XX +XXX,XX @@ +/* + * QEMU simulated pvpanic device. + * + * Copyright Fujitsu, Corp. 2013 + * + * Authors: + * Wen Congyang <wency@cn.fujitsu.com> + * Hu Tao <hutao@cn.fujitsu.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + */ + +#include "qemu/osdep.h" +#include "qemu/log.h" +#include "qemu/module.h" +#include "sysemu/runstate.h" + +#include "hw/nvram/fw_cfg.h" +#include "hw/qdev-properties.h" +#include "hw/misc/pvpanic.h" +#include "qom/object.h" +#include "hw/isa/isa.h" + +OBJECT_DECLARE_SIMPLE_TYPE(PVPanicISAState, PVPANIC_ISA_DEVICE) + +/* + * PVPanicISAState for ISA device and + * use ioport. + */ +struct PVPanicISAState { + ISADevice parent_obj; + + uint16_t ioport; + PVPanicState pvpanic; +}; + +static void pvpanic_isa_initfn(Object *obj) +{ + PVPanicISAState *s = PVPANIC_ISA_DEVICE(obj); + + pvpanic_setup_io(&s->pvpanic, DEVICE(s), 1); +} + +static void pvpanic_isa_realizefn(DeviceState *dev, Error **errp) +{ + ISADevice *d = ISA_DEVICE(dev); + PVPanicISAState *s = PVPANIC_ISA_DEVICE(dev); + PVPanicState *ps = &s->pvpanic; + FWCfgState *fw_cfg = fw_cfg_find(); + uint16_t *pvpanic_port; + + if (!fw_cfg) { + return; + } + + pvpanic_port = g_malloc(sizeof(*pvpanic_port)); + *pvpanic_port = cpu_to_le16(s->ioport); + fw_cfg_add_file(fw_cfg, "etc/pvpanic-port", pvpanic_port, + sizeof(*pvpanic_port)); + + isa_register_ioport(d, &ps->mr, s->ioport); +} + +static Property pvpanic_isa_properties[] = { + DEFINE_PROP_UINT16(PVPANIC_IOPORT_PROP, PVPanicISAState, ioport, 0x505), + DEFINE_PROP_UINT8("events", PVPanicISAState, pvpanic.events, PVPANIC_PANICKED | PVPANIC_CRASHLOADED), + DEFINE_PROP_END_OF_LIST(), +}; + +static void pvpanic_isa_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + + dc->realize = pvpanic_isa_realizefn; + device_class_set_props(dc, pvpanic_isa_properties); + set_bit(DEVICE_CATEGORY_MISC, dc->categories); +} + +static TypeInfo pvpanic_isa_info = { + .name = TYPE_PVPANIC_ISA_DEVICE, + .parent = TYPE_ISA_DEVICE, + .instance_size = sizeof(PVPanicISAState), + .instance_init = pvpanic_isa_initfn, + .class_init = pvpanic_isa_class_init, +}; + +static void pvpanic_register_types(void) +{ + type_register_static(&pvpanic_isa_info); +} + +type_init(pvpanic_register_types) diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/pvpanic.c +++ b/hw/misc/pvpanic.c @@ -XXX,XX +XXX,XX @@ #include "hw/misc/pvpanic.h" #include "qom/object.h" -/* The bit of supported pv event, TODO: include uapi header and remove this */ -#define PVPANIC_F_PANICKED 0 -#define PVPANIC_F_CRASHLOADED 1 - -/* The pv event value */ -#define PVPANIC_PANICKED (1 << PVPANIC_F_PANICKED) -#define PVPANIC_CRASHLOADED (1 << PVPANIC_F_CRASHLOADED) - -typedef struct PVPanicState PVPanicState; -DECLARE_INSTANCE_CHECKER(PVPanicState, ISA_PVPANIC_DEVICE, - TYPE_PVPANIC) - static void handle_event(int event) { static bool logged; @@ -XXX,XX +XXX,XX @@ static void handle_event(int event) } } -#include "hw/isa/isa.h" - -struct PVPanicState { - ISADevice parent_obj; - - MemoryRegion io; - uint16_t ioport; - uint8_t events; -}; - /* return supported events on read */ -static uint64_t pvpanic_ioport_read(void *opaque, hwaddr addr, unsigned size) +static uint64_t pvpanic_read(void *opaque, hwaddr addr, unsigned size) { PVPanicState *pvp = opaque; return pvp->events; } -static void pvpanic_ioport_write(void *opaque, hwaddr addr, uint64_t val, +static void pvpanic_write(void *opaque, hwaddr addr, uint64_t val, unsigned size) { handle_event(val); } static const MemoryRegionOps pvpanic_ops = { - .read = pvpanic_ioport_read, - .write = pvpanic_ioport_write, + .read = pvpanic_read, + .write = pvpanic_write, .impl = { .min_access_size = 1, .max_access_size = 1, }, }; -static void pvpanic_isa_initfn(Object *obj) +void pvpanic_setup_io(PVPanicState *s, DeviceState *dev, unsigned size) { - PVPanicState *s = ISA_PVPANIC_DEVICE(obj); - - memory_region_init_io(&s->io, OBJECT(s), &pvpanic_ops, s, "pvpanic", 1); + memory_region_init_io(&s->mr, OBJECT(dev), &pvpanic_ops, s, "pvpanic", size); } - -static void pvpanic_isa_realizefn(DeviceState *dev, Error **errp) -{ - ISADevice *d = ISA_DEVICE(dev); - PVPanicState *s = ISA_PVPANIC_DEVICE(dev); - FWCfgState *fw_cfg = fw_cfg_find(); - uint16_t *pvpanic_port; - - if (!fw_cfg) { - return; - } - - pvpanic_port = g_malloc(sizeof(*pvpanic_port)); - *pvpanic_port = cpu_to_le16(s->ioport); - fw_cfg_add_file(fw_cfg, "etc/pvpanic-port", pvpanic_port, - sizeof(*pvpanic_port)); - - isa_register_ioport(d, &s->io, s->ioport); -} - -static Property pvpanic_isa_properties[] = { - DEFINE_PROP_UINT16(PVPANIC_IOPORT_PROP, PVPanicState, ioport, 0x505), - DEFINE_PROP_UINT8("events", PVPanicState, events, PVPANIC_PANICKED | PVPANIC_CRASHLOADED), - DEFINE_PROP_END_OF_LIST(), -}; - -static void pvpanic_isa_class_init(ObjectClass *klass, void *data) -{ - DeviceClass *dc = DEVICE_CLASS(klass); - - dc->realize = pvpanic_isa_realizefn; - device_class_set_props(dc, pvpanic_isa_properties); - set_bit(DEVICE_CATEGORY_MISC, dc->categories); -} - -static TypeInfo pvpanic_isa_info = { - .name = TYPE_PVPANIC, - .parent = TYPE_ISA_DEVICE, - .instance_size = sizeof(PVPanicState), - .instance_init = pvpanic_isa_initfn, - .class_init = pvpanic_isa_class_init, -}; - -static void pvpanic_register_types(void) -{ - type_register_static(&pvpanic_isa_info); -} - -type_init(pvpanic_register_types) diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/hw/i386/Kconfig +++ b/hw/i386/Kconfig @@ -XXX,XX +XXX,XX @@ config PC imply ISA_DEBUG imply PARALLEL imply PCI_DEVICES - imply PVPANIC + imply PVPANIC_ISA imply QXL imply SEV imply SGA diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/Kconfig +++ b/hw/misc/Kconfig @@ -XXX,XX +XXX,XX @@ config IOTKIT_SYSCTL config IOTKIT_SYSINFO bool -config PVPANIC +config PVPANIC_COMMON + bool + +config PVPANIC_ISA bool depends on ISA_BUS + select PVPANIC_COMMON config AUX bool diff --git a/hw/misc/meson.build b/hw/misc/meson.build index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/meson.build +++ b/hw/misc/meson.build @@ -XXX,XX +XXX,XX @@ softmmu_ss.add(when: 'CONFIG_EMC141X', if_true: files('emc141x.c')) softmmu_ss.add(when: 'CONFIG_UNIMP', if_true: files('unimp.c')) softmmu_ss.add(when: 'CONFIG_EMPTY_SLOT', if_true: files('empty_slot.c')) softmmu_ss.add(when: 'CONFIG_LED', if_true: files('led.c')) +softmmu_ss.add(when: 'CONFIG_PVPANIC_COMMON', if_true: files('pvpanic.c')) # ARM devices softmmu_ss.add(when: 'CONFIG_PL310', if_true: files('arm_l2x0.c')) @@ -XXX,XX +XXX,XX @@ softmmu_ss.add(when: 'CONFIG_IOTKIT_SYSINFO', if_true: files('iotkit-sysinfo.c') softmmu_ss.add(when: 'CONFIG_ARMSSE_CPUID', if_true: files('armsse-cpuid.c')) softmmu_ss.add(when: 'CONFIG_ARMSSE_MHU', if_true: files('armsse-mhu.c')) -softmmu_ss.add(when: 'CONFIG_PVPANIC', if_true: files('pvpanic.c')) +softmmu_ss.add(when: 'CONFIG_PVPANIC_ISA', if_true: files('pvpanic-isa.c')) softmmu_ss.add(when: 'CONFIG_AUX', if_true: files('auxbus.c')) softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_scu.c', 'aspeed_sdmc.c', 'aspeed_xdma.c')) softmmu_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-sysreg.c')) diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -XXX,XX +XXX,XX @@ qtests_i386 = \ (config_host.has_key('CONFIG_LINUX') and \ config_all_devices.has_key('CONFIG_ISA_IPMI_BT') ? ['ipmi-bt-test'] : []) + \ (config_all_devices.has_key('CONFIG_WDT_IB700') ? ['wdt_ib700-test'] : []) + \ - (config_all_devices.has_key('CONFIG_PVPANIC') ? ['pvpanic-test'] : []) + \ + (config_all_devices.has_key('CONFIG_PVPANIC_ISA') ? ['pvpanic-test'] : []) + \ (config_all_devices.has_key('CONFIG_HDA') ? ['intel-hda-test'] : []) + \ (config_all_devices.has_key('CONFIG_I82801B11') ? ['i82801b11-test'] : []) + \ (config_all_devices.has_key('CONFIG_IOH3420') ? ['ioh3420-test'] : []) + \ -- 2.20.1 From: Mihai Carabas <mihai.carabas@oracle.com> Add PCI interface support for PVPANIC device. Create a new file pvpanic-pci.c where the PCI specific routines reside and update the build system with the new files and config structure. Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- docs/specs/pci-ids.txt | 1 + include/hw/misc/pvpanic.h | 1 + include/hw/pci/pci.h | 1 + hw/misc/pvpanic-pci.c | 94 +++++++++++++++++++++++++++++++++++++++ hw/misc/Kconfig | 6 +++ hw/misc/meson.build | 1 + 6 files changed, 104 insertions(+) create mode 100644 hw/misc/pvpanic-pci.c diff --git a/docs/specs/pci-ids.txt b/docs/specs/pci-ids.txt index XXXXXXX..XXXXXXX 100644 --- a/docs/specs/pci-ids.txt +++ b/docs/specs/pci-ids.txt @@ -XXX,XX +XXX,XX @@ PCI devices (other than virtio): 1b36:000d PCI xhci usb host adapter 1b36:000f mdpy (mdev sample device), linux/samples/vfio-mdev/mdpy.c 1b36:0010 PCIe NVMe device (-device nvme) +1b36:0011 PCI PVPanic device (-device pvpanic-pci) All these devices are documented in docs/specs. diff --git a/include/hw/misc/pvpanic.h b/include/hw/misc/pvpanic.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/misc/pvpanic.h +++ b/include/hw/misc/pvpanic.h @@ -XXX,XX +XXX,XX @@ #include "qom/object.h" #define TYPE_PVPANIC_ISA_DEVICE "pvpanic" +#define TYPE_PVPANIC_PCI_DEVICE "pvpanic-pci" #define PVPANIC_IOPORT_PROP "ioport" diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -XXX,XX +XXX,XX @@ extern bool pci_available; #define PCI_DEVICE_ID_REDHAT_PCIE_BRIDGE 0x000e #define PCI_DEVICE_ID_REDHAT_MDPY 0x000f #define PCI_DEVICE_ID_REDHAT_NVME 0x0010 +#define PCI_DEVICE_ID_REDHAT_PVPANIC 0x0011 #define PCI_DEVICE_ID_REDHAT_QXL 0x0100 #define FMT_PCIBUS PRIx64 diff --git a/hw/misc/pvpanic-pci.c b/hw/misc/pvpanic-pci.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/hw/misc/pvpanic-pci.c @@ -XXX,XX +XXX,XX @@ +/* + * QEMU simulated PCI pvpanic device. + * + * Copyright (C) 2020 Oracle + * + * Authors: + * Mihai Carabas <mihai.carabas@oracle.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + */ + +#include "qemu/osdep.h" +#include "qemu/log.h" +#include "qemu/module.h" +#include "sysemu/runstate.h" + +#include "hw/nvram/fw_cfg.h" +#include "hw/qdev-properties.h" +#include "migration/vmstate.h" +#include "hw/misc/pvpanic.h" +#include "qom/object.h" +#include "hw/pci/pci.h" + +OBJECT_DECLARE_SIMPLE_TYPE(PVPanicPCIState, PVPANIC_PCI_DEVICE) + +/* + * PVPanicPCIState for PCI device + */ +typedef struct PVPanicPCIState { + PCIDevice dev; + PVPanicState pvpanic; +} PVPanicPCIState; + +static const VMStateDescription vmstate_pvpanic_pci = { + .name = "pvpanic-pci", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_PCI_DEVICE(dev, PVPanicPCIState), + VMSTATE_END_OF_LIST() + } +}; + +static void pvpanic_pci_realizefn(PCIDevice *dev, Error **errp) +{ + PVPanicPCIState *s = PVPANIC_PCI_DEVICE(dev); + PVPanicState *ps = &s->pvpanic; + + pvpanic_setup_io(&s->pvpanic, DEVICE(s), 2); + + pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &ps->mr); +} + +static Property pvpanic_pci_properties[] = { + DEFINE_PROP_UINT8("events", PVPanicPCIState, pvpanic.events, PVPANIC_PANICKED | PVPANIC_CRASHLOADED), + DEFINE_PROP_END_OF_LIST(), +}; + +static void pvpanic_pci_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass); + + device_class_set_props(dc, pvpanic_pci_properties); + + pc->realize = pvpanic_pci_realizefn; + pc->vendor_id = PCI_VENDOR_ID_REDHAT; + pc->device_id = PCI_DEVICE_ID_REDHAT_PVPANIC; + pc->revision = 1; + pc->class_id = PCI_CLASS_SYSTEM_OTHER; + dc->vmsd = &vmstate_pvpanic_pci; + + set_bit(DEVICE_CATEGORY_MISC, dc->categories); +} + +static TypeInfo pvpanic_pci_info = { + .name = TYPE_PVPANIC_PCI_DEVICE, + .parent = TYPE_PCI_DEVICE, + .instance_size = sizeof(PVPanicPCIState), + .class_init = pvpanic_pci_class_init, + .interfaces = (InterfaceInfo[]) { + { INTERFACE_CONVENTIONAL_PCI_DEVICE }, + { } + } +}; + +static void pvpanic_register_types(void) +{ + type_register_static(&pvpanic_pci_info); +} + +type_init(pvpanic_register_types); diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/Kconfig +++ b/hw/misc/Kconfig @@ -XXX,XX +XXX,XX @@ config IOTKIT_SYSINFO config PVPANIC_COMMON bool +config PVPANIC_PCI + bool + default y if PCI_DEVICES + depends on PCI + select PVPANIC_COMMON + config PVPANIC_ISA bool depends on ISA_BUS diff --git a/hw/misc/meson.build b/hw/misc/meson.build index XXXXXXX..XXXXXXX 100644 --- a/hw/misc/meson.build +++ b/hw/misc/meson.build @@ -XXX,XX +XXX,XX @@ softmmu_ss.add(when: 'CONFIG_ARMSSE_CPUID', if_true: files('armsse-cpuid.c')) softmmu_ss.add(when: 'CONFIG_ARMSSE_MHU', if_true: files('armsse-mhu.c')) softmmu_ss.add(when: 'CONFIG_PVPANIC_ISA', if_true: files('pvpanic-isa.c')) +softmmu_ss.add(when: 'CONFIG_PVPANIC_PCI', if_true: files('pvpanic-pci.c')) softmmu_ss.add(when: 'CONFIG_AUX', if_true: files('auxbus.c')) softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_scu.c', 'aspeed_sdmc.c', 'aspeed_xdma.c')) softmmu_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-sysreg.c')) -- 2.20.1 From: Mihai Carabas <mihai.carabas@oracle.com> Add pvpanic PCI device support details in docs/specs/pvpanic.txt. Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- docs/specs/pvpanic.txt | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/docs/specs/pvpanic.txt b/docs/specs/pvpanic.txt index XXXXXXX..XXXXXXX 100644 --- a/docs/specs/pvpanic.txt +++ b/docs/specs/pvpanic.txt @@ -XXX,XX +XXX,XX @@ PVPANIC DEVICE ============== -pvpanic device is a simulated ISA device, through which a guest panic +pvpanic device is a simulated device, through which a guest panic event is sent to qemu, and a QMP event is generated. This allows management apps (e.g. libvirt) to be notified and respond to the event. @@ -XXX,XX +XXX,XX @@ The management app has the option of waiting for GUEST_PANICKED events, and/or polling for guest-panicked RunState, to learn when the pvpanic device has fired a panic event. +The pvpanic device can be implemented as an ISA device (using IOPORT) or as a +PCI device. + ISA Interface ------------- @@ -XXX,XX +XXX,XX @@ bit 1: a guest panic has happened and will be handled by the guest; the host should record it or report it, but should not affect the execution of the guest. +PCI Interface +------------- + +The PCI interface is similar to the ISA interface except that it uses an MMIO +address space provided by its BAR0, 1 byte long. Any machine with a PCI bus +can enable a pvpanic device by adding '-device pvpanic-pci' to the command +line. + ACPI Interface -------------- -- 2.20.1 From: Mihai Carabas <mihai.carabas@oracle.com> Add a test case for pvpanic-pci device. The scenario is the same as pvpanic ISA device, but is using the PCI bus. Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com> Acked-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- tests/qtest/pvpanic-pci-test.c | 94 ++++++++++++++++++++++++++++++++++ tests/qtest/meson.build | 1 + 2 files changed, 95 insertions(+) create mode 100644 tests/qtest/pvpanic-pci-test.c diff --git a/tests/qtest/pvpanic-pci-test.c b/tests/qtest/pvpanic-pci-test.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/tests/qtest/pvpanic-pci-test.c @@ -XXX,XX +XXX,XX @@ +/* + * QTest testcase for PV Panic PCI device + * + * Copyright (C) 2020 Oracle + * + * Authors: + * Mihai Carabas <mihai.carabas@oracle.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + */ + +#include "qemu/osdep.h" +#include "libqos/libqtest.h" +#include "qapi/qmp/qdict.h" +#include "libqos/pci.h" +#include "libqos/pci-pc.h" +#include "hw/pci/pci_regs.h" + +static void test_panic_nopause(void) +{ + uint8_t val; + QDict *response, *data; + QTestState *qts; + QPCIBus *pcibus; + QPCIDevice *dev; + QPCIBar bar; + + qts = qtest_init("-device pvpanic-pci,addr=04.0 -action panic=none"); + pcibus = qpci_new_pc(qts, NULL); + dev = qpci_device_find(pcibus, QPCI_DEVFN(0x4, 0x0)); + qpci_device_enable(dev); + bar = qpci_iomap(dev, 0, NULL); + + qpci_memread(dev, bar, 0, &val, sizeof(val)); + g_assert_cmpuint(val, ==, 3); + + val = 1; + qpci_memwrite(dev, bar, 0, &val, sizeof(val)); + + response = qtest_qmp_eventwait_ref(qts, "GUEST_PANICKED"); + g_assert(qdict_haskey(response, "data")); + data = qdict_get_qdict(response, "data"); + g_assert(qdict_haskey(data, "action")); + g_assert_cmpstr(qdict_get_str(data, "action"), ==, "run"); + qobject_unref(response); + + qtest_quit(qts); +} + +static void test_panic(void) +{ + uint8_t val; + QDict *response, *data; + QTestState *qts; + QPCIBus *pcibus; + QPCIDevice *dev; + QPCIBar bar; + + qts = qtest_init("-device pvpanic-pci,addr=04.0 -action panic=pause"); + pcibus = qpci_new_pc(qts, NULL); + dev = qpci_device_find(pcibus, QPCI_DEVFN(0x4, 0x0)); + qpci_device_enable(dev); + bar = qpci_iomap(dev, 0, NULL); + + qpci_memread(dev, bar, 0, &val, sizeof(val)); + g_assert_cmpuint(val, ==, 3); + + val = 1; + qpci_memwrite(dev, bar, 0, &val, sizeof(val)); + + response = qtest_qmp_eventwait_ref(qts, "GUEST_PANICKED"); + g_assert(qdict_haskey(response, "data")); + data = qdict_get_qdict(response, "data"); + g_assert(qdict_haskey(data, "action")); + g_assert_cmpstr(qdict_get_str(data, "action"), ==, "pause"); + qobject_unref(response); + + qtest_quit(qts); +} + +int main(int argc, char **argv) +{ + int ret; + + g_test_init(&argc, &argv, NULL); + qtest_add_func("/pvpanic-pci/panic", test_panic); + qtest_add_func("/pvpanic-pci/panic-nopause", test_panic_nopause); + + ret = g_test_run(); + + return ret; +} diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -XXX,XX +XXX,XX @@ qtests_i386 = \ config_all_devices.has_key('CONFIG_ISA_IPMI_BT') ? ['ipmi-bt-test'] : []) + \ (config_all_devices.has_key('CONFIG_WDT_IB700') ? ['wdt_ib700-test'] : []) + \ (config_all_devices.has_key('CONFIG_PVPANIC_ISA') ? ['pvpanic-test'] : []) + \ + (config_all_devices.has_key('CONFIG_PVPANIC_PCI') ? ['pvpanic-pci-test'] : []) + \ (config_all_devices.has_key('CONFIG_HDA') ? ['intel-hda-test'] : []) + \ (config_all_devices.has_key('CONFIG_I82801B11') ? ['i82801b11-test'] : []) + \ (config_all_devices.has_key('CONFIG_IOH3420') ? ['ioh3420-test'] : []) + \ -- 2.20.1 The ptimer API currently provides two methods for setting the period: ptimer_set_period(), which takes a period in nanoseconds, and ptimer_set_freq(), which takes a frequency in Hz. Neither of these lines up nicely with the Clock API, because although both the Clock and the ptimer track the frequency using a representation of whole and fractional nanoseconds, conversion via either period-in-ns or frequency-in-Hz will introduce a rounding error. Add a new function ptimer_set_period_from_clock() which takes the Clock object directly to avoid the rounding issues. This includes a facility for the user to specify that there is a frequency divider between the Clock proper and the timer, as some timer devices like the CMSDK APB dualtimer need this. To avoid having to drag in clock.h from ptimer.h we add the Clock type to typedefs.h. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-2-peter.maydell@linaro.org Message-id: 20210121190622.22000-2-peter.maydell@linaro.org --- include/hw/ptimer.h | 22 ++++++++++++++++++++++ include/qemu/typedefs.h | 1 + hw/core/ptimer.c | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/include/hw/ptimer.h b/include/hw/ptimer.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/ptimer.h +++ b/include/hw/ptimer.h @@ -XXX,XX +XXX,XX @@ void ptimer_transaction_commit(ptimer_state *s); */ void ptimer_set_period(ptimer_state *s, int64_t period); +/** + * ptimer_set_period_from_clock - Set counter increment from a Clock + * @s: ptimer to configure + * @clk: pointer to Clock object to take period from + * @divisor: value to scale the clock frequency down by + * + * If the ptimer is being driven from a Clock, this is the preferred + * way to tell the ptimer about the period, because it avoids any + * possible rounding errors that might happen if the internal + * representation of the Clock period was converted to either a period + * in ns or a frequency in Hz. + * + * If the ptimer should run at the same frequency as the clock, + * pass 1 as the @divisor; if the ptimer should run at half the + * frequency, pass 2, and so on. + * + * This function will assert if it is called outside a + * ptimer_transaction_begin/commit block. + */ +void ptimer_set_period_from_clock(ptimer_state *s, const Clock *clock, + unsigned int divisor); + /** * ptimer_set_freq - Set counter frequency in Hz * @s: ptimer to configure diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index XXXXXXX..XXXXXXX 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -XXX,XX +XXX,XX @@ typedef struct BlockDriverState BlockDriverState; typedef struct BusClass BusClass; typedef struct BusState BusState; typedef struct Chardev Chardev; +typedef struct Clock Clock; typedef struct CompatProperty CompatProperty; typedef struct CoMutex CoMutex; typedef struct CPUAddressSpace CPUAddressSpace; diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c index XXXXXXX..XXXXXXX 100644 --- a/hw/core/ptimer.c +++ b/hw/core/ptimer.c @@ -XXX,XX +XXX,XX @@ #include "sysemu/qtest.h" #include "block/aio.h" #include "sysemu/cpus.h" +#include "hw/clock.h" #define DELTA_ADJUST 1 #define DELTA_NO_ADJUST -1 @@ -XXX,XX +XXX,XX @@ void ptimer_set_period(ptimer_state *s, int64_t period) } } +/* Set counter increment interval from a Clock */ +void ptimer_set_period_from_clock(ptimer_state *s, const Clock *clk, + unsigned int divisor) +{ + /* + * The raw clock period is a 64-bit value in units of 2^-32 ns; + * put another way it's a 32.32 fixed-point ns value. Our internal + * representation of the period is 64.32 fixed point ns, so + * the conversion is simple. + */ + uint64_t raw_period = clock_get(clk); + uint64_t period_frac; + + assert(s->in_transaction); + s->delta = ptimer_get_count(s); + s->period = extract64(raw_period, 32, 32); + period_frac = extract64(raw_period, 0, 32); + /* + * divisor specifies a possible frequency divisor between the + * clock and the timer, so it is a multiplier on the period. + * We do the multiply after splitting the raw period out into + * period and frac to avoid having to do a 32*64->96 multiply. + */ + s->period *= divisor; + period_frac *= divisor; + s->period += extract64(period_frac, 32, 32); + s->period_frac = (uint32_t)period_frac; + + if (s->enabled) { + s->need_reload = true; + } +} + /* Set counter frequency in Hz. */ void ptimer_set_freq(ptimer_state *s, uint32_t freq) { -- 2.20.1 Add a function for checking whether a clock has a source. This is useful for devices which have input clocks that must be wired up by the board as it allows them to fail in realize rather than ploughing on with a zero-period clock. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-3-peter.maydell@linaro.org Message-id: 20210121190622.22000-3-peter.maydell@linaro.org --- docs/devel/clocks.rst | 16 ++++++++++++++++ include/hw/clock.h | 15 +++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/docs/devel/clocks.rst b/docs/devel/clocks.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/devel/clocks.rst +++ b/docs/devel/clocks.rst @@ -XXX,XX +XXX,XX @@ object during device instance init. For example: /* set initial value to 10ns / 100MHz */ clock_set_ns(clk, 10); +To enforce that the clock is wired up by the board code, you can +call ``clock_has_source()`` in your device's realize method: + +.. code-block:: c + + if (!clock_has_source(s->clk)) { + error_setg(errp, "MyDevice: clk input must be connected"); + return; + } + +Note that this only checks that the clock has been wired up; it is +still possible that the output clock connected to it is disabled +or has not yet been configured, in which case the period will be +zero. You should use the clock callback to find out when the clock +period changes. + Fetching clock frequency/period ------------------------------- diff --git a/include/hw/clock.h b/include/hw/clock.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/clock.h +++ b/include/hw/clock.h @@ -XXX,XX +XXX,XX @@ void clock_clear_callback(Clock *clk); */ void clock_set_source(Clock *clk, Clock *src); +/** + * clock_has_source: + * @clk: the clock + * + * Returns true if the clock has a source clock connected to it. + * This is useful for devices which have input clocks which must + * be connected by the board/SoC code which creates them. The + * device code can use this to check in its realize method that + * the clock has been connected. + */ +static inline bool clock_has_source(const Clock *clk) +{ + return clk->source != NULL; +} + /** * clock_set: * @clk: the clock to initialize. -- 2.20.1 Add a simple test of the CMSDK APB timer, since we're about to do some refactoring of how it is clocked. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-4-peter.maydell@linaro.org Message-id: 20210121190622.22000-4-peter.maydell@linaro.org --- tests/qtest/cmsdk-apb-timer-test.c | 75 ++++++++++++++++++++++++++++++ MAINTAINERS | 1 + tests/qtest/meson.build | 1 + 3 files changed, 77 insertions(+) create mode 100644 tests/qtest/cmsdk-apb-timer-test.c diff --git a/tests/qtest/cmsdk-apb-timer-test.c b/tests/qtest/cmsdk-apb-timer-test.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/tests/qtest/cmsdk-apb-timer-test.c @@ -XXX,XX +XXX,XX @@ +/* + * QTest testcase for the CMSDK APB timer device + * + * Copyright (c) 2021 Linaro Limited + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ + +#include "qemu/osdep.h" +#include "libqtest-single.h" + +/* IoTKit/ARMSSE-200 timer0; driven at 25MHz in mps2-an385, so 40ns per tick */ +#define TIMER_BASE 0x40000000 + +#define CTRL 0 +#define VALUE 4 +#define RELOAD 8 +#define INTSTATUS 0xc + +static void test_timer(void) +{ + g_assert_true(readl(TIMER_BASE + INTSTATUS) == 0); + + /* Start timer: will fire after 40 * 1000 == 40000 ns */ + writel(TIMER_BASE + RELOAD, 1000); + writel(TIMER_BASE + CTRL, 9); + + /* Step to just past the 500th tick and check VALUE */ + clock_step(40 * 500 + 1); + g_assert_cmpuint(readl(TIMER_BASE + INTSTATUS), ==, 0); + g_assert_cmpuint(readl(TIMER_BASE + VALUE), ==, 500); + + /* Just past the 1000th tick: timer should have fired */ + clock_step(40 * 500); + g_assert_cmpuint(readl(TIMER_BASE + INTSTATUS), ==, 1); + g_assert_cmpuint(readl(TIMER_BASE + VALUE), ==, 0); + + /* VALUE reloads at the following tick */ + clock_step(40); + g_assert_cmpuint(readl(TIMER_BASE + VALUE), ==, 1000); + + /* Check write-1-to-clear behaviour of INTSTATUS */ + writel(TIMER_BASE + INTSTATUS, 0); + g_assert_cmpuint(readl(TIMER_BASE + INTSTATUS), ==, 1); + writel(TIMER_BASE + INTSTATUS, 1); + g_assert_cmpuint(readl(TIMER_BASE + INTSTATUS), ==, 0); + + /* Turn off the timer */ + writel(TIMER_BASE + CTRL, 0); +} + +int main(int argc, char **argv) +{ + int r; + + g_test_init(&argc, &argv, NULL); + + qtest_start("-machine mps2-an385"); + + qtest_add_func("/cmsdk-apb-timer/timer", test_timer); + + r = g_test_run(); + + qtest_end(); + + return r; +} diff --git a/MAINTAINERS b/MAINTAINERS index XXXXXXX..XXXXXXX 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -XXX,XX +XXX,XX @@ F: include/hw/rtc/pl031.h F: include/hw/arm/primecell.h F: hw/timer/cmsdk-apb-timer.c F: include/hw/timer/cmsdk-apb-timer.h +F: tests/qtest/cmsdk-apb-timer-test.c F: hw/timer/cmsdk-apb-dualtimer.c F: include/hw/timer/cmsdk-apb-dualtimer.h F: hw/char/cmsdk-apb-uart.c diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -XXX,XX +XXX,XX @@ qtests_npcm7xx = \ 'npcm7xx_timer-test', 'npcm7xx_watchdog_timer-test'] qtests_arm = \ + (config_all_devices.has_key('CONFIG_CMSDK_APB_TIMER') ? ['cmsdk-apb-timer-test'] : []) + \ (config_all_devices.has_key('CONFIG_PFLASH_CFI02') ? ['pflash-cfi02-test'] : []) + \ (config_all_devices.has_key('CONFIG_NPCM7XX') ? qtests_npcm7xx : []) + \ ['arm-cpu-features', -- 2.20.1 Add a simple test of the CMSDK watchdog, since we're about to do some refactoring of how it is clocked. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-5-peter.maydell@linaro.org Message-id: 20210121190622.22000-5-peter.maydell@linaro.org Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- tests/qtest/cmsdk-apb-watchdog-test.c | 79 +++++++++++++++++++++++++++ MAINTAINERS | 1 + tests/qtest/meson.build | 1 + 3 files changed, 81 insertions(+) create mode 100644 tests/qtest/cmsdk-apb-watchdog-test.c diff --git a/tests/qtest/cmsdk-apb-watchdog-test.c b/tests/qtest/cmsdk-apb-watchdog-test.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/tests/qtest/cmsdk-apb-watchdog-test.c @@ -XXX,XX +XXX,XX @@ +/* + * QTest testcase for the CMSDK APB watchdog device + * + * Copyright (c) 2021 Linaro Limited + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ + +#include "qemu/osdep.h" +#include "libqtest-single.h" + +/* + * lm3s811evb watchdog; at board startup this runs at 200MHz / 16 == 12.5MHz, + * which is 80ns per tick. + */ +#define WDOG_BASE 0x40000000 + +#define WDOGLOAD 0 +#define WDOGVALUE 4 +#define WDOGCONTROL 8 +#define WDOGINTCLR 0xc +#define WDOGRIS 0x10 +#define WDOGMIS 0x14 +#define WDOGLOCK 0xc00 + +static void test_watchdog(void) +{ + g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 0); + + writel(WDOG_BASE + WDOGCONTROL, 1); + writel(WDOG_BASE + WDOGLOAD, 1000); + + /* Step to just past the 500th tick */ + clock_step(500 * 80 + 1); + g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 0); + g_assert_cmpuint(readl(WDOG_BASE + WDOGVALUE), ==, 500); + + /* Just past the 1000th tick: timer should have fired */ + clock_step(500 * 80); + g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 1); + g_assert_cmpuint(readl(WDOG_BASE + WDOGVALUE), ==, 0); + + /* VALUE reloads at following tick */ + clock_step(80); + g_assert_cmpuint(readl(WDOG_BASE + WDOGVALUE), ==, 1000); + + /* Writing any value to WDOGINTCLR clears the interrupt and reloads */ + clock_step(500 * 80); + g_assert_cmpuint(readl(WDOG_BASE + WDOGVALUE), ==, 500); + g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 1); + writel(WDOG_BASE + WDOGINTCLR, 0); + g_assert_cmpuint(readl(WDOG_BASE + WDOGVALUE), ==, 1000); + g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 0); +} + +int main(int argc, char **argv) +{ + int r; + + g_test_init(&argc, &argv, NULL); + + qtest_start("-machine lm3s811evb"); + + qtest_add_func("/cmsdk-apb-watchdog/watchdog", test_watchdog); + + r = g_test_run(); + + qtest_end(); + + return r; +} diff --git a/MAINTAINERS b/MAINTAINERS index XXXXXXX..XXXXXXX 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -XXX,XX +XXX,XX @@ F: hw/char/cmsdk-apb-uart.c F: include/hw/char/cmsdk-apb-uart.h F: hw/watchdog/cmsdk-apb-watchdog.c F: include/hw/watchdog/cmsdk-apb-watchdog.h +F: tests/qtest/cmsdk-apb-watchdog-test.c F: hw/misc/tz-ppc.c F: include/hw/misc/tz-ppc.h F: hw/misc/tz-mpc.c diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -XXX,XX +XXX,XX @@ qtests_npcm7xx = \ 'npcm7xx_watchdog_timer-test'] qtests_arm = \ (config_all_devices.has_key('CONFIG_CMSDK_APB_TIMER') ? ['cmsdk-apb-timer-test'] : []) + \ + (config_all_devices.has_key('CONFIG_CMSDK_APB_WATCHDOG') ? ['cmsdk-apb-watchdog-test'] : []) + \ (config_all_devices.has_key('CONFIG_PFLASH_CFI02') ? ['pflash-cfi02-test'] : []) + \ (config_all_devices.has_key('CONFIG_NPCM7XX') ? qtests_npcm7xx : []) + \ ['arm-cpu-features', -- 2.20.1 Add a simple test of the CMSDK dual timer, since we're about to do some refactoring of how it is clocked. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Message-id: 20210128114145.20536-6-peter.maydell@linaro.org Message-id: 20210121190622.22000-6-peter.maydell@linaro.org --- tests/qtest/cmsdk-apb-dualtimer-test.c | 130 +++++++++++++++++++++++++ MAINTAINERS | 1 + tests/qtest/meson.build | 1 + 3 files changed, 132 insertions(+) create mode 100644 tests/qtest/cmsdk-apb-dualtimer-test.c diff --git a/tests/qtest/cmsdk-apb-dualtimer-test.c b/tests/qtest/cmsdk-apb-dualtimer-test.c new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/tests/qtest/cmsdk-apb-dualtimer-test.c @@ -XXX,XX +XXX,XX @@ +/* + * QTest testcase for the CMSDK APB dualtimer device + * + * Copyright (c) 2021 Linaro Limited + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ + +#include "qemu/osdep.h" +#include "libqtest-single.h" + +/* IoTKit/ARMSSE dualtimer; driven at 25MHz in mps2-an385, so 40ns per tick */ +#define TIMER_BASE 0x40002000 + +#define TIMER1LOAD 0 +#define TIMER1VALUE 4 +#define TIMER1CONTROL 8 +#define TIMER1INTCLR 0xc +#define TIMER1RIS 0x10 +#define TIMER1MIS 0x14 +#define TIMER1BGLOAD 0x18 + +#define TIMER2LOAD 0x20 +#define TIMER2VALUE 0x24 +#define TIMER2CONTROL 0x28 +#define TIMER2INTCLR 0x2c +#define TIMER2RIS 0x30 +#define TIMER2MIS 0x34 +#define TIMER2BGLOAD 0x38 + +#define CTRL_ENABLE (1 << 7) +#define CTRL_PERIODIC (1 << 6) +#define CTRL_INTEN (1 << 5) +#define CTRL_PRESCALE_1 (0 << 2) +#define CTRL_PRESCALE_16 (1 << 2) +#define CTRL_PRESCALE_256 (2 << 2) +#define CTRL_32BIT (1 << 1) +#define CTRL_ONESHOT (1 << 0) + +static void test_dualtimer(void) +{ + g_assert_true(readl(TIMER_BASE + TIMER1RIS) == 0); + + /* Start timer: will fire after 40000 ns */ + writel(TIMER_BASE + TIMER1LOAD, 1000); + /* enable in free-running, wrapping, interrupt mode */ + writel(TIMER_BASE + TIMER1CONTROL, CTRL_ENABLE | CTRL_INTEN); + + /* Step to just past the 500th tick and check VALUE */ + clock_step(500 * 40 + 1); + g_assert_cmpuint(readl(TIMER_BASE + TIMER1RIS), ==, 0); + g_assert_cmpuint(readl(TIMER_BASE + TIMER1VALUE), ==, 500); + + /* Just past the 1000th tick: timer should have fired */ + clock_step(500 * 40); + g_assert_cmpuint(readl(TIMER_BASE + TIMER1RIS), ==, 1); + g_assert_cmpuint(readl(TIMER_BASE + TIMER1VALUE), ==, 0); + + /* + * We are in free-running wrapping 16-bit mode, so on the following + * tick VALUE should have wrapped round to 0xffff. + */ + clock_step(40); + g_assert_cmpuint(readl(TIMER_BASE + TIMER1VALUE), ==, 0xffff); + + /* Check that any write to INTCLR clears interrupt */ + writel(TIMER_BASE + TIMER1INTCLR, 1); + g_assert_cmpuint(readl(TIMER_BASE + TIMER1RIS), ==, 0); + + /* Turn off the timer */ + writel(TIMER_BASE + TIMER1CONTROL, 0); +} + +static void test_prescale(void) +{ + g_assert_true(readl(TIMER_BASE + TIMER2RIS) == 0); + + /* Start timer: will fire after 40 * 256 * 1000 == 1024000 ns */ + writel(TIMER_BASE + TIMER2LOAD, 1000); + /* enable in periodic, wrapping, interrupt mode, prescale 256 */ + writel(TIMER_BASE + TIMER2CONTROL, + CTRL_ENABLE | CTRL_INTEN | CTRL_PERIODIC | CTRL_PRESCALE_256); + + /* Step to just past the 500th tick and check VALUE */ + clock_step(40 * 256 * 501); + g_assert_cmpuint(readl(TIMER_BASE + TIMER2RIS), ==, 0); + g_assert_cmpuint(readl(TIMER_BASE + TIMER2VALUE), ==, 500); + + /* Just past the 1000th tick: timer should have fired */ + clock_step(40 * 256 * 500); + g_assert_cmpuint(readl(TIMER_BASE + TIMER2RIS), ==, 1); + g_assert_cmpuint(readl(TIMER_BASE + TIMER2VALUE), ==, 0); + + /* In periodic mode the tick VALUE now reloads */ + clock_step(40 * 256); + g_assert_cmpuint(readl(TIMER_BASE + TIMER2VALUE), ==, 1000); + + /* Check that any write to INTCLR clears interrupt */ + writel(TIMER_BASE + TIMER2INTCLR, 1); + g_assert_cmpuint(readl(TIMER_BASE + TIMER2RIS), ==, 0); + + /* Turn off the timer */ + writel(TIMER_BASE + TIMER2CONTROL, 0); +} + +int main(int argc, char **argv) +{ + int r; + + g_test_init(&argc, &argv, NULL); + + qtest_start("-machine mps2-an385"); + + qtest_add_func("/cmsdk-apb-dualtimer/dualtimer", test_dualtimer); + qtest_add_func("/cmsdk-apb-dualtimer/prescale", test_prescale); + + r = g_test_run(); + + qtest_end(); + + return r; +} diff --git a/MAINTAINERS b/MAINTAINERS index XXXXXXX..XXXXXXX 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -XXX,XX +XXX,XX @@ F: include/hw/timer/cmsdk-apb-timer.h F: tests/qtest/cmsdk-apb-timer-test.c F: hw/timer/cmsdk-apb-dualtimer.c F: include/hw/timer/cmsdk-apb-dualtimer.h +F: tests/qtest/cmsdk-apb-dualtimer-test.c F: hw/char/cmsdk-apb-uart.c F: include/hw/char/cmsdk-apb-uart.h F: hw/watchdog/cmsdk-apb-watchdog.c diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -XXX,XX +XXX,XX @@ qtests_npcm7xx = \ 'npcm7xx_timer-test', 'npcm7xx_watchdog_timer-test'] qtests_arm = \ + (config_all_devices.has_key('CONFIG_CMSDK_APB_DUALTIMER') ? ['cmsdk-apb-dualtimer-test'] : []) + \ (config_all_devices.has_key('CONFIG_CMSDK_APB_TIMER') ? ['cmsdk-apb-timer-test'] : []) + \ (config_all_devices.has_key('CONFIG_CMSDK_APB_WATCHDOG') ? ['cmsdk-apb-watchdog-test'] : []) + \ (config_all_devices.has_key('CONFIG_PFLASH_CFI02') ? ['pflash-cfi02-test'] : []) + \ -- 2.20.1 The state struct for the CMSDK APB timer device doesn't follow our usual naming convention of camelcase -- "CMSDK" and "APB" are both acronyms, but "TIMER" is not so should not be all-uppercase. Globally rename the struct to "CMSDKAPBTimer" (bringing it into line with CMSDKAPBWatchdog and CMSDKAPBDualTimer; CMSDKAPBUART remains as-is because "UART" is an acronym). Commit created with: perl -p -i -e 's/CMSDKAPBTIMER/CMSDKAPBTimer/g' hw/timer/cmsdk-apb-timer.c include/hw/arm/armsse.h include/hw/timer/cmsdk-apb-timer.h Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-7-peter.maydell@linaro.org Message-id: 20210121190622.22000-7-peter.maydell@linaro.org --- include/hw/arm/armsse.h | 6 +++--- include/hw/timer/cmsdk-apb-timer.h | 4 ++-- hw/timer/cmsdk-apb-timer.c | 28 ++++++++++++++-------------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/include/hw/arm/armsse.h b/include/hw/arm/armsse.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/armsse.h +++ b/include/hw/arm/armsse.h @@ -XXX,XX +XXX,XX @@ struct ARMSSE { TZPPC apb_ppc0; TZPPC apb_ppc1; TZMPC mpc[IOTS_NUM_MPC]; - CMSDKAPBTIMER timer0; - CMSDKAPBTIMER timer1; - CMSDKAPBTIMER s32ktimer; + CMSDKAPBTimer timer0; + CMSDKAPBTimer timer1; + CMSDKAPBTimer s32ktimer; qemu_or_irq ppc_irq_orgate; SplitIRQ sec_resp_splitter; SplitIRQ ppc_irq_splitter[NUM_PPCS]; diff --git a/include/hw/timer/cmsdk-apb-timer.h b/include/hw/timer/cmsdk-apb-timer.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/timer/cmsdk-apb-timer.h +++ b/include/hw/timer/cmsdk-apb-timer.h @@ -XXX,XX +XXX,XX @@ #include "qom/object.h" #define TYPE_CMSDK_APB_TIMER "cmsdk-apb-timer" -OBJECT_DECLARE_SIMPLE_TYPE(CMSDKAPBTIMER, CMSDK_APB_TIMER) +OBJECT_DECLARE_SIMPLE_TYPE(CMSDKAPBTimer, CMSDK_APB_TIMER) -struct CMSDKAPBTIMER { +struct CMSDKAPBTimer { /*< private >*/ SysBusDevice parent_obj; diff --git a/hw/timer/cmsdk-apb-timer.c b/hw/timer/cmsdk-apb-timer.c index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/cmsdk-apb-timer.c +++ b/hw/timer/cmsdk-apb-timer.c @@ -XXX,XX +XXX,XX @@ static const int timer_id[] = { 0x0d, 0xf0, 0x05, 0xb1, /* CID0..CID3 */ }; -static void cmsdk_apb_timer_update(CMSDKAPBTIMER *s) +static void cmsdk_apb_timer_update(CMSDKAPBTimer *s) { qemu_set_irq(s->timerint, !!(s->intstatus & R_INTSTATUS_IRQ_MASK)); } static uint64_t cmsdk_apb_timer_read(void *opaque, hwaddr offset, unsigned size) { - CMSDKAPBTIMER *s = CMSDK_APB_TIMER(opaque); + CMSDKAPBTimer *s = CMSDK_APB_TIMER(opaque); uint64_t r; switch (offset) { @@ -XXX,XX +XXX,XX @@ static uint64_t cmsdk_apb_timer_read(void *opaque, hwaddr offset, unsigned size) static void cmsdk_apb_timer_write(void *opaque, hwaddr offset, uint64_t value, unsigned size) { - CMSDKAPBTIMER *s = CMSDK_APB_TIMER(opaque); + CMSDKAPBTimer *s = CMSDK_APB_TIMER(opaque); trace_cmsdk_apb_timer_write(offset, value, size); @@ -XXX,XX +XXX,XX @@ static const MemoryRegionOps cmsdk_apb_timer_ops = { static void cmsdk_apb_timer_tick(void *opaque) { - CMSDKAPBTIMER *s = CMSDK_APB_TIMER(opaque); + CMSDKAPBTimer *s = CMSDK_APB_TIMER(opaque); if (s->ctrl & R_CTRL_IRQEN_MASK) { s->intstatus |= R_INTSTATUS_IRQ_MASK; @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_timer_tick(void *opaque) static void cmsdk_apb_timer_reset(DeviceState *dev) { - CMSDKAPBTIMER *s = CMSDK_APB_TIMER(dev); + CMSDKAPBTimer *s = CMSDK_APB_TIMER(dev); trace_cmsdk_apb_timer_reset(); s->ctrl = 0; @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_timer_reset(DeviceState *dev) static void cmsdk_apb_timer_init(Object *obj) { SysBusDevice *sbd = SYS_BUS_DEVICE(obj); - CMSDKAPBTIMER *s = CMSDK_APB_TIMER(obj); + CMSDKAPBTimer *s = CMSDK_APB_TIMER(obj); memory_region_init_io(&s->iomem, obj, &cmsdk_apb_timer_ops, s, "cmsdk-apb-timer", 0x1000); @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_timer_init(Object *obj) static void cmsdk_apb_timer_realize(DeviceState *dev, Error **errp) { - CMSDKAPBTIMER *s = CMSDK_APB_TIMER(dev); + CMSDKAPBTimer *s = CMSDK_APB_TIMER(dev); if (s->pclk_frq == 0) { error_setg(errp, "CMSDK APB timer: pclk-frq property must be set"); @@ -XXX,XX +XXX,XX @@ static const VMStateDescription cmsdk_apb_timer_vmstate = { .version_id = 1, .minimum_version_id = 1, .fields = (VMStateField[]) { - VMSTATE_PTIMER(timer, CMSDKAPBTIMER), - VMSTATE_UINT32(ctrl, CMSDKAPBTIMER), - VMSTATE_UINT32(value, CMSDKAPBTIMER), - VMSTATE_UINT32(reload, CMSDKAPBTIMER), - VMSTATE_UINT32(intstatus, CMSDKAPBTIMER), + VMSTATE_PTIMER(timer, CMSDKAPBTimer), + VMSTATE_UINT32(ctrl, CMSDKAPBTimer), + VMSTATE_UINT32(value, CMSDKAPBTimer), + VMSTATE_UINT32(reload, CMSDKAPBTimer), + VMSTATE_UINT32(intstatus, CMSDKAPBTimer), VMSTATE_END_OF_LIST() } }; static Property cmsdk_apb_timer_properties[] = { - DEFINE_PROP_UINT32("pclk-frq", CMSDKAPBTIMER, pclk_frq, 0), + DEFINE_PROP_UINT32("pclk-frq", CMSDKAPBTimer, pclk_frq, 0), DEFINE_PROP_END_OF_LIST(), }; @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_timer_class_init(ObjectClass *klass, void *data) static const TypeInfo cmsdk_apb_timer_info = { .name = TYPE_CMSDK_APB_TIMER, .parent = TYPE_SYS_BUS_DEVICE, - .instance_size = sizeof(CMSDKAPBTIMER), + .instance_size = sizeof(CMSDKAPBTimer), .instance_init = cmsdk_apb_timer_init, .class_init = cmsdk_apb_timer_class_init, }; -- 2.20.1 As the first step in converting the CMSDK_APB_TIMER device to the Clock framework, add a Clock input. For the moment we do nothing with this clock; we will change the behaviour from using the pclk-frq property to using the Clock once all the users of this device have been converted to wire up the Clock. Since the device doesn't already have a doc comment for its "QEMU interface", we add one including the new Clock. This is a migration compatibility break for machines mps2-an505, mps2-an521, musca-a, musca-b1. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-8-peter.maydell@linaro.org Message-id: 20210121190622.22000-8-peter.maydell@linaro.org --- include/hw/timer/cmsdk-apb-timer.h | 9 +++++++++ hw/timer/cmsdk-apb-timer.c | 7 +++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/include/hw/timer/cmsdk-apb-timer.h b/include/hw/timer/cmsdk-apb-timer.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/timer/cmsdk-apb-timer.h +++ b/include/hw/timer/cmsdk-apb-timer.h @@ -XXX,XX +XXX,XX @@ #include "hw/qdev-properties.h" #include "hw/sysbus.h" #include "hw/ptimer.h" +#include "hw/clock.h" #include "qom/object.h" #define TYPE_CMSDK_APB_TIMER "cmsdk-apb-timer" OBJECT_DECLARE_SIMPLE_TYPE(CMSDKAPBTimer, CMSDK_APB_TIMER) +/* + * QEMU interface: + * + QOM property "pclk-frq": frequency at which the timer is clocked + * + Clock input "pclk": clock for the timer + * + sysbus MMIO region 0: the register bank + * + sysbus IRQ 0: timer interrupt TIMERINT + */ struct CMSDKAPBTimer { /*< private >*/ SysBusDevice parent_obj; @@ -XXX,XX +XXX,XX @@ struct CMSDKAPBTimer { qemu_irq timerint; uint32_t pclk_frq; struct ptimer_state *timer; + Clock *pclk; uint32_t ctrl; uint32_t value; diff --git a/hw/timer/cmsdk-apb-timer.c b/hw/timer/cmsdk-apb-timer.c index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/cmsdk-apb-timer.c +++ b/hw/timer/cmsdk-apb-timer.c @@ -XXX,XX +XXX,XX @@ #include "hw/sysbus.h" #include "hw/irq.h" #include "hw/registerfields.h" +#include "hw/qdev-clock.h" #include "hw/timer/cmsdk-apb-timer.h" #include "migration/vmstate.h" @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_timer_init(Object *obj) s, "cmsdk-apb-timer", 0x1000); sysbus_init_mmio(sbd, &s->iomem); sysbus_init_irq(sbd, &s->timerint); + s->pclk = qdev_init_clock_in(DEVICE(s), "pclk", NULL, NULL); } static void cmsdk_apb_timer_realize(DeviceState *dev, Error **errp) @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_timer_realize(DeviceState *dev, Error **errp) static const VMStateDescription cmsdk_apb_timer_vmstate = { .name = "cmsdk-apb-timer", - .version_id = 1, - .minimum_version_id = 1, + .version_id = 2, + .minimum_version_id = 2, .fields = (VMStateField[]) { VMSTATE_PTIMER(timer, CMSDKAPBTimer), + VMSTATE_CLOCK(pclk, CMSDKAPBTimer), VMSTATE_UINT32(ctrl, CMSDKAPBTimer), VMSTATE_UINT32(value, CMSDKAPBTimer), VMSTATE_UINT32(reload, CMSDKAPBTimer), -- 2.20.1 As the first step in converting the CMSDK_APB_DUALTIMER device to the Clock framework, add a Clock input. For the moment we do nothing with this clock; we will change the behaviour from using the pclk-frq property to using the Clock once all the users of this device have been converted to wire up the Clock. We take the opportunity to correct the name of the clock input to match the hardware -- the dual timer names the clock which drives the timers TIMCLK. (It does also have a 'pclk' input, which is used only for the register and APB bus logic; on the SSE-200 these clocks are both connected together.) This is a migration compatibility break for machines mps2-an385, mps2-an386, mps2-an500, mps2-an511, mps2-an505, mps2-an521, musca-a, musca-b1. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-9-peter.maydell@linaro.org Message-id: 20210121190622.22000-9-peter.maydell@linaro.org --- include/hw/timer/cmsdk-apb-dualtimer.h | 3 +++ hw/timer/cmsdk-apb-dualtimer.c | 7 +++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/include/hw/timer/cmsdk-apb-dualtimer.h b/include/hw/timer/cmsdk-apb-dualtimer.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/timer/cmsdk-apb-dualtimer.h +++ b/include/hw/timer/cmsdk-apb-dualtimer.h @@ -XXX,XX +XXX,XX @@ * * QEMU interface: * + QOM property "pclk-frq": frequency at which the timer is clocked + * + Clock input "TIMCLK": clock (for both timers) * + sysbus MMIO region 0: the register bank * + sysbus IRQ 0: combined timer interrupt TIMINTC * + sysbus IRO 1: timer block 1 interrupt TIMINT1 @@ -XXX,XX +XXX,XX @@ #include "hw/sysbus.h" #include "hw/ptimer.h" +#include "hw/clock.h" #include "qom/object.h" #define TYPE_CMSDK_APB_DUALTIMER "cmsdk-apb-dualtimer" @@ -XXX,XX +XXX,XX @@ struct CMSDKAPBDualTimer { MemoryRegion iomem; qemu_irq timerintc; uint32_t pclk_frq; + Clock *timclk; CMSDKAPBDualTimerModule timermod[CMSDK_APB_DUALTIMER_NUM_MODULES]; uint32_t timeritcr; diff --git a/hw/timer/cmsdk-apb-dualtimer.c b/hw/timer/cmsdk-apb-dualtimer.c index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/cmsdk-apb-dualtimer.c +++ b/hw/timer/cmsdk-apb-dualtimer.c @@ -XXX,XX +XXX,XX @@ #include "hw/irq.h" #include "hw/qdev-properties.h" #include "hw/registerfields.h" +#include "hw/qdev-clock.h" #include "hw/timer/cmsdk-apb-dualtimer.h" #include "migration/vmstate.h" @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_dualtimer_init(Object *obj) for (i = 0; i < ARRAY_SIZE(s->timermod); i++) { sysbus_init_irq(sbd, &s->timermod[i].timerint); } + s->timclk = qdev_init_clock_in(DEVICE(s), "TIMCLK", NULL, NULL); } static void cmsdk_apb_dualtimer_realize(DeviceState *dev, Error **errp) @@ -XXX,XX +XXX,XX @@ static const VMStateDescription cmsdk_dualtimermod_vmstate = { static const VMStateDescription cmsdk_apb_dualtimer_vmstate = { .name = "cmsdk-apb-dualtimer", - .version_id = 1, - .minimum_version_id = 1, + .version_id = 2, + .minimum_version_id = 2, .fields = (VMStateField[]) { + VMSTATE_CLOCK(timclk, CMSDKAPBDualTimer), VMSTATE_STRUCT_ARRAY(timermod, CMSDKAPBDualTimer, CMSDK_APB_DUALTIMER_NUM_MODULES, 1, cmsdk_dualtimermod_vmstate, -- 2.20.1 As the first step in converting the CMSDK_APB_TIMER device to the Clock framework, add a Clock input. For the moment we do nothing with this clock; we will change the behaviour from using the wdogclk-frq property to using the Clock once all the users of this device have been converted to wire up the Clock. This is a migration compatibility break for machines mps2-an385, mps2-an386, mps2-an500, mps2-an511, mps2-an505, mps2-an521, musca-a, musca-b1, lm3s811evb, lm3s6965evb. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-10-peter.maydell@linaro.org Message-id: 20210121190622.22000-10-peter.maydell@linaro.org --- include/hw/watchdog/cmsdk-apb-watchdog.h | 3 +++ hw/watchdog/cmsdk-apb-watchdog.c | 7 +++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/include/hw/watchdog/cmsdk-apb-watchdog.h b/include/hw/watchdog/cmsdk-apb-watchdog.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/watchdog/cmsdk-apb-watchdog.h +++ b/include/hw/watchdog/cmsdk-apb-watchdog.h @@ -XXX,XX +XXX,XX @@ * * QEMU interface: * + QOM property "wdogclk-frq": frequency at which the watchdog is clocked + * + Clock input "WDOGCLK": clock for the watchdog's timer * + sysbus MMIO region 0: the register bank * + sysbus IRQ 0: watchdog interrupt * @@ -XXX,XX +XXX,XX @@ #include "hw/sysbus.h" #include "hw/ptimer.h" +#include "hw/clock.h" #include "qom/object.h" #define TYPE_CMSDK_APB_WATCHDOG "cmsdk-apb-watchdog" @@ -XXX,XX +XXX,XX @@ struct CMSDKAPBWatchdog { uint32_t wdogclk_frq; bool is_luminary; struct ptimer_state *timer; + Clock *wdogclk; uint32_t control; uint32_t intstatus; diff --git a/hw/watchdog/cmsdk-apb-watchdog.c b/hw/watchdog/cmsdk-apb-watchdog.c index XXXXXXX..XXXXXXX 100644 --- a/hw/watchdog/cmsdk-apb-watchdog.c +++ b/hw/watchdog/cmsdk-apb-watchdog.c @@ -XXX,XX +XXX,XX @@ #include "hw/irq.h" #include "hw/qdev-properties.h" #include "hw/registerfields.h" +#include "hw/qdev-clock.h" #include "hw/watchdog/cmsdk-apb-watchdog.h" #include "migration/vmstate.h" @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_watchdog_init(Object *obj) s, "cmsdk-apb-watchdog", 0x1000); sysbus_init_mmio(sbd, &s->iomem); sysbus_init_irq(sbd, &s->wdogint); + s->wdogclk = qdev_init_clock_in(DEVICE(s), "WDOGCLK", NULL, NULL); s->is_luminary = false; s->id = cmsdk_apb_watchdog_id; @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_watchdog_realize(DeviceState *dev, Error **errp) static const VMStateDescription cmsdk_apb_watchdog_vmstate = { .name = "cmsdk-apb-watchdog", - .version_id = 1, - .minimum_version_id = 1, + .version_id = 2, + .minimum_version_id = 2, .fields = (VMStateField[]) { + VMSTATE_CLOCK(wdogclk, CMSDKAPBWatchdog), VMSTATE_PTIMER(timer, CMSDKAPBWatchdog), VMSTATE_UINT32(control, CMSDKAPBWatchdog), VMSTATE_UINT32(intstatus, CMSDKAPBWatchdog), -- 2.20.1 While we transition the ARMSSE code from integer properties specifying clock frequencies to Clock objects, we want to have the device provide both at once. We want the final name of the main input Clock to be "MAINCLK", following the hardware name. Unfortunately creating an input Clock with a name X creates an under-the-hood QOM property X; for "MAINCLK" this clashes with the existing UINT32 property of that name. Rename the UINT32 property to MAINCLK_FRQ so it can coexist with the MAINCLK Clock; once the transition is complete MAINCLK_FRQ will be deleted. Commit created with: perl -p -i -e 's/MAINCLK/MAINCLK_FRQ/g' hw/arm/{armsse,mps2-tz,musca}.c include/hw/arm/armsse.h Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-11-peter.maydell@linaro.org Message-id: 20210121190622.22000-11-peter.maydell@linaro.org --- include/hw/arm/armsse.h | 2 +- hw/arm/armsse.c | 6 +++--- hw/arm/mps2-tz.c | 2 +- hw/arm/musca.c | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/hw/arm/armsse.h b/include/hw/arm/armsse.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/armsse.h +++ b/include/hw/arm/armsse.h @@ -XXX,XX +XXX,XX @@ * QEMU interface: * + QOM property "memory" is a MemoryRegion containing the devices provided * by the board model. - * + QOM property "MAINCLK" is the frequency of the main system clock + * + QOM property "MAINCLK_FRQ" is the frequency of the main system clock * + QOM property "EXP_NUMIRQ" sets the number of expansion interrupts. * (In hardware, the SSE-200 permits the number of expansion interrupts * for the two CPUs to be configured separately, but we restrict it to diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/armsse.c +++ b/hw/arm/armsse.c @@ -XXX,XX +XXX,XX @@ static Property iotkit_properties[] = { DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION, MemoryRegion *), DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64), - DEFINE_PROP_UINT32("MAINCLK", ARMSSE, mainclk_frq, 0), + DEFINE_PROP_UINT32("MAINCLK_FRQ", ARMSSE, mainclk_frq, 0), DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15), DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000), DEFINE_PROP_BOOL("CPU0_FPU", ARMSSE, cpu_fpu[0], true), @@ -XXX,XX +XXX,XX @@ static Property armsse_properties[] = { DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION, MemoryRegion *), DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64), - DEFINE_PROP_UINT32("MAINCLK", ARMSSE, mainclk_frq, 0), + DEFINE_PROP_UINT32("MAINCLK_FRQ", ARMSSE, mainclk_frq, 0), DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15), DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000), DEFINE_PROP_BOOL("CPU0_FPU", ARMSSE, cpu_fpu[0], false), @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) } if (!s->mainclk_frq) { - error_setg(errp, "MAINCLK property was not set"); + error_setg(errp, "MAINCLK_FRQ property was not set"); return; } diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/mps2-tz.c +++ b/hw/arm/mps2-tz.c @@ -XXX,XX +XXX,XX @@ static void mps2tz_common_init(MachineState *machine) object_property_set_link(OBJECT(&mms->iotkit), "memory", OBJECT(system_memory), &error_abort); qdev_prop_set_uint32(iotkitdev, "EXP_NUMIRQ", MPS2TZ_NUMIRQ); - qdev_prop_set_uint32(iotkitdev, "MAINCLK", SYSCLK_FRQ); + qdev_prop_set_uint32(iotkitdev, "MAINCLK_FRQ", SYSCLK_FRQ); sysbus_realize(SYS_BUS_DEVICE(&mms->iotkit), &error_fatal); /* diff --git a/hw/arm/musca.c b/hw/arm/musca.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/musca.c +++ b/hw/arm/musca.c @@ -XXX,XX +XXX,XX @@ static void musca_init(MachineState *machine) qdev_prop_set_uint32(ssedev, "EXP_NUMIRQ", mmc->num_irqs); qdev_prop_set_uint32(ssedev, "init-svtor", mmc->init_svtor); qdev_prop_set_uint32(ssedev, "SRAM_ADDR_WIDTH", mmc->sram_addr_width); - qdev_prop_set_uint32(ssedev, "MAINCLK", SYSCLK_FRQ); + qdev_prop_set_uint32(ssedev, "MAINCLK_FRQ", SYSCLK_FRQ); /* * Musca-A takes the default SSE-200 FPU/DSP settings (ie no for * CPU0 and yes for CPU1); Musca-B1 explicitly enables them for CPU0. -- 2.20.1 Create two input clocks on the ARMSSE devices, one for the normal MAINCLK, and one for the 32KHz S32KCLK, and wire these up to the appropriate devices. The old property-based clock frequency setting will remain in place until conversion is complete. This is a migration compatibility break for machines mps2-an505, mps2-an521, musca-a, musca-b1. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-12-peter.maydell@linaro.org Message-id: 20210121190622.22000-12-peter.maydell@linaro.org --- include/hw/arm/armsse.h | 6 ++++++ hw/arm/armsse.c | 17 +++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/include/hw/arm/armsse.h b/include/hw/arm/armsse.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/armsse.h +++ b/include/hw/arm/armsse.h @@ -XXX,XX +XXX,XX @@ * per-CPU identity and control register blocks * * QEMU interface: + * + Clock input "MAINCLK": clock for CPUs and most peripherals + * + Clock input "S32KCLK": slow 32KHz clock used for a few peripherals * + QOM property "memory" is a MemoryRegion containing the devices provided * by the board model. * + QOM property "MAINCLK_FRQ" is the frequency of the main system clock @@ -XXX,XX +XXX,XX @@ #include "hw/misc/armsse-mhu.h" #include "hw/misc/unimp.h" #include "hw/or-irq.h" +#include "hw/clock.h" #include "hw/core/split-irq.h" #include "hw/cpu/cluster.h" #include "qom/object.h" @@ -XXX,XX +XXX,XX @@ struct ARMSSE { uint32_t nsccfg; + Clock *mainclk; + Clock *s32kclk; + /* Properties */ MemoryRegion *board_memory; uint32_t exp_numirq; diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/armsse.c +++ b/hw/arm/armsse.c @@ -XXX,XX +XXX,XX @@ #include "hw/arm/armsse.h" #include "hw/arm/boot.h" #include "hw/irq.h" +#include "hw/qdev-clock.h" /* Format of the System Information block SYS_CONFIG register */ typedef enum SysConfigFormat { @@ -XXX,XX +XXX,XX @@ static void armsse_init(Object *obj) assert(info->sram_banks <= MAX_SRAM_BANKS); assert(info->num_cpus <= SSE_MAX_CPUS); + s->mainclk = qdev_init_clock_in(DEVICE(s), "MAINCLK", NULL, NULL); + s->s32kclk = qdev_init_clock_in(DEVICE(s), "S32KCLK", NULL, NULL); + memory_region_init(&s->container, obj, "armsse-container", UINT64_MAX); for (i = 0; i < info->num_cpus; i++) { @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) * map its upstream ends to the right place in the container. */ qdev_prop_set_uint32(DEVICE(&s->timer0), "pclk-frq", s->mainclk_frq); + qdev_connect_clock_in(DEVICE(&s->timer0), "pclk", s->mainclk); if (!sysbus_realize(SYS_BUS_DEVICE(&s->timer0), errp)) { return; } @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) &error_abort); qdev_prop_set_uint32(DEVICE(&s->timer1), "pclk-frq", s->mainclk_frq); + qdev_connect_clock_in(DEVICE(&s->timer1), "pclk", s->mainclk); if (!sysbus_realize(SYS_BUS_DEVICE(&s->timer1), errp)) { return; } @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) &error_abort); qdev_prop_set_uint32(DEVICE(&s->dualtimer), "pclk-frq", s->mainclk_frq); + qdev_connect_clock_in(DEVICE(&s->dualtimer), "TIMCLK", s->mainclk); if (!sysbus_realize(SYS_BUS_DEVICE(&s->dualtimer), errp)) { return; } @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) * 0x4002f000: S32K timer */ qdev_prop_set_uint32(DEVICE(&s->s32ktimer), "pclk-frq", S32KCLK); + qdev_connect_clock_in(DEVICE(&s->s32ktimer), "pclk", s->s32kclk); if (!sysbus_realize(SYS_BUS_DEVICE(&s->s32ktimer), errp)) { return; } @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) qdev_get_gpio_in_named(DEVICE(&s->armv7m), "NMI", 0)); qdev_prop_set_uint32(DEVICE(&s->s32kwatchdog), "wdogclk-frq", S32KCLK); + qdev_connect_clock_in(DEVICE(&s->s32kwatchdog), "WDOGCLK", s->s32kclk); if (!sysbus_realize(SYS_BUS_DEVICE(&s->s32kwatchdog), errp)) { return; } @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) /* 0x40080000 .. 0x4008ffff : ARMSSE second Base peripheral region */ qdev_prop_set_uint32(DEVICE(&s->nswatchdog), "wdogclk-frq", s->mainclk_frq); + qdev_connect_clock_in(DEVICE(&s->nswatchdog), "WDOGCLK", s->mainclk); if (!sysbus_realize(SYS_BUS_DEVICE(&s->nswatchdog), errp)) { return; } @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) sysbus_mmio_map(SYS_BUS_DEVICE(&s->nswatchdog), 0, 0x40081000); qdev_prop_set_uint32(DEVICE(&s->swatchdog), "wdogclk-frq", s->mainclk_frq); + qdev_connect_clock_in(DEVICE(&s->swatchdog), "WDOGCLK", s->mainclk); if (!sysbus_realize(SYS_BUS_DEVICE(&s->swatchdog), errp)) { return; } @@ -XXX,XX +XXX,XX @@ static void armsse_idau_check(IDAUInterface *ii, uint32_t address, static const VMStateDescription armsse_vmstate = { .name = "iotkit", - .version_id = 1, - .minimum_version_id = 1, + .version_id = 2, + .minimum_version_id = 2, .fields = (VMStateField[]) { + VMSTATE_CLOCK(mainclk, ARMSSE), + VMSTATE_CLOCK(s32kclk, ARMSSE), VMSTATE_UINT32(nsccfg, ARMSSE), VMSTATE_END_OF_LIST() } -- 2.20.1 The old-style convenience function cmsdk_apb_timer_create() for creating CMSDK_APB_TIMER objects is used in only two places in mps2.c. Most of the rest of the code in that file uses the new "initialize in place" coding style. We want to connect up a Clock object which should be done between the object creation and realization; rather than adding a Clock* argument to the convenience function, convert the timer creation code in mps2.c to the same style as is used already for the watchdog, dualtimer and other devices, and delete the now-unused convenience function. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-13-peter.maydell@linaro.org Message-id: 20210121190622.22000-13-peter.maydell@linaro.org --- include/hw/timer/cmsdk-apb-timer.h | 21 --------------------- hw/arm/mps2.c | 18 ++++++++++++++++-- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/include/hw/timer/cmsdk-apb-timer.h b/include/hw/timer/cmsdk-apb-timer.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/timer/cmsdk-apb-timer.h +++ b/include/hw/timer/cmsdk-apb-timer.h @@ -XXX,XX +XXX,XX @@ struct CMSDKAPBTimer { uint32_t intstatus; }; -/** - * cmsdk_apb_timer_create - convenience function to create TYPE_CMSDK_APB_TIMER - * @addr: location in system memory to map registers - * @pclk_frq: frequency in Hz of the PCLK clock (used for calculating baud rate) - */ -static inline DeviceState *cmsdk_apb_timer_create(hwaddr addr, - qemu_irq timerint, - uint32_t pclk_frq) -{ - DeviceState *dev; - SysBusDevice *s; - - dev = qdev_new(TYPE_CMSDK_APB_TIMER); - s = SYS_BUS_DEVICE(dev); - qdev_prop_set_uint32(dev, "pclk-frq", pclk_frq); - sysbus_realize_and_unref(s, &error_fatal); - sysbus_mmio_map(s, 0, addr); - sysbus_connect_irq(s, 0, timerint); - return dev; -} - #endif diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/mps2.c +++ b/hw/arm/mps2.c @@ -XXX,XX +XXX,XX @@ struct MPS2MachineState { /* CMSDK APB subsystem */ CMSDKAPBDualTimer dualtimer; CMSDKAPBWatchdog watchdog; + CMSDKAPBTimer timer[2]; }; #define TYPE_MPS2_MACHINE "mps2" @@ -XXX,XX +XXX,XX @@ static void mps2_common_init(MachineState *machine) } /* CMSDK APB subsystem */ - cmsdk_apb_timer_create(0x40000000, qdev_get_gpio_in(armv7m, 8), SYSCLK_FRQ); - cmsdk_apb_timer_create(0x40001000, qdev_get_gpio_in(armv7m, 9), SYSCLK_FRQ); + for (i = 0; i < ARRAY_SIZE(mms->timer); i++) { + g_autofree char *name = g_strdup_printf("timer%d", i); + hwaddr base = 0x40000000 + i * 0x1000; + int irqno = 8 + i; + SysBusDevice *sbd; + + object_initialize_child(OBJECT(mms), name, &mms->timer[i], + TYPE_CMSDK_APB_TIMER); + sbd = SYS_BUS_DEVICE(&mms->timer[i]); + qdev_prop_set_uint32(DEVICE(&mms->timer[i]), "pclk-frq", SYSCLK_FRQ); + sysbus_realize_and_unref(sbd, &error_fatal); + sysbus_mmio_map(sbd, 0, base); + sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(armv7m, irqno)); + } + object_initialize_child(OBJECT(mms), "dualtimer", &mms->dualtimer, TYPE_CMSDK_APB_DUALTIMER); qdev_prop_set_uint32(DEVICE(&mms->dualtimer), "pclk-frq", SYSCLK_FRQ); -- 2.20.1 Create a fixed-frequency Clock object to be the SYSCLK, and wire it up to the devices that require it. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-14-peter.maydell@linaro.org Message-id: 20210121190622.22000-14-peter.maydell@linaro.org --- hw/arm/mps2.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/mps2.c +++ b/hw/arm/mps2.c @@ -XXX,XX +XXX,XX @@ #include "hw/net/lan9118.h" #include "net/net.h" #include "hw/watchdog/cmsdk-apb-watchdog.h" +#include "hw/qdev-clock.h" #include "qom/object.h" typedef enum MPS2FPGAType { @@ -XXX,XX +XXX,XX @@ struct MPS2MachineState { CMSDKAPBDualTimer dualtimer; CMSDKAPBWatchdog watchdog; CMSDKAPBTimer timer[2]; + Clock *sysclk; }; #define TYPE_MPS2_MACHINE "mps2" @@ -XXX,XX +XXX,XX @@ static void mps2_common_init(MachineState *machine) exit(EXIT_FAILURE); } + /* This clock doesn't need migration because it is fixed-frequency */ + mms->sysclk = clock_new(OBJECT(machine), "SYSCLK"); + clock_set_hz(mms->sysclk, SYSCLK_FRQ); + /* The FPGA images have an odd combination of different RAMs, * because in hardware they are different implementations and * connected to different buses, giving varying performance/size @@ -XXX,XX +XXX,XX @@ static void mps2_common_init(MachineState *machine) TYPE_CMSDK_APB_TIMER); sbd = SYS_BUS_DEVICE(&mms->timer[i]); qdev_prop_set_uint32(DEVICE(&mms->timer[i]), "pclk-frq", SYSCLK_FRQ); + qdev_connect_clock_in(DEVICE(&mms->timer[i]), "pclk", mms->sysclk); sysbus_realize_and_unref(sbd, &error_fatal); sysbus_mmio_map(sbd, 0, base); sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(armv7m, irqno)); @@ -XXX,XX +XXX,XX @@ static void mps2_common_init(MachineState *machine) object_initialize_child(OBJECT(mms), "dualtimer", &mms->dualtimer, TYPE_CMSDK_APB_DUALTIMER); qdev_prop_set_uint32(DEVICE(&mms->dualtimer), "pclk-frq", SYSCLK_FRQ); + qdev_connect_clock_in(DEVICE(&mms->dualtimer), "TIMCLK", mms->sysclk); sysbus_realize(SYS_BUS_DEVICE(&mms->dualtimer), &error_fatal); sysbus_connect_irq(SYS_BUS_DEVICE(&mms->dualtimer), 0, qdev_get_gpio_in(armv7m, 10)); @@ -XXX,XX +XXX,XX @@ static void mps2_common_init(MachineState *machine) object_initialize_child(OBJECT(mms), "watchdog", &mms->watchdog, TYPE_CMSDK_APB_WATCHDOG); qdev_prop_set_uint32(DEVICE(&mms->watchdog), "wdogclk-frq", SYSCLK_FRQ); + qdev_connect_clock_in(DEVICE(&mms->watchdog), "WDOGCLK", mms->sysclk); sysbus_realize(SYS_BUS_DEVICE(&mms->watchdog), &error_fatal); sysbus_connect_irq(SYS_BUS_DEVICE(&mms->watchdog), 0, qdev_get_gpio_in_named(armv7m, "NMI", 0)); -- 2.20.1 Create and connect the two clocks needed by the ARMSSE. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-15-peter.maydell@linaro.org Message-id: 20210121190622.22000-15-peter.maydell@linaro.org --- hw/arm/mps2-tz.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/mps2-tz.c +++ b/hw/arm/mps2-tz.c @@ -XXX,XX +XXX,XX @@ #include "hw/net/lan9118.h" #include "net/net.h" #include "hw/core/split-irq.h" +#include "hw/qdev-clock.h" #include "qom/object.h" #define MPS2TZ_NUMIRQ 92 @@ -XXX,XX +XXX,XX @@ struct MPS2TZMachineState { qemu_or_irq uart_irq_orgate; DeviceState *lan9118; SplitIRQ cpu_irq_splitter[MPS2TZ_NUMIRQ]; + Clock *sysclk; + Clock *s32kclk; }; #define TYPE_MPS2TZ_MACHINE "mps2tz" @@ -XXX,XX +XXX,XX @@ OBJECT_DECLARE_TYPE(MPS2TZMachineState, MPS2TZMachineClass, MPS2TZ_MACHINE) /* Main SYSCLK frequency in Hz */ #define SYSCLK_FRQ 20000000 +/* Slow 32Khz S32KCLK frequency in Hz */ +#define S32KCLK_FRQ (32 * 1000) /* Create an alias of an entire original MemoryRegion @orig * located at @base in the memory map. @@ -XXX,XX +XXX,XX @@ static void mps2tz_common_init(MachineState *machine) exit(EXIT_FAILURE); } + /* These clocks don't need migration because they are fixed-frequency */ + mms->sysclk = clock_new(OBJECT(machine), "SYSCLK"); + clock_set_hz(mms->sysclk, SYSCLK_FRQ); + mms->s32kclk = clock_new(OBJECT(machine), "S32KCLK"); + clock_set_hz(mms->s32kclk, S32KCLK_FRQ); + object_initialize_child(OBJECT(machine), TYPE_IOTKIT, &mms->iotkit, mmc->armsse_type); iotkitdev = DEVICE(&mms->iotkit); @@ -XXX,XX +XXX,XX @@ static void mps2tz_common_init(MachineState *machine) OBJECT(system_memory), &error_abort); qdev_prop_set_uint32(iotkitdev, "EXP_NUMIRQ", MPS2TZ_NUMIRQ); qdev_prop_set_uint32(iotkitdev, "MAINCLK_FRQ", SYSCLK_FRQ); + qdev_connect_clock_in(iotkitdev, "MAINCLK", mms->sysclk); + qdev_connect_clock_in(iotkitdev, "S32KCLK", mms->s32kclk); sysbus_realize(SYS_BUS_DEVICE(&mms->iotkit), &error_fatal); /* -- 2.20.1 Create and connect the two clocks needed by the ARMSSE. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-16-peter.maydell@linaro.org Message-id: 20210121190622.22000-16-peter.maydell@linaro.org --- hw/arm/musca.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/hw/arm/musca.c b/hw/arm/musca.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/musca.c +++ b/hw/arm/musca.c @@ -XXX,XX +XXX,XX @@ #include "hw/misc/tz-ppc.h" #include "hw/misc/unimp.h" #include "hw/rtc/pl031.h" +#include "hw/qdev-clock.h" #include "qom/object.h" #define MUSCA_NUMIRQ_MAX 96 @@ -XXX,XX +XXX,XX @@ struct MuscaMachineState { UnimplementedDeviceState sdio; UnimplementedDeviceState gpio; UnimplementedDeviceState cryptoisland; + Clock *sysclk; + Clock *s32kclk; }; #define TYPE_MUSCA_MACHINE "musca" @@ -XXX,XX +XXX,XX @@ OBJECT_DECLARE_TYPE(MuscaMachineState, MuscaMachineClass, MUSCA_MACHINE) * don't model that in our SSE-200 model yet. */ #define SYSCLK_FRQ 40000000 +/* Slow 32Khz S32KCLK frequency in Hz */ +#define S32KCLK_FRQ (32 * 1000) static qemu_irq get_sse_irq_in(MuscaMachineState *mms, int irqno) { @@ -XXX,XX +XXX,XX @@ static void musca_init(MachineState *machine) exit(1); } + mms->sysclk = clock_new(OBJECT(machine), "SYSCLK"); + clock_set_hz(mms->sysclk, SYSCLK_FRQ); + mms->s32kclk = clock_new(OBJECT(machine), "S32KCLK"); + clock_set_hz(mms->s32kclk, S32KCLK_FRQ); + object_initialize_child(OBJECT(machine), "sse-200", &mms->sse, TYPE_SSE200); ssedev = DEVICE(&mms->sse); @@ -XXX,XX +XXX,XX @@ static void musca_init(MachineState *machine) qdev_prop_set_uint32(ssedev, "init-svtor", mmc->init_svtor); qdev_prop_set_uint32(ssedev, "SRAM_ADDR_WIDTH", mmc->sram_addr_width); qdev_prop_set_uint32(ssedev, "MAINCLK_FRQ", SYSCLK_FRQ); + qdev_connect_clock_in(ssedev, "MAINCLK", mms->sysclk); + qdev_connect_clock_in(ssedev, "S32KCLK", mms->s32kclk); /* * Musca-A takes the default SSE-200 FPU/DSP settings (ie no for * CPU0 and yes for CPU1); Musca-B1 explicitly enables them for CPU0. -- 2.20.1 Convert the SSYS code in the Stellaris boards (which encapsulates the system registers) to a proper QOM device. This will provide us with somewhere to put the output Clock whose frequency depends on the setting of the PLL configuration registers. This is a migration compatibility break for lm3s811evb, lm3s6965evb. We use 3-phase reset here because the Clock will need to propagate its value in the hold phase. For the moment we reset the device during the board creation so that the system_clock_scale global gets set; this will be removed in a subsequent commit. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-17-peter.maydell@linaro.org Message-id: 20210121190622.22000-17-peter.maydell@linaro.org Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/arm/stellaris.c | 132 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 107 insertions(+), 25 deletions(-) diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -XXX,XX +XXX,XX @@ static void stellaris_gptm_realize(DeviceState *dev, Error **errp) /* System controller. */ -typedef struct { +#define TYPE_STELLARIS_SYS "stellaris-sys" +OBJECT_DECLARE_SIMPLE_TYPE(ssys_state, STELLARIS_SYS) + +struct ssys_state { + SysBusDevice parent_obj; + MemoryRegion iomem; uint32_t pborctl; uint32_t ldopctl; @@ -XXX,XX +XXX,XX @@ typedef struct { uint32_t dcgc[3]; uint32_t clkvclr; uint32_t ldoarst; + qemu_irq irq; + /* Properties (all read-only registers) */ uint32_t user0; uint32_t user1; - qemu_irq irq; - stellaris_board_info *board; -} ssys_state; + uint32_t did0; + uint32_t did1; + uint32_t dc0; + uint32_t dc1; + uint32_t dc2; + uint32_t dc3; + uint32_t dc4; +}; static void ssys_update(ssys_state *s) { @@ -XXX,XX +XXX,XX @@ static uint32_t pllcfg_fury[16] = { static int ssys_board_class(const ssys_state *s) { - uint32_t did0 = s->board->did0; + uint32_t did0 = s->did0; switch (did0 & DID0_VER_MASK) { case DID0_VER_0: return DID0_CLASS_SANDSTORM; @@ -XXX,XX +XXX,XX @@ static uint64_t ssys_read(void *opaque, hwaddr offset, switch (offset) { case 0x000: /* DID0 */ - return s->board->did0; + return s->did0; case 0x004: /* DID1 */ - return s->board->did1; + return s->did1; case 0x008: /* DC0 */ - return s->board->dc0; + return s->dc0; case 0x010: /* DC1 */ - return s->board->dc1; + return s->dc1; case 0x014: /* DC2 */ - return s->board->dc2; + return s->dc2; case 0x018: /* DC3 */ - return s->board->dc3; + return s->dc3; case 0x01c: /* DC4 */ - return s->board->dc4; + return s->dc4; case 0x030: /* PBORCTL */ return s->pborctl; case 0x034: /* LDOPCTL */ @@ -XXX,XX +XXX,XX @@ static const MemoryRegionOps ssys_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -static void ssys_reset(void *opaque) +static void stellaris_sys_reset_enter(Object *obj, ResetType type) { - ssys_state *s = (ssys_state *)opaque; + ssys_state *s = STELLARIS_SYS(obj); s->pborctl = 0x7ffd; s->rcc = 0x078e3ac0; @@ -XXX,XX +XXX,XX @@ static void ssys_reset(void *opaque) s->rcgc[0] = 1; s->scgc[0] = 1; s->dcgc[0] = 1; +} + +static void stellaris_sys_reset_hold(Object *obj) +{ + ssys_state *s = STELLARIS_SYS(obj); + ssys_calculate_system_clock(s); } +static void stellaris_sys_reset_exit(Object *obj) +{ +} + static int stellaris_sys_post_load(void *opaque, int version_id) { ssys_state *s = opaque; @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_stellaris_sys = { } }; +static Property stellaris_sys_properties[] = { + DEFINE_PROP_UINT32("user0", ssys_state, user0, 0), + DEFINE_PROP_UINT32("user1", ssys_state, user1, 0), + DEFINE_PROP_UINT32("did0", ssys_state, did0, 0), + DEFINE_PROP_UINT32("did1", ssys_state, did1, 0), + DEFINE_PROP_UINT32("dc0", ssys_state, dc0, 0), + DEFINE_PROP_UINT32("dc1", ssys_state, dc1, 0), + DEFINE_PROP_UINT32("dc2", ssys_state, dc2, 0), + DEFINE_PROP_UINT32("dc3", ssys_state, dc3, 0), + DEFINE_PROP_UINT32("dc4", ssys_state, dc4, 0), + DEFINE_PROP_END_OF_LIST() +}; + +static void stellaris_sys_instance_init(Object *obj) +{ + ssys_state *s = STELLARIS_SYS(obj); + SysBusDevice *sbd = SYS_BUS_DEVICE(s); + + memory_region_init_io(&s->iomem, obj, &ssys_ops, s, "ssys", 0x00001000); + sysbus_init_mmio(sbd, &s->iomem); + sysbus_init_irq(sbd, &s->irq); +} + static int stellaris_sys_init(uint32_t base, qemu_irq irq, stellaris_board_info * board, uint8_t *macaddr) { - ssys_state *s; + DeviceState *dev = qdev_new(TYPE_STELLARIS_SYS); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); - s = g_new0(ssys_state, 1); - s->irq = irq; - s->board = board; /* Most devices come preprogrammed with a MAC address in the user data. */ - s->user0 = macaddr[0] | (macaddr[1] << 8) | (macaddr[2] << 16); - s->user1 = macaddr[3] | (macaddr[4] << 8) | (macaddr[5] << 16); + qdev_prop_set_uint32(dev, "user0", + macaddr[0] | (macaddr[1] << 8) | (macaddr[2] << 16)); + qdev_prop_set_uint32(dev, "user1", + macaddr[3] | (macaddr[4] << 8) | (macaddr[5] << 16)); + qdev_prop_set_uint32(dev, "did0", board->did0); + qdev_prop_set_uint32(dev, "did1", board->did1); + qdev_prop_set_uint32(dev, "dc0", board->dc0); + qdev_prop_set_uint32(dev, "dc1", board->dc1); + qdev_prop_set_uint32(dev, "dc2", board->dc2); + qdev_prop_set_uint32(dev, "dc3", board->dc3); + qdev_prop_set_uint32(dev, "dc4", board->dc4); + + sysbus_realize_and_unref(sbd, &error_fatal); + sysbus_mmio_map(sbd, 0, base); + sysbus_connect_irq(sbd, 0, irq); + + /* + * Normally we should not be resetting devices like this during + * board creation. For the moment we need to do so, because + * system_clock_scale will only get set when the STELLARIS_SYS + * device is reset, and we need its initial value to pass to + * the watchdog device. This hack can be removed once the + * watchdog has been converted to use a Clock input instead. + */ + device_cold_reset(dev); - memory_region_init_io(&s->iomem, NULL, &ssys_ops, s, "ssys", 0x00001000); - memory_region_add_subregion(get_system_memory(), base, &s->iomem); - ssys_reset(s); - vmstate_register(NULL, VMSTATE_INSTANCE_ID_ANY, &vmstate_stellaris_sys, s); return 0; } - /* I2C controller. */ #define TYPE_STELLARIS_I2C "stellaris-i2c" @@ -XXX,XX +XXX,XX @@ static const TypeInfo stellaris_adc_info = { .class_init = stellaris_adc_class_init, }; +static void stellaris_sys_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + ResettableClass *rc = RESETTABLE_CLASS(klass); + + dc->vmsd = &vmstate_stellaris_sys; + rc->phases.enter = stellaris_sys_reset_enter; + rc->phases.hold = stellaris_sys_reset_hold; + rc->phases.exit = stellaris_sys_reset_exit; + device_class_set_props(dc, stellaris_sys_properties); +} + +static const TypeInfo stellaris_sys_info = { + .name = TYPE_STELLARIS_SYS, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(ssys_state), + .instance_init = stellaris_sys_instance_init, + .class_init = stellaris_sys_class_init, +}; + static void stellaris_register_types(void) { type_register_static(&stellaris_i2c_info); type_register_static(&stellaris_gptm_info); type_register_static(&stellaris_adc_info); + type_register_static(&stellaris_sys_info); } type_init(stellaris_register_types) -- 2.20.1 Create and connect the Clock input for the watchdog device on the Stellaris boards. Because the Stellaris boards model the ability to change the clock rate by programming PLL registers, we have to create an output Clock on the ssys_state device and wire it up to the watchdog. Note that the old comment on ssys_calculate_system_clock() got the units wrong -- system_clock_scale is in nanoseconds, not milliseconds. Improve the commentary to clarify how we are calculating the period. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-18-peter.maydell@linaro.org Message-id: 20210121190622.22000-18-peter.maydell@linaro.org Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/arm/stellaris.c | 43 +++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -XXX,XX +XXX,XX @@ #include "hw/watchdog/cmsdk-apb-watchdog.h" #include "migration/vmstate.h" #include "hw/misc/unimp.h" +#include "hw/qdev-clock.h" #include "cpu.h" #include "qom/object.h" @@ -XXX,XX +XXX,XX @@ struct ssys_state { uint32_t clkvclr; uint32_t ldoarst; qemu_irq irq; + Clock *sysclk; /* Properties (all read-only registers) */ uint32_t user0; uint32_t user1; @@ -XXX,XX +XXX,XX @@ static bool ssys_use_rcc2(ssys_state *s) } /* - * Caculate the sys. clock period in ms. + * Calculate the system clock period. We only want to propagate + * this change to the rest of the system if we're not being called + * from migration post-load. */ -static void ssys_calculate_system_clock(ssys_state *s) +static void ssys_calculate_system_clock(ssys_state *s, bool propagate_clock) { + /* + * SYSDIV field specifies divisor: 0 == /1, 1 == /2, etc. Input + * clock is 200MHz, which is a period of 5 ns. Dividing the clock + * frequency by X is the same as multiplying the period by X. + */ if (ssys_use_rcc2(s)) { system_clock_scale = 5 * (((s->rcc2 >> 23) & 0x3f) + 1); } else { system_clock_scale = 5 * (((s->rcc >> 23) & 0xf) + 1); } + clock_set_ns(s->sysclk, system_clock_scale); + if (propagate_clock) { + clock_propagate(s->sysclk); + } } static void ssys_write(void *opaque, hwaddr offset, @@ -XXX,XX +XXX,XX @@ static void ssys_write(void *opaque, hwaddr offset, s->int_status |= (1 << 6); } s->rcc = value; - ssys_calculate_system_clock(s); + ssys_calculate_system_clock(s, true); break; case 0x070: /* RCC2 */ if (ssys_board_class(s) == DID0_CLASS_SANDSTORM) { @@ -XXX,XX +XXX,XX @@ static void ssys_write(void *opaque, hwaddr offset, s->int_status |= (1 << 6); } s->rcc2 = value; - ssys_calculate_system_clock(s); + ssys_calculate_system_clock(s, true); break; case 0x100: /* RCGC0 */ s->rcgc[0] = value; @@ -XXX,XX +XXX,XX @@ static void stellaris_sys_reset_hold(Object *obj) { ssys_state *s = STELLARIS_SYS(obj); - ssys_calculate_system_clock(s); + /* OK to propagate clocks from the hold phase */ + ssys_calculate_system_clock(s, true); } static void stellaris_sys_reset_exit(Object *obj) @@ -XXX,XX +XXX,XX @@ static int stellaris_sys_post_load(void *opaque, int version_id) { ssys_state *s = opaque; - ssys_calculate_system_clock(s); + ssys_calculate_system_clock(s, false); return 0; } @@ -XXX,XX +XXX,XX @@ static const VMStateDescription vmstate_stellaris_sys = { VMSTATE_UINT32_ARRAY(dcgc, ssys_state, 3), VMSTATE_UINT32(clkvclr, ssys_state), VMSTATE_UINT32(ldoarst, ssys_state), + /* No field for sysclk -- handled in post-load instead */ VMSTATE_END_OF_LIST() } }; @@ -XXX,XX +XXX,XX @@ static void stellaris_sys_instance_init(Object *obj) memory_region_init_io(&s->iomem, obj, &ssys_ops, s, "ssys", 0x00001000); sysbus_init_mmio(sbd, &s->iomem); sysbus_init_irq(sbd, &s->irq); + s->sysclk = qdev_init_clock_out(DEVICE(s), "SYSCLK"); } -static int stellaris_sys_init(uint32_t base, qemu_irq irq, - stellaris_board_info * board, - uint8_t *macaddr) +static DeviceState *stellaris_sys_init(uint32_t base, qemu_irq irq, + stellaris_board_info *board, + uint8_t *macaddr) { DeviceState *dev = qdev_new(TYPE_STELLARIS_SYS); SysBusDevice *sbd = SYS_BUS_DEVICE(dev); @@ -XXX,XX +XXX,XX @@ static int stellaris_sys_init(uint32_t base, qemu_irq irq, */ device_cold_reset(dev); - return 0; + return dev; } /* I2C controller. */ @@ -XXX,XX +XXX,XX @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board) int flash_size; I2CBus *i2c; DeviceState *dev; + DeviceState *ssys_dev; int i; int j; @@ -XXX,XX +XXX,XX @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board) } } - stellaris_sys_init(0x400fe000, qdev_get_gpio_in(nvic, 28), - board, nd_table[0].macaddr.a); + ssys_dev = stellaris_sys_init(0x400fe000, qdev_get_gpio_in(nvic, 28), + board, nd_table[0].macaddr.a); if (board->dc1 & (1 << 3)) { /* watchdog present */ @@ -XXX,XX +XXX,XX @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board) /* system_clock_scale is valid now */ uint32_t mainclk = NANOSECONDS_PER_SECOND / system_clock_scale; qdev_prop_set_uint32(dev, "wdogclk-frq", mainclk); + qdev_connect_clock_in(dev, "WDOGCLK", + qdev_get_clock_out(ssys_dev, "SYSCLK")); sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); sysbus_mmio_map(SYS_BUS_DEVICE(dev), -- 2.20.1 Switch the CMSDK APB timer device over to using its Clock input; the pclk-frq property is now ignored. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-19-peter.maydell@linaro.org Message-id: 20210121190622.22000-19-peter.maydell@linaro.org --- hw/timer/cmsdk-apb-timer.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/hw/timer/cmsdk-apb-timer.c b/hw/timer/cmsdk-apb-timer.c index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/cmsdk-apb-timer.c +++ b/hw/timer/cmsdk-apb-timer.c @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_timer_reset(DeviceState *dev) ptimer_transaction_commit(s->timer); } +static void cmsdk_apb_timer_clk_update(void *opaque) +{ + CMSDKAPBTimer *s = CMSDK_APB_TIMER(opaque); + + ptimer_transaction_begin(s->timer); + ptimer_set_period_from_clock(s->timer, s->pclk, 1); + ptimer_transaction_commit(s->timer); +} + static void cmsdk_apb_timer_init(Object *obj) { SysBusDevice *sbd = SYS_BUS_DEVICE(obj); @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_timer_init(Object *obj) s, "cmsdk-apb-timer", 0x1000); sysbus_init_mmio(sbd, &s->iomem); sysbus_init_irq(sbd, &s->timerint); - s->pclk = qdev_init_clock_in(DEVICE(s), "pclk", NULL, NULL); + s->pclk = qdev_init_clock_in(DEVICE(s), "pclk", + cmsdk_apb_timer_clk_update, s); } static void cmsdk_apb_timer_realize(DeviceState *dev, Error **errp) { CMSDKAPBTimer *s = CMSDK_APB_TIMER(dev); - if (s->pclk_frq == 0) { - error_setg(errp, "CMSDK APB timer: pclk-frq property must be set"); + if (!clock_has_source(s->pclk)) { + error_setg(errp, "CMSDK APB timer: pclk clock must be connected"); return; } @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_timer_realize(DeviceState *dev, Error **errp) PTIMER_POLICY_NO_COUNTER_ROUND_DOWN); ptimer_transaction_begin(s->timer); - ptimer_set_freq(s->timer, s->pclk_frq); + ptimer_set_period_from_clock(s->timer, s->pclk, 1); ptimer_transaction_commit(s->timer); } -- 2.20.1 Switch the CMSDK APB dualtimer device over to using its Clock input; the pclk-frq property is now ignored. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-20-peter.maydell@linaro.org Message-id: 20210121190622.22000-20-peter.maydell@linaro.org Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/timer/cmsdk-apb-dualtimer.c | 42 ++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/hw/timer/cmsdk-apb-dualtimer.c b/hw/timer/cmsdk-apb-dualtimer.c index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/cmsdk-apb-dualtimer.c +++ b/hw/timer/cmsdk-apb-dualtimer.c @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_dualtimer_update(CMSDKAPBDualTimer *s) qemu_set_irq(s->timerintc, timintc); } +static int cmsdk_dualtimermod_divisor(CMSDKAPBDualTimerModule *m) +{ + /* Return the divisor set by the current CONTROL.PRESCALE value */ + switch (FIELD_EX32(m->control, CONTROL, PRESCALE)) { + case 0: + return 1; + case 1: + return 16; + case 2: + case 3: /* UNDEFINED, we treat like 2 (and complained when it was set) */ + return 256; + default: + g_assert_not_reached(); + } +} + static void cmsdk_dualtimermod_write_control(CMSDKAPBDualTimerModule *m, uint32_t newctrl) { @@ -XXX,XX +XXX,XX @@ static void cmsdk_dualtimermod_write_control(CMSDKAPBDualTimerModule *m, default: g_assert_not_reached(); } - ptimer_set_freq(m->timer, m->parent->pclk_frq / divisor); + ptimer_set_period_from_clock(m->timer, m->parent->timclk, divisor); } if (changed & R_CONTROL_MODE_MASK) { @@ -XXX,XX +XXX,XX @@ static void cmsdk_dualtimermod_reset(CMSDKAPBDualTimerModule *m) * limit must both be set to 0xffff, so we wrap at 16 bits. */ ptimer_set_limit(m->timer, 0xffff, 1); - ptimer_set_freq(m->timer, m->parent->pclk_frq); + ptimer_set_period_from_clock(m->timer, m->parent->timclk, + cmsdk_dualtimermod_divisor(m)); ptimer_transaction_commit(m->timer); } @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_dualtimer_reset(DeviceState *dev) s->timeritop = 0; } +static void cmsdk_apb_dualtimer_clk_update(void *opaque) +{ + CMSDKAPBDualTimer *s = CMSDK_APB_DUALTIMER(opaque); + int i; + + for (i = 0; i < ARRAY_SIZE(s->timermod); i++) { + CMSDKAPBDualTimerModule *m = &s->timermod[i]; + ptimer_transaction_begin(m->timer); + ptimer_set_period_from_clock(m->timer, m->parent->timclk, + cmsdk_dualtimermod_divisor(m)); + ptimer_transaction_commit(m->timer); + } +} + static void cmsdk_apb_dualtimer_init(Object *obj) { SysBusDevice *sbd = SYS_BUS_DEVICE(obj); @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_dualtimer_init(Object *obj) for (i = 0; i < ARRAY_SIZE(s->timermod); i++) { sysbus_init_irq(sbd, &s->timermod[i].timerint); } - s->timclk = qdev_init_clock_in(DEVICE(s), "TIMCLK", NULL, NULL); + s->timclk = qdev_init_clock_in(DEVICE(s), "TIMCLK", + cmsdk_apb_dualtimer_clk_update, s); } static void cmsdk_apb_dualtimer_realize(DeviceState *dev, Error **errp) @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_dualtimer_realize(DeviceState *dev, Error **errp) CMSDKAPBDualTimer *s = CMSDK_APB_DUALTIMER(dev); int i; - if (s->pclk_frq == 0) { - error_setg(errp, "CMSDK APB timer: pclk-frq property must be set"); + if (!clock_has_source(s->timclk)) { + error_setg(errp, "CMSDK APB dualtimer: TIMCLK clock must be connected"); return; } -- 2.20.1 Switch the CMSDK APB watchdog device over to using its Clock input; the wdogclk_frq property is now ignored. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-21-peter.maydell@linaro.org Message-id: 20210121190622.22000-21-peter.maydell@linaro.org --- hw/watchdog/cmsdk-apb-watchdog.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/hw/watchdog/cmsdk-apb-watchdog.c b/hw/watchdog/cmsdk-apb-watchdog.c index XXXXXXX..XXXXXXX 100644 --- a/hw/watchdog/cmsdk-apb-watchdog.c +++ b/hw/watchdog/cmsdk-apb-watchdog.c @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_watchdog_reset(DeviceState *dev) ptimer_transaction_commit(s->timer); } +static void cmsdk_apb_watchdog_clk_update(void *opaque) +{ + CMSDKAPBWatchdog *s = CMSDK_APB_WATCHDOG(opaque); + + ptimer_transaction_begin(s->timer); + ptimer_set_period_from_clock(s->timer, s->wdogclk, 1); + ptimer_transaction_commit(s->timer); +} + static void cmsdk_apb_watchdog_init(Object *obj) { SysBusDevice *sbd = SYS_BUS_DEVICE(obj); @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_watchdog_init(Object *obj) s, "cmsdk-apb-watchdog", 0x1000); sysbus_init_mmio(sbd, &s->iomem); sysbus_init_irq(sbd, &s->wdogint); - s->wdogclk = qdev_init_clock_in(DEVICE(s), "WDOGCLK", NULL, NULL); + s->wdogclk = qdev_init_clock_in(DEVICE(s), "WDOGCLK", + cmsdk_apb_watchdog_clk_update, s); s->is_luminary = false; s->id = cmsdk_apb_watchdog_id; @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_watchdog_realize(DeviceState *dev, Error **errp) { CMSDKAPBWatchdog *s = CMSDK_APB_WATCHDOG(dev); - if (s->wdogclk_frq == 0) { + if (!clock_has_source(s->wdogclk)) { error_setg(errp, - "CMSDK APB watchdog: wdogclk-frq property must be set"); + "CMSDK APB watchdog: WDOGCLK clock must be connected"); return; } @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_watchdog_realize(DeviceState *dev, Error **errp) PTIMER_POLICY_NO_COUNTER_ROUND_DOWN); ptimer_transaction_begin(s->timer); - ptimer_set_freq(s->timer, s->wdogclk_frq); + ptimer_set_period_from_clock(s->timer, s->wdogclk, 1); ptimer_transaction_commit(s->timer); } -- 2.20.1 Now that the CMSDK APB watchdog uses its Clock input, it will correctly respond when the system clock frequency is changed using the RCC register on in the Stellaris board system registers. Test that when the RCC register is written it causes the watchdog timer to change speed. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-22-peter.maydell@linaro.org Message-id: 20210121190622.22000-22-peter.maydell@linaro.org --- tests/qtest/cmsdk-apb-watchdog-test.c | 52 +++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/qtest/cmsdk-apb-watchdog-test.c b/tests/qtest/cmsdk-apb-watchdog-test.c index XXXXXXX..XXXXXXX 100644 --- a/tests/qtest/cmsdk-apb-watchdog-test.c +++ b/tests/qtest/cmsdk-apb-watchdog-test.c @@ -XXX,XX +XXX,XX @@ */ #include "qemu/osdep.h" +#include "qemu/bitops.h" #include "libqtest-single.h" /* @@ -XXX,XX +XXX,XX @@ #define WDOGMIS 0x14 #define WDOGLOCK 0xc00 +#define SSYS_BASE 0x400fe000 +#define RCC 0x60 +#define SYSDIV_SHIFT 23 +#define SYSDIV_LENGTH 4 + static void test_watchdog(void) { g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 0); @@ -XXX,XX +XXX,XX @@ static void test_watchdog(void) g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 0); } +static void test_clock_change(void) +{ + uint32_t rcc; + + /* + * Test that writing to the stellaris board's RCC register to + * change the system clock frequency causes the watchdog + * to change the speed it counts at. + */ + g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 0); + + writel(WDOG_BASE + WDOGCONTROL, 1); + writel(WDOG_BASE + WDOGLOAD, 1000); + + /* Step to just past the 500th tick */ + clock_step(80 * 500 + 1); + g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 0); + g_assert_cmpuint(readl(WDOG_BASE + WDOGVALUE), ==, 500); + + /* Rewrite RCC.SYSDIV from 16 to 8, so the clock is now 40ns per tick */ + rcc = readl(SSYS_BASE + RCC); + g_assert_cmpuint(extract32(rcc, SYSDIV_SHIFT, SYSDIV_LENGTH), ==, 0xf); + rcc = deposit32(rcc, SYSDIV_SHIFT, SYSDIV_LENGTH, 7); + writel(SSYS_BASE + RCC, rcc); + + /* Just past the 1000th tick: timer should have fired */ + clock_step(40 * 500); + g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 1); + + g_assert_cmpuint(readl(WDOG_BASE + WDOGVALUE), ==, 0); + + /* VALUE reloads at following tick */ + clock_step(41); + g_assert_cmpuint(readl(WDOG_BASE + WDOGVALUE), ==, 1000); + + /* Writing any value to WDOGINTCLR clears the interrupt and reloads */ + clock_step(40 * 500); + g_assert_cmpuint(readl(WDOG_BASE + WDOGVALUE), ==, 500); + g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 1); + writel(WDOG_BASE + WDOGINTCLR, 0); + g_assert_cmpuint(readl(WDOG_BASE + WDOGVALUE), ==, 1000); + g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 0); +} + int main(int argc, char **argv) { int r; @@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv) qtest_start("-machine lm3s811evb"); qtest_add_func("/cmsdk-apb-watchdog/watchdog", test_watchdog); + qtest_add_func("/cmsdk-apb-watchdog/watchdog_clock_change", + test_clock_change); r = g_test_run(); -- 2.20.1 Use the MAINCLK Clock input to set the system_clock_scale variable rather than using the mainclk_frq property. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Message-id: 20210128114145.20536-23-peter.maydell@linaro.org Message-id: 20210121190622.22000-23-peter.maydell@linaro.org --- hw/arm/armsse.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/armsse.c +++ b/hw/arm/armsse.c @@ -XXX,XX +XXX,XX @@ static void armsse_forward_sec_resp_cfg(ARMSSE *s) qdev_connect_gpio_out(dev_splitter, 2, s->sec_resp_cfg_in); } +static void armsse_mainclk_update(void *opaque) +{ + ARMSSE *s = ARM_SSE(opaque); + /* + * Set system_clock_scale from our Clock input; this is what + * controls the tick rate of the CPU SysTick timer. + */ + system_clock_scale = clock_ticks_to_ns(s->mainclk, 1); +} + static void armsse_init(Object *obj) { ARMSSE *s = ARM_SSE(obj); @@ -XXX,XX +XXX,XX @@ static void armsse_init(Object *obj) assert(info->sram_banks <= MAX_SRAM_BANKS); assert(info->num_cpus <= SSE_MAX_CPUS); - s->mainclk = qdev_init_clock_in(DEVICE(s), "MAINCLK", NULL, NULL); + s->mainclk = qdev_init_clock_in(DEVICE(s), "MAINCLK", + armsse_mainclk_update, s); s->s32kclk = qdev_init_clock_in(DEVICE(s), "S32KCLK", NULL, NULL); memory_region_init(&s->container, obj, "armsse-container", UINT64_MAX); @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) return; } - if (!s->mainclk_frq) { - error_setg(errp, "MAINCLK_FRQ property was not set"); - return; + if (!clock_has_source(s->mainclk)) { + error_setg(errp, "MAINCLK clock was not connected"); + } + if (!clock_has_source(s->s32kclk)) { + error_setg(errp, "S32KCLK clock was not connected"); } assert(info->num_cpus <= SSE_MAX_CPUS); @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) */ sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->container); - system_clock_scale = NANOSECONDS_PER_SECOND / s->mainclk_frq; + /* Set initial system_clock_scale from MAINCLK */ + armsse_mainclk_update(s); } static void armsse_idau_check(IDAUInterface *ii, uint32_t address, -- 2.20.1 Remove all the code that sets frequency properties on the CMSDK timer, dualtimer and watchdog devices and on the ARMSSE SoC device: these properties are unused now that the devices rely on their Clock inputs instead. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-24-peter.maydell@linaro.org Message-id: 20210121190622.22000-24-peter.maydell@linaro.org --- hw/arm/armsse.c | 7 ------- hw/arm/mps2-tz.c | 1 - hw/arm/mps2.c | 3 --- hw/arm/musca.c | 1 - hw/arm/stellaris.c | 3 --- 5 files changed, 15 deletions(-) diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/armsse.c +++ b/hw/arm/armsse.c @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) * it to the appropriate PPC port; then we can realize the PPC and * map its upstream ends to the right place in the container. */ - qdev_prop_set_uint32(DEVICE(&s->timer0), "pclk-frq", s->mainclk_frq); qdev_connect_clock_in(DEVICE(&s->timer0), "pclk", s->mainclk); if (!sysbus_realize(SYS_BUS_DEVICE(&s->timer0), errp)) { return; @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) object_property_set_link(OBJECT(&s->apb_ppc0), "port[0]", OBJECT(mr), &error_abort); - qdev_prop_set_uint32(DEVICE(&s->timer1), "pclk-frq", s->mainclk_frq); qdev_connect_clock_in(DEVICE(&s->timer1), "pclk", s->mainclk); if (!sysbus_realize(SYS_BUS_DEVICE(&s->timer1), errp)) { return; @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) object_property_set_link(OBJECT(&s->apb_ppc0), "port[1]", OBJECT(mr), &error_abort); - qdev_prop_set_uint32(DEVICE(&s->dualtimer), "pclk-frq", s->mainclk_frq); qdev_connect_clock_in(DEVICE(&s->dualtimer), "TIMCLK", s->mainclk); if (!sysbus_realize(SYS_BUS_DEVICE(&s->dualtimer), errp)) { return; @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) /* Devices behind APB PPC1: * 0x4002f000: S32K timer */ - qdev_prop_set_uint32(DEVICE(&s->s32ktimer), "pclk-frq", S32KCLK); qdev_connect_clock_in(DEVICE(&s->s32ktimer), "pclk", s->s32kclk); if (!sysbus_realize(SYS_BUS_DEVICE(&s->s32ktimer), errp)) { return; @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) qdev_connect_gpio_out(DEVICE(&s->nmi_orgate), 0, qdev_get_gpio_in_named(DEVICE(&s->armv7m), "NMI", 0)); - qdev_prop_set_uint32(DEVICE(&s->s32kwatchdog), "wdogclk-frq", S32KCLK); qdev_connect_clock_in(DEVICE(&s->s32kwatchdog), "WDOGCLK", s->s32kclk); if (!sysbus_realize(SYS_BUS_DEVICE(&s->s32kwatchdog), errp)) { return; @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) /* 0x40080000 .. 0x4008ffff : ARMSSE second Base peripheral region */ - qdev_prop_set_uint32(DEVICE(&s->nswatchdog), "wdogclk-frq", s->mainclk_frq); qdev_connect_clock_in(DEVICE(&s->nswatchdog), "WDOGCLK", s->mainclk); if (!sysbus_realize(SYS_BUS_DEVICE(&s->nswatchdog), errp)) { return; @@ -XXX,XX +XXX,XX @@ static void armsse_realize(DeviceState *dev, Error **errp) armsse_get_common_irq_in(s, 1)); sysbus_mmio_map(SYS_BUS_DEVICE(&s->nswatchdog), 0, 0x40081000); - qdev_prop_set_uint32(DEVICE(&s->swatchdog), "wdogclk-frq", s->mainclk_frq); qdev_connect_clock_in(DEVICE(&s->swatchdog), "WDOGCLK", s->mainclk); if (!sysbus_realize(SYS_BUS_DEVICE(&s->swatchdog), errp)) { return; diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/mps2-tz.c +++ b/hw/arm/mps2-tz.c @@ -XXX,XX +XXX,XX @@ static void mps2tz_common_init(MachineState *machine) object_property_set_link(OBJECT(&mms->iotkit), "memory", OBJECT(system_memory), &error_abort); qdev_prop_set_uint32(iotkitdev, "EXP_NUMIRQ", MPS2TZ_NUMIRQ); - qdev_prop_set_uint32(iotkitdev, "MAINCLK_FRQ", SYSCLK_FRQ); qdev_connect_clock_in(iotkitdev, "MAINCLK", mms->sysclk); qdev_connect_clock_in(iotkitdev, "S32KCLK", mms->s32kclk); sysbus_realize(SYS_BUS_DEVICE(&mms->iotkit), &error_fatal); diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/mps2.c +++ b/hw/arm/mps2.c @@ -XXX,XX +XXX,XX @@ static void mps2_common_init(MachineState *machine) object_initialize_child(OBJECT(mms), name, &mms->timer[i], TYPE_CMSDK_APB_TIMER); sbd = SYS_BUS_DEVICE(&mms->timer[i]); - qdev_prop_set_uint32(DEVICE(&mms->timer[i]), "pclk-frq", SYSCLK_FRQ); qdev_connect_clock_in(DEVICE(&mms->timer[i]), "pclk", mms->sysclk); sysbus_realize_and_unref(sbd, &error_fatal); sysbus_mmio_map(sbd, 0, base); @@ -XXX,XX +XXX,XX @@ static void mps2_common_init(MachineState *machine) object_initialize_child(OBJECT(mms), "dualtimer", &mms->dualtimer, TYPE_CMSDK_APB_DUALTIMER); - qdev_prop_set_uint32(DEVICE(&mms->dualtimer), "pclk-frq", SYSCLK_FRQ); qdev_connect_clock_in(DEVICE(&mms->dualtimer), "TIMCLK", mms->sysclk); sysbus_realize(SYS_BUS_DEVICE(&mms->dualtimer), &error_fatal); sysbus_connect_irq(SYS_BUS_DEVICE(&mms->dualtimer), 0, @@ -XXX,XX +XXX,XX @@ static void mps2_common_init(MachineState *machine) sysbus_mmio_map(SYS_BUS_DEVICE(&mms->dualtimer), 0, 0x40002000); object_initialize_child(OBJECT(mms), "watchdog", &mms->watchdog, TYPE_CMSDK_APB_WATCHDOG); - qdev_prop_set_uint32(DEVICE(&mms->watchdog), "wdogclk-frq", SYSCLK_FRQ); qdev_connect_clock_in(DEVICE(&mms->watchdog), "WDOGCLK", mms->sysclk); sysbus_realize(SYS_BUS_DEVICE(&mms->watchdog), &error_fatal); sysbus_connect_irq(SYS_BUS_DEVICE(&mms->watchdog), 0, diff --git a/hw/arm/musca.c b/hw/arm/musca.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/musca.c +++ b/hw/arm/musca.c @@ -XXX,XX +XXX,XX @@ static void musca_init(MachineState *machine) qdev_prop_set_uint32(ssedev, "EXP_NUMIRQ", mmc->num_irqs); qdev_prop_set_uint32(ssedev, "init-svtor", mmc->init_svtor); qdev_prop_set_uint32(ssedev, "SRAM_ADDR_WIDTH", mmc->sram_addr_width); - qdev_prop_set_uint32(ssedev, "MAINCLK_FRQ", SYSCLK_FRQ); qdev_connect_clock_in(ssedev, "MAINCLK", mms->sysclk); qdev_connect_clock_in(ssedev, "S32KCLK", mms->s32kclk); /* diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -XXX,XX +XXX,XX @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board) if (board->dc1 & (1 << 3)) { /* watchdog present */ dev = qdev_new(TYPE_LUMINARY_WATCHDOG); - /* system_clock_scale is valid now */ - uint32_t mainclk = NANOSECONDS_PER_SECOND / system_clock_scale; - qdev_prop_set_uint32(dev, "wdogclk-frq", mainclk); qdev_connect_clock_in(dev, "WDOGCLK", qdev_get_clock_out(ssys_dev, "SYSCLK")); -- 2.20.1 Now no users are setting the frq properties on the CMSDK timer, dualtimer, watchdog or ARMSSE SoC devices, we can remove the properties and the struct fields that back them. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-25-peter.maydell@linaro.org Message-id: 20210121190622.22000-25-peter.maydell@linaro.org --- include/hw/arm/armsse.h | 2 -- include/hw/timer/cmsdk-apb-dualtimer.h | 2 -- include/hw/timer/cmsdk-apb-timer.h | 2 -- include/hw/watchdog/cmsdk-apb-watchdog.h | 2 -- hw/arm/armsse.c | 2 -- hw/timer/cmsdk-apb-dualtimer.c | 6 ------ hw/timer/cmsdk-apb-timer.c | 6 ------ hw/watchdog/cmsdk-apb-watchdog.c | 6 ------ 8 files changed, 28 deletions(-) diff --git a/include/hw/arm/armsse.h b/include/hw/arm/armsse.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/armsse.h +++ b/include/hw/arm/armsse.h @@ -XXX,XX +XXX,XX @@ * + Clock input "S32KCLK": slow 32KHz clock used for a few peripherals * + QOM property "memory" is a MemoryRegion containing the devices provided * by the board model. - * + QOM property "MAINCLK_FRQ" is the frequency of the main system clock * + QOM property "EXP_NUMIRQ" sets the number of expansion interrupts. * (In hardware, the SSE-200 permits the number of expansion interrupts * for the two CPUs to be configured separately, but we restrict it to @@ -XXX,XX +XXX,XX @@ struct ARMSSE { /* Properties */ MemoryRegion *board_memory; uint32_t exp_numirq; - uint32_t mainclk_frq; uint32_t sram_addr_width; uint32_t init_svtor; bool cpu_fpu[SSE_MAX_CPUS]; diff --git a/include/hw/timer/cmsdk-apb-dualtimer.h b/include/hw/timer/cmsdk-apb-dualtimer.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/timer/cmsdk-apb-dualtimer.h +++ b/include/hw/timer/cmsdk-apb-dualtimer.h @@ -XXX,XX +XXX,XX @@ * https://developer.arm.com/products/system-design/system-design-kits/cortex-m-system-design-kit * * QEMU interface: - * + QOM property "pclk-frq": frequency at which the timer is clocked * + Clock input "TIMCLK": clock (for both timers) * + sysbus MMIO region 0: the register bank * + sysbus IRQ 0: combined timer interrupt TIMINTC @@ -XXX,XX +XXX,XX @@ struct CMSDKAPBDualTimer { /*< public >*/ MemoryRegion iomem; qemu_irq timerintc; - uint32_t pclk_frq; Clock *timclk; CMSDKAPBDualTimerModule timermod[CMSDK_APB_DUALTIMER_NUM_MODULES]; diff --git a/include/hw/timer/cmsdk-apb-timer.h b/include/hw/timer/cmsdk-apb-timer.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/timer/cmsdk-apb-timer.h +++ b/include/hw/timer/cmsdk-apb-timer.h @@ -XXX,XX +XXX,XX @@ OBJECT_DECLARE_SIMPLE_TYPE(CMSDKAPBTimer, CMSDK_APB_TIMER) /* * QEMU interface: - * + QOM property "pclk-frq": frequency at which the timer is clocked * + Clock input "pclk": clock for the timer * + sysbus MMIO region 0: the register bank * + sysbus IRQ 0: timer interrupt TIMERINT @@ -XXX,XX +XXX,XX @@ struct CMSDKAPBTimer { /*< public >*/ MemoryRegion iomem; qemu_irq timerint; - uint32_t pclk_frq; struct ptimer_state *timer; Clock *pclk; diff --git a/include/hw/watchdog/cmsdk-apb-watchdog.h b/include/hw/watchdog/cmsdk-apb-watchdog.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/watchdog/cmsdk-apb-watchdog.h +++ b/include/hw/watchdog/cmsdk-apb-watchdog.h @@ -XXX,XX +XXX,XX @@ * https://developer.arm.com/products/system-design/system-design-kits/cortex-m-system-design-kit * * QEMU interface: - * + QOM property "wdogclk-frq": frequency at which the watchdog is clocked * + Clock input "WDOGCLK": clock for the watchdog's timer * + sysbus MMIO region 0: the register bank * + sysbus IRQ 0: watchdog interrupt @@ -XXX,XX +XXX,XX @@ struct CMSDKAPBWatchdog { /*< public >*/ MemoryRegion iomem; qemu_irq wdogint; - uint32_t wdogclk_frq; bool is_luminary; struct ptimer_state *timer; Clock *wdogclk; diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/armsse.c +++ b/hw/arm/armsse.c @@ -XXX,XX +XXX,XX @@ static Property iotkit_properties[] = { DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION, MemoryRegion *), DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64), - DEFINE_PROP_UINT32("MAINCLK_FRQ", ARMSSE, mainclk_frq, 0), DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15), DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000), DEFINE_PROP_BOOL("CPU0_FPU", ARMSSE, cpu_fpu[0], true), @@ -XXX,XX +XXX,XX @@ static Property armsse_properties[] = { DEFINE_PROP_LINK("memory", ARMSSE, board_memory, TYPE_MEMORY_REGION, MemoryRegion *), DEFINE_PROP_UINT32("EXP_NUMIRQ", ARMSSE, exp_numirq, 64), - DEFINE_PROP_UINT32("MAINCLK_FRQ", ARMSSE, mainclk_frq, 0), DEFINE_PROP_UINT32("SRAM_ADDR_WIDTH", ARMSSE, sram_addr_width, 15), DEFINE_PROP_UINT32("init-svtor", ARMSSE, init_svtor, 0x10000000), DEFINE_PROP_BOOL("CPU0_FPU", ARMSSE, cpu_fpu[0], false), diff --git a/hw/timer/cmsdk-apb-dualtimer.c b/hw/timer/cmsdk-apb-dualtimer.c index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/cmsdk-apb-dualtimer.c +++ b/hw/timer/cmsdk-apb-dualtimer.c @@ -XXX,XX +XXX,XX @@ static const VMStateDescription cmsdk_apb_dualtimer_vmstate = { } }; -static Property cmsdk_apb_dualtimer_properties[] = { - DEFINE_PROP_UINT32("pclk-frq", CMSDKAPBDualTimer, pclk_frq, 0), - DEFINE_PROP_END_OF_LIST(), -}; - static void cmsdk_apb_dualtimer_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_dualtimer_class_init(ObjectClass *klass, void *data) dc->realize = cmsdk_apb_dualtimer_realize; dc->vmsd = &cmsdk_apb_dualtimer_vmstate; dc->reset = cmsdk_apb_dualtimer_reset; - device_class_set_props(dc, cmsdk_apb_dualtimer_properties); } static const TypeInfo cmsdk_apb_dualtimer_info = { diff --git a/hw/timer/cmsdk-apb-timer.c b/hw/timer/cmsdk-apb-timer.c index XXXXXXX..XXXXXXX 100644 --- a/hw/timer/cmsdk-apb-timer.c +++ b/hw/timer/cmsdk-apb-timer.c @@ -XXX,XX +XXX,XX @@ static const VMStateDescription cmsdk_apb_timer_vmstate = { } }; -static Property cmsdk_apb_timer_properties[] = { - DEFINE_PROP_UINT32("pclk-frq", CMSDKAPBTimer, pclk_frq, 0), - DEFINE_PROP_END_OF_LIST(), -}; - static void cmsdk_apb_timer_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_timer_class_init(ObjectClass *klass, void *data) dc->realize = cmsdk_apb_timer_realize; dc->vmsd = &cmsdk_apb_timer_vmstate; dc->reset = cmsdk_apb_timer_reset; - device_class_set_props(dc, cmsdk_apb_timer_properties); } static const TypeInfo cmsdk_apb_timer_info = { diff --git a/hw/watchdog/cmsdk-apb-watchdog.c b/hw/watchdog/cmsdk-apb-watchdog.c index XXXXXXX..XXXXXXX 100644 --- a/hw/watchdog/cmsdk-apb-watchdog.c +++ b/hw/watchdog/cmsdk-apb-watchdog.c @@ -XXX,XX +XXX,XX @@ static const VMStateDescription cmsdk_apb_watchdog_vmstate = { } }; -static Property cmsdk_apb_watchdog_properties[] = { - DEFINE_PROP_UINT32("wdogclk-frq", CMSDKAPBWatchdog, wdogclk_frq, 0), - DEFINE_PROP_END_OF_LIST(), -}; - static void cmsdk_apb_watchdog_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -XXX,XX +XXX,XX @@ static void cmsdk_apb_watchdog_class_init(ObjectClass *klass, void *data) dc->realize = cmsdk_apb_watchdog_realize; dc->vmsd = &cmsdk_apb_watchdog_vmstate; dc->reset = cmsdk_apb_watchdog_reset; - device_class_set_props(dc, cmsdk_apb_watchdog_properties); } static const TypeInfo cmsdk_apb_watchdog_info = { -- 2.20.1 Now that the watchdog device uses its Clock input rather than being passed the value of system_clock_scale at creation time, we can remove the hack where we reset the STELLARIS_SYS at board creation time to force it to set system_clock_scale. Instead it will be reset at the usual point in startup and will inform the watchdog of the clock frequency at that point. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Luc Michel <luc@lmichel.fr> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 20210128114145.20536-26-peter.maydell@linaro.org Message-id: 20210121190622.22000-26-peter.maydell@linaro.org Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/arm/stellaris.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index XXXXXXX..XXXXXXX 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -XXX,XX +XXX,XX @@ static DeviceState *stellaris_sys_init(uint32_t base, qemu_irq irq, sysbus_mmio_map(sbd, 0, base); sysbus_connect_irq(sbd, 0, irq); - /* - * Normally we should not be resetting devices like this during - * board creation. For the moment we need to do so, because - * system_clock_scale will only get set when the STELLARIS_SYS - * device is reset, and we need its initial value to pass to - * the watchdog device. This hack can be removed once the - * watchdog has been converted to use a Clock input instead. - */ - device_cold_reset(dev); - return dev; } -- 2.20.1