Re: [RFC v2 02/12] target/ppc: powerpc_excp: Set vector earlier

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

Richard Henderson <richard.henderson@linaro.org> writes:

> On 12/20/21 10:18 AM, Fabiano Rosas wrote:
>> None of the interrupt setup code touches 'vector', so we can move it
>> earlier in the function. This will allow us to later move the System
>> Call Vectored setup that is on the top level into the
>> POWERPC_EXCP_SYSCALL_VECTORED code block.
>> 
>> This patch also moves the verification for when 'excp' does not have
>> an address associated with it. We now bail a little earlier when that
>> is the case. This should not cause any visible effects.
>> 
>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>> ---
>>   target/ppc/excp_helper.c | 16 ++++++++--------
>>   1 file changed, 8 insertions(+), 8 deletions(-)
>> 
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 8b9c6bc5a8..14fd0213a0 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -352,6 +352,14 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>>       }
>>   #endif
>>   
>> +    vector = env->excp_vectors[excp];
>> +    if (vector == (target_ulong)-1ULL) {
>> +        cpu_abort(cs, "Raised an exception without defined vector %d\n",
>> +                  excp);
>> +    }
>> +
>> +    vector |= env->excp_prefix;
>> +
>>       switch (excp) {
>>       case POWERPC_EXCP_NONE:
>>           /* Should never happen */
>
> You've moved the cpu_abort above the excp check above the early return for NONE (which 
> possibly shouldn't exist) and above the excp default

Right, I think I had this patch initially after patch 7 which moves the
NONE check ealier in the function. I'll reorganize.

>          cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
>
> I would certainly expect invalid excp to not have a defined vector
> either.

One thing we always had that this series makes more explicit is the
distinction between what interrupts QEMU knows about and what vectors a
processor uses. I have played around with using the same data structure
to hold both, which would perhaps match the expectation more closely,
but nothing came out of it. I think there's still space after this
series for further improvement in that regard.

> I'll also note that the excp_vectors[] index is no longer bounds checked by the switch.

Ack.