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?


Reply via email to