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

[Fabiano Rosas]

Cédric Le Goater <clg@kaod.org> writes:

> 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 :)

Yep, I think is better to fix everything that is broken before the
cleanup so we have more code working and being tested before the
changes.

I would have sent this patch months ago if I knew how to fix it then =)