Re: [PATCH v4 2/2] target/s390x: support SHA-512 extensions

[Harald Freudenberger <freude@linux.ibm.com>]

On 2022-08-03 14:14, Jason A. Donenfeld wrote:
> Hi David,
> 
> On Wed, Aug 03, 2022 at 01:55:21PM +0200, David Hildenbrand wrote:
>> On 02.08.22 21:00, Jason A. Donenfeld wrote:
>> > In order to fully support MSA_EXT_5, we have to also support the SHA-512
>> > special instructions. So implement those.
>> >
>> > The implementation began as something TweetNacl-like, and then was
>> > adjusted to be useful here. It's not very beautiful, but it is quite
>> > short and compact, which is what we're going for.
>> >
>> 
>> Do we have to worry about copyright/authorship of the original code or
>> did you write that from scratch?
> 
> I actually don't really remember how much of that is leftover from
> tweetnacl and how much I've rewritten - I've had some variant of this
> code or another kicking around in various projects and repos for a long
> time. But the tweetnacl stuff is public domain to begin with, so all
> good.
> 
>> Are we properly handling the length register (r2 + 1) in the
>> 24-bit/31-bit addressing mode?
>> Similarly, are we properly handling updates to the message register 
>> (r2)
>> depending on the addressing mode?
> 
> Ugh, probably not... I didn't do any of the deposit_64 stuff. I guess
> I'll look into that.
> 
>> It's worth noting that we might want to implement (also for 
>> PRNO-TRNG):
>> 
>> "The operation is ended when all
>> source bytes in the second operand have been pro-
>> cessed (called normal completion), or when a CPU-
>> determined number of blocks that is less than the
>> length of the second operand have been processed
>> (called partial completion). The CPU-determined
>> number of blocks depends on the model, and may be
>> a different number each time the instruction is exe-
>> cuted. The CPU-determined number of blocks is usu-
>> ally nonzero. In certain unusual situations, this
>> number may be zero, and condition code 3 may be
>> set with no progress."
>> 
>> Otherwise, a large length can make us loop quite a while in QEMU,
>> without the chance to deliver any other interrupts.
> 
> Hmm, okay. Looking at the Linux code, I see:
> 
>         s.even = (unsigned long)src;
>         s.odd  = (unsigned long)src_len;
>         asm volatile(
>                 "       lgr     0,%[fc]\n"
>                 "       lgr     1,%[pba]\n"
>                 "0:     .insn   rre,%[opc] << 16,0,%[src]\n"
>                 "       brc     1,0b\n" /* handle partial completion */
>                 : [src] "+&d" (s.pair)
>                 : [fc] "d" (func), [pba] "d" ((unsigned long)(param)),
>                   [opc] "i" (CPACF_KIMD)
>                 : "cc", "memory", "0", "1");
> 
> So I guess that means it'll just loop until it's done? Or do I need to
> return "1" from HELPER(msa)?
> 
> Jason

Hm, you don't really want to implement some kind of particial complete.
Qemu is an emulation and you would have to implement some kind of
fragmenting this based on machine generation. For my feeling this is
way too overengineered. Btw. as there came the request to handle
the 24-bit/31-bit addressing correctly. Is Qemu 32 bit supported ?