Hi Nate, I am out of office most of this week and I will look into that next week.
Would you consider that a requirement to extend onto this patch series? Or could it be a follow up series? Asking due to to the length of this series.
Thanks, Michael On 5/26/2021 4:57 PM, Desimone, Nathaniel L wrote:
Hi Michael, I have been thinking about this more from a long-term maintainability standpoint. The IFWI region enum FLASH_REGION_TYPE looks pretty ripe for causing issues years from now. We should probably convert each member of that enum into a EFI_GUID so that regions can be added/removed as needed. Some of those enum types probably don't belong in IntelSiliconPkg either, like FlashRegion10Gbe_B for example. Thanks, Nate -----Original Message----- From: mikub...@linux.microsoft.com <mikub...@linux.microsoft.com> Sent: Tuesday, May 18, 2021 8:59 PM To: devel@edk2.groups.io Cc: Agyeman, Prince <prince.agye...@intel.com>; Chiu, Chasel <chasel.c...@intel.com>; Kethi Reddy, Deepika <deepika.kethi.re...@intel.com>; Dong, Eric <eric.d...@intel.com>; Luo, Heng <heng....@intel.com>; Jeremy Soller <jer...@system76.com>; Esakkithevar, Kathappan <kathappan.esakkithe...@intel.com>; Liming Gao <gaolim...@byosoft.com.cn>; Desimone, Nathaniel L <nathaniel.l.desim...@intel.com>; Chaganty, Rangasai V <rangasai.v.chaga...@intel.com> Subject: [edk2-platforms][PATCH v2 00/35] Consolidate SpiFlashCommonLib instances From: Michael Kubacki <michael.kuba...@microsoft.com> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3307 SpiFlashCommonLib is duplicated in multiple places across the MinPlatform design in edk2-platforms. I'm planning to build some additional functionality on top of SpiFlashCommonLib and, ideally, this duplication will be consolidated into a single instance usable across all current library consumers. This patch series focuses on consolidating the various SpiFlashCommonLib instances as agreed upon in https://edk2.groups.io/g/devel/message/71701. Read the BZ for more general background around this series. I only have an UpXtreme board on hand so maintainers/reviewers of other board packages should test these changes on those boards. V2 changes: - Rebased patch series on current edk2-platforms master (32183bdaa91) Note: Patch series only received a couple review comments after being on the mailing list for over 1 month. Please be more prompt with v2. Cc: Agyeman Prince <prince.agye...@intel.com> Cc: Chasel Chiu <chasel.c...@intel.com> Cc: Deepika Kethi Reddy <deepika.kethi.re...@intel.com> Cc: Eric Dong <eric.d...@intel.com> Cc: Heng Luo <heng....@intel.com> Cc: Jeremy Soller <jer...@system76.com> Cc: Kathappan Esakkithevar <kathappan.esakkithe...@intel.com> Cc: Liming Gao <gaolim...@byosoft.com.cn> Cc: Nate DeSimone <nathaniel.l.desim...@intel.com> Cc: Rangasai V Chaganty <rangasai.v.chaga...@intel.com> Signed-off-by: Michael Kubacki <michael.kuba...@microsoft.com> Michael Kubacki (35): CometlakeOpenBoardPkg: Remove redundant IntelSiliconPkg.dec entry WhiskeylakeOpenBoardPkg: Remove redundant IntelSiliconPkg.dec entry CometlakeOpenBoardPkg/PeiPolicyUpdateLib: Add missing GUID to INF IntelSiliconPkg: Add BIOS area base address and size PCDs IntelSiliconPkg: Add microcode FV PCDs IntelSiliconPkg: Add PCH SPI PPI IntelSiliconPkg: Add PCH SPI Protocol IntelSiliconPkg: Add SpiFlashCommonLib IntelSiliconPkg: Add SmmSpiFlashCommonLib IntelSiliconPkg: Add MM SPI FVB services CometlakeOpenBoardPkg: Use IntelSiliconPkg BIOS area and ucode PCDs KabylakeOpenBoardPkg: Use IntelSiliconPkg BIOS area and ucode PCDs SimicsOpenBoardPkg: Use IntelSiliconPkg BIOS area and ucode PCDs TigerlakeOpenBoardPkg: Use IntelSiliconPkg BIOS area and ucode PCDs WhiskeylakeOpenBoardPkg: Use IntelSiliconPkg BIOS area and ucode PCDs CoffeelakeSiliconPkg: Use IntelSiliconPkg BIOS area and ucode PCDs KabylakeSiliconPkg: Use IntelSiliconPkg BIOS area and ucode PCDs SimicsIch10Pkg: Use IntelSiliconPkg BIOS area and ucode PCDs TigerlakeSiliconPkg: Use IntelSiliconPkg BIOS are and ucode PCDs CometlakeOpenBoardPkg: Update SpiFvbService & SpiFlashCommonLib KabylakeOpenBoardPkg: Update SpiFvbService & SpiFlashCommonLib SimicsOpenBoardPkg: Update SpiFvbService & SpiFlashCommonLib TigerlakeOpenBoardPkg: Update SpiFvbService & SpiFlashCommonLib WhiskeylakeOpenBoardPkg: Update SpiFvbService & SpiFlashCommonLib MinPlatformPkg: Remove SpiFvbService modules CoffeelakeSiliconPkg: Remove SmmSpiFlashCommonLib KabylakeSiliconPkg: Remove SmmSpiFlashCommonLib SimicsIch10Pkg: Remove SmmSpiFlashCommonLib TigerlakeOpenBoardPkg: Remove SmmSpiFlashCommonLib MinPlatformPkg: Remove SpiFlashCommonLibNull KabylakeOpenBoardPkg/PeiSerialPortLibSpiFlash: Add IntelSiliconPkg.dec CoffeelakeSiliconPkg: Remove PCH SPI PPI and Protocol from package KabylakeSiliconPkg: Remove PCH SPI PPI and Protocol from package SimicsIch10Pkg: Remove PCH SPI SMM Protocol from package TigerlakeSiliconPkg: Remove PCH SPI PPI and Protocol from package Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFlashCommon.c | 196 ------------- Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFlashCommonSmmLib.c | 54 ---- {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/FvbInfo.c | 0 {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/SpiFvbServiceCommon.c | 4 +- {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/SpiFvbServiceMm.c | 8 +- {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/SpiFvbServiceStandaloneMm.c | 0 {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/SpiFvbServiceTraditionalMm.c | 0 Platform/Intel/TigerlakeOpenBoardPkg/Library/SmmSpiFlashCommonLib/SpiFlashCommonSmmLib.c => Silicon/Intel/IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.c | 0 {Platform/Intel/TigerlakeOpenBoardPkg => Silicon/Intel/IntelSiliconPkg}/Library/SmmSpiFlashCommonLib/SpiFlashCommon.c | 3 +- {Platform/Intel/MinPlatformPkg/Flash => Silicon/Intel/IntelSiliconPkg}/Library/SpiFlashCommonLibNull/SpiFlashCommonLibNull.c | 12 +- Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFlashCommon.c | 196 ------------- Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFlashCommonSmmLib.c | 54 ---- Silicon/Intel/SimicsIch10Pkg/Library/SmmSpiFlashCommonLib/SpiFlashCommon.c | 194 ------------- Silicon/Intel/SimicsIch10Pkg/Library/SmmSpiFlashCommonLib/SpiFlashCommonSmmLib.c | 54 ---- Silicon/Intel/SimicsIch10Pkg/LibraryPrivate/BasePchSpiCommonLib/SpiCommon.c | 26 +- Silicon/Intel/SimicsIch10Pkg/Spi/Smm/PchSpi.c | 4 +- Platform/Intel/CometlakeOpenBoardPkg/BiosInfo/BiosInfo.inf | 4 +- Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/Include/Fdf/FlashMapInclude.fdf | 4 +- Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.dsc | 7 +- Platform/Intel/CometlakeOpenBoardPkg/CometlakeURvp/OpenBoardPkg.fdf | 38 +-- Platform/Intel/CometlakeOpenBoardPkg/Policy/Library/PeiPolicyUpdateLib/PeiPolicyUpdateLib.inf | 2 +- Platform/Intel/CometlakeOpenBoardPkg/Policy/PolicyInitDxe/PolicyInitDxe.inf | 4 +- Platform/Intel/KabylakeOpenBoardPkg/BiosInfo/BiosInfo.inf | 4 +- Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/Include/Fdf/FlashMapInclude.fdf | 4 +- Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.dsc | 7 +- Platform/Intel/KabylakeOpenBoardPkg/GalagoPro3/OpenBoardPkg.fdf | 40 +-- Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Include/Fdf/FlashMapInclude.fdf | 4 +- Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.dsc | 7 +- Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/OpenBoardPkg.fdf | 40 +-- Platform/Intel/KabylakeOpenBoardPkg/KabylakeRvp3/Policy/Library/PeiSiliconPolicyUpdateLib/PeiSiliconPolicyUpdateLib.inf | 4 +- Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/PeiSerialPortLibSpiFlash.inf | 1 + Platform/Intel/MinPlatformPkg/Include/Library/SpiFlashCommonLib.h | 98 ------- Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec | 2 - Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc | 6 - Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.dsc | 6 +- Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf | 2 +- Platform/Intel/SimicsOpenBoardPkg/BoardX58Ich10/OpenBoardPkg.fdf.inc | 8 +- Platform/Intel/TigerlakeOpenBoardPkg/BiosInfo/BiosInfo.inf | 8 +- Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/Include/Fdf/FlashMapInclude.fdf | 4 +- Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.dsc | 7 +- Platform/Intel/TigerlakeOpenBoardPkg/TigerlakeURvp/OpenBoardPkg.fdf | 40 +-- Platform/Intel/WhiskeylakeOpenBoardPkg/BiosInfo/BiosInfo.inf | 4 +- Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/Library/PeiPolicyUpdateLib/PeiPolicyUpdateLib.inf | 1 - Platform/Intel/WhiskeylakeOpenBoardPkg/Policy/PolicyInitDxe/PolicyInitDxe.inf | 4 +- Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/Include/Fdf/FlashMapInclude.fdf | 4 +- Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/Library/BoardInitLib/PeiMultiBoardInitPreMemLib.inf | 2 +- Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.dsc | 7 +- Platform/Intel/WhiskeylakeOpenBoardPkg/UpXtreme/OpenBoardPkg.fdf | 38 +-- Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/Include/Fdf/FlashMapInclude.fdf | 4 +- Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.dsc | 7 +- Platform/Intel/WhiskeylakeOpenBoardPkg/WhiskeylakeURvp/OpenBoardPkg.fdf | 38 +-- Silicon/Intel/CoffeelakeSiliconPkg/Cpu/Library/PeiCpuPolicyLib/PeiCpuPolicyLib.inf | 4 +- Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Protocol/Spi.h | 295 -------------------- Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.inf | 1 + Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/Private/BasePchSpiCommonLib/BasePchSpiCommonLib.inf | 1 + Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.inf | 51 ---- Silicon/Intel/CoffeelakeSiliconPkg/Pch/Spi/Smm/PchSpiSmm.inf | 1 + Silicon/Intel/CoffeelakeSiliconPkg/SiPkg.dec | 8 - {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/SpiFvbServiceCommon.h | 0 {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/SpiFvbServiceMm.h | 0 {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/SpiFvbServiceSmm.inf | 6 +- {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/SpiFvbServiceStandaloneMm.inf | 6 +- Silicon/Intel/{CoffeelakeSiliconPkg/Pch => IntelSiliconPkg}/Include/Library/SpiFlashCommonLib.h | 2 +- Silicon/Intel/{CoffeelakeSiliconPkg/Pch => IntelSiliconPkg}/Include/Ppi/Spi.h | 4 +- Silicon/Intel/{TigerlakeSiliconPkg => IntelSiliconPkg}/Include/Protocol/Spi.h | 0 Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 19 ++ Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc | 17 ++ {Platform/Intel/TigerlakeOpenBoardPkg => Silicon/Intel/IntelSiliconPkg}/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.inf | 21 +- {Platform/Intel/MinPlatformPkg/Flash => Silicon/Intel/IntelSiliconPkg}/Library/SpiFlashCommonLibNull/SpiFlashCommonLibNull.inf | 3 +- Silicon/Intel/KabylakeSiliconPkg/Cpu/Library/PeiCpuPolicyLib/PeiCpuPolicyLib.inf | 4 +- Silicon/Intel/KabylakeSiliconPkg/Hsti/Dxe/HstiSiliconDxe.inf | 3 +- Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Library/SpiFlashCommonLib.h | 98 ------- Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Ppi/Spi.h | 26 -- Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Protocol/Spi.h | 293 ------------------- Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.inf | 1 + Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.inf | 53 ---- Silicon/Intel/KabylakeSiliconPkg/Pch/Spi/Smm/PchSpiSmm.inf | 1 + Silicon/Intel/KabylakeSiliconPkg/SiPkg.dec | 13 +- Silicon/Intel/SimicsIch10Pkg/Ich10Pkg.dec | 11 - Silicon/Intel/SimicsIch10Pkg/Include/Library/SpiFlashCommonLib.h | 98 ------- Silicon/Intel/SimicsIch10Pkg/Include/Protocol/Spi.h | 295 -------------------- Silicon/Intel/SimicsIch10Pkg/IncludePrivate/Library/PchSpiCommonLib.h | 26 +- Silicon/Intel/SimicsIch10Pkg/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.inf | 50 ---- Silicon/Intel/SimicsIch10Pkg/LibraryPrivate/BasePchSpiCommonLib/BasePchSpiCommonLib.inf | 5 +- Silicon/Intel/SimicsIch10Pkg/Spi/Smm/PchSpiSmm.inf | 3 +- Silicon/Intel/TigerlakeSiliconPkg/IpBlock/Spi/LibraryPrivate/BaseSpiCommonLib/BaseSpiCommonLib.inf | 1 + Silicon/Intel/TigerlakeSiliconPkg/IpBlock/Spi/Smm/SpiSmm.inf | 1 + Silicon/Intel/TigerlakeSiliconPkg/Pch/PchInit/Dxe/PchInitDxeTgl.inf | 1 + Silicon/Intel/TigerlakeSiliconPkg/SiPkg.dec | 8 - 89 files changed, 303 insertions(+), 2392 deletions(-) delete mode 100644 Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFlashCommon.c delete mode 100644 Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFlashCommonSmmLib.c rename {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/FvbInfo.c (100%) rename {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/SpiFvbServiceCommon.c (96%) rename {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/SpiFvbServiceMm.c (94%) rename {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/SpiFvbServiceStandaloneMm.c (100%) rename {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/SpiFvbServiceTraditionalMm.c (100%) rename Platform/Intel/TigerlakeOpenBoardPkg/Library/SmmSpiFlashCommonLib/SpiFlashCommonSmmLib.c => Silicon/Intel/IntelSiliconPkg/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.c (100%) rename {Platform/Intel/TigerlakeOpenBoardPkg => Silicon/Intel/IntelSiliconPkg}/Library/SmmSpiFlashCommonLib/SpiFlashCommon.c (95%) rename {Platform/Intel/MinPlatformPkg/Flash => Silicon/Intel/IntelSiliconPkg}/Library/SpiFlashCommonLibNull/SpiFlashCommonLibNull.c (83%) delete mode 100644 Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFlashCommon.c delete mode 100644 Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SpiFlashCommonSmmLib.c delete mode 100644 Silicon/Intel/SimicsIch10Pkg/Library/SmmSpiFlashCommonLib/SpiFlashCommon.c delete mode 100644 Silicon/Intel/SimicsIch10Pkg/Library/SmmSpiFlashCommonLib/SpiFlashCommonSmmLib.c delete mode 100644 Platform/Intel/MinPlatformPkg/Include/Library/SpiFlashCommonLib.h delete mode 100644 Silicon/Intel/CoffeelakeSiliconPkg/Pch/Include/Protocol/Spi.h delete mode 100644 Silicon/Intel/CoffeelakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.inf rename {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/SpiFvbServiceCommon.h (100%) rename {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/SpiFvbServiceMm.h (100%) rename {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/SpiFvbServiceSmm.inf (88%) rename {Platform/Intel/MinPlatformPkg => Silicon/Intel/IntelSiliconPkg/Feature}/Flash/SpiFvbService/SpiFvbServiceStandaloneMm.inf (88%) rename Silicon/Intel/{CoffeelakeSiliconPkg/Pch => IntelSiliconPkg}/Include/Library/SpiFlashCommonLib.h (96%) rename Silicon/Intel/{CoffeelakeSiliconPkg/Pch => IntelSiliconPkg}/Include/Ppi/Spi.h (85%) rename Silicon/Intel/{TigerlakeSiliconPkg => IntelSiliconPkg}/Include/Protocol/Spi.h (100%) rename {Platform/Intel/TigerlakeOpenBoardPkg => Silicon/Intel/IntelSiliconPkg}/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.inf (69%) rename {Platform/Intel/MinPlatformPkg/Flash => Silicon/Intel/IntelSiliconPkg}/Library/SpiFlashCommonLibNull/SpiFlashCommonLibNull.inf (91%) delete mode 100644 Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Library/SpiFlashCommonLib.h delete mode 100644 Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Ppi/Spi.h delete mode 100644 Silicon/Intel/KabylakeSiliconPkg/Pch/Include/Protocol/Spi.h delete mode 100644 Silicon/Intel/KabylakeSiliconPkg/Pch/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.inf delete mode 100644 Silicon/Intel/SimicsIch10Pkg/Include/Library/SpiFlashCommonLib.h delete mode 100644 Silicon/Intel/SimicsIch10Pkg/Include/Protocol/Spi.h delete mode 100644 Silicon/Intel/SimicsIch10Pkg/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.inf -- 2.28.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#75814): https://edk2.groups.io/g/devel/message/75814 Mute This Topic: https://groups.io/mt/82929191/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
