Re: [EXTERNAL] Re: [edk2-devel] [PATCH v2 16/16] ArmPlatformPkg: Resolve build errors resulting from package moves

["Bret Barkelew" <bret.barkelew@microsoft.com>]

[-- Attachment #1: Type: text/plain, Size: 4664 bytes --]

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<mailto:mhaeuser=posteo.de@groups.io>
Sent: Saturday, November 6, 2021 2:50 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; bret@corthon.com<mailto:bret@corthon.com>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Lindholm, Leif<mailto:leif@nuviainc.com>; Ard Biesheuvel<mailto:ardb+tianocore@kernel.org>; Sean Brogan<mailto:sean.brogan@microsoft.com>
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 <bret@corthon.com>:

> From: Bret Barkelew <brbarkel@microsoft.com>
>
> REF: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3652&amp;data=04%7C01%7CBret.Barkelew%40microsoft.com%7C2d061d1ff7a845914be808d9a10addee%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637717890363754993%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=r5yRvqRrCGPQLNCXfXv4KdmldhxFTpCwdsjMoPcvl4M%3D&amp;reserved=0
>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Signed-off-by: Bret Barkelew <bret.barkelew@microsoft.com>
> ---
> 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
>
>
>
>






[-- Attachment #2: Type: text/html, Size: 7796 bytes --]