|
From: | Nikola Brkovic |
Subject: | Re: [PATCH] arm/monitor: add register name resolution |
Date: | Fri, 23 Sep 2022 20:11:01 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.0 |
Hello, thank you for your feedback. The choice of registers was relatively arbitrary, I chose registers that would be accessible and valid under all profiles and not part of the CP15. I will look into how this could be implemented through gdbstub, as I was not aware of that possibility. Kind regards, Nikola On 9/22/22 15:15, Peter Maydell wrote:
On Sat, 10 Sept 2022 at 15:14, Nikola Brkovic <nb91605@student.uni-lj.si> wrote:This patch allows the monitor to resolve the stack pointer, instruction pointer, system status register and FPU status register on ARM targets. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1145 Signed-off-by: Nikola Brkovic <nb91605@student.uni-lj.si> --- target/arm/monitor.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/target/arm/monitor.c b/target/arm/monitor.c index 80c64fa355..143c95bca4 100644 --- a/target/arm/monitor.c +++ b/target/arm/monitor.c @@ -31,6 +31,7 @@ #include "qapi/qmp/qerror.h" #include "qapi/qmp/qdict.h" #include "qom/qom-qobject.h" +#include "monitor/hmp-target.h" static GICCapability *gic_cap_new(int version) { @@ -228,3 +229,31 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type, return expansion_info; } + +static target_long monitor_get_cpsr(Monitor *mon, const struct MonitorDef *md, + int val) +{ + CPUArchState *env = mon_get_cpu_env(mon); + return cpsr_read(env); +} + +const MonitorDef monitor_defs[] = { + { "sp|r13", offsetof(CPUARMState, regs[13])}, + { "lr|r14", offsetof(CPUARMState, regs[14])}, +#ifndef TARGET_AARCH64 + { "pc|r15|ip", offsetof(CPUARMState, regs[15]) }, +#else + { "pc|ip", offsetof(CPUARMState, pc) }, +#endif + + /* State registers */ + { "cpsr", 0, &monitor_get_cpsr}, + { "fpscr", offsetof(CPUARMState, vfp.fp_status)},Hi; thanks for this patch. Unfortunately, it doesn't look to me like it handles the fact that 64-bit vs 32-bit is a runtime question, not a compile-time one: (1) if this is a 64-bit CPU executing AArch64 code then we shouldn't be resolving sp/lr as regs[13] and regs[14] (2) if this is a 64-bit CPU executing AArch32 code then we shouldn't be resolving pc/ip as env->pc. As a more minor bug: (3) fpscr isn't only in vfp.fp_status, it's more complicated than that -- you need to call vfp_get_fpscr(). Also, why only these registers ? As a wider design thing, I'm not really a fan of target_monitor_defs(): it's the kind of quick hack that got added to QEMU decades ago when we supported about three different target architectures but which doesn't scale to the set we support now. It would be nicer to be able to tie this into the existing gdbstub support code for reading and writing registers. (Though that too has issues with CPUs which support multiple modes like AArch32 vs AArch64). thanks -- PMM
[Prev in Thread] | Current Thread | [Next in Thread] |