Re: [edk2-devel] [PATCH v1 3/3] MdeModulePkg/DxeIplPeim : RISC-V platform level DxeIPL

["Dandan Bi" <dandan.bi@intel.com>]

Hi Abner,

1. What's following definition for? It seems not be used.
typedef
VOID*
(EFIAPI *DXEENTRYPOINT) (
  IN  VOID *HobStart
  );

2. When reviewing this patch, found the RSIC-V switchstack related code are not in BaseLib in MdePkg.
  But then noticed that you have covered them in another patch set.
So here I may suggest that maybe you can make the patches which have dependency in one patch set and then CC all reviewers, then it can avoid such confusion and also can make the patches submit in right dependency order.
Since now these patches are in different patch series, please pay attention to the submit order to avoid any build break in this way.


Thanks,
Dandan
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Abner Chang
> Sent: Friday, April 10, 2020 3:26 PM
> To: devel@edk2.groups.io
> Cc: abner.chang@hpe.com; Gilbert Chen <gilbert.chen@hpe.com>; Leif
> Lindholm <leif.lindholm@linaro.org>; Bi, Dandan <dandan.bi@intel.com>;
> Gao, Liming <liming.gao@intel.com>
> Subject: [edk2-devel] [PATCH v1 3/3] MdeModulePkg/DxeIplPeim : RISC-V
> platform level DxeIPL
> 
> Implementation of RISC-V DxeIPL.
> 
> Signed-off-by: Abner Chang <abner.chang@hpe.com>
> Co-authored-by: Gilbert Chen <gilbert.chen@hpe.com>
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> 
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Gilbert Chen <gilbert.chen@hpe.com>
> ---
>  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf       |  6 +-
>  .../Core/DxeIplPeim/RiscV64/DxeLoadFunc.c     | 80 +++++++++++++++++++
>  2 files changed, 85 insertions(+), 1 deletion(-)  create mode 100644
> MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c
> 
> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> index 98bc17fc9d..3f17028546 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> @@ -7,6 +7,7 @@
>  # #  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR> #
> Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>+#  Copyright
> (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved.<BR>
> # #  SPDX-License-Identifier: BSD-2-Clause-Patent #@@ -25,7 +26,7 @@
>  # # The following information is for reference only and not required by the
> build tools. #-#  VALID_ARCHITECTURES           = IA32 X64 EBC (EBC is for build
> only) AARCH64+#  VALID_ARCHITECTURES           = IA32 X64 EBC (EBC is for
> build only) AARCH64 RISCV64 #  [Sources]@@ -49,6 +50,9 @@
>  [Sources.ARM, Sources.AARCH64]   Arm/DxeLoadFunc.c
> +[Sources.RISCV64]+  RiscV64/DxeLoadFunc.c+ [Packages]
> MdePkg/MdePkg.dec   MdeModulePkg/MdeModulePkg.decdiff --git
> a/MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c
> b/MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c
> new file mode 100644
> index 0000000000..051d11de25
> --- /dev/null
> +++ b/MdeModulePkg/Core/DxeIplPeim/RiscV64/DxeLoadFunc.c
> @@ -0,0 +1,80 @@
> +/** @file+  RISC-V specific functionality for DxeLoad.++  Copyright (c) 2020,
> Hewlett Packard Enterprise Development LP. All rights reserved.<BR>++
> SPDX-License-Identifier: BSD-2-Clause-Patent++**/++#include
> "DxeIpl.h"++typedef+VOID*+(EFIAPI *DXEENTRYPOINT) (+  IN  VOID
> *HobStart+  );++/**+   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+  )+{+  VOID                            *BaseOfStack;+
> VOID                            *TopOfStack;+  EFI_STATUS                      Status;+  //+  //+
> // Allocate 128KB for the Stack+  //+  BaseOfStack = AllocatePages
> (EFI_SIZE_TO_PAGES (STACK_SIZE));+  if (BaseOfStack == NULL) {+
> DEBUG((DEBUG_ERROR, "%a: Can't allocate memory for stack.",
> __FUNCTION__));+    ASSERT(FALSE);+  }++  //+  // Compute the top of the
> stack we were allocated. Pre-allocate a UINTN+  // for safety.+  //+
> TopOfStack = (VOID *)((UINTN) BaseOfStack + EFI_SIZE_TO_PAGES
> (STACK_SIZE) * EFI_PAGE_SIZE - CPU_STACK_ALIGNMENT);+  TopOfStack =
> ALIGN_POINTER (TopOfStack, CPU_STACK_ALIGNMENT);++  //+  // End of
> PEI phase signal+  //+  Status = PeiServicesInstallPpi (&gEndOfPeiSignalPpi);+
> if (EFI_ERROR (Status)) {+    DEBUG((DEBUG_ERROR, "%a: Fail to signal End of
> PEI event.", __FUNCTION__));+    ASSERT(FALSE);+  }+  //+  // Update the
> contents of BSP stack HOB to reflect the real stack info passed to DxeCore.+
> //+  UpdateStackHob ((EFI_PHYSICAL_ADDRESS)(UINTN) BaseOfStack,
> STACK_SIZE);++  DEBUG ((DEBUG_INFO, "DXE Core new stack at %x, stack
> pointer at %x\n", BaseOfStack, TopOfStack));++  //+  // Transfer the control
> to the entry point of DxeCore.+  //+  SwitchStack (+
> (SWITCH_STACK_ENTRY_POINT)(UINTN)DxeCoreEntryPoint,+
> HobList.Raw,+    NULL,+    TopOfStack+    );+}+--
> 2.25.0
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> 
> View/Reply Online (#57204): https://edk2.groups.io/g/devel/message/57204
> Mute This Topic: https://groups.io/mt/72916401/1768738
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [dandan.bi@intel.com]
> -=-=-=-=-=-=