add `powerpc` target #814

flier · 2019-04-30T03:55:59Z

10: 0x10307a6b7 - build_script_build::build_c_code::hd9ffeb4799056596
at /Users/fl0120/github/ring/build.rs:372
11: 0x10307996d - build_script_build::ring_build_rs_main::h5507f897a7458b6f
at /Users/fl0120/github/ring/build.rs:294
12: 0x10307910e - build_script_build::main::h912ac4a7682e0b5c
at /Users/fl0120/github/ring/build.rs:256

When cross compile ring for power target which not in ASM_TARGETS, the build.rs will crash on unwrap()

RustCrypto/hashes#82

briansmith · 2019-04-30T04:10:37Z

I intentionally don't support targets that are currently not supported. In the case of Power, we need to add the proper support to ring (from OpenSSL upstream) to get Power builds working.

flier · 2019-04-30T06:48:05Z

I'm not fan of powerpc, I just need a working build for testing.

From my perspective, the assembly code is just a performance optimizing that should be able to degenerate to a pure software implementation, right?

Anyway, we could try to migrate those code form the upstream OpenSSL.

q66 · 2019-05-01T12:34:56Z

I will test this as soon as I can.

q66 · 2019-05-01T13:48:02Z

I took a rough look at it. There are at least two issues:

build.rs does not seem to properly pass endianness information into the perl scripts as a part of the flavor, which means linux64le will never match
it seems to determine whether to use ELFv2 ABI based on if the target architecture is little endian, which is wrong, as ELFv2 ABI can also be used on big endian (it's just not used by default with glibc, but e.g. it is with musl libc) - this is a problem with the upstream perl script ppc-xlate.pl and should be modified

The correct way to determine ELFv2 ABI in this case is from your compiler, like this

$ gcc -dM -E - < /dev/null|grep CALL_ELF
#define _CALL_ELF 2

q66 · 2019-05-01T13:59:14Z

I raised the problem in openssl upstream: openssl/openssl#8858

briansmith · 2019-05-02T06:58:05Z

2. it seems to determine whether to use ELFv2 ABI based on if the target architecture is little endian, which is wrong, as ELFv2 ABI can also be used on big endian (it's just not used by default with glibc, but e.g. it is with musl libc) - this is a problem with the upstream perl script ppc-xlate.pl and should be modified

I suggest that we try to take an approach that doesn't require modifying the OpenSSL code. If OpenSSL doesn't support a particular configuration, that's a pretty good indication that that configuration isn't urgent to support, right?

q66 · 2019-05-02T09:49:46Z

It is when you don't have a fallback, in OpenSSL you can just disable asm and carry on. It's definitely wrong, I don't see a single reason to carry over incorrect code and break perfectly valid configurations, especially with as much of a pain in the ass as it is to patch rust dependencies downstream, and especially when all it takes is modifying some Perl code.

awilfox · 2019-05-02T13:15:23Z

If OpenSSL doesn't support a particular configuration, that's a pretty good indication that that configuration isn't urgent to support, right?

Very wrong, for two reasons. One: All remaining, maintained, cared-for PowerPC distros are targeting ELFv2. The fact that OpenSSL doesn't support it yet is because the C fallbacks are performant enough that none of us really care about fixing the asm code at this point. Two: Since OpenSSL does support this configuration with disable-asm, that means it is supported.

q66 · 2019-05-02T13:23:38Z

The asm code should be okay by itself, but the perl preprocessor needs to be changed to also use ELFv2 on big endian when requested. The least invasive way would be something like, coming up with a new flavor linux64elfv2 and then matching like, /linux.*(32|64le|64elfv2)/ instead of /linux.*(32|64le)/ and so on, this should not break any of the existing checks for little endian vs big endian instructions, as it matches 64le for little endian.

briansmith · 2019-05-02T19:55:47Z

First, it would be useful if people could list ALL the POWER/PowerPC target triples, including the operating system and specific POWER or PowerPC chips that they are interested in, including endianness in particular. In the case of Linux, it would be useful to know which distros you are targetting.

briansmith · 2019-05-02T20:06:20Z

Also, BoringSSL has a policy of only supporting Little Endian mode. I'm not sure why they have that policy but I do wonder why, if they can get away with that, why we can't too?

q66 · 2019-05-02T20:06:32Z

For Linux, these are relevant these days:

powerpc64le-linux-gnu (elfv2, little endian elfv1 technically existed but was never widespread and is not worth supporting, all current little endian distros are power8+ and elfv2)
powerpc64-linux-gnu (elfv2 and elfv1, elfv1 for legacy distributions like debian, elfv2 for modern distributions)
powerpc64le-linux-musl (elfv2)
powerpc64-linux-musl (elfv2 only, musl libc never had elfv1 support)
powerpc-linux-gnu (single abi)
powerpc-linux-musl (same abi)

additionally powerpc64-freebsd and powerpc-freebsd, these can use either elfv1 or elfv2 for ppc64 and are big endian (32-bit ppc is single ABI), AFAIK elfv1 is default and elfv2 is what they're going to migrate to once they update their toolchain; if little endian support comes to freebsd sometime, it will 100% be ELFv2

The assembly supports 32-bit ppc as well as ppc64, both ELFv1 and ELFv2. The glibc vs musl and linux vs freebsd distinction should not matter if the ABI version is handled correctly. Basically all that is needed to make sure is

ELFv2 logic is taken with little endian 64-bit
ELFv2 logic is conditionally taken with big endian 64-bit
ELFv1 logic is taken with big endian 64-bit for legacy systems, as distros like Debian are unlikely to migrate off ELFv1 anytime soon
32-bit ppc stuff stays as it is

As for specific distributions it should not really matter. My distribution where I maintain the ppc64 port (Void Linux) supports both LE and BE, with ELFv2, we have a 32-bit port as well, 32-bit ppc is always BE. Adélie Linux (@awilfox) supports BE on ppc64 and also supports ppc32. Most major Linux distros are little endian, some (Debian) are little endian ELFv2 and big endian ELFv1, and distros like Gentoo let you choose your endianness and ABI.

q66 · 2019-05-02T20:07:35Z

boringssl rejects non-LE because it's google and they have their political reasons; don't be as bad as google

briansmith · 2019-05-02T20:15:50Z

@q66 Which specific chips are you using? Version and vendor? e.g. IBM POWER9? Are you planning to test this PR on all the targets you mentioned?

awilfox · 2019-05-02T20:15:52Z

We (@AdelieLinux) specifically target powerpc64-linux-musl and powerpc-linux-musl; the 32-bit and 64-bit big endian musl ports.

We test on many CPUs; currently in our test matrix:

Motorola 750cx (32-bit, Apple "G3")
IBM 750 (32-bit, "Broadway" / Wii)
Motorola 7450 (32-bit, Apple "G4")
IBM 970 (like POWER4+; 64-bit, Apple "G5")
IBM POWER5 (64-bit, IBM blade server)
IBM POWER9 (64-bit, Raptor "Talos II")

q66 · 2019-05-02T20:17:53Z

I can test on all 64-bit ELFv2 targets (glibc/musl, little endian and big endian) as that's what my distro handles, I'm not handing the 32-bit port and I don't think we have Rust on it at all yet.

I'm testing on POWER8 and POWER9. It should be the same for both. (our second maintainer can also test on older powerpc G5 on big endian, should not matter much)

awilfox · 2019-05-02T20:20:11Z

We would be willing to test the powerpc64-linux-musl target. There is no way for us to test 32-bit PPC right now. Due to LLVM issues we can't build Rust for 32-bit PowerPC yet. (This is specific to musl; it works fine on glibc.)

sayrer · 2019-05-03T12:52:40Z

It seems like the most transparent thing to do would be to donate test PPC hardware.

awilfox · 2019-05-03T13:52:43Z

Is @briansmith even accepting donations? Alternatively, there are many different resources for getting access to PowerPC hardware for little or no cost for open source projects. Then Brian wouldn't have to fill his house with hardware.

sayrer · 2019-05-05T04:45:04Z

I don't think assuming I want to fill someone's house with PowerPC hardware is a very generous interpretation of my comment.

Maybe it would be more productive to determine a good protocol for releases. Most platforms find their way into an automated test harness run as part of a release and review process. That seems like the best way to support PowerPC.

q66 · 2019-05-05T22:36:01Z

By the way, the OpenSSL assembly code works perfectly fine with ELFv2 big endian. All that's needed is this simple change: dot-asm/cryptogams@50e1ff6 followed by making it actually use the linux64v2 scheme for ELFv2. The whole test suite passes with no issues.

q66 · 2019-05-05T23:38:42Z

Anyway, trying to find the best way to test the C compiler for which perlasm format to use. I need to do a test like this:

#if defined(_CALL_ELF) && (_CALL_ELF == 2)
#  if defined(__LITTLE_ENDIAN__)
#    define PERLASM_FORMAT linux64le
#  else
#    define PERLASM_FORMAT linux64v2
#  endif
#else
#  define PERLASM_FORMAT linux64
#endif

The value of PERLASM_FORMAT is then the format to use. For 32-bit ppc, one can always use linux32. There is no support for FreeBSD in the asm, so that one can be skipped for now and someone can contribute it separately if necessary.

Any ideas?

(also, @briansmith, did you ever mention why not have support for platform independent fallbacks, so that different OSes can actually use this without having to add separate support for each?)

q66 · 2019-05-07T08:48:33Z

There is no powerpc64le in Rust, there is just powerpc64 and endianness is checked separately (with the exception of the compiler target triple, but that's not what's being checked here)

q66 · 2019-05-07T11:36:07Z

There is no support for ppc OS X in the code, so you can remove those. Also, it'll be necessary to integrate the linux64 vs linux64v2 logic somehow... for that it'll be needed to query the C compiler, as Rust doesn't yet have a way to check the ABI version.

q66 · 2019-05-07T12:32:05Z

I have a potential solution for the checks. The cc.rs crate has an .expand() function on cc::Build. Therefore, we could simply reduce the powerpc64 lines in ASM_TARGETS to one, like

("powerpc64", Some("linux"), "linux64"),

Then we could create a special .c file with contents like this:

#if defined(_CALL_ELF) && (_CALL_ELF == 2)
#  if defined(__LITTLE_ENDIAN__)
linux64le
#  else
linux64v2
#  endif
#else
linux64
#endif

Afterwards, in pregenerate_asm_main, when iterating ASM_TARGETS, we could do something like

let real_perlasm_format = if target_arch == "powerpc64" {
    let pp_vec = cc::Build::new().file("path_to_ppc64_test_file.c").expand();
    let pp_str = String::from_utf8(pp_vec).unwrap();
    let pp_arr: Vec<&str> = pp_str.trim().rsplit('\n').collect();
    pp_arr[0]
} else
    perlasm_format
};

or something like that anyway. Not exactly like that, as I don't know Rust too well and and that check would probably be best moved out of the loop, but something along those lines.

q66 · 2019-05-07T12:42:52Z

One could also apparently use tempfile() rather than pre-creating the test file, for safe cleanup and having everything in the same location.

q66 · 2019-05-08T03:10:04Z

Need to add

        #[cfg(target_arch = "powerpc64")]
        const SYS_GETRANDOM: libc::c_long = 359;

in src/rand.rs. Probably one for powerpc as well, but with a different value. Then I got it to compile, except I'm getting lots of stuff like

          /bin/ld: /home/q66/ring-test/target/debug/deps/libring-3f4e5ceee5ae030a.rlib(ecp_nistz256.o): in function `GFp_nistz256_point_mul':
          /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:110: undefined reference to `GFp_nistz256_point_double'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:111: undefined reference to `GFp_nistz256_point_add'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:112: undefined reference to `GFp_nistz256_point_double'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:113: undefined reference to `GFp_nistz256_point_double'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:114: undefined reference to `GFp_nistz256_point_double'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:115: undefined reference to `GFp_nistz256_point_double'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:116: undefined reference to `GFp_nistz256_point_add'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:117: undefined reference to `GFp_nistz256_point_add'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:118: undefined reference to `GFp_nistz256_point_add'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:119: undefined reference to `GFp_nistz256_point_add'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:120: undefined reference to `GFp_nistz256_point_double'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:121: undefined reference to `GFp_nistz256_point_double'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:122: undefined reference to `GFp_nistz256_point_add'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:123: undefined reference to `GFp_nistz256_point_add'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:124: undefined reference to `GFp_nistz256_point_double'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:151: undefined reference to `GFp_nistz256_neg'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:154: undefined reference to `GFp_nistz256_point_add'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:159: undefined reference to `GFp_nistz256_point_double'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:160: undefined reference to `GFp_nistz256_point_double'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:161: undefined reference to `GFp_nistz256_point_double'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:162: undefined reference to `GFp_nistz256_point_double'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:163: undefined reference to `GFp_nistz256_point_double'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:172: undefined reference to `GFp_nistz256_neg'
          /bin/ld: /home/q66/ring-test/crypto/fipsmodule/ec/ecp_nistz256.c:174: undefined reference to `GFp_nistz256_point_add'

when i try to cargo test it. It seems that a bunch of asm is not imported, and also the functions seem to have renamed prefixes.

pietro · 2019-05-08T15:29:58Z

The function prefix problem is still there, like undefined reference to GFp_nistz256_point_double'when in the asm it'secp_nistz256_point_double`, there's lots of instances like this

The functions were prefixed with GFp_ in ring to avoid conflict with other libraries. For example:

ring/crypto/fipsmodule/ec/ecp_nistz256.c

Line 78 in 717d0c2

void GFp_nistz256_point_double(P256_POINT *r, const P256_POINT *a);

ring/crypto/fipsmodule/ec/asm/ecp_nistz256-armv8.pl

Lines 551 to 554 in 9814ac4

    
           .globl	GFp_nistz256_point_double 
        
           .type	GFp_nistz256_point_double,%function 
        
           .align	5 
        
           GFp_nistz256_point_double:

q66 · 2019-05-08T18:50:06Z

I did my own attempt and got a bit further than this pull request, after one workaround I can make the entire testsuite pass on at least ppc64le: #819

q66 · 2019-05-09T00:33:56Z

I think this can probably be closed at this point. #819 is a lot more complete and is currently functional, passing the testsuite OOTB.

flier · 2019-05-09T02:18:50Z

sure, thanks for your help

q66 · 2019-05-09T02:37:04Z

Yeah, no problem. I figured it would be faster this way, as I run ppc64 as my primary environment and therefore have machines to test on :)

skip asm_srcs when target not in ASM_TARGETS

f8e5c3c

migrate PPC related asm from OpenSSL

bedf0b0

flier added 2 commits April 30, 2019 17:37

fix CI build script

60015bd

add missed files

9f2dba5

flier changed the title ~~skip asm_srcs when target not in ASM_TARGETS~~ add powerpc target Apr 30, 2019

briansmith mentioned this pull request May 1, 2019

Support Power/PowerPC #389

Closed

flier added 2 commits May 7, 2019 09:46

add linux64v2 flavour

4bf96a9

add ppc-xlate.pl to build.rs tracking

932afd9

flier added 2 commits May 7, 2019 12:22

add powerpc64le target

5f9a951

check powerpc/powerpc64

1e17068

flier added 2 commits May 7, 2019 19:03

fix compile issue for powerpc

29db83e

fix typo issue

e73a24a

flier added 2 commits May 8, 2019 10:56

detect perlasm_format w/ C preprocessor for ppc64

f0d211b

force to use u32::to_be to convert {integer}

91853bc

flier added 4 commits May 8, 2019 14:31

add SYS_GETRANDOM syscal for powerpc

0d83b24

add missed asm code

2aa654d

fix perlasm path

010da38

add missed file

a41bcac

flier closed this May 9, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add `powerpc` target #814

add `powerpc` target #814

flier commented Apr 30, 2019

briansmith commented Apr 30, 2019

flier commented Apr 30, 2019

q66 commented May 1, 2019

q66 commented May 1, 2019

q66 commented May 1, 2019

briansmith commented May 2, 2019

q66 commented May 2, 2019

awilfox commented May 2, 2019

q66 commented May 2, 2019

briansmith commented May 2, 2019

briansmith commented May 2, 2019

q66 commented May 2, 2019 •

edited

Loading

q66 commented May 2, 2019

briansmith commented May 2, 2019

awilfox commented May 2, 2019 •

edited

Loading

q66 commented May 2, 2019 •

edited

Loading

awilfox commented May 2, 2019

sayrer commented May 3, 2019

awilfox commented May 3, 2019

sayrer commented May 5, 2019

q66 commented May 5, 2019

q66 commented May 5, 2019 •

edited

Loading

q66 commented May 7, 2019

q66 commented May 7, 2019

q66 commented May 7, 2019 •

edited

Loading

q66 commented May 7, 2019

q66 commented May 8, 2019

pietro commented May 8, 2019

q66 commented May 8, 2019

q66 commented May 9, 2019

flier commented May 9, 2019

q66 commented May 9, 2019

add powerpc target #814

add powerpc target #814

Conversation

flier commented Apr 30, 2019

briansmith commented Apr 30, 2019

flier commented Apr 30, 2019

q66 commented May 1, 2019

q66 commented May 1, 2019

q66 commented May 1, 2019

briansmith commented May 2, 2019

q66 commented May 2, 2019

awilfox commented May 2, 2019

q66 commented May 2, 2019

briansmith commented May 2, 2019

briansmith commented May 2, 2019

q66 commented May 2, 2019 • edited Loading

q66 commented May 2, 2019

briansmith commented May 2, 2019

awilfox commented May 2, 2019 • edited Loading

q66 commented May 2, 2019 • edited Loading

awilfox commented May 2, 2019

sayrer commented May 3, 2019

awilfox commented May 3, 2019

sayrer commented May 5, 2019

q66 commented May 5, 2019

q66 commented May 5, 2019 • edited Loading

q66 commented May 7, 2019

q66 commented May 7, 2019

q66 commented May 7, 2019 • edited Loading

q66 commented May 7, 2019

q66 commented May 8, 2019

pietro commented May 8, 2019

q66 commented May 8, 2019

q66 commented May 9, 2019

flier commented May 9, 2019

q66 commented May 9, 2019

add `powerpc` target #814

add `powerpc` target #814

q66 commented May 2, 2019 •

edited

Loading

awilfox commented May 2, 2019 •

edited

Loading

q66 commented May 2, 2019 •

edited

Loading

q66 commented May 5, 2019 •

edited

Loading

q66 commented May 7, 2019 •

edited

Loading