"Ivan Shcherbakov" <i...@sysprogs.com> writes:
> Hi Alex and Peter, > > It would be great to hear your feedback on handling the WHPX stepping > via an added argument to vm_prepare_start(). It is only called from 2 > places, so the changes will be minimal. I this works for you, I will > gladly send an updated patch. 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? > I am also fully open to other suggestions. You obviously know the QEMU > codebase much better, so I'm happy to refactor it so that it blends in > well with the rest of the code. > > Best, > Ivan > > -----Original Message----- > From: Qemu-devel <qemu-devel-bounces+ivan=sysprogs....@nongnu.org> On Behalf > Of Ivan Shcherbakov > Sent: Thursday, February 24, 2022 7:54 AM > To: 'Alex Bennée' <alex.ben...@linaro.org>; 'Peter Maydell' > <peter.mayd...@linaro.org> > Cc: '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 > >> I haven't looked at the rest of the patch -- but can you explain where >> whpx is different from how other accelerators handle debug such that >> it needs to know whether gdb is connected or not ? > This mainly comes from the way single-stepping is handled. WHPX needs to know > whether you want to trap INT1 before starting the first VCPU. The current > gdbstub implementation doesn’t make it easy. Assume the scenario: > > 1. You have 2 VCPUs. You run the first one and step the second one. > 2. gdb_continue_partial() calls cpu_resume(0) 3. gdb_continue_partial() calls > cpu_single_step(1). > > WHPX needs to know whether to trap INT1 at step #2 (starting the first CPU), > but it won't know it until step #3. So, the current logic simply checks if > gdb is connected at all in step #2. > >>Just the fact you have connected to the gdbserver shouldn't affect how you >>run WHPX up until the point there are things you need to trap - i.e. >>handling installed breakpoints. > > This can be addressed by adding a "bool stepping_expected" argument to >> vm_prepare_start(). It will be set to true if gdb_continue_partial() >> expects ANY thread to be stepped, and will be false otherwise. It >> will also require a new callback in AccelOpsClass (e.g. >> on_vm_starting(bool stepping_expected)) that will be called from >> vm_prepare_start(). The WHPX implementation will then check if any >> breakpoints are set, and if the last call to this function expected >> stepping, and use it to decide whether to trap INT1. > > Let me know if this will work better. > > Best, > Ivan -- Alex Bennée
