Re: [qemu-s390x] [PATCH v3 6/9] s390x/tcg: check for AFP-register, BFP and DFP data exceptions

[Thomas Huth]

On 2018-09-17 11:34, David Hildenbrand wrote:
> With the annotated functions, we can now easily check this at a central
> place.
> 
> DXC 1 is to be injected if an AFP register is used (for a HFP instruction)
> when AFP is disabled.
> DXC 2 is to be injected if a BFP instruction is used when AFP is
> disabled.
> DXC 3 is to be injected if a DFP instruction is used when AFP is
> disabled.
> 
> Reviewed-by: Richard Henderson <address@hidden>
> Signed-off-by: David Hildenbrand <address@hidden>
> ---
>  target/s390x/translate.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 8322c81e90..a0c834ebb9 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -6055,6 +6055,11 @@ static const DisasInsn *extract_insn(CPUS390XState 
> *env, DisasContext *s,
>      return info;
>  }
>  
> +static bool is_afp_reg(int reg)
> +{
> +    return reg % 2 || reg > 6;
> +}
> +
>  static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
>  {
>      const DisasInsn *insn;
> @@ -6081,6 +6086,34 @@ static DisasJumpType translate_one(CPUS390XState *env, 
> DisasContext *s)
>      }
>  #endif
>  
> +    /* process flags */
> +    if (insn->flags) {
> +        /* if AFP is not enabled, instructions and registers are forbidden */
> +        if (!(s->base.tb->flags & FLAG_MASK_AFP)) {
> +            uint8_t dxc = 0;
> +
> +            if ((insn->flags & IF_HFP1) && is_afp_reg(get_field(&f, r1))) {
> +                dxc = 1;
> +            }
> +            if ((insn->flags & IF_HFP2) && is_afp_reg(get_field(&f, r2))) {
> +                dxc = 1;
> +            }
> +            if ((insn->flags & IF_HFP3) && is_afp_reg(get_field(&f, r3))) {
> +                dxc = 1;
> +            }
> +            if (insn->flags & IF_BFP) {
> +                dxc = 2;
> +            }
> +            if (insn->flags & IF_DFP) {
> +                dxc = 3;
> +            }
> +            if (dxc) {
> +                gen_data_exception(dxc);
> +                return DISAS_NORETURN;
> +            }
> +        }
> +    }
> +
>      /* Check for insn specification exceptions.  */
>      if (insn->spec) {
>          int spec = insn->spec, excp = 0, r;
> 

I'd prefer IF_AFP[123] again instead of IF_HFP, but apart from that:

Reviewed-by: Thomas Huth <address@hidden>