Re: [edk2-devel] [PATCH v5 0/4] MdePkg: Add MipiSysTLib library
Thanks a lot Mike for your detail reviewing and providing better implementation suggestions! > -Original Message- > From: devel@edk2.groups.io On Behalf Of Michael D > Kinney > Sent: Wednesday, May 10, 2023 9:00 AM > To: Guo, Gua ; devel@edk2.groups.io; Gao, Liming > > Cc: Kinney, Michael D > Subject: Re: [edk2-devel] [PATCH v5 0/4] MdePkg: Add MipiSysTLib library > > Series Reviewed-by: Michael D Kinney > > Liming, this code review started well before the soft freeze. It has now > passed > review. > > We should include this in this stable-tag release. > > Mike > > > > -Original Message- > > From: Guo, Gua > > Sent: Wednesday, May 10, 2023 2:20 AM > > To: devel@edk2.groups.io > > Cc: Kinney, Michael D ; Guo, Gua > > > > Subject: [PATCH v5 0/4] MdePkg: Add MipiSysTLib library > > > > From: Gua Guo > > > > V5: if no other open, it will be final change > > - https://github.com/tianocore/edk2/pull/3901 > > Fix random exception when long run catalog debug message > > > > V4 > > - https://github.com/tianocore/edk2/pull/3901 - Done > > Enhance SwapBytesGuid to use CopyGuid instead of CopyMem, to make > > implement code more simple. > > > > V3 > > - https://github.com/tianocore/edk2/pull/3901 - Done > > - Open: MdeModulePkg/Include/Guid/TraceHubDebugInfoHob.h: why > > MAX_TRACE_HUB_DEBUG_INSTANCE hardcoded to 5? > > Solution: Remove this macro, use Library Constructor to allocate > > it dynamiclly. > > - Open: > > MdeModulePkg/Library/TraceHubDebugSysTLib/InternalTraceHubApiCommon > > .c: SwapBytesGuid () algorithm wrong. > > Solution: Follow correct algorithm to implement it. > > VOID > > EFIAPI > > SwapBytesGuid ( > > IN GUID *Guid,<--- In PreMem, guid is global > > data so region > > is readonly, add output data to support it. > > OUT GUID *ConvertedGuid > > ); > > > > - Open: Merge MSFT and GCC CC_FLAGS as they both supports -D > > Solution: use *_*_*_CC_FLAGS = -DMIPI_SYST_STATIC to unified both. > > > > > > V2 > > - https://github.com/tianocore/edk2/pull/3901 > > - Open: MdeModulePkg/Include/Guid/TraceHubDebugInfoHob.h: why > > MAX_TRACE_HUB_DEBUG_INSTANCE hardcoded to 5? > > - Open: > > MdeModulePkg/Library/TraceHubDebugSysTLib/InternalTraceHubApiCommon > > .c: SwapBytesGuid () algorithm wrong. > > - Open: Merge MSFT and GCC CC_FLAGS as they both supports -D > > > > V1 > > Previous PR: > > - https://github.com/tianocore/edk2/pull/3613 > > - TraceHubDebugLib without submodule - Reject > > > > - https://github.com/tianocore/edk2/pull/3793 > > - TraceHubDebugLib with submodule and without seperate into > > MipiSysTLib and TraceHubDebugLib - Reject > > > > Gua Guo (4): > > MdePkg: Add MipiSysTLib library > > MdePkg: Add NULL library of TraceHubDebugSysTLib > > MdeModulePkg: Add TraceHubDebugSysTLib library > > Maintainers.txt: Update reviewers and maintainers for > > TraceHubDebugLib. > > > > .gitmodules | 11 +- > > .pytool/CISettings.py | 2 + > > Maintainers.txt | 18 + > > .../Include/Guid/TraceHubDebugInfoHob.h | 24 + > > .../BaseTraceHubDebugSysTLib.c| 245 ++ > > .../BaseTraceHubDebugSysTLib.inf | 44 + > > .../DxeSmmTraceHubDebugSysTLib.c | 263 ++ > > .../DxeSmmTraceHubDebugSysTLib.inf| 51 ++ > > .../InternalTraceHubApi.c | 74 ++ > > .../InternalTraceHubApi.h | 37 + > > .../InternalTraceHubApiCommon.c | 200 + > > .../InternalTraceHubApiCommon.h | 119 +++ > > .../PeiTraceHubDebugSysTLib.c | 282 +++ > > .../PeiTraceHubDebugSysTLib.inf | 50 ++ > > .../Library/TraceHubDebugSysTLib/Readme.md| 26 + > > MdeModulePkg/MdeModulePkg.dec | 21 + > > MdeModulePkg/MdeModulePkg.dsc | 3 + > > MdeModulePkg/MdeModulePkg.uni | 18 + > > MdePkg/Include/Library/MipiSysTLib.h | 66 ++ > > MdePkg/Include/Library/TraceHubDebugSysTLib.h | 81 ++ > > MdePkg/Library/MipiSysTLib/GenMipiSystH.py| 132 +++ > > MdePkg/Library/MipiSysTLib/MipiSysTLib.c | 123 +++ > > MdePkg/Library/MipiSysTLib/MipiSysTLib.inf| 52 ++ > > MdePkg/Library/
Re: [edk2-devel] [PATCH v5 0/4] MdePkg: Add MipiSysTLib library
Series Reviewed-by: Michael D Kinney Liming, this code review started well before the soft freeze. It has now passed review. We should include this in this stable-tag release. Mike > -Original Message- > From: Guo, Gua > Sent: Wednesday, May 10, 2023 2:20 AM > To: devel@edk2.groups.io > Cc: Kinney, Michael D ; Guo, Gua > > Subject: [PATCH v5 0/4] MdePkg: Add MipiSysTLib library > > From: Gua Guo > > V5: if no other open, it will be final change > - https://github.com/tianocore/edk2/pull/3901 > Fix random exception when long run catalog debug message > > V4 > - https://github.com/tianocore/edk2/pull/3901 - Done > Enhance SwapBytesGuid to use CopyGuid instead of CopyMem, to make > implement code more simple. > > V3 > - https://github.com/tianocore/edk2/pull/3901 - Done > - Open: MdeModulePkg/Include/Guid/TraceHubDebugInfoHob.h: why > MAX_TRACE_HUB_DEBUG_INSTANCE hardcoded to 5? > Solution: Remove this macro, use Library Constructor to allocate it > dynamiclly. > - Open: > MdeModulePkg/Library/TraceHubDebugSysTLib/InternalTraceHubApiCommon > .c: SwapBytesGuid () algorithm wrong. > Solution: Follow correct algorithm to implement it. > VOID > EFIAPI > SwapBytesGuid ( > IN GUID *Guid,<--- In PreMem, guid is global data > so region > is readonly, add output data to support it. > OUT GUID *ConvertedGuid > ); > > - Open: Merge MSFT and GCC CC_FLAGS as they both supports -D > Solution: use *_*_*_CC_FLAGS = -DMIPI_SYST_STATIC to unified both. > > > V2 > - https://github.com/tianocore/edk2/pull/3901 > - Open: MdeModulePkg/Include/Guid/TraceHubDebugInfoHob.h: why > MAX_TRACE_HUB_DEBUG_INSTANCE hardcoded to 5? > - Open: > MdeModulePkg/Library/TraceHubDebugSysTLib/InternalTraceHubApiCommon > .c: SwapBytesGuid () algorithm wrong. > - Open: Merge MSFT and GCC CC_FLAGS as they both supports -D > > V1 > Previous PR: > - https://github.com/tianocore/edk2/pull/3613 > - TraceHubDebugLib without submodule - Reject > > - https://github.com/tianocore/edk2/pull/3793 > - TraceHubDebugLib with submodule and without seperate into MipiSysTLib > and TraceHubDebugLib - Reject > > Gua Guo (4): > MdePkg: Add MipiSysTLib library > MdePkg: Add NULL library of TraceHubDebugSysTLib > MdeModulePkg: Add TraceHubDebugSysTLib library > Maintainers.txt: Update reviewers and maintainers for > TraceHubDebugLib. > > .gitmodules | 11 +- > .pytool/CISettings.py | 2 + > Maintainers.txt | 18 + > .../Include/Guid/TraceHubDebugInfoHob.h | 24 + > .../BaseTraceHubDebugSysTLib.c| 245 ++ > .../BaseTraceHubDebugSysTLib.inf | 44 + > .../DxeSmmTraceHubDebugSysTLib.c | 263 ++ > .../DxeSmmTraceHubDebugSysTLib.inf| 51 ++ > .../InternalTraceHubApi.c | 74 ++ > .../InternalTraceHubApi.h | 37 + > .../InternalTraceHubApiCommon.c | 200 + > .../InternalTraceHubApiCommon.h | 119 +++ > .../PeiTraceHubDebugSysTLib.c | 282 +++ > .../PeiTraceHubDebugSysTLib.inf | 50 ++ > .../Library/TraceHubDebugSysTLib/Readme.md| 26 + > MdeModulePkg/MdeModulePkg.dec | 21 + > MdeModulePkg/MdeModulePkg.dsc | 3 + > MdeModulePkg/MdeModulePkg.uni | 18 + > MdePkg/Include/Library/MipiSysTLib.h | 66 ++ > MdePkg/Include/Library/TraceHubDebugSysTLib.h | 81 ++ > MdePkg/Library/MipiSysTLib/GenMipiSystH.py| 132 +++ > MdePkg/Library/MipiSysTLib/MipiSysTLib.c | 123 +++ > MdePkg/Library/MipiSysTLib/MipiSysTLib.inf| 52 ++ > MdePkg/Library/MipiSysTLib/Platform.c | 164 > MdePkg/Library/MipiSysTLib/Platform.h | 138 +++ > MdePkg/Library/MipiSysTLib/Readme.md | 25 + > MdePkg/Library/MipiSysTLib/mipi_syst.h| 789 ++ > MdePkg/Library/MipiSysTLib/mipisyst | 1 + > .../TraceHubDebugSysTLibNull.c| 76 ++ > .../TraceHubDebugSysTLibNull.inf | 29 + > MdePkg/MdePkg.ci.yaml | 12 +- > MdePkg/MdePkg.dec | 9 + > MdePkg/MdePkg.dsc | 2 + > ReadMe.rst| 1 + > 34 files changed, 3181 insertions(+), 7 deletions(-) > create mode 100644 MdeModulePkg/Include/Guid/TraceHubDebugInfoHob.h > create mode 100644 > MdeModulePkg/Library/TraceHubDebugSysTLib/BaseTraceHubDebugSysTLib.c > create mode 100644 > MdeModulePkg/Library/TraceHubDebugSysTLib/BaseTraceHubDebugSysTLib.i > nf > create mode 100644 > MdeModulePkg/Library/TraceHubDebugSysTLib/DxeSmmTraceHubDebugSysTL > ib.c > create mode 100644 > MdeModulePkg/Library/TraceHubDebugSysTLib/DxeSmmTraceHubDebugSysTL > ib.inf > create mode 100644 >
[edk2-devel] [PATCH v5 0/4] MdePkg: Add MipiSysTLib library
From: Gua Guo V5: if no other open, it will be final change - https://github.com/tianocore/edk2/pull/3901 Fix random exception when long run catalog debug message V4 - https://github.com/tianocore/edk2/pull/3901 - Done Enhance SwapBytesGuid to use CopyGuid instead of CopyMem, to make implement code more simple. V3 - https://github.com/tianocore/edk2/pull/3901 - Done - Open: MdeModulePkg/Include/Guid/TraceHubDebugInfoHob.h: why MAX_TRACE_HUB_DEBUG_INSTANCE hardcoded to 5? Solution: Remove this macro, use Library Constructor to allocate it dynamiclly. - Open: MdeModulePkg/Library/TraceHubDebugSysTLib/InternalTraceHubApiCommon.c: SwapBytesGuid () algorithm wrong. Solution: Follow correct algorithm to implement it. VOID EFIAPI SwapBytesGuid ( IN GUID *Guid,<--- In PreMem, guid is global data so region is readonly, add output data to support it. OUT GUID *ConvertedGuid ); - Open: Merge MSFT and GCC CC_FLAGS as they both supports -D Solution: use *_*_*_CC_FLAGS = -DMIPI_SYST_STATIC to unified both. V2 - https://github.com/tianocore/edk2/pull/3901 - Open: MdeModulePkg/Include/Guid/TraceHubDebugInfoHob.h: why MAX_TRACE_HUB_DEBUG_INSTANCE hardcoded to 5? - Open: MdeModulePkg/Library/TraceHubDebugSysTLib/InternalTraceHubApiCommon.c: SwapBytesGuid () algorithm wrong. - Open: Merge MSFT and GCC CC_FLAGS as they both supports -D V1 Previous PR: - https://github.com/tianocore/edk2/pull/3613 - TraceHubDebugLib without submodule - Reject - https://github.com/tianocore/edk2/pull/3793 - TraceHubDebugLib with submodule and without seperate into MipiSysTLib and TraceHubDebugLib - Reject Gua Guo (4): MdePkg: Add MipiSysTLib library MdePkg: Add NULL library of TraceHubDebugSysTLib MdeModulePkg: Add TraceHubDebugSysTLib library Maintainers.txt: Update reviewers and maintainers for TraceHubDebugLib. .gitmodules | 11 +- .pytool/CISettings.py | 2 + Maintainers.txt | 18 + .../Include/Guid/TraceHubDebugInfoHob.h | 24 + .../BaseTraceHubDebugSysTLib.c| 245 ++ .../BaseTraceHubDebugSysTLib.inf | 44 + .../DxeSmmTraceHubDebugSysTLib.c | 263 ++ .../DxeSmmTraceHubDebugSysTLib.inf| 51 ++ .../InternalTraceHubApi.c | 74 ++ .../InternalTraceHubApi.h | 37 + .../InternalTraceHubApiCommon.c | 200 + .../InternalTraceHubApiCommon.h | 119 +++ .../PeiTraceHubDebugSysTLib.c | 282 +++ .../PeiTraceHubDebugSysTLib.inf | 50 ++ .../Library/TraceHubDebugSysTLib/Readme.md| 26 + MdeModulePkg/MdeModulePkg.dec | 21 + MdeModulePkg/MdeModulePkg.dsc | 3 + MdeModulePkg/MdeModulePkg.uni | 18 + MdePkg/Include/Library/MipiSysTLib.h | 66 ++ MdePkg/Include/Library/TraceHubDebugSysTLib.h | 81 ++ MdePkg/Library/MipiSysTLib/GenMipiSystH.py| 132 +++ MdePkg/Library/MipiSysTLib/MipiSysTLib.c | 123 +++ MdePkg/Library/MipiSysTLib/MipiSysTLib.inf| 52 ++ MdePkg/Library/MipiSysTLib/Platform.c | 164 MdePkg/Library/MipiSysTLib/Platform.h | 138 +++ MdePkg/Library/MipiSysTLib/Readme.md | 25 + MdePkg/Library/MipiSysTLib/mipi_syst.h| 789 ++ MdePkg/Library/MipiSysTLib/mipisyst | 1 + .../TraceHubDebugSysTLibNull.c| 76 ++ .../TraceHubDebugSysTLibNull.inf | 29 + MdePkg/MdePkg.ci.yaml | 12 +- MdePkg/MdePkg.dec | 9 + MdePkg/MdePkg.dsc | 2 + ReadMe.rst| 1 + 34 files changed, 3181 insertions(+), 7 deletions(-) create mode 100644 MdeModulePkg/Include/Guid/TraceHubDebugInfoHob.h create mode 100644 MdeModulePkg/Library/TraceHubDebugSysTLib/BaseTraceHubDebugSysTLib.c create mode 100644 MdeModulePkg/Library/TraceHubDebugSysTLib/BaseTraceHubDebugSysTLib.inf create mode 100644 MdeModulePkg/Library/TraceHubDebugSysTLib/DxeSmmTraceHubDebugSysTLib.c create mode 100644 MdeModulePkg/Library/TraceHubDebugSysTLib/DxeSmmTraceHubDebugSysTLib.inf create mode 100644 MdeModulePkg/Library/TraceHubDebugSysTLib/InternalTraceHubApi.c create mode 100644 MdeModulePkg/Library/TraceHubDebugSysTLib/InternalTraceHubApi.h create mode 100644 MdeModulePkg/Library/TraceHubDebugSysTLib/InternalTraceHubApiCommon.c create mode 100644 MdeModulePkg/Library/TraceHubDebugSysTLib/InternalTraceHubApiCommon.h create mode 100644 MdeModulePkg/Library/TraceHubDebugSysTLib/PeiTraceHubDebugSysTLib.c create mode 100644 MdeModulePkg/Library/TraceHubDebugSysTLib/PeiTraceHubDebugSysTLib.inf create mode 100644 MdeModulePkg/Library/TraceHubDebugSysTLib/Readme.md create mode 100644
