Michael Weiser <mich...@weiser.dinsnail.net> writes:
> Hi Niels,
>
> On Wed, Feb 07, 2018 at 01:13:32PM +0100, Niels Möller wrote:
>
>> Can you check if it's detected correctly also when cross-compiling?
> [...]
> Seems fine.
Good!
>> > FAIL: memxor
>> This also does some tricks with word reads and rotate. (The C code does
>> that too, but with conditions on WORDS_BIGENDIAN).
>
> I think I got memxor, sha1 and sha256 sorted. Patch below.
Nice. Only some quick comments for now.
> This one is still failing, even though memxor and sha are fixed. I've
> been looking at the code and can't find any apparent reason. In
> chacha-core-internal.c I see the following bit of code that does seem to
> do endianness handling:
>
> dst[i] = LE_SWAP32 (t);
>
> Would this apply to chacha-core-internal.asm, too?
That's right, it is expected to produce the output as an array of 16
32-bit words, stored in *little* endian order. So byteswap is needed
before the final
vstm DST, {X0,X1,X2,X3}
instruction. The idea is that the buffer can be used directly with
memxor, without the C code having to do any byte swaps.
>> > FAIL: umac
>> Similar problem, I would guess. But this time, loading 64 bits at a time
>> into neon registers.
>
> I'm drawing a bit of a blank on this one. It fails on the very first
> test case of umac32 where only umac-nh is used and all the input is
> zeroes. So there does seem to be another endianness dependency in the
> actual computation code.
It could be that if the all-zero input is misaligned, the aligned read +
rotation tricks gets non-zero data from outside of the input area into
the computation.
> Have I understood correctly, that vld1.8 reads a byte stream and
> should be endianness-neutral
Don't remember, have to consult the arm docs.
> and the keys are in host endianness?
I think so.
> I was wrong: While the compiler is able to output big-endian objects
> with -mbig-endian, it needs matching libs as well (e.g. libgcc_s).
> Debian doesn't have anything precompiled for armeb.
Maybe full-system qemu is easier then (assuming there's some dist
supporting big-endian arm)?
> Instead I augmented the default action (which is documented and
> shouldn't change) by setting ASM_WORDS_BIGENDIAN directly. Also this
> should make the explicit value checking in IF_BE redundant because we
> now know for sure configure will never emit anything other than yes and
> no. Documentation says that AC_C_BIGENDIAN will abort if endianness
> can't be determined.
[...]
> --- a/configure.ac
> +++ b/configure.ac
> @@ -201,7 +201,9 @@ LSH_FUNC_STRERROR
> # getenv_secure is used for fat overrides,
> # getline is used in the testsuite
> AC_CHECK_FUNCS(secure_getenv getline)
> -AC_C_BIGENDIAN
> +AC_C_BIGENDIAN([AC_DEFINE([WORDS_BIGENDIAN], 1)
> + [ASM_WORDS_BIGENDIAN=yes]],
> + [ASM_WORDS_BIGENDIAN=no])
I think I'd indent differently, to group the two parts ACTION-IF-TRUE
together, and drop the redundant square brackets in
"[ASM_WORDS_BIGENDIAN=yes]".
You leave the ACTION-IS-UNIVERSAL as default, which I think is good. I
hope that's not relevant for arm. Might still be good to set
ASM_WORDS_BIGENDIAN to some default value before this check, and have
IF_BE fail if used with an unknown endianness.
Regards,
/Niels
--
Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677.
Internet email is subject to wholesale government surveillance.
_______________________________________________
nettle-bugs mailing list
nettle-bugs@lists.lysator.liu.se
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
