Skip to content

Support cpuid for win32 #8339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Oct 23, 2023
Merged

Support cpuid for win32 #8339

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
308cb23
aesni: support cpuid on WIN32
lpy4105 Oct 10, 2023
e8c4bf1
aesni: declare cpuinfo as int
lpy4105 Oct 10, 2023
0ecb635
aesni: select `__cpuid` impl based on compiler type
lpy4105 Oct 11, 2023
64cca2f
all.sh: Re-enable MBEDTLS_AESNI_C in some components
lpy4105 Oct 10, 2023
1322613
all.sh: revert changes in test_m32*
lpy4105 Oct 16, 2023
bb4f63c
all.sh: build_mingw: test default config without MBEDTLS_AESNI_C
lpy4105 Oct 19, 2023
158eead
all.sh: build_mingw: only test build lib without MBEDTLS_AESNI_C
lpy4105 Oct 19, 2023
74f2c15
Add a changelog entry
lpy4105 Oct 19, 2023
d2d3d63
Reword the changelog entry
lpy4105 Oct 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions library/aesni.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,12 @@
#if defined(MBEDTLS_AESNI_HAVE_CODE)

#if MBEDTLS_AESNI_HAVE_CODE == 2
#if !defined(_WIN32)
#if defined(__GNUC__)
#include <cpuid.h>
#else
#elif defined(_MSC_VER)
#include <intrin.h>
#else
#error "`__cpuid` required by MBEDTLS_AESNI_C is not supported by the compiler"
#endif
#include <immintrin.h>
#endif
Expand All @@ -52,7 +54,7 @@ int mbedtls_aesni_has_support(unsigned int what)

if (!done) {
#if MBEDTLS_AESNI_HAVE_CODE == 2
static unsigned info[4] = { 0, 0, 0, 0 };
static int info[4] = { 0, 0, 0, 0 };
#if defined(_MSC_VER)
__cpuid(info, 1);
#else
Expand Down
22 changes: 16 additions & 6 deletions tests/scripts/all.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4626,7 +4626,7 @@ component_test_m32_o0 () {
# build) and not the i386-specific inline assembly.
msg "build: i386, make, gcc -O0 (ASan build)" # ~ 30s
scripts/config.py full
scripts/config.py unset MBEDTLS_AESNI_C # AESNI depends on cpu modifiers
scripts/config.py unset MBEDTLS_AESNI_C # AESNI for 32-bit is tested in test_aesni_m32
make CC=gcc CFLAGS="$ASAN_CFLAGS -m32 -O0" LDFLAGS="-m32 $ASAN_CFLAGS"

msg "test: i386, make, gcc -O0 (ASan build)"
Expand All @@ -4644,7 +4644,7 @@ component_test_m32_o2 () {
# and go faster for tests.
msg "build: i386, make, gcc -O2 (ASan build)" # ~ 30s
scripts/config.py full
scripts/config.py unset MBEDTLS_AESNI_C # AESNI depends on cpu modifiers
scripts/config.py unset MBEDTLS_AESNI_C # AESNI for 32-bit is tested in test_aesni_m32
make CC=gcc CFLAGS="$ASAN_CFLAGS -m32 -O2" LDFLAGS="-m32 $ASAN_CFLAGS"

msg "test: i386, make, gcc -O2 (ASan build)"
Expand All @@ -4660,7 +4660,7 @@ support_test_m32_o2 () {
component_test_m32_everest () {
msg "build: i386, Everest ECDH context (ASan build)" # ~ 6 min
scripts/config.py set MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED
scripts/config.py unset MBEDTLS_AESNI_C # AESNI depends on cpu modifiers
scripts/config.py unset MBEDTLS_AESNI_C # AESNI for 32-bit is tested in test_aesni_m32
make CC=gcc CFLAGS="$ASAN_CFLAGS -m32 -O2" LDFLAGS="-m32 $ASAN_CFLAGS"

msg "test: i386, Everest ECDH context - main suites (inc. selftests) (ASan build)" # ~ 50s
Expand Down Expand Up @@ -5114,14 +5114,24 @@ component_test_tls13_only_record_size_limit () {

component_build_mingw () {
msg "build: Windows cross build - mingw64, make (Link Library)" # ~ 30s
scripts/config.py unset MBEDTLS_AESNI_C # AESNI depends on cpu modifiers
make CC=i686-w64-mingw32-gcc AR=i686-w64-mingw32-ar LD=i686-w64-minggw32-ld CFLAGS='-Werror -Wall -Wextra' WINDOWS_BUILD=1 lib programs
make CC=i686-w64-mingw32-gcc AR=i686-w64-mingw32-ar LD=i686-w64-minggw32-ld CFLAGS='-Werror -Wall -Wextra -maes -msse2 -mpclmul' WINDOWS_BUILD=1 lib programs
Copy link
Contributor

@tom-cosgrove-arm tom-cosgrove-arm Oct 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to make lib without MBEDTLS_AESNI_C to check that it works on mingw?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean to test the build of pure C impl of AES? I think they have been covered because MBEDTLS_AES_USE_HARDWARE_ONLY is unset in defaul config.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I mean to check that the C code builds under mingw, which is quite a "strange" environment - just lib, not programs, which shouldn't take long

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I didn't get the point. We do build lib and programs and later tests for both static and dynamic linked libraries in this mingw test...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So with this PR, under mingw we are now building only with MBEDTLS_AESNI_C set, and not without it? So we are no longer checking that we build correctly under mingw without MBEDTLS_AESNI_C. If that's right, I think we should add a build of lib on its own without MBEDTLS_AESNI_C


# note Make tests only builds the tests, but doesn't run them
make CC=i686-w64-mingw32-gcc AR=i686-w64-mingw32-ar LD=i686-w64-minggw32-ld CFLAGS='-Werror' WINDOWS_BUILD=1 tests
make CC=i686-w64-mingw32-gcc AR=i686-w64-mingw32-ar LD=i686-w64-minggw32-ld CFLAGS='-Werror -maes -msse2 -mpclmul' WINDOWS_BUILD=1 tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change should be replace with Pragma or function attribute in future.

make WINDOWS_BUILD=1 clean

msg "build: Windows cross build - mingw64, make (DLL)" # ~ 30s
make CC=i686-w64-mingw32-gcc AR=i686-w64-mingw32-ar LD=i686-w64-minggw32-ld CFLAGS='-Werror -Wall -Wextra -maes -msse2 -mpclmul' WINDOWS_BUILD=1 SHARED=1 lib programs
make CC=i686-w64-mingw32-gcc AR=i686-w64-mingw32-ar LD=i686-w64-minggw32-ld CFLAGS='-Werror -Wall -Wextra -maes -msse2 -mpclmul' WINDOWS_BUILD=1 SHARED=1 tests
make WINDOWS_BUILD=1 clean

msg "build: Windows cross build - mingw64, make (Link Library, default config without MBEDTLS_AESNI_C)" # ~ 30s
./scripts/config.py unset MBEDTLS_AESNI_C #
make CC=i686-w64-mingw32-gcc AR=i686-w64-mingw32-ar LD=i686-w64-minggw32-ld CFLAGS='-Werror -Wall -Wextra' WINDOWS_BUILD=1 lib programs
make CC=i686-w64-mingw32-gcc AR=i686-w64-mingw32-ar LD=i686-w64-minggw32-ld CFLAGS='-Werror' WINDOWS_BUILD=1 tests
Copy link
Contributor

@tom-cosgrove-arm tom-cosgrove-arm Oct 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the config without MBEDTLS_AESNI_C we only need to make lib - we don't need to build programs or tests. The main reason for doing this is to ensure that the strange mingw environment (not quite *nix, not quite Windows) doesn't cause something related to AES-NI to get mis-compiled in. Outside of the aes*.c modules this is very unlikely, and will add unnecessary time to CI runs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not need to test DLL either, I think?

make WINDOWS_BUILD=1 clean

msg "build: Windows cross build - mingw64, make (DLL, default config without MBEDTLS_AESNI_C)" # ~ 30s
make CC=i686-w64-mingw32-gcc AR=i686-w64-mingw32-ar LD=i686-w64-minggw32-ld CFLAGS='-Werror -Wall -Wextra' WINDOWS_BUILD=1 SHARED=1 lib programs
make CC=i686-w64-mingw32-gcc AR=i686-w64-mingw32-ar LD=i686-w64-minggw32-ld CFLAGS='-Werror -Wall -Wextra' WINDOWS_BUILD=1 SHARED=1 tests
make WINDOWS_BUILD=1 clean
Expand Down