[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v8.1 1/2] target/s390x: support SHA-512 extensions

From: David Hildenbrand
Subject: Re: [PATCH v8.1 1/2] target/s390x: support SHA-512 extensions
Date: Fri, 23 Sep 2022 08:37:50 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.1

On 23.09.22 08:23, Thomas Huth wrote:
On 22/09/2022 19.18, David Hildenbrand wrote:
On 22.09.22 17:55, Thomas Huth wrote:
On 22/09/2022 17.38, David Hildenbrand wrote:
From: "Jason A. Donenfeld" <Jason@zx2c4.com>

In order to fully support MSA_EXT_5, we have to 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.
@@ -52,6 +278,9 @@ uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1,
uint32_t r2, uint32_t r3,
                cpu_stb_data_ra(env, param_addr, subfunc[i], ra);
+    case 3: /* CPACF_*_SHA_512 */
+        return cpacf_sha512(env, ra, env->regs[1], &env->regs[r2],
+                            &env->regs[r2 + 1], type);

I have to say that I liked Jason's v8 better here. Code 3 is also used for
other instructions with completely different meaning, e.g. PCKMO uses 3 for
TDEA-192 ... so having the "type" check here made more sense.
(meta comment: maybe we should split up the msa function and stop using just
one function for all crypto/rng related instructions? ... but that's of
course something for a different patch series)

It kind-f made sense in v8 where we actually had different functions. We no
longer have that here.

But the point is that the "msa" helper is also called for other instructions
like PCKMO which can also be called with code 3. And there it means
something completely different. ... unless I completely misunderstood the
code, of course.

test_be_bit() fences everything off we don't support. Simply falling through here and returning 0 at the end doesn't make any sense either.

I think I'll go with Jason's v8 (+ compat machine handling on top) for now,
so that we better record his state of the patch, and then we can do cleanup
patches on top later.

Feel free to commit what you want (I'll be happy to see Jason's work upstream for good), but note that

1) I don't feel confident to give the original patch my ack/rb
2) I am not a supporter of committing code that has known issues
3) I won't follow up with additional cleanup patches because I already spent more time on this than I originally planned.


David / dhildenb

reply via email to

[Prev in Thread] Current Thread [Next in Thread]