On 1/18/21 10:10 AM, Claudio Fontana wrote: > On 1/17/21 5:48 PM, Philippe Mathieu-Daudé wrote: >> Watchpoint funtions use cpu_restore_state() which is only >> available when TCG accelerator is built. Restrict them >> to TCG. >> >> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org> > > I am doing some of this in my series, and I did not notice that > cpu_watchpoint_insert was also TCG only. > > Probably we should merge this somehow. > > I thought it was used by gdbstub.c as well, passing flags BP_GDB .
BP_MEM_ACCESS for watchpoint. > I noticed that gdbstub does something else entirely for kvm_enabled(), ie, > kvm_insert_breakpoint, > but what about the other accels, it seems that the code flows to the > cpu_breakpoint_insert and watchpoint_insert..? > > should cpu_breakpoint_insert have the same fate then? > > And is this really all TCG specific? > > From gdbstub.c:1020: > > static int gdb_breakpoint_insert(int type, target_ulong addr, target_ulong > len) > { > CPUState *cpu; > int err = 0; > > if (kvm_enabled()) { > return kvm_insert_breakpoint(gdbserver_state.c_cpu, addr, len, type); Doh I missed that. I remember Peter and Richard explained it once but I forgot and couldn't find on the list, maybe it was on IRC. See i.e. in target/arm/kvm64.c: 312 int kvm_arch_insert_hw_breakpoint(target_ulong addr, 313 target_ulong len, int type) 314 { 315 switch (type) { 316 case GDB_BREAKPOINT_HW: 317 return insert_hw_breakpoint(addr); 318 break; 319 case GDB_WATCHPOINT_READ: 320 case GDB_WATCHPOINT_WRITE: 321 case GDB_WATCHPOINT_ACCESS: 322 return insert_hw_watchpoint(addr, len, type); 323 default: 324 return -ENOSYS; 325 } 326 } > >> --- >> RFC because we could keep that code by adding an empty >> stub for cpu_restore_state(), but it is unclear as >> the function is named generically. >> --- >> include/hw/core/cpu.h | 4 ++-- >> softmmu/physmem.c | 4 ++++ >> 2 files changed, 6 insertions(+), 2 deletions(-)