CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c | 8 +- .../Library/BaseCryptLib/Hmac/CryptHmacSha1.c | 9 +- .../Library/BaseCryptLib/Hmac/CryptHmacSha256.c | 8 +- CryptoPkg/Library/Include/CrtLibSupport.h | 11 + CryptoPkg/Library/Include/openssl/opensslconf.h | 54 +++- CryptoPkg/Library/Include/sys/syscall.h | 9 + CryptoPkg/Library/IntrinsicLib/Ia32/MathFtol.c | 22 ++ CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf | 4 +- CryptoPkg/Library/OpensslLib/OpensslLib.inf | 63 +++- CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 54 +++- CryptoPkg/Library/OpensslLib/buildinf.h | 2 + CryptoPkg/Library/OpensslLib/openssl | 2 +- CryptoPkg/Library/OpensslLib/ossl_store.c | 17 ++ CryptoPkg/Library/OpensslLib/process_files.pl | 11 +- CryptoPkg/Library/OpensslLib/rand_pool.c | 339 +++++++++++++++++++++ 15 files changed, 564 insertions(+), 49 deletions(-) create mode 100644 CryptoPkg/Library/Include/sys/syscall.h create mode 100644 CryptoPkg/Library/IntrinsicLib/Ia32/MathFtol.c create mode 100644 CryptoPkg/Library/OpensslLib/ossl_store.c create mode 100644 CryptoPkg/Library/OpensslLib/rand_pool.c
(1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). So add --with-rand-seed=none to process_files.pl. (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl When running process_files.py to configure OpenSSL, we can exclude some unnecessary files. This can reduce porting time, compiling time and library size. (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL Disable warning for building OpenSSL_1_1_1b (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b Update OpenSSL submodule to OpenSSL_1_1_1b OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) OpenSSL doesn't implement some rand_pool function for UEFI. Use EFI_RNG_PROTOCOL to generate random for entropy. If EFI_RNG_PROTOCOL is not avaliable, fall back to performance counter, but we not sure about the amount of randomness it provides. (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible Note: Will be remove next update. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 Ref: https://github.com/openssl/openssl/pull/4338 Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Ting Ye <ting.ye@intel.com> Xiaoyu Lu (3): CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue CryptoPkg: Upgrade OpenSSL to 1.1.1b CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible Xiaoyu lu (3): CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c | 8 +- .../Library/BaseCryptLib/Hmac/CryptHmacSha1.c | 9 +- .../Library/BaseCryptLib/Hmac/CryptHmacSha256.c | 8 +- CryptoPkg/Library/Include/CrtLibSupport.h | 11 + CryptoPkg/Library/Include/openssl/opensslconf.h | 54 +++- CryptoPkg/Library/Include/sys/syscall.h | 9 + CryptoPkg/Library/IntrinsicLib/Ia32/MathFtol.c | 22 ++ CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf | 4 +- CryptoPkg/Library/OpensslLib/OpensslLib.inf | 63 +++- CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 54 +++- CryptoPkg/Library/OpensslLib/buildinf.h | 2 + CryptoPkg/Library/OpensslLib/openssl | 2 +- CryptoPkg/Library/OpensslLib/ossl_store.c | 17 ++ CryptoPkg/Library/OpensslLib/process_files.pl | 11 +- CryptoPkg/Library/OpensslLib/rand_pool.c | 339 +++++++++++++++++++++ 15 files changed, 564 insertions(+), 49 deletions(-) create mode 100644 CryptoPkg/Library/Include/sys/syscall.h create mode 100644 CryptoPkg/Library/IntrinsicLib/Ia32/MathFtol.c create mode 100644 CryptoPkg/Library/OpensslLib/ossl_store.c create mode 100644 CryptoPkg/Library/OpensslLib/rand_pool.c -- 2.7.4 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#40500): https://edk2.groups.io/g/devel/message/40500 Mute This Topic: https://groups.io/mt/31606972/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 05/13/19 15:25, Xiaoyu lu wrote: > (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL > OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). > So add --with-rand-seed=none to process_files.pl. > > (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl > When running process_files.py to configure OpenSSL, we can exclude some unnecessary files. This can reduce porting time, compiling time and library size. > > (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue > > (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL > Disable warning for building OpenSSL_1_1_1b > > (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b > Update OpenSSL submodule to OpenSSL_1_1_1b > OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) > > OpenSSL doesn't implement some rand_pool function for UEFI. > Use EFI_RNG_PROTOCOL to generate random for entropy. > If EFI_RNG_PROTOCOL is not avaliable, fall back to performance > counter, but we not sure about the amount of randomness it provides. > > (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible > > Note: Will be remove next update. > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 > Ref: https://github.com/openssl/openssl/pull/4338 > > > Cc: Jian J Wang <jian.j.wang@intel.com> > Cc: Ting Ye <ting.ye@intel.com> I'm withdrawing from reviewing or testing this series. Gary, if you have the time, can you please regression test this (for HTTPS boot) in both OVMF and ArmVirtQemu? Thank you Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#40534): https://edk2.groups.io/g/devel/message/40534 Mute This Topic: https://groups.io/mt/31606972/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Mon, May 13, 2019 at 09:24:39PM +0200, Laszlo Ersek wrote: > On 05/13/19 15:25, Xiaoyu lu wrote: > > (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL > > OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). > > So add --with-rand-seed=none to process_files.pl. > > > > (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl > > When running process_files.py to configure OpenSSL, we can exclude some unnecessary files. This can reduce porting time, compiling time and library size. > > > > (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue > > > > (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL > > Disable warning for building OpenSSL_1_1_1b > > > > (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b > > Update OpenSSL submodule to OpenSSL_1_1_1b > > OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) > > > > OpenSSL doesn't implement some rand_pool function for UEFI. > > Use EFI_RNG_PROTOCOL to generate random for entropy. > > If EFI_RNG_PROTOCOL is not avaliable, fall back to performance > > counter, but we not sure about the amount of randomness it provides. > > > > (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible > > > > Note: Will be remove next update. > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 > > Ref: https://github.com/openssl/openssl/pull/4338 > > > > > > Cc: Jian J Wang <jian.j.wang@intel.com> > > Cc: Ting Ye <ting.ye@intel.com> > > I'm withdrawing from reviewing or testing this series. > > Gary, if you have the time, can you please regression test this (for > HTTPS boot) in both OVMF and ArmVirtQemu? > I'll find some time to do the regression test tomorrorw. Cheers, Gary Lin -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#40575): https://edk2.groups.io/g/devel/message/40575 Mute This Topic: https://groups.io/mt/31606972/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 05/14/19 08:16, Gary Lin wrote: > On Mon, May 13, 2019 at 09:24:39PM +0200, Laszlo Ersek wrote: >> On 05/13/19 15:25, Xiaoyu lu wrote: >>> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL >>> OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). >>> So add --with-rand-seed=none to process_files.pl. >>> >>> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl >>> When running process_files.py to configure OpenSSL, we can exclude some unnecessary files. This can reduce porting time, compiling time and library size. >>> >>> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue >>> >>> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL >>> Disable warning for building OpenSSL_1_1_1b >>> >>> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b >>> Update OpenSSL submodule to OpenSSL_1_1_1b >>> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) >>> >>> OpenSSL doesn't implement some rand_pool function for UEFI. >>> Use EFI_RNG_PROTOCOL to generate random for entropy. >>> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance >>> counter, but we not sure about the amount of randomness it provides. >>> >>> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible >>> >>> Note: Will be remove next update. >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 >>> Ref: https://github.com/openssl/openssl/pull/4338 >>> >>> >>> Cc: Jian J Wang <jian.j.wang@intel.com> >>> Cc: Ting Ye <ting.ye@intel.com> >> >> I'm withdrawing from reviewing or testing this series. >> >> Gary, if you have the time, can you please regression test this (for >> HTTPS boot) in both OVMF and ArmVirtQemu? >> > I'll find some time to do the regression test tomorrorw. Thanks, Gary! Xiaoyu might post a v4 with a remote topic branch for reviewers to fetch; I suggest awaiting that. (The series is difficult to apply with git-am.) Thanks Laszlo > Cheers, > > Gary Lin > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#40592): https://edk2.groups.io/g/devel/message/40592 Mute This Topic: https://groups.io/mt/31606972/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Yes, please wait for v4 version of this patch series. Regards, Jian > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Tuesday, May 14, 2019 8:06 PM > To: devel@edk2.groups.io; glin@suse.com > Cc: Lu, XiaoyuX <xiaoyux.lu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; > Ye, Ting <ting.ye@intel.com> > Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b > > On 05/14/19 08:16, Gary Lin wrote: > > On Mon, May 13, 2019 at 09:24:39PM +0200, Laszlo Ersek wrote: > >> On 05/13/19 15:25, Xiaoyu lu wrote: > >>> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL > >>> OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). > >>> So add --with-rand-seed=none to process_files.pl. > >>> > >>> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl > >>> When running process_files.py to configure OpenSSL, we can exclude some > unnecessary files. This can reduce porting time, compiling time and library size. > >>> > >>> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue > >>> > >>> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL > >>> Disable warning for building OpenSSL_1_1_1b > >>> > >>> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b > >>> Update OpenSSL submodule to OpenSSL_1_1_1b > >>> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) > >>> > >>> OpenSSL doesn't implement some rand_pool function for UEFI. > >>> Use EFI_RNG_PROTOCOL to generate random for entropy. > >>> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance > >>> counter, but we not sure about the amount of randomness it provides. > >>> > >>> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible > >>> > >>> Note: Will be remove next update. > >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 > >>> Ref: https://github.com/openssl/openssl/pull/4338 > >>> > >>> > >>> Cc: Jian J Wang <jian.j.wang@intel.com> > >>> Cc: Ting Ye <ting.ye@intel.com> > >> > >> I'm withdrawing from reviewing or testing this series. > >> > >> Gary, if you have the time, can you please regression test this (for > >> HTTPS boot) in both OVMF and ArmVirtQemu? > >> > > I'll find some time to do the regression test tomorrorw. > > Thanks, Gary! > > Xiaoyu might post a v4 with a remote topic branch for reviewers to > fetch; I suggest awaiting that. (The series is difficult to apply with > git-am.) > > Thanks > Laszlo > > > Cheers, > > > > Gary Lin > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#40600): https://edk2.groups.io/g/devel/message/40600 Mute This Topic: https://groups.io/mt/31606972/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Tue, May 14, 2019 at 01:26:15PM +0000, Wang, Jian J wrote: > Yes, please wait for v4 version of this patch series. Good. I'm looking forward to the new series :) Thanks, Gary Lin > > Regards, > Jian > > > > -----Original Message----- > > From: Laszlo Ersek [mailto:lersek@redhat.com] > > Sent: Tuesday, May 14, 2019 8:06 PM > > To: devel@edk2.groups.io; glin@suse.com > > Cc: Lu, XiaoyuX <xiaoyux.lu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; > > Ye, Ting <ting.ye@intel.com> > > Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b > > > > On 05/14/19 08:16, Gary Lin wrote: > > > On Mon, May 13, 2019 at 09:24:39PM +0200, Laszlo Ersek wrote: > > >> On 05/13/19 15:25, Xiaoyu lu wrote: > > >>> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL > > >>> OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). > > >>> So add --with-rand-seed=none to process_files.pl. > > >>> > > >>> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl > > >>> When running process_files.py to configure OpenSSL, we can exclude some > > unnecessary files. This can reduce porting time, compiling time and library size. > > >>> > > >>> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue > > >>> > > >>> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL > > >>> Disable warning for building OpenSSL_1_1_1b > > >>> > > >>> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b > > >>> Update OpenSSL submodule to OpenSSL_1_1_1b > > >>> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) > > >>> > > >>> OpenSSL doesn't implement some rand_pool function for UEFI. > > >>> Use EFI_RNG_PROTOCOL to generate random for entropy. > > >>> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance > > >>> counter, but we not sure about the amount of randomness it provides. > > >>> > > >>> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible > > >>> > > >>> Note: Will be remove next update. > > >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 > > >>> Ref: https://github.com/openssl/openssl/pull/4338 > > >>> > > >>> > > >>> Cc: Jian J Wang <jian.j.wang@intel.com> > > >>> Cc: Ting Ye <ting.ye@intel.com> > > >> > > >> I'm withdrawing from reviewing or testing this series. > > >> > > >> Gary, if you have the time, can you please regression test this (for > > >> HTTPS boot) in both OVMF and ArmVirtQemu? > > >> > > > I'll find some time to do the regression test tomorrorw. > > > > Thanks, Gary! > > > > Xiaoyu might post a v4 with a remote topic branch for reviewers to > > fetch; I suggest awaiting that. (The series is difficult to apply with > > git-am.) > > > > Thanks > > Laszlo > > > > > Cheers, > > > > > > Gary Lin > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#40626): https://edk2.groups.io/g/devel/message/40626 Mute This Topic: https://groups.io/mt/31606972/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Gary Lin: I also need to modify the code about the entropy source today. But I have uploaded a TimerLib based implementation. https://github.com/xiaoyuxlu/edk2/commits/bz_1089_patch_v4 Thanks. Xiaoyu -----Original Message----- From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Gary Lin Sent: Wednesday, May 15, 2019 9:54 AM To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com> Cc: Laszlo Ersek <lersek@redhat.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Ye, Ting <ting.ye@intel.com> Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b On Tue, May 14, 2019 at 01:26:15PM +0000, Wang, Jian J wrote: > Yes, please wait for v4 version of this patch series. Good. I'm looking forward to the new series :) Thanks, Gary Lin > > Regards, > Jian > > > > -----Original Message----- > > From: Laszlo Ersek [mailto:lersek@redhat.com] > > Sent: Tuesday, May 14, 2019 8:06 PM > > To: devel@edk2.groups.io; glin@suse.com > > Cc: Lu, XiaoyuX <xiaoyux.lu@intel.com>; Wang, Jian J > > <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com> > > Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL > > to 1.1.1b > > > > On 05/14/19 08:16, Gary Lin wrote: > > > On Mon, May 13, 2019 at 09:24:39PM +0200, Laszlo Ersek wrote: > > >> On 05/13/19 15:25, Xiaoyu lu wrote: > > >>> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL > > >>> OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). > > >>> So add --with-rand-seed=none to process_files.pl. > > >>> > > >>> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl > > >>> When running process_files.py to configure OpenSSL, we can > > >>> exclude some > > unnecessary files. This can reduce porting time, compiling time and library size. > > >>> > > >>> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external > > >>> symbol issue > > >>> > > >>> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL > > >>> Disable warning for building OpenSSL_1_1_1b > > >>> > > >>> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b > > >>> Update OpenSSL submodule to OpenSSL_1_1_1b > > >>> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) > > >>> > > >>> OpenSSL doesn't implement some rand_pool function for UEFI. > > >>> Use EFI_RNG_PROTOCOL to generate random for entropy. > > >>> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance > > >>> counter, but we not sure about the amount of randomness it provides. > > >>> > > >>> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward > > >>> compatible > > >>> > > >>> Note: Will be remove next update. > > >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 > > >>> Ref: https://github.com/openssl/openssl/pull/4338 > > >>> > > >>> > > >>> Cc: Jian J Wang <jian.j.wang@intel.com> > > >>> Cc: Ting Ye <ting.ye@intel.com> > > >> > > >> I'm withdrawing from reviewing or testing this series. > > >> > > >> Gary, if you have the time, can you please regression test this > > >> (for HTTPS boot) in both OVMF and ArmVirtQemu? > > >> > > > I'll find some time to do the regression test tomorrorw. > > > > Thanks, Gary! > > > > Xiaoyu might post a v4 with a remote topic branch for reviewers to > > fetch; I suggest awaiting that. (The series is difficult to apply > > with > > git-am.) > > > > Thanks > > Laszlo > > > > > Cheers, > > > > > > Gary Lin > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#40627): https://edk2.groups.io/g/devel/message/40627 Mute This Topic: https://groups.io/mt/31606972/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Xiaoyu, On 05/15/19 04:00, Lu, XiaoyuX wrote: > Hi Gary Lin: > I also need to modify the code about the entropy source today. > But I have uploaded a TimerLib based implementation. > > https://github.com/xiaoyuxlu/edk2/commits/bz_1089_patch_v4 This is not a good strategy. Please refer to contributor step 31: https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-31 You should push a topic branch called "xxxx_v4" *only* if you are ready to post it immediately to the list, as "PATCH v4". Topic branches in personal repos must be *identical* to the corresponding posting on edk2-devel. And once such a topic branch is pushed and referenced in an edk2-devel posting, the branch should never ever be modified again. Not rebased, not force-pushed, not fast-forwarded to additional commits on top. Once you have a v4 posting on edk2-devel, the topic branch *for that version* becomes read-only. If you need updates, you need to prepare a v5. It's OK to push (even force-push) branches to your personal repo that are work-in-progress. However, the name of the branch should be very clear about that. For example, you could call the branch "bz_1089_patch_v4_wip", with the "_wip" suffix standing for "work-in-progress". Then people fetching that branch will understand it's not final, and may easily change until the mailing list posting. When you decide it's time to post, you can rename the branch (drop the "_wip" suffix), from which point on you should treat the branch as read-only. Thanks Laszlo > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Gary Lin > Sent: Wednesday, May 15, 2019 9:54 AM > To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Ye, Ting <ting.ye@intel.com> > Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b > > On Tue, May 14, 2019 at 01:26:15PM +0000, Wang, Jian J wrote: >> Yes, please wait for v4 version of this patch series. > > Good. I'm looking forward to the new series :) > > Thanks, > > Gary Lin > >> >> Regards, >> Jian >> >> >>> -----Original Message----- >>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>> Sent: Tuesday, May 14, 2019 8:06 PM >>> To: devel@edk2.groups.io; glin@suse.com >>> Cc: Lu, XiaoyuX <xiaoyux.lu@intel.com>; Wang, Jian J >>> <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com> >>> Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL >>> to 1.1.1b >>> >>> On 05/14/19 08:16, Gary Lin wrote: >>>> On Mon, May 13, 2019 at 09:24:39PM +0200, Laszlo Ersek wrote: >>>>> On 05/13/19 15:25, Xiaoyu lu wrote: >>>>>> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL >>>>>> OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). >>>>>> So add --with-rand-seed=none to process_files.pl. >>>>>> >>>>>> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl >>>>>> When running process_files.py to configure OpenSSL, we can >>>>>> exclude some >>> unnecessary files. This can reduce porting time, compiling time and library size. >>>>>> >>>>>> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external >>>>>> symbol issue >>>>>> >>>>>> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL >>>>>> Disable warning for building OpenSSL_1_1_1b >>>>>> >>>>>> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b >>>>>> Update OpenSSL submodule to OpenSSL_1_1_1b >>>>>> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) >>>>>> >>>>>> OpenSSL doesn't implement some rand_pool function for UEFI. >>>>>> Use EFI_RNG_PROTOCOL to generate random for entropy. >>>>>> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance >>>>>> counter, but we not sure about the amount of randomness it provides. >>>>>> >>>>>> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward >>>>>> compatible >>>>>> >>>>>> Note: Will be remove next update. >>>>>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 >>>>>> Ref: https://github.com/openssl/openssl/pull/4338 >>>>>> >>>>>> >>>>>> Cc: Jian J Wang <jian.j.wang@intel.com> >>>>>> Cc: Ting Ye <ting.ye@intel.com> >>>>> >>>>> I'm withdrawing from reviewing or testing this series. >>>>> >>>>> Gary, if you have the time, can you please regression test this >>>>> (for HTTPS boot) in both OVMF and ArmVirtQemu? >>>>> >>>> I'll find some time to do the regression test tomorrorw. >>> >>> Thanks, Gary! >>> >>> Xiaoyu might post a v4 with a remote topic branch for reviewers to >>> fetch; I suggest awaiting that. (The series is difficult to apply >>> with >>> git-am.) >>> >>> Thanks >>> Laszlo >>> >>>> Cheers, >>>> >>>> Gary Lin >>>> >>>> >>>> >> >> >> >> > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#40643): https://edk2.groups.io/g/devel/message/40643 Mute This Topic: https://groups.io/mt/31606972/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Laszlo: Thanks for your information. If I send the patch v4, I will provide a new branch in my personal repos and not modify it. Thanks, Xiaoyu -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com] Sent: Wednesday, May 15, 2019 4:07 PM To: Lu, XiaoyuX <xiaoyux.lu@intel.com>; devel@edk2.groups.io; glin@suse.com; Wang, Jian J <jian.j.wang@intel.com> Cc: Ye, Ting <ting.ye@intel.com> Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Hi Xiaoyu, On 05/15/19 04:00, Lu, XiaoyuX wrote: > Hi Gary Lin: > I also need to modify the code about the entropy source today. > But I have uploaded a TimerLib based implementation. > > https://github.com/xiaoyuxlu/edk2/commits/bz_1089_patch_v4 This is not a good strategy. Please refer to contributor step 31: https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-31 You should push a topic branch called "xxxx_v4" *only* if you are ready to post it immediately to the list, as "PATCH v4". Topic branches in personal repos must be *identical* to the corresponding posting on edk2-devel. And once such a topic branch is pushed and referenced in an edk2-devel posting, the branch should never ever be modified again. Not rebased, not force-pushed, not fast-forwarded to additional commits on top. Once you have a v4 posting on edk2-devel, the topic branch *for that version* becomes read-only. If you need updates, you need to prepare a v5. It's OK to push (even force-push) branches to your personal repo that are work-in-progress. However, the name of the branch should be very clear about that. For example, you could call the branch "bz_1089_patch_v4_wip", with the "_wip" suffix standing for "work-in-progress". Then people fetching that branch will understand it's not final, and may easily change until the mailing list posting. When you decide it's time to post, you can rename the branch (drop the "_wip" suffix), from which point on you should treat the branch as read-only. Thanks Laszlo > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Gary Lin > Sent: Wednesday, May 15, 2019 9:54 AM > To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com>; Lu, XiaoyuX > <xiaoyux.lu@intel.com>; Ye, Ting <ting.ye@intel.com> > Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to > 1.1.1b > > On Tue, May 14, 2019 at 01:26:15PM +0000, Wang, Jian J wrote: >> Yes, please wait for v4 version of this patch series. > > Good. I'm looking forward to the new series :) > > Thanks, > > Gary Lin > >> >> Regards, >> Jian >> >> >>> -----Original Message----- >>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>> Sent: Tuesday, May 14, 2019 8:06 PM >>> To: devel@edk2.groups.io; glin@suse.com >>> Cc: Lu, XiaoyuX <xiaoyux.lu@intel.com>; Wang, Jian J >>> <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com> >>> Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL >>> to 1.1.1b >>> >>> On 05/14/19 08:16, Gary Lin wrote: >>>> On Mon, May 13, 2019 at 09:24:39PM +0200, Laszlo Ersek wrote: >>>>> On 05/13/19 15:25, Xiaoyu lu wrote: >>>>>> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL >>>>>> OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). >>>>>> So add --with-rand-seed=none to process_files.pl. >>>>>> >>>>>> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl >>>>>> When running process_files.py to configure OpenSSL, we can >>>>>> exclude some >>> unnecessary files. This can reduce porting time, compiling time and library size. >>>>>> >>>>>> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external >>>>>> symbol issue >>>>>> >>>>>> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL >>>>>> Disable warning for building OpenSSL_1_1_1b >>>>>> >>>>>> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b >>>>>> Update OpenSSL submodule to OpenSSL_1_1_1b >>>>>> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) >>>>>> >>>>>> OpenSSL doesn't implement some rand_pool function for UEFI. >>>>>> Use EFI_RNG_PROTOCOL to generate random for entropy. >>>>>> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance >>>>>> counter, but we not sure about the amount of randomness it provides. >>>>>> >>>>>> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward >>>>>> compatible >>>>>> >>>>>> Note: Will be remove next update. >>>>>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 >>>>>> Ref: https://github.com/openssl/openssl/pull/4338 >>>>>> >>>>>> >>>>>> Cc: Jian J Wang <jian.j.wang@intel.com> >>>>>> Cc: Ting Ye <ting.ye@intel.com> >>>>> >>>>> I'm withdrawing from reviewing or testing this series. >>>>> >>>>> Gary, if you have the time, can you please regression test this >>>>> (for HTTPS boot) in both OVMF and ArmVirtQemu? >>>>> >>>> I'll find some time to do the regression test tomorrorw. >>> >>> Thanks, Gary! >>> >>> Xiaoyu might post a v4 with a remote topic branch for reviewers to >>> fetch; I suggest awaiting that. (The series is difficult to apply >>> with >>> git-am.) >>> >>> Thanks >>> Laszlo >>> >>>> Cheers, >>>> >>>> Gary Lin >>>> >>>> >>>> >> >> >> >> > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#40677): https://edk2.groups.io/g/devel/message/40677 Mute This Topic: https://groups.io/mt/31606972/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Wed, May 15, 2019 at 02:00:27AM +0000, Xiaoyu lu wrote: > Hi Gary Lin: > I also need to modify the code about the entropy source today. > But I have uploaded a TimerLib based implementation. > > https://github.com/xiaoyuxlu/edk2/commits/bz_1089_patch_v4 Thanks! I'll go through the commits today and prepare for the incoming series. Gary Lin > > Thanks. > Xiaoyu > > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Gary Lin > Sent: Wednesday, May 15, 2019 9:54 AM > To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Ye, Ting <ting.ye@intel.com> > Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b > > On Tue, May 14, 2019 at 01:26:15PM +0000, Wang, Jian J wrote: > > Yes, please wait for v4 version of this patch series. > > Good. I'm looking forward to the new series :) > > Thanks, > > Gary Lin > > > > > Regards, > > Jian > > > > > > > -----Original Message----- > > > From: Laszlo Ersek [mailto:lersek@redhat.com] > > > Sent: Tuesday, May 14, 2019 8:06 PM > > > To: devel@edk2.groups.io; glin@suse.com > > > Cc: Lu, XiaoyuX <xiaoyux.lu@intel.com>; Wang, Jian J > > > <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com> > > > Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL > > > to 1.1.1b > > > > > > On 05/14/19 08:16, Gary Lin wrote: > > > > On Mon, May 13, 2019 at 09:24:39PM +0200, Laszlo Ersek wrote: > > > >> On 05/13/19 15:25, Xiaoyu lu wrote: > > > >>> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL > > > >>> OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). > > > >>> So add --with-rand-seed=none to process_files.pl. > > > >>> > > > >>> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl > > > >>> When running process_files.py to configure OpenSSL, we can > > > >>> exclude some > > > unnecessary files. This can reduce porting time, compiling time and library size. > > > >>> > > > >>> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external > > > >>> symbol issue > > > >>> > > > >>> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL > > > >>> Disable warning for building OpenSSL_1_1_1b > > > >>> > > > >>> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b > > > >>> Update OpenSSL submodule to OpenSSL_1_1_1b > > > >>> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) > > > >>> > > > >>> OpenSSL doesn't implement some rand_pool function for UEFI. > > > >>> Use EFI_RNG_PROTOCOL to generate random for entropy. > > > >>> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance > > > >>> counter, but we not sure about the amount of randomness it provides. > > > >>> > > > >>> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward > > > >>> compatible > > > >>> > > > >>> Note: Will be remove next update. > > > >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 > > > >>> Ref: https://github.com/openssl/openssl/pull/4338 > > > >>> > > > >>> > > > >>> Cc: Jian J Wang <jian.j.wang@intel.com> > > > >>> Cc: Ting Ye <ting.ye@intel.com> > > > >> > > > >> I'm withdrawing from reviewing or testing this series. > > > >> > > > >> Gary, if you have the time, can you please regression test this > > > >> (for HTTPS boot) in both OVMF and ArmVirtQemu? > > > >> > > > > I'll find some time to do the regression test tomorrorw. > > > > > > Thanks, Gary! > > > > > > Xiaoyu might post a v4 with a remote topic branch for reviewers to > > > fetch; I suggest awaiting that. (The series is difficult to apply > > > with > > > git-am.) > > > > > > Thanks > > > Laszlo > > > > > > > Cheers, > > > > > > > > Gary Lin > > > > > > > > > > > > > > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#40637): https://edk2.groups.io/g/devel/message/40637 Mute This Topic: https://groups.io/mt/31606972/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 05/13/19 21:24, Laszlo Ersek wrote:
> On 05/13/19 15:25, Xiaoyu lu wrote:
>> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading
>> OpenSSL OpenSSL only support seeding NONE for UEFI(rand_unix.c line
>> 93). So add --with-rand-seed=none to process_files.pl.
>>
>> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in
>> process_files.pl
>> When running process_files.py to configure OpenSSL, we can exclude
>> some unnecessary files. This can reduce porting time, compiling
>> time and library size.
>>
>> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol
>> issue
>>
>> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL
>> Disable warning for building OpenSSL_1_1_1b
>>
>> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b
>> Update OpenSSL submodule to OpenSSL_1_1_1b
>> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687)
>>
>> OpenSSL doesn't implement some rand_pool function for UEFI.
>> Use EFI_RNG_PROTOCOL to generate random for entropy.
>> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance
>> counter, but we not sure about the amount of randomness it
>> provides.
>>
>> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible
>>
>> Note: Will be remove next update.
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792
>> Ref: https://github.com/openssl/openssl/pull/4338
>>
>>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Ting Ye <ting.ye@intel.com>
>
> I'm withdrawing from reviewing or testing this series.
To be clear, the reason I abandoned reviewing / testing this series is
not due to the use of TimerLib as entropy source, in patch #5. I
addressed that separately, stating that I wouldn't review patch #5, only
regression-test it.
The reason I intend to leave upcoming reviews & testing of this series
as a whole to others is that I've found a number of mistakes in relation
to the development workflow. And, it's exhausting for me to repeat all
the same guidelines, when I had documented them in the wiki [*].
At the same time, I realize that it may be difficult for a new edk2
contributor to adhere to everything described in [*] -- especially given
that [*] is not an official edk2 document, just something that I
personally distilled from experience.
In other words, my insisting on [*] in many repeated emails is
exhausting for both new contributors, and myself as a reviewer. Which is
why I thought I'd save us both some busywork, by withdrawing from this
series.
If you'd like me to look over this series again, then a v4 will be
necessary, just in order to remedy the following workflow-level
problems. (Afterwards, a v5 may be necessary for further technical
fixes.)
(1) Some of your patches are authored by "Xiaoyu Lu", some others by
"Xiaoyu lu" (lower case). This messes up the shortlog in the blurb
(and other statistics collected from the git log); you are
represented as two different people.
Please pick *one* email address (name included), and stick with
that. Rebase the series, and use
git commit --amend --author=...
for fixing up the authorship on the patches that need it.
Make sure your Signed-off-by follows suit in the commit messages.
(2) The series is hard to apply for local testing, with "git am", due to
patch #5 modifying both CRLF and LF files. That's not necessarily a
problem with the patch itself, but the norm has been, for
non-trivial patch series, to push a topic branch to a personal repo,
and to reference that repo & branch in the blurb. It permits easy
fetching and easy commenting both.
This wasn't done in v2, and I struggled with "git am". It hasn't
been done in v3 either. Please do it in v4 and further versions.
(3) In my review of the v1 series, I requested that the CC_FLAGS
changes, for the "OpensslLib.inf" and "OpensslLibCrypto.inf" files,
be isolated to their own standalone patch. In v2, this was nicely
addressed in patch #4, and I gave my R-b. In v3 however, you
squashed a totally unrelated -- but at the series level, necessary
-- change into the same patch (namely "sys/syscall.h"). While that
improved the end result of the series for sure, it *negated* the v2
improvement in the specific patch.
In my v2 review, this was how I asked for "sys/syscall.h":
So please include a patch in the v3 series that adds
"CryptoPkg/Library/Include/sys/syscall.h" like suggested above.
*Separate patch*.
If you disagree with my request, that's 100% part of the process,
but then please respond under the request, rather than dumping an
entire new version of the series on me that does not comply with my
request.
(4) In version 3, you failed to pick up my Reviewed-by tags that I had
given for v2 1/6 and v2 6/6.
In more technical terms, this means that you should have run "git
rebase -i", selected the "reword" action for patches v2 1/6 and v2
6/6, and appended -- using the clipboard -- my R-b tags, from my
review emails, to the commit messages.
This is documented in detail, in [*] (contributor step 28).
(Referring to the previous bullet, you also failed to pick up my R-b
for patch v2 4/6. However, ultimately, that was the correct action
for that patch, given that you modified the patch in v3. If a patch
is modified significantly in a revision, then review tags garnered
earlier should be dropped, so that reviewers check the patch again.)
(5) Jian had some questions still open under v2 5/6, when you posted v3.
The questions were addressed to me. Sometimes I cannot answer on the
next day, and yes, there was a weekend to.
If you think a reviewer missed something, please wait one or two
business days, and ping them off-list or on-list, before sending the
next version.
If there isn't enough time left to catch the upcoming stable tag
with this work, then we should postpone this work to the next stable
tag.
[*] https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#40591): https://edk2.groups.io/g/devel/message/40591
Mute This Topic: https://groups.io/mt/31606972/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Thank you, Laszlo.
I am very appreciate to you for being so patient with me .
(1) I cleaned the authored name.
(2) CryptoPkg/Library/Include/openssl/opensslconf.h This file is LF file, It copy from openssl, I think it should not be modified.
Pushed my private repository to https://github.com/xiaoyuxlu/edk2/commits/bz_1089_patch_v4
I have not finished yet. When I finish it, I will push v4 patches
(3) Thank you for explaining this clearly. I changed it back and added a patch.
(4) Now I know I should take R-b tags into commit message and the meaning to modify 'R-b tags patch'.
If I modify it, should refer to R-b tags owner's opinion. I apologize for modify your R-b tags patch which makes you feel bad.
(5) Got it.
I think it is very useful for me.
[*] https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers
Thank you again.
Xiaoyu.
-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek
Sent: Tuesday, May 14, 2019 7:59 PM
To: Lu, XiaoyuX <xiaoyux.lu@intel.com>
Cc: devel@edk2.groups.io; Gary Lin <glin@suse.com>; Wang, Jian J <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
On 05/13/19 21:24, Laszlo Ersek wrote:
> On 05/13/19 15:25, Xiaoyu lu wrote:
>> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading
>> OpenSSL OpenSSL only support seeding NONE for UEFI(rand_unix.c line
>> 93). So add --with-rand-seed=none to process_files.pl.
>>
>> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in
>> process_files.pl
>> When running process_files.py to configure OpenSSL, we can exclude
>> some unnecessary files. This can reduce porting time, compiling
>> time and library size.
>>
>> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol
>> issue
>>
>> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL
>> Disable warning for building OpenSSL_1_1_1b
>>
>> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b
>> Update OpenSSL submodule to OpenSSL_1_1_1b
>> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687)
>>
>> OpenSSL doesn't implement some rand_pool function for UEFI.
>> Use EFI_RNG_PROTOCOL to generate random for entropy.
>> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance
>> counter, but we not sure about the amount of randomness it
>> provides.
>>
>> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible
>>
>> Note: Will be remove next update.
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792
>> Ref: https://github.com/openssl/openssl/pull/4338
>>
>>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Ting Ye <ting.ye@intel.com>
>
> I'm withdrawing from reviewing or testing this series.
To be clear, the reason I abandoned reviewing / testing this series is not due to the use of TimerLib as entropy source, in patch #5. I addressed that separately, stating that I wouldn't review patch #5, only regression-test it.
The reason I intend to leave upcoming reviews & testing of this series as a whole to others is that I've found a number of mistakes in relation to the development workflow. And, it's exhausting for me to repeat all the same guidelines, when I had documented them in the wiki [*].
At the same time, I realize that it may be difficult for a new edk2 contributor to adhere to everything described in [*] -- especially given that [*] is not an official edk2 document, just something that I personally distilled from experience.
In other words, my insisting on [*] in many repeated emails is exhausting for both new contributors, and myself as a reviewer. Which is why I thought I'd save us both some busywork, by withdrawing from this series.
If you'd like me to look over this series again, then a v4 will be necessary, just in order to remedy the following workflow-level problems. (Afterwards, a v5 may be necessary for further technical
fixes.)
(1) Some of your patches are authored by "Xiaoyu Lu", some others by
"Xiaoyu lu" (lower case). This messes up the shortlog in the blurb
(and other statistics collected from the git log); you are
represented as two different people.
Please pick *one* email address (name included), and stick with
that. Rebase the series, and use
git commit --amend --author=...
for fixing up the authorship on the patches that need it.
Make sure your Signed-off-by follows suit in the commit messages.
(2) The series is hard to apply for local testing, with "git am", due to
patch #5 modifying both CRLF and LF files. That's not necessarily a
problem with the patch itself, but the norm has been, for
non-trivial patch series, to push a topic branch to a personal repo,
and to reference that repo & branch in the blurb. It permits easy
fetching and easy commenting both.
This wasn't done in v2, and I struggled with "git am". It hasn't
been done in v3 either. Please do it in v4 and further versions.
(3) In my review of the v1 series, I requested that the CC_FLAGS
changes, for the "OpensslLib.inf" and "OpensslLibCrypto.inf" files,
be isolated to their own standalone patch. In v2, this was nicely
addressed in patch #4, and I gave my R-b. In v3 however, you
squashed a totally unrelated -- but at the series level, necessary
-- change into the same patch (namely "sys/syscall.h"). While that
improved the end result of the series for sure, it *negated* the v2
improvement in the specific patch.
In my v2 review, this was how I asked for "sys/syscall.h":
So please include a patch in the v3 series that adds
"CryptoPkg/Library/Include/sys/syscall.h" like suggested above.
*Separate patch*.
If you disagree with my request, that's 100% part of the process,
but then please respond under the request, rather than dumping an
entire new version of the series on me that does not comply with my
request.
(4) In version 3, you failed to pick up my Reviewed-by tags that I had
given for v2 1/6 and v2 6/6.
In more technical terms, this means that you should have run "git
rebase -i", selected the "reword" action for patches v2 1/6 and v2
6/6, and appended -- using the clipboard -- my R-b tags, from my
review emails, to the commit messages.
This is documented in detail, in [*] (contributor step 28).
(Referring to the previous bullet, you also failed to pick up my R-b
for patch v2 4/6. However, ultimately, that was the correct action
for that patch, given that you modified the patch in v3. If a patch
is modified significantly in a revision, then review tags garnered
earlier should be dropped, so that reviewers check the patch again.)
(5) Jian had some questions still open under v2 5/6, when you posted v3.
The questions were addressed to me. Sometimes I cannot answer on the
next day, and yes, there was a weekend to.
If you think a reviewer missed something, please wait one or two
business days, and ping them off-list or on-list, before sending the
next version.
If there isn't enough time left to catch the upcoming stable tag
with this work, then we should postpone this work to the next stable
tag.
[*] https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#40612): https://edk2.groups.io/g/devel/message/40612
Mute This Topic: https://groups.io/mt/31606972/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.