[PATCH 10/11] target/i386: document incorrect semantics of watchpoint following MOV/POP SS

Paolo Bonzini posted 11 patches 5 months, 3 weeks ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>
[PATCH 10/11] target/i386: document incorrect semantics of watchpoint following MOV/POP SS
Posted by Paolo Bonzini 5 months, 3 weeks ago
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/tcg/sysemu/bpt_helper.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/i386/tcg/sysemu/bpt_helper.c b/target/i386/tcg/sysemu/bpt_helper.c
index c1d5fce250c..b29acf41c38 100644
--- a/target/i386/tcg/sysemu/bpt_helper.c
+++ b/target/i386/tcg/sysemu/bpt_helper.c
@@ -215,6 +215,12 @@ void breakpoint_handler(CPUState *cs)
         if (cs->watchpoint_hit->flags & BP_CPU) {
             cs->watchpoint_hit = NULL;
             if (check_hw_breakpoints(env, false)) {
+                /*
+                 * FIXME: #DB should be delayed by one instruction if
+                 * INHIBIT_IRQ is set (STI cannot trigger a watchpoint).
+                 * The delayed #DB should also fuse with one generated
+                 * by ICEBP (aka INT1).
+                 */
                 raise_exception(env, EXCP01_DB);
             } else {
                 cpu_loop_exit_noexc(cs);
-- 
2.45.1
Re: [PATCH 10/11] target/i386: document incorrect semantics of watchpoint following MOV/POP SS
Posted by Richard Henderson 5 months, 3 weeks ago
On 6/4/24 02:18, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/tcg/sysemu/bpt_helper.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/target/i386/tcg/sysemu/bpt_helper.c b/target/i386/tcg/sysemu/bpt_helper.c
> index c1d5fce250c..b29acf41c38 100644
> --- a/target/i386/tcg/sysemu/bpt_helper.c
> +++ b/target/i386/tcg/sysemu/bpt_helper.c
> @@ -215,6 +215,12 @@ void breakpoint_handler(CPUState *cs)
>           if (cs->watchpoint_hit->flags & BP_CPU) {
>               cs->watchpoint_hit = NULL;
>               if (check_hw_breakpoints(env, false)) {
> +                /*
> +                 * FIXME: #DB should be delayed by one instruction if
> +                 * INHIBIT_IRQ is set (STI cannot trigger a watchpoint).
> +                 * The delayed #DB should also fuse with one generated
> +                 * by ICEBP (aka INT1).
> +                 */
>                   raise_exception(env, EXCP01_DB);
>               } else {
>                   cpu_loop_exit_noexc(cs);

Should be fixable with some sort of state machine initiated with 
TCGCPUOps.debug_check_watchpoint, but not easy.

Acked-by: Richard Henderson <richard.henderson@linaro.org>


r~