Re: [Qemu-devel] [PATCH v2 2/8] target/s390x: Implement CONVERT UNICODE insns

[Aurelien Jarno]

On 2017-07-01 13:25, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/s390x/helper.h      |   6 +
>  target/s390x/mem_helper.c  | 310 
> +++++++++++++++++++++++++++++++++++++++++++++
>  target/s390x/translate.c   |  44 +++++++
>  target/s390x/insn-data.def |  13 ++
>  4 files changed, 373 insertions(+)
> 

...

> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index e739525..9301daa 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -2122,6 +2122,49 @@ static ExitStatus op_ct(DisasContext *s, DisasOps *o)
>      return NO_EXIT;
>  }
>  
> +static ExitStatus op_cuXX(DisasContext *s, DisasOps *o)
> +{
> +    int m3 = get_field(s->fields, m3);
> +    TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
> +    TCGv_i32 r2 = tcg_const_i32(get_field(s->fields, r2));
> +    TCGv_i32 chk;
> +
> +    if (!s390_has_feat(s->insn->fac == S390_FEAT_EXTENDED_TRANSLATION_3
> +                       ? S390_FEAT_ETF3_ENH : S390_FEAT_ETF2_ENH)) {
> +        m3 = 0;
> +    }

This doesn't look correct to me. The well-formedness checking is part of
ETF3_ENH facility, for both convert unicode instructions that are part
of the Z architecture (CU12 and CU21) and for the ones added by the ETF3
facility (CU14 and CU24).

The rest of the patch now looks fine.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
address@hidden                 http://www.aurel32.net