Re: [PATCH v2] target/s390x: fix execution with icount

[Philippe Mathieu-Daudé]

On 11/2/20 10:26 AM, Pavel Dovgalyuk wrote:
> This patch adds some gen_io_start() calls to allow execution
> of s390x targets in icount mode with -smp 1.
> It enables deterministic timers and record/replay features.
> 
> Signed-off-by: Pavel Dovgalyuk <pavel.dovgalyuk@ispras.ru>
> 
> ---
> 
> v2:
>  - added IF_IO flag to reuse icount code in translate_one()
>    (suggested by Richard Henderson)

Suggested-by: Richard Henderson <richard.henderson@linaro.org>

> ---
>  target/s390x/insn-data.def |   70 
> ++++++++++++++++++++++----------------------
>  target/s390x/translate.c   |   15 +++++++++
>  2 files changed, 50 insertions(+), 35 deletions(-)
...

> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index ac10f42f10..7a8ff1f2de 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -1214,6 +1214,7 @@ typedef struct {
>  #define IF_DFP      0x0010      /* decimal floating point instruction */
>  #define IF_PRIV     0x0020      /* privileged instruction */
>  #define IF_VEC      0x0040      /* vector instruction */
> +#define IF_IO       0x0080      /* input/output instruction */
>  
>  struct DisasInsn {
>      unsigned opc:16;
> @@ -6369,6 +6370,7 @@ static DisasJumpType translate_one(CPUS390XState *env, 
> DisasContext *s)
>      const DisasInsn *insn;
>      DisasJumpType ret = DISAS_NEXT;
>      DisasOps o = {};
> +    bool icount = false;
>  
>      /* Search for the insn in the table.  */
>      insn = extract_insn(env, s);
> @@ -6435,6 +6437,14 @@ static DisasJumpType translate_one(CPUS390XState *env, 
> DisasContext *s)
>                  return DISAS_NORETURN;
>              }
>          }
> +
> +        /* input/output is the special case for icount mode */
> +        if (insn->flags & IF_IO) {
> +            icount = tb_cflags(s->base.tb) & CF_USE_ICOUNT;
> +            if (icount) {
> +                gen_io_start();
> +            }
> +        }
>      }
>  
>      /* Check for insn specification exceptions.  */
> @@ -6488,6 +6498,11 @@ static DisasJumpType translate_one(CPUS390XState *env, 
> DisasContext *s)
>          tcg_temp_free_i64(o.addr1);
>      }
>  
> +    /* io should be the last instruction in tb when icount is enabled */
> +    if (icount && ret == DISAS_NEXT) {
> +        ret = DISAS_PC_STALE;
> +    }
> +
>  #ifndef CONFIG_USER_ONLY
>      if (s->base.tb->flags & FLAG_MASK_PER) {
>          /* An exception might be triggered, save PSW if not already done.  */
> 
> 

A bit too s390x-specific to me, but the generic approach looks great :)