Re: [PATCH v3] target/ppc: Machine check on invalid real address access on POWER9/10

[Daniel Henrique Barboza <danielhb413@gmail.com>]

Queued in gitlab.com/danielhb/qemu/tree/ppc-next. Thanks,


Daniel

On 7/3/23 09:03, Nicholas Piggin wrote:
> ppc currently silently accepts invalid real address access. Catch
> these and turn them into machine checks on POWER9/10 machines.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> Since v1:
> - Only implement this for POWER9/10. Seems like previous IBM processors
>    may not catch this, trying to get info.
> 
> Since v2:
> - Split out from larger series since it is independent.
> 
>   target/ppc/cpu_init.c    |  1 +
>   target/ppc/excp_helper.c | 49 ++++++++++++++++++++++++++++++++++++++++
>   target/ppc/internal.h    |  5 ++++
>   3 files changed, 55 insertions(+)
> 
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 720aad9e05..6ac1765a8d 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -7335,6 +7335,7 @@ static const struct TCGCPUOps ppc_tcg_ops = {
>     .cpu_exec_enter = ppc_cpu_exec_enter,
>     .cpu_exec_exit = ppc_cpu_exec_exit,
>     .do_unaligned_access = ppc_cpu_do_unaligned_access,
> +  .do_transaction_failed = ppc_cpu_do_transaction_failed,
>   #endif /* !CONFIG_USER_ONLY */
>   };
>   #endif /* CONFIG_TCG */
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 354392668e..e49e13a30d 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1428,7 +1428,9 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>           /* machine check exceptions don't have ME set */
>           new_msr &= ~((target_ulong)1 << MSR_ME);
>   
> +        msr |= env->error_code;
>           break;
> +
>       case POWERPC_EXCP_DSI:       /* Data storage exception                   */
>           trace_ppc_excp_dsi(env->spr[SPR_DSISR], env->spr[SPR_DAR]);
>           break;
> @@ -3184,5 +3186,52 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
>       env->error_code = insn & 0x03FF0000;
>       cpu_loop_exit(cs);
>   }
> +
> +void ppc_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
> +                                   vaddr vaddr, unsigned size,
> +                                   MMUAccessType access_type,
> +                                   int mmu_idx, MemTxAttrs attrs,
> +                                   MemTxResult response, uintptr_t retaddr)
> +{
> +    CPUPPCState *env = cs->env_ptr;
> +
> +    switch (env->excp_model) {
> +#if defined(TARGET_PPC64)
> +    case POWERPC_EXCP_POWER9:
> +    case POWERPC_EXCP_POWER10:
> +        /*
> +         * Machine check codes can be found in processor User Manual or
> +         * Linux or skiboot source.
> +         */
> +        if (access_type == MMU_DATA_LOAD) {
> +            env->spr[SPR_DAR] = vaddr;
> +            env->spr[SPR_DSISR] = PPC_BIT(57);
> +            env->error_code = PPC_BIT(42);
> +
> +        } else if (access_type == MMU_DATA_STORE) {
> +            /*
> +             * MCE for stores in POWER is asynchronous so hardware does
> +             * not set DAR, but QEMU can do better.
> +             */
> +            env->spr[SPR_DAR] = vaddr;
> +            env->error_code = PPC_BIT(36) | PPC_BIT(43) | PPC_BIT(45);
> +            env->error_code |= PPC_BIT(42);
> +
> +        } else { /* Fetch */
> +            env->error_code = PPC_BIT(36) | PPC_BIT(44) | PPC_BIT(45);
> +        }
> +        break;
> +#endif
> +    default:
> +        /*
> +         * TODO: Check behaviour for other CPUs, for now do nothing.
> +         * Could add a basic MCE even if real hardware ignores.
> +         */
> +        return;
> +    }
> +
> +    cs->exception_index = POWERPC_EXCP_MCHECK;
> +    cpu_loop_exit_restore(cs, retaddr);
> +}
>   #endif /* CONFIG_TCG */
>   #endif /* !CONFIG_USER_ONLY */
> diff --git a/target/ppc/internal.h b/target/ppc/internal.h
> index 901bae6d39..57acb3212c 100644
> --- a/target/ppc/internal.h
> +++ b/target/ppc/internal.h
> @@ -296,6 +296,11 @@ bool ppc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>   G_NORETURN void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
>                                               MMUAccessType access_type, int mmu_idx,
>                                               uintptr_t retaddr);
> +void ppc_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
> +                                   vaddr addr, unsigned size,
> +                                   MMUAccessType access_type,
> +                                   int mmu_idx, MemTxAttrs attrs,
> +                                   MemTxResult response, uintptr_t retaddr);
>   #endif
>   
>   FIELD(GER_MSK, XMSK, 0, 4)