target-arm queue: * mostly my latest v8M stuff, plus a couple of minor patches The following changes since commit a0b261db8c030813e30a39eae47359ac2a37f7e2: Merge remote-tracking branch 'remotes/ehabkost/tags/python-next-pull-request' into staging (2017-10-12 10:02:09 +0100) are available in the git repository at: git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20171012 for you to fetch changes up to cf5f7937b05c84d5565134f058c00cd48304a117: nvic: Fix miscalculation of offsets into ITNS array (2017-10-12 16:33:16 +0100) ---------------------------------------------------------------- target-arm queue: * v8M: SG, BLXNS, secure-return * v8M: fixes for coverity issues in previous patches * arm: fix armv7m_init() declaration to match definition * watchdog/aspeed: fix variable type to store reload value ---------------------------------------------------------------- Cédric Le Goater (1): watchdog/aspeed: fix variable type to store reload value Igor Mammedov (1): arm: fix armv7m_init() declaration to match definition Peter Maydell (11): target/arm: Add M profile secure MMU index values to get_a32_user_mem_index() target/arm: Implement SG instruction target/arm: Implement BLXNS target/arm: Implement secure function return target-arm: Don't check for "Thumb2 or M profile" for not-Thumb1 target/arm: Pull Thumb insn word loads up to top level target-arm: Simplify insn_crosses_page() target/arm: Support some Thumb insns being always unconditional target/arm: Implement SG instruction corner cases nvic: Add missing 'break' nvic: Fix miscalculation of offsets into ITNS array include/hw/arm/arm.h | 2 +- target/arm/helper.h | 1 + target/arm/internals.h | 8 ++ hw/intc/armv7m_nvic.c | 5 +- hw/watchdog/wdt_aspeed.c | 4 +- target/arm/helper.c | 306 ++++++++++++++++++++++++++++++++++++++++++++-- target/arm/translate.c | 310 ++++++++++++++++++++++++++++++++--------------- 7 files changed, 521 insertions(+), 115 deletions(-) From: Cédric Le Goater <clg@kaod.org> Initially from Anton D. Kachalov" <mouse@yandex-team.ru> but the SoB was missing. Signed-off-by: Cédric Le Goater <clg@kaod.org> Acked-by: Andrew Jeffery <andrew@aj.id.au> Message-id: 20170920064915.30027-1-clg@kaod.org [clg: change commit log and subject replace UL suffix by ULL ] Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/watchdog/wdt_aspeed.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c index XXXXXXX..XXXXXXX 100644 --- a/hw/watchdog/wdt_aspeed.c +++ b/hw/watchdog/wdt_aspeed.c @@ -XXX,XX +XXX,XX @@ static uint64_t aspeed_wdt_read(void *opaque, hwaddr offset, unsigned size) static void aspeed_wdt_reload(AspeedWDTState *s, bool pclk) { - uint32_t reload; + uint64_t reload; if (pclk) { reload = muldiv64(s->regs[WDT_RELOAD_VALUE], NANOSECONDS_PER_SECOND, s->pclk_freq); } else { - reload = s->regs[WDT_RELOAD_VALUE] * 1000; + reload = s->regs[WDT_RELOAD_VALUE] * 1000ULL; } if (aspeed_wdt_is_enabled(s)) { -- 2.7.4 From: Igor Mammedov <imammedo@redhat.com> s/cpu_model/cpu_type/ that has been forgotten during conversion (ba1ba5cc), while touching the line also fixup alignment. Signed-off-by: Igor Mammedov <imammedo@redhat.com> Message-id: 1507710805-221721-1-git-send-email-imammedo@redhat.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/hw/arm/arm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h index XXXXXXX..XXXXXXX 100644 --- a/include/hw/arm/arm.h +++ b/include/hw/arm/arm.h @@ -XXX,XX +XXX,XX @@ typedef enum { /* armv7m.c */ DeviceState *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq, - const char *kernel_filename, const char *cpu_model); + const char *kernel_filename, const char *cpu_type); /** * armv7m_load_kernel: * @cpu: CPU -- 2.7.4 Add the M profile secure MMU index values to the switch in get_a32_user_mem_index() so that LDRT/STRT work correctly rather than asserting at translate time. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1507556919-24992-2-git-send-email-peter.maydell@linaro.org --- target/arm/translate.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static inline int get_a32_user_mem_index(DisasContext *s) case ARMMMUIdx_MPriv: case ARMMMUIdx_MNegPri: return arm_to_core_mmu_idx(ARMMMUIdx_MUser); + case ARMMMUIdx_MSUser: + case ARMMMUIdx_MSPriv: + case ARMMMUIdx_MSNegPri: + return arm_to_core_mmu_idx(ARMMMUIdx_MSUser); case ARMMMUIdx_S2NS: default: g_assert_not_reached(); -- 2.7.4 Implement the SG instruction, which we emulate 'by hand' in the exception handling code path. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1507556919-24992-3-git-send-email-peter.maydell@linaro.org --- target/arm/helper.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 127 insertions(+), 5 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 @@ typedef struct V8M_SAttributes { bool irvalid; } V8M_SAttributes; +static void v8m_security_lookup(CPUARMState *env, uint32_t address, + MMUAccessType access_type, ARMMMUIdx mmu_idx, + V8M_SAttributes *sattrs); + /* Definitions for the PMCCNTR and PMCR registers */ #define PMCRD 0x8 #define PMCRC 0x4 @@ -XXX,XX +XXX,XX @@ static void arm_log_exception(int idx) } } +static bool v7m_read_half_insn(ARMCPU *cpu, ARMMMUIdx mmu_idx, + uint32_t addr, uint16_t *insn) +{ + /* Load a 16-bit portion of a v7M instruction, returning true on success, + * or false on failure (in which case we will have pended the appropriate + * exception). + * We need to do the instruction fetch's MPU and SAU checks + * like this because there is no MMU index that would allow + * doing the load with a single function call. Instead we must + * first check that the security attributes permit the load + * and that they don't mismatch on the two halves of the instruction, + * and then we do the load as a secure load (ie using the security + * attributes of the address, not the CPU, as architecturally required). + */ + CPUState *cs = CPU(cpu); + CPUARMState *env = &cpu->env; + V8M_SAttributes sattrs = {}; + MemTxAttrs attrs = {}; + ARMMMUFaultInfo fi = {}; + MemTxResult txres; + target_ulong page_size; + hwaddr physaddr; + int prot; + uint32_t fsr; + + v8m_security_lookup(env, addr, MMU_INST_FETCH, mmu_idx, &sattrs); + if (!sattrs.nsc || sattrs.ns) { + /* This must be the second half of the insn, and it straddles a + * region boundary with the second half not being S&NSC. + */ + env->v7m.sfsr |= R_V7M_SFSR_INVEP_MASK; + armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SECURE, false); + qemu_log_mask(CPU_LOG_INT, + "...really SecureFault with SFSR.INVEP\n"); + return false; + } + if (get_phys_addr(env, addr, MMU_INST_FETCH, mmu_idx, + &physaddr, &attrs, &prot, &page_size, &fsr, &fi)) { + /* the MPU lookup failed */ + env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_IACCVIOL_MASK; + armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM, env->v7m.secure); + qemu_log_mask(CPU_LOG_INT, "...really MemManage with CFSR.IACCVIOL\n"); + return false; + } + *insn = address_space_lduw_le(arm_addressspace(cs, attrs), physaddr, + attrs, &txres); + if (txres != MEMTX_OK) { + env->v7m.cfsr[M_REG_NS] |= R_V7M_CFSR_IBUSERR_MASK; + armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_BUS, false); + qemu_log_mask(CPU_LOG_INT, "...really BusFault with CFSR.IBUSERR\n"); + return false; + } + return true; +} + +static bool v7m_handle_execute_nsc(ARMCPU *cpu) +{ + /* Check whether this attempt to execute code in a Secure & NS-Callable + * memory region is for an SG instruction; if so, then emulate the + * effect of the SG instruction and return true. Otherwise pend + * the correct kind of exception and return false. + */ + CPUARMState *env = &cpu->env; + ARMMMUIdx mmu_idx; + uint16_t insn; + + /* We should never get here unless get_phys_addr_pmsav8() caused + * an exception for NS executing in S&NSC memory. + */ + assert(!env->v7m.secure); + assert(arm_feature(env, ARM_FEATURE_M_SECURITY)); + + /* We want to do the MPU lookup as secure; work out what mmu_idx that is */ + mmu_idx = arm_v7m_mmu_idx_for_secstate(env, true); + + if (!v7m_read_half_insn(cpu, mmu_idx, env->regs[15], &insn)) { + return false; + } + + if (!env->thumb) { + goto gen_invep; + } + + if (insn != 0xe97f) { + /* Not an SG instruction first half (we choose the IMPDEF + * early-SG-check option). + */ + goto gen_invep; + } + + if (!v7m_read_half_insn(cpu, mmu_idx, env->regs[15] + 2, &insn)) { + return false; + } + + if (insn != 0xe97f) { + /* Not an SG instruction second half (yes, both halves of the SG + * insn have the same hex value) + */ + goto gen_invep; + } + + /* OK, we have confirmed that we really have an SG instruction. + * We know we're NS in S memory so don't need to repeat those checks. + */ + qemu_log_mask(CPU_LOG_INT, "...really an SG instruction at 0x%08" PRIx32 + ", executing it\n", env->regs[15]); + env->regs[14] &= ~1; + switch_v7m_security_state(env, true); + xpsr_write(env, 0, XPSR_IT); + env->regs[15] += 4; + return true; + +gen_invep: + env->v7m.sfsr |= R_V7M_SFSR_INVEP_MASK; + armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SECURE, false); + qemu_log_mask(CPU_LOG_INT, + "...really SecureFault with SFSR.INVEP\n"); + return false; +} + void arm_v7m_cpu_do_interrupt(CPUState *cs) { ARMCPU *cpu = ARM_CPU(cs); @@ -XXX,XX +XXX,XX @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) * the SG instruction have the same security attributes.) * Everything else must generate an INVEP SecureFault, so we * emulate the SG instruction here. - * TODO: actually emulate SG. */ - env->v7m.sfsr |= R_V7M_SFSR_INVEP_MASK; - armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SECURE, false); - qemu_log_mask(CPU_LOG_INT, - "...really SecureFault with SFSR.INVEP\n"); + if (v7m_handle_execute_nsc(cpu)) { + return; + } break; case M_FAKE_FSR_SFAULT: /* Various flavours of SecureFault for attempts to execute or -- 2.7.4 Implement the BLXNS instruction, which allows secure code to call non-secure code. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1507556919-24992-4-git-send-email-peter.maydell@linaro.org --- target/arm/helper.h | 1 + target/arm/internals.h | 1 + target/arm/helper.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++ target/arm/translate.c | 17 +++++++++++++-- 4 files changed, 76 insertions(+), 2 deletions(-) diff --git a/target/arm/helper.h b/target/arm/helper.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -XXX,XX +XXX,XX @@ DEF_HELPER_3(v7m_msr, void, env, i32, i32) DEF_HELPER_2(v7m_mrs, i32, env, i32) DEF_HELPER_2(v7m_bxns, void, env, i32) +DEF_HELPER_2(v7m_blxns, void, env, i32) DEF_HELPER_4(access_check_cp_reg, void, env, ptr, i32, i32) DEF_HELPER_3(set_cp_reg, void, env, ptr, i32) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ static inline bool excp_is_internal(int excp) FIELD(V7M_CONTROL, NPRIV, 0, 1) FIELD(V7M_CONTROL, SPSEL, 1, 1) FIELD(V7M_CONTROL, FPCA, 2, 1) +FIELD(V7M_CONTROL, SFPA, 3, 1) /* Bit definitions for v7M exception return payload */ FIELD(V7M_EXCRET, ES, 0, 1) 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 HELPER(v7m_bxns)(CPUARMState *env, uint32_t dest) g_assert_not_reached(); } +void HELPER(v7m_blxns)(CPUARMState *env, uint32_t dest) +{ + /* translate.c should never generate calls here in user-only mode */ + g_assert_not_reached(); +} + void switch_mode(CPUARMState *env, int mode) { ARMCPU *cpu = arm_env_get_cpu(env); @@ -XXX,XX +XXX,XX @@ void HELPER(v7m_bxns)(CPUARMState *env, uint32_t dest) env->regs[15] = dest & ~1; } +void HELPER(v7m_blxns)(CPUARMState *env, uint32_t dest) +{ + /* Handle v7M BLXNS: + * - bit 0 of the destination address is the target security state + */ + + /* At this point regs[15] is the address just after the BLXNS */ + uint32_t nextinst = env->regs[15] | 1; + uint32_t sp = env->regs[13] - 8; + uint32_t saved_psr; + + /* translate.c will have made BLXNS UNDEF unless we're secure */ + assert(env->v7m.secure); + + if (dest & 1) { + /* target is Secure, so this is just a normal BLX, + * except that the low bit doesn't indicate Thumb/not. + */ + env->regs[14] = nextinst; + env->thumb = 1; + env->regs[15] = dest & ~1; + return; + } + + /* Target is non-secure: first push a stack frame */ + if (!QEMU_IS_ALIGNED(sp, 8)) { + qemu_log_mask(LOG_GUEST_ERROR, + "BLXNS with misaligned SP is UNPREDICTABLE\n"); + } + + saved_psr = env->v7m.exception; + if (env->v7m.control[M_REG_S] & R_V7M_CONTROL_SFPA_MASK) { + saved_psr |= XPSR_SFPA; + } + + /* Note that these stores can throw exceptions on MPU faults */ + cpu_stl_data(env, sp, nextinst); + cpu_stl_data(env, sp + 4, saved_psr); + + env->regs[13] = sp; + env->regs[14] = 0xfeffffff; + if (arm_v7m_is_handler_mode(env)) { + /* Write a dummy value to IPSR, to avoid leaking the current secure + * exception number to non-secure code. This is guaranteed not + * to cause write_v7m_exception() to actually change stacks. + */ + write_v7m_exception(env, 1); + } + switch_v7m_security_state(env, 0); + env->thumb = 1; + env->regs[15] = dest; +} + static uint32_t *get_v7m_sp_ptr(CPUARMState *env, bool secure, bool threadmode, bool spsel) { 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_bxns(DisasContext *s, int rm) s->base.is_jmp = DISAS_EXIT; } +static inline void gen_blxns(DisasContext *s, int rm) +{ + TCGv_i32 var = load_reg(s, rm); + + /* We don't need to sync condexec state, for the same reason as bxns. + * We do however need to set the PC, because the blxns helper reads it. + * The blxns helper may throw an exception. + */ + gen_set_pc_im(s, s->pc); + gen_helper_v7m_blxns(cpu_env, var); + tcg_temp_free_i32(var); + s->base.is_jmp = DISAS_EXIT; +} + /* Variant of store_reg which uses branch&exchange logic when storing to r15 in ARM architecture v7 and above. The source must be a temporary and will be marked as dead. */ @@ -XXX,XX +XXX,XX @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s) goto undef; } if (link) { - /* BLXNS: not yet implemented */ - goto undef; + gen_blxns(s, rm); } else { gen_bxns(s, rm); } -- 2.7.4 Secure function return happens when a non-secure function has been called using BLXNS and so has a particular magic LR value (either 0xfefffffe or 0xfeffffff). The function return via BX behaves specially when the new PC value is this magic value, in the same way that exception returns are handled. Adjust our BX excret guards so that they recognize the function return magic number as well, and perform the function-return unstacking in do_v7m_exception_exit(). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Acked-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1507556919-24992-5-git-send-email-peter.maydell@linaro.org --- target/arm/internals.h | 7 +++ target/arm/helper.c | 115 +++++++++++++++++++++++++++++++++++++++++++++---- target/arm/translate.c | 14 +++++- 3 files changed, 126 insertions(+), 10 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index XXXXXXX..XXXXXXX 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -XXX,XX +XXX,XX @@ FIELD(V7M_EXCRET, DCRS, 5, 1) FIELD(V7M_EXCRET, S, 6, 1) FIELD(V7M_EXCRET, RES1, 7, 25) /* including the must-be-1 prefix */ +/* Minimum value which is a magic number for exception return */ +#define EXC_RETURN_MIN_MAGIC 0xff000000 +/* Minimum number which is a magic number for function or exception return + * when using v8M security extension + */ +#define FNC_RETURN_MIN_MAGIC 0xfefffffe + /* We use a few fake FSR values for internal purposes in M profile. * M profile cores don't have A/R format FSRs, but currently our * get_phys_addr() code assumes A/R profile and reports failures via 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 HELPER(v7m_bxns)(CPUARMState *env, uint32_t dest) * - if the return value is a magic value, do exception return (like BX) * - otherwise bit 0 of the return value is the target security state */ - if (dest >= 0xff000000) { + uint32_t min_magic; + + if (arm_feature(env, ARM_FEATURE_M_SECURITY)) { + /* Covers FNC_RETURN and EXC_RETURN magic */ + min_magic = FNC_RETURN_MIN_MAGIC; + } else { + /* EXC_RETURN magic only */ + min_magic = EXC_RETURN_MIN_MAGIC; + } + + if (dest >= min_magic) { /* This is an exception return magic value; put it where * do_v7m_exception_exit() expects and raise EXCEPTION_EXIT. * Note that if we ever add gen_ss_advance() singlestep support to @@ -XXX,XX +XXX,XX @@ static void do_v7m_exception_exit(ARMCPU *cpu) bool exc_secure = false; bool return_to_secure; - /* We can only get here from an EXCP_EXCEPTION_EXIT, and - * gen_bx_excret() enforces the architectural rule - * that jumps to magic addresses don't have magic behaviour unless - * we're in Handler mode (compare pseudocode BXWritePC()). + /* If we're not in Handler mode then jumps to magic exception-exit + * addresses don't have magic behaviour. However for the v8M + * security extensions the magic secure-function-return has to + * work in thread mode too, so to avoid doing an extra check in + * the generated code we allow exception-exit magic to also cause the + * internal exception and bring us here in thread mode. Correct code + * will never try to do this (the following insn fetch will always + * fault) so we the overhead of having taken an unnecessary exception + * doesn't matter. */ - assert(arm_v7m_is_handler_mode(env)); + if (!arm_v7m_is_handler_mode(env)) { + return; + } /* In the spec pseudocode ExceptionReturn() is called directly * from BXWritePC() and gets the full target PC value including @@ -XXX,XX +XXX,XX @@ static void do_v7m_exception_exit(ARMCPU *cpu) qemu_log_mask(CPU_LOG_INT, "...successful exception return\n"); } +static bool do_v7m_function_return(ARMCPU *cpu) +{ + /* v8M security extensions magic function return. + * We may either: + * (1) throw an exception (longjump) + * (2) return true if we successfully handled the function return + * (3) return false if we failed a consistency check and have + * pended a UsageFault that needs to be taken now + * + * At this point the magic return value is split between env->regs[15] + * and env->thumb. We don't bother to reconstitute it because we don't + * need it (all values are handled the same way). + */ + CPUARMState *env = &cpu->env; + uint32_t newpc, newpsr, newpsr_exc; + + qemu_log_mask(CPU_LOG_INT, "...really v7M secure function return\n"); + + { + bool threadmode, spsel; + TCGMemOpIdx oi; + ARMMMUIdx mmu_idx; + uint32_t *frame_sp_p; + uint32_t frameptr; + + /* Pull the return address and IPSR from the Secure stack */ + threadmode = !arm_v7m_is_handler_mode(env); + spsel = env->v7m.control[M_REG_S] & R_V7M_CONTROL_SPSEL_MASK; + + frame_sp_p = get_v7m_sp_ptr(env, true, threadmode, spsel); + frameptr = *frame_sp_p; + + /* These loads may throw an exception (for MPU faults). We want to + * do them as secure, so work out what MMU index that is. + */ + mmu_idx = arm_v7m_mmu_idx_for_secstate(env, true); + oi = make_memop_idx(MO_LE, arm_to_core_mmu_idx(mmu_idx)); + newpc = helper_le_ldul_mmu(env, frameptr, oi, 0); + newpsr = helper_le_ldul_mmu(env, frameptr + 4, oi, 0); + + /* Consistency checks on new IPSR */ + newpsr_exc = newpsr & XPSR_EXCP; + if (!((env->v7m.exception == 0 && newpsr_exc == 0) || + (env->v7m.exception == 1 && newpsr_exc != 0))) { + /* Pend the fault and tell our caller to take it */ + env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_INVPC_MASK; + armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE, + env->v7m.secure); + qemu_log_mask(CPU_LOG_INT, + "...taking INVPC UsageFault: " + "IPSR consistency check failed\n"); + return false; + } + + *frame_sp_p = frameptr + 8; + } + + /* This invalidates frame_sp_p */ + switch_v7m_security_state(env, true); + env->v7m.exception = newpsr_exc; + env->v7m.control[M_REG_S] &= ~R_V7M_CONTROL_SFPA_MASK; + if (newpsr & XPSR_SFPA) { + env->v7m.control[M_REG_S] |= R_V7M_CONTROL_SFPA_MASK; + } + xpsr_write(env, 0, XPSR_IT); + env->thumb = newpc & 1; + env->regs[15] = newpc & ~1; + + qemu_log_mask(CPU_LOG_INT, "...function return successful\n"); + return true; +} + static void arm_log_exception(int idx) { if (qemu_loglevel_mask(CPU_LOG_INT)) { @@ -XXX,XX +XXX,XX @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) case EXCP_IRQ: break; case EXCP_EXCEPTION_EXIT: - do_v7m_exception_exit(cpu); - return; + if (env->regs[15] < EXC_RETURN_MIN_MAGIC) { + /* Must be v8M security extension function return */ + assert(env->regs[15] >= FNC_RETURN_MIN_MAGIC); + assert(arm_feature(env, ARM_FEATURE_M_SECURITY)); + if (do_v7m_function_return(cpu)) { + return; + } + } else { + do_v7m_exception_exit(cpu); + return; + } + break; default: cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index); return; /* Never happens. Keep compiler happy. */ 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_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); - if (s->v7m_handler_mode && arm_dc_feature(s, ARM_FEATURE_M)) { + 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; } } @@ -XXX,XX +XXX,XX @@ static inline void gen_bx_excret_final_code(DisasContext *s) { /* Generate the code to finish possible exception return and end the TB */ TCGLabel *excret_label = gen_new_label(); + uint32_t min_magic; + + if (arm_dc_feature(s, ARM_FEATURE_M_SECURITY)) { + /* Covers FNC_RETURN and EXC_RETURN magic */ + min_magic = FNC_RETURN_MIN_MAGIC; + } else { + /* EXC_RETURN magic only */ + min_magic = EXC_RETURN_MIN_MAGIC; + } /* Is the new PC value in the magic range indicating exception return? */ - tcg_gen_brcondi_i32(TCG_COND_GEU, cpu_R[15], 0xff000000, excret_label); + tcg_gen_brcondi_i32(TCG_COND_GEU, cpu_R[15], min_magic, excret_label); /* No: end the TB as we would for a DISAS_JMP */ if (is_singlestepping(s)) { gen_singlestep_exception(s); -- 2.7.4 The code which implements the Thumb1 split BL/BLX instructions is guarded by a check on "not M or THUMB2". All we really need to check here is "not THUMB2" (and we assume that elsewhere too, eg in the ARCH(6T2) test that UNDEFs the Thumb2 insns). This doesn't change behaviour because all M profile cores have Thumb2 and so ARM_FEATURE_M implies ARM_FEATURE_THUMB2. (v6M implements a very restricted subset of Thumb2, but we can cross that bridge when we get to it with appropriate feature bits.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1507556919-24992-6-git-send-email-peter.maydell@linaro.org --- target/arm/translate.c | 3 +-- 1 file changed, 1 insertion(+), 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 int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw int conds; int logic_cc; - if (!(arm_dc_feature(s, ARM_FEATURE_THUMB2) - || arm_dc_feature(s, ARM_FEATURE_M))) { + if (!arm_dc_feature(s, ARM_FEATURE_THUMB2)) { /* Thumb-1 cores may need to treat bl and blx as a pair of 16-bit instructions to get correct prefetch abort behavior. */ insn = insn_hw1; -- 2.7.4 Refactor the Thumb decode to do the loads of the instruction words at the top level rather than only loading the second half of a 32-bit Thumb insn in the middle of the decode. This is simple apart from the awkward case of Thumb1, where the BL/BLX prefix and suffix instructions live in what in Thumb2 is the 32-bit insn space. To handle these we decode enough to identify whether we're looking at a prefix/suffix that we handle as a 16 bit insn, or a prefix that we're going to merge with the following suffix to consider as a 32 bit insn. The translation of the 16 bit cases then moves from disas_thumb2_insn() to disas_thumb_insn(). The refactoring has the benefit that we don't need to pass the CPUARMState* down into the decoder code any more, but the major reason for doing this is that some Thumb instructions must be always unconditional regardless of the IT state bits, so we need to know the whole insn before we emit the "skip this insn if the IT bits and cond state tell us to" code. (The always unconditional insns are BKPT, HLT and SG; the last of these is 32 bits.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1507556919-24992-7-git-send-email-peter.maydell@linaro.org --- target/arm/translate.c | 178 ++++++++++++++++++++++++++++++------------------- 1 file changed, 108 insertions(+), 70 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) } } +static bool thumb_insn_is_16bit(DisasContext *s, uint32_t insn) +{ + /* Return true if this is a 16 bit instruction. We must be precise + * about this (matching the decode). We assume that s->pc still + * points to the first 16 bits of the insn. + */ + if ((insn >> 11) < 0x1d) { + /* Definitely a 16-bit instruction */ + return true; + } + + /* Top five bits 0b11101 / 0b11110 / 0b11111 : this is the + * first half of a 32-bit Thumb insn. Thumb-1 cores might + * end up actually treating this as two 16-bit insns, though, + * if it's half of a bl/blx pair that might span a page boundary. + */ + if (arm_dc_feature(s, ARM_FEATURE_THUMB2)) { + /* Thumb2 cores (including all M profile ones) always treat + * 32-bit insns as 32-bit. + */ + return false; + } + + if ((insn >> 11) == 0x1e && (s->pc < s->next_page_start - 3)) { + /* 0b1111_0xxx_xxxx_xxxx : BL/BLX prefix, and the suffix + * is not on the next page; we merge this into a 32-bit + * insn. + */ + return false; + } + /* 0b1110_1xxx_xxxx_xxxx : BLX suffix (or UNDEF); + * 0b1111_1xxx_xxxx_xxxx : BL suffix; + * 0b1111_0xxx_xxxx_xxxx : BL/BLX prefix on the end of a page + * -- handle as single 16 bit insn + */ + return true; +} + /* Return true if this is a Thumb-2 logical op. */ static int thumb2_logic_op(int op) @@ -XXX,XX +XXX,XX @@ gen_thumb2_data_op(DisasContext *s, int op, int conds, uint32_t shifter_out, /* Translate a 32-bit thumb instruction. Returns nonzero if the instruction is not legal. */ -static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw1) +static int disas_thumb2_insn(DisasContext *s, uint32_t insn) { - uint32_t insn, imm, shift, offset; + uint32_t imm, shift, offset; uint32_t rd, rn, rm, rs; TCGv_i32 tmp; TCGv_i32 tmp2; @@ -XXX,XX +XXX,XX @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw int conds; int logic_cc; - if (!arm_dc_feature(s, ARM_FEATURE_THUMB2)) { - /* Thumb-1 cores may need to treat bl and blx as a pair of - 16-bit instructions to get correct prefetch abort behavior. */ - insn = insn_hw1; - if ((insn & (1 << 12)) == 0) { - ARCH(5); - /* Second half of blx. */ - offset = ((insn & 0x7ff) << 1); - tmp = load_reg(s, 14); - tcg_gen_addi_i32(tmp, tmp, offset); - tcg_gen_andi_i32(tmp, tmp, 0xfffffffc); - - tmp2 = tcg_temp_new_i32(); - tcg_gen_movi_i32(tmp2, s->pc | 1); - store_reg(s, 14, tmp2); - gen_bx(s, tmp); - return 0; - } - if (insn & (1 << 11)) { - /* Second half of bl. */ - offset = ((insn & 0x7ff) << 1) | 1; - tmp = load_reg(s, 14); - tcg_gen_addi_i32(tmp, tmp, offset); - - tmp2 = tcg_temp_new_i32(); - tcg_gen_movi_i32(tmp2, s->pc | 1); - store_reg(s, 14, tmp2); - gen_bx(s, tmp); - return 0; - } - if ((s->pc & ~TARGET_PAGE_MASK) == 0) { - /* Instruction spans a page boundary. Implement it as two - 16-bit instructions in case the second half causes an - prefetch abort. */ - offset = ((int32_t)insn << 21) >> 9; - tcg_gen_movi_i32(cpu_R[14], s->pc + 2 + offset); - return 0; - } - /* Fall through to 32-bit decode. */ - } - - insn = arm_lduw_code(env, s->pc, s->sctlr_b); - s->pc += 2; - insn |= (uint32_t)insn_hw1 << 16; - + /* The only 32 bit insn that's allowed for Thumb1 is the combined + * BL/BLX prefix and suffix. + */ if ((insn & 0xf800e800) != 0xf000e800) { ARCH(6T2); } @@ -XXX,XX +XXX,XX @@ illegal_op: return 1; } -static void disas_thumb_insn(CPUARMState *env, DisasContext *s) +static void disas_thumb_insn(DisasContext *s, uint32_t insn) { - uint32_t val, insn, op, rm, rn, rd, shift, cond; + uint32_t val, op, rm, rn, rd, shift, cond; int32_t offset; int i; TCGv_i32 tmp; TCGv_i32 tmp2; TCGv_i32 addr; - if (s->condexec_mask) { - cond = s->condexec_cond; - if (cond != 0x0e) { /* Skip conditional when condition is AL. */ - s->condlabel = gen_new_label(); - arm_gen_test_cc(cond ^ 1, s->condlabel); - s->condjmp = 1; - } - } - - insn = arm_lduw_code(env, s->pc, s->sctlr_b); - s->pc += 2; - switch (insn >> 12) { case 0: case 1: @@ -XXX,XX +XXX,XX @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s) case 14: if (insn & (1 << 11)) { - if (disas_thumb2_insn(env, s, insn)) - goto undef32; + /* thumb_insn_is_16bit() ensures we can't get here for + * a Thumb2 CPU, so this must be a thumb1 split BL/BLX: + * 0b1110_1xxx_xxxx_xxxx : BLX suffix (or UNDEF) + */ + assert(!arm_dc_feature(s, ARM_FEATURE_THUMB2)); + ARCH(5); + offset = ((insn & 0x7ff) << 1); + tmp = load_reg(s, 14); + tcg_gen_addi_i32(tmp, tmp, offset); + tcg_gen_andi_i32(tmp, tmp, 0xfffffffc); + + tmp2 = tcg_temp_new_i32(); + tcg_gen_movi_i32(tmp2, s->pc | 1); + store_reg(s, 14, tmp2); + gen_bx(s, tmp); break; } /* unconditional branch */ @@ -XXX,XX +XXX,XX @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s) break; case 15: - if (disas_thumb2_insn(env, s, insn)) - goto undef32; + /* thumb_insn_is_16bit() ensures we can't get here for + * a Thumb2 CPU, so this must be a thumb1 split BL/BLX. + */ + assert(!arm_dc_feature(s, ARM_FEATURE_THUMB2)); + + if (insn & (1 << 11)) { + /* 0b1111_1xxx_xxxx_xxxx : BL suffix */ + offset = ((insn & 0x7ff) << 1) | 1; + tmp = load_reg(s, 14); + tcg_gen_addi_i32(tmp, tmp, offset); + + tmp2 = tcg_temp_new_i32(); + tcg_gen_movi_i32(tmp2, s->pc | 1); + store_reg(s, 14, tmp2); + gen_bx(s, tmp); + } else { + /* 0b1111_0xxx_xxxx_xxxx : BL/BLX prefix */ + uint32_t uoffset = ((int32_t)insn << 21) >> 9; + + tcg_gen_movi_i32(cpu_R[14], s->pc + 2 + uoffset); + } break; } return; -undef32: - gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(), - default_exception_el(s)); - return; illegal_op: undef: gen_exception_insn(s, 2, EXCP_UDEF, syn_uncategorized(), @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) { DisasContext *dc = container_of(dcbase, DisasContext, base); CPUARMState *env = cpu->env_ptr; + uint32_t insn; + bool is_16bit; if (arm_pre_translate_insn(dc)) { return; } - disas_thumb_insn(env, dc); + insn = arm_lduw_code(env, dc->pc, dc->sctlr_b); + is_16bit = thumb_insn_is_16bit(dc, insn); + dc->pc += 2; + if (!is_16bit) { + uint32_t insn2 = arm_lduw_code(env, dc->pc, dc->sctlr_b); + + insn = insn << 16 | insn2; + dc->pc += 2; + } + + if (dc->condexec_mask) { + uint32_t cond = dc->condexec_cond; + + if (cond != 0x0e) { /* Skip conditional when condition is AL. */ + dc->condlabel = gen_new_label(); + arm_gen_test_cc(cond ^ 1, dc->condlabel); + dc->condjmp = 1; + } + } + + if (is_16bit) { + disas_thumb_insn(dc, insn); + } else { + disas_thumb2_insn(dc, insn); + } /* Advance the Thumb condexec condition. */ if (dc->condexec_mask) { -- 2.7.4 Recent changes have left insn_crosses_page() more complicated than it needed to be: * it's only called from thumb_tr_translate_insn() so we know for certain that we're looking at a Thumb insn * the caller's check for dc->pc >= dc->next_page_start - 3 means that dc->pc can't possibly be 4 aligned, so there's no need to check that (the check was partly there to ensure that we didn't treat an ARM insn as Thumb, I think) * we now have thumb_insn_is_16bit() which lets us do a precise check of the length of the next insn, rather than opencoding an inaccurate check Simplify it down to just loading the first half of the insn and calling thumb_insn_is_16bit() on it. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1507556919-24992-8-git-send-email-peter.maydell@linaro.org --- target/arm/translate.c | 27 ++++++--------------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/target/arm/translate.c b/target/arm/translate.c index XXXXXXX..XXXXXXX 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -XXX,XX +XXX,XX @@ static bool insn_crosses_page(CPUARMState *env, DisasContext *s) { /* Return true if the insn at dc->pc might cross a page boundary. * (False positives are OK, false negatives are not.) + * We know this is a Thumb insn, and our caller ensures we are + * only called if dc->pc is less than 4 bytes from the page + * boundary, so we cross the page if the first 16 bits indicate + * that this is a 32 bit insn. */ - uint16_t insn; + uint16_t insn = arm_lduw_code(env, s->pc, s->sctlr_b); - if ((s->pc & 3) == 0) { - /* At a 4-aligned address we can't be crossing a page */ - return false; - } - - /* This must be a Thumb insn */ - insn = arm_lduw_code(env, s->pc, s->sctlr_b); - - if ((insn >> 11) >= 0x1d) { - /* Top five bits 0b11101 / 0b11110 / 0b11111 : this is the - * First half of a 32-bit Thumb insn. Thumb-1 cores might - * end up actually treating this as two 16-bit insns (see the - * code at the start of disas_thumb2_insn()) but we don't bother - * to check for that as it is unlikely, and false positives here - * are harmless. - */ - return true; - } - /* Definitely a 16-bit insn, can't be crossing a page. */ - return false; + return !thumb_insn_is_16bit(s, insn); } static int arm_tr_init_disas_context(DisasContextBase *dcbase, -- 2.7.4 A few Thumb instructions are always unconditional even inside an IT block (as opposed to being UNPREDICTABLE if used inside an IT block): BKPT, the v8M SG instruction, and the A profile HLT (debug halt) instruction. This means we need to suppress the jump-over-instruction-on-condfail code generation (though the IT state still advances as usual and subsequent insns in the IT block may be conditional). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1507556919-24992-9-git-send-email-peter.maydell@linaro.org --- target/arm/translate.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 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 void arm_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) in init_disas_context by adjusting max_insns. */ } +static bool thumb_insn_is_unconditional(DisasContext *s, uint32_t insn) +{ + /* Return true if this Thumb insn is always unconditional, + * even inside an IT block. This is true of only a very few + * instructions: BKPT, HLT, and SG. + * + * A larger class of instructions are UNPREDICTABLE if used + * inside an IT block; we do not need to detect those here, because + * what we do by default (perform the cc check and update the IT + * bits state machine) is a permitted CONSTRAINED UNPREDICTABLE + * choice for those situations. + * + * insn is either a 16-bit or a 32-bit instruction; the two are + * distinguishable because for the 16-bit case the top 16 bits + * are zeroes, and that isn't a valid 32-bit encoding. + */ + if ((insn & 0xffffff00) == 0xbe00) { + /* BKPT */ + return true; + } + + if ((insn & 0xffffffc0) == 0xba80 && arm_dc_feature(s, ARM_FEATURE_V8) && + !arm_dc_feature(s, ARM_FEATURE_M)) { + /* HLT: v8A only. This is unconditional even when it is going to + * UNDEF; see the v8A ARM ARM DDI0487B.a H3.3. + * For v7 cores this was a plain old undefined encoding and so + * honours its cc check. (We might be using the encoding as + * a semihosting trap, but we don't change the cc check behaviour + * on that account, because a debugger connected to a real v7A + * core and emulating semihosting traps by catching the UNDEF + * exception would also only see cases where the cc check passed. + * No guest code should be trying to do a HLT semihosting trap + * in an IT block anyway. + */ + return true; + } + + if (insn == 0xe97fe97f && arm_dc_feature(s, ARM_FEATURE_V8) && + arm_dc_feature(s, ARM_FEATURE_M)) { + /* SG: v8M only */ + return true; + } + + return false; +} + static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) { DisasContext *dc = container_of(dcbase, DisasContext, base); @@ -XXX,XX +XXX,XX @@ static void thumb_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) dc->pc += 2; } - if (dc->condexec_mask) { + if (dc->condexec_mask && !thumb_insn_is_unconditional(dc, insn)) { uint32_t cond = dc->condexec_cond; if (cond != 0x0e) { /* Skip conditional when condition is AL. */ -- 2.7.4 The common situation of the SG instruction is that it is executed from S&NSC memory by a CPU in NS state. That case is handled by v7m_handle_execute_nsc(). However the instruction also has defined behaviour in a couple of other cases: * SG instruction in NS memory (behaves as a NOP) * SG in S memory but CPU already secure (clears IT bits and does nothing else) * SG instruction in v8M without Security Extension (NOP) These can be implemented in translate.c. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1507556919-24992-10-git-send-email-peter.maydell@linaro.org --- target/arm/translate.c | 23 ++++++++++++++++++++++- 1 file changed, 22 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 int disas_thumb2_insn(DisasContext *s, uint32_t insn) * - load/store doubleword, load/store exclusive, ldacq/strel, * table branch. */ - if (insn & 0x01200000) { + if (insn == 0xe97fe97f && arm_dc_feature(s, ARM_FEATURE_M) && + arm_dc_feature(s, ARM_FEATURE_V8)) { + /* 0b1110_1001_0111_1111_1110_1001_0111_111 + * - SG (v8M only) + * The bulk of the behaviour for this instruction is implemented + * in v7m_handle_execute_nsc(), which deals with the insn when + * it is executed by a CPU in non-secure state from memory + * which is Secure & NonSecure-Callable. + * Here we only need to handle the remaining cases: + * * in NS memory (including the "security extension not + * implemented" case) : NOP + * * in S memory but CPU already secure (clear IT bits) + * We know that the attribute for the memory this insn is + * in must match the current CPU state, because otherwise + * get_phys_addr_pmsav8 would have generated an exception. + */ + if (s->v8m_secure) { + /* Like the IT insn, we don't need to generate any code */ + s->condexec_cond = 0; + s->condexec_mask = 0; + } + } else if (insn & 0x01200000) { /* 0b1110_1000_x11x_xxxx_xxxx_xxxx_xxxx_xxxx * - load/store dual (post-indexed) * 0b1111_1001_x10x_xxxx_xxxx_xxxx_xxxx_xxxx -- 2.7.4 Coverity points out that we forgot the 'break' for the SAU_CTRL write case (CID1381683). This has no actual visible consequences because it happens that the following case is effectively a no-op. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-id: 1507742676-9908-1-git-send-email-peter.maydell@linaro.org Reviewed-by: Richard Henderson <richard.henderson@linaro.org> --- hw/intc/armv7m_nvic.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, return; } cpu->env.sau.ctrl = value & 3; + break; case 0xdd4: /* SAU_TYPE */ if (!arm_feature(&cpu->env, ARM_FEATURE_V8)) { goto bad_offset; -- 2.7.4 This calculation of the first exception vector in the ITNS<n> register being accessed: int startvec = 32 * (offset - 0x380) + NVIC_FIRST_IRQ; is incorrect, because offset is in bytes, so we only want to multiply by 8. Spotted by Coverity (CID 1381484, CID 1381488), though it is not correct that it actually overflows the buffer, because we have a 'startvec + i < s->num_irq' guard. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 1507650856-11718-1-git-send-email-peter.maydell@linaro.org --- hw/intc/armv7m_nvic.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c index XXXXXXX..XXXXXXX 100644 --- a/hw/intc/armv7m_nvic.c +++ b/hw/intc/armv7m_nvic.c @@ -XXX,XX +XXX,XX @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, MemTxAttrs attrs) return ((s->num_irq - NVIC_FIRST_IRQ) / 32) - 1; case 0x380 ... 0x3bf: /* NVIC_ITNS<n> */ { - int startvec = 32 * (offset - 0x380) + NVIC_FIRST_IRQ; + int startvec = 8 * (offset - 0x380) + NVIC_FIRST_IRQ; int i; if (!arm_feature(&cpu->env, ARM_FEATURE_V8)) { @@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value, switch (offset) { case 0x380 ... 0x3bf: /* NVIC_ITNS<n> */ { - int startvec = 32 * (offset - 0x380) + NVIC_FIRST_IRQ; + int startvec = 8 * (offset - 0x380) + NVIC_FIRST_IRQ; int i; if (!arm_feature(&cpu->env, ARM_FEATURE_V8)) { -- 2.7.4 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