Hi Andrei Warkentin:
Thank you very much for your attention！
> -----原始邮件-----
> 发件人: "Andrei Warkentin" <andrei.warken...@intel.com>
> 发送时间: 2023-12-27 05:12:07 (星期三)
> 收件人: 王洋 <wangy...@bosc.ac.cn>, "devel@edk2.groups.io" <devel@edk2.groups.io>,
> "suni...@ventanamicro.com" <suni...@ventanamicro.com>, "Gao, Liming"
> <gaolim...@byosoft.com.cn>, "Kinney, Michael D" <michael.d.kin...@intel.com>,
> "Liu, Zhiguang" <zhiguang....@intel.com>
> 抄送:
> 主题: Re: [edk2-devel] [Resend PATCH] MdePkg/BaseLib:Fix boot DxeCore hang on
> riscv platform
>
> I'd go for using "mv" over add with immediate of zero.
I agree.
>
> 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.
Yes, but with modifications.Fixed the problem scenario described by my commit.
>
> 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.
Thank you very much for your suggestion;I think your suggestion is more logical.
I have tested the patch you suggested, and it can also work.
I will second V2 of the patch based on your suggestion.
Thank you very much for your suggestion.
Regards,
Yang
>
> 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 (#112938): https://edk2.groups.io/g/devel/message/112938
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]
-=-=-=-=-=-=-=-=-=-=-=-
