Re: [edk2-devel] [edk2/master PATCH DxeIplHandoffLib v1] MdePkg/DxeIplHandoffLibNullLib: Abstract DxeIpl

["Abner Chang" <abner.chang@hpe.com>]

Hi Dandan,
We can just halt this patch for now, we are looking for the alternative.

Thanks
Abner

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Abner Chang
> Sent: Monday, March 16, 2020 10:17 AM
> To: Bi, Dandan <dandan.bi@intel.com>; devel@edk2.groups.io
> Cc: Leif Lindholm <leif@nuviainc.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Ard
> Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek <lersek@redhat.com>;
> Schaefer, Daniel (DualStudy) <daniel.schaefer@hpe.com>; Chen, Gilbert
> <gilbert.chen@hpe.com>
> Subject: Re: [edk2-devel] [edk2/master PATCH DxeIplHandoffLib v1]
> MdePkg/DxeIplHandoffLibNullLib: Abstract DxeIpl
> 
> 
> 
> > -----Original Message-----
> > From: Bi, Dandan [mailto:dandan.bi@intel.com]
> > Sent: Monday, March 16, 2020 9:31 AM
> > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>;
> > devel@edk2.groups.io
> > Cc: Leif Lindholm <leif@nuviainc.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Ard
> > Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek
> > <lersek@redhat.com>; Schaefer, Daniel (DualStudy)
> > <daniel.schaefer@hpe.com>; Chen, Gilbert <gilbert.chen@hpe.com>
> > Subject: RE: [edk2/master PATCH DxeIplHandoffLib v1]
> > MdePkg/DxeIplHandoffLibNullLib: Abstract DxeIpl
> >
> > Hi Abner,
> >
> > Some comments as below.
> >
> > 1. For the patch itself
> > a)  it introduces a new library class, so besides the instance, we
> > also need to add the header file (public interface definitions) in the
> > include/library directory and define the library class in dec file.
> > b)  EFIAPI keyword should be added with the public API definition (
> > HandOffToDxeCore).
> Ok.  Will resend the patch.
> 
> >
> > 2. This path is just to add an empty instance for now.
> > Abner, will you also add other instances for other Archs and update
> > DxeIpl to consume the new library?
> The purpose of this change is to abstract arch from DxeIpl module under
> MdeModulePkg and remove the dependencies with arch package from
> MdeModulePkg.ci.yaml.
> Yes, I already added an instance for RISC-V and revised DxeIpl to consume
> new lib *only* for RISCV64 arch,  this change will be in the set of RISC-V edk2
> patches. The patch you are reviewing now is the prerequisite for RISC-V edk2
> port.
> 
> > And for platform, it's incompatible change to use new added library
> instance.
> > So it's better review it in TianoCore Design Meeting firstly.
> It will stay the same for X86, ARM may have to use this NULL instance and
> remove the dependencies from MdeModulePkg.ci.yaml as well. However, I
> can't speak for ARM because the dependence with ARM package in
> MdeModulePkg has been in MdeModulePkg.ci.yaml when edk2 CI was
> introduced (my guess, not sure the history). RISCV64 is a new arch and was
> requested to be decoupled from MdeModulePkg.  This change don't bring
> incompatible issue though. ARM arch can still stay the same as it is in edk2
> now.
> 
> >
> >
> >
> > Thanks,
> > Dandan
> > > -----Original Message-----
> > > From: Abner Chang [mailto:abner.chang@hpe.com]
> > > Sent: Monday, March 9, 2020 6:28 PM
> > > To: devel@edk2.groups.io
> > > Cc: abner.chang@hpe.com; Leif Lindholm <leif@nuviainc.com>; Kinney,
> > > Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > <liming.gao@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>;
> > > Laszlo Ersek <lersek@redhat.com>; Bi, Dandan <dandan.bi@intel.com>;
> > > Daniel Schaefer <daniel.schaefer@hpe.com>; Gilbert Chen
> > > <gilbert.chen@hpe.com>
> > > Subject: [edk2/master PATCH DxeIplHandoffLib v1]
> > > MdePkg/DxeIplHandoffLibNullLib: Abstract DxeIpl
> > >
> > > BZ:2583:
> > > INVALID URI REMOVED
> > > e.org_show-5Fbug.cgi-3Fid-
> > 3D2583&d=DwIFAg&c=C5b8zRQO1miGmBeVZ2LFWg&r=_
> > >
> >
> SN6FZBN4Vgi4Ulkskz6qU3NYRO03nHp9P7Z5q59A3E&m=YQR7NX_kxz4BPRET
> > p5nNLWWOK
> > >
> >
> NOimkostEzdrvyvPkA&s=7wyyAOitp2IvMKv19tlpbJxt2m0bn_ZsR4R7llYI19c&
> > e=
> > >
> > > Current DxeIpl has bindings for different processor architectures,
> > > this results in MdeModulePkg has the dependence with processor
> > > architecture packages such as ArmPkg or RiscVPkg. This also leads CI
> > > testing to error during package dependency check. Provide a default
> > > DxeIplHandoff library to abstract processor architecture from DxeIpl
> > > driver, platform can provide its own library instance for the
> > > processor
> > architecture- specific implementation.
> > >
> > > Signed-off-by: Abner Chang <abner.chang@hpe.com>
> > >
> > > Cc: Leif Lindholm <leif@nuviainc.com>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > Cc: Liming Gao <liming.gao@intel.com>
> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > Cc: Dandan Bi <dandan.bi@intel.com>
> > > Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> > > Cc: Gilbert Chen <gilbert.chen@hpe.com>
> > > ---
> > >  MdeModulePkg/MdeModulePkg.dsc                 |  2 ++
> > >  .../DxeIplHandoffLibNull.inf                  | 30 +++++++++++++++++
> > >  .../DxeIplHandoffLibNull.c                    | 33 +++++++++++++++++++
> > >  .../DxeIplHandoffLibNull.uni                  | 14 ++++++++
> > >  4 files changed, 79 insertions(+)
> > >  create mode 100644
> > > MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHandoffLibNull.inf
> > >  create mode 100644
> > > MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHandoffLibNull.c
> > >  create mode 100644
> > > MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHandoffLibNull.uni
> > >
> > > diff --git a/MdeModulePkg/MdeModulePkg.dsc
> > > b/MdeModulePkg/MdeModulePkg.dsc index f7dbb27ce2..6eb922dfaa
> > > 100644
> > > --- a/MdeModulePkg/MdeModulePkg.dsc
> > > +++ b/MdeModulePkg/MdeModulePkg.dsc
> > > @@ -3,6 +3,7 @@
> > >  # # (C) Copyright 2014 Hewlett-Packard Development Company,
> > > L.P.<BR> # Copyright (c) 2007 - 2019, Intel Corporation. All rights
> > > reserved.<BR>+# Copyright (c) 2020, Hewlett Packard Enterprise
> > Development LP. All rights
> > > reserved.<BR> # #    SPDX-License-Identifier: BSD-2-Clause-Patent #@@ -
> > > 321,6 +322,7 @@
> > >    MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
> > >
> >
> MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateP
> > > rogressLibGraphics.inf
> > >
> >
> MdeModulePkg/Library/DisplayUpdateProgressLibText/DisplayUpdateProgr
> > > essLibText.inf+
> > > MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHandoffLibNull.inf
> > > MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> > >
> >
> MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenuAp
> > > p.infdiff --git
> > >
> a/MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHandoffLibNull.inf
> > >
> b/MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHandoffLibNull.inf
> > > new file mode 100644
> > > index 0000000000..b7210656b7
> > > --- /dev/null
> > > +++
> > >
> b/MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHandoffLibNull.inf
> > > @@ -0,0 +1,30 @@
> > > +## @file+#  Null DXE IPL handoff to DXE Core Library instance.+#+#
> > > Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All
> > > rights reserved.<BR>+#+#  SPDX-License-Identifier: BSD-2-Clause-
> > > Patent+#+##++[Defines]+  INF_VERSION                    = 0x0001001b+
> > > BASE_NAME                      = DxeIplHandoffLibNull+  MODULE_UNI_FILE
> > > = DxeIplHandoffLibNull.uni+  FILE_GUID                      = 5c18812d-3684-4093-
> > > bc75-fc846a595353+  MODULE_TYPE                    = BASE+  VERSION_STRING
> > > = 1.0+  LIBRARY_CLASS                  = DxeIplHandoffLib++#+# The following
> > > information is for reference only and not required by the build tools.+#+#
> > > VALID_ARCHITECTURES           = RISCV64+#++[Sources]+
> > > DxeIplHandoffLibNull.c++[Packages]+  MdePkg/MdePkg.dec+diff --git
> > > a/MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHandoffLibNull.c
> > > b/MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHandoffLibNull.c
> > > new file mode 100644
> > > index 0000000000..c83922cb6e
> > > --- /dev/null
> > > +++
> > b/MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHandoffLibNull.c
> > > @@ -0,0 +1,33 @@
> > > +/** @file+  NULL instance of DXE IPL handoff to DXE Core Library.++
> > > Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All
> > > rights reserved.<BR>++  SPDX-License-Identifier: BSD-2-Clause-
> > > Patent++**/++#include <PiPei.h>+#include
> <Library/DebugLib.h>++/**+
> > > Transfers control to DxeCore.++   This function performs a CPU
> architecture
> > > specific operations to execute+   the entry point of DxeCore with the
> > > parameters of HobList.+   It also installs EFI_END_OF_PEI_PPI to signal the
> > > end of PEI phase.++   @param DxeCoreEntryPoint         The entry point of
> > > DxeCore.+   @param HobList                   The start of HobList passed to
> > > DxeCore.++**/+VOID+HandOffToDxeCore (+  IN
> EFI_PHYSICAL_ADDRESS
> > > DxeCoreEntryPoint,+  IN EFI_PEI_HOB_POINTERS   HobList+  )+{+  DEBUG
> > > ((DEBUG_INFO, "No implementation of DXE IPL handoff to DXE Core
> > > library.\r\n"));+  ASSERT (FALSE);+}+diff --git
> > >
> a/MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHandoffLibNull.uni
> > >
> b/MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHandoffLibNull.uni
> > > new file mode 100644
> > > index 0000000000..5a8973d6e5
> > > --- /dev/null
> > > +++
> > >
> b/MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHandoffLibNull.uni
> > > @@ -0,0 +1,14 @@
> > > +// /** @file+// Null DXE IPL handoff to DXE Core Library
> > > +instance.+//+//
> > > Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All
> > > rights reserved.<BR>+//+// SPDX-License-Identifier: BSD-2-Clause-
> > Patent+//+//
> > > **/+++#string STR_MODULE_ABSTRACT             #language en-US "Null
> DXE
> > > IPL handoff to DXE Core Library instance."++#string
> > > STR_MODULE_DESCRIPTION          #language en-US "Null DXE IPL handoff
> to
> > > DXE Core Library instance."+--
> > > 2.25.0
> 
> 
>