Re: [PATCH 4/8] squash target/ppc: Introduce powerpc_excp_40x

From: Fabiano Rosas <farosas@linux.ibm.com>
To: BALATON Zoltan <balaton@eik.bme.hu>
Cc: danielhb413@gmail.com, richard.henderson@linaro.org,
	qemu-devel@nongnu.org, qemu-ppc@nongnu.org, clg@kaod.org,
	david@gibson.dropbear.id.au
Subject: Re: [PATCH 4/8] squash target/ppc: Introduce powerpc_excp_40x
Date: Mon, 10 Jan 2022 16:00:50 -0300	[thread overview]
Message-ID: <875yqra00t.fsf@linux.ibm.com> (raw)
In-Reply-To: <17495cce-79f1-1483-aab3-86ffcde7492e@eik.bme.hu>

BALATON Zoltan <balaton@eik.bme.hu> writes:

> On Mon, 10 Jan 2022, Fabiano Rosas wrote:
>> Introduce a new exception dispatcher for 40x CPUs.
>>
>> Differences from the generic powerpc_excp code:
>>
>> - Not BookE, so some MSR bits are cleared at interrupt dispatch;
>> - No MSR_HV or MSR_LE;
>> - No power saving states;
>> - No Hypervisor Emulation Assistance;
>> - Not 64 bits;
>> - No System call vectored;
>> - No Interrupts Little Endian;
>> - No Alternate Interrupt Location.
>>
>> Exceptions used:
>>
>> POWERPC_EXCP_ALIGN
>> POWERPC_EXCP_APU
>> POWERPC_EXCP_CRITICAL
>> POWERPC_EXCP_DEBUG
>> POWERPC_EXCP_DSI
>> POWERPC_EXCP_DTLB
>> POWERPC_EXCP_EXTERNAL
>> POWERPC_EXCP_FIT
>> POWERPC_EXCP_FPU
>> POWERPC_EXCP_ISI
>> POWERPC_EXCP_ITLB
>> POWERPC_EXCP_MCHECK
>> POWERPC_EXCP_PIT
>> POWERPC_EXCP_PROGRAM
>> POWERPC_EXCP_SYSCALL
>> POWERPC_EXCP_WDT
>>
>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>> ---
>> target/ppc/excp_helper.c | 165 +++------------------------------------
>> 1 file changed, 13 insertions(+), 152 deletions(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 12ab5e1b34..1d997c4d6b 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -409,54 +409,18 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>>                   excp, env->error_code);
>>
>>     /* new srr1 value excluding must-be-zero bits */
>> -    if (excp_model == POWERPC_EXCP_BOOKE) {
>> -        msr = env->msr;
>> -    } else {
>> -        msr = env->msr & ~0x783f0000ULL;
>> -    }
>> +    msr = env->msr & ~0x783f0000ULL;
>>
>>     /*
>> -     * new interrupt handler msr preserves existing HV and ME unless
>> -     * explicitly overriden
>> +     * new interrupt handler msr preserves existing ME unless
>> +     * explicitly overriden.
>>      */
>> -    new_msr = env->msr & (((target_ulong)1 << MSR_ME) | MSR_HVB);
>> +    new_msr = env->msr & (((target_ulong)1 << MSR_ME));
>>
>>     /* target registers */
>>     srr0 = SPR_SRR0;
>>     srr1 = SPR_SRR1;
>>
>> -    /*
>> -     * check for special resume at 0x100 from doze/nap/sleep/winkle on
>> -     * P7/P8/P9
>> -     */
>> -    if (env->resume_as_sreset) {
>> -        excp = powerpc_reset_wakeup(cs, env, excp, &msr);
>> -    }
>> -
>> -    /*
>> -     * Hypervisor emulation assistance interrupt only exists on server
>> -     * arch 2.05 server or later. We also don't want to generate it if
>> -     * we don't have HVB in msr_mask (PAPR mode).
>> -     */
>> -    if (excp == POWERPC_EXCP_HV_EMU
>> -#if defined(TARGET_PPC64)
>> -        && !(mmu_is_64bit(env->mmu_model) && (env->msr_mask & MSR_HVB))
>> -#endif /* defined(TARGET_PPC64) */
>> -
>> -    ) {
>> -        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
>> -
>>     vector = env->excp_vectors[excp];
>>     if (vector == (target_ulong)-1ULL) {
>>         cpu_abort(cs, "Raised an exception without defined vector %d\n",
>> @@ -581,6 +545,11 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>>          */
>>         env->spr[SPR_DSISR] |= (env->error_code & 0x03FF0000) >> 16;
>>         break;
>> +    case POWERPC_EXCP_HV_EMU:
>> +        /*
>> +         * Hypervisor emulation assistance interrupt only exists on server
>> +         * arch 2.05 server or later.
>> +         */
>>     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
>>         switch (env->error_code & ~0xF) {
>>         case POWERPC_EXCP_FP:
>> @@ -645,22 +614,8 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>>             new_msr |= (target_ulong)MSR_HVB;
>>         }
>>         break;
>> -    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);
>> -
>> -        vector += lev * 0x20;
>> -
>> -        env->lr = env->nip;
>> -        env->ctr = msr;
>> -        break;
>>     case POWERPC_EXCP_FPU:       /* Floating-point unavailable exception     */
>>     case POWERPC_EXCP_APU:       /* Auxiliary processor unavailable          */
>> -    case POWERPC_EXCP_DECR:      /* Decrementer exception                    */
>> -        break;
>
> Removing the break here makes FPU and APU fall through to FIT. Is that 
> intentional?

No, that is a mistake indeed. Thanks.