Re: [edk2-devel] [PATCH 1/2] Reconfigure OpensslLib to add elliptic curve chipher algorithms

["yi1 li" <yi1.li@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1708 bytes --]

Hi Vineel,

I noticed that there are some CI errors still in PR,


  1.  The VsIntrinscicLib is only used in OpenSSL related lib, putting it only in the CryptoPkg would make more sense and simplify the review process.



  1.  A BKM: NULL LibraryClass means that its internal API will not be called by external modules, the correct usage of a library is to give it a name and use it in other modules,

And this link will be more clear: https://edk2.groups.io/g/devel/topic/what_is_a_null_library/80192232?p=,,,20,0,0,0::recentpostdate%2Fsticky,,,20,2,20,80192232,

This is also the root cause of the remaining CI errors.



  1.  I drafted a demo patch and it passed the CI test,

Seems we only need three patch:

CryptoPkg: Reconfigure OpensslLib to add elliptic curve cipher algori…

CryptoPkg: Add instrinsics to support building ECC on IA32 windows

OvmfPkg: Increase DXEFV size to accommodate ECC ciphers related changes

FYR.

Thanks!
Yi Li
From: vineelko via groups.io <vineelko=microsoft.com@groups.io>
Sent: Saturday, February 26, 2022 1:52 AM
To: Li, Yi1 <yi1.li@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH 1/2] Reconfigure OpensslLib to add elliptic curve chipher algorithms

Huge Thanks for "You can submit PR to edk2 mater branch directly to check for CI bugs(will not be reviewed or merged)."

I am fixing them. Regarding the style(extra spaces and tabs), It is actually coming from openssl when we run CryptoPkg/Library/OpensslLib/process_files.pl
Not sure if there a way to exclude some of the files from checking the style?

Sample PR against EDK2 master: https://github.com/tianocore/edk2/pull/2546/files

Thanks,
Vineel

[-- Attachment #1.2: Type: text/html, Size: 6791 bytes --]

[-- Attachment #2: 0001-CryptoPkg-Add-instrinsics-to-support-building-ECC-on.patch --]
[-- Type: application/octet-stream, Size: 6342 bytes --]

From d4622c67ae10557ac379f1e388175869c2e86f85 Mon Sep 17 00:00:00 2001
From: yi1 li <yi1.li@intel.com>
Date: Mon, 28 Feb 2022 14:54:05 +0800
Subject: [PATCH 1/1] CryptoPkg: Add instrinsics to support building ECC on
 IA32 windows

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3679

This dependency is needed to build openssl lib with ECC ciphers
under IA32 Windows and adds implementation for _allmul and _allshr
instrinsics.

It is taken from Project Mu:
microsoft/mu_basecore@b55b341

Signed-off-by: yi1 li <yi1.li@intel.com>
---
 .../Library/IntrinsicLib/Ia32/MathLlmul.asm   | 98 +++++++++++++++++++
 .../Library/IntrinsicLib/Ia32/MathLlshr.asm   | 78 +++++++++++++++
 .../Library/IntrinsicLib/IntrinsicLib.inf     |  2 +
 3 files changed, 178 insertions(+)
 create mode 100644 CryptoPkg/Library/IntrinsicLib/Ia32/MathLlmul.asm
 create mode 100644 CryptoPkg/Library/IntrinsicLib/Ia32/MathLlshr.asm

diff --git a/CryptoPkg/Library/IntrinsicLib/Ia32/MathLlmul.asm b/CryptoPkg/Library/IntrinsicLib/Ia32/MathLlmul.asm
new file mode 100644
index 000000000000..341ea8a7bc0d
--- /dev/null
+++ b/CryptoPkg/Library/IntrinsicLib/Ia32/MathLlmul.asm
@@ -0,0 +1,98 @@
+;***
+;llmul.asm - long multiply routine
+;
+;       Copyright (c) Microsoft Corporation. All rights reserved.
+;       SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+;Purpose:
+;       Defines long multiply routine
+;       Both signed and unsigned routines are the same, since multiply's
+;       work out the same in 2's complement
+;       creates the following routine:
+;           __allmul
+;
+;Original Implemenation: MSVC 14.12.25827
+;
+;*******************************************************************************
+    .686
+    .model  flat,C
+    .code
+
+
+;***
+;llmul - long multiply routine
+;
+;Purpose:
+;       Does a long multiply (same for signed/unsigned)
+;       Parameters are not changed.
+;
+;Entry:
+;       Parameters are passed on the stack:
+;               1st pushed: multiplier (QWORD)
+;               2nd pushed: multiplicand (QWORD)
+;
+;Exit:
+;       EDX:EAX - product of multiplier and multiplicand
+;       NOTE: parameters are removed from the stack
+;
+;Uses:
+;       ECX
+;
+;Exceptions:
+;
+;*******************************************************************************
+_allmul PROC NEAR
+
+A       EQU     [esp + 4]       ; stack address of a
+B       EQU     [esp + 12]      ; stack address of b
+
+HIGH_PART  EQU     [4]             ;
+LOW_PART   EQU     [0]
+
+;
+;       AHI, BHI : upper 32 bits of A and B
+;       ALO, BLO : lower 32 bits of A and B
+;
+;             ALO * BLO
+;       ALO * BHI
+; +     BLO * AHI
+; ---------------------
+;
+
+        mov     eax,HIGH_PART(A)
+        mov     ecx,HIGH_PART(B)
+        or      ecx,eax         ;test for both high dwords zero.
+        mov     ecx,LOW_PART(B)
+        jnz     short hard      ;both are zero, just mult ALO and BLO
+
+        mov     eax,LOW_PART(A)
+        mul     ecx
+
+        ret     16              ; callee restores the stack
+
+hard:
+        push    ebx
+
+; must redefine A and B since esp has been altered
+
+A2      EQU     [esp + 8]       ; stack address of a
+B2      EQU     [esp + 16]      ; stack address of b
+
+        mul     ecx             ;eax has AHI, ecx has BLO, so AHI * BLO
+        mov     ebx,eax         ;save result
+
+        mov     eax,LOW_PART(A2)
+        mul     dword ptr HIGH_PART(B2) ;ALO * BHI
+        add     ebx,eax         ;ebx = ((ALO * BHI) + (AHI * BLO))
+
+        mov     eax,LOW_PART(A2);ecx = BLO
+        mul     ecx             ;so edx:eax = ALO*BLO
+        add     edx,ebx         ;now edx has all the LO*HI stuff
+
+        pop     ebx
+
+        ret     16              ; callee restores the stack
+
+_allmul ENDP
+
+        end
diff --git a/CryptoPkg/Library/IntrinsicLib/Ia32/MathLlshr.asm b/CryptoPkg/Library/IntrinsicLib/Ia32/MathLlshr.asm
new file mode 100644
index 000000000000..ab8294580f16
--- /dev/null
+++ b/CryptoPkg/Library/IntrinsicLib/Ia32/MathLlshr.asm
@@ -0,0 +1,78 @@
+;***
+;llshr.asm - long shift right
+;
+;       Copyright (c) Microsoft Corporation. All rights reserved.
+;       SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+;Purpose:
+;       define signed long shift right routine
+;           __allshr
+;
+;Original Implemenation: MSVC 14.12.25827
+;
+;*******************************************************************************
+    .686
+    .model  flat,C
+    .code
+
+
+
+;***
+;llshr - long shift right
+;
+;Purpose:
+;       Does a signed Long Shift Right
+;       Shifts a long right any number of bits.
+;
+;Entry:
+;       EDX:EAX - long value to be shifted
+;       CL    - number of bits to shift by
+;
+;Exit:
+;       EDX:EAX - shifted value
+;
+;Uses:
+;       CL is destroyed.
+;
+;Exceptions:
+;
+;*******************************************************************************
+_allshr PROC NEAR
+
+;
+; Handle shifts of 64 bits or more (if shifting 64 bits or more, the result
+; depends only on the high order bit of edx).
+;
+        cmp     cl,64
+        jae     short RETSIGN
+
+;
+; Handle shifts of between 0 and 31 bits
+;
+        cmp     cl, 32
+        jae     short MORE32
+        shrd    eax,edx,cl
+        sar     edx,cl
+        ret
+
+;
+; Handle shifts of between 32 and 63 bits
+;
+MORE32:
+        mov     eax,edx
+        sar     edx,31
+        and     cl,31
+        sar     eax,cl
+        ret
+
+;
+; Return double precision 0 or -1, depending on the sign of edx
+;
+RETSIGN:
+        sar     edx,31
+        mov     eax,edx
+        ret
+
+_allshr ENDP
+
+        end
diff --git a/CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf b/CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
index fcbb93316cf7..86e74b57b109 100644
--- a/CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+++ b/CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
@@ -30,6 +30,8 @@
   Ia32/MathLShiftS64.c      | MSFT
   Ia32/MathRShiftU64.c      | MSFT
   Ia32/MathFtol.c           | MSFT
+  Ia32/MathLlmul.asm        | MSFT
+  Ia32/MathLlshr.asm        | MSFT
 
   Ia32/MathLShiftS64.c      | INTEL
   Ia32/MathRShiftU64.c      | INTEL
-- 
2.33.0.windows.2