Hi Alex, Is there anything I could do to get the WHPX debugging support accepted into QEMU? Would the proposed callback AccelOpsClass work for you, or would you prefer another approach?
Best, Ivan -----Original Message----- From: Qemu-devel <qemu-devel-bounces+ivan=sysprogs....@nongnu.org> On Behalf Of Ivan Shcherbakov Sent: Monday, February 28, 2022 6:09 PM To: 'Alex Bennée' <alex.ben...@linaro.org> Cc: 'Peter Maydell' <peter.mayd...@linaro.org>; m...@redhat.com; qemu-devel@nongnu.org; arm...@redhat.com; 'Paolo Bonzini' <pbonz...@redhat.com> Subject: RE: [PATCH 3/3] whpx: Added support for breakpoints and stepping Hi Alex, Thanks for getting back to me. It is definitely the latter case (i.e. it is possible to change it while the target is stopped at a breakpoint before resuming any VCPUs). vm_state_notify() does look like it's intended for this type of notifications, but unfortunately, it doesn't make a distinction between stepping and running normally. Below is the relevant code from gdbstub.c: >static int gdb_continue_partial(char *newstates) { > int flag = 0; > > /* Various corner cases omitted for brevity */ > if (vm_prepare_start()) { > return 0; > } > CPU_FOREACH(cpu) { > switch (newstates[cpu->cpu_index]) { > case 's': > trace_gdbstub_op_stepping(cpu->cpu_index); > cpu_single_step(cpu, gdbserver_state.sstep_flags); > cpu_resume(cpu); > flag = 1; > break; > case 'c': > trace_gdbstub_op_continue_cpu(cpu->cpu_index); > cpu_resume(cpu); > flag = 1; > break; > default: > res = -1; > break; > } > } >} Also: >int vm_prepare_start(void) >{ > runstate_set(RUN_STATE_RUNNING); > vm_state_notify(1, RUN_STATE_RUNNING); > return 0; >} and: >void vm_state_notify(bool running, RunState state); So, currently, vm_prepare_start() has no way of distinguishing between single-stepping and normal running unless gdb_continue_partial() scans the 'newstates' array before calling it, and passes some extra argument to vm_prepare_start(), indicating whether a step request was present anywhere in the array. I couldn't find any existing run state that would match single-stepping, and adding another run state looks like a very non-trivial change that can easily backfire. Adding another argument to vm_state_notify() could be easier, but I am still afraid it could break some other part of QEMU, so I thought adding a new member to AccelOpsClass would be a safer bet. But again, if you think another way to do it is better, I am very open to it. Best regards, Ivan -----Original Message----- From: Alex Bennée <alex.ben...@linaro.org> Sent: Monday, February 28, 2022 2:28 AM To: Ivan Shcherbakov <i...@sysprogs.com> Cc: 'Peter Maydell' <peter.mayd...@linaro.org>; 'Paolo Bonzini' <pbonz...@redhat.com>; qemu-devel@nongnu.org; arm...@redhat.com; m...@redhat.com Subject: Re: [PATCH 3/3] whpx: Added support for breakpoints and stepping Is the limitation that whpx_set_exception_exit_bitmap needs to be set before any vCPU can be run or that it cannot be set if any vCPUs are currently running? If it is the later wouldn't adding a hook into the vm_change_state_head callbacks be enough?