Re: [PATCH v1 2/4] target/arm: only update pc after semihosting completes

[Richard Henderson <richard.henderson@linaro.org>]

On 12/18/19 8:00 AM, Alex Bennée wrote:
> Before we introduce blocking semihosting calls we need to ensure we
> can restart the system on semi hosting exception. To be able to do
> this the EXCP_SEMIHOST operation should be idempotent until it finally
> completes. Practically this means ensureing we only update the pc

ensuring.

> after the semihosting call has completed.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  linux-user/aarch64/cpu_loop.c | 1 +
>  linux-user/arm/cpu_loop.c     | 1 +
>  target/arm/helper.c           | 2 ++
>  target/arm/m_helper.c         | 1 +
>  target/arm/translate-a64.c    | 2 +-
>  target/arm/translate.c        | 6 +++---
>  6 files changed, 9 insertions(+), 4 deletions(-)
...
> +++ b/target/arm/m_helper.c
> @@ -2185,6 +2185,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
>                        "...handling as semihosting call 0x%x\n",
>                        env->regs[0]);
>          env->regs[0] = do_arm_semihosting(env);
> +        env->regs[15] += env->thumb ? 2 : 4;

... although thumb should never be false here, it does match the other instances.

I do wonder if it's worth inventing do_arm{32,64}_semihosting wrappers that
consolidate this register manipulation.

But either way,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~