Series comparison

From f15d405067860a8087c5eb4080bc3e08ca5e0e21 Mon Sep 17 00:00:00 2001
From: wangyang <wangyang@bosc.ac.cn>
Date: Wed, 20 Dec 2023 20:27:42 +0800
Subject: [PATCH] MdePkg/BaseLib:Fix boot DxeCore hang on riscv platform

For scene of
HandOffToDxeCore()->SwitchStack(DxeCoreEntryPoint)->
InternalSwitchStack()->LongJump(),Variable HobList.Raw
will be passed (from *Context1 to register a0) to
DxeMain() in parameter *HobStart.

However, meanwhile the function LongJump() overrides
register a0 with a1 (-1)  due to commit (ea628f28e5 "RISCV: Fix
InternalLongJump to return correct value"), then cause hang.

Replacing calling LongJump() with new InternalSwitchStackAsm() to pass
addres data in register s0 to register a0 could fix this issue (just
like the solution in MdePkg/Library/BaseLib/LoongArch64/SwitchStack.S)

Signed-off-by: Yang Wang <wangyang@bosc.ac.cn>
Reviewed-by: Ran Wang <wangran@bosc.ac.cn>
Cc:Andrei Warkentin <andrei.warkentin@intel.com>
Cc:Liming Gao <gaoliming@byosoft.com.cn>
Cc:Michael D Kinney <michael.d.kinney@intel.com>
Cc:Sunil V L <sunilvl@ventanamicro.com>
Cc:Zhiguang Liu <zhiguang.liu@intel.com>
---
 .../BaseLib/RiscV64/InternalSwitchStack.c     |  7 +++-
 MdePkg/Library/BaseLib/RiscV64/SwitchStack.S  | 40 +++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)
 create mode 100644 MdePkg/Library/BaseLib/RiscV64/SwitchStack.S

diff --git a/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c b/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c
index XXXXXXX..XXXXXXX 100644
--- a/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c
+++ b/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c
@@ -XXX,XX +XXX,XX @@

#include "BaseLibInternals.h"

+UINTN
+EFIAPI
+InternalSwitchStackAsm (
+  IN     BASE_LIBRARY_JUMP_BUFFER  *JumpBuffer
+  );
 /**
   Transfers control to a function starting with a new stack.

@@ -XXX,XX +XXX,XX @@ InternalSwitchStack (
   JumpBuffer.SP = (UINTN)NewStack - sizeof (VOID *);
   JumpBuffer.S0 = (UINT64)(UINTN)Context1;
   JumpBuffer.S1 = (UINT64)(UINTN)Context2;
-  LongJump (&JumpBuffer, (UINTN)-1);
+  InternalSwitchStackAsm (&JumpBuffer);
   ASSERT (FALSE);
 }
diff --git a/MdePkg/Library/BaseLib/RiscV64/SwitchStack.S b/MdePkg/Library/BaseLib/RiscV64/SwitchStack.S
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/MdePkg/Library/BaseLib/RiscV64/SwitchStack.S
@@ -XXX,XX +XXX,XX @@
+//------------------------------------------------------------------------------
+//
+// InternalSwitchStackAsm for RISC-V
+//
+// Copyright (c) 2023, Bosc Corporation. All rights reserved.<BR>
+//
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+//------------------------------------------------------------------------------
+# define REG_S  sd
+# define REG_L  ld
+# define SZREG  8
+.align 3
+
+/**
+  This allows the caller to switch the stack and goes to the new entry point
+
+  @param  JumpBuffer    A pointer to CPU context buffer.
+**/
+
+    .globl InternalSwitchStackAsm
+InternalSwitchStackAsm:
+  REG_L ra,  0*SZREG(a0)
+  REG_L s0,  1*SZREG(a0)
+  REG_L s1,  2*SZREG(a0)
+  REG_L s2,  3*SZREG(a0)
+  REG_L s3,  4*SZREG(a0)
+  REG_L s4,  5*SZREG(a0)
+  REG_L s5,  6*SZREG(a0)
+  REG_L s6,  7*SZREG(a0)
+  REG_L s7,  8*SZREG(a0)
+  REG_L s8,  9*SZREG(a0)
+  REG_L s9,  10*SZREG(a0)
+  REG_L s10, 11*SZREG(a0)
+  REG_L s11, 12*SZREG(a0)
+  REG_L sp,  13*SZREG(a0)
+
+  add   a0, s0, 0
+  add   a1, s1, 0
+  ret
--
2.25.1

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112919): https://edk2.groups.io/g/devel/message/112919
Mute This Topic: https://groups.io/mt/103369616/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

However, meanwhile the function LongJump() overrides
register a0 with a1 (-1)  due to commit (ea628f28e5 "RISCV: Fix
InternalLongJump to return correct value"), then cause hang.

Signed-off-by: Yang Wang <wangyang@bosc.ac.cn>
Reviewed-by: Ran Wang <wangran@bosc.ac.cn>
Cc: Bamvor Jian ZHANG <zhangjian@bosc.ac.cn>
Cc: Andrei Warkentin <andrei.warkentin@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Sunil V L <sunilvl@ventanamicro.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
---
Change in v2:
        - Remove JumpBuffer variable parameter
        - Take these in the order of Context1, Context2, EntryPoint, NewStack 
        - Fix BaseLib.inf, add Compilation SwitchStack.S
        - Drop REG_S/REG_L

MdePkg/Library/BaseLib/BaseLib.inf            |  1 +
 .../BaseLib/RiscV64/InternalSwitchStack.c     | 31 ++++++++++++----
 MdePkg/Library/BaseLib/RiscV64/SwitchStack.S  | 37 +++++++++++++++++++
 3 files changed, 62 insertions(+), 7 deletions(-)
 create mode 100644 MdePkg/Library/BaseLib/RiscV64/SwitchStack.S

diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
index XXXXXXX..XXXXXXX 100644
--- a/MdePkg/Library/BaseLib/BaseLib.inf
+++ b/MdePkg/Library/BaseLib/BaseLib.inf
@@ -XXX,XX +XXX,XX @@
   RiscV64/CpuPause.c
   RiscV64/MemoryFence.S             | GCC
   RiscV64/RiscVSetJumpLongJump.S    | GCC
+  RiscV64/SwitchStack.S             | GCC
   RiscV64/RiscVCpuBreakpoint.S      | GCC
   RiscV64/RiscVCpuPause.S           | GCC
   RiscV64/RiscVInterrupt.S          | GCC
diff --git a/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c b/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c
index XXXXXXX..XXXXXXX 100644
--- a/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c
+++ b/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c
@@ -XXX,XX +XXX,XX @@
 
 #include "BaseLibInternals.h"
 
+/**
+  Transfers control to a function starting with a new stack.
+
+  This internal worker function transfers control to the function
+  specified by EntryPoint using the new stack specified by NewStack,
+  and passes in the parameters specified by Context1 and Context2.
+  Context1 and Context2 are optional and may be NULL.
+  The function EntryPoint must never return.
+
+  @param Context1     The first parameter to pass in.
+  @param Context2     The second Parameter to pass in
+  @param EntryPoint   The pointer to the function to enter.
+  @param NewStack     The new Location of the stack
+
+**/
+VOID
+EFIAPI
+InternalSwitchStackAsm (
+  IN      VOID                      *Context1    OPTIONAL,
+  IN      VOID                      *Context2    OPTIONAL,
+  IN      SWITCH_STACK_ENTRY_POINT  EntryPoint,
+  IN      VOID                      *NewStack
+  );
 /**
   Transfers control to a function starting with a new stack.
 
@@ -XXX,XX +XXX,XX @@ InternalSwitchStack (
   IN      VA_LIST                   Marker
   )
 {
-  BASE_LIBRARY_JUMP_BUFFER  JumpBuffer;
-
-  JumpBuffer.RA = (UINTN)EntryPoint;
-  JumpBuffer.SP = (UINTN)NewStack - sizeof (VOID *);
-  JumpBuffer.S0 = (UINT64)(UINTN)Context1;
-  JumpBuffer.S1 = (UINT64)(UINTN)Context2;
-  LongJump (&JumpBuffer, (UINTN)-1);
+  InternalSwitchStackAsm (Context1, Context2, EntryPoint, (VOID *)((UINTN)NewStack - sizeof (VOID *)));
   ASSERT (FALSE);
 }
diff --git a/MdePkg/Library/BaseLib/RiscV64/SwitchStack.S b/MdePkg/Library/BaseLib/RiscV64/SwitchStack.S
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/MdePkg/Library/BaseLib/RiscV64/SwitchStack.S
@@ -XXX,XX +XXX,XX @@
+//------------------------------------------------------------------------------
+//
+// InternalSwitchStackAsm for RISC-V
+//
+// Copyright (c) 2023, Bosc Corporation. All rights reserved.<BR>
+//
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+//------------------------------------------------------------------------------
+.align 3
+
+#/**
+#
+# This allows the caller to switch the stack and goes to the new entry point
+#
+# @param      Context      Parameter to pass in
+# @param      Context2     Parameter2 to pass in
+# @param      EntryPoint   The pointer to the location to enter
+# @param      NewStack     New Location of the stack
+#
+# @return     Nothing. Goes to the Entry Point passing in the new parameters
+#
+#**/
+#VOID
+#EFIAPI
+#InternalSwitchStackAsm (
+#  VOID  *Context,
+#  VOID  *Context2,
+#  SWITCH_STACK_ENTRY_POINT EntryPoint,
+#  VOID  *NewStack
+#  );
+#
+    .globl InternalSwitchStackAsm
+InternalSwitchStackAsm:
+  mv ra, a2
+  mv sp, a3
+  ret
-- 
2.25.1

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112940): https://edk2.groups.io/g/devel/message/112940
Mute This Topic: https://groups.io/mt/103395756/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-