Marvin, Thanks for the question! You’re right, I need to squash this patch into the applicable patches in the series so that the build fixes are present when the initial changes are added (to maintain bisectability). Surprised no one had mentioned it yet. 😉 I would actually be happy to keep the Arm entry point in StandaloneMmPkg if someone else wanted to tackle the work needed to remove the ArmPkg interface dependencies from the implementation. The necessary abstractions are beyond my ability to bite off right now, so the most direct solution was to relocate the implementation (leveraging the abstraction at the EntryPoint level) to the ArmPkg so that the necessary platforms could consume it. I could also see an argument that it should be platform code, but I think it’s more common than that and would prefer to keep it in EDK2. - Bret From: Marvin Häuser via groups.io Sent: Saturday, November 6, 2021 2:50 AM To: devel@edk2.groups.io; bret@corthon.com Cc: devel@edk2.groups.io; Lindholm, Leif; Ard Biesheuvel; Sean Brogan Subject: [EXTERNAL] Re: [edk2-devel] [PATCH v2 16/16] ArmPlatformPkg: Resolve build errors resulting from package moves Hey Bret, If I understood this correctly, this fixes build issues introduced with the move patch of the same series? In that case, is there no edk2 rule that every commit must compile for the whole tree? We have such a rule downstream that overrides any colliding rules (e.g. "mod only one package at a time") to not break bisectioning. We actually have per-commit builds readily available in a database for some projects to ease it further. No big deal for us as we don't do that with edk2 (yet), but maybe worth considering for the future? :) I'll just ask about another patch here because it doesn't matter for review, but why move the ARM entry point to ArmPkg? I guess because abstracting the ARM-specific things would more or less just make the StandaloneMm library a trivial wrapper? My "issue" with this is that ARM kind of has its own ecosystem in edk2 and without keeping up, it's hard to tell whether to look for ARM implementations of modules and libraries in the "generic" or the ARM packages. Thanks a lot for the series! Best regards, Marvin 02.11.2021 21:21:59 Bret Barkelew : > From: Bret Barkelew > > REF: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3652&data=04%7C01%7CBret.Barkelew%40microsoft.com%7C2d061d1ff7a845914be808d9a10addee%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637717890363754993%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=r5yRvqRrCGPQLNCXfXv4KdmldhxFTpCwdsjMoPcvl4M%3D&reserved=0 > > Cc: Leif Lindholm > Cc: Ard Biesheuvel > Cc: Sean Brogan > Signed-off-by: Bret Barkelew > --- > ArmPlatformPkg/ArmPlatformPkg.dsc | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/ArmPlatformPkg/ArmPlatformPkg.dsc b/ArmPlatformPkg/ArmPlatformPkg.dsc > index 661a4cea220d..3ed0bae87c41 100644 > --- a/ArmPlatformPkg/ArmPlatformPkg.dsc > +++ b/ArmPlatformPkg/ArmPlatformPkg.dsc > @@ -79,6 +79,8 @@ [LibraryClasses.common] > NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf > NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > > + ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf > + > [LibraryClasses.common.PEIM] > HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf > MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf > @@ -92,7 +94,7 @@ [LibraryClasses.common.SEC] > MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf > PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf > > -[LibraryClasses.AARCH64.MM_STANDALONE] > +[LibraryClasses.common.MM_STANDALONE] > HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf > MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf > MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf > -- > 2.31.1.windows.1 > > > >