Re: [PATCH v7 2/2] target/s390x: support PRNO_TRNG instruction

[Jason A. Donenfeld]

On Fri, Aug 26, 2022 at 01:28:11PM +0200, Thomas Huth wrote:
> > +        qemu_guest_getrandom_nofail(tmp, block);
> > +        for (size_t i = 0; i < block; ++i) {
> > +            cpu_stb_data_ra(env, wrap_address(env, *buf_reg), tmp[i], ra);
> > +            *buf_reg = deposit64(*buf_reg, 0, message_reg_len, *buf_reg + 
> > 1);
> > +            --*len_reg;
> 
> I know it's annoying, but technically, you must not touch the upper bits of 
> the len_reg if running in 31- or 24-bit addressing mode. The Principles of 
> Operations say:
> 
> "In either the 24- or 31-bit addressing mode, bits 32-63 of the odd-numbered 
> register are decremented by the number
> of bytes processed for the respective operand, and
> bits 0-31 of the register remain unchanged."
> 

This is what I was trying to do with the use of deposit64, following
David's guidance. Did I mess something up?

> > +        }
> > +        len -= block;
> > +    }
> > +}
> > +
> >   uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, uint32_t r2, 
> > uint32_t r3,
> >                        uint32_t type)
> >   {
> 
> Don't you also need to modify the "query" part to signal the availability of 
> the function? Doesn't Linux in the guest check the availability first before 
> using it?

I think this is already handled at the upper layers. Linux detects it
fine.

> 
> > @@ -209,6 +235,10 @@ uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, 
> > uint32_t r2, uint32_t r3,
> >               return klmd_sha512(env, ra, env->regs[1], &env->regs[r2], 
> > &env->regs[r2 + 1]);
> >           }
> >           break;
> > +    case 114: /* CPACF_PRNO_TRNG */
> > +        fill_buf_random(env, ra, &env->regs[r1], &env->regs[r1 + 1]);
> > +        fill_buf_random(env, ra, &env->regs[r2], &env->regs[r2 + 1]);
> > +        break;
> >       default:
> >           /* we don't implement any other subfunction yet */
> >           g_assert_not_reached();
> 
> Maybe one more thing to check (according the "Special Conditions" section in 
> the Principles of Operation):
> 
> "A specification exception is recognized and no other
> action is taken if any of the following conditions exist:
> 
> ...
> 
> 2. The R1 or R2 fields designate an odd-numbered
> register or general register 0. This exception is
> recognized regardless of the function code.
> "

This is taken care of already by the function that calls into this
function.

Jason