On Fri, Sep 23, 2022 at 2:05 PM Thomas Huth <th...@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 <da...@redhat.com> wrote: > >> > >> On 23.09.22 13:19, Jason A. Donenfeld wrote: > >>> On Fri, Sep 23, 2022 at 12:47 PM David Hildenbrand <da...@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 about. Do you want me to rebase 2/2 on top of v8.1? Jason