On 1/7/2021 7:40 AM, Alex Bennée wrote:
> Steve Sistare <steven.sist...@oracle.com> writes:
>
>> Modify the gdb server so a continue command appears to resume execution
>> when in RUN_STATE_SUSPENDED. Do not print the next gdb prompt, but do not
>> actually resume instruction fetch. While in this "fake" running mode, a
>> ctrl-C returns the user to the gdb prompt.
>
> What exactly is the purpose of this? To hide the details of the runstate
> as controlled by the user? I wouldn't expect someone using gdb debugging
> not to also have control of the HMP/QMP interface.
Without this fix, a user that attaches gdb to a suspended guest breaks the
guest. The state is RUN_STATE_SUSPENDED. After attaching gdb and typing
continue or quit, qemu transitions to RUN_STATE_RUNNING (wrong) and the
guest continues execution (wrong). The guest loops polling on an acpi port,
deep in a call stack under acpi_suspend_enter(). Sending a system_wakeup
request via qmp or hmp fails with the message "Error: Unable to wake up:
guest is not in suspended state".
With the fix, the state remains RUN_STATE_SUSPENDED throughout, until the
system_wakeup request, and the guest pc does not change. gdb interprets
RUN_STATE_SUSPENDED as "target is running", without causing instruction
fetch to resume.
If you are satisfied, I will add this explanation to the commit message.
- Steve
>> Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
>> ---
>> gdbstub.c | 11 +++++++++--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdbstub.c b/gdbstub.c
>> index f3a318c..2f0d9ff 100644
>> --- a/gdbstub.c
>> +++ b/gdbstub.c
>> @@ -461,7 +461,9 @@ static inline void gdb_continue(void)
>> #else
>> if (!runstate_needs_reset()) {
>> trace_gdbstub_op_continue();
>> - vm_start();
>> + if (!runstate_check(RUN_STATE_SUSPENDED)) {
>> + vm_start();
>> + }
>> }
>> #endif
>> }
>> @@ -490,7 +492,7 @@ static int gdb_continue_partial(char *newstates)
>> int flag = 0;
>>
>> if (!runstate_needs_reset()) {
>> - if (vm_prepare_start()) {
>> + if (!runstate_check(RUN_STATE_SUSPENDED) && vm_prepare_start()) {
>> return 0;
>> }
>>
>> @@ -2835,6 +2837,9 @@ static void gdb_read_byte(uint8_t ch)
>> /* when the CPU is running, we cannot do anything except stop
>> it when receiving a char */
>> vm_stop(RUN_STATE_PAUSED);
>> + } else if (runstate_check(RUN_STATE_SUSPENDED) && ch == 3) {
>> + /* Received ctrl-c from gdb */
>> + gdb_vm_state_change(0, 0, RUN_STATE_PAUSED);
>> } else
>> #endif
>> {
>> @@ -3282,6 +3287,8 @@ static void gdb_sigterm_handler(int signal)
>> {
>> if (runstate_is_running()) {
>> vm_stop(RUN_STATE_PAUSED);
>> + } else if (runstate_check(RUN_STATE_SUSPENDED)) {
>> + gdb_vm_state_change(0, 0, RUN_STATE_PAUSED);
>> }
>> }
>> #endif
>
>
