[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH v2] fix WFI/WFE length in syndrome register
From: |
Peter Maydell |
Subject: |
Re: [Qemu-arm] [PATCH v2] fix WFI/WFE length in syndrome register |
Date: |
Tue, 24 Oct 2017 16:53:29 +0100 |
On 21 October 2017 at 19:09, Stefano Stabellini <address@hidden> wrote:
> WFI/E are often, but not always, 4 bytes long. When they are, we need to
> set ARM_EL_IL_SHIFT in the syndrome register.
>
> Pass the instruction length to HELPER(wfi), use it to decrement pc
> appropriately and to pass an is_16bit flag to syn_wfx, which sets
> ARM_EL_IL_SHIFT if needed.
>
> Signed-off-by: Stefano Stabellini <address@hidden>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index a39b9d3..6f74589 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -11380,17 +11380,20 @@ static void aarch64_tr_tb_stop(DisasContextBase
> *dcbase, CPUState *cpu)
> gen_helper_yield(cpu_env);
> break;
> case DISAS_WFI:
> + {
> + TCGv_i32 tmp = tcg_const_i32((dc->insn & (1U << 31)) ? 4 : 2);
This is the A64 translator, we know that WFI (like every instruction)
is 32 bit.
> /* This is a special case because we don't want to just halt the
> CPU
> * if trying to debug across a WFI.
> */
> gen_a64_set_pc_im(dc->pc);
> - gen_helper_wfi(cpu_env);
> + gen_helper_wfi(cpu_env, tmp);
As Philippe says, you need to free the TCG temp here.
> /* The helper doesn't necessarily throw an exception, but we
> * must go back to the main loop to check for interrupts anyway.
> */
> tcg_gen_exit_tb(0);
> break;
> }
> + }
> }
>
> /* Functions above can change dc->pc, so re-align db->pc_next */
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index 4da1a4c..a89518f 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -12325,12 +12325,15 @@ static void arm_tr_tb_stop(DisasContextBase
> *dcbase, CPUState *cpu)
> /* nothing more to generate */
> break;
> case DISAS_WFI:
> - gen_helper_wfi(cpu_env);
> + {
> + TCGv_i32 tmp = tcg_const_i32((dc->insn & (1U << 31)) ? 4 : 2);
This won't work, because dc->insn is only populated by the translate-a64.c
A64 translator, not by the A32/T32 code. I guess 'principle of least
surprise' suggests we should populate it for Thumb and ARM too.
> + gen_helper_wfi(cpu_env, tmp);
> /* The helper doesn't necessarily throw an exception, but we
> * must go back to the main loop to check for interrupts anyway.
> */
> tcg_gen_exit_tb(0);
> break;
> + }
> case DISAS_WFE:
> gen_helper_wfe(cpu_env);
> break;
thanks
-- PMM