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

[Richard Henderson]

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.

(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.


r~