Re: [PATCH] target/ppc: powerpc_excp: Guard ALIGNMENT interrupt with CONFIG_TCG

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

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

> Richard,
>
> On 12/9/21 16:05, Fabiano Rosas wrote:
>> Cédric Le Goater <clg@kaod.org> writes:
>> 
>>> On 12/9/21 00:06, Fabiano Rosas wrote:
>>>> We cannot have TCG code in powerpc_excp because the function is called
>>>> from kvm-only code via ppc_cpu_do_interrupt:
>>>>
>>>>    ../target/ppc/excp_helper.c:463:29: error: implicit declaration of
>>>>    function ‘cpu_ldl_code’ [-Werror=implicit-function-declaration]
>>>>
>>>> Fortunately, the Alignment interrupt is not among the ones dispatched
>>>> from kvm-only code, so we can keep it out of the disable-tcg build for
>>>> now.
>>>>
>>>> Fixes: 336e91f853 ("target/ppc: Move SPR_DSISR setting to powerpc_excp")
>>>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>>>>
>>>> ---
>>>>
>>>> Perhaps we could make powerpc_excp TCG only and have a separate
>>>> function that only knows the two interrupts that we use with KVM
>>>> (Program, Machine check). But for now this fix will do, I think.
>>>> ---
>>>>    target/ppc/excp_helper.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>>>> index 17607adbe4..dcf22440cc 100644
>>>> --- a/target/ppc/excp_helper.c
>>>> +++ b/target/ppc/excp_helper.c
>>>> @@ -453,6 +453,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>>>>            }
>>>>            break;
>>>>        }
>>>> +#ifdef CONFIG_TCG
>>>>        case POWERPC_EXCP_ALIGN:     /* Alignment exception                      */
>>>>            /*
>>>>             * Get rS/rD and rA from faulting opcode.
>>>> @@ -464,6 +465,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>>>>                env->spr[SPR_DSISR] |= (insn & 0x03FF0000) >> 16;
>>>>            }
>>>>            break;
>>>> +#endif
>>>>        case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
>>>>            switch (env->error_code & ~0xF) {
>>>>            case POWERPC_EXCP_FP:
>>>>
>>>
>>> Shouldn't we move that code under ppc_cpu_do_unaligned_access ?
>> 
>> Well, it came from there initially. We could revert 336e91f853 and that
>> would fix the issue as well.
>
> What would you prefer ?

Well none of this interfere with the work I'm doing, so it really makes
no difference. I guess reverting the patch is cleaner than having an
ifdef loose in the middle of the code. I'll send a v2 with the revert.

>
> Thanks,
>
> C.