I'd go for using "mv" over add with immediate of zero. Also the REG_S/REG_L business doesn't really help with legibility. Why not just use sd and ld? I get that you're copying from RiscVSetJumpLongJump.S...which seems like another issue.
If the function is called SwitchStack, why are you restoring all the other regs? Following the AArch64 version is probably a better idea. Just don't take a BASE_LIBRARY_JUMP_BUFFER as a parameter, take EntryPoint, Context, Context2 and NewStack. And you can even take these in the order of Context, Context2, EntryPoint, NewStack to avoid the extra movement of values in registers. A > -----Original Message----- > From: 王洋 <wangy...@bosc.ac.cn> > Sent: Sunday, December 24, 2023 8:47 PM > To: devel@edk2.groups.io; suni...@ventanamicro.com; Warkentin, Andrei > <andrei.warken...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>; > Kinney, Michael D <michael.d.kin...@intel.com>; Liu, Zhiguang > <zhiguang....@intel.com> > Subject: [Resend PATCH] MdePkg/BaseLib:Fix boot DxeCore hang on riscv > platform > > From f15d405067860a8087c5eb4080bc3e08ca5e0e21 Mon Sep 17 00:00:00 > 2001 > From: wangyang <wangy...@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 <wangy...@bosc.ac.cn> > Reviewed-by: Ran Wang <wang...@bosc.ac.cn> Cc:Andrei Warkentin > <andrei.warken...@intel.com> Cc:Liming Gao <gaolim...@byosoft.com.cn> > Cc:Michael D Kinney <michael.d.kin...@intel.com> Cc:Sunil V L > <suni...@ventanamicro.com> Cc:Zhiguang Liu <zhiguang....@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 (#112927): https://edk2.groups.io/g/devel/message/112927 Mute This Topic: https://groups.io/mt/103369618/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
