|
[PATCH V2 0/3] CryptoPkg: Add EC key retrieving and signature interface.
Qi Zhang
This patch is used to retrieve EC key from PEM and X509 and
carry out the EC-DSA signature and verify it. The interface was tested by: 1. DeviceSecurity on edk2-staging https://github.com/tianocore/edk2-staging/tree/DeviceSecurity. 2. Unit test in CryptoPkg/Test REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4102 PR: https://github.com/tianocore/edk2/pull/3464 V2 change: change the protocol version. Cc: Jiewen Yao <jiewen.yao@...> Cc: Jian J Wang <jian.j.wang@...> Cc: Xiaoyu Lu <xiaoyu1.lu@...> Cc: Guomin Jiang <guomin.jiang@...> Signed-off-by: Qi Zhang <qi1.zhang@...> Qi Zhang (3): CryptoPkg: Add EC key retrieving and signature interface. CryptoPkg: Add EC key interface to DXE and protocol CryptoPkg: add unit test for EC key interface. CryptoPkg/Driver/Crypto.c | 143 +++++++++- CryptoPkg/Include/Library/BaseCryptLib.h | 129 +++++++++ .../Pcd/PcdCryptoServiceFamilyEnable.h | 4 + CryptoPkg/Library/BaseCryptLib/Pem/CryptPem.c | 87 ++++++ .../Library/BaseCryptLib/Pem/CryptPemNull.c | 30 ++ CryptoPkg/Library/BaseCryptLib/Pk/CryptEc.c | 258 ++++++++++++++++++ .../Library/BaseCryptLib/Pk/CryptEcNull.c | 82 ++++++ CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 83 ++++++ .../Library/BaseCryptLib/Pk/CryptX509Null.c | 28 ++ .../BaseCryptLibNull/Pem/CryptPemNull.c | 30 ++ .../Library/BaseCryptLibNull/Pk/CryptEcNull.c | 82 ++++++ .../BaseCryptLibNull/Pk/CryptX509Null.c | 28 ++ .../BaseCryptLibOnProtocolPpi/CryptLib.c | 136 +++++++++ CryptoPkg/Private/Protocol/Crypto.h | 131 ++++++++- .../UnitTest/Library/BaseCryptLib/EcTests.c | 156 +++++++++++ 15 files changed, 1405 insertions(+), 2 deletions(-) -- 2.26.2.windows.1 |
||||
|
|
||||
|
Re: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt OpensslLibs
Michael D Kinney
Hi Jiewen,
toggle quoted message
Show quoted text
comments below. Mike -----Original Message-----Yes. This these are implemented in: https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_MergeOptimizedOpensslLibs/CryptoPkg/Library/OpensslLib/SslNull.c https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_MergeOptimizedOpensslLibs/CryptoPkg/Library/OpensslLib/EcSm2Null.c New instances are easy to find because you will get a link failure in CI for missing symbols if BaseCryptLib or TlsLib uses a new API that is only present in OpensslLibFull.inf. When that occurs add the Null version of API that always returns a failure condition. The current implementation before this PR is using OpensslLib class, but the OpensslLib instances are inconsistent in the APIs produced by the lib instances. This is what caused the introduction of #if for EC feature into the layers above OpensslLib. This means the EDK II rules for lib class/lib instance were not followed, and the workaround was to use #if which are also not allowed. The correct fix is to follow EDK II lib class/lib instance rules. All lib instances of the same lib class always produce the same APIs, and then all components that use the lib class can depend on all the services being present without link failures. Yes, Because that hides all the EC related types required to implement the Null EC APIs and the EC types required in the layer above OpensslLib that calls EC services. They will not compile without the EC types being defined.
|
||||
|
|
||||
|
Re: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt OpensslLibs
Yao, Jiewen
I just remember we cannot use macro in BaseCryptoLib. Please ignore my comment on NO_EC macro.
toggle quoted message
Show quoted text
-----Original Message----- |
||||
|
|
||||
|
Re: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt OpensslLibs
Yao, Jiewen
OK. That means we need manual write a wrapper for all EC APIs in Openssl Lib for internal usage.
toggle quoted message
Show quoted text
More precisely, a wrapper for the delta between OpensslLib and OpensslLibFull, right? There will be size difference between those two solutions, because the code other than EC will be include in the function. Why not use NO_EC macro from openssl directly? Thank you Yao Jiewen -----Original Message----- |
||||
|
|
||||
|
Re: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt OpensslLibs
Michael D Kinney
Jiewen,
toggle quoted message
Show quoted text
The BKM is to just call the EC services from the layers above OpensslLib. The EC APIs will always be present. Either real EC service or Null EC service. This way we can remove all the #ifdef on EC PCD. Platform developers select the OpensslLib instance with EC (OpensslLibFull.inf) or without EC (OpensslLib.inf). Mike -----Original Message----- |
||||
|
|
||||
|
Re: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt OpensslLibs
Yao, Jiewen
Hi Mike
toggle quoted message
Show quoted text
For PcdOpensslEcEnabled, I am seeing other usage. For example: https://github.com/qizhangz/edk2/blob/EC_Upstream/CryptoPkg/Library/BaseCryptLib/Pem/CryptPem.c#L156 I think we may need BKM on "how to disable EC". Thank you Yao, Jiewen -----Original Message----- |
||||
|
|
||||
|
Re: [edk2-platforms][PATCH V1 0/2] Platforms/Intel: Build fixes
Isaac Oram
Pushed as 07d0c989089f94133e7b71e38c18d206134863e7
toggle quoted message
Show quoted text
-----Original Message-----
From: Desimone, Nathaniel L <nathaniel.l.desimone@...> Sent: Tuesday, October 11, 2022 4:40 PM To: Oram, Isaac W <isaac.w.oram@...>; devel@edk2.groups.io Cc: Chaganty, Rangasai V <rangasai.v.chaganty@...>; Gao, Liming <gaoliming@...>; Chiu, Chasel <chasel.chiu@...>; Dong, Eric <eric.dong@...>; Benjamin Doron <benjamin.doron00@...> Subject: RE: [edk2-devel][edk2-platforms][PATCH V1 0/2] Platforms/Intel: Build fixes For the series... Reviewed-by: Nate DeSimone <nathaniel.l.desimone@...> -----Original Message----- From: Oram, Isaac W <isaac.w.oram@...> Sent: Wednesday, September 14, 2022 11:40 AM To: devel@edk2.groups.io Cc: Oram, Isaac W <isaac.w.oram@...>; Chaganty, Rangasai V <rangasai.v.chaganty@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Gao, Liming <gaoliming@...>; Chiu, Chasel <chasel.chiu@...>; Dong, Eric <eric.dong@...>; Benjamin Doron <benjamin.doron00@...> Subject: [edk2-devel][edk2-platforms][PATCH V1 0/2] Platforms/Intel: Build fixes The S3FeaturePkg changes in [edk2-platforms][PATCH v3 2/4] S3FeaturePkg: Implement working S3 resume Introduces some build issues with standalone package build for S3FeaturePkg and AdvancedFeaturePkg. There are also some type cast related compiler warnings. We do not currently have continuous integration testing. We do not currently have documented build testing configuration requirements. Therefore I am just fixing the minor issues and intend to merge both patch series together to maintain git bisect to the best of my ability. I do plan to document required and recommended board port and feature pkg builds. Note that the use of UINTN for intermediate data instead of EFI_PHYSICAL_ADDRESS is only to be consistent with other ACPI implementations of similar functionality. Cc: Sai Chaganty <rangasai.v.chaganty@...> Cc: Nate DeSimone <nathaniel.l.desimone@...> Cc: Liming Gao <gaoliming@...> Cc: Chasel Chiu <chasel.chiu@...> Cc: Eric Dong <eric.dong@...> Cc: Benjamin Doron <benjamin.doron00@...> Signed-off-by: Isaac Oram <isaac.w.oram@...> Isaac Oram (2): S3FeaturePkg/Build: Add libraries needed by S3FeaturePkg MinPlatformPkg/S3: Use EFI_PHYSICAL_ADDRESS for address .../Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc | 3 +++ .../Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.c | 10 +++++----- .../PowerManagement/S3FeaturePkg/S3FeaturePkg.dsc | 3 +++ .../Intel/PowerManagement/S3FeaturePkg/S3Pei/S3Pei.c | 2 +- .../Intel/MinPlatformPkg/Include/AcpiS3MemoryNvData.h | 4 ++-- 5 files changed, 14 insertions(+), 8 deletions(-) -- 2.36.1.windows.1 |
||||
|
|
||||
|
Re: [edk2-platforms][PATCH v3 0/4] Implement S3 resume
Isaac Oram
Pushed as ee8d078e39..4d99e03828
toggle quoted message
Show quoted text
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Isaac Oram Sent: Tuesday, October 11, 2022 6:31 PM To: Benjamin Doron <benjamin.doron00@...>; devel@edk2.groups.io Cc: Chaganty, Rangasai V <rangasai.v.chaganty@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sinha, Ankit <ankit.sinha@...>; Chiu, Chasel <chasel.chiu@...>; Gao, Liming <gaoliming@...>; Dong, Eric <eric.dong@...>; Soller, Jeremy <jeremy@...> Subject: Re: [edk2-devel] [edk2-platforms][PATCH v3 0/4] Implement S3 resume Series Reviewed-by: Isaac Oram <Isaac.w.oram@...> -----Original Message----- From: Benjamin Doron <benjamin.doron00@...> Sent: Monday, September 12, 2022 10:13 AM To: devel@edk2.groups.io Cc: Chaganty, Rangasai V <rangasai.v.chaganty@...>; Oram, Isaac W <isaac.w.oram@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sinha, Ankit <ankit.sinha@...>; Chiu, Chasel <chasel.chiu@...>; Gao, Liming <gaoliming@...>; Dong, Eric <eric.dong@...>; Soller, Jeremy <jeremy@...> Subject: [edk2-platforms][PATCH v3 0/4] Implement S3 resume MinPlatform is an open-source EDK2 firmware project that can boot some mainstream boards. However, it lacked working support for S3 resume, an important feature for mobile platforms, which means that its applicability as-is to mainstream use is limited. Therefore, I have now implemented working S3 resume support on MinPlatform. This patch series comprises a majority of one of my work products for GSoC 2022. The BootScript-related modules are EDK2 open-source and fairly straightforward to include. However, the partial dependency PiSmmCommunicationPei (for signalling SMM) creates a dependency on both the SmmAccess and SmmControl PPIs, so these are implemented here as libraries, ported from the DXE drivers. As the register definitions are generic, one library shim is implemented for compatibility, so the library can be generic too. SmmAccess shall be required regardless of SMM signalling, for the LockBox. Like all boots, S3 resume will require working DRAM. To my understanding, we do not need to parse the memory map HOBs again, so some memory is allocated in DXE phase (to reserve it), the address stashed in a variable and consumed on S3 resume flows. Some optimisation can be performed here, regarding how much is necessary. Stashing in a variable is imperfect, but the details must be available without DRAM. As the FSP HOBs are published later, SmmAccess cannot be used to retrieve from the LockBox. Per my suspicions, notes from my mentors, Nate and Ankit, and the coreboot code, the PAMs are opened for access to the AP wake vectors. Presently, all are opened, more research can be performed here. As noted, either FSP or PiSmmCpuDxeSmm can apply boot CPU structures (GDT, IDT, MTRRs, etc). Per my research and findings by my mentors, the closed-source module CpuInitDxe would be required, so this implementation includes CpuS3DataDxe and open-source code will perform this task instead. Unfortunately, board-specific code has some tasks to perform too. I've implemented these for Kabylake, the platform I can test. This includes detecting the boot mode, policy (memory overwrite is contraindicated, do not pass a VBT so FSP does not initialise graphics again) and ensuring a special provision, if desired, for debugging BootScriptExecutorDxe. One major bug that blocked progress was simply specific to debugging. DebugLibReportStatusCode should not be used for that module, because RSC has uninstalled the serial port handler at end-of-BS. Furthermore, it's obvious that the boot services are now unavailable. So, DebugLibSerialPort should be used. As an aside, due to AutoGen ordering, gBS cannot be used in SerialPortInitialize(). What really makes this module a unique case is the fact that it's behaviour is very like runtime drivers. For the integrity of the platform's security, the module is copied to the LockBox at DxeSmmReadyToLock. This caused one major bug that was blocking progress: libraries cannot attempt to modify globals (the data section) with end-of-BS events; they will never reach the true copy. So, rather than using flags to control code flow, it must be coded this way instead. For instance, there is now a special variant of I2cHdmiDebugSerialPortLib that does not use gBS in SerialPortWrite(). Previous revisions of this patch-set tested on my Aspire VN7-572G (Skylake). It's my belief and intention that this implementation be ready for other platforms too (some Intel-specific assumptions made), with a minimum of porting effort, though readying for some debugging is recommended. Some potential bugs include: - Power failure is being set (PMC PWR_FLR), so BootMode == 0x0. This bit is RW/1C, so mitigate it. Until finalised and I close the laptop chassis, I don't know if this is a bug in Kabylake's PchPmcLib. - Very early in testing I saw a memory init error, which means that self-refresh failed. A BaseMemoryTest() predictably failed too, inserted before the PEI core installs memory. Either this was fixed in the code as I finished the implementation, or it's a bug. A major difference in build options is SerialPortSpiFlash -> I2cHdmiDebugSerialPortLib, but this seemed irrelevant. If it's simply the finalised implementation, I think this isn't worth a diff against the reflog. Cc: Sai Chaganty <rangasai.v.chaganty@...> Cc: Isaac Oram <isaac.w.oram@...> Cc: Nate DeSimone <nathaniel.l.desimone@...> Cc: Ankit Sinha <ankit.sinha@...> Cc: Chasel Chiu <chasel.chiu@...> Cc: Liming Gao <gaoliming@...> Cc: Eric Dong <eric.dong@...> Cc: Jeremy Soller <jeremy@...> Signed-off-by: Benjamin Doron <benjamin.doron00@...> Benjamin Doron (4): MinPlatformPkg: Add SmmLockBox to build S3FeaturePkg: Implement working S3 resume MinPlatformPkg: Implement working S3 resume KabylakeOpenBoardPkg: Example of board S3 .../S3FeaturePkg/Include/PostMemory.fdf | 12 ++ .../S3FeaturePkg/Include/PreMemory.fdf | 8 +- .../S3FeaturePkg/Include/S3Feature.dsc | 36 +++- .../S3FeaturePkg/S3Dxe/S3Dxe.c | 155 ++++++++++++++++++ .../S3FeaturePkg/S3Dxe/S3Dxe.inf | 49 ++++++ .../S3FeaturePkg/S3FeaturePkg.dsc | 3 + .../S3FeaturePkg/S3Pei/S3Pei.c | 83 +++++++++- .../S3FeaturePkg/S3Pei/S3Pei.inf | 8 +- .../PeiFspMiscUpdUpdateLib.c | 12 +- .../PeiSaPolicyUpdate.c | 12 +- .../PeiAspireVn7Dash572GInitPreMemLib.c | 38 +++-- .../BoardInitLib/PeiBoardInitPreMemLib.inf | 3 + .../AspireVn7Dash572G/OpenBoardPkg.dsc | 21 +++ .../AspireVn7Dash572G/OpenBoardPkgPcd.dsc | 16 +- .../PeiSiliconPolicyUpdateLib.c | 11 +- .../PeiSiliconPolicyUpdateLib.inf | 1 + .../PeiFspMiscUpdUpdateLib.c | 11 +- .../PeiSaPolicyUpdate.c | 12 +- .../BoardInitLib/PeiBoardInitPreMemLib.inf | 1 + .../BoardInitLib/PeiGalagoPro3InitPreMemLib.c | 27 ++- .../PeiMultiBoardInitPreMemLib.inf | 1 + .../GalagoPro3/OpenBoardPkg.dsc | 15 ++ .../GalagoPro3/OpenBoardPkgPcd.dsc | 2 +- .../PeiFspMiscUpdUpdateLib.c | 12 +- .../PeiSaPolicyUpdate.c | 12 +- .../BoardInitLib/PeiBoardInitPreMemLib.inf | 1 + .../PeiKabylakeRvp3InitPreMemLib.c | 27 ++- .../PeiMultiBoardInitPreMemLib.inf | 1 + .../KabylakeRvp3/OpenBoardPkg.dsc | 12 ++ .../KabylakeRvp3/OpenBoardPkgPcd.dsc | 2 +- .../PeiSiliconPolicyUpdateLib.c | 11 +- .../PeiSiliconPolicyUpdateLib.inf | 1 + .../FspWrapperHobProcessLib.c | 69 +++++++- .../PeiFspWrapperHobProcessLib.inf | 2 + .../Include/AcpiS3MemoryNvData.h | 22 +++ .../Include/Dsc/CoreDxeInclude.dsc | 1 + .../Include/Dsc/CorePeiInclude.dsc | 2 + .../Include/Fdf/CoreOsBootInclude.fdf | 1 + .../Include/Fdf/CorePostMemoryInclude.fdf | 4 + 39 files changed, 665 insertions(+), 52 deletions(-) create mode 100644 Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.c create mode 100644 Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.inf create mode 100644 Platform/Intel/MinPlatformPkg/Include/AcpiS3MemoryNvData.h -- 2.37.2 |
||||
|
|
||||
|
Re: [edk2-platforms][PATCH v3 0/4] Implement S3 resume
Isaac Oram
Series Reviewed-by: Isaac Oram <Isaac.w.oram@...>
toggle quoted message
Show quoted text
-----Original Message-----
From: Benjamin Doron <benjamin.doron00@...> Sent: Monday, September 12, 2022 10:13 AM To: devel@edk2.groups.io Cc: Chaganty, Rangasai V <rangasai.v.chaganty@...>; Oram, Isaac W <isaac.w.oram@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Sinha, Ankit <ankit.sinha@...>; Chiu, Chasel <chasel.chiu@...>; Gao, Liming <gaoliming@...>; Dong, Eric <eric.dong@...>; Soller, Jeremy <jeremy@...> Subject: [edk2-platforms][PATCH v3 0/4] Implement S3 resume MinPlatform is an open-source EDK2 firmware project that can boot some mainstream boards. However, it lacked working support for S3 resume, an important feature for mobile platforms, which means that its applicability as-is to mainstream use is limited. Therefore, I have now implemented working S3 resume support on MinPlatform. This patch series comprises a majority of one of my work products for GSoC 2022. The BootScript-related modules are EDK2 open-source and fairly straightforward to include. However, the partial dependency PiSmmCommunicationPei (for signalling SMM) creates a dependency on both the SmmAccess and SmmControl PPIs, so these are implemented here as libraries, ported from the DXE drivers. As the register definitions are generic, one library shim is implemented for compatibility, so the library can be generic too. SmmAccess shall be required regardless of SMM signalling, for the LockBox. Like all boots, S3 resume will require working DRAM. To my understanding, we do not need to parse the memory map HOBs again, so some memory is allocated in DXE phase (to reserve it), the address stashed in a variable and consumed on S3 resume flows. Some optimisation can be performed here, regarding how much is necessary. Stashing in a variable is imperfect, but the details must be available without DRAM. As the FSP HOBs are published later, SmmAccess cannot be used to retrieve from the LockBox. Per my suspicions, notes from my mentors, Nate and Ankit, and the coreboot code, the PAMs are opened for access to the AP wake vectors. Presently, all are opened, more research can be performed here. As noted, either FSP or PiSmmCpuDxeSmm can apply boot CPU structures (GDT, IDT, MTRRs, etc). Per my research and findings by my mentors, the closed-source module CpuInitDxe would be required, so this implementation includes CpuS3DataDxe and open-source code will perform this task instead. Unfortunately, board-specific code has some tasks to perform too. I've implemented these for Kabylake, the platform I can test. This includes detecting the boot mode, policy (memory overwrite is contraindicated, do not pass a VBT so FSP does not initialise graphics again) and ensuring a special provision, if desired, for debugging BootScriptExecutorDxe. One major bug that blocked progress was simply specific to debugging. DebugLibReportStatusCode should not be used for that module, because RSC has uninstalled the serial port handler at end-of-BS. Furthermore, it's obvious that the boot services are now unavailable. So, DebugLibSerialPort should be used. As an aside, due to AutoGen ordering, gBS cannot be used in SerialPortInitialize(). What really makes this module a unique case is the fact that it's behaviour is very like runtime drivers. For the integrity of the platform's security, the module is copied to the LockBox at DxeSmmReadyToLock. This caused one major bug that was blocking progress: libraries cannot attempt to modify globals (the data section) with end-of-BS events; they will never reach the true copy. So, rather than using flags to control code flow, it must be coded this way instead. For instance, there is now a special variant of I2cHdmiDebugSerialPortLib that does not use gBS in SerialPortWrite(). Previous revisions of this patch-set tested on my Aspire VN7-572G (Skylake). It's my belief and intention that this implementation be ready for other platforms too (some Intel-specific assumptions made), with a minimum of porting effort, though readying for some debugging is recommended. Some potential bugs include: - Power failure is being set (PMC PWR_FLR), so BootMode == 0x0. This bit is RW/1C, so mitigate it. Until finalised and I close the laptop chassis, I don't know if this is a bug in Kabylake's PchPmcLib. - Very early in testing I saw a memory init error, which means that self-refresh failed. A BaseMemoryTest() predictably failed too, inserted before the PEI core installs memory. Either this was fixed in the code as I finished the implementation, or it's a bug. A major difference in build options is SerialPortSpiFlash -> I2cHdmiDebugSerialPortLib, but this seemed irrelevant. If it's simply the finalised implementation, I think this isn't worth a diff against the reflog. Cc: Sai Chaganty <rangasai.v.chaganty@...> Cc: Isaac Oram <isaac.w.oram@...> Cc: Nate DeSimone <nathaniel.l.desimone@...> Cc: Ankit Sinha <ankit.sinha@...> Cc: Chasel Chiu <chasel.chiu@...> Cc: Liming Gao <gaoliming@...> Cc: Eric Dong <eric.dong@...> Cc: Jeremy Soller <jeremy@...> Signed-off-by: Benjamin Doron <benjamin.doron00@...> Benjamin Doron (4): MinPlatformPkg: Add SmmLockBox to build S3FeaturePkg: Implement working S3 resume MinPlatformPkg: Implement working S3 resume KabylakeOpenBoardPkg: Example of board S3 .../S3FeaturePkg/Include/PostMemory.fdf | 12 ++ .../S3FeaturePkg/Include/PreMemory.fdf | 8 +- .../S3FeaturePkg/Include/S3Feature.dsc | 36 +++- .../S3FeaturePkg/S3Dxe/S3Dxe.c | 155 ++++++++++++++++++ .../S3FeaturePkg/S3Dxe/S3Dxe.inf | 49 ++++++ .../S3FeaturePkg/S3FeaturePkg.dsc | 3 + .../S3FeaturePkg/S3Pei/S3Pei.c | 83 +++++++++- .../S3FeaturePkg/S3Pei/S3Pei.inf | 8 +- .../PeiFspMiscUpdUpdateLib.c | 12 +- .../PeiSaPolicyUpdate.c | 12 +- .../PeiAspireVn7Dash572GInitPreMemLib.c | 38 +++-- .../BoardInitLib/PeiBoardInitPreMemLib.inf | 3 + .../AspireVn7Dash572G/OpenBoardPkg.dsc | 21 +++ .../AspireVn7Dash572G/OpenBoardPkgPcd.dsc | 16 +- .../PeiSiliconPolicyUpdateLib.c | 11 +- .../PeiSiliconPolicyUpdateLib.inf | 1 + .../PeiFspMiscUpdUpdateLib.c | 11 +- .../PeiSaPolicyUpdate.c | 12 +- .../BoardInitLib/PeiBoardInitPreMemLib.inf | 1 + .../BoardInitLib/PeiGalagoPro3InitPreMemLib.c | 27 ++- .../PeiMultiBoardInitPreMemLib.inf | 1 + .../GalagoPro3/OpenBoardPkg.dsc | 15 ++ .../GalagoPro3/OpenBoardPkgPcd.dsc | 2 +- .../PeiFspMiscUpdUpdateLib.c | 12 +- .../PeiSaPolicyUpdate.c | 12 +- .../BoardInitLib/PeiBoardInitPreMemLib.inf | 1 + .../PeiKabylakeRvp3InitPreMemLib.c | 27 ++- .../PeiMultiBoardInitPreMemLib.inf | 1 + .../KabylakeRvp3/OpenBoardPkg.dsc | 12 ++ .../KabylakeRvp3/OpenBoardPkgPcd.dsc | 2 +- .../PeiSiliconPolicyUpdateLib.c | 11 +- .../PeiSiliconPolicyUpdateLib.inf | 1 + .../FspWrapperHobProcessLib.c | 69 +++++++- .../PeiFspWrapperHobProcessLib.inf | 2 + .../Include/AcpiS3MemoryNvData.h | 22 +++ .../Include/Dsc/CoreDxeInclude.dsc | 1 + .../Include/Dsc/CorePeiInclude.dsc | 2 + .../Include/Fdf/CoreOsBootInclude.fdf | 1 + .../Include/Fdf/CorePostMemoryInclude.fdf | 4 + 39 files changed, 665 insertions(+), 52 deletions(-) create mode 100644 Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.c create mode 100644 Features/Intel/PowerManagement/S3FeaturePkg/S3Dxe/S3Dxe.inf create mode 100644 Platform/Intel/MinPlatformPkg/Include/AcpiS3MemoryNvData.h -- 2.37.2 |
||||
|
|
||||
|
Re: [PATCH 1/3] UefiCpuPkg: Add Unit tests for DxeCpuExceptionHandlerLib
duntan
Ok, I'll do this in V2 patch. Thanks for your comments.
toggle quoted message
Show quoted text
Thanks, Dun
-----Original Message-----
From: Ni, Ray <ray.ni@...> Sent: Tuesday, October 11, 2022 5:38 PM To: Tan, Dun <dun.tan@...>; devel@edk2.groups.io Cc: Dong, Eric <eric.dong@...>; Kumar, Rahul R <rahul.r.kumar@...> Subject: RE: [PATCH 1/3] UefiCpuPkg: Add Unit tests for DxeCpuExceptionHandlerLib Can you provide more details in the commit message? Especially explain what "consistent" means for #3 and what "CpuStackGuard in both BSP and AP" means in #4? Thanks, Ray -----Original Message----- |
||||
|
|
||||
|
Re: The principles of EDK2 module reconstruction for archs
Chang, Abner
[AMD Official Use Only - General]
Don’t remove it for sure if there is already some use cases. Hah RedfishPkg, I forget this. 😊
From: Kinney, Michael D <michael.d.kinney@...>
There are many instances of both in edk2 repo. I would not remove from spec.
NetworkPkg/SnpDxe/Get_status.c NetworkPkg/SnpDxe/Mcast_ip_to_mac.c NetworkPkg/SnpDxe/Receive_filters.c NetworkPkg/SnpDxe/Station_address.c OvmfPkg/Include/IndustryStandard/Xen/arch-x86/hvm/start_info.h OvmfPkg/Include/IndustryStandard/Xen/arch-x86/xen-x86_32.h OvmfPkg/Include/IndustryStandard/Xen/arch-x86/xen-x86_64.h OvmfPkg/Include/IndustryStandard/Xen/event_channel.h OvmfPkg/Include/IndustryStandard/Xen/grant_table.h OvmfPkg/Include/IndustryStandard/Xen/hvm/hvm_op.h OvmfPkg/Include/IndustryStandard/Xen/io/xs_wire.h OvmfPkg/PlatformCI/iasl_ext_dep.yaml RedfishPkg/Library/JsonLib/jansson_config.h RedfishPkg/Library/JsonLib/jansson_private_config.h
Mike
From: Chang, Abner <Abner.Chang@...>
[AMD Official Use Only - General]
I was thinking to remove ‘_’ from edkII coding standard if there is no use case or rare people like it appears in file/dir name.
Abner
From: Kinney, Michael D <michael.d.kinney@...>
[AMD Official Use Only - General]
I do not have a strong opinion either way.
Some searching indicates there is some debate between use of spaces, underscores, and dashes in filenames.
The filename/dirname requirements for EDK II allow ‘_’ and ‘-‘ as long as they are not the first or last character. Spaces ‘ ‘ are not allowed.
[A-Za-z][_A-Za-z0-9-]*[A-Za-z0-9]+
Mike
From: Chang, Abner <Abner.Chang@...>
[AMD Official Use Only - General]
Removing '_' seems make the folder hard to read, but not too bad to me though. I am fine with removing '_'. Leif and Mike, how do you think?
Ex: Riscv64Ia32X64 compares Riscv64_Ia32_X64. ArmAArch64 compares to Arm_AArch64.
Abner From: Ni, Ray <ray.ni@...>
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> -----Original Message-----
> From: Chang, Abner <Abner.Chang@...> > Sent: Thursday, October 6, 2022 4:37 PM > To: Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io; > quic_llindhol@...; Ni, Ray <ray.ni@...>; Attar, AbdulLateef > (Abdul Lateef) <AbdulLateef.Attar@...>; Sunil V L > <sunilvl@...> > Cc: lichao <lichao@...>; Kirkendall, Garrett > <Garrett.Kirkendall@...>; Grimes, Paul <Paul.Grimes@...>; He, > Jiangang <Jiangang.He@...>; Andrew Fish <afish@...> > Subject: RE: [edk2-devel] The principles of EDK2 module reconstruction for > archs > > [AMD Official Use Only - General] > > Here is the update of the Wiki page for the consistency with edk2 C Coding > Standard Spec. > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fchangab%2Ftianocore.github.io%2Fwiki%2FThe-Guidelines-of-&data=05%7C01%7CAbner.Chang%40amd.com%7C1c6973cf412c4ba09ef908daab2b1ea6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638010498938218357%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=i5RSe41cuzD48VB32KeG0M3Vp7T%2FEqe3ncKNfGCjfIU%3D&reserved=0 > Reconstruct-EDK-II-Modules-for-Processor-Architectures-and-Vendors'- > Implementation > > Thanks > Abner > > > -----Original Message----- > > From: Chang, Abner > > Sent: Wednesday, October 5, 2022 1:39 PM > > To: Kinney, Michael D <michael.d.kinney@...>; > devel@edk2.groups.io; > > quic_llindhol@...; Ni, Ray <ray.ni@...>; Attar, AbdulLateef > > (Abdul Lateef) <AbdulLateef.Attar@...>; Sunil V L > > <sunilvl@...> > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > <Garrett.Kirkendall@...>; Grimes, Paul <Paul.Grimes@...>; > He, > > Jiangang <Jiangang.He@...>; Andrew Fish <afish@...> > > Subject: RE: [edk2-devel] The principles of EDK2 module reconstruction for > > archs > > > > [AMD Official Use Only - General] > > > > PR updated > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore-docs%2Fedk2-&data=05%7C01%7CAbner.Chang%40amd.com%7C1c6973cf412c4ba09ef908daab2b1ea6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638010498938218357%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=YDYXODgrQhuLlP8DTsLr%2F4ct2JH3aw8y2SPg8tk5fgg%3D&reserved=0 > > CCodingStandardsSpecification/pull/2/commits. Please check it. > > > > Thanks > > Abner > > > > > -----Original Message----- > > > From: Kinney, Michael D <michael.d.kinney@...> > > > Sent: Tuesday, October 4, 2022 10:18 PM > > > To: Chang, Abner <Abner.Chang@...>; devel@edk2.groups.io; > > > quic_llindhol@...; Ni, Ray <ray.ni@...>; Attar, > > > AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@...>; Sunil V L > > > <sunilvl@...>; Kinney, Michael D > > > <michael.d.kinney@...> > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > <Garrett.Kirkendall@...>; Grimes, Paul <Paul.Grimes@...>; > > He, > > > Jiangang <Jiangang.He@...>; Andrew Fish <afish@...> > > > Subject: RE: [edk2-devel] The principles of EDK2 module reconstruction > > > for archs > > > > > > Caution: This message originated from an External Source. Use proper > > > caution when opening attachments, clicking links, or responding. > > > > > > > > > I would not add link to Wiki from EDK II C Coding Standard Specification. > > > > > > We want documents published from tianocore-docs to support > standalone > > > formats such as PDF and if there is a link in one of those documents, > > > we want to know that it is a permanent link. I am concerned we may > > > reorganize Wiki pages and links in PDF would become stale. > > > > > > Links from Wiki to specs makes sense. > > > > > > Mike > > > > > > > -----Original Message----- > > > > From: Chang, Abner <Abner.Chang@...> > > > > Sent: Tuesday, October 4, 2022 7:05 AM > > > > To: Kinney, Michael D <michael.d.kinney@...>; > > > > devel@edk2.groups.io; quic_llindhol@...; Ni, Ray > > > > <ray.ni@...>; Attar, AbdulLateef (Abdul Lateef) > > > > <AbdulLateef.Attar@...>; Sunil V L <sunilvl@...> > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > <Garrett.Kirkendall@...>; Grimes, Paul > <Paul.Grimes@...>; > > > > He, Jiangang <Jiangang.He@...>; Andrew Fish > <afish@...> > > > > Subject: RE: [edk2-devel] The principles of EDK2 module > > > > reconstruction for archs > > > > > > > > [AMD Official Use Only - General] > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Kinney, Michael D <michael.d.kinney@...> > > > > > Sent: Tuesday, October 4, 2022 12:54 AM > > > > > To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@...>; > > > > > quic_llindhol@...; Ni, Ray <ray.ni@...>; Attar, > > > > > AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@...>; Sunil V L > > > > > <sunilvl@...>; Kinney, Michael D > > > > > <michael.d.kinney@...> > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > <Paul.Grimes@...>; > > > > > He, Jiangang <Jiangang.He@...>; Andrew Fish > > <afish@...> > > > > > Subject: RE: [edk2-devel] The principles of EDK2 module > > > > > reconstruction for archs > > > > > > > > > > Caution: This message originated from an External Source. Use > > > > > proper caution when opening attachments, clicking links, or > responding. > > > > > > > > > > > > > > > Hi Abner, > > > > > > > > > > responses below. > > > > > > > > > > Mike > > > > > > > > > > > -----Original Message----- > > > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of > > > > > > Chang, Abner via groups.io > > > > > > Sent: Sunday, October 2, 2022 10:37 PM > > > > > > To: Kinney, Michael D <michael.d.kinney@...>; > > > > > > devel@edk2.groups.io; quic_llindhol@...; Ni, Ray > > > > > > <ray.ni@...>; Attar, AbdulLateef (Abdul Lateef) > > > > > > <AbdulLateef.Attar@...>; Sunil V L > > > > > > <sunilvl@...> > > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > <Paul.Grimes@...>; > > > > > He, > > > > > > Jiangang <Jiangang.He@...>; Andrew Fish <afish@...> > > > > > > Subject: Re: [edk2-devel] The principles of EDK2 module > > > > > > reconstruction for archs > > > > > > > > > > > > [AMD Official Use Only - General] > > > > > > > > > > > > Mike, > > > > > > Agree. This can be mentioned on the Wiki page. Also, this would > > > > > > require the discussion between maintainer and contributor. I > > > > > > would say > > > > > maintainer has the responsibility to make sure an arch folder is > > > > > only created when necessary. > > > > > > > > > > Agreed > > > > Ok, I will update Directory and file names section. > > > > > > > > > > > > > > > > > Do you agree with the arch concatenate solution for arch folder? > > > > > > That > > > > > means IA32_X64 instead of X86 (I am fine with this)? > > > > > > > > > > Yes > > > > > > > > > > > However, there is one scenario. Assume there were two arch > > > > > > folders > > > > > > IA32_X64 and RISCV64. Then ARM shares the code with IA32_X64, > we > > > > > > may > > > > > rename IA32_X64 to IA32_X64_ARM. > > > > > > Although the directory naming is not real a problem to the > > > > > > build, a separate ARM folder seems easier? Or we can just allow > > > > > > this kind of folder > > > > > naming structure, however we let maintainer to make the decision? > > > > > > > > > > I would let the maintainer make the decision. For your example, > > > > > another approach may be to move the IA32_X64 content up a level so > > > > > it is common and is used by IA32, X64, ARM. And leave RISCV64 > > > > > folder for an arch that has special requirements. I think having > > > > > some flexibility in the guidelines is very beneficial. The main > > > > > goal is for consistency when a specific guideline is followed. > > > > I think we can have the naming rules in the edk2 C coding standard > > > > spec and > > > put these guidelines on the Wiki page. > > > > Is that ok to have a link to Wiki page in the edk2 C coding standard spec? > > > > > > > > Abner > > > > > > > > > > > > > > > > > > > > > Abner > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > From: Kinney, Michael D <michael.d.kinney@...> > > > > > > > Sent: Monday, October 3, 2022 1:18 PM > > > > > > > To: Chang, Abner <Abner.Chang@...>; > > devel@edk2.groups.io; > > > > > > > quic_llindhol@...; Ni, Ray <ray.ni@...>; Attar, > > > > > > > AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@...>; Sunil > > > > > > > V L <sunilvl@...>; Kinney, Michael D > > > > > > > <michael.d.kinney@...> > > > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > <Paul.Grimes@...>; He, Jiangang > <Jiangang.He@...>; > > > > > > > Andrew Fish <afish@...> > > > > > > > Subject: RE: [edk2-devel] The principles of EDK2 module > > > > > > > reconstruction for archs > > > > > > > > > > > > > > Caution: This message originated from an External Source. Use > > > > > > > proper caution when opening attachments, clicking links, or > > responding. > > > > > > > > > > > > > > > > > > > > > Abner, > > > > > > > > > > > > > > I think another guideline is to minimize the number of > > subdirectories. > > > > > > > > > > > > > > Only create them if it helps with the organization and long > > > > > > > term maintenance of the component. > > > > > > > > > > > > > > Mike > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > From: Chang, Abner <Abner.Chang@...> > > > > > > > > Sent: Sunday, October 2, 2022 8:13 PM > > > > > > > > To: Kinney, Michael D <michael.d.kinney@...>; > > > > > > > > devel@edk2.groups.io; quic_llindhol@...; Ni, Ray > > > > > > > > <ray.ni@...>; Attar, AbdulLateef (Abdul Lateef) > > > > > > > > <AbdulLateef.Attar@...>; Sunil V L > > > > > > > > <sunilvl@...> > > > > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > > > > <Paul.Grimes@...>; > > > > > > > He, > > > > > > > > Jiangang <Jiangang.He@...>; Andrew Fish > > > > > > > > <afish@...> > > > > > > > > Subject: RE: [edk2-devel] The principles of EDK2 module > > > > > > > > reconstruction for archs > > > > > > > > > > > > > > > > [AMD Official Use Only - General] > > > > > > > > > > > > > > > > Hi Mike and Leif, > > > > > > > > First of all, we don't use arch folder if the module is > > > > > > > > mainly for a specific arch in obviously. So we will have > > > > > > > > both common and arch-specific > > > > > > > files constructed under module/library root. > > > > > > > > > > > > > > > > > > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2F > > > > > ed > > > > > > > k > > > > > > > 2 > > > > > > > > > > > > > > > > > > .groups.io%2Fg%2Fdevel%2Fmessage%2F94567&data=05%7C01%7C > A > > > > > > > bner.Chan > > > > > > > > > > > > > > > > > > > > > > > > > > g%40amd.com%7Cd49cbbe6d3d942bd69a308daa4fea41b%7C3dd8961fe4884 > > > > > > > e608e11a > > > > > > > > > > > > > > > > > > > > > > > > > > 82d994e183d%7C0%7C0%7C638003710850252776%7CUnknown%7CTWFpbGZ > > > > > > > sb3d8eyJWI > > > > > > > > > > > > > > > > > > > > > > > > > > joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3 > > > > > > > 000%7 > > > > > > > > > > > > > > > > > > > > > > > C%7C%7C&sdata=eiLOC0G9WZWKqm2ALcAiKr7SPBP5AgDdAxogHlv5pI > > > > > > > M%3D&r > > > > > > > > eserved=0 > > > > > > > > SmmCpuFeatureLib\Ia32 > > > > > > > > SmmCpuFeatureLib\X64 > > > > > > > > SmmCpuFeatureLib\SmmCpuFeatureLib.c > > > > > > > > SmmCpuFeatureLib\SmmCpuFeatureLibCommon.c > > > > > > > > SmmCpuFeatureLib\IntelSmmCPuFeaturesLib > > > > > > > > SmmCpuFeatureLib\AmdlSmmCPuFeaturesLib > > > > > > > > > > > > > > > > > > > > > > > > > > Could we concatenate architectures? > > > > > > > > > > I.e. AARCH64_ARM, IA32_X64, AARCH64_RISCV64... ? > > > > > > > > Looks like below? > > > > > > > > > > > > > > > > CpuDxe\IA32_X64\IA32\abc.nasm > > CpuDxe\IA32_X64\X64\abc.nasm > > > > > > > > CpuDxe\IA32_X64\CpuDxe.c CpuDxe\IA32_X64\AmdCpuDxe.c > > > > > > > > CpuDxe\IA32_X64\IntelCpuDxe.c CpuDxe\RiscV64\CpuDxe.c > > > > > > > > CpuDxe\ARM\CpuDxe.c CpuDxe\ > > > > > > > > CpuDxeCommon.c // If required. > > > > > > > > CpuDxe.inf // Use INF section arch modifier for > X86, > > > > > RISC-V > > > > > > > and ARM. > > > > > > > > CpuDxeAmd.inf // Separate INF for AMD. > > > > > > > > > > > > > > > > Seems ok, but is AARCH64_RISCV64 a real case? Or it means > > > > > > > > we can create a folder "AARCH64_RISCV64" when there are > some > > > > > > > > common > > > > > files > > > > > > > shared by AARCH64 and RISCV64? > > > > > > > > > > > > > > > > For the 32/64 bit support, it can still stay under module > > > > > > > > root and don't need to assign a folder for them unless the > > > > > > > > arch has the different > > > > > > > implementation. > > > > > > > > Regards, > > > > > > > > Abner > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > From: Kinney, Michael D <michael.d.kinney@...> > > > > > > > > > Sent: Saturday, October 1, 2022 2:52 AM > > > > > > > > > To: devel@edk2.groups.io; quic_llindhol@...; > > > > > > > > > Chang, Abner <Abner.Chang@...>; Ni, Ray > > > > > > > > > <ray.ni@...>; Attar, AbdulLateef (Abdul Lateef) > > > > > > > > > <AbdulLateef.Attar@...>; Sunil V L > > > > > > > > > <sunilvl@...>; Kinney, Michael D > > > > > > > > > <michael.d.kinney@...> > > > > > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > <Paul.Grimes@...>; He, Jiangang > > <Jiangang.He@...>; > > > > > > > > > Andrew Fish <afish@...> > > > > > > > > > Subject: RE: [edk2-devel] The principles of EDK2 module > > > > > > > > > reconstruction for archs > > > > > > > > > > > > > > > > > > Caution: This message originated from an External Source. > > > > > > > > > Use proper caution when opening attachments, clicking > > > > > > > > > links, or > > > > > responding. > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Leif, > > > > > > > > > > > > > > > > > > Concatenation is a good idea. Makes it more obvious and > > > > > > > > > can be easily adopted for new CPU archs. > > > > > > > > > > > > > > > > > > There is an additional case where an implementation does > > > > > > > > > not have differences based on CPU Arch or Vendor, but it > > > > > > > > > does have differences based on the bit- width of the CPU. > > > > > > > > > Take BaseSafeIntLib as > > > > > > > an example. > > > > > > > > > It has source files for 32-bit CPUs, 64-bit CPUs, and CPU > > > > > > > > > arch specific file for Ebc because Ebc adapts to 32-bit or > > > > > > > > > 64-bit depending on the CPU type the EBC Interpreter is > running. > > > > > > > > > > > > > > > > > > MdePkg/Library/BaseSafeIntLib/ > > > > > > > > > BaseSafeIntLib.inf > > > > > > > > > SafeIntLib.c > > > > > > > > > SafeIntLib32.c > > > > > > > > > SafeIntLib64.c > > > > > > > > > SafeIntLibEbc.c > > > > > > > > > > > > > > > > > > Should we add "32" and "64" as supported suffices in file > names? > > > > > > > > > > > > > > > > > > If we wanted to put type content into a subdirectory, what > > > > > > > > > would be the suggested directory name for "32" and "64". > > > > > > > > > Or do we want to require this type of difference to always > > > > > > > > > be files in top level directory of > > > > > > > the modules/library? > > > > > > > > > > > > > > > > > > Best regards, > > > > > > > > > > > > > > > > > > Mike > > > > > > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On > > > > > > > > > > Behalf Of Leif Lindholm > > > > > > > > > > Sent: Friday, September 30, 2022 9:09 AM > > > > > > > > > > To: devel@edk2.groups.io; Kinney, Michael D > > > > > > > > > > <michael.d.kinney@...>; Chang, Abner > > > > > > > <Abner.Chang@...>; > > > > > > > > > > Ni, Ray <ray.ni@...>; Attar, AbdulLateef (Abdul > > > > > > > > > > Lateef) <AbdulLateef.Attar@...>; Sunil V L > > > > > > > > > > <sunilvl@...> > > > > > > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > <Paul.Grimes@...>; > > > > > > > > > > He, Jiangang <Jiangang.He@...>; Andrew Fish > > > > > > > <afish@...> > > > > > > > > > > Subject: Re: [edk2-devel] The principles of EDK2 module > > > > > > > > > > reconstruction for archs > > > > > > > > > > > > > > > > > > > > I agree similar things will certainly happen for > > > > > > > > > > ARM/AARCH64, which will probably be noticeable when I > > > > > > > > > > start eradicating ArmPkg and putting the arch-standard > > > > > > > > > > bits into > > > (mostly) MdePkg. > > > > > > > > > > > > > > > > > > > > But I like the ability to see already at the filesystem > > > > > > > > > > level which files belong to the architecture I'm > > > > > > > > > > currently working on and > > > > > > > which don't. > > > > > > > > > > > > > > > > > > > > Could we concatenate architectures? > > > > > > > > > > I.e. AARCH64_ARM, IA32_X64, AARCH64_RISCV64... ? > > > > > > > > > > > > > > > > > > > > / > > > > > > > > > > Leif > > > > > > > > > > > > > > > > > > > > On 2022-09-30 08:28, Michael D Kinney wrote: > > > > > > > > > > > Hi Abner, > > > > > > > > > > > > > > > > > > > > > > One comment is on adding a new CPU type dir name of > 'X86' > > > > > > > > > > > for content that is common for IA32/X64. > > > > > > > > > > > > > > > > > > > > > > I can imagine a similar case for ARM/AARCH64 and for > > > > > > > > > > > the RISCV (32, 64, 128) cases. > > > > > > > > > > > > > > > > > > > > > > I think I would prefer to drop X86 and if there are > > > > > > > > > > > files that are common to multiple CPU architectures > > > > > > > > > > > then they are considered common and are in top > > > > > > > > > > > directory of module and the file header and INF file > > > > > > > > > > > can clearly document which CPU archs the file > > > > > > > applies. > > > > > > > > > > > > > > > > > > > > > > Mike > > > > > > > > > > > > > > > > > > > > > >> -----Original Message----- > > > > > > > > > > >> From: Chang, Abner <Abner.Chang@...> > > > > > > > > > > >> Sent: Friday, September 30, 2022 12:11 AM > > > > > > > > > > >> To: Ni, Ray <ray.ni@...>; Attar, AbdulLateef > > > > > > > > > > >> (Abdul > > > > > > > > > > >> Lateef) <AbdulLateef.Attar@...>; Sunil V L > > > > > > > > > > >> <sunilvl@...>; devel@edk2.groups.io; > > > > > > > > > > >> Kinney, Michael D <michael.d.kinney@...> > > > > > > > > > > >> Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > > >> <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > > >> <Paul.Grimes@...>; He, Jiangang > > > > > <Jiangang.He@...>; > > > > > > > Leif > > > > > > > > > > >> Lindholm <quic_llindhol@...>; Andrew Fish > > > > > > > > > > >> <afish@...> > > > > > > > > > > >> Subject: RE: [edk2-devel] The principles of EDK2 > > > > > > > > > > >> module reconstruction for archs > > > > > > > > > > >> > > > > > > > > > > >> [AMD Official Use Only - General] > > > > > > > > > > >> > > > > > > > > > > >> Thanks Ray, here are my responses. > > > > > > > > > > >> https://nam11.safelinks.protection.outlook.com/?url=h > > > > > > > > > > >> tt > > > > > > > > > > >> ps%3 > > > > > > > > > > >> A%2F > > > > > > > > > > >> %2Fg > > > > > > > > > > >> ithub.com%2Ftianocore-docs%2Fedk2- > > > > > > > CCodingStandardsSpecification > > > > > > > > > > >> %2Fp > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > ull%2F2&data=05%7C01%7CAbner.Chang%40amd.com%7C7c3292c8bd2 > > > > > > > f4 > > > > > > > > > 86f > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > 920908daa314e8e6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6 > > > > > > > > > 3800 > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > 1607445876768%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLC > > > > > > > JQ > > > > > > > > > IjoiV > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata > > > > > > > = > > > > > > > > > HAq > > > > > > > > > > >> > > > > > > > > > > > > > > ou4JyY1yxDthuQ1dEKcF7Q3o4W77Oo9rOOvkXNWU%3D&reserved=0 > > > > > > > > > > >> > > > > > > > > > > >> @Kinney, Michael D we may also need your > > > > > > > > > > >> clarification on the > > > > > > > comments. > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >>> -----Original Message----- > > > > > > > > > > >>> From: Ni, Ray <ray.ni@...> > > > > > > > > > > >>> Sent: Thursday, September 29, 2022 3:42 PM > > > > > > > > > > >>> To: Attar, AbdulLateef (Abdul Lateef) > > > > > > > > > > >>> <AbdulLateef.Attar@...>; Chang, Abner > > > > > > > > > > >>> <Abner.Chang@...>; Sunil V L > > > > > > > > > > >>> <sunilvl@...>; devel@edk2.groups.io > > > > > > > > > > >>> Cc: Kinney, Michael D <michael.d.kinney@...>; > > > > > > > > > > >>> lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > > >>> <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > > >>> <Paul.Grimes@...>; He, Jiangang > > > > > <Jiangang.He@...>; > > > > > > > > > > >>> Leif Lindholm <quic_llindhol@...>; Andrew > > > > > > > > > > >>> Fish <afish@...> > > > > > > > > > > >>> Subject: RE: [edk2-devel] The principles of EDK2 > > > > > > > > > > >>> module reconstruction for archs > > > > > > > > > > >>> > > > > > > > > > > >>> Caution: This message originated from an External > Source. > > > > > > > > > > >>> Use proper caution when opening attachments, > > > > > > > > > > >>> clicking links, or > > > > > > > responding. > > > > > > > > > > >>> > > > > > > > > > > >>> > > > > > > > > > > >>> Abner, > > > > > > > > > > >>> Comments in > > > > > > > > > > >>> https://nam11.safelinks.protection.outlook.com/?url= > > > > > > > > > > >>> ht > > > > > > > > > > >>> tps% > > > > > > > > > > >>> 3A%2 > > > > > > > > > > >>> F%2F > > > > > > > > > > >>> gith > > > > > > > > > > >>> ub.com%2Ftianocore-docs%2Fedk2- > > > > > > > > > > >>> > CCodingStandardsSpecification%2Fpull%2F2%23pullreque > > > > > > > > > > >>> st > > > > > > > > > > >>> revi > > > > > > > > > > >>> ew- > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 1124763311&data=05%7C01%7CAbner.Chang%40amd.com%7Cd825e24 > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 8625541e3f43e08daa1ee2883%7C3dd8961fe4884e608e11a82d994e183d%7C0 > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > %7C0%7C638000341502885565%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM > C > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000% > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > 7C%7C%7C&sdata=RXxgpbEr6ivbqP1R6%2B3Rxl%2ByJAnaUJuaYYKdfCH > > > > > > > > > > >>> 8jo8%3D&reserved=0 > > > > > > > > > > >>> > > > > > > > > > > >>> We can discuss more in tomorrow's meeting. > > > > > > > > > > >>> > > > > > > > > > > >>> > > > > > > > > > > >>>> -----Original Message----- > > > > > > > > > > >>>> From: Attar, AbdulLateef (Abdul Lateef) > > > > > > > > > > >>>> <AbdulLateef.Attar@...> > > > > > > > > > > >>>> Sent: Thursday, September 29, 2022 3:11 PM > > > > > > > > > > >>>> To: Chang, Abner <Abner.Chang@...>; Sunil V L > > > > > > > > > > >>>> <sunilvl@...>; devel@edk2.groups.io; > > > > > > > > > > >>>> Ni, Ray <ray.ni@...> > > > > > > > > > > >>>> Cc: Kinney, Michael D <michael.d.kinney@...>; > > > > > > > > > > >>>> lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > > >>>> <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > > >>>> <Paul.Grimes@...>; > > > > > > > > > > >>> He, > > > > > > > > > > >>>> Jiangang <Jiangang.He@...>; Leif Lindholm > > > > > > > > > > >>>> <quic_llindhol@...>; Andrew Fish > > > > > > > > > > >>>> <afish@...> > > > > > > > > > > >>>> Subject: RE: [edk2-devel] The principles of EDK2 > > > > > > > > > > >>>> module reconstruction for archs > > > > > > > > > > >>>> > > > > > > > > > > >>>> Hi Abner, > > > > > > > > > > >>>> Looks good to me. > > > > > > > > > > >>>> Reviewed-by: Abdul Lateef Attar <abdattar@...> > > > > > > > > > > >>>> > > > > > > > > > > >>>> Thanks > > > > > > > > > > >>>> AbduL > > > > > > > > > > >>>> > > > > > > > > > > >>>> -----Original Message----- > > > > > > > > > > >>>> From: Chang, Abner <Abner.Chang@...> > > > > > > > > > > >>>> Sent: 28 September 2022 20:31 > > > > > > > > > > >>>> To: Sunil V L <sunilvl@...>; > > > > > > > > > > >>>> devel@edk2.groups.io; ray.ni@... > > > > > > > > > > >>>> Cc: Kinney, Michael D <michael.d.kinney@...>; > > > > > > > > > > >>>> lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > > >>>> <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > > >>>> <Paul.Grimes@...>; > > > > > > > > > > >>> He, > > > > > > > > > > >>>> Jiangang <Jiangang.He@...>; Attar, AbdulLateef > > > > > > > > > > >>>> (Abdul > > > > > > > > > > >>>> Lateef) <AbdulLateef.Attar@...>; Leif Lindholm > > > > > > > > > > >>>> <quic_llindhol@...>; Andrew Fish > > > > > > > > > > >>>> <afish@...> > > > > > > > > > > >>>> Subject: RE: [edk2-devel] The principles of EDK2 > > > > > > > > > > >>>> module reconstruction for archs > > > > > > > > > > >>>> > > > > > > > > > > >>>> [AMD Official Use Only - General] > > > > > > > > > > >>>> > > > > > > > > > > >>>> I just had created PR to update edkII C coding > > > > > > > > > > >>>> standard spec for the file and directory naming. We > > > > > > > > > > >>>> can review and confirm this update first and then > > > > > > > > > > >>>> go back to the principles of EDK2 module > > > > > > > > > reconstruction for archs. > > > > > > > > > > >>>> Here is the PR: > > > > > > > > > > >>>> > > > > > > > > > > >>> https://nam11.safelinks.protection.outlook.com/?url= > > > > > > > > > > >>> ht > > > > > > > > > > >>> tps% > > > > > > > > > > >>> 3A%2 > > > > > > > > > > >>> F%2F > > > > > > > > > > >>> gith > > > > > > > > > > >>>> ub.com%2Ftianocore-docs%2Fedk2- > > > > > > > > > > >>> &data=05%7C01%7CAbner.Chang%40amd.c > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > om%7Cd825e248625541e3f43e08daa1ee2883%7C3dd8961fe4884e608e11a82 > > > > > > > > > > >>> d994e18 > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 3d%7C0%7C0%7C638000341502885565%7CUnknown%7CTWFpbGZsb3d8eyJ > > > > > > > > > > >>> WIjoiMC4wLj > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C% > > > > > > > > > > >>> 7C%7C&a > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > mp;sdata=X4z9puj81nIGTqtMxC9igDZyBPOT6OTWSU%2BjoIowo%2BE%3D&a > > > > > > > > > > >>> mp;reserv > > > > > > > > > > >>>> ed=0 > > > > > > > > > > >>>> CCodingStandardsSpecification/pull/2 > > > > > > > > > > >>>> > > > > > > > > > > >>>> The naming rule is mainly for the new module or new > file > > IMO. > > > > > > > > > > >>>> Some existing module may not meet the guidelines > > > > > > > > > > >>>> mentioned in this > > > > > > > > > spec. > > > > > > > > > > >>>> Thus we need the principles of EDK2 module > > > > > > > > > > >>>> reconstruction on the existing module to support > > > > > > > > > > >>>> other processor archs and not impacting the > > > > > > > > > > >>> existing platforms (e.g. > > > > > > > > > > >>>> rename the INF file or directory to meet the > guidelines). > > > > > > > > > > >>>> > > > > > > > > > > >>>> Sunil, seems RISC-V CpuDxe meet the guideline. > > > > > > > > > > >>>> Please check > > > > > it. > > > > > > > > > > >>>> Just feel that having CpuDxe.c to Riscv64 folder > > > > > > > > > > >>>> is not quite a best solution. I think at least we > > > > > > > > > > >>>> can abstract the protocol structure and protocol > > > > > > > > > > >>>> installation under CpuDxe\ and have the arch > > > > > > > > > > >>>> implementation under arch folder. We can discuss > > > > > > > > > > >>>> this later after we confirming the > > > > > > > > > > >>> guideline and principles. > > > > > > > > > > >>>> > > > > > > > > > > >>>> Thanks > > > > > > > > > > >>>> Abner > > > > > > > > > > >>>> > > > > > > > > > > >>>>> -----Original Message----- > > > > > > > > > > >>>>> From: Sunil V L <sunilvl@...> > > > > > > > > > > >>>>> Sent: Wednesday, September 28, 2022 3:34 PM > > > > > > > > > > >>>>> To: devel@edk2.groups.io; ray.ni@... > > > > > > > > > > >>>>> Cc: Chang, Abner <Abner.Chang@...>; Kinney, > > > > > Michael > > > > > > > > > > >>>>> D <michael.d.kinney@...>; lichao > > > > > > > > > > >>>>> <lichao@...>; Kirkendall, Garrett > > > > > > > > > > >>>>> <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > > >>>>> <Paul.Grimes@...>; He, Jiangang > > > > > > > > > > >>>>> <Jiangang.He@...>; Attar, AbdulLateef (Abdul > > > > > > > > > > >>>>> Lateef) <AbdulLateef.Attar@...>; Leif > Lindholm > > > > > > > > > > >>>>> <quic_llindhol@...>; Andrew Fish > > > > > > > > > > >>>>> <afish@...> > > > > > > > > > > >>>>> Subject: Re: [edk2-devel] The principles of EDK2 > > > > > > > > > > >>>>> module reconstruction for archs > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> Caution: This message originated from an External > > Source. > > > > > > > > > > >>>>> Use proper caution when opening attachments, > > > > > > > > > > >>>>> clicking links, or > > > > > > > responding. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> On Wed, Sep 28, 2022 at 03:33:45AM +0000, Ni, Ray > > wrote: > > > > > > > > > > >>>>> Hi Ray, > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> 1. When a new arch's implementation is > > > > > > > > > > >>>>>> introduced to the existing > > > > > > > > > > >>>>> module which was developed for the specific arch: > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> 1. The folder reconstruction: > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> * Create arch folder for the existing arch > > implementation > > > > > > > > > > >>>>>> [Ray] Do you move existing arch implementation to > > > > > > > > > > >>>>>> that arch > > > > > > > folder? > > > > > > > > > > >>>>>> It will > > > > > > > > > > >>>>> break existing platforms a lot. > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> * Create the arch folder for the new introduced > > arch > > > > > > > > > > >>>>>> [Ray] I agree. But if we don't create arch folder > > > > > > > > > > >>>>>> for existing arch > > > > > > > > > > >>>>> implementation, the pkg layout will be a mess. > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> [Ray] Hard for me to understand all the principles > here. > > > > > > > > > > >>>>>> Maybe we review > > > > > > > > > > >>>>> existing code including to-be-upstreamed code and > > > > > > > > > > >>>>> decide how > > > > > > > to go. > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> Could you please take a look below changes which > > > > > > > > > > >>>>> is trying to add RISC-V support for CpuDxe? > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> https://nam11.safelinks.protection.outlook.com/?url= > > > > > > > > > > >>> ht > > > > > > > > > > >>> tps% > > > > > > > > > > >>> 3A%2 > > > > > > > > > > >>> F%2F > > > > > > > > > > >>> gith > > > > > > > > > > >>>>> ub.com%2Ftianocore%2Fedk2- > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > staging%2Fcommit%2Fbba1a11be47dd091734e185afbed73ea75708749& > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > data=05%7C01%7Cabner.chang%40amd.com%7Ca419e6a010d34fde464b08d > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > aa123e080%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63799947 > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 2732494527%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sd > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > ata=Vq6pJLnn8yJrJhFZn7LfLbZzrtpG4n1VLWgAil6J38U%3D&reserved=0 > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> https://nam11.safelinks.protection.outlook.com/?url= > > > > > > > > > > >>> ht > > > > > > > > > > >>> tps% > > > > > > > > > > >>> 3A%2 > > > > > > > > > > >>> F%2F > > > > > > > > > > >>> gith > > > > > > > > > > >>>>> ub.com%2Ftianocore%2Fedk2- > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > staging%2Fcommit%2F7fccf92a97a6d0618a20f10622220e78b3687906&da > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > ta=05%7C01%7Cabner.chang%40amd.com%7Ca419e6a010d34fde464b08daa1 > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 23e080%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63799947273 > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 2494527%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > =xFmvUv58vh4AUAM17Qy9G5jZWFZlK2Ozt3njpG1e8%2BY%3D&reserv > > > > > > > > > > >>>>> ed=0 > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> What do you suggest with above example? > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> 1) Common INF for all architectures - but modify > > > > > > > > > > >>>>> INF alone, no > > > > > > > > > > >>>>> X86 folder creation. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> This is what I have done in the commit above. May > > > > > > > > > > >>>>> be of least impact to existing code since it is only INF > > change. > > > > > > > > > > >>>>> But like you mentioned this is bit weird that X86 > > > > > > > > > > >>>>> files will remain in root folder directly along > > > > > > > > > > >>>>> with some common > > > > > files. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> 2) Common INF (CpuDxe.inf) + create arch folders > > > > > > > > > > >>>>> X86, X64, IA32, > > > > > > > > > > >>>>> RiscV64 etc > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> IMO, this is probably the best approach. What > > > > > > > > > > >>>>> would be the challenges with this? > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> 3) Separate INF for arch like CpuDxe.inf for x86, > > > > > > > > > > >>>>> CpuDxeRiscV64.inf for > > > > > > > > > > >>>> RISC-V. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> This again probably is not a good idea. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> 4) If the module/library is specific to one arch (ex: > > > > > > > > > > >>>>> SMM(X86), SBI(RISC-V)), then create separate INF. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> Thanks! > > > > > > > > > > >>>>> Sunil > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > |
||||
|
|
||||
|
Re: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt OpensslLibs
Michael D Kinney
Hi Jiewen,
toggle quoted message
Show quoted text
Comments below. Mike -----Original Message-----Good catch. I will fix. This was added since I started this work and was added back in by rebase. I will fix. We can just remove the check for the PCD. If the OpensslLib instance does not include SSL services, then the Null SSL services are present and the call to SSL_ctrl() will return 0 which will force TlsSetEcCurve() to return EFI_UNSUPPORTED. It will also ASSERT() informing the developer that a call to a service that depends on SSL was made without SSL services available. long SSL_ctrl ( SSL *ssl, int cmd, long larg, void *parg ) { ASSERT (FALSE); return 0; } Likewise, if the OpensslLib instance does not support EC services, then the Null EC services will be included and the call to EC_KEY_new_by_curve_name() will return NULL which will force TlsSetEcCurve() to return EFI_UNSUPPORTED. It will also ASSERT() informing the developer that a call to a service that depends on EC was made without EC services available. EC_KEY * EC_KEY_new_by_curve_name ( int nid ) { ASSERT (FALSE); return NULL; }
|
||||
|
|
||||
|
Event: TianoCore Bug Triage - APAC / NAMO - 10/11/2022
#cal-reminder
Group Notification <noreply@...>
Reminder: TianoCore Bug Triage - APAC / NAMO When: Where: Organizer: Liming Gao gaoliming@... Description: TianoCore Bug Triage - APAC / NAMO Hosted by Liming Gao
________________________________________________________________________________ Microsoft Teams meeting Join on your computer or mobile app Click here to join the meeting Join with a video conferencing device Video Conference ID: 116 062 094 0 Alternate VTC dialing instructions Or call in (audio only) +1 916-245-6934,,77463821# United States, Sacramento Phone Conference ID: 774 638 21# |
||||
|
|
||||
|
Re: The principles of EDK2 module reconstruction for archs
Michael D Kinney
There are many instances of both in edk2 repo. I would not remove from spec.
NetworkPkg/SnpDxe/Get_status.c NetworkPkg/SnpDxe/Mcast_ip_to_mac.c NetworkPkg/SnpDxe/Receive_filters.c NetworkPkg/SnpDxe/Station_address.c OvmfPkg/Include/IndustryStandard/Xen/arch-x86/hvm/start_info.h OvmfPkg/Include/IndustryStandard/Xen/arch-x86/xen-x86_32.h OvmfPkg/Include/IndustryStandard/Xen/arch-x86/xen-x86_64.h OvmfPkg/Include/IndustryStandard/Xen/event_channel.h OvmfPkg/Include/IndustryStandard/Xen/grant_table.h OvmfPkg/Include/IndustryStandard/Xen/hvm/hvm_op.h OvmfPkg/Include/IndustryStandard/Xen/io/xs_wire.h OvmfPkg/PlatformCI/iasl_ext_dep.yaml RedfishPkg/Library/JsonLib/jansson_config.h RedfishPkg/Library/JsonLib/jansson_private_config.h
Mike
From: Chang, Abner <Abner.Chang@...>
[AMD Official Use Only - General]
I was thinking to remove ‘_’ from edkII coding standard if there is no use case or rare people like it appears in file/dir name.
Abner
From: Kinney, Michael D <michael.d.kinney@...>
[AMD Official Use Only - General]
I do not have a strong opinion either way.
Some searching indicates there is some debate between use of spaces, underscores, and dashes in filenames.
The filename/dirname requirements for EDK II allow ‘_’ and ‘-‘ as long as they are not the first or last character. Spaces ‘ ‘ are not allowed.
[A-Za-z][_A-Za-z0-9-]*[A-Za-z0-9]+
Mike
From: Chang, Abner <Abner.Chang@...>
[AMD Official Use Only - General]
Removing '_' seems make the folder hard to read, but not too bad to me though. I am fine with removing '_'. Leif and Mike, how do you think?
Ex: Riscv64Ia32X64 compares Riscv64_Ia32_X64. ArmAArch64 compares to Arm_AArch64.
Abner From: Ni, Ray <ray.ni@...>
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> -----Original Message-----
> From: Chang, Abner <Abner.Chang@...> > Sent: Thursday, October 6, 2022 4:37 PM > To: Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io; > quic_llindhol@...; Ni, Ray <ray.ni@...>; Attar, AbdulLateef > (Abdul Lateef) <AbdulLateef.Attar@...>; Sunil V L > <sunilvl@...> > Cc: lichao <lichao@...>; Kirkendall, Garrett > <Garrett.Kirkendall@...>; Grimes, Paul <Paul.Grimes@...>; He, > Jiangang <Jiangang.He@...>; Andrew Fish <afish@...> > Subject: RE: [edk2-devel] The principles of EDK2 module reconstruction for > archs > > [AMD Official Use Only - General] > > Here is the update of the Wiki page for the consistency with edk2 C Coding > Standard Spec. > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fchangab%2Ftianocore.github.io%2Fwiki%2FThe-Guidelines-of-&data=05%7C01%7CAbner.Chang%40amd.com%7C1c6973cf412c4ba09ef908daab2b1ea6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638010498938218357%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=i5RSe41cuzD48VB32KeG0M3Vp7T%2FEqe3ncKNfGCjfIU%3D&reserved=0 > Reconstruct-EDK-II-Modules-for-Processor-Architectures-and-Vendors'- > Implementation > > Thanks > Abner > > > -----Original Message----- > > From: Chang, Abner > > Sent: Wednesday, October 5, 2022 1:39 PM > > To: Kinney, Michael D <michael.d.kinney@...>; > devel@edk2.groups.io; > > quic_llindhol@...; Ni, Ray <ray.ni@...>; Attar, AbdulLateef > > (Abdul Lateef) <AbdulLateef.Attar@...>; Sunil V L > > <sunilvl@...> > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > <Garrett.Kirkendall@...>; Grimes, Paul <Paul.Grimes@...>; > He, > > Jiangang <Jiangang.He@...>; Andrew Fish <afish@...> > > Subject: RE: [edk2-devel] The principles of EDK2 module reconstruction for > > archs > > > > [AMD Official Use Only - General] > > > > PR updated > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore-docs%2Fedk2-&data=05%7C01%7CAbner.Chang%40amd.com%7C1c6973cf412c4ba09ef908daab2b1ea6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638010498938218357%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=YDYXODgrQhuLlP8DTsLr%2F4ct2JH3aw8y2SPg8tk5fgg%3D&reserved=0 > > CCodingStandardsSpecification/pull/2/commits. Please check it. > > > > Thanks > > Abner > > > > > -----Original Message----- > > > From: Kinney, Michael D <michael.d.kinney@...> > > > Sent: Tuesday, October 4, 2022 10:18 PM > > > To: Chang, Abner <Abner.Chang@...>; devel@edk2.groups.io; > > > quic_llindhol@...; Ni, Ray <ray.ni@...>; Attar, > > > AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@...>; Sunil V L > > > <sunilvl@...>; Kinney, Michael D > > > <michael.d.kinney@...> > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > <Garrett.Kirkendall@...>; Grimes, Paul <Paul.Grimes@...>; > > He, > > > Jiangang <Jiangang.He@...>; Andrew Fish <afish@...> > > > Subject: RE: [edk2-devel] The principles of EDK2 module reconstruction > > > for archs > > > > > > Caution: This message originated from an External Source. Use proper > > > caution when opening attachments, clicking links, or responding. > > > > > > > > > I would not add link to Wiki from EDK II C Coding Standard Specification. > > > > > > We want documents published from tianocore-docs to support > standalone > > > formats such as PDF and if there is a link in one of those documents, > > > we want to know that it is a permanent link. I am concerned we may > > > reorganize Wiki pages and links in PDF would become stale. > > > > > > Links from Wiki to specs makes sense. > > > > > > Mike > > > > > > > -----Original Message----- > > > > From: Chang, Abner <Abner.Chang@...> > > > > Sent: Tuesday, October 4, 2022 7:05 AM > > > > To: Kinney, Michael D <michael.d.kinney@...>; > > > > devel@edk2.groups.io; quic_llindhol@...; Ni, Ray > > > > <ray.ni@...>; Attar, AbdulLateef (Abdul Lateef) > > > > <AbdulLateef.Attar@...>; Sunil V L <sunilvl@...> > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > <Garrett.Kirkendall@...>; Grimes, Paul > <Paul.Grimes@...>; > > > > He, Jiangang <Jiangang.He@...>; Andrew Fish > <afish@...> > > > > Subject: RE: [edk2-devel] The principles of EDK2 module > > > > reconstruction for archs > > > > > > > > [AMD Official Use Only - General] > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Kinney, Michael D <michael.d.kinney@...> > > > > > Sent: Tuesday, October 4, 2022 12:54 AM > > > > > To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@...>; > > > > > quic_llindhol@...; Ni, Ray <ray.ni@...>; Attar, > > > > > AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@...>; Sunil V L > > > > > <sunilvl@...>; Kinney, Michael D > > > > > <michael.d.kinney@...> > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > <Paul.Grimes@...>; > > > > > He, Jiangang <Jiangang.He@...>; Andrew Fish > > <afish@...> > > > > > Subject: RE: [edk2-devel] The principles of EDK2 module > > > > > reconstruction for archs > > > > > > > > > > Caution: This message originated from an External Source. Use > > > > > proper caution when opening attachments, clicking links, or > responding. > > > > > > > > > > > > > > > Hi Abner, > > > > > > > > > > responses below. > > > > > > > > > > Mike > > > > > > > > > > > -----Original Message----- > > > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of > > > > > > Chang, Abner via groups.io > > > > > > Sent: Sunday, October 2, 2022 10:37 PM > > > > > > To: Kinney, Michael D <michael.d.kinney@...>; > > > > > > devel@edk2.groups.io; quic_llindhol@...; Ni, Ray > > > > > > <ray.ni@...>; Attar, AbdulLateef (Abdul Lateef) > > > > > > <AbdulLateef.Attar@...>; Sunil V L > > > > > > <sunilvl@...> > > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > <Paul.Grimes@...>; > > > > > He, > > > > > > Jiangang <Jiangang.He@...>; Andrew Fish <afish@...> > > > > > > Subject: Re: [edk2-devel] The principles of EDK2 module > > > > > > reconstruction for archs > > > > > > > > > > > > [AMD Official Use Only - General] > > > > > > > > > > > > Mike, > > > > > > Agree. This can be mentioned on the Wiki page. Also, this would > > > > > > require the discussion between maintainer and contributor. I > > > > > > would say > > > > > maintainer has the responsibility to make sure an arch folder is > > > > > only created when necessary. > > > > > > > > > > Agreed > > > > Ok, I will update Directory and file names section. > > > > > > > > > > > > > > > > > Do you agree with the arch concatenate solution for arch folder? > > > > > > That > > > > > means IA32_X64 instead of X86 (I am fine with this)? > > > > > > > > > > Yes > > > > > > > > > > > However, there is one scenario. Assume there were two arch > > > > > > folders > > > > > > IA32_X64 and RISCV64. Then ARM shares the code with IA32_X64, > we > > > > > > may > > > > > rename IA32_X64 to IA32_X64_ARM. > > > > > > Although the directory naming is not real a problem to the > > > > > > build, a separate ARM folder seems easier? Or we can just allow > > > > > > this kind of folder > > > > > naming structure, however we let maintainer to make the decision? > > > > > > > > > > I would let the maintainer make the decision. For your example, > > > > > another approach may be to move the IA32_X64 content up a level so > > > > > it is common and is used by IA32, X64, ARM. And leave RISCV64 > > > > > folder for an arch that has special requirements. I think having > > > > > some flexibility in the guidelines is very beneficial. The main > > > > > goal is for consistency when a specific guideline is followed. > > > > I think we can have the naming rules in the edk2 C coding standard > > > > spec and > > > put these guidelines on the Wiki page. > > > > Is that ok to have a link to Wiki page in the edk2 C coding standard spec? > > > > > > > > Abner > > > > > > > > > > > > > > > > > > > > > Abner > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > From: Kinney, Michael D <michael.d.kinney@...> > > > > > > > Sent: Monday, October 3, 2022 1:18 PM > > > > > > > To: Chang, Abner <Abner.Chang@...>; > > devel@edk2.groups.io; > > > > > > > quic_llindhol@...; Ni, Ray <ray.ni@...>; Attar, > > > > > > > AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@...>; Sunil > > > > > > > V L <sunilvl@...>; Kinney, Michael D > > > > > > > <michael.d.kinney@...> > > > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > <Paul.Grimes@...>; He, Jiangang > <Jiangang.He@...>; > > > > > > > Andrew Fish <afish@...> > > > > > > > Subject: RE: [edk2-devel] The principles of EDK2 module > > > > > > > reconstruction for archs > > > > > > > > > > > > > > Caution: This message originated from an External Source. Use > > > > > > > proper caution when opening attachments, clicking links, or > > responding. > > > > > > > > > > > > > > > > > > > > > Abner, > > > > > > > > > > > > > > I think another guideline is to minimize the number of > > subdirectories. > > > > > > > > > > > > > > Only create them if it helps with the organization and long > > > > > > > term maintenance of the component. > > > > > > > > > > > > > > Mike > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > From: Chang, Abner <Abner.Chang@...> > > > > > > > > Sent: Sunday, October 2, 2022 8:13 PM > > > > > > > > To: Kinney, Michael D <michael.d.kinney@...>; > > > > > > > > devel@edk2.groups.io; quic_llindhol@...; Ni, Ray > > > > > > > > <ray.ni@...>; Attar, AbdulLateef (Abdul Lateef) > > > > > > > > <AbdulLateef.Attar@...>; Sunil V L > > > > > > > > <sunilvl@...> > > > > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > > > > <Paul.Grimes@...>; > > > > > > > He, > > > > > > > > Jiangang <Jiangang.He@...>; Andrew Fish > > > > > > > > <afish@...> > > > > > > > > Subject: RE: [edk2-devel] The principles of EDK2 module > > > > > > > > reconstruction for archs > > > > > > > > > > > > > > > > [AMD Official Use Only - General] > > > > > > > > > > > > > > > > Hi Mike and Leif, > > > > > > > > First of all, we don't use arch folder if the module is > > > > > > > > mainly for a specific arch in obviously. So we will have > > > > > > > > both common and arch-specific > > > > > > > files constructed under module/library root. > > > > > > > > > > > > > > > > > > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2F > > > > > ed > > > > > > > k > > > > > > > 2 > > > > > > > > > > > > > > > > > > .groups.io%2Fg%2Fdevel%2Fmessage%2F94567&data=05%7C01%7C > A > > > > > > > bner.Chan > > > > > > > > > > > > > > > > > > > > > > > > > > g%40amd.com%7Cd49cbbe6d3d942bd69a308daa4fea41b%7C3dd8961fe4884 > > > > > > > e608e11a > > > > > > > > > > > > > > > > > > > > > > > > > > 82d994e183d%7C0%7C0%7C638003710850252776%7CUnknown%7CTWFpbGZ > > > > > > > sb3d8eyJWI > > > > > > > > > > > > > > > > > > > > > > > > > > joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3 > > > > > > > 000%7 > > > > > > > > > > > > > > > > > > > > > > > C%7C%7C&sdata=eiLOC0G9WZWKqm2ALcAiKr7SPBP5AgDdAxogHlv5pI > > > > > > > M%3D&r > > > > > > > > eserved=0 > > > > > > > > SmmCpuFeatureLib\Ia32 > > > > > > > > SmmCpuFeatureLib\X64 > > > > > > > > SmmCpuFeatureLib\SmmCpuFeatureLib.c > > > > > > > > SmmCpuFeatureLib\SmmCpuFeatureLibCommon.c > > > > > > > > SmmCpuFeatureLib\IntelSmmCPuFeaturesLib > > > > > > > > SmmCpuFeatureLib\AmdlSmmCPuFeaturesLib > > > > > > > > > > > > > > > > > > > > > > > > > > Could we concatenate architectures? > > > > > > > > > > I.e. AARCH64_ARM, IA32_X64, AARCH64_RISCV64... ? > > > > > > > > Looks like below? > > > > > > > > > > > > > > > > CpuDxe\IA32_X64\IA32\abc.nasm > > CpuDxe\IA32_X64\X64\abc.nasm > > > > > > > > CpuDxe\IA32_X64\CpuDxe.c CpuDxe\IA32_X64\AmdCpuDxe.c > > > > > > > > CpuDxe\IA32_X64\IntelCpuDxe.c CpuDxe\RiscV64\CpuDxe.c > > > > > > > > CpuDxe\ARM\CpuDxe.c CpuDxe\ > > > > > > > > CpuDxeCommon.c // If required. > > > > > > > > CpuDxe.inf // Use INF section arch modifier for > X86, > > > > > RISC-V > > > > > > > and ARM. > > > > > > > > CpuDxeAmd.inf // Separate INF for AMD. > > > > > > > > > > > > > > > > Seems ok, but is AARCH64_RISCV64 a real case? Or it means > > > > > > > > we can create a folder "AARCH64_RISCV64" when there are > some > > > > > > > > common > > > > > files > > > > > > > shared by AARCH64 and RISCV64? > > > > > > > > > > > > > > > > For the 32/64 bit support, it can still stay under module > > > > > > > > root and don't need to assign a folder for them unless the > > > > > > > > arch has the different > > > > > > > implementation. > > > > > > > > Regards, > > > > > > > > Abner > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > From: Kinney, Michael D <michael.d.kinney@...> > > > > > > > > > Sent: Saturday, October 1, 2022 2:52 AM > > > > > > > > > To: devel@edk2.groups.io; quic_llindhol@...; > > > > > > > > > Chang, Abner <Abner.Chang@...>; Ni, Ray > > > > > > > > > <ray.ni@...>; Attar, AbdulLateef (Abdul Lateef) > > > > > > > > > <AbdulLateef.Attar@...>; Sunil V L > > > > > > > > > <sunilvl@...>; Kinney, Michael D > > > > > > > > > <michael.d.kinney@...> > > > > > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > <Paul.Grimes@...>; He, Jiangang > > <Jiangang.He@...>; > > > > > > > > > Andrew Fish <afish@...> > > > > > > > > > Subject: RE: [edk2-devel] The principles of EDK2 module > > > > > > > > > reconstruction for archs > > > > > > > > > > > > > > > > > > Caution: This message originated from an External Source. > > > > > > > > > Use proper caution when opening attachments, clicking > > > > > > > > > links, or > > > > > responding. > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Leif, > > > > > > > > > > > > > > > > > > Concatenation is a good idea. Makes it more obvious and > > > > > > > > > can be easily adopted for new CPU archs. > > > > > > > > > > > > > > > > > > There is an additional case where an implementation does > > > > > > > > > not have differences based on CPU Arch or Vendor, but it > > > > > > > > > does have differences based on the bit- width of the CPU. > > > > > > > > > Take BaseSafeIntLib as > > > > > > > an example. > > > > > > > > > It has source files for 32-bit CPUs, 64-bit CPUs, and CPU > > > > > > > > > arch specific file for Ebc because Ebc adapts to 32-bit or > > > > > > > > > 64-bit depending on the CPU type the EBC Interpreter is > running. > > > > > > > > > > > > > > > > > > MdePkg/Library/BaseSafeIntLib/ > > > > > > > > > BaseSafeIntLib.inf > > > > > > > > > SafeIntLib.c > > > > > > > > > SafeIntLib32.c > > > > > > > > > SafeIntLib64.c > > > > > > > > > SafeIntLibEbc.c > > > > > > > > > > > > > > > > > > Should we add "32" and "64" as supported suffices in file > names? > > > > > > > > > > > > > > > > > > If we wanted to put type content into a subdirectory, what > > > > > > > > > would be the suggested directory name for "32" and "64". > > > > > > > > > Or do we want to require this type of difference to always > > > > > > > > > be files in top level directory of > > > > > > > the modules/library? > > > > > > > > > > > > > > > > > > Best regards, > > > > > > > > > > > > > > > > > > Mike > > > > > > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On > > > > > > > > > > Behalf Of Leif Lindholm > > > > > > > > > > Sent: Friday, September 30, 2022 9:09 AM > > > > > > > > > > To: devel@edk2.groups.io; Kinney, Michael D > > > > > > > > > > <michael.d.kinney@...>; Chang, Abner > > > > > > > <Abner.Chang@...>; > > > > > > > > > > Ni, Ray <ray.ni@...>; Attar, AbdulLateef (Abdul > > > > > > > > > > Lateef) <AbdulLateef.Attar@...>; Sunil V L > > > > > > > > > > <sunilvl@...> > > > > > > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > <Paul.Grimes@...>; > > > > > > > > > > He, Jiangang <Jiangang.He@...>; Andrew Fish > > > > > > > <afish@...> > > > > > > > > > > Subject: Re: [edk2-devel] The principles of EDK2 module > > > > > > > > > > reconstruction for archs > > > > > > > > > > > > > > > > > > > > I agree similar things will certainly happen for > > > > > > > > > > ARM/AARCH64, which will probably be noticeable when I > > > > > > > > > > start eradicating ArmPkg and putting the arch-standard > > > > > > > > > > bits into > > > (mostly) MdePkg. > > > > > > > > > > > > > > > > > > > > But I like the ability to see already at the filesystem > > > > > > > > > > level which files belong to the architecture I'm > > > > > > > > > > currently working on and > > > > > > > which don't. > > > > > > > > > > > > > > > > > > > > Could we concatenate architectures? > > > > > > > > > > I.e. AARCH64_ARM, IA32_X64, AARCH64_RISCV64... ? > > > > > > > > > > > > > > > > > > > > / > > > > > > > > > > Leif > > > > > > > > > > > > > > > > > > > > On 2022-09-30 08:28, Michael D Kinney wrote: > > > > > > > > > > > Hi Abner, > > > > > > > > > > > > > > > > > > > > > > One comment is on adding a new CPU type dir name of > 'X86' > > > > > > > > > > > for content that is common for IA32/X64. > > > > > > > > > > > > > > > > > > > > > > I can imagine a similar case for ARM/AARCH64 and for > > > > > > > > > > > the RISCV (32, 64, 128) cases. > > > > > > > > > > > > > > > > > > > > > > I think I would prefer to drop X86 and if there are > > > > > > > > > > > files that are common to multiple CPU architectures > > > > > > > > > > > then they are considered common and are in top > > > > > > > > > > > directory of module and the file header and INF file > > > > > > > > > > > can clearly document which CPU archs the file > > > > > > > applies. > > > > > > > > > > > > > > > > > > > > > > Mike > > > > > > > > > > > > > > > > > > > > > >> -----Original Message----- > > > > > > > > > > >> From: Chang, Abner <Abner.Chang@...> > > > > > > > > > > >> Sent: Friday, September 30, 2022 12:11 AM > > > > > > > > > > >> To: Ni, Ray <ray.ni@...>; Attar, AbdulLateef > > > > > > > > > > >> (Abdul > > > > > > > > > > >> Lateef) <AbdulLateef.Attar@...>; Sunil V L > > > > > > > > > > >> <sunilvl@...>; devel@edk2.groups.io; > > > > > > > > > > >> Kinney, Michael D <michael.d.kinney@...> > > > > > > > > > > >> Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > > >> <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > > >> <Paul.Grimes@...>; He, Jiangang > > > > > <Jiangang.He@...>; > > > > > > > Leif > > > > > > > > > > >> Lindholm <quic_llindhol@...>; Andrew Fish > > > > > > > > > > >> <afish@...> > > > > > > > > > > >> Subject: RE: [edk2-devel] The principles of EDK2 > > > > > > > > > > >> module reconstruction for archs > > > > > > > > > > >> > > > > > > > > > > >> [AMD Official Use Only - General] > > > > > > > > > > >> > > > > > > > > > > >> Thanks Ray, here are my responses. > > > > > > > > > > >> https://nam11.safelinks.protection.outlook.com/?url=h > > > > > > > > > > >> tt > > > > > > > > > > >> ps%3 > > > > > > > > > > >> A%2F > > > > > > > > > > >> %2Fg > > > > > > > > > > >> ithub.com%2Ftianocore-docs%2Fedk2- > > > > > > > CCodingStandardsSpecification > > > > > > > > > > >> %2Fp > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > ull%2F2&data=05%7C01%7CAbner.Chang%40amd.com%7C7c3292c8bd2 > > > > > > > f4 > > > > > > > > > 86f > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > 920908daa314e8e6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6 > > > > > > > > > 3800 > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > 1607445876768%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLC > > > > > > > JQ > > > > > > > > > IjoiV > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata > > > > > > > = > > > > > > > > > HAq > > > > > > > > > > >> > > > > > > > > > > > > > > ou4JyY1yxDthuQ1dEKcF7Q3o4W77Oo9rOOvkXNWU%3D&reserved=0 > > > > > > > > > > >> > > > > > > > > > > >> @Kinney, Michael D we may also need your > > > > > > > > > > >> clarification on the > > > > > > > comments. > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >>> -----Original Message----- > > > > > > > > > > >>> From: Ni, Ray <ray.ni@...> > > > > > > > > > > >>> Sent: Thursday, September 29, 2022 3:42 PM > > > > > > > > > > >>> To: Attar, AbdulLateef (Abdul Lateef) > > > > > > > > > > >>> <AbdulLateef.Attar@...>; Chang, Abner > > > > > > > > > > >>> <Abner.Chang@...>; Sunil V L > > > > > > > > > > >>> <sunilvl@...>; devel@edk2.groups.io > > > > > > > > > > >>> Cc: Kinney, Michael D <michael.d.kinney@...>; > > > > > > > > > > >>> lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > > >>> <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > > >>> <Paul.Grimes@...>; He, Jiangang > > > > > <Jiangang.He@...>; > > > > > > > > > > >>> Leif Lindholm <quic_llindhol@...>; Andrew > > > > > > > > > > >>> Fish <afish@...> > > > > > > > > > > >>> Subject: RE: [edk2-devel] The principles of EDK2 > > > > > > > > > > >>> module reconstruction for archs > > > > > > > > > > >>> > > > > > > > > > > >>> Caution: This message originated from an External > Source. > > > > > > > > > > >>> Use proper caution when opening attachments, > > > > > > > > > > >>> clicking links, or > > > > > > > responding. > > > > > > > > > > >>> > > > > > > > > > > >>> > > > > > > > > > > >>> Abner, > > > > > > > > > > >>> Comments in > > > > > > > > > > >>> https://nam11.safelinks.protection.outlook.com/?url= > > > > > > > > > > >>> ht > > > > > > > > > > >>> tps% > > > > > > > > > > >>> 3A%2 > > > > > > > > > > >>> F%2F > > > > > > > > > > >>> gith > > > > > > > > > > >>> ub.com%2Ftianocore-docs%2Fedk2- > > > > > > > > > > >>> > CCodingStandardsSpecification%2Fpull%2F2%23pullreque > > > > > > > > > > >>> st > > > > > > > > > > >>> revi > > > > > > > > > > >>> ew- > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 1124763311&data=05%7C01%7CAbner.Chang%40amd.com%7Cd825e24 > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 8625541e3f43e08daa1ee2883%7C3dd8961fe4884e608e11a82d994e183d%7C0 > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > %7C0%7C638000341502885565%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM > C > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000% > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > 7C%7C%7C&sdata=RXxgpbEr6ivbqP1R6%2B3Rxl%2ByJAnaUJuaYYKdfCH > > > > > > > > > > >>> 8jo8%3D&reserved=0 > > > > > > > > > > >>> > > > > > > > > > > >>> We can discuss more in tomorrow's meeting. > > > > > > > > > > >>> > > > > > > > > > > >>> > > > > > > > > > > >>>> -----Original Message----- > > > > > > > > > > >>>> From: Attar, AbdulLateef (Abdul Lateef) > > > > > > > > > > >>>> <AbdulLateef.Attar@...> > > > > > > > > > > >>>> Sent: Thursday, September 29, 2022 3:11 PM > > > > > > > > > > >>>> To: Chang, Abner <Abner.Chang@...>; Sunil V L > > > > > > > > > > >>>> <sunilvl@...>; devel@edk2.groups.io; > > > > > > > > > > >>>> Ni, Ray <ray.ni@...> > > > > > > > > > > >>>> Cc: Kinney, Michael D <michael.d.kinney@...>; > > > > > > > > > > >>>> lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > > >>>> <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > > >>>> <Paul.Grimes@...>; > > > > > > > > > > >>> He, > > > > > > > > > > >>>> Jiangang <Jiangang.He@...>; Leif Lindholm > > > > > > > > > > >>>> <quic_llindhol@...>; Andrew Fish > > > > > > > > > > >>>> <afish@...> > > > > > > > > > > >>>> Subject: RE: [edk2-devel] The principles of EDK2 > > > > > > > > > > >>>> module reconstruction for archs > > > > > > > > > > >>>> > > > > > > > > > > >>>> Hi Abner, > > > > > > > > > > >>>> Looks good to me. > > > > > > > > > > >>>> Reviewed-by: Abdul Lateef Attar <abdattar@...> > > > > > > > > > > >>>> > > > > > > > > > > >>>> Thanks > > > > > > > > > > >>>> AbduL > > > > > > > > > > >>>> > > > > > > > > > > >>>> -----Original Message----- > > > > > > > > > > >>>> From: Chang, Abner <Abner.Chang@...> > > > > > > > > > > >>>> Sent: 28 September 2022 20:31 > > > > > > > > > > >>>> To: Sunil V L <sunilvl@...>; > > > > > > > > > > >>>> devel@edk2.groups.io; ray.ni@... > > > > > > > > > > >>>> Cc: Kinney, Michael D <michael.d.kinney@...>; > > > > > > > > > > >>>> lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > > >>>> <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > > >>>> <Paul.Grimes@...>; > > > > > > > > > > >>> He, > > > > > > > > > > >>>> Jiangang <Jiangang.He@...>; Attar, AbdulLateef > > > > > > > > > > >>>> (Abdul > > > > > > > > > > >>>> Lateef) <AbdulLateef.Attar@...>; Leif Lindholm > > > > > > > > > > >>>> <quic_llindhol@...>; Andrew Fish > > > > > > > > > > >>>> <afish@...> > > > > > > > > > > >>>> Subject: RE: [edk2-devel] The principles of EDK2 > > > > > > > > > > >>>> module reconstruction for archs > > > > > > > > > > >>>> > > > > > > > > > > >>>> [AMD Official Use Only - General] > > > > > > > > > > >>>> > > > > > > > > > > >>>> I just had created PR to update edkII C coding > > > > > > > > > > >>>> standard spec for the file and directory naming. We > > > > > > > > > > >>>> can review and confirm this update first and then > > > > > > > > > > >>>> go back to the principles of EDK2 module > > > > > > > > > reconstruction for archs. > > > > > > > > > > >>>> Here is the PR: > > > > > > > > > > >>>> > > > > > > > > > > >>> https://nam11.safelinks.protection.outlook.com/?url= > > > > > > > > > > >>> ht > > > > > > > > > > >>> tps% > > > > > > > > > > >>> 3A%2 > > > > > > > > > > >>> F%2F > > > > > > > > > > >>> gith > > > > > > > > > > >>>> ub.com%2Ftianocore-docs%2Fedk2- > > > > > > > > > > >>> &data=05%7C01%7CAbner.Chang%40amd.c > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > om%7Cd825e248625541e3f43e08daa1ee2883%7C3dd8961fe4884e608e11a82 > > > > > > > > > > >>> d994e18 > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 3d%7C0%7C0%7C638000341502885565%7CUnknown%7CTWFpbGZsb3d8eyJ > > > > > > > > > > >>> WIjoiMC4wLj > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C% > > > > > > > > > > >>> 7C%7C&a > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > mp;sdata=X4z9puj81nIGTqtMxC9igDZyBPOT6OTWSU%2BjoIowo%2BE%3D&a > > > > > > > > > > >>> mp;reserv > > > > > > > > > > >>>> ed=0 > > > > > > > > > > >>>> CCodingStandardsSpecification/pull/2 > > > > > > > > > > >>>> > > > > > > > > > > >>>> The naming rule is mainly for the new module or new > file > > IMO. > > > > > > > > > > >>>> Some existing module may not meet the guidelines > > > > > > > > > > >>>> mentioned in this > > > > > > > > > spec. > > > > > > > > > > >>>> Thus we need the principles of EDK2 module > > > > > > > > > > >>>> reconstruction on the existing module to support > > > > > > > > > > >>>> other processor archs and not impacting the > > > > > > > > > > >>> existing platforms (e.g. > > > > > > > > > > >>>> rename the INF file or directory to meet the > guidelines). > > > > > > > > > > >>>> > > > > > > > > > > >>>> Sunil, seems RISC-V CpuDxe meet the guideline. > > > > > > > > > > >>>> Please check > > > > > it. > > > > > > > > > > >>>> Just feel that having CpuDxe.c to Riscv64 folder > > > > > > > > > > >>>> is not quite a best solution. I think at least we > > > > > > > > > > >>>> can abstract the protocol structure and protocol > > > > > > > > > > >>>> installation under CpuDxe\ and have the arch > > > > > > > > > > >>>> implementation under arch folder. We can discuss > > > > > > > > > > >>>> this later after we confirming the > > > > > > > > > > >>> guideline and principles. > > > > > > > > > > >>>> > > > > > > > > > > >>>> Thanks > > > > > > > > > > >>>> Abner > > > > > > > > > > >>>> > > > > > > > > > > >>>>> -----Original Message----- > > > > > > > > > > >>>>> From: Sunil V L <sunilvl@...> > > > > > > > > > > >>>>> Sent: Wednesday, September 28, 2022 3:34 PM > > > > > > > > > > >>>>> To: devel@edk2.groups.io; ray.ni@... > > > > > > > > > > >>>>> Cc: Chang, Abner <Abner.Chang@...>; Kinney, > > > > > Michael > > > > > > > > > > >>>>> D <michael.d.kinney@...>; lichao > > > > > > > > > > >>>>> <lichao@...>; Kirkendall, Garrett > > > > > > > > > > >>>>> <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > > >>>>> <Paul.Grimes@...>; He, Jiangang > > > > > > > > > > >>>>> <Jiangang.He@...>; Attar, AbdulLateef (Abdul > > > > > > > > > > >>>>> Lateef) <AbdulLateef.Attar@...>; Leif > Lindholm > > > > > > > > > > >>>>> <quic_llindhol@...>; Andrew Fish > > > > > > > > > > >>>>> <afish@...> > > > > > > > > > > >>>>> Subject: Re: [edk2-devel] The principles of EDK2 > > > > > > > > > > >>>>> module reconstruction for archs > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> Caution: This message originated from an External > > Source. > > > > > > > > > > >>>>> Use proper caution when opening attachments, > > > > > > > > > > >>>>> clicking links, or > > > > > > > responding. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> On Wed, Sep 28, 2022 at 03:33:45AM +0000, Ni, Ray > > wrote: > > > > > > > > > > >>>>> Hi Ray, > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> 1. When a new arch's implementation is > > > > > > > > > > >>>>>> introduced to the existing > > > > > > > > > > >>>>> module which was developed for the specific arch: > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> 1. The folder reconstruction: > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> * Create arch folder for the existing arch > > implementation > > > > > > > > > > >>>>>> [Ray] Do you move existing arch implementation to > > > > > > > > > > >>>>>> that arch > > > > > > > folder? > > > > > > > > > > >>>>>> It will > > > > > > > > > > >>>>> break existing platforms a lot. > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> * Create the arch folder for the new introduced > > arch > > > > > > > > > > >>>>>> [Ray] I agree. But if we don't create arch folder > > > > > > > > > > >>>>>> for existing arch > > > > > > > > > > >>>>> implementation, the pkg layout will be a mess. > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> [Ray] Hard for me to understand all the principles > here. > > > > > > > > > > >>>>>> Maybe we review > > > > > > > > > > >>>>> existing code including to-be-upstreamed code and > > > > > > > > > > >>>>> decide how > > > > > > > to go. > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> Could you please take a look below changes which > > > > > > > > > > >>>>> is trying to add RISC-V support for CpuDxe? > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> https://nam11.safelinks.protection.outlook.com/?url= > > > > > > > > > > >>> ht > > > > > > > > > > >>> tps% > > > > > > > > > > >>> 3A%2 > > > > > > > > > > >>> F%2F > > > > > > > > > > >>> gith > > > > > > > > > > >>>>> ub.com%2Ftianocore%2Fedk2- > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > staging%2Fcommit%2Fbba1a11be47dd091734e185afbed73ea75708749& > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > data=05%7C01%7Cabner.chang%40amd.com%7Ca419e6a010d34fde464b08d > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > aa123e080%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63799947 > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 2732494527%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sd > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > ata=Vq6pJLnn8yJrJhFZn7LfLbZzrtpG4n1VLWgAil6J38U%3D&reserved=0 > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> https://nam11.safelinks.protection.outlook.com/?url= > > > > > > > > > > >>> ht > > > > > > > > > > >>> tps% > > > > > > > > > > >>> 3A%2 > > > > > > > > > > >>> F%2F > > > > > > > > > > >>> gith > > > > > > > > > > >>>>> ub.com%2Ftianocore%2Fedk2- > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > staging%2Fcommit%2F7fccf92a97a6d0618a20f10622220e78b3687906&da > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > ta=05%7C01%7Cabner.chang%40amd.com%7Ca419e6a010d34fde464b08daa1 > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 23e080%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63799947273 > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 2494527%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > =xFmvUv58vh4AUAM17Qy9G5jZWFZlK2Ozt3njpG1e8%2BY%3D&reserv > > > > > > > > > > >>>>> ed=0 > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> What do you suggest with above example? > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> 1) Common INF for all architectures - but modify > > > > > > > > > > >>>>> INF alone, no > > > > > > > > > > >>>>> X86 folder creation. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> This is what I have done in the commit above. May > > > > > > > > > > >>>>> be of least impact to existing code since it is only INF > > change. > > > > > > > > > > >>>>> But like you mentioned this is bit weird that X86 > > > > > > > > > > >>>>> files will remain in root folder directly along > > > > > > > > > > >>>>> with some common > > > > > files. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> 2) Common INF (CpuDxe.inf) + create arch folders > > > > > > > > > > >>>>> X86, X64, IA32, > > > > > > > > > > >>>>> RiscV64 etc > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> IMO, this is probably the best approach. What > > > > > > > > > > >>>>> would be the challenges with this? > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> 3) Separate INF for arch like CpuDxe.inf for x86, > > > > > > > > > > >>>>> CpuDxeRiscV64.inf for > > > > > > > > > > >>>> RISC-V. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> This again probably is not a good idea. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> 4) If the module/library is specific to one arch (ex: > > > > > > > > > > >>>>> SMM(X86), SBI(RISC-V)), then create separate INF. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> Thanks! > > > > > > > > > > >>>>> Sunil > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > |
||||
|
|
||||
|
Re: [Patch 00/12] CryptoPkg: Remove EC PCD and merge perf opt OpensslLibs
Yao, Jiewen
Thank you Mike.
toggle quoted message
Show quoted text
1) I like the idea to combine multiple OpensslLibIA32/X64.inf into one OpensslLibAccel.inf. Also the cleanup looks good to me. 2) I also like the summary in readme in https://github.com/mdkinney/edk2/tree/CryptoPkg_RemoveEcPcd_MergeOptimizedOpensslLibs/CryptoPkg I notice some algorithms are listed Y(Deprecated) but N(Don't Use), such as Tdes, Arc4, Aes.Ecb*. But I don't see the use case for those algorithms and I suggest a Y(Deprecated) have Y(Don't Use). 3) About PcdOpensslEcEnabled I notice it is used in existing code - https://github.com/mdkinney/edk2/blob/CryptoPkg_RemoveEcPcd_MergeOptimizedOpensslLibs/CryptoPkg/Library/TlsLib/TlsConfig.c#L1139 Is this right way? Thank you Yao, Jiewen -----Original Message----- |
||||
|
|
||||
|
Re: The principles of EDK2 module reconstruction for archs
Chang, Abner
[AMD Official Use Only - General]
I was thinking to remove ‘_’ from edkII coding standard if there is no use case or rare people like it appears in file/dir name.
Abner
From: Kinney, Michael D <michael.d.kinney@...>
[AMD Official Use Only - General]
I do not have a strong opinion either way.
Some searching indicates there is some debate between use of spaces, underscores, and dashes in filenames.
The filename/dirname requirements for EDK II allow ‘_’ and ‘-‘ as long as they are not the first or last character. Spaces ‘ ‘ are not allowed.
[A-Za-z][_A-Za-z0-9-]*[A-Za-z0-9]+
Mike
From: Chang, Abner <Abner.Chang@...>
[AMD Official Use Only - General]
Removing '_' seems make the folder hard to read, but not too bad to me though. I am fine with removing '_'. Leif and Mike, how do you think?
Ex: Riscv64Ia32X64 compares Riscv64_Ia32_X64. ArmAArch64 compares to Arm_AArch64.
Abner From: Ni, Ray <ray.ni@...>
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> -----Original Message-----
> From: Chang, Abner <Abner.Chang@...> > Sent: Thursday, October 6, 2022 4:37 PM > To: Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io; > quic_llindhol@...; Ni, Ray <ray.ni@...>; Attar, AbdulLateef > (Abdul Lateef) <AbdulLateef.Attar@...>; Sunil V L > <sunilvl@...> > Cc: lichao <lichao@...>; Kirkendall, Garrett > <Garrett.Kirkendall@...>; Grimes, Paul <Paul.Grimes@...>; He, > Jiangang <Jiangang.He@...>; Andrew Fish <afish@...> > Subject: RE: [edk2-devel] The principles of EDK2 module reconstruction for > archs > > [AMD Official Use Only - General] > > Here is the update of the Wiki page for the consistency with edk2 C Coding > Standard Spec. > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fchangab%2Ftianocore.github.io%2Fwiki%2FThe-Guidelines-of-&data=05%7C01%7CAbner.Chang%40amd.com%7C1c6973cf412c4ba09ef908daab2b1ea6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638010498938218357%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=i5RSe41cuzD48VB32KeG0M3Vp7T%2FEqe3ncKNfGCjfIU%3D&reserved=0 > Reconstruct-EDK-II-Modules-for-Processor-Architectures-and-Vendors'- > Implementation > > Thanks > Abner > > > -----Original Message----- > > From: Chang, Abner > > Sent: Wednesday, October 5, 2022 1:39 PM > > To: Kinney, Michael D <michael.d.kinney@...>; > devel@edk2.groups.io; > > quic_llindhol@...; Ni, Ray <ray.ni@...>; Attar, AbdulLateef > > (Abdul Lateef) <AbdulLateef.Attar@...>; Sunil V L > > <sunilvl@...> > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > <Garrett.Kirkendall@...>; Grimes, Paul <Paul.Grimes@...>; > He, > > Jiangang <Jiangang.He@...>; Andrew Fish <afish@...> > > Subject: RE: [edk2-devel] The principles of EDK2 module reconstruction for > > archs > > > > [AMD Official Use Only - General] > > > > PR updated > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore-docs%2Fedk2-&data=05%7C01%7CAbner.Chang%40amd.com%7C1c6973cf412c4ba09ef908daab2b1ea6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638010498938218357%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=YDYXODgrQhuLlP8DTsLr%2F4ct2JH3aw8y2SPg8tk5fgg%3D&reserved=0 > > CCodingStandardsSpecification/pull/2/commits. Please check it. > > > > Thanks > > Abner > > > > > -----Original Message----- > > > From: Kinney, Michael D <michael.d.kinney@...> > > > Sent: Tuesday, October 4, 2022 10:18 PM > > > To: Chang, Abner <Abner.Chang@...>; devel@edk2.groups.io; > > > quic_llindhol@...; Ni, Ray <ray.ni@...>; Attar, > > > AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@...>; Sunil V L > > > <sunilvl@...>; Kinney, Michael D > > > <michael.d.kinney@...> > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > <Garrett.Kirkendall@...>; Grimes, Paul <Paul.Grimes@...>; > > He, > > > Jiangang <Jiangang.He@...>; Andrew Fish <afish@...> > > > Subject: RE: [edk2-devel] The principles of EDK2 module reconstruction > > > for archs > > > > > > Caution: This message originated from an External Source. Use proper > > > caution when opening attachments, clicking links, or responding. > > > > > > > > > I would not add link to Wiki from EDK II C Coding Standard Specification. > > > > > > We want documents published from tianocore-docs to support > standalone > > > formats such as PDF and if there is a link in one of those documents, > > > we want to know that it is a permanent link. I am concerned we may > > > reorganize Wiki pages and links in PDF would become stale. > > > > > > Links from Wiki to specs makes sense. > > > > > > Mike > > > > > > > -----Original Message----- > > > > From: Chang, Abner <Abner.Chang@...> > > > > Sent: Tuesday, October 4, 2022 7:05 AM > > > > To: Kinney, Michael D <michael.d.kinney@...>; > > > > devel@edk2.groups.io; quic_llindhol@...; Ni, Ray > > > > <ray.ni@...>; Attar, AbdulLateef (Abdul Lateef) > > > > <AbdulLateef.Attar@...>; Sunil V L <sunilvl@...> > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > <Garrett.Kirkendall@...>; Grimes, Paul > <Paul.Grimes@...>; > > > > He, Jiangang <Jiangang.He@...>; Andrew Fish > <afish@...> > > > > Subject: RE: [edk2-devel] The principles of EDK2 module > > > > reconstruction for archs > > > > > > > > [AMD Official Use Only - General] > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Kinney, Michael D <michael.d.kinney@...> > > > > > Sent: Tuesday, October 4, 2022 12:54 AM > > > > > To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@...>; > > > > > quic_llindhol@...; Ni, Ray <ray.ni@...>; Attar, > > > > > AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@...>; Sunil V L > > > > > <sunilvl@...>; Kinney, Michael D > > > > > <michael.d.kinney@...> > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > <Paul.Grimes@...>; > > > > > He, Jiangang <Jiangang.He@...>; Andrew Fish > > <afish@...> > > > > > Subject: RE: [edk2-devel] The principles of EDK2 module > > > > > reconstruction for archs > > > > > > > > > > Caution: This message originated from an External Source. Use > > > > > proper caution when opening attachments, clicking links, or > responding. > > > > > > > > > > > > > > > Hi Abner, > > > > > > > > > > responses below. > > > > > > > > > > Mike > > > > > > > > > > > -----Original Message----- > > > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of > > > > > > Chang, Abner via groups.io > > > > > > Sent: Sunday, October 2, 2022 10:37 PM > > > > > > To: Kinney, Michael D <michael.d.kinney@...>; > > > > > > devel@edk2.groups.io; quic_llindhol@...; Ni, Ray > > > > > > <ray.ni@...>; Attar, AbdulLateef (Abdul Lateef) > > > > > > <AbdulLateef.Attar@...>; Sunil V L > > > > > > <sunilvl@...> > > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > <Paul.Grimes@...>; > > > > > He, > > > > > > Jiangang <Jiangang.He@...>; Andrew Fish <afish@...> > > > > > > Subject: Re: [edk2-devel] The principles of EDK2 module > > > > > > reconstruction for archs > > > > > > > > > > > > [AMD Official Use Only - General] > > > > > > > > > > > > Mike, > > > > > > Agree. This can be mentioned on the Wiki page. Also, this would > > > > > > require the discussion between maintainer and contributor. I > > > > > > would say > > > > > maintainer has the responsibility to make sure an arch folder is > > > > > only created when necessary. > > > > > > > > > > Agreed > > > > Ok, I will update Directory and file names section. > > > > > > > > > > > > > > > > > Do you agree with the arch concatenate solution for arch folder? > > > > > > That > > > > > means IA32_X64 instead of X86 (I am fine with this)? > > > > > > > > > > Yes > > > > > > > > > > > However, there is one scenario. Assume there were two arch > > > > > > folders > > > > > > IA32_X64 and RISCV64. Then ARM shares the code with IA32_X64, > we > > > > > > may > > > > > rename IA32_X64 to IA32_X64_ARM. > > > > > > Although the directory naming is not real a problem to the > > > > > > build, a separate ARM folder seems easier? Or we can just allow > > > > > > this kind of folder > > > > > naming structure, however we let maintainer to make the decision? > > > > > > > > > > I would let the maintainer make the decision. For your example, > > > > > another approach may be to move the IA32_X64 content up a level so > > > > > it is common and is used by IA32, X64, ARM. And leave RISCV64 > > > > > folder for an arch that has special requirements. I think having > > > > > some flexibility in the guidelines is very beneficial. The main > > > > > goal is for consistency when a specific guideline is followed. > > > > I think we can have the naming rules in the edk2 C coding standard > > > > spec and > > > put these guidelines on the Wiki page. > > > > Is that ok to have a link to Wiki page in the edk2 C coding standard spec? > > > > > > > > Abner > > > > > > > > > > > > > > > > > > > > > Abner > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > From: Kinney, Michael D <michael.d.kinney@...> > > > > > > > Sent: Monday, October 3, 2022 1:18 PM > > > > > > > To: Chang, Abner <Abner.Chang@...>; > > devel@edk2.groups.io; > > > > > > > quic_llindhol@...; Ni, Ray <ray.ni@...>; Attar, > > > > > > > AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@...>; Sunil > > > > > > > V L <sunilvl@...>; Kinney, Michael D > > > > > > > <michael.d.kinney@...> > > > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > <Paul.Grimes@...>; He, Jiangang > <Jiangang.He@...>; > > > > > > > Andrew Fish <afish@...> > > > > > > > Subject: RE: [edk2-devel] The principles of EDK2 module > > > > > > > reconstruction for archs > > > > > > > > > > > > > > Caution: This message originated from an External Source. Use > > > > > > > proper caution when opening attachments, clicking links, or > > responding. > > > > > > > > > > > > > > > > > > > > > Abner, > > > > > > > > > > > > > > I think another guideline is to minimize the number of > > subdirectories. > > > > > > > > > > > > > > Only create them if it helps with the organization and long > > > > > > > term maintenance of the component. > > > > > > > > > > > > > > Mike > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > From: Chang, Abner <Abner.Chang@...> > > > > > > > > Sent: Sunday, October 2, 2022 8:13 PM > > > > > > > > To: Kinney, Michael D <michael.d.kinney@...>; > > > > > > > > devel@edk2.groups.io; quic_llindhol@...; Ni, Ray > > > > > > > > <ray.ni@...>; Attar, AbdulLateef (Abdul Lateef) > > > > > > > > <AbdulLateef.Attar@...>; Sunil V L > > > > > > > > <sunilvl@...> > > > > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > > > > <Paul.Grimes@...>; > > > > > > > He, > > > > > > > > Jiangang <Jiangang.He@...>; Andrew Fish > > > > > > > > <afish@...> > > > > > > > > Subject: RE: [edk2-devel] The principles of EDK2 module > > > > > > > > reconstruction for archs > > > > > > > > > > > > > > > > [AMD Official Use Only - General] > > > > > > > > > > > > > > > > Hi Mike and Leif, > > > > > > > > First of all, we don't use arch folder if the module is > > > > > > > > mainly for a specific arch in obviously. So we will have > > > > > > > > both common and arch-specific > > > > > > > files constructed under module/library root. > > > > > > > > > > > > > > > > > > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2F > > > > > ed > > > > > > > k > > > > > > > 2 > > > > > > > > > > > > > > > > > > .groups.io%2Fg%2Fdevel%2Fmessage%2F94567&data=05%7C01%7C > A > > > > > > > bner.Chan > > > > > > > > > > > > > > > > > > > > > > > > > > g%40amd.com%7Cd49cbbe6d3d942bd69a308daa4fea41b%7C3dd8961fe4884 > > > > > > > e608e11a > > > > > > > > > > > > > > > > > > > > > > > > > > 82d994e183d%7C0%7C0%7C638003710850252776%7CUnknown%7CTWFpbGZ > > > > > > > sb3d8eyJWI > > > > > > > > > > > > > > > > > > > > > > > > > > joiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3 > > > > > > > 000%7 > > > > > > > > > > > > > > > > > > > > > > > C%7C%7C&sdata=eiLOC0G9WZWKqm2ALcAiKr7SPBP5AgDdAxogHlv5pI > > > > > > > M%3D&r > > > > > > > > eserved=0 > > > > > > > > SmmCpuFeatureLib\Ia32 > > > > > > > > SmmCpuFeatureLib\X64 > > > > > > > > SmmCpuFeatureLib\SmmCpuFeatureLib.c > > > > > > > > SmmCpuFeatureLib\SmmCpuFeatureLibCommon.c > > > > > > > > SmmCpuFeatureLib\IntelSmmCPuFeaturesLib > > > > > > > > SmmCpuFeatureLib\AmdlSmmCPuFeaturesLib > > > > > > > > > > > > > > > > > > > > > > > > > > Could we concatenate architectures? > > > > > > > > > > I.e. AARCH64_ARM, IA32_X64, AARCH64_RISCV64... ? > > > > > > > > Looks like below? > > > > > > > > > > > > > > > > CpuDxe\IA32_X64\IA32\abc.nasm > > CpuDxe\IA32_X64\X64\abc.nasm > > > > > > > > CpuDxe\IA32_X64\CpuDxe.c CpuDxe\IA32_X64\AmdCpuDxe.c > > > > > > > > CpuDxe\IA32_X64\IntelCpuDxe.c CpuDxe\RiscV64\CpuDxe.c > > > > > > > > CpuDxe\ARM\CpuDxe.c CpuDxe\ > > > > > > > > CpuDxeCommon.c // If required. > > > > > > > > CpuDxe.inf // Use INF section arch modifier for > X86, > > > > > RISC-V > > > > > > > and ARM. > > > > > > > > CpuDxeAmd.inf // Separate INF for AMD. > > > > > > > > > > > > > > > > Seems ok, but is AARCH64_RISCV64 a real case? Or it means > > > > > > > > we can create a folder "AARCH64_RISCV64" when there are > some > > > > > > > > common > > > > > files > > > > > > > shared by AARCH64 and RISCV64? > > > > > > > > > > > > > > > > For the 32/64 bit support, it can still stay under module > > > > > > > > root and don't need to assign a folder for them unless the > > > > > > > > arch has the different > > > > > > > implementation. > > > > > > > > Regards, > > > > > > > > Abner > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > From: Kinney, Michael D <michael.d.kinney@...> > > > > > > > > > Sent: Saturday, October 1, 2022 2:52 AM > > > > > > > > > To: devel@edk2.groups.io; quic_llindhol@...; > > > > > > > > > Chang, Abner <Abner.Chang@...>; Ni, Ray > > > > > > > > > <ray.ni@...>; Attar, AbdulLateef (Abdul Lateef) > > > > > > > > > <AbdulLateef.Attar@...>; Sunil V L > > > > > > > > > <sunilvl@...>; Kinney, Michael D > > > > > > > > > <michael.d.kinney@...> > > > > > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > <Paul.Grimes@...>; He, Jiangang > > <Jiangang.He@...>; > > > > > > > > > Andrew Fish <afish@...> > > > > > > > > > Subject: RE: [edk2-devel] The principles of EDK2 module > > > > > > > > > reconstruction for archs > > > > > > > > > > > > > > > > > > Caution: This message originated from an External Source. > > > > > > > > > Use proper caution when opening attachments, clicking > > > > > > > > > links, or > > > > > responding. > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Leif, > > > > > > > > > > > > > > > > > > Concatenation is a good idea. Makes it more obvious and > > > > > > > > > can be easily adopted for new CPU archs. > > > > > > > > > > > > > > > > > > There is an additional case where an implementation does > > > > > > > > > not have differences based on CPU Arch or Vendor, but it > > > > > > > > > does have differences based on the bit- width of the CPU. > > > > > > > > > Take BaseSafeIntLib as > > > > > > > an example. > > > > > > > > > It has source files for 32-bit CPUs, 64-bit CPUs, and CPU > > > > > > > > > arch specific file for Ebc because Ebc adapts to 32-bit or > > > > > > > > > 64-bit depending on the CPU type the EBC Interpreter is > running. > > > > > > > > > > > > > > > > > > MdePkg/Library/BaseSafeIntLib/ > > > > > > > > > BaseSafeIntLib.inf > > > > > > > > > SafeIntLib.c > > > > > > > > > SafeIntLib32.c > > > > > > > > > SafeIntLib64.c > > > > > > > > > SafeIntLibEbc.c > > > > > > > > > > > > > > > > > > Should we add "32" and "64" as supported suffices in file > names? > > > > > > > > > > > > > > > > > > If we wanted to put type content into a subdirectory, what > > > > > > > > > would be the suggested directory name for "32" and "64". > > > > > > > > > Or do we want to require this type of difference to always > > > > > > > > > be files in top level directory of > > > > > > > the modules/library? > > > > > > > > > > > > > > > > > > Best regards, > > > > > > > > > > > > > > > > > > Mike > > > > > > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On > > > > > > > > > > Behalf Of Leif Lindholm > > > > > > > > > > Sent: Friday, September 30, 2022 9:09 AM > > > > > > > > > > To: devel@edk2.groups.io; Kinney, Michael D > > > > > > > > > > <michael.d.kinney@...>; Chang, Abner > > > > > > > <Abner.Chang@...>; > > > > > > > > > > Ni, Ray <ray.ni@...>; Attar, AbdulLateef (Abdul > > > > > > > > > > Lateef) <AbdulLateef.Attar@...>; Sunil V L > > > > > > > > > > <sunilvl@...> > > > > > > > > > > Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > > <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > <Paul.Grimes@...>; > > > > > > > > > > He, Jiangang <Jiangang.He@...>; Andrew Fish > > > > > > > <afish@...> > > > > > > > > > > Subject: Re: [edk2-devel] The principles of EDK2 module > > > > > > > > > > reconstruction for archs > > > > > > > > > > > > > > > > > > > > I agree similar things will certainly happen for > > > > > > > > > > ARM/AARCH64, which will probably be noticeable when I > > > > > > > > > > start eradicating ArmPkg and putting the arch-standard > > > > > > > > > > bits into > > > (mostly) MdePkg. > > > > > > > > > > > > > > > > > > > > But I like the ability to see already at the filesystem > > > > > > > > > > level which files belong to the architecture I'm > > > > > > > > > > currently working on and > > > > > > > which don't. > > > > > > > > > > > > > > > > > > > > Could we concatenate architectures? > > > > > > > > > > I.e. AARCH64_ARM, IA32_X64, AARCH64_RISCV64... ? > > > > > > > > > > > > > > > > > > > > / > > > > > > > > > > Leif > > > > > > > > > > > > > > > > > > > > On 2022-09-30 08:28, Michael D Kinney wrote: > > > > > > > > > > > Hi Abner, > > > > > > > > > > > > > > > > > > > > > > One comment is on adding a new CPU type dir name of > 'X86' > > > > > > > > > > > for content that is common for IA32/X64. > > > > > > > > > > > > > > > > > > > > > > I can imagine a similar case for ARM/AARCH64 and for > > > > > > > > > > > the RISCV (32, 64, 128) cases. > > > > > > > > > > > > > > > > > > > > > > I think I would prefer to drop X86 and if there are > > > > > > > > > > > files that are common to multiple CPU architectures > > > > > > > > > > > then they are considered common and are in top > > > > > > > > > > > directory of module and the file header and INF file > > > > > > > > > > > can clearly document which CPU archs the file > > > > > > > applies. > > > > > > > > > > > > > > > > > > > > > > Mike > > > > > > > > > > > > > > > > > > > > > >> -----Original Message----- > > > > > > > > > > >> From: Chang, Abner <Abner.Chang@...> > > > > > > > > > > >> Sent: Friday, September 30, 2022 12:11 AM > > > > > > > > > > >> To: Ni, Ray <ray.ni@...>; Attar, AbdulLateef > > > > > > > > > > >> (Abdul > > > > > > > > > > >> Lateef) <AbdulLateef.Attar@...>; Sunil V L > > > > > > > > > > >> <sunilvl@...>; devel@edk2.groups.io; > > > > > > > > > > >> Kinney, Michael D <michael.d.kinney@...> > > > > > > > > > > >> Cc: lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > > >> <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > > >> <Paul.Grimes@...>; He, Jiangang > > > > > <Jiangang.He@...>; > > > > > > > Leif > > > > > > > > > > >> Lindholm <quic_llindhol@...>; Andrew Fish > > > > > > > > > > >> <afish@...> > > > > > > > > > > >> Subject: RE: [edk2-devel] The principles of EDK2 > > > > > > > > > > >> module reconstruction for archs > > > > > > > > > > >> > > > > > > > > > > >> [AMD Official Use Only - General] > > > > > > > > > > >> > > > > > > > > > > >> Thanks Ray, here are my responses. > > > > > > > > > > >> https://nam11.safelinks.protection.outlook.com/?url=h > > > > > > > > > > >> tt > > > > > > > > > > >> ps%3 > > > > > > > > > > >> A%2F > > > > > > > > > > >> %2Fg > > > > > > > > > > >> ithub.com%2Ftianocore-docs%2Fedk2- > > > > > > > CCodingStandardsSpecification > > > > > > > > > > >> %2Fp > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > ull%2F2&data=05%7C01%7CAbner.Chang%40amd.com%7C7c3292c8bd2 > > > > > > > f4 > > > > > > > > > 86f > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > 920908daa314e8e6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6 > > > > > > > > > 3800 > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > 1607445876768%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLC > > > > > > > JQ > > > > > > > > > IjoiV > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata > > > > > > > = > > > > > > > > > HAq > > > > > > > > > > >> > > > > > > > > > > > > > > ou4JyY1yxDthuQ1dEKcF7Q3o4W77Oo9rOOvkXNWU%3D&reserved=0 > > > > > > > > > > >> > > > > > > > > > > >> @Kinney, Michael D we may also need your > > > > > > > > > > >> clarification on the > > > > > > > comments. > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >>> -----Original Message----- > > > > > > > > > > >>> From: Ni, Ray <ray.ni@...> > > > > > > > > > > >>> Sent: Thursday, September 29, 2022 3:42 PM > > > > > > > > > > >>> To: Attar, AbdulLateef (Abdul Lateef) > > > > > > > > > > >>> <AbdulLateef.Attar@...>; Chang, Abner > > > > > > > > > > >>> <Abner.Chang@...>; Sunil V L > > > > > > > > > > >>> <sunilvl@...>; devel@edk2.groups.io > > > > > > > > > > >>> Cc: Kinney, Michael D <michael.d.kinney@...>; > > > > > > > > > > >>> lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > > >>> <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > > >>> <Paul.Grimes@...>; He, Jiangang > > > > > <Jiangang.He@...>; > > > > > > > > > > >>> Leif Lindholm <quic_llindhol@...>; Andrew > > > > > > > > > > >>> Fish <afish@...> > > > > > > > > > > >>> Subject: RE: [edk2-devel] The principles of EDK2 > > > > > > > > > > >>> module reconstruction for archs > > > > > > > > > > >>> > > > > > > > > > > >>> Caution: This message originated from an External > Source. > > > > > > > > > > >>> Use proper caution when opening attachments, > > > > > > > > > > >>> clicking links, or > > > > > > > responding. > > > > > > > > > > >>> > > > > > > > > > > >>> > > > > > > > > > > >>> Abner, > > > > > > > > > > >>> Comments in > > > > > > > > > > >>> https://nam11.safelinks.protection.outlook.com/?url= > > > > > > > > > > >>> ht > > > > > > > > > > >>> tps% > > > > > > > > > > >>> 3A%2 > > > > > > > > > > >>> F%2F > > > > > > > > > > >>> gith > > > > > > > > > > >>> ub.com%2Ftianocore-docs%2Fedk2- > > > > > > > > > > >>> > CCodingStandardsSpecification%2Fpull%2F2%23pullreque > > > > > > > > > > >>> st > > > > > > > > > > >>> revi > > > > > > > > > > >>> ew- > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 1124763311&data=05%7C01%7CAbner.Chang%40amd.com%7Cd825e24 > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 8625541e3f43e08daa1ee2883%7C3dd8961fe4884e608e11a82d994e183d%7C0 > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > %7C0%7C638000341502885565%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM > C > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > 4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000% > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > 7C%7C%7C&sdata=RXxgpbEr6ivbqP1R6%2B3Rxl%2ByJAnaUJuaYYKdfCH > > > > > > > > > > >>> 8jo8%3D&reserved=0 > > > > > > > > > > >>> > > > > > > > > > > >>> We can discuss more in tomorrow's meeting. > > > > > > > > > > >>> > > > > > > > > > > >>> > > > > > > > > > > >>>> -----Original Message----- > > > > > > > > > > >>>> From: Attar, AbdulLateef (Abdul Lateef) > > > > > > > > > > >>>> <AbdulLateef.Attar@...> > > > > > > > > > > >>>> Sent: Thursday, September 29, 2022 3:11 PM > > > > > > > > > > >>>> To: Chang, Abner <Abner.Chang@...>; Sunil V L > > > > > > > > > > >>>> <sunilvl@...>; devel@edk2.groups.io; > > > > > > > > > > >>>> Ni, Ray <ray.ni@...> > > > > > > > > > > >>>> Cc: Kinney, Michael D <michael.d.kinney@...>; > > > > > > > > > > >>>> lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > > >>>> <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > > >>>> <Paul.Grimes@...>; > > > > > > > > > > >>> He, > > > > > > > > > > >>>> Jiangang <Jiangang.He@...>; Leif Lindholm > > > > > > > > > > >>>> <quic_llindhol@...>; Andrew Fish > > > > > > > > > > >>>> <afish@...> > > > > > > > > > > >>>> Subject: RE: [edk2-devel] The principles of EDK2 > > > > > > > > > > >>>> module reconstruction for archs > > > > > > > > > > >>>> > > > > > > > > > > >>>> Hi Abner, > > > > > > > > > > >>>> Looks good to me. > > > > > > > > > > >>>> Reviewed-by: Abdul Lateef Attar <abdattar@...> > > > > > > > > > > >>>> > > > > > > > > > > >>>> Thanks > > > > > > > > > > >>>> AbduL > > > > > > > > > > >>>> > > > > > > > > > > >>>> -----Original Message----- > > > > > > > > > > >>>> From: Chang, Abner <Abner.Chang@...> > > > > > > > > > > >>>> Sent: 28 September 2022 20:31 > > > > > > > > > > >>>> To: Sunil V L <sunilvl@...>; > > > > > > > > > > >>>> devel@edk2.groups.io; ray.ni@... > > > > > > > > > > >>>> Cc: Kinney, Michael D <michael.d.kinney@...>; > > > > > > > > > > >>>> lichao <lichao@...>; Kirkendall, Garrett > > > > > > > > > > >>>> <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > > >>>> <Paul.Grimes@...>; > > > > > > > > > > >>> He, > > > > > > > > > > >>>> Jiangang <Jiangang.He@...>; Attar, AbdulLateef > > > > > > > > > > >>>> (Abdul > > > > > > > > > > >>>> Lateef) <AbdulLateef.Attar@...>; Leif Lindholm > > > > > > > > > > >>>> <quic_llindhol@...>; Andrew Fish > > > > > > > > > > >>>> <afish@...> > > > > > > > > > > >>>> Subject: RE: [edk2-devel] The principles of EDK2 > > > > > > > > > > >>>> module reconstruction for archs > > > > > > > > > > >>>> > > > > > > > > > > >>>> [AMD Official Use Only - General] > > > > > > > > > > >>>> > > > > > > > > > > >>>> I just had created PR to update edkII C coding > > > > > > > > > > >>>> standard spec for the file and directory naming. We > > > > > > > > > > >>>> can review and confirm this update first and then > > > > > > > > > > >>>> go back to the principles of EDK2 module > > > > > > > > > reconstruction for archs. > > > > > > > > > > >>>> Here is the PR: > > > > > > > > > > >>>> > > > > > > > > > > >>> https://nam11.safelinks.protection.outlook.com/?url= > > > > > > > > > > >>> ht > > > > > > > > > > >>> tps% > > > > > > > > > > >>> 3A%2 > > > > > > > > > > >>> F%2F > > > > > > > > > > >>> gith > > > > > > > > > > >>>> ub.com%2Ftianocore-docs%2Fedk2- > > > > > > > > > > >>> &data=05%7C01%7CAbner.Chang%40amd.c > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > om%7Cd825e248625541e3f43e08daa1ee2883%7C3dd8961fe4884e608e11a82 > > > > > > > > > > >>> d994e18 > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 3d%7C0%7C0%7C638000341502885565%7CUnknown%7CTWFpbGZsb3d8eyJ > > > > > > > > > > >>> WIjoiMC4wLj > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C% > > > > > > > > > > >>> 7C%7C&a > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > mp;sdata=X4z9puj81nIGTqtMxC9igDZyBPOT6OTWSU%2BjoIowo%2BE%3D&a > > > > > > > > > > >>> mp;reserv > > > > > > > > > > >>>> ed=0 > > > > > > > > > > >>>> CCodingStandardsSpecification/pull/2 > > > > > > > > > > >>>> > > > > > > > > > > >>>> The naming rule is mainly for the new module or new > file > > IMO. > > > > > > > > > > >>>> Some existing module may not meet the guidelines > > > > > > > > > > >>>> mentioned in this > > > > > > > > > spec. > > > > > > > > > > >>>> Thus we need the principles of EDK2 module > > > > > > > > > > >>>> reconstruction on the existing module to support > > > > > > > > > > >>>> other processor archs and not impacting the > > > > > > > > > > >>> existing platforms (e.g. > > > > > > > > > > >>>> rename the INF file or directory to meet the > guidelines). > > > > > > > > > > >>>> > > > > > > > > > > >>>> Sunil, seems RISC-V CpuDxe meet the guideline. > > > > > > > > > > >>>> Please check > > > > > it. > > > > > > > > > > >>>> Just feel that having CpuDxe.c to Riscv64 folder > > > > > > > > > > >>>> is not quite a best solution. I think at least we > > > > > > > > > > >>>> can abstract the protocol structure and protocol > > > > > > > > > > >>>> installation under CpuDxe\ and have the arch > > > > > > > > > > >>>> implementation under arch folder. We can discuss > > > > > > > > > > >>>> this later after we confirming the > > > > > > > > > > >>> guideline and principles. > > > > > > > > > > >>>> > > > > > > > > > > >>>> Thanks > > > > > > > > > > >>>> Abner > > > > > > > > > > >>>> > > > > > > > > > > >>>>> -----Original Message----- > > > > > > > > > > >>>>> From: Sunil V L <sunilvl@...> > > > > > > > > > > >>>>> Sent: Wednesday, September 28, 2022 3:34 PM > > > > > > > > > > >>>>> To: devel@edk2.groups.io; ray.ni@... > > > > > > > > > > >>>>> Cc: Chang, Abner <Abner.Chang@...>; Kinney, > > > > > Michael > > > > > > > > > > >>>>> D <michael.d.kinney@...>; lichao > > > > > > > > > > >>>>> <lichao@...>; Kirkendall, Garrett > > > > > > > > > > >>>>> <Garrett.Kirkendall@...>; Grimes, Paul > > > > > > > > > > >>>>> <Paul.Grimes@...>; He, Jiangang > > > > > > > > > > >>>>> <Jiangang.He@...>; Attar, AbdulLateef (Abdul > > > > > > > > > > >>>>> Lateef) <AbdulLateef.Attar@...>; Leif > Lindholm > > > > > > > > > > >>>>> <quic_llindhol@...>; Andrew Fish > > > > > > > > > > >>>>> <afish@...> > > > > > > > > > > >>>>> Subject: Re: [edk2-devel] The principles of EDK2 > > > > > > > > > > >>>>> module reconstruction for archs > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> Caution: This message originated from an External > > Source. > > > > > > > > > > >>>>> Use proper caution when opening attachments, > > > > > > > > > > >>>>> clicking links, or > > > > > > > responding. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> On Wed, Sep 28, 2022 at 03:33:45AM +0000, Ni, Ray > > wrote: > > > > > > > > > > >>>>> Hi Ray, > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> 1. When a new arch's implementation is > > > > > > > > > > >>>>>> introduced to the existing > > > > > > > > > > >>>>> module which was developed for the specific arch: > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> 1. The folder reconstruction: > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> * Create arch folder for the existing arch > > implementation > > > > > > > > > > >>>>>> [Ray] Do you move existing arch implementation to > > > > > > > > > > >>>>>> that arch > > > > > > > folder? > > > > > > > > > > >>>>>> It will > > > > > > > > > > >>>>> break existing platforms a lot. > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> * Create the arch folder for the new introduced > > arch > > > > > > > > > > >>>>>> [Ray] I agree. But if we don't create arch folder > > > > > > > > > > >>>>>> for existing arch > > > > > > > > > > >>>>> implementation, the pkg layout will be a mess. > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>>> [Ray] Hard for me to understand all the principles > here. > > > > > > > > > > >>>>>> Maybe we review > > > > > > > > > > >>>>> existing code including to-be-upstreamed code and > > > > > > > > > > >>>>> decide how > > > > > > > to go. > > > > > > > > > > >>>>>> > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> Could you please take a look below changes which > > > > > > > > > > >>>>> is trying to add RISC-V support for CpuDxe? > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> https://nam11.safelinks.protection.outlook.com/?url= > > > > > > > > > > >>> ht > > > > > > > > > > >>> tps% > > > > > > > > > > >>> 3A%2 > > > > > > > > > > >>> F%2F > > > > > > > > > > >>> gith > > > > > > > > > > >>>>> ub.com%2Ftianocore%2Fedk2- > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > staging%2Fcommit%2Fbba1a11be47dd091734e185afbed73ea75708749& > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > data=05%7C01%7Cabner.chang%40amd.com%7Ca419e6a010d34fde464b08d > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > aa123e080%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63799947 > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 2732494527%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIj > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > oiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sd > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > ata=Vq6pJLnn8yJrJhFZn7LfLbZzrtpG4n1VLWgAil6J38U%3D&reserved=0 > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> https://nam11.safelinks.protection.outlook.com/?url= > > > > > > > > > > >>> ht > > > > > > > > > > >>> tps% > > > > > > > > > > >>> 3A%2 > > > > > > > > > > >>> F%2F > > > > > > > > > > >>> gith > > > > > > > > > > >>>>> ub.com%2Ftianocore%2Fedk2- > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > staging%2Fcommit%2F7fccf92a97a6d0618a20f10622220e78b3687906&da > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > ta=05%7C01%7Cabner.chang%40amd.com%7Ca419e6a010d34fde464b08daa1 > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 23e080%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63799947273 > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 2494527%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > > > 2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata > > > > > > > > > > >>>>> > > > > > > > > > > >>>> > > > > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > =xFmvUv58vh4AUAM17Qy9G5jZWFZlK2Ozt3njpG1e8%2BY%3D&reserv > > > > > > > > > > >>>>> ed=0 > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> What do you suggest with above example? > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> 1) Common INF for all architectures - but modify > > > > > > > > > > >>>>> INF alone, no > > > > > > > > > > >>>>> X86 folder creation. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> This is what I have done in the commit above. May > > > > > > > > > > >>>>> be of least impact to existing code since it is only INF > > change. > > > > > > > > > > >>>>> But like you mentioned this is bit weird that X86 > > > > > > > > > > >>>>> files will remain in root folder directly along > > > > > > > > > > >>>>> with some common > > > > > files. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> 2) Common INF (CpuDxe.inf) + create arch folders > > > > > > > > > > >>>>> X86, X64, IA32, > > > > > > > > > > >>>>> RiscV64 etc > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> IMO, this is probably the best approach. What > > > > > > > > > > >>>>> would be the challenges with this? > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> 3) Separate INF for arch like CpuDxe.inf for x86, > > > > > > > > > > >>>>> CpuDxeRiscV64.inf for > > > > > > > > > > >>>> RISC-V. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> This again probably is not a good idea. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> 4) If the module/library is specific to one arch (ex: > > > > > > > > > > >>>>> SMM(X86), SBI(RISC-V)), then create separate INF. > > > > > > > > > > >>>>> > > > > > > > > > > >>>>> Thanks! > > > > > > > > > > >>>>> Sunil > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > |
||||
|
|
||||
|
Re: [edk2-platforms][PATCH v1 3/3] TigerlakeSiliconPkg: Fix invalid debug macros
Hi Michael,
toggle quoted message
Show quoted text
Please see feedback inline. Thanks, Nate -----Original Message-----That does not seem to be what the original author intended. I suspect this is the intent: DEBUG ((DEBUG_INFO, "GbeMdiGetLanPhyRevision failed to read Revision. Overriding LANPHYPC. Status: %r\n", Status)); //That does not seem to be what the original author intended. I suspect this is the intent: DEBUG ((DEBUG_INFO, "IoApicFound @%x:%x:%x:%x\n", Sbdf.Seg, Sbdf.Bus, Sbdf.Dev, Sbdf.Func)); return TRUE; |
||||
|
|
||||
|
Re: [edk2-platforms][PATCH v1 2/3] KabylakeSiliconPkg: Fix invalid debug macros
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@...>
toggle quoted message
Show quoted text
-----Original Message----- |
||||
|
|
||||
|
Re: [edk2-platforms][PATCH v1 1/3] CoffeelakeSiliconPkg: Fix invalid debug macros
Hi Michael,
toggle quoted message
Show quoted text
Please see feedback inline. Thanks, Nate -----Original Message-----That does not seem to be what the original author intended. I suspect this is the intent: DEBUG ((DEBUG_INFO, "GbeMdiGetLanPhyRevision failed to read Revision. Overriding LANPHYPC. Status: %r\n", Status)); //That does not seem to be what the original author intended. I suspect this is the intent: DEBUG ((DEBUG_INFO, "IoApicFound @%x:%x:%x:%x\n", Sbdf.Seg, Sbdf.Bus, Sbdf.Dev, Sbdf.Func)); return TRUE; |
||||
|
|
||||
|
Re: [PATCH 0/3] CryptoPkg: Add EC key retrieving and signature interface.
Yao, Jiewen
Thanks Qi
toggle quoted message
Show quoted text
Please change the protocol version! -----Original Message----- |
||||
|
|