[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC v2 02/12] target/ppc: powerpc_excp: Set vector earlier
From: |
Fabiano Rosas |
Subject: |
Re: [RFC v2 02/12] target/ppc: powerpc_excp: Set vector earlier |
Date: |
Fri, 24 Dec 2021 08:14:27 -0300 |
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.