[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: Thomas Huth
Subject: Re: [PATCH v8.1 1/2] target/s390x: support SHA-512 extensions
Date: Fri, 23 Sep 2022 14:45:19 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.13.0

On 23/09/2022 14.07, Jason A. Donenfeld wrote:
On Fri, Sep 23, 2022 at 2:05 PM Thomas Huth <thuth@redhat.com> wrote:

On 23/09/2022 13.46, Jason A. Donenfeld wrote:
Hi David,

On Fri, Sep 23, 2022 at 1:35 PM David Hildenbrand <david@redhat.com> wrote:

On 23.09.22 13:19, Jason A. Donenfeld wrote:
On Fri, Sep 23, 2022 at 12:47 PM David Hildenbrand <david@redhat.com> wrote:
You must be fortunate if "one afternoon" is not a significant time
investment. For me it is a significant investment.

For me too, to say the least of the multiple afternoons I've spent on
this patch set. Getting back to technical content:

and sort out the remaining issues. I thought we (Thomas included) had an
agreement that that's the way we are going to do it. Apparently I was wrong.
Most probably I lived in the kernel space too long such that we don't
rush something upstream. For that reason *I* sent out a patch with
Here I am, getting told by Thomas that we now do it differently now.
What I really tried to express here is: if Thomas wants to commit things
differently now, maybe he can just separate the cleanup parts. I really
*don't want* to send out a multi-patch series to cleanup individual
parts -- that takes significantly more time. Especially not if something
is not committed yet.

To recap what's been fixed in your v8.1, versus what's been refactored
out of style preference:

1) It handles the machine versioning.
2) It throws an exception on length alignment in KIMD mode:
+    /* KIMD: length has to be properly aligned. */
+    if (type == S390_FEAT_TYPE_KIMD && !QEMU_IS_ALIGNED(len, 128)) {
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+    }
3) It asserts if type is neither KIMD vs KLMD, with:
+    g_assert(type == S390_FEAT_TYPE_KIMD || type == S390_FEAT_TYPE_KLMD);

One important part is

4) No memory modifications before all inputs were read

Ahhh, which v8's klmd doesn't do, since it updates the parameter block
before doing the last block. Is this a requirement of the spec? If
not, then it doesn't matter. But if so, v8's approach is truly
hopeless, and we'd be remiss to not go with your refactoring that
accomplishes this.

Ok, great, if you're fine with the rework, I'll go with David's v8.1
instead. (keeping an entry on my TODO list to rework that ugly generic "msa"
helper function one day - this really kept me being confused for much of my
patch review time)

Okay, sure. Can one of you just look to see if that g_assert() is
going to be a DoS vector, though, or if it'll never be reached if the
prior code goes well? That's the one remaining thing I'm not sure

Do you want me to rebase 2/2 on top of v8.1?

Thanks, but I think it's a trivial contextual conflict only - I can fix that up when picking up the patches.


reply via email to

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