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

+ 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?

Shiva, Could you also please check and confirm?

Also, there seems to be a copy-paste mistake in existing code as 
identified in another thread, may be we can fix along?

bool pr = extract32(dawrx, PPC_BIT_NR(63), 1);

regards,
Harsh
> +    if ((env->msr & (1ull << MSR_PR)) && !pr) {
>           return false;
> -    }
> -
> -    wt = extract32(dawrx, PPC_BIT_NR(59), 1);
> -    wti = extract32(dawrx, PPC_BIT_NR(60), 1);
> -    hv = extract32(dawrx, PPC_BIT_NR(61), 1);
> -    sv = extract32(dawrx, PPC_BIT_NR(62), 1);
> -    pr = extract32(dawrx, PPC_BIT_NR(62), 1);
> -
> -    if ((env->msr & ((target_ulong)1 << MSR_PR)) && !pr) {
> -        return false;
> -    } else if ((env->msr & ((target_ulong)1 << MSR_HV)) && !hv) {
> +    } else if ((env->msr & (1ull << MSR_HV)) && !hv) {
>           return false;
>       } else if (!sv) {
>           return false;
>       }
>   
>       if (!wti) {
> -        if (env->msr & ((target_ulong)1 << MSR_DR)) {
> +        if (env->msr & (1ull << MSR_DR)) {
>               return wt;
>           } else {
>               return !wt;