[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-arm] [PATCH v2 18/23] target/arm: Move CPU state
|
From: |
Alex Bennée |
|
Subject: |
Re: [Qemu-devel] [Qemu-arm] [PATCH v2 18/23] target/arm: Move CPU state dumping routines to helper.c |
|
Date: |
Mon, 17 Jun 2019 15:41:13 +0100 |
|
User-agent: |
mu4e 1.3.2; emacs 26.1 |
Philippe Mathieu-Daudé <address@hidden> writes:
> From: Samuel Ortiz <address@hidden>
>
> They're not TCG specific and should be living the generic helper file
> instead.
>
> Signed-off-by: Samuel Ortiz <address@hidden>
> Reviewed-by: Robert Bradford <address@hidden>
> [PMD: Rebased]
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
> target/arm/helper.c | 214
> +++++++++++++++++++++++++++++++++++++
Hmm so helper is a mix of non-TCG and TCG bits whereas helper-a64.c is
basically just TCG helpers. It makes me wonder if we are breaking a
convention here as helper.c is traditionally only TCG helpers.
It feels like there should be different file that is unambiguously used
for both TCG and KVM based workloads where things like the cpu dump code
can live.
> target/arm/internals.h | 8 ++
> target/arm/translate-a64.c | 127 ----------------------
> target/arm/translate.c | 87 ---------------
> target/arm/translate.h | 5 -
> 5 files changed, 222 insertions(+), 219 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index a4af02c984..8c32b2bc0d 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -11293,4 +11293,218 @@ void aarch64_sve_change_el(CPUARMState *env, int
> old_el,
> aarch64_sve_narrow_vq(env, new_len + 1);
> }
> }
> +
> +void aarch64_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> +{
> + ARMCPU *cpu = ARM_CPU(cs);
> + CPUARMState *env = &cpu->env;
> + uint32_t psr = pstate_read(env);
> + int i;
> + int el = arm_current_el(env);
> + const char *ns_status;
> +
> + qemu_fprintf(f, " PC=%016" PRIx64 " ", env->pc);
> + for (i = 0; i < 32; i++) {
> + if (i == 31) {
> + qemu_fprintf(f, " SP=%016" PRIx64 "\n", env->xregs[i]);
> + } else {
> + qemu_fprintf(f, "X%02d=%016" PRIx64 "%s", i, env->xregs[i],
> + (i + 2) % 3 ? " " : "\n");
> + }
> + }
> +
> + if (arm_feature(env, ARM_FEATURE_EL3) && el != 3) {
> + ns_status = env->cp15.scr_el3 & SCR_NS ? "NS " : "S ";
> + } else {
> + ns_status = "";
> + }
> + qemu_fprintf(f, "PSTATE=%08x %c%c%c%c %sEL%d%c",
> + psr,
> + psr & PSTATE_N ? 'N' : '-',
> + psr & PSTATE_Z ? 'Z' : '-',
> + psr & PSTATE_C ? 'C' : '-',
> + psr & PSTATE_V ? 'V' : '-',
> + ns_status,
> + el,
> + psr & PSTATE_SP ? 'h' : 't');
> +
> + if (cpu_isar_feature(aa64_bti, cpu)) {
> + qemu_fprintf(f, " BTYPE=%d", (psr & PSTATE_BTYPE) >> 10);
> + }
> + if (!(flags & CPU_DUMP_FPU)) {
> + qemu_fprintf(f, "\n");
> + return;
> + }
> + if (fp_exception_el(env, el) != 0) {
> + qemu_fprintf(f, " FPU disabled\n");
> + return;
> + }
> + qemu_fprintf(f, " FPCR=%08x FPSR=%08x\n",
> + vfp_get_fpcr(env), vfp_get_fpsr(env));
> +
> + if (cpu_isar_feature(aa64_sve, cpu) && sve_exception_el(env, el) == 0) {
> + int j, zcr_len = sve_zcr_len_for_el(env, el);
> +
> + for (i = 0; i <= FFR_PRED_NUM; i++) {
> + bool eol;
> + if (i == FFR_PRED_NUM) {
> + qemu_fprintf(f, "FFR=");
> + /* It's last, so end the line. */
> + eol = true;
> + } else {
> + qemu_fprintf(f, "P%02d=", i);
> + switch (zcr_len) {
> + case 0:
> + eol = i % 8 == 7;
> + break;
> + case 1:
> + eol = i % 6 == 5;
> + break;
> + case 2:
> + case 3:
> + eol = i % 3 == 2;
> + break;
> + default:
> + /* More than one quadword per predicate. */
> + eol = true;
> + break;
> + }
> + }
> + for (j = zcr_len / 4; j >= 0; j--) {
> + int digits;
> + if (j * 4 + 4 <= zcr_len + 1) {
> + digits = 16;
> + } else {
> + digits = (zcr_len % 4 + 1) * 4;
> + }
> + qemu_fprintf(f, "%0*" PRIx64 "%s", digits,
> + env->vfp.pregs[i].p[j],
> + j ? ":" : eol ? "\n" : " ");
> + }
> + }
> +
> + for (i = 0; i < 32; i++) {
> + if (zcr_len == 0) {
> + qemu_fprintf(f, "Z%02d=%016" PRIx64 ":%016" PRIx64 "%s",
> + i, env->vfp.zregs[i].d[1],
> + env->vfp.zregs[i].d[0], i & 1 ? "\n" : " ");
> + } else if (zcr_len == 1) {
> + qemu_fprintf(f, "Z%02d=%016" PRIx64 ":%016" PRIx64
> + ":%016" PRIx64 ":%016" PRIx64 "\n",
> + i, env->vfp.zregs[i].d[3],
> env->vfp.zregs[i].d[2],
> + env->vfp.zregs[i].d[1], env->vfp.zregs[i].d[0]);
> + } else {
> + for (j = zcr_len; j >= 0; j--) {
> + bool odd = (zcr_len - j) % 2 != 0;
> + if (j == zcr_len) {
> + qemu_fprintf(f, "Z%02d[%x-%x]=", i, j, j - 1);
> + } else if (!odd) {
> + if (j > 0) {
> + qemu_fprintf(f, " [%x-%x]=", j, j - 1);
> + } else {
> + qemu_fprintf(f, " [%x]=", j);
> + }
> + }
> + qemu_fprintf(f, "%016" PRIx64 ":%016" PRIx64 "%s",
> + env->vfp.zregs[i].d[j * 2 + 1],
> + env->vfp.zregs[i].d[j * 2],
> + odd || j == 0 ? "\n" : ":");
> + }
> + }
> + }
> + } else {
> + for (i = 0; i < 32; i++) {
> + uint64_t *q = aa64_vfp_qreg(env, i);
> + qemu_fprintf(f, "Q%02d=%016" PRIx64 ":%016" PRIx64 "%s",
> + i, q[1], q[0], (i & 1 ? "\n" : " "));
> + }
> + }
> +}
> #endif
> +
> +void arm_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> +{
> + ARMCPU *cpu = ARM_CPU(cs);
> + CPUARMState *env = &cpu->env;
> + int i;
> +
> + if (is_a64(env)) {
> + aarch64_cpu_dump_state(cs, f, flags);
> + return;
> + }
> +
> + for (i = 0; i < 16; i++) {
> + qemu_fprintf(f, "R%02d=%08x", i, env->regs[i]);
> + if ((i % 4) == 3) {
> + qemu_fprintf(f, "\n");
> + } else {
> + qemu_fprintf(f, " ");
> + }
> + }
> +
> + if (arm_feature(env, ARM_FEATURE_M)) {
> + uint32_t xpsr = xpsr_read(env);
> + const char *mode;
> + const char *ns_status = "";
> +
> + if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {
> + ns_status = env->v7m.secure ? "S " : "NS ";
> + }
> +
> + if (xpsr & XPSR_EXCP) {
> + mode = "handler";
> + } else {
> + if (env->v7m.control[env->v7m.secure] &
> R_V7M_CONTROL_NPRIV_MASK) {
> + mode = "unpriv-thread";
> + } else {
> + mode = "priv-thread";
> + }
> + }
> +
> + qemu_fprintf(f, "XPSR=%08x %c%c%c%c %c %s%s\n",
> + xpsr,
> + xpsr & XPSR_N ? 'N' : '-',
> + xpsr & XPSR_Z ? 'Z' : '-',
> + xpsr & XPSR_C ? 'C' : '-',
> + xpsr & XPSR_V ? 'V' : '-',
> + xpsr & XPSR_T ? 'T' : 'A',
> + ns_status,
> + mode);
> + } else {
> + uint32_t psr = cpsr_read(env);
> + const char *ns_status = "";
> +
> + if (arm_feature(env, ARM_FEATURE_EL3) &&
> + (psr & CPSR_M) != ARM_CPU_MODE_MON) {
> + ns_status = env->cp15.scr_el3 & SCR_NS ? "NS " : "S ";
> + }
> +
> + qemu_fprintf(f, "PSR=%08x %c%c%c%c %c %s%s%d\n",
> + psr,
> + psr & CPSR_N ? 'N' : '-',
> + psr & CPSR_Z ? 'Z' : '-',
> + psr & CPSR_C ? 'C' : '-',
> + psr & CPSR_V ? 'V' : '-',
> + psr & CPSR_T ? 'T' : 'A',
> + ns_status,
> + aarch32_mode_name(psr), (psr & 0x10) ? 32 : 26);
> + }
> +
> + if (flags & CPU_DUMP_FPU) {
> + int numvfpregs = 0;
> + if (arm_feature(env, ARM_FEATURE_VFP)) {
> + numvfpregs += 16;
> + }
> + if (arm_feature(env, ARM_FEATURE_VFP3)) {
> + numvfpregs += 16;
> + }
> + for (i = 0; i < numvfpregs; i++) {
> + uint64_t v = *aa32_vfp_dreg(env, i);
> + qemu_fprintf(f, "s%02d=%08x s%02d=%08x d%02d=%016" PRIx64 "\n",
> + i * 2, (uint32_t)v,
> + i * 2 + 1, (uint32_t)(v >> 32),
> + i, v);
> + }
> + qemu_fprintf(f, "FPSCR: %08x\n", vfp_get_fpscr(env));
> + }
> +}
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 06e676bf62..56281d8ece 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1042,4 +1042,12 @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t
> address,
> int *prot, bool *is_subpage,
> ARMMMUFaultInfo *fi, uint32_t *mregion);
>
> +#ifdef TARGET_AARCH64
> +void aarch64_cpu_dump_state(CPUState *cs, FILE *f, int flags);
> +#else
> +static inline void aarch64_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> +{
> +}
> +#endif
> +
> #endif
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index ae739f6575..8abe1f0e4f 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -152,133 +152,6 @@ static void set_btype(DisasContext *s, int val)
> s->btype = -1;
> }
>
> -void aarch64_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> -{
> - ARMCPU *cpu = ARM_CPU(cs);
> - CPUARMState *env = &cpu->env;
> - uint32_t psr = pstate_read(env);
> - int i;
> - int el = arm_current_el(env);
> - const char *ns_status;
> -
> - qemu_fprintf(f, " PC=%016" PRIx64 " ", env->pc);
> - for (i = 0; i < 32; i++) {
> - if (i == 31) {
> - qemu_fprintf(f, " SP=%016" PRIx64 "\n", env->xregs[i]);
> - } else {
> - qemu_fprintf(f, "X%02d=%016" PRIx64 "%s", i, env->xregs[i],
> - (i + 2) % 3 ? " " : "\n");
> - }
> - }
> -
> - if (arm_feature(env, ARM_FEATURE_EL3) && el != 3) {
> - ns_status = env->cp15.scr_el3 & SCR_NS ? "NS " : "S ";
> - } else {
> - ns_status = "";
> - }
> - qemu_fprintf(f, "PSTATE=%08x %c%c%c%c %sEL%d%c",
> - psr,
> - psr & PSTATE_N ? 'N' : '-',
> - psr & PSTATE_Z ? 'Z' : '-',
> - psr & PSTATE_C ? 'C' : '-',
> - psr & PSTATE_V ? 'V' : '-',
> - ns_status,
> - el,
> - psr & PSTATE_SP ? 'h' : 't');
> -
> - if (cpu_isar_feature(aa64_bti, cpu)) {
> - qemu_fprintf(f, " BTYPE=%d", (psr & PSTATE_BTYPE) >> 10);
> - }
> - if (!(flags & CPU_DUMP_FPU)) {
> - qemu_fprintf(f, "\n");
> - return;
> - }
> - if (fp_exception_el(env, el) != 0) {
> - qemu_fprintf(f, " FPU disabled\n");
> - return;
> - }
> - qemu_fprintf(f, " FPCR=%08x FPSR=%08x\n",
> - vfp_get_fpcr(env), vfp_get_fpsr(env));
> -
> - if (cpu_isar_feature(aa64_sve, cpu) && sve_exception_el(env, el) == 0) {
> - int j, zcr_len = sve_zcr_len_for_el(env, el);
> -
> - for (i = 0; i <= FFR_PRED_NUM; i++) {
> - bool eol;
> - if (i == FFR_PRED_NUM) {
> - qemu_fprintf(f, "FFR=");
> - /* It's last, so end the line. */
> - eol = true;
> - } else {
> - qemu_fprintf(f, "P%02d=", i);
> - switch (zcr_len) {
> - case 0:
> - eol = i % 8 == 7;
> - break;
> - case 1:
> - eol = i % 6 == 5;
> - break;
> - case 2:
> - case 3:
> - eol = i % 3 == 2;
> - break;
> - default:
> - /* More than one quadword per predicate. */
> - eol = true;
> - break;
> - }
> - }
> - for (j = zcr_len / 4; j >= 0; j--) {
> - int digits;
> - if (j * 4 + 4 <= zcr_len + 1) {
> - digits = 16;
> - } else {
> - digits = (zcr_len % 4 + 1) * 4;
> - }
> - qemu_fprintf(f, "%0*" PRIx64 "%s", digits,
> - env->vfp.pregs[i].p[j],
> - j ? ":" : eol ? "\n" : " ");
> - }
> - }
> -
> - for (i = 0; i < 32; i++) {
> - if (zcr_len == 0) {
> - qemu_fprintf(f, "Z%02d=%016" PRIx64 ":%016" PRIx64 "%s",
> - i, env->vfp.zregs[i].d[1],
> - env->vfp.zregs[i].d[0], i & 1 ? "\n" : " ");
> - } else if (zcr_len == 1) {
> - qemu_fprintf(f, "Z%02d=%016" PRIx64 ":%016" PRIx64
> - ":%016" PRIx64 ":%016" PRIx64 "\n",
> - i, env->vfp.zregs[i].d[3],
> env->vfp.zregs[i].d[2],
> - env->vfp.zregs[i].d[1], env->vfp.zregs[i].d[0]);
> - } else {
> - for (j = zcr_len; j >= 0; j--) {
> - bool odd = (zcr_len - j) % 2 != 0;
> - if (j == zcr_len) {
> - qemu_fprintf(f, "Z%02d[%x-%x]=", i, j, j - 1);
> - } else if (!odd) {
> - if (j > 0) {
> - qemu_fprintf(f, " [%x-%x]=", j, j - 1);
> - } else {
> - qemu_fprintf(f, " [%x]=", j);
> - }
> - }
> - qemu_fprintf(f, "%016" PRIx64 ":%016" PRIx64 "%s",
> - env->vfp.zregs[i].d[j * 2 + 1],
> - env->vfp.zregs[i].d[j * 2],
> - odd || j == 0 ? "\n" : ":");
> - }
> - }
> - }
> - } else {
> - for (i = 0; i < 32; i++) {
> - uint64_t *q = aa64_vfp_qreg(env, i);
> - qemu_fprintf(f, "Q%02d=%016" PRIx64 ":%016" PRIx64 "%s",
> - i, q[1], q[0], (i & 1 ? "\n" : " "));
> - }
> - }
> -}
> -
> void gen_a64_set_pc_im(uint64_t val)
> {
> tcg_gen_movi_i64(cpu_pc, val);
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index d0ab3e27e6..1e50627690 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -12416,93 +12416,6 @@ void gen_intermediate_code(CPUState *cpu,
> TranslationBlock *tb, int max_insns)
> translator_loop(ops, &dc.base, cpu, tb, max_insns);
> }
>
> -void arm_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> -{
> - ARMCPU *cpu = ARM_CPU(cs);
> - CPUARMState *env = &cpu->env;
> - int i;
> -
> - if (is_a64(env)) {
> - aarch64_cpu_dump_state(cs, f, flags);
> - return;
> - }
> -
> - for (i = 0; i < 16; i++) {
> - qemu_fprintf(f, "R%02d=%08x", i, env->regs[i]);
> - if ((i % 4) == 3) {
> - qemu_fprintf(f, "\n");
> - } else {
> - qemu_fprintf(f, " ");
> - }
> - }
> -
> - if (arm_feature(env, ARM_FEATURE_M)) {
> - uint32_t xpsr = xpsr_read(env);
> - const char *mode;
> - const char *ns_status = "";
> -
> - if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {
> - ns_status = env->v7m.secure ? "S " : "NS ";
> - }
> -
> - if (xpsr & XPSR_EXCP) {
> - mode = "handler";
> - } else {
> - if (env->v7m.control[env->v7m.secure] &
> R_V7M_CONTROL_NPRIV_MASK) {
> - mode = "unpriv-thread";
> - } else {
> - mode = "priv-thread";
> - }
> - }
> -
> - qemu_fprintf(f, "XPSR=%08x %c%c%c%c %c %s%s\n",
> - xpsr,
> - xpsr & XPSR_N ? 'N' : '-',
> - xpsr & XPSR_Z ? 'Z' : '-',
> - xpsr & XPSR_C ? 'C' : '-',
> - xpsr & XPSR_V ? 'V' : '-',
> - xpsr & XPSR_T ? 'T' : 'A',
> - ns_status,
> - mode);
> - } else {
> - uint32_t psr = cpsr_read(env);
> - const char *ns_status = "";
> -
> - if (arm_feature(env, ARM_FEATURE_EL3) &&
> - (psr & CPSR_M) != ARM_CPU_MODE_MON) {
> - ns_status = env->cp15.scr_el3 & SCR_NS ? "NS " : "S ";
> - }
> -
> - qemu_fprintf(f, "PSR=%08x %c%c%c%c %c %s%s%d\n",
> - psr,
> - psr & CPSR_N ? 'N' : '-',
> - psr & CPSR_Z ? 'Z' : '-',
> - psr & CPSR_C ? 'C' : '-',
> - psr & CPSR_V ? 'V' : '-',
> - psr & CPSR_T ? 'T' : 'A',
> - ns_status,
> - aarch32_mode_name(psr), (psr & 0x10) ? 32 : 26);
> - }
> -
> - if (flags & CPU_DUMP_FPU) {
> - int numvfpregs = 0;
> - if (arm_feature(env, ARM_FEATURE_VFP)) {
> - numvfpregs += 16;
> - }
> - if (arm_feature(env, ARM_FEATURE_VFP3)) {
> - numvfpregs += 16;
> - }
> - for (i = 0; i < numvfpregs; i++) {
> - uint64_t v = *aa32_vfp_dreg(env, i);
> - qemu_fprintf(f, "s%02d=%08x s%02d=%08x d%02d=%016" PRIx64 "\n",
> - i * 2, (uint32_t)v,
> - i * 2 + 1, (uint32_t)(v >> 32),
> - i, v);
> - }
> - qemu_fprintf(f, "FPSCR: %08x\n", vfp_get_fpscr(env));
> - }
> -}
> -
> void restore_state_to_opc(CPUARMState *env, TranslationBlock *tb,
> target_ulong *data)
> {
> diff --git a/target/arm/translate.h b/target/arm/translate.h
> index dc06dce767..1dd3ac0a41 100644
> --- a/target/arm/translate.h
> +++ b/target/arm/translate.h
> @@ -169,7 +169,6 @@ static inline void disas_set_insn_syndrome(DisasContext
> *s, uint32_t syn)
> #ifdef TARGET_AARCH64
> void a64_translate_init(void);
> void gen_a64_set_pc_im(uint64_t val);
> -void aarch64_cpu_dump_state(CPUState *cs, FILE *f, int flags);
> extern const TranslatorOps aarch64_translator_ops;
> #else
> static inline void a64_translate_init(void)
> @@ -179,10 +178,6 @@ static inline void a64_translate_init(void)
> static inline void gen_a64_set_pc_im(uint64_t val)
> {
> }
> -
> -static inline void aarch64_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> -{
> -}
> #endif
>
> void arm_test_cc(DisasCompare *cmp, int cc);
--
Alex Bennée
- Re: [Qemu-devel] [Qemu-arm] [PATCH v2 11/23] target/arm: Declare v7m_cpacr_pass() publicly, (continued)
- [Qemu-devel] [PATCH v2 10/23] target/arm: Move the v7-M Security State helpers to v7m_helper, Philippe Mathieu-Daudé, 2019/06/15
- [Qemu-devel] [PATCH v2 15/23] target/arm: Make ARM TLB filling routine static, Philippe Mathieu-Daudé, 2019/06/15
- [Qemu-devel] [PATCH v2 16/23] target/arm: Make arm_deliver_fault() static, Philippe Mathieu-Daudé, 2019/06/15
- [Qemu-devel] [PATCH v2 20/23] target/arm: Define TCG dependent functions when TCG is enabled, Philippe Mathieu-Daudé, 2019/06/15
- [Qemu-devel] [PATCH v2 18/23] target/arm: Move CPU state dumping routines to helper.c, Philippe Mathieu-Daudé, 2019/06/15
- Re: [Qemu-devel] [Qemu-arm] [PATCH v2 18/23] target/arm: Move CPU state dumping routines to helper.c,
Alex Bennée <=
- [Qemu-devel] [PATCH v2 21/23] target/arm: Do not build TCG objects when TCG is off, Philippe Mathieu-Daudé, 2019/06/15
- [Qemu-devel] [PATCH v2 19/23] target/arm: Move watchpoints APIs to helper.c, Philippe Mathieu-Daudé, 2019/06/15
- [Qemu-devel] [RFC PATCH v2 22/23] target/arm: Restrict semi-hosting to TCG, Philippe Mathieu-Daudé, 2019/06/15
- [Qemu-devel] [NOTFORMERGE PATCH v2 23/23] Missing symbols when building with --disable-tcg, Philippe Mathieu-Daudé, 2019/06/15