Re: [PATCH] target/ppc: Fix e6500 boot

["Cédric Le Goater" <clg@kaod.org>]

On 12/13/21 14:35, Fabiano Rosas wrote:
> When Altivec support was added to the e6500 kernel in 2012[1], the
> QEMU code was not changed, so we don't register the VPU/VPUA
> exceptions for the e6500:
> 
>    qemu: fatal: Raised an exception without defined vector 73
> 
> Note that the error message says 73, instead of 32, which is the IVOR
> for VPU. This is because QEMU knows only knows about the VPU interrupt
> for the 7400s. In theory, we should not be raising _that_ VPU
> interrupt, but instead another one specific for the e6500.
> 
> We unfortunately cannot register e6500-specific VPU/VPUA interrupts
> because the SPEU/EFPDI interrupts also use IVOR32/33. These are
> present only in the e500v1/2 versions. From the user manual:
> 
> e500v1, e500v2: only SPEU/EFPDI/EFPRI
> e500mc, e5500:  no SPEU/EFPDI/EFPRI/VPU/VPUA
> e6500:          only VPU/VPUA
> 
> So I'm leaving IVOR32/33 as SPEU/EFPDI, but altering the dispatch code
> to convert the VPU #73 to a #32 when we're in the e6500. Since the
> handling for SPEU and VPU is the same this is the only change that's
> needed. The EFPDI is not implemented and will cause an abort. I don't
> think it worth it changing the error message to take VPUA into
> consideration, so I'm not changing anything there.
> 
> This bug was discussed in the thread:
> https://lists.gnu.org/archive/html/qemu-ppc/2021-06/msg00222.html
> 
> 1- https://git.kernel.org/torvalds/c/cd66cc2ee52
> 
> Reported-by: <mario@locati.it>
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

One comment,

> ---
>   target/ppc/cpu_init.c    |  6 ++++++
>   target/ppc/excp_helper.c | 12 +++++++++++-
>   2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 6695985e9b..d8efcb24ed 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -2273,8 +2273,14 @@ static void init_excp_e200(CPUPPCState *env, target_ulong ivpr_mask)
>       env->excp_vectors[POWERPC_EXCP_DTLB]     = 0x00000000;
>       env->excp_vectors[POWERPC_EXCP_ITLB]     = 0x00000000;
>       env->excp_vectors[POWERPC_EXCP_DEBUG]    = 0x00000000;
> +    /*
> +     * These two are the same IVOR as POWERPC_EXCP_VPU and
> +     * POWERPC_EXCP_VPUA. We deal with that when dispatching at
> +     * powerpc_excp().
> +     */
>       env->excp_vectors[POWERPC_EXCP_SPEU]     = 0x00000000;
>       env->excp_vectors[POWERPC_EXCP_EFPDI]    = 0x00000000;
> +
>       env->excp_vectors[POWERPC_EXCP_EFPRI]    = 0x00000000;
>       env->ivor_mask = 0x0000FFF7UL;
>       env->ivpr_mask = ivpr_mask;
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 17607adbe4..7bb170f440 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -344,6 +344,16 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>           excp = POWERPC_EXCP_PROGRAM;
>       }
>   
> +#ifdef TARGET_PPC64
> +    /*
> +     * SPEU and VPU share the same IVOR but they exist in different
> +     * processors. SPEU is e500v1/2 only and VPU is e6500 only.
> +     */
> +    if (excp_model == POWERPC_EXCP_BOOKE && excp == POWERPC_EXCP_VPU) {
> +        excp = POWERPC_EXCP_SPEU;
> +    }
> +#endif

I am not in favor of changing powerpc_excp() but I know you have
plans for a major clean up :)

Thanks,

C.


>       switch (excp) {
>       case POWERPC_EXCP_NONE:
>           /* Should never happen */
> @@ -569,7 +579,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>               cpu_abort(cs, "Debug exception triggered on unsupported model\n");
>           }
>           break;
> -    case POWERPC_EXCP_SPEU:      /* SPE/embedded floating-point unavailable  */
> +    case POWERPC_EXCP_SPEU:   /* SPE/embedded floating-point unavailable/VPU  */
>           env->spr[SPR_BOOKE_ESR] = ESR_SPV;
>           break;
>       case POWERPC_EXCP_EFPDI:     /* Embedded floating-point data interrupt   */
>