Re: [Qemu-devel] [PATCH v2 3/3] s390x/tcg: add basic MSA features

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

On 09/19/2017 09:26 AM, David Hildenbrand wrote:
> +    const uint8_t fc = env->regs[0] & 0x7fULL;

Don't mask here...

> +    if (fc >= 128 || !test_be_bit(fc, subfunc)) {
> +        cpu_restore_state(cs, ra);
> +        program_interrupt(env, PGM_SPECIFICATION, 4);
> +        return 0;
> +    }

... because you are indeed supposed to test bit 56.  But you've already ensured
that won't be set above.

Do you in fact need to set bit 63 in subfunc, since that itself is query?
Certainly you won't get past this test, regardless...


> +static ExitStatus op_msa(DisasContext *s, DisasOps *o)
> +{
> +    int r1 = have_field(s->fields, r1) ? get_field(s->fields, r1) : 0;
> +    int r2 = have_field(s->fields, r2) ? get_field(s->fields, r2) : 0;
> +    int r3 = have_field(s->fields, r3) ? get_field(s->fields, r3) : 0;
> +    TCGv_i32 t_r1, t_r2, t_r3, type;
> +
> +    switch (s->insn->data) {
> +    case S390_FEAT_TYPE_KMCTR:
> +        if (r3 & 1 || !r3) {
> +            gen_program_exception(s, PGM_SPECIFICATION);
> +            return EXIT_NORETURN;
> +        }
> +        /* FALL THROUGH */
> +    case S390_FEAT_TYPE_PPNO:
> +    case S390_FEAT_TYPE_KMF:
> +    case S390_FEAT_TYPE_KMC:
> +    case S390_FEAT_TYPE_KMO:
> +    case S390_FEAT_TYPE_KM:
> +        if (r1 & 1 || !r1) {
> +            gen_program_exception(s, PGM_SPECIFICATION);
> +            return EXIT_NORETURN;
> +        }
> +        /* FALL THROUGH */
> +    case S390_FEAT_TYPE_KMAC:
> +    case S390_FEAT_TYPE_KIMD:
> +    case S390_FEAT_TYPE_KLMD:
> +        if (r2 & 1 || !r2) {
> +            gen_program_exception(s, PGM_SPECIFICATION);
> +            return EXIT_NORETURN;
> +        }

Even though it would be silly to do so, considering that r0+r1 are implicit
arguments to these insns, I see nothing that insists that r[123] are not 0,
only that they are even.


r~