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

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

> -----Original Message-----
> From: Kinney, Michael D [mailto:michael.d.kinney@intel.com]
> Sent: Friday, March 20, 2020 9:00 AM
> To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>;
> devel@edk2.groups.io; Bi, Dandan <dandan.bi@intel.com>; Kinney, Michael
> D <michael.d.kinney@intel.com>
> Cc: Leif Lindholm <leif@nuviainc.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
> 
> Abner,
> 
> If the ARM/AARCH64/RISCV64 content required by DxeIpl were added to
> MdePkg or MdeModulePkg could we remove the need for this new lib class?
That is no problem I can see now if DxeIpl has no any dependency with processor architecture package.
> 
> What RISCV64 content needs to be added to MdePkg/MdeModulePkg so all
For RISC-V, we leverage open source RISC-V opensbi library in edk2 port. We switch to SMode (Supervisor mode through OpenSBI in DxeIpl ) for executing DXE phase, we also need to access to opensbi structure which is defined in RiscVPkg.

> CPUs are supported the same way?
For RISC-V processors? Yes.
> 
> Thanks,
> 
> Mike
> 
> > -----Original Message-----
> > From: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>
> > Sent: Wednesday, March 18, 2020 8:22 PM
> > To: devel@edk2.groups.io; Bi, Dandan
> > <dandan.bi@intel.com>
> > 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: devel@edk2.groups.io
> > [mailto:devel@edk2.groups.io] On Behalf Of
> > > Dandan Bi
> > > Sent: Thursday, March 19, 2020 11:09 AM
> > > To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW
> > Technologist)
> > > <abner.chang@hpe.com>
> > > 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: 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.
> > > Abner,
> > > So you will only and one instance used for RISCV64,
> > no instance for other
> > > archs.
> > Yes.
> >
> > > HandOffToDxeCore logic in DxeLoadFunc.c for IA32 and
> > X64 will keep same
> > > with before, do not leverage new DxeIplHandoffLib
> > Then there is no
> > > incompatible concern for IA32 and X64, is that
> > correct?
> > No, no impacts on architectures other than RISC-V.  X86 doesn't have
> > to change because there is no package dependency issues when edk2 CI
> > testing.
> > ARM still have problem if they keep the same way, however they are
> > good because CI package test has an exception for ARM in
> > MdeModulePkg.ci.yaml.
> >
> > I will get on TianoCore design meeting tomorrow and have a short
> > presentation of this change. FYI.
> >
> > >
> > > Thanks,
> > > Dandan
> > > >
> > > > >
> > > > >
> > > > >
> > > > > 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_kxz
> > 4BPRET
> > > > > p5nNLWWOK
> > > > > >
> > > > >
> > > >
> > >
> > NOimkostEzdrvyvPkA&s=7wyyAOitp2IvMKv19tlpbJxt2m0bn_ZsR4
> > R7llYI19c&
> > > > > 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/DxeIplHandoff
> > LibNull.inf
> > > > > >  create mode 100644
> > > > > >
> > MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHandoff
> > LibNull.c
> > > > > >  create mode 100644
> > > > > >
> > >
> > MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHandoff
> > LibNull.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/BaseBmpSupportLi
> > b.inf
> > > > > >
> > > > >
> > > >
> > >
> > MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/D
> > isplayUpdateP
> > > > > > rogressLibGraphics.inf
> > > > > >
> > > > >
> > > >
> > >
> > MdeModulePkg/Library/DisplayUpdateProgressLibText/Displ
> > ayUpdateProgr
> > > > > > essLibText.inf+
> > > > > >
> > >
> > MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHandoff
> > LibNull.inf
> > > > > > MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> > > > > >
> > > > >
> > > >
> > >
> > MdeModulePkg/Application/BootManagerMenuApp/BootManager
> > MenuAp
> > > > > > p.infdiff --git
> > > > > >
> > > >
> > a/MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHando
> > ffLibNull.inf
> > > > > >
> > > >
> > b/MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHando
> > ffLibNull.inf
> > > > > > new file mode 100644
> > > > > > index 0000000000..b7210656b7
> > > > > > --- /dev/null
> > > > > > +++
> > > > > >
> > > >
> > b/MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHando
> > ffLibNull.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/DxeIplHando
> > ffLibNull.c
> > > > > >
> > >
> > b/MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHando
> > ffLibNull.c
> > > > > > new file mode 100644
> > > > > > index 0000000000..c83922cb6e
> > > > > > --- /dev/null
> > > > > > +++
> > > > >
> > b/MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHando
> > ffLibNull.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/DxeIplHando
> > ffLibNull.uni
> > > > > >
> > > >
> > b/MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHando
> > ffLibNull.uni
> > > > > > new file mode 100644
> > > > > > index 0000000000..5a8973d6e5
> > > > > > --- /dev/null
> > > > > > +++
> > > > > >
> > > >
> > b/MdeModulePkg/Library/DxeIplHandoffLibNull/DxeIplHando
> > ffLibNull.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
> > > >
> > > >
> > > >
> > >
> > >
> > > 
> >