Merged PR 12171827: Small build fixes

+ Make testing against legacy implementations entirely opt-in
  + Do not checkout SymCryptDependencies submodule by default
  + Never build against SymCryptDependencies in the official build pipeline
+ Enable AMD64 OP-TEE static lib build by adding CPUID detection logic

Author: samuel-lee-msft

.gitmodules

@@ -1,6 +1,7 @@
 [submodule "SymCryptDependencies"]
 	path = unittest/SymCryptDependencies
 	url = https://microsoft.visualstudio.com/DefaultCollection/SymCryptDependencies/_git/SymCryptDependencies
+	update = none
 
 [submodule "jitterentropy-library"]
 	path = 3rdparty/jitterentropy-library

.pipelines/OneBranch.Official.yml

@@ -85,25 +85,21 @@ extends:
           arch: 'AMD64'
           config: 'Debug'
           sign: true
-          additionalArgs: '--test-legacy-impl'
       - template: .pipelines/templates/build-windows-cmake.yml@self
         parameters:
           arch: 'AMD64'
           config: 'Release'
           sign: true
-          additionalArgs: '--test-legacy-impl'
       - template: .pipelines/templates/build-windows-cmake.yml@self
         parameters:
           arch: 'X86'
           config: 'Debug'
           sign: true
-          additionalArgs: '--test-legacy-impl'
       - template: .pipelines/templates/build-windows-cmake.yml@self
         parameters:
           arch: 'X86'
           config: 'Release'
           sign: true
-          additionalArgs: '--test-legacy-impl'
       - template: .pipelines/templates/build-windows-cmake.yml@self
         parameters:
           arch: 'AMD64'

.pipelines/OneBranch.PullRequest.yml

@@ -69,12 +69,10 @@ extends:
         parameters:
           arch: 'AMD64'
           config: 'Debug'
-          additionalArgs: '--test-legacy-impl'
       - template: .pipelines/templates/build-windows-cmake.yml@self
         parameters:
           arch: 'AMD64'
           config: 'Release'
-          additionalArgs: '--test-legacy-impl'
           libcrux: true
       - template: .pipelines/templates/build-windows-cmake.yml@self
         parameters:
@@ -95,12 +93,10 @@ extends:
         parameters:
           arch: 'X86'
           config: 'Debug'
-          additionalArgs: '--test-legacy-impl'
       - template: .pipelines/templates/build-windows-cmake.yml@self
         parameters:
           arch: 'X86'
           config: 'Release'
-          additionalArgs: '--test-legacy-impl'
           libcrux: true
       - template: .pipelines/templates/build-windows-cmake.yml@self
         parameters:

.pipelines/templates/build-windows-undocked.yml

@@ -134,7 +134,7 @@ jobs:
       maximumCpuCount: true
       restoreNugetPackages: ${{ parameters.restoreNugetPackages }}
       msbuildArchitecture: 'x64'
-      msbuildArgs: '-p:UndockedOfficial=${{ parameters.nativeCompiler }} -p:UndockedBuildId=$(Build.BuildId) -p:VER_BUILD_ID=$(Build.BuildId) -p:VER_SUFFIX=${{ parameters.buildType }} -p:SymCryptTestLegacyImpl=true ${{ parameters.msbuildArgs }}'
+      msbuildArgs: '-p:UndockedOfficial=${{ parameters.nativeCompiler }} -p:UndockedBuildId=$(Build.BuildId) -p:VER_BUILD_ID=$(Build.BuildId) -p:VER_SUFFIX=${{ parameters.buildType }} ${{ parameters.msbuildArgs }}'
 
   - task: SDLNativeRules@3
     displayName: '🛡 Guardian: PreFast'

BUILD.md

@@ -41,9 +41,9 @@ but not limited to the warranties of merchantability, fitness for a particular p
 ## Build Instructions
 For Microsoft employees building the library internally, to include msbignum and RSA32 implementation benchmarks in
 the unit tests, make sure the SymCryptDependencies submodule is initialized by following the steps above
-(`git submodule update --init`). When building, specify `/p:SymCryptTestLegacyImpl=true` for MSBuild,
-or `-DSYMCRYPT_TEST_LEGACY_IMPL=ON` for CMake. This only affects the unit tests, and does not change the
-functionality of the SymCrypt library itself.
+(`git submodule init && git submodule update --checkout unittest/SymCryptDependencies`). When building, specify
+`/p:SymCryptTestLegacyImpl=true` for MSBuild, or `-DSYMCRYPT_TEST_LEGACY_IMPL=ON` for CMake. This only affects the
+unit tests, and does not change the functionality of the SymCrypt library itself.
 
 ### Using Python scripts
 Building SymCrypt can be complicated due to the number of platforms, architectures and targets supported. To improve

README.md

@@ -22,7 +22,7 @@ Like any engineering project, SymCrypt is a compromise between conflicting requi
 In some of our Linux modules, SymCrypt uses [Jitterentropy](https://github.com/smuellerDD/jitterentropy-library)
 as a source of FIPS-certifiable entropy. To build these modules, you will need to ensure that the
 jitterentropy-library submodule is also cloned. You can do this by running
-`git submodule update --init -- 3rdparty/jitterentropy-library` after cloning.
+`git submodule update --init` after cloning.
 
 The `unittest/SymCryptDependencies` submodule provides the RSA32 and msbignum implementations which are used as
 benchmarks in the unit tests when compiled on Windows. Due to licensing restrictions, we cannot release these

cmake-configs/SymCrypt-Platforms.cmake

@@ -81,21 +81,7 @@ if(CMAKE_SYSTEM_NAME MATCHES "Linux|Darwin")
         # Enable a baseline of features for the compiler to support everywhere
         # Assumes that the compiler will not emit crypto instructions as a result of normal C code
         set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -march=armv8-a+simd+crypto")
-        
-        if(SYMCRYPT_TARGET_ENV MATCHES "OPTEE")
-            # TA DEV KIT is require for OPTEE TA compilation 
-            if(DEFINED TA_DEV_KIT_INC)
-                # Get the compiler toolchain include
-                execute_process(COMMAND ${CMAKE_C_COMPILER} -print-file-name=include OUTPUT_VARIABLE TOOLCHAIN_INCLUDE)
-                string(STRIP "${TOOLCHAIN_INCLUDE}" TOOLCHAIN_INCLUDE)
-                # OPTEE env has a different stdlib and doesn't support atomic operations or multithreading.
-                add_compile_options(-mno-outline-atomics -nostdinc -isystem ${TOOLCHAIN_INCLUDE})
-                include_directories(${TA_DEV_KIT_INC})
-            else()
-                message(FATAL_ERROR "TA_DEV_KIT_INC must be defined for OPTEE build")
-            endif()
-        endif()
-
+    
         # GCC complains about implicit casting between ASIMD registers (i.e. uint8x16_t -> uint64x2_t) by default,
         # whereas clang and MSVC do not. Setting -flax-vector-conversions to build Arm64 intrinsics code with GCC.
         set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -flax-vector-conversions")
@@ -107,6 +93,27 @@ if(CMAKE_SYSTEM_NAME MATCHES "Linux|Darwin")
         link_libraries(c)
         link_libraries(gcc)
     endif()
+
+    # OPTEE specific compilation options
+    if(SYMCRYPT_TARGET_ENV MATCHES "OPTEE")
+        # TA DEV KIT is require for OPTEE TA compilation 
+        if(DEFINED TA_DEV_KIT_INC)
+            # Get the compiler toolchain include
+            execute_process(COMMAND ${CMAKE_C_COMPILER} -print-file-name=include OUTPUT_VARIABLE TOOLCHAIN_INCLUDE)
+            string(STRIP "${TOOLCHAIN_INCLUDE}" TOOLCHAIN_INCLUDE)
+
+            if(SYMCRYPT_TARGET_ARCH MATCHES "ARM64")
+                # OPTEE env on Arm64 does not support atomic operations
+                add_compile_options(-mno-outline-atomics)
+            endif()
+
+            # OPTEE env has a different stdlib
+            add_compile_options(-nostdinc -isystem ${TOOLCHAIN_INCLUDE})
+            include_directories(${TA_DEV_KIT_INC})
+        else()
+            message(FATAL_ERROR "TA_DEV_KIT_INC must be defined for OPTEE build")
+        endif()
+    endif()
 
     # add_compile_options(-Wall)
     add_compile_options(-Wno-unknown-pragmas)

lib/CMakeLists.txt

@@ -415,14 +415,16 @@ if(NOT WIN32)
         set_target_properties(symcrypt_posixusermode PROPERTIES PREFIX "")
         target_link_libraries(symcrypt_posixusermode symcrypt_common)
     elseif(SYMCRYPT_TARGET_ENV MATCHES "OPTEE")
-        # Remove files from symcrypt_common that are not supported in optee env.
-        # aes-asm.c - use SymCryptAesDecryptAsm which is not defined for ARM64
-        # cpuid_um.c - include auxv.h and use getauxval
-        # session.c - use atomic operations (__atomic_compare_exchange)
-        list(REMOVE_ITEM SOURCES_COMMON
-            aes-asm.c
-            cpuid_um.c
-            session.c)
+        if(SYMCRYPT_TARGET_ARCH MATCHES "ARM64")
+            # Remove files from symcrypt_common that are not supported in ARM64 optee env.
+            # aes-asm.c - use SymCryptAesDecryptAsm which is not defined for ARM64
+            # cpuid_um.c - include auxv.h and use getauxval
+            # session.c - use atomic operations (__atomic_compare_exchange)
+            list(REMOVE_ITEM SOURCES_COMMON
+                aes-asm.c
+                cpuid_um.c
+                session.c)
+        endif()
 
         add_library(symcrypt_envOpteeTa STATIC env_opteeTa.c)
         set_target_properties(symcrypt_envOpteeTa PROPERTIES PREFIX "")

lib/env_opteeTa.c

@@ -29,9 +29,26 @@ SymCryptInitEnvOpteeTa( UINT32 version )
         return;
     }
 
-    // Optee module relies on the unconditional availability of certain CPU features (ASIMD, AES, PMULL, SHA256)
+#if SYMCRYPT_CPU_ARM64
+
+    // Optee on Arm64 currently relies on the unconditional availability of certain CPU features (ASIMD, AES, PMULL, SHA256)
     g_SymCryptCpuFeaturesNotPresent = (SYMCRYPT_CPU_FEATURES) ~(SYMCRYPT_CPU_FEATURE_NEON|SYMCRYPT_CPU_FEATURE_NEON_AES|SYMCRYPT_CPU_FEATURE_NEON_PMULL|SYMCRYPT_CPU_FEATURE_NEON_SHA256);
 
+#elif SYMCRYPT_CPU_AMD64
+
+    SymCryptDetectCpuFeaturesByCpuid( SYMCRYPT_CPUID_DETECT_FLAG_CHECK_OS_SUPPORT_FOR_YMM );
+
+    //
+    // Our SaveXmm function never fails because it doesn't have to do anything in User mode.
+    //
+    g_SymCryptCpuFeaturesNotPresent &= ~SYMCRYPT_CPU_FEATURE_SAVEXMM_NOFAIL;
+
+#else
+    
+    #error We only support ARM64 and AMD64 for OPTEE environment
+
+#endif
+
     SymCryptInitEnvCommon( version );
 }
 
@@ -76,3 +93,46 @@ SymCryptTestInjectErrorEnvOpteeTa( PBYTE pbBuf, SIZE_T cbBuf )
     UNREFERENCED_PARAMETER( cbBuf );
 }
 
+
+#if SYMCRYPT_CPU_AMD64
+
+SYMCRYPT_ERROR
+SYMCRYPT_CALL
+SymCryptSaveXmmEnvOpteeTa( _Out_ PSYMCRYPT_EXTENDED_SAVE_DATA pSaveData )
+{
+    UNREFERENCED_PARAMETER( pSaveData );
+
+    return SYMCRYPT_NO_ERROR;
+}
+
+VOID
+SYMCRYPT_CALL
+SymCryptRestoreXmmEnvOpteeTa( _Inout_ PSYMCRYPT_EXTENDED_SAVE_DATA pSaveData )
+{
+    UNREFERENCED_PARAMETER( pSaveData );
+}
+
+SYMCRYPT_ERROR
+SYMCRYPT_CALL
+SymCryptSaveYmmEnvOpteeTa( _Out_ PSYMCRYPT_EXTENDED_SAVE_DATA pSaveData )
+{
+    UNREFERENCED_PARAMETER( pSaveData );
+
+    return SYMCRYPT_NO_ERROR;
+}
+
+VOID
+SYMCRYPT_CALL
+SymCryptRestoreYmmEnvOpteeTa( _Inout_ PSYMCRYPT_EXTENDED_SAVE_DATA pSaveData )
+{
+    UNREFERENCED_PARAMETER( pSaveData );
+}
+
+VOID
+SYMCRYPT_CALL
+SymCryptCpuidExFuncEnvOpteeTa( int cpuInfo[4], int function_id, int subfunction_id )
+{
+    __cpuidex( cpuInfo, function_id, subfunction_id );
+}
+
+#endif