Hi Vineel,

Code is good to me, just some BKM for edk2 upstream:


  1.  It's a little strange that there are submodule changes in the patch 0004, maybe you forget to run git submodule update:

diff --git a/BaseTools/Source/C/BrotliCompress/brotli b/BaseTools/Source/C/BrotliCompress/brotli

index f4153a09f8..666c3280cc 160000

--- a/BaseTools/Source/C/BrotliCompress/brotli

+++ b/BaseTools/Source/C/BrotliCompress/brotli



  1.  Good commit titles and comments can get feedback from the community more quickly and more accurately, refer: https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format,

And CC Maintainers about changed pkg in commit will remind relevant people to review the code as soon as possible, you can find them at: https://github.com/tianocore/edk2/blob/master/Maintainers.txt,

A demo:



CryptoPkg: Enable ECC ciphers



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



Reconfigure OpenSSLLib to add elliptic curve ciphers  # detail info



Cc: Vineel Kovvuri <vineelko@microsoft.com>

Cc: # Maintainers or other people you want to Cc

Signed-off-by: Vineel Kovvuri <vineelko@microsoft.com>



  1.  According to 2, it is best to split the changes of different PKGs, such in patch 0003.


  1.  Extra spaces or tabs can cause formatting errors in CI, make sure there are no unnecessary changes in the patch. Such:

#ifndef OSSL_CRYPTO_DSO_CONF_H
-#define OSSL_CRYPTO_DSO_CONF_H
-#define DSO_NONE
-#define DSO_EXTENSION  ".so"
+# define OSSL_CRYPTO_DSO_CONF_H
+# define DSO_NONE
+# define DSO_EXTENSION ".so"
#endif

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


Thanks!
Yi Li
From: vineelko via groups.io <vineelko=microsoft.com@groups.io>
Sent: Thursday, February 24, 2022 2:51 PM
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

Hi Yi Li,

I have posted the recent patch set to enable ECC ciphers in OpenSSLLib to the bug https://bugzilla.tianocore.org/show_bug.cgi?id=3679

I have ran the entire OVMF Azure pipeline locally and confirm that the code gets build without any issue. Thanks for the inputs after enlarging DXEFV the build succeeded.

I am new to EDK build and to the overall process so please review the patch set and provide your comments. I am happy to address them. Once reviewed I can add it to the proposed feature to the release planning wiki
0001-Crypto-Enable-ECC-ciphers.patch
0002-Port-VsIntrinsicLib-from-Project-Mu.patch
0003-Reference-VsIntrinsicLib.patch
0004-Increase-FV-size.patch

Thanks,
Vineel