Re: [PATCH v3 33/51] target/ppc: Simplify ppc_cpu_debug_check_watchpoint

On Sat, 11 Jul, 2026, 10:22 pm Richard Henderson, <
[email protected]> wrote:

> On 7/11/26 03:54, Harsh Prateek Bora wrote:
> > + Cedric, Shiva
> >
> > On 11/07/26 2:23 am, Richard Henderson wrote:
> >> We don't need to check either ISA bit, because that's already
> >> done starting with SPR registration.  Replace target_ulong with
> >> uint64_t, as we're under TARGET_PPC64.
> >>
> >> Signed-off-by: Richard Henderson <[email protected]>
> >> ---
> >>   target/ppc/tcg-excp_helper.c | 30 +++++++++---------------------
> >>   1 file changed, 9 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/target/ppc/tcg-excp_helper.c b/target/ppc/tcg-excp_helper.c
> >> index 99ffe4a820..4065440949 100644
> >> --- a/target/ppc/tcg-excp_helper.c
> >> +++ b/target/ppc/tcg-excp_helper.c
> >> @@ -353,35 +353,23 @@ bool ppc_cpu_debug_check_watchpoint(CPUState *cs,
> CPUBreakpoint *wp)
> >>   {
> >>   #if defined(TARGET_PPC64)
> >>       CPUPPCState *env = cpu_env(cs);
> >> -    bool wt, wti, hv, sv, pr;
> >> -    uint32_t dawrx;
> >> +    uint32_t dawrx = env->spr[wp->id ? SPR_DAWRX1 : SPR_DAWRX0];
> >> +    bool wt = extract32(dawrx, PPC_BIT_NR(59), 1);
> >> +    bool wti = extract32(dawrx, PPC_BIT_NR(60), 1);
> >> +    bool hv = extract32(dawrx, PPC_BIT_NR(61), 1);
> >> +    bool sv = extract32(dawrx, PPC_BIT_NR(62), 1);
> >> +    bool pr = extract32(dawrx, PPC_BIT_NR(62), 1);
> >> -    if ((env->insns_flags2 & PPC2_ISA207S) &&
> >> -        (wp == env->dawr_watchpoint[0])) {
> >> -        dawrx = env->spr[SPR_DAWRX0];
> >> -    } else if ((env->insns_flags2 & PPC2_ISA310) &&
> >> -               (wp == env->dawr_watchpoint[1])) {
> >> -        dawrx = env->spr[SPR_DAWRX1];
> >> -    } else {
> >
> > Previously, there were different ISA version checks resulting in
> different behaviour, now
> > with this change we are allowing for both ISA versions. Not sure if this
> would be an
> > expected behaviour?
>
> Per the commit comment, the ISA version checks are done when registering
> the system
> register.  One cannot write to DAWRX1 unless ISA310, and therefore
> dawr_watchpoint[1] will
> not be set nor wp->id be 1.
>

Thanks for confirming, Richard!

Regards
Harsh

>
>
> r~
>
>