qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v8 23/25] target-arm: introduce ARM_CP_EXIT_PC


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH v8 23/25] target-arm: introduce ARM_CP_EXIT_PC
Date: Tue, 31 Jan 2017 10:57:54 +0000

On 27 January 2017 at 10:39, Alex Bennée <address@hidden> wrote:
> Some helpers may trigger an immediate exit of the cpu_loop. If this
> happens the PC need to be rectified to ensure the restart will begin
> on the next instruction.
>
> Signed-off-by: Alex Bennée <address@hidden>
> ---
>  target/arm/cpu.h           | 3 ++-
>  target/arm/translate-a64.c | 4 ++++
>  target/arm/translate.c     | 4 ++++
>  3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index f56a96c675..1b0670ae11 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -1411,7 +1411,8 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
>  #define ARM_CP_NZCV            (ARM_CP_SPECIAL | (3 << 8))
>  #define ARM_CP_CURRENTEL       (ARM_CP_SPECIAL | (4 << 8))
>  #define ARM_CP_DC_ZVA          (ARM_CP_SPECIAL | (5 << 8))
> -#define ARM_LAST_SPECIAL       ARM_CP_DC_ZVA
> +#define ARM_CP_EXIT_PC         (ARM_CP_SPECIAL | (6 << 8))
> +#define ARM_LAST_SPECIAL       ARM_CP_EXIT_PC

There's a comment above this list of defines that documents
what all the flags mean; can you add an entry to it for the
new flag?

>  /* Used only as a terminator for ARMCPRegInfo lists */
>  #define ARM_CP_SENTINEL 0xffff
>  /* Mask of only the flag bits in a type field */
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 05162f335e..a3f37d8bec 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -1561,6 +1561,10 @@ static void handle_sys(DisasContext *s, uint32_t insn, 
> bool isread,
>          tcg_rt = cpu_reg(s, rt);
>          gen_helper_dc_zva(cpu_env, tcg_rt);
>          return;
> +    case ARM_CP_EXIT_PC:
> +        /* The helper may exit the cpu_loop so ensure PC is correct */
> +        gen_a64_set_pc_im(s->pc);
> +        break;

This will work, but it's a little odd because it breaks the
existing invariant that cp helpers never throw exceptions
(except in the access function).

Does single-stepping (of the emulated architectural
debug step, and gdbstub singlestep) work across one of
these instructions?

Should we also set dc->is_jmp to force ending the TB here?

This is probably a question answered in the rest of the series,
but why do we need the helper to be able to longjump out to the
top level? Can't we just have the helper do its work and then
end the TB with tcg_gen_exit_tb(0) so we return to the top level
loop in the normal way?

>      default:
>          break;
>      }
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index 444a24c2b6..7bd18cd25d 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -7508,6 +7508,10 @@ static int disas_coproc_insn(DisasContext *s, uint32_t 
> insn)
>              gen_set_pc_im(s, s->pc);
>              s->is_jmp = DISAS_WFI;
>              return 0;
> +        case ARM_CP_EXIT_PC:
> +            /* The helper may exit the cpu_loop so ensure PC is correct */
> +            gen_set_pc_im(s, s->pc);
> +            break;

Do we also need to gen_set_condexec() ?

>          default:
>              break;
>          }
> --
> 2.11.0

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]