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 <vinee...@microsoft.com> Cc: # Maintainers or other people you want to Cc Signed-off-by: Vineel Kovvuri <vinee...@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....@groups.io> Sent: Thursday, February 24, 2022 2:51 PM To: Li, Yi1 <yi1...@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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86961): https://edk2.groups.io/g/devel/message/86961 Mute This Topic: https://groups.io/mt/86257810/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
