Make is_prefix_insn_excp() prototype but have it guarded by
a tcg_enabled() check. Inline part of it in powerpc_excp_books().

Extract POWERPC_EXCP_HV_EMU handling code to ppc_tcg_hv_emu(),
also exposing its prototype in "internal.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/ppc/internal.h        |   6 +++
 target/ppc/excp_helper.c     | 101 +++++------------------------------
 target/ppc/tcg-excp_helper.c |  75 ++++++++++++++++++++++++++
 3 files changed, 93 insertions(+), 89 deletions(-)

diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index 62186bc1e61..0e66b29ec68 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -291,6 +291,12 @@ bool ppc_cpu_debug_check_breakpoint(CPUState *cs);
 bool ppc_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
 
 G_NORETURN void powerpc_checkstop(CPUPPCState *env, const char *reason);
+
+#if defined(TARGET_PPC64)
+bool is_prefix_insn_excp(CPUPPCState *env, int excp);
+void ppc_tcg_hv_emu(CPUPPCState *env, target_ulong *new_msr,
+                    int *srr0, int *srr1);
+#endif /* TARGET_PPC64 */
 #endif /* !CONFIG_USER_ONLY */
 
 FIELD(GER_MSK, XMSK, 0, 4)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 6a12402b23a..56a56148a40 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -19,6 +19,7 @@
 #include "qemu/osdep.h"
 #include "qemu/main-loop.h"
 #include "qemu/log.h"
+#include "system/kvm.h"
 #include "system/tcg.h"
 #include "system/system.h"
 #include "system/runstate.h"
@@ -1194,81 +1195,6 @@ static bool books_vhyp_handles_hv_excp(PowerPCCPU *cpu)
     return false;
 }
 
-#ifdef CONFIG_TCG
-static bool is_prefix_insn(CPUPPCState *env, uint32_t insn)
-{
-    if (!(env->insns_flags2 & PPC2_ISA310)) {
-        return false;
-    }
-    return ((insn & 0xfc000000) == 0x04000000);
-}
-
-static bool is_prefix_insn_excp(PowerPCCPU *cpu, int excp)
-{
-    CPUPPCState *env = &cpu->env;
-
-    if (!(env->insns_flags2 & PPC2_ISA310)) {
-        return false;
-    }
-
-    if (!tcg_enabled()) {
-        /*
-         * This does not load instructions and set the prefix bit correctly
-         * for injected interrupts with KVM. That may have to be discovered
-         * and set by the KVM layer before injecting.
-         */
-        return false;
-    }
-
-    switch (excp) {
-    case POWERPC_EXCP_MCHECK:
-        if (!(env->error_code & PPC_BIT(42))) {
-            /*
-             * Fetch attempt caused a machine check, so attempting to fetch
-             * again would cause a recursive machine check.
-             */
-            return false;
-        }
-        break;
-    case POWERPC_EXCP_HDSI:
-        /* HDSI PRTABLE_FAULT has the originating access type in error_code */
-        if ((env->spr[SPR_HDSISR] & DSISR_PRTABLE_FAULT) &&
-            (env->error_code == MMU_INST_FETCH)) {
-            /*
-             * Fetch failed due to partition scope translation, so prefix
-             * indication is not relevant (and attempting to load the
-             * instruction at NIP would cause recursive faults with the same
-             * translation).
-             */
-            return false;
-        }
-        break;
-
-    case POWERPC_EXCP_DSI:
-    case POWERPC_EXCP_DSEG:
-    case POWERPC_EXCP_ALIGN:
-    case POWERPC_EXCP_PROGRAM:
-    case POWERPC_EXCP_FPU:
-    case POWERPC_EXCP_TRACE:
-    case POWERPC_EXCP_HV_EMU:
-    case POWERPC_EXCP_VPU:
-    case POWERPC_EXCP_VSXU:
-    case POWERPC_EXCP_FU:
-    case POWERPC_EXCP_HV_FU:
-        break;
-    default:
-        return false;
-    }
-
-    return is_prefix_insn(env, ppc_ldl_code(env, env->nip));
-}
-#else
-static bool is_prefix_insn_excp(PowerPCCPU *cpu, int excp)
-{
-    return false;
-}
-#endif
-
 static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
 {
     CPUPPCState *env = &cpu->env;
@@ -1310,7 +1236,15 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
     }
     vector |= env->excp_prefix;
 
-    if (is_prefix_insn_excp(cpu, excp)) {
+    if (env->insns_flags2 & PPC2_ISA310) {
+        /* nothing to do */
+    } else if (kvm_enabled()) {
+        /*
+         * This does not load instructions and set the prefix bit correctly
+         * for injected interrupts with KVM. That may have to be discovered
+         * and set by the KVM layer before injecting.
+         */
+    } else if (tcg_enabled() && is_prefix_insn_excp(env, excp)) {
         msr |= PPC_BIT(34);
     }
 
@@ -1484,20 +1418,9 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
         new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
         break;
 #ifdef CONFIG_TCG
-    case POWERPC_EXCP_HV_EMU: {
-        uint32_t insn = ppc_ldl_code(env, env->nip);
-        env->spr[SPR_HEIR] = insn;
-        if (is_prefix_insn(env, insn)) {
-            uint32_t insn2 = ppc_ldl_code(env, env->nip + 4);
-            env->spr[SPR_HEIR] <<= 32;
-            env->spr[SPR_HEIR] |= insn2;
-        }
-        srr0 = SPR_HSRR0;
-        srr1 = SPR_HSRR1;
-        new_msr |= (target_ulong)MSR_HVB;
-        new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
+    case POWERPC_EXCP_HV_EMU:
+        ppc_tcg_hv_emu(env, &new_msr, &srr0, &srr1);
         break;
-    }
 #endif
     case POWERPC_EXCP_VPU:       /* Vector unavailable exception             */
     case POWERPC_EXCP_VSXU:       /* VSX unavailable exception               */
diff --git a/target/ppc/tcg-excp_helper.c b/target/ppc/tcg-excp_helper.c
index 268a1614597..dc5601a4577 100644
--- a/target/ppc/tcg-excp_helper.c
+++ b/target/ppc/tcg-excp_helper.c
@@ -283,4 +283,79 @@ uint32_t ppc_ldl_code(CPUArchState *env, target_ulong addr)
     return insn;
 }
 
+#if defined(TARGET_PPC64)
+
+static bool is_prefix_insn(CPUPPCState *env, uint32_t insn)
+{
+    if (!(env->insns_flags2 & PPC2_ISA310)) {
+        return false;
+    }
+    return ((insn & 0xfc000000) == 0x04000000);
+}
+
+bool is_prefix_insn_excp(CPUPPCState *env, int excp)
+{
+    switch (excp) {
+    case POWERPC_EXCP_MCHECK:
+        if (!(env->error_code & PPC_BIT(42))) {
+            /*
+             * Fetch attempt caused a machine check, so attempting to fetch
+             * again would cause a recursive machine check.
+             */
+            return false;
+        }
+        break;
+    case POWERPC_EXCP_HDSI:
+        /* HDSI PRTABLE_FAULT has the originating access type in error_code */
+        if ((env->spr[SPR_HDSISR] & DSISR_PRTABLE_FAULT) &&
+            (env->error_code == MMU_INST_FETCH)) {
+            /*
+             * Fetch failed due to partition scope translation, so prefix
+             * indication is not relevant (and attempting to load the
+             * instruction at NIP would cause recursive faults with the same
+             * translation).
+             */
+            return false;
+        }
+        break;
+
+    case POWERPC_EXCP_DSI:
+    case POWERPC_EXCP_DSEG:
+    case POWERPC_EXCP_ALIGN:
+    case POWERPC_EXCP_PROGRAM:
+    case POWERPC_EXCP_FPU:
+    case POWERPC_EXCP_TRACE:
+    case POWERPC_EXCP_HV_EMU:
+    case POWERPC_EXCP_VPU:
+    case POWERPC_EXCP_VSXU:
+    case POWERPC_EXCP_FU:
+    case POWERPC_EXCP_HV_FU:
+        break;
+    default:
+        return false;
+    }
+
+    return is_prefix_insn(env, ppc_ldl_code(env, env->nip));
+}
+
+void ppc_tcg_hv_emu(CPUPPCState *env, target_ulong *new_msr,
+                    int *srr0, int *srr1)
+{
+    uint32_t insn = ppc_ldl_code(env, env->nip);
+
+    env->spr[SPR_HEIR] = insn;
+    if (is_prefix_insn(env, insn)) {
+        uint32_t insn2 = ppc_ldl_code(env, env->nip + 4);
+
+        env->spr[SPR_HEIR] <<= 32;
+        env->spr[SPR_HEIR] |= insn2;
+    }
+    *srr0 = SPR_HSRR0;
+    *srr1 = SPR_HSRR1;
+    *new_msr |= (target_ulong)MSR_HVB;
+    *new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
+}
+
+#endif /* TARGET_PPC64 */
+
 #endif /* !CONFIG_USER_ONLY */
-- 
2.47.1

On 1/27/25 15:56, Philippe Mathieu-Daudé wrote:
> Make is_prefix_insn_excp() prototype but have it guarded by
> a tcg_enabled() check. Inline part of it in powerpc_excp_books().
> 
> Extract POWERPC_EXCP_HV_EMU handling code to ppc_tcg_hv_emu(),
> also exposing its prototype in "internal.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/ppc/internal.h        |   6 +++
>   target/ppc/excp_helper.c     | 101 +++++------------------------------
>   target/ppc/tcg-excp_helper.c |  75 ++++++++++++++++++++++++++
>   3 files changed, 93 insertions(+), 89 deletions(-)
> 
> diff --git a/target/ppc/internal.h b/target/ppc/internal.h
> index 62186bc1e61..0e66b29ec68 100644
> --- a/target/ppc/internal.h
> +++ b/target/ppc/internal.h
> @@ -291,6 +291,12 @@ bool ppc_cpu_debug_check_breakpoint(CPUState *cs);
>   bool ppc_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
>   
>   G_NORETURN void powerpc_checkstop(CPUPPCState *env, const char *reason);
> +
> +#if defined(TARGET_PPC64)
> +bool is_prefix_insn_excp(CPUPPCState *env, int excp);
> +void ppc_tcg_hv_emu(CPUPPCState *env, target_ulong *new_msr,
> +                    int *srr0, int *srr1);
> +#endif /* TARGET_PPC64 */
>   #endif /* !CONFIG_USER_ONLY */
>   
>   FIELD(GER_MSK, XMSK, 0, 4)
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 6a12402b23a..56a56148a40 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -19,6 +19,7 @@
>   #include "qemu/osdep.h"
>   #include "qemu/main-loop.h"
>   #include "qemu/log.h"
> +#include "system/kvm.h"
>   #include "system/tcg.h"
>   #include "system/system.h"
>   #include "system/runstate.h"
> @@ -1194,81 +1195,6 @@ static bool books_vhyp_handles_hv_excp(PowerPCCPU *cpu)
>       return false;
>   }
>   
> -#ifdef CONFIG_TCG
> -static bool is_prefix_insn(CPUPPCState *env, uint32_t insn)
> -{
> -    if (!(env->insns_flags2 & PPC2_ISA310)) {
> -        return false;
> -    }
> -    return ((insn & 0xfc000000) == 0x04000000);
> -}
> -
> -static bool is_prefix_insn_excp(PowerPCCPU *cpu, int excp)
> -{
> -    CPUPPCState *env = &cpu->env;
> -
> -    if (!(env->insns_flags2 & PPC2_ISA310)) {
> -        return false;
> -    }
> -
> -    if (!tcg_enabled()) {
> -        /*
> -         * This does not load instructions and set the prefix bit correctly
> -         * for injected interrupts with KVM. That may have to be discovered
> -         * and set by the KVM layer before injecting.
> -         */
> -        return false;
> -    }
> -
> -    switch (excp) {
> -    case POWERPC_EXCP_MCHECK:
> -        if (!(env->error_code & PPC_BIT(42))) {
> -            /*
> -             * Fetch attempt caused a machine check, so attempting to fetch
> -             * again would cause a recursive machine check.
> -             */
> -            return false;
> -        }
> -        break;
> -    case POWERPC_EXCP_HDSI:
> -        /* HDSI PRTABLE_FAULT has the originating access type in error_code */
> -        if ((env->spr[SPR_HDSISR] & DSISR_PRTABLE_FAULT) &&
> -            (env->error_code == MMU_INST_FETCH)) {
> -            /*
> -             * Fetch failed due to partition scope translation, so prefix
> -             * indication is not relevant (and attempting to load the
> -             * instruction at NIP would cause recursive faults with the same
> -             * translation).
> -             */
> -            return false;
> -        }
> -        break;
> -
> -    case POWERPC_EXCP_DSI:
> -    case POWERPC_EXCP_DSEG:
> -    case POWERPC_EXCP_ALIGN:
> -    case POWERPC_EXCP_PROGRAM:
> -    case POWERPC_EXCP_FPU:
> -    case POWERPC_EXCP_TRACE:
> -    case POWERPC_EXCP_HV_EMU:
> -    case POWERPC_EXCP_VPU:
> -    case POWERPC_EXCP_VSXU:
> -    case POWERPC_EXCP_FU:
> -    case POWERPC_EXCP_HV_FU:
> -        break;
> -    default:
> -        return false;
> -    }
> -
> -    return is_prefix_insn(env, ppc_ldl_code(env, env->nip));
> -}
> -#else
> -static bool is_prefix_insn_excp(PowerPCCPU *cpu, int excp)
> -{
> -    return false;
> -}
> -#endif
> -
>   static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>   {
>       CPUPPCState *env = &cpu->env;
> @@ -1310,7 +1236,15 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>       }
>       vector |= env->excp_prefix;
>   
> -    if (is_prefix_insn_excp(cpu, excp)) {
> +    if (env->insns_flags2 & PPC2_ISA310) {

I guess you meant checking for ! of above.
is_prefix_insn_excp() returns false for ! of above.

> +        /* nothing to do */
> +    } else if (kvm_enabled()) {
> +        /*
> +         * This does not load instructions and set the prefix bit correctly
> +         * for injected interrupts with KVM. That may have to be discovered
> +         * and set by the KVM layer before injecting.
> +         */
> +    } else if (tcg_enabled() && is_prefix_insn_excp(env, excp)) {
>           msr |= PPC_BIT(34);
>       }
>   
> @@ -1484,20 +1418,9 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>           new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
>           break;
>   #ifdef CONFIG_TCG
> -    case POWERPC_EXCP_HV_EMU: {
> -        uint32_t insn = ppc_ldl_code(env, env->nip);
> -        env->spr[SPR_HEIR] = insn;
> -        if (is_prefix_insn(env, insn)) {
> -            uint32_t insn2 = ppc_ldl_code(env, env->nip + 4);
> -            env->spr[SPR_HEIR] <<= 32;
> -            env->spr[SPR_HEIR] |= insn2;
> -        }
> -        srr0 = SPR_HSRR0;
> -        srr1 = SPR_HSRR1;
> -        new_msr |= (target_ulong)MSR_HVB;
> -        new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
> +    case POWERPC_EXCP_HV_EMU:
> +        ppc_tcg_hv_emu(env, &new_msr, &srr0, &srr1);

Naming suggestion: ppc_excp_hv_emu may be more apt.

Thanks,
Harsh

>           break;
> -    }
>   #endif
>       case POWERPC_EXCP_VPU:       /* Vector unavailable exception             */
>       case POWERPC_EXCP_VSXU:       /* VSX unavailable exception               */
> diff --git a/target/ppc/tcg-excp_helper.c b/target/ppc/tcg-excp_helper.c
> index 268a1614597..dc5601a4577 100644
> --- a/target/ppc/tcg-excp_helper.c
> +++ b/target/ppc/tcg-excp_helper.c
> @@ -283,4 +283,79 @@ uint32_t ppc_ldl_code(CPUArchState *env, target_ulong addr)
>       return insn;
>   }
>   
> +#if defined(TARGET_PPC64)
> +
> +static bool is_prefix_insn(CPUPPCState *env, uint32_t insn)
> +{
> +    if (!(env->insns_flags2 & PPC2_ISA310)) {
> +        return false;
> +    }
> +    return ((insn & 0xfc000000) == 0x04000000);
> +}
> +
> +bool is_prefix_insn_excp(CPUPPCState *env, int excp)
> +{
> +    switch (excp) {
> +    case POWERPC_EXCP_MCHECK:
> +        if (!(env->error_code & PPC_BIT(42))) {
> +            /*
> +             * Fetch attempt caused a machine check, so attempting to fetch
> +             * again would cause a recursive machine check.
> +             */
> +            return false;
> +        }
> +        break;
> +    case POWERPC_EXCP_HDSI:
> +        /* HDSI PRTABLE_FAULT has the originating access type in error_code */
> +        if ((env->spr[SPR_HDSISR] & DSISR_PRTABLE_FAULT) &&
> +            (env->error_code == MMU_INST_FETCH)) {
> +            /*
> +             * Fetch failed due to partition scope translation, so prefix
> +             * indication is not relevant (and attempting to load the
> +             * instruction at NIP would cause recursive faults with the same
> +             * translation).
> +             */
> +            return false;
> +        }
> +        break;
> +
> +    case POWERPC_EXCP_DSI:
> +    case POWERPC_EXCP_DSEG:
> +    case POWERPC_EXCP_ALIGN:
> +    case POWERPC_EXCP_PROGRAM:
> +    case POWERPC_EXCP_FPU:
> +    case POWERPC_EXCP_TRACE:
> +    case POWERPC_EXCP_HV_EMU:
> +    case POWERPC_EXCP_VPU:
> +    case POWERPC_EXCP_VSXU:
> +    case POWERPC_EXCP_FU:
> +    case POWERPC_EXCP_HV_FU:
> +        break;
> +    default:
> +        return false;
> +    }
> +
> +    return is_prefix_insn(env, ppc_ldl_code(env, env->nip));
> +}
> +
> +void ppc_tcg_hv_emu(CPUPPCState *env, target_ulong *new_msr,
> +                    int *srr0, int *srr1)
> +{
> +    uint32_t insn = ppc_ldl_code(env, env->nip);
> +
> +    env->spr[SPR_HEIR] = insn;
> +    if (is_prefix_insn(env, insn)) {
> +        uint32_t insn2 = ppc_ldl_code(env, env->nip + 4);
> +
> +        env->spr[SPR_HEIR] <<= 32;
> +        env->spr[SPR_HEIR] |= insn2;
> +    }
> +    *srr0 = SPR_HSRR0;
> +    *srr1 = SPR_HSRR1;
> +    *new_msr |= (target_ulong)MSR_HVB;
> +    *new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
> +}
> +
> +#endif /* TARGET_PPC64 */
> +
>   #endif /* !CONFIG_USER_ONLY */