Re: [PATCH v3 42/51] accel/tcg: Merge CPUState.{break, watch}point_hit

On 10/7/26 22:53, Richard Henderson wrote:
> Merge and name bp_wp_hit.  Now that all targets have been
> converted to use debug_excp_handler hit argument, we only
> need one pointer to an outstanding debug event.
> 
> Signed-off-by: Richard Henderson <[email protected]>
> ---
>   include/hw/core/cpu.h  |  3 +--
>   accel/tcg/cpu-exec.c   |  7 +++---
>   accel/tcg/watchpoint.c |  4 ++--
>   gdbstub/system.c       | 50 ++++++++++++++++++++++--------------------
>   target/arm/hvf/hvf.c   |  5 ++---
>   target/arm/kvm.c       |  2 +-
>   target/i386/kvm/kvm.c  |  4 ++--
>   target/ppc/kvm.c       |  2 +-
>   target/s390x/kvm/kvm.c |  2 +-
>   9 files changed, 39 insertions(+), 40 deletions(-)
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 1a42940d92..5401d7c14b 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -526,8 +526,7 @@ struct CPUState {
>       /* ice debug support */
>       IntervalTreeRoot breakpoints;
>       IntervalTreeRoot watchpoints;
> -    CPUBreakpoint *breakpoint_hit;
> -    CPUBreakpoint *watchpoint_hit;
> +    CPUBreakpoint *bp_wp_hit;

During the last QEMU community call Alex said these fields are for
TCG, and this series confirms it (also this patch $subject).
I planned to move that to the TCG accelerator state at some point,
but then I'm confused by the uses in the hw accelerators...

> diff --git a/gdbstub/system.c b/gdbstub/system.c
> index c78fb330c4..3f31b75173 100644
> --- a/gdbstub/system.c
> +++ b/gdbstub/system.c
> @@ -150,34 +150,36 @@ static void gdb_vm_state_change(void *opaque, bool running, RunState state)
>   
>       switch (state) {
>       case RUN_STATE_DEBUG:
> -        if (cpu->watchpoint_hit) {
> -            const char *type;
> +        {
> +            CPUBreakpoint *hit = cpu->bp_wp_hit;
>   
> -            switch (cpu->watchpoint_hit->flags & BP_MEM_ACCESS) {
> -            case BP_MEM_READ:
> -                type = "r";
> -                break;
> -            case BP_MEM_ACCESS:
> -                type = "a";
> -                break;
> -            default:
> -                type = "";
> -                break;
> +            if (hit) {
> +                unsigned watch = hit->flags & BP_MEM_ACCESS;
> +
> +                hit->flags &= ~BP_WATCHPOINT_HIT;
> +                cpu->bp_wp_hit = NULL;
> +
> +                if (watch) {
> +                    static const char hit_type[4][2] = {
> +                        [BP_MEM_READ] = "r",
> +                        [BP_MEM_WRITE] = "",
> +                        [BP_MEM_ACCESS] = "a",
> +                    };
> +                    const char *type = hit_type[watch];
> +
> +                    trace_gdbstub_hit_watchpoint(type, gdb_get_cpu_index(cpu),
> +                                                 hit->hitaddr);
> +                    g_string_printf(buf, "T%02xthread:%s;%swatch:%"
> +                                    VADDR_PRIx ";",
> +                                    GDB_SIGNAL_TRAP, tid->str, type,
> +                                    hit->hitaddr);
> +                    goto send_packet;
> +                }
> +                /* else breakpoint */
>               }
> -            trace_gdbstub_hit_watchpoint(type,
> -                                         gdb_get_cpu_index(cpu),
> -                                         cpu->watchpoint_hit->hitaddr);
> -            g_string_printf(buf, "T%02xthread:%s;%swatch:%" VADDR_PRIx ";",
> -                            GDB_SIGNAL_TRAP, tid->str, type,
> -                            cpu->watchpoint_hit->hitaddr);
> -
> -            cpu->watchpoint_hit->flags &= ~BP_WATCHPOINT_HIT;
> -            cpu->watchpoint_hit = NULL;
> -            goto send_packet;
> -        } else {
>               trace_gdbstub_hit_break();
> +            ret = GDB_SIGNAL_TRAP;
>           }
> -        ret = GDB_SIGNAL_TRAP;
>           break;
>       case RUN_STATE_PAUSED:
>           trace_gdbstub_hit_paused();
> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
> index 32e5f43ee3..0fd72c5bde 100644
> --- a/target/arm/hvf/hvf.c
> +++ b/target/arm/hvf/hvf.c
> @@ -2370,13 +2370,12 @@ static int hvf_handle_exception(CPUState *cpu, hv_vcpu_exit_exception_t *excp)
>   
>           cpu_synchronize_state(cpu);
>   
> -        CPUBreakpoint *wp =
> -            find_hw_watchpoint(cpu, excp->virtual_address);
> +        CPUBreakpoint *wp = find_hw_watchpoint(cpu, excp->virtual_address);
>           if (!wp) {
>               error_report("EXCP_DEBUG but unknown hw watchpoint");
>           }
>           wp->hitaddr = excp->virtual_address;
> -        cpu->watchpoint_hit = wp;
> +        cpu->bp_wp_hit = wp;
>           break;
>       }
>       case EC_DATAABORT: {
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index c2f921095a..f66332299c 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -1520,7 +1520,7 @@ static bool kvm_arm_handle_debug(ARMCPU *cpu,
>           CPUBreakpoint *wp = find_hw_watchpoint(cs, debug_exit->far);
>           if (wp) {
>               wp->hitaddr = debug_exit->far;
> -            cs->watchpoint_hit = wp;
> +            cs->bp_wp_hit = wp;
>               return true;
>           }
>           break;
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 2e1ecb1c83..c4f0dc1e02 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -6263,13 +6263,13 @@ static int kvm_handle_debug(X86CPU *cpu,
>                           break;
>                       case 0x1:
>                           ret = EXCP_DEBUG;
> -                        cs->watchpoint_hit = &hw_watchpoint;
> +                        cs->bp_wp_hit = &hw_watchpoint;
>                           hw_watchpoint.hitaddr = hw_breakpoint[n].addr;
>                           hw_watchpoint.flags = BP_MEM_WRITE;
>                           break;
>                       case 0x3:
>                           ret = EXCP_DEBUG;
> -                        cs->watchpoint_hit = &hw_watchpoint;
> +                        cs->bp_wp_hit = &hw_watchpoint;
>                           hw_watchpoint.hitaddr = hw_breakpoint[n].addr;
>                           hw_watchpoint.flags = BP_MEM_ACCESS;
>                           break;
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index f161ba6415..f11f8457d0 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -1592,7 +1592,7 @@ static int kvm_handle_hw_breakpoint(CPUState *cs,
>               n = find_hw_watchpoint(arch_info->address, &flag);
>               if (n >= 0) {
>                   handle = DEBUG_RETURN_GDB;
> -                cs->watchpoint_hit = &hw_watchpoint;
> +                cs->bp_wp_hit = &hw_watchpoint;
>                   hw_watchpoint.hitaddr = arch_info->address;
>                   hw_watchpoint.flags = flag;
>               }
> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> index b75dab5f3c..5c3cde97b8 100644
> --- a/target/s390x/kvm/kvm.c
> +++ b/target/s390x/kvm/kvm.c
> @@ -1863,7 +1863,7 @@ static int kvm_arch_handle_debug_exit(S390CPU *cpu)
>       switch (arch_info->type) {
>       case KVM_HW_WP_WRITE:
>           if (find_hw_breakpoint(arch_info->addr, -1, arch_info->type)) {
> -            cs->watchpoint_hit = &hw_watchpoint;
> +            cs->bp_wp_hit = &hw_watchpoint;
>               hw_watchpoint.hitaddr = arch_info->addr;
>               hw_watchpoint.flags = BP_MEM_WRITE;
>               ret = EXCP_DEBUG;