* [PATCH] accel/tcg/cpu-exec: fix precise single-stepping after interrupt
@ 2022-02-14 13:26 Luc Michel
2022-02-23 10:51 ` Luc Michel
2022-02-25 0:23 ` Richard Henderson
0 siblings, 2 replies; 6+ messages in thread
From: Luc Michel @ 2022-02-14 13:26 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, Richard Henderson, Peter Maydell,
Philippe Mathieu-Daudé,
Paolo Bonzini
In some cases, cpu->exit_request can be false after handling the
interrupt, leading to another TB being executed instead of returning
to the main loop.
Fix this by returning true unconditionally when in single-step mode.
Fixes: ba3c35d9c4026361fd380b269dc6def9510b7166
Signed-off-by: Luc Michel <lmichel@kalray.eu>
---
Coming back on this issue I worked on with Richard in 2020. The issue is
that when debugging the guest with GDB, the first instruction of the IRQ
handler is missed by GDB (it's still executed though).
It happened to me again in TCG RR mode (but not in MTTCG). It seems that
cpu->exit_request can be false in RR mode when returning from
cc->tcg_ops->cpu_exec_interrupt, leading to cpu_handle_interrupt
returning false and the next TB being executed, instead of the EXCP_DEBUG
being handled.
---
accel/tcg/cpu-exec.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 8b4cd6c59d..74d7f83f34 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -796,13 +796,17 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
/*
* After processing the interrupt, ensure an EXCP_DEBUG is
* raised when single-stepping so that GDB doesn't miss the
* next instruction.
*/
- cpu->exception_index =
- (cpu->singlestep_enabled ? EXCP_DEBUG : -1);
- *last_tb = NULL;
+ if (unlikely(cpu->singlestep_enabled)) {
+ cpu->exception_index = EXCP_DEBUG;
+ return true;
+ } else {
+ cpu->exception_index = -1;
+ *last_tb = NULL;
+ }
}
/* The target hook may have updated the 'cpu->interrupt_request';
* reload the 'interrupt_request' value */
interrupt_request = cpu->interrupt_request;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] accel/tcg/cpu-exec: fix precise single-stepping after interrupt
2022-02-14 13:26 [PATCH] accel/tcg/cpu-exec: fix precise single-stepping after interrupt Luc Michel
@ 2022-02-23 10:51 ` Luc Michel
2022-02-25 0:23 ` Richard Henderson
1 sibling, 0 replies; 6+ messages in thread
From: Luc Michel @ 2022-02-23 10:51 UTC (permalink / raw)
To: qemu-devel
Cc: Richard Henderson, Peter Maydell, Philippe Mathieu-Daudé,
Paolo Bonzini
On 14:26 Mon 14 Feb , Luc Michel wrote:
> In some cases, cpu->exit_request can be false after handling the
> interrupt, leading to another TB being executed instead of returning
> to the main loop.
>
> Fix this by returning true unconditionally when in single-step mode.
>
> Fixes: ba3c35d9c4026361fd380b269dc6def9510b7166
>
> Signed-off-by: Luc Michel <lmichel@kalray.eu>
Hi Richard, did you have time to have a look at this patch?
Thanks,
Luc
> ---
> Coming back on this issue I worked on with Richard in 2020. The issue is
> that when debugging the guest with GDB, the first instruction of the IRQ
> handler is missed by GDB (it's still executed though).
>
> It happened to me again in TCG RR mode (but not in MTTCG). It seems that
> cpu->exit_request can be false in RR mode when returning from
> cc->tcg_ops->cpu_exec_interrupt, leading to cpu_handle_interrupt
> returning false and the next TB being executed, instead of the EXCP_DEBUG
> being handled.
> ---
> accel/tcg/cpu-exec.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 8b4cd6c59d..74d7f83f34 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -796,13 +796,17 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
> /*
> * After processing the interrupt, ensure an EXCP_DEBUG is
> * raised when single-stepping so that GDB doesn't miss the
> * next instruction.
> */
> - cpu->exception_index =
> - (cpu->singlestep_enabled ? EXCP_DEBUG : -1);
> - *last_tb = NULL;
> + if (unlikely(cpu->singlestep_enabled)) {
> + cpu->exception_index = EXCP_DEBUG;
> + return true;
> + } else {
> + cpu->exception_index = -1;
> + *last_tb = NULL;
> + }
> }
> /* The target hook may have updated the 'cpu->interrupt_request';
> * reload the 'interrupt_request' value */
> interrupt_request = cpu->interrupt_request;
> }
> --
> 2.17.1
>
--
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] accel/tcg/cpu-exec: fix precise single-stepping after interrupt
2022-02-14 13:26 [PATCH] accel/tcg/cpu-exec: fix precise single-stepping after interrupt Luc Michel
2022-02-23 10:51 ` Luc Michel
@ 2022-02-25 0:23 ` Richard Henderson
2022-02-25 0:52 ` Richard Henderson
1 sibling, 1 reply; 6+ messages in thread
From: Richard Henderson @ 2022-02-25 0:23 UTC (permalink / raw)
To: Luc Michel, qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini
On 2/14/22 03:26, Luc Michel wrote:
> In some cases, cpu->exit_request can be false after handling the
> interrupt, leading to another TB being executed instead of returning
> to the main loop.
>
> Fix this by returning true unconditionally when in single-step mode.
>
> Fixes: ba3c35d9c4026361fd380b269dc6def9510b7166
>
> Signed-off-by: Luc Michel <lmichel@kalray.eu>
> ---
> Coming back on this issue I worked on with Richard in 2020. The issue is
> that when debugging the guest with GDB, the first instruction of the IRQ
> handler is missed by GDB (it's still executed though).
>
> It happened to me again in TCG RR mode (but not in MTTCG). It seems that
> cpu->exit_request can be false in RR mode when returning from
> cc->tcg_ops->cpu_exec_interrupt, leading to cpu_handle_interrupt
> returning false and the next TB being executed, instead of the EXCP_DEBUG
> being handled.
> ---
> accel/tcg/cpu-exec.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 8b4cd6c59d..74d7f83f34 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -796,13 +796,17 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
> /*
> * After processing the interrupt, ensure an EXCP_DEBUG is
> * raised when single-stepping so that GDB doesn't miss the
> * next instruction.
> */
> - cpu->exception_index =
> - (cpu->singlestep_enabled ? EXCP_DEBUG : -1);
> - *last_tb = NULL;
> + if (unlikely(cpu->singlestep_enabled)) {
> + cpu->exception_index = EXCP_DEBUG;
> + return true;
By returning here, you also need to qemu_mutex_unlock_iothread().
> + } else {
You can remove the else after the return.
Otherwise this looks good; sorry for the delay.
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] accel/tcg/cpu-exec: fix precise single-stepping after interrupt
2022-02-25 0:23 ` Richard Henderson
@ 2022-02-25 0:52 ` Richard Henderson
0 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2022-02-25 0:52 UTC (permalink / raw)
To: Luc Michel, qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini
On 2/24/22 14:23, Richard Henderson wrote:
> On 2/14/22 03:26, Luc Michel wrote:
>> In some cases, cpu->exit_request can be false after handling the
>> interrupt, leading to another TB being executed instead of returning
>> to the main loop.
>>
>> Fix this by returning true unconditionally when in single-step mode.
>>
>> Fixes: ba3c35d9c4026361fd380b269dc6def9510b7166
>>
>> Signed-off-by: Luc Michel <lmichel@kalray.eu>
>> ---
>> Coming back on this issue I worked on with Richard in 2020. The issue is
>> that when debugging the guest with GDB, the first instruction of the IRQ
>> handler is missed by GDB (it's still executed though).
>>
>> It happened to me again in TCG RR mode (but not in MTTCG). It seems that
>> cpu->exit_request can be false in RR mode when returning from
>> cc->tcg_ops->cpu_exec_interrupt, leading to cpu_handle_interrupt
>> returning false and the next TB being executed, instead of the EXCP_DEBUG
>> being handled.
>> ---
>> accel/tcg/cpu-exec.c | 10 +++++++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
>> index 8b4cd6c59d..74d7f83f34 100644
>> --- a/accel/tcg/cpu-exec.c
>> +++ b/accel/tcg/cpu-exec.c
>> @@ -796,13 +796,17 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
>> /*
>> * After processing the interrupt, ensure an EXCP_DEBUG is
>> * raised when single-stepping so that GDB doesn't miss the
>> * next instruction.
>> */
>> - cpu->exception_index =
>> - (cpu->singlestep_enabled ? EXCP_DEBUG : -1);
>> - *last_tb = NULL;
>> + if (unlikely(cpu->singlestep_enabled)) {
>> + cpu->exception_index = EXCP_DEBUG;
>> + return true;
>
> By returning here, you also need to qemu_mutex_unlock_iothread().
>
>> + } else {
>
> You can remove the else after the return.
> Otherwise this looks good; sorry for the delay.
I'll just fix this up when applying, it's minor enough.
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] accel/tcg/cpu-exec: fix precise single-stepping after interrupt
2022-06-20 14:24 ` [PATCH] accel/tcg/cpu-exec: fix precise single-stepping after interrupt Luc Michel
@ 2022-06-20 14:27 ` Luc Michel
0 siblings, 0 replies; 6+ messages in thread
From: Luc Michel @ 2022-06-20 14:27 UTC (permalink / raw)
To: qemu-devel
Cc: Richard Henderson, Peter Maydell, Philippe Mathieu-Daudé,
Paolo Bonzini
On 16:24 Mon 20 Jun , Luc Michel wrote:
> In some cases, cpu->exit_request can be false after handling the
> interrupt, leading to another TB being executed instead of returning
> to the main loop.
>
> Fix this by returning true unconditionally when in single-step mode.
>
> Fixes: ba3c35d9c4026361fd380b269dc6def9510b7166
>
Please ignore this old patch
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] accel/tcg/cpu-exec: fix precise single-stepping after interrupt
2022-06-20 14:24 [PATCH 0/7] semihosting: proper QEMU exit on semihosted exit syscall Luc Michel
@ 2022-06-20 14:24 ` Luc Michel
2022-06-20 14:27 ` Luc Michel
0 siblings, 1 reply; 6+ messages in thread
From: Luc Michel @ 2022-06-20 14:24 UTC (permalink / raw)
To: qemu-devel
Cc: Luc Michel, Richard Henderson, Peter Maydell,
Philippe Mathieu-Daudé,
Paolo Bonzini
In some cases, cpu->exit_request can be false after handling the
interrupt, leading to another TB being executed instead of returning
to the main loop.
Fix this by returning true unconditionally when in single-step mode.
Fixes: ba3c35d9c4026361fd380b269dc6def9510b7166
Signed-off-by: Luc Michel <lmichel@kalray.eu>
---
Coming back on this issue I worked on with Richard in 2020. The issue is
that when debugging the guest with GDB, the first instruction of the IRQ
handler is missed by GDB (it's still executed though).
It happened to me again in TCG RR mode (but not in MTTCG). It seems that
cpu->exit_request can be false in RR mode when returning from
cc->tcg_ops->cpu_exec_interrupt, leading to cpu_handle_interrupt
returning false and the next TB being executed, instead of the EXCP_DEBUG
being handled.
---
accel/tcg/cpu-exec.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 8b4cd6c59d..74d7f83f34 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -796,13 +796,17 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
/*
* After processing the interrupt, ensure an EXCP_DEBUG is
* raised when single-stepping so that GDB doesn't miss the
* next instruction.
*/
- cpu->exception_index =
- (cpu->singlestep_enabled ? EXCP_DEBUG : -1);
- *last_tb = NULL;
+ if (unlikely(cpu->singlestep_enabled)) {
+ cpu->exception_index = EXCP_DEBUG;
+ return true;
+ } else {
+ cpu->exception_index = -1;
+ *last_tb = NULL;
+ }
}
/* The target hook may have updated the 'cpu->interrupt_request';
* reload the 'interrupt_request' value */
interrupt_request = cpu->interrupt_request;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-06-20 14:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-14 13:26 [PATCH] accel/tcg/cpu-exec: fix precise single-stepping after interrupt Luc Michel
2022-02-23 10:51 ` Luc Michel
2022-02-25 0:23 ` Richard Henderson
2022-02-25 0:52 ` Richard Henderson
2022-06-20 14:24 [PATCH 0/7] semihosting: proper QEMU exit on semihosted exit syscall Luc Michel
2022-06-20 14:24 ` [PATCH] accel/tcg/cpu-exec: fix precise single-stepping after interrupt Luc Michel
2022-06-20 14:27 ` Luc Michel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.