Re: [PATCH] target/ppc: restore powerpc_excp_booke doorbell interrupts

[Fabiano Rosas <farosas@linux.ibm.com>]

Nicholas Piggin <npiggin@gmail.com> writes:

> This partially reverts commit 9dc20cc37db9 ("target/ppc: Simplify
> powerpc_excp_booke"), which removed DOORI and DOORCI interrupts.
> Without this patch, a -cpu e5500 -smp 2 machine booting Linux
> crashes with:
>
>   qemu: fatal: Invalid PowerPC exception 36. Aborting
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  target/ppc/excp_helper.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 214acf5ac4..43f2480e94 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1247,6 +1247,12 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
>      case POWERPC_EXCP_SPEU:   /* SPE/embedded floating-point unavailable/VPU  */
>          env->spr[SPR_BOOKE_ESR] = ESR_SPV;
>          break;
> +    case POWERPC_EXCP_DOORI:     /* Embedded doorbell interrupt              */
> +        break;
> +    case POWERPC_EXCP_DOORCI:    /* Embedded doorbell critical interrupt     */
> +        srr0 = SPR_BOOKE_CSRR0;
> +        srr1 = SPR_BOOKE_CSRR1;
> +        break;
>      case POWERPC_EXCP_RESET:     /* System reset exception                   */
>          if (FIELD_EX64(env->msr, MSR, POW)) {
>              cpu_abort(cs, "Trying to deliver power-saving system
> reset "

Ah, booke always trips me up.

Since BookE has exception handler locations defined at runtime, the
env->excp_vectors array doesn't really need to be initialized with valid
exceptions/addresses (that is done on demand at
translate.c:spr_write_excp_vector). So, unlike the other ppc variants,
the init_excp_BookE/e200 code is not an exhaustive list of the
exceptions that might exist.

Note that with this patch the doorbells behave differently if we try to
dispatch the exception before the OS has touched the IVORs. For all
other exceptions we dispatch at 0x0 + prefix while for the doorbells
we'd bail here:

    vector = env->excp_vectors[excp];
    if (vector == (target_ulong)-1ULL) {
        cpu_abort(cs, "Raised an exception without defined vector %d\n",
                  excp);
    }

We might want to consider adding DOORI/DOORCI to init_excp_BookE. Or
alternatively remove all of the 0x0 initialization from that
function.

The fix is good anyway:

Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>