Re: [PATCH v2] Revert "target/ppc: Move SPR_DSISR setting to powerpc_excp"

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

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

> On 12/9/21 9:33 AM, Fabiano Rosas wrote:
>> This reverts commit 336e91f85332dda0ede4c1d15b87a19a0fb898a2.
>> 
>> It breaks the --disable-tcg build:
>> 
>>   ../target/ppc/excp_helper.c:463:29: error: implicit declaration of
>>   function ‘cpu_ldl_code’ [-Werror=implicit-function-declaration]
>> 
>> We should not have TCG code in powerpc_excp because some kvm-only
>> routines use it indirectly to dispatch interrupts. See
>> kvm_handle_debug, spapr_mce_req_event and
>> spapr_do_system_reset_on_cpu.
>> 
>> We can re-introduce the change once we have split the interrupt
>> injection code between KVM and TCG.
>> 
>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>> ---
>>   target/ppc/excp_helper.c | 21 ++++++++++++---------
>>   1 file changed, 12 insertions(+), 9 deletions(-)
>
> This is fine.  I had thought it would turn out to be helpful in conjunction with my 
> user-only unaligned patch set, but in the end I went a different way and have a separate 
> hook for user-only.
>
> It is correct to simply revert the patch first.
>
> I have ideas for further cleanup, if you have time:
>
> (1) The assignment to DSISR does not need to wait until powerpc_excp.  I believe we can 
> assign to it directly in do_unaligned_access, and avoid using env->error_code as an 
> intermediary.

Makes sense. I see that not all processors use DSISR during the
Alignment interrupt. I'll check the manuals and fix that as well.

I'm writing some tests to check each individual Alignment case and DAR
is not being set for ALIGN_LE. There might be others missing. I'll
probably end up moving the DAR code from ppc_cpu_do_unaligned_access
into powerpc_excp after all.

>
> (2) The note about opcode fields being set incorrectly could be fixed during translation. 
>   You'd use TARGET_INSN_START_EXTRA_WORDS to record the necessary information during 
> translation, is provided to restore_state_to_opc during unwinding, and then moved into 
> DSISR in do_unaligned_access.  Similar to target/arm and how env->exception.syndrome is 
> managed, especially disas_set_insn_syndrome.
>

Ok, I'll give it a shot. Thanks for the detailed pointers.