| Message ID | 1519815685-29200-5-git-send-email-abdallah.bouassida@lauterbach.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/arm: Add a dynamic XML-description of the cp-registers to GDB | expand |
On 28 February 2018 at 11:01, Abdallah Bouassida <abdallah.bouassida@lauterbach.com> wrote: > This is a callback to set the cp-regs registered by the dynamic XML. > > Signed-off-by: Abdallah Bouassida <abdallah.bouassida@lauterbach.com> > --- >>> Some of our customers need to connect to Qemu using our tool TRACE32® >>> via GDB, >>> and for some use case they need to have write access to some particular >>> cpregs. >>> So, it will be nice to have this capability! >>> Usually, a user won't modify these registers unless he knows what he is >>> doing! > >> I also still don't really like using write_raw_cp_reg() here -- >> it will bypass some behaviour you want and in some cases will >> just break the emulation because invariants we assume will >> hold no longer hold. It would be a lot lot safer to not >> provide write access at all, only read access. > > Adding to that our customers may need this write access, our tool TRACE32® > needs this also in some particular cases. For example: temporary disabling MMU > to do a physical memory access. By clearing the SCTLR bit? That's a good example of a case that won't work reliably. If you clear the SCTLR.M bit via raw_write this will not perform the tlb_flush() that it needs to, which means that if anything does a memory access via the QEMU TLB it may get the wrong cached results. If you always clear the bit, do one gdb memory access then set the bit then it will probably not run into problems but you're walking on thin ice. thanks -- PMM
diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 0e35f64..f4fea98 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -2120,6 +2120,8 @@ static inline bool cp_access_ok(int current_el, /* Raw read of a coprocessor register (as needed for migration, etc) */ uint64_t read_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri); +void write_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t v); + /** * write_list_to_cpustate * @cpu: ARMCPU diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c index e08ad79..57bd418 100644 --- a/target/arm/gdbstub.c +++ b/target/arm/gdbstub.c @@ -183,12 +183,31 @@ static int arm_gdb_get_sysreg(CPUARMState *env, uint8_t *buf, int reg) return 0; } +static int arm_gdb_set_sysreg(CPUARMState *env, uint8_t *buf, int reg) +{ + ARMCPU *cpu = arm_env_get_cpu(env); + const ARMCPRegInfo *ri; + uint32_t key; + uint32_t tmp; + + tmp = ldl_p(buf); + key = cpu->dyn_xml.cpregs_keys[reg]; + ri = get_arm_cp_reginfo(arm_env_get_cpu(env)->cp_regs, key); + if (ri) { + if (!(ri->type & ARM_CP_CONST)) { + write_raw_cp_reg(env, ri, tmp); + return cpreg_field_is_64bit(ri) ? 8 : 4; + } + } + return 0; +} + void arm_register_gdb_regs_for_features(CPUState *cs) { int n; n = arm_gen_dynamic_xml(cs); - gdb_register_coprocessor(cs, arm_gdb_get_sysreg, NULL, + gdb_register_coprocessor(cs, arm_gdb_get_sysreg, arm_gdb_set_sysreg, n, "system-registers.xml", 0); } diff --git a/target/arm/helper.c b/target/arm/helper.c index 1594ec45..4a4afbf 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -200,7 +200,7 @@ uint64_t read_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri) } } -static void write_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri, +void write_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t v) { /* Raw write of a coprocessor register (as needed for migration, etc).