[edk2-devel] [PATCH] MdePkg/BaseLib:Fix boot DxeCore hang on riscv platform

王洋 posted 1 patch 4 months, 1 week ago
Failed in applying to current master (apply log)
There is a newer version of this series
.../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
[edk2-devel] [PATCH] MdePkg/BaseLib:Fix boot DxeCore hang on riscv platform
Posted by 王洋 4 months, 1 week ago
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 b78424c163..c60fbdb896 100644
--- a/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c
+++ b/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c
@@ -8,6 +8,11 @@

 #include "BaseLibInternals.h"

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

@@ -48,6 +53,6 @@ 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 0000000000..59b8d60e7e
--- /dev/null
+++ b/MdePkg/Library/BaseLib/RiscV64/SwitchStack.S
@@ -0,0 +1,40 @@
+//------------------------------------------------------------------------------
+//
+// 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]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] MdePkg/BaseLib:Fix boot DxeCore hang on riscv platform
Posted by Pedro Falcato 4 months ago
On Tue, Dec 26, 2023 at 2:14 PM 王洋 <wangyang@bosc.ac.cn> wrote:
>
> 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 b78424c163..c60fbdb896 100644
> --- a/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c
> +++ b/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c
> @@ -8,6 +8,11 @@
>
>  #include "BaseLibInternals.h"
>
> +UINTN
> +EFIAPI
> +InternalSwitchStackAsm (
> +  IN     BASE_LIBRARY_JUMP_BUFFER  *JumpBuffer
> +  );
>  /**
>    Transfers control to a function starting with a new stack.
>
> @@ -48,6 +53,6 @@ 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 0000000000..59b8d60e7e
> --- /dev/null
> +++ b/MdePkg/Library/BaseLib/RiscV64/SwitchStack.S
> @@ -0,0 +1,40 @@
> +//------------------------------------------------------------------------------
> +//
> +// 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

Please use git send-email, as per
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process

Thanks,
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112925): https://edk2.groups.io/g/devel/message/112925
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]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdePkg/BaseLib:Fix boot DxeCore hang on riscv platform
Posted by 王洋 4 months ago
Hi,Pedro Falcato


> -----原始邮件-----
> 发件人: "Pedro Falcato" <pedro.falcato@gmail.com>
> 发送时间: 2023-12-26 22:28:53 (星期二)
> 收件人: devel@edk2.groups.io, wangyang@bosc.ac.cn
> 抄送: sunilvl@ventanamicro.com, zhiguang.liu@intel.com, michael.d.kinney@intel.com, gaoliming@byosoft.com.cn, andrei.warkentin@intel.com, wangran@bosc.ac.cn
> 主题: Re: [edk2-devel] [PATCH] MdePkg/BaseLib:Fix boot DxeCore hang on riscv platform
> 
> On Tue, Dec 26, 2023 at 2:14 PM 王洋 <wangyang@bosc.ac.cn> wrote:
> >
> > 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 b78424c163..c60fbdb896 100644
> > --- a/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c
> > +++ b/MdePkg/Library/BaseLib/RiscV64/InternalSwitchStack.c
> > @@ -8,6 +8,11 @@
> >
> >  #include "BaseLibInternals.h"
> >
> > +UINTN
> > +EFIAPI
> > +InternalSwitchStackAsm (
> > +  IN     BASE_LIBRARY_JUMP_BUFFER  *JumpBuffer
> > +  );
> >  /**
> >    Transfers control to a function starting with a new stack.
> >
> > @@ -48,6 +53,6 @@ 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 0000000000..59b8d60e7e
> > --- /dev/null
> > +++ b/MdePkg/Library/BaseLib/RiscV64/SwitchStack.S
> > @@ -0,0 +1,40 @@
> > +//------------------------------------------------------------------------------
> > +//
> > +// 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
> 
> Please use git send-email, as per
> https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process

Thank you for your reminder.

Regards,

Yang

> 
> Thanks,
> Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112939): https://edk2.groups.io/g/devel/message/112939
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]
-=-=-=-=-=-=-=-=-=-=-=-