https://bugs.kde.org/show_bug.cgi?id=424298
Mark Wielaard <m...@klomp.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|REPORTED |CONFIRMED
Ever confirmed|0 |1
--- Comment #4 from Mark Wielaard <m...@klomp.org> ---
(In reply to Alexandra Hajkova from comment #3)
> Created attachment 131196 [details]
> patch
>
> This patch implements RDSEED support and also adds the rdseed test to the
> testsuite.
This looks really good, thanks.
The commit message could explain the patch a bit more, which would make review
a bit easier. And please add the bug number in the commit message or the URL to
this bug.
Also, please add a NEWS entry for the bug.
It is interesting how similar rdrand and rdseed are, but they do have separate
cpuid flags, and are apparently using a different way to generate the random
bits. I am not sure the way a 64bit value is created from two calls to rdseed
produces the correct "randomness", but it is exactly how rdrand does it. So
we'll have to assume that was done correctly.
Given that the rdrand and rdseed implementation are so similar I wonder if you
could merge the /* 0F C7 /6 no-F2-or-F3 = RDRAND */ and /* 0F C7 /7 = RDSEED */
parts. RDSEED also has the no-F3-or-F3 part, so basically you could just check:
(gregLO3ofRM(modrm) == 6 && (archinfo->hwcaps & VEX_HWCAPS_AMD64_RDRAND))
|| (gregLO3ofRM(modrm) == 7 && (archinfo->hwcaps &
VEX_HWCAPS_AMD64_RDSEED))
&& epartIsReg(modrm) && haveNoF2noF3(pfx)
&& (sz == 8 || sz == 4 || sz == 2))
And then make the dLO and dHI assignments depend on gregLO3ofRM(modrm) == 6 for
amd64g_dirtyhelper_RDRAND or gregLO3ofRM(modrm) == 7 for
amd64g_dirtyhelper_RDSEED (and same for the DIP). That would remove some
duplicate code.
+/* This is wrong...
assert( !(cmask != 0 && dmask != 0) );
assert( !(cmask == 0 && dmask == 0) );
+*/
It is indeed wrong. But please instead of commenting it out, either remove it
or adjust the asserts to also take the new bmask into account.
But all the above are really just nitpicks, nice work.
--
You are receiving this mail because:
You are watching all bug changes.
