Re: [PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv

["Nicholas Piggin" <npiggin@gmail.com>]

On Fri Jun 16, 2023 at 9:03 AM AEST, BALATON Zoltan wrote:
> From: Nicholas Piggin <npiggin@gmail.com>
>
> Unlike sc, for scv a facility unavailable interrupt must be generated
> if FSCR[SCV]=0 so we can't raise the exception with nip set to next
> instruction but we can move advancing nip if the FSCR check passes to
> helper_scv so the exception handler does not need to change it.
>
> [balaton: added commit message]

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Thanks, sorry for the delay :( Would you be able to resend the series?
You could drop the machine check one for now perhaps until we sort out
what to do with it.

Thanks,
Nick

> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> This needs SoB from Nick
>
>  target/ppc/excp_helper.c | 2 +-
>  target/ppc/translate.c   | 6 +++++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 903216c2a6..ef363b0285 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1304,7 +1304,6 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>      case POWERPC_EXCP_SYSCALL_VECTORED: /* scv exception                     */
>          lev = env->error_code;
>          dump_syscall(env);
> -        env->nip += 4;
>          new_msr |= env->msr & ((target_ulong)1 << MSR_EE);
>          new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
>  
> @@ -2410,6 +2409,7 @@ void helper_ppc_maybe_interrupt(CPUPPCState *env)
>  void helper_scv(CPUPPCState *env, uint32_t lev)
>  {
>      if (env->spr[SPR_FSCR] & (1ull << FSCR_SCV)) {
> +        env->nip += 4;
>          raise_exception_err(env, POWERPC_EXCP_SYSCALL_VECTORED, lev);
>      } else {
>          raise_exception_err(env, POWERPC_EXCP_FU, FSCR_IC_SCV);
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 4260d3d66f..0360a17fb3 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4433,7 +4433,11 @@ static void gen_scv(DisasContext *ctx)
>  {
>      uint32_t lev = (ctx->opcode >> 5) & 0x7F;
>  
> -    /* Set the PC back to the faulting instruction. */
> +    /*
> +     * Set the PC back to the scv instruction (unlike sc), because a facility
> +     * unavailable interrupt must be generated if FSCR[SCV]=0. The helper
> +     * advances nip if the FSCR check passes.
> +     */
>      gen_update_nip(ctx, ctx->cia);
>      gen_helper_scv(cpu_env, tcg_constant_i32(lev));
>