Re: [PATCH] StandaloneMmPkg: make package .DSC file build again

["Ard Biesheuvel" <ard.biesheuvel@linaro.org>]

On Mon, 16 Sep 2019 at 19:26, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 09/16/19 17:06, Yao, Jiewen wrote:
> > That is correct.
> >
> > Current trunk only supports ARM system.
> >
> > I have branch to support x86 - https://github.com/jyao1/edk2/tree/StandaloneSmmX86Poc
> > But that is not merged into trunk yet.
>
> In that case, for this patch:
>
> Build-tested-by: Laszlo Ersek <lersek@redhat.com>
>
> (apologies, my prior R-b was a typo; I pressed the wrong hotkey for
> inserting the tag in the email)
>
> I still think the IA32/X64 part should be removed from
> SUPPORTED_ARCHITECTURES (and their addition should be a part of Jiewen's
> topic branch). But, I agree such a cleanup is out of scope for this
> patch; the patch does fix the AARCH64 regression.
>
> Thank you, Ard!
> Laszlo
>

Pushed as 9790f62be1aa..82c1a2120855

Thanks all.

> >> -----Original Message-----
> >> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> Sent: Monday, September 16, 2019 1:18 PM
> >> To: Laszlo Ersek <lersek@redhat.com>
> >> Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Achin Gupta
> >> <achin.gupta@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>
> >> Subject: Re: [PATCH] StandaloneMmPkg: make package .DSC file build again
> >>
> >> On Mon, 16 Sep 2019 at 12:13, Laszlo Ersek <lersek@redhat.com> wrote:
> >>>
> >>> Hi Ard,
> >>>
> >>> On 09/13/19 21:04, Ard Biesheuvel wrote:
> >>>> The StandaloneMmPkg .DSC file went out of sync with the changes
> >>>> applied to the package when I enabled this code on the Synquacer
> >>>> platform in edk2-platforms. So apply the necessary changes to make
> >>>> this package build in isolation.
> >>>>
> >>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>>> ---
> >>>>  StandaloneMmPkg/StandaloneMmPkg.dsc | 19 +++++++++++--------
> >>>>  1 file changed, 11 insertions(+), 8 deletions(-)
> >>>>
> >>>> diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc
> >> b/StandaloneMmPkg/StandaloneMmPkg.dsc
> >>>> index 8c5b9b3a3d47..8a68d397469b 100644
> >>>> --- a/StandaloneMmPkg/StandaloneMmPkg.dsc
> >>>> +++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
> >>>> @@ -39,29 +39,32 @@
> >>>>    BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> >>>>    DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
> >>>>
> >> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDe
> >> bugPrintErrorLevelLib.inf
> >>>> +
> >> ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/P
> >> rePiExtractGuidedSectionLib.inf
> >>>>    FvLib|StandaloneMmPkg/Library/FvLib/FvLib.inf
> >>>> -
> >> HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmC
> >> oreHobLib.inf
> >>>> +
> >> HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLi
> >> b.inf
> >>>>    IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> >>>>
> >> MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMe
> >> mLib.inf
> >>>>
> >> MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmCoreMemoryAl
> >> locationLib/StandaloneMmCoreMemoryAllocationLib.inf
> >>>> +
> >> MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/Standalo
> >> neMmServicesTableLib.inf
> >>>>    PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> >>>> +
> >> PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtraAct
> >> ionLib/StandaloneMmPeCoffExtraActionLib.inf
> >>>>    PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
> >>>>    PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> >>>>
> >> ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseRepo
> >> rtStatusCodeLibNull.inf
> >>>> -
> >>>> -  #
> >>>> -  # Entry point
> >>>> -  #
> >>>> -
> >> StandaloneMmDriverEntryPoint|StandaloneMmPkg/Library/StandaloneMmDriv
> >> erEntryPoint/StandaloneMmDriverEntryPoint.inf
> >>>> +
> >> StandaloneMmCoreEntryPoint|StandaloneMmPkg/Library/StandaloneMmCoreE
> >> ntryPoint/StandaloneMmCoreEntryPoint.inf
> >>>> +
> >> StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoin
> >> t/StandaloneMmDriverEntryPoint.inf
> >>>>
> >>>>  [LibraryClasses.AARCH64]
> >>>>    ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> >>>>
> >> StandaloneMmMmuLib|ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStand
> >> aloneMmLib.inf
> >>>>    ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
> >>>>
> >> CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMa
> >> intenanceLib.inf
> >>>> -
> >> PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtraAct
> >> ionLib/StandaloneMmPeCoffExtraActionLib.inf
> >>>>
> >>>> -
> >> StandaloneMmCoreEntryPoint|StandaloneMmPkg/Library/StandaloneMmCoreE
> >> ntryPoint/StandaloneMmCoreEntryPoint.inf
> >>>> +  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> >>>> +  NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> >>>> +
> >>>> +[LibraryClasses.common.MM_CORE_STANDALONE]
> >>>> +
> >> HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmC
> >> oreHobLib.inf
> >>>>
> >>>>
> >> #################################################################
> >> ###############
> >>>>  #
> >>>>
> >>>
> >>> With this patch applied on top of 9b5a1c789d39, the DSC builds fine for
> >>> AARCH64.
> >>>
> >>> However, the DSC also advertizes IA32 and X64 in
> >>> "SUPPORTED_ARCHITECTURES", and those builds still fail, with:
> >>>
> >>> StandaloneMmPkg/StandaloneMmPkg.dsc(...): error 4000: Instance of
> >>> library class [CacheMaintenanceLib] is not found
> >>>         in [StandaloneMmPkg/Core/StandaloneMmCore.inf] [IA32]
> >>>
> >>> Now, I'm not sure if *this* build breakage is related to the Synquacer
> >>> work you mention in the blurb. If it is, then fixing it would be nice.
> >>> If not, then can we perhaps remove IA32 and X64 from
> >>> SUPPORTED_ARCHITECTURES?
> >>>
> >>> If you end up reposting with IA32/X64 removed from
> >>> SUPPORTED_ARCHITECTURES, then, for that version:
> >>>
> >>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> >>>
> >>
> >> Jiewen should correct me if I am wrong, but as far as I know,
> >> StandaloneMmPkg is ARM-only in practice, even if the PI spec is arch
> >> agnostic about it.
>