| Message ID | 20210602014201.246899-1-atish.patra@wdc.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | firmware: Do not allow harts with different XLEN to boot | expand |
On 2 Jun 2021, at 02:42, Atish Patra <atish.patra@wdc.com> wrote: > > OpenSBI is built separately for RV32/RV64. Thus, it shouldn't try to boot > harts with a different XLEN compared to what it is compiled with. > > This patch fixes the issue observed in beagleV which has two U74(RV64) and > one E24 (RV32) harts. Is this really necessary? Can the different clusters not boot different OpenSBIs? This is a really gross hack for what seems like a stupid hardware decision if they both blindly load and run from the same place. Jess
> -----Original Message----- > From: Jessica Clarke <jrtc27@jrtc27.com> > Sent: 02 June 2021 07:25 > To: Atish Patra <Atish.Patra@wdc.com> > Cc: opensbi@lists.infradead.org; Anup Patel <Anup.Patel@wdc.com> > Subject: Re: [PATCH] firmware: Do not allow harts with different XLEN to boot > > On 2 Jun 2021, at 02:42, Atish Patra <atish.patra@wdc.com> wrote: > > > > OpenSBI is built separately for RV32/RV64. Thus, it shouldn't try to > > boot harts with a different XLEN compared to what it is compiled with. > > > > This patch fixes the issue observed in beagleV which has two U74(RV64) > > and one E24 (RV32) harts. > > Is this really necessary? Can the different clusters not boot different > OpenSBIs? This is a really gross hack for what seems like a stupid hardware > decision if they both blindly load and run from the same place. Ideally, a platform should boot different OpenSBI firmwares for clusters with differing XLEN. In any case, if a RV32 HART enters RV64 OpenSBI firmware (or vice-versa) then the HART should enter a WFI hang loop very early. This patch is more of a precautionary check for platforms like BeagleV where HART with differing XLEN jumps to OpenSBI firmware. Regards, Anup
> -----Original Message----- > From: Atish Patra <atish.patra@wdc.com> > Sent: 02 June 2021 07:12 > To: opensbi@lists.infradead.org > Cc: Anup Patel <Anup.Patel@wdc.com>; Atish Patra <Atish.Patra@wdc.com> > Subject: [PATCH] firmware: Do not allow harts with different XLEN to boot > > OpenSBI is built separately for RV32/RV64. Thus, it shouldn't try to boot harts > with a different XLEN compared to what it is compiled with. > > This patch fixes the issue observed in beagleV which has two U74(RV64) and > one E24 (RV32) harts. > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > --- > firmware/fw_base.S | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/firmware/fw_base.S b/firmware/fw_base.S index > a5ce946aa3fe..ae12a126ca66 100644 > --- a/firmware/fw_base.S > +++ b/firmware/fw_base.S > @@ -47,6 +47,16 @@ > .globl _start > .globl _start_warm > _start: > + csrr t0, misa > + slti t1, t0, 0 > + slli t1, t1, 1 > + slli t0, t0, 1 > + slti t0, t0, 0 > + add t2, t0, t1 > + li t3, __riscv_xlen > + srli t3, t3, 5 > + bne t2, t3, _start_hang > + Unfortunately, MISA can be wired to zero in which case platform will provide non-standard hooks to check extensions. How about this ? li t0, 1 slli t0, t0, 32 #if __riscv_xlen == 32 bne t0, zero, _start_hang #else beq t0, zero, _start_hang #endif Probably above can be ASM macro so that we can call this macro at start of both _start() and _start_warm() > /* Find preferred boot HART id */ > MOV_3R s0, a0, s1, a1, s2, a2 > call fw_boot_hart > -- > 2.31.1 Regards, Anup
在 2021-06-02星期三的 11:51 +0000，Anup Patel写道： > > > > -----Original Message----- > > From: Jessica Clarke <jrtc27@jrtc27.com> > > Sent: 02 June 2021 07:25 > > To: Atish Patra <Atish.Patra@wdc.com> > > Cc: opensbi@lists.infradead.org; Anup Patel <Anup.Patel@wdc.com> > > Subject: Re: [PATCH] firmware: Do not allow harts with different > > XLEN to boot > > > > On 2 Jun 2021, at 02:42, Atish Patra <atish.patra@wdc.com> wrote: > > > > > > OpenSBI is built separately for RV32/RV64. Thus, it shouldn't try > > > to > > > boot harts with a different XLEN compared to what it is compiled > > > with. > > > > > > This patch fixes the issue observed in beagleV which has two > > > U74(RV64) > > > and one E24 (RV32) harts. > > > > Is this really necessary? Can the different clusters not boot > > different > > OpenSBIs? This is a really gross hack for what seems like a stupid > > hardware > > decision if they both blindly load and run from the same place. > > Ideally, a platform should boot different OpenSBI firmwares for > clusters > with differing XLEN. > > In any case, if a RV32 HART enters RV64 OpenSBI firmware (or vice- > versa) > then the HART should enter a WFI hang loop very early. > > This patch is more of a precautionary check for platforms like > BeagleV > where HART with differing XLEN jumps to OpenSBI firmware. > > Regards, > Anup > MXL can be modified, if it does not match, whether to try to modify MXL, and then continue to run Regards, Xiang W
On Wed, Jun 2, 2021 at 5:22 AM Anup Patel <Anup.Patel@wdc.com> wrote: > > > > > -----Original Message----- > > From: Atish Patra <atish.patra@wdc.com> > > Sent: 02 June 2021 07:12 > > To: opensbi@lists.infradead.org > > Cc: Anup Patel <Anup.Patel@wdc.com>; Atish Patra <Atish.Patra@wdc.com> > > Subject: [PATCH] firmware: Do not allow harts with different XLEN to boot > > > > OpenSBI is built separately for RV32/RV64. Thus, it shouldn't try to boot harts > > with a different XLEN compared to what it is compiled with. > > > > This patch fixes the issue observed in beagleV which has two U74(RV64) and > > one E24 (RV32) harts. > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > --- > > firmware/fw_base.S | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/firmware/fw_base.S b/firmware/fw_base.S index > > a5ce946aa3fe..ae12a126ca66 100644 > > --- a/firmware/fw_base.S > > +++ b/firmware/fw_base.S > > @@ -47,6 +47,16 @@ > > .globl _start > > .globl _start_warm > > _start: > > + csrr t0, misa > > + slti t1, t0, 0 > > + slli t1, t1, 1 > > + slli t0, t0, 1 > > + slti t0, t0, 0 > > + add t2, t0, t1 > > + li t3, __riscv_xlen > > + srli t3, t3, 5 > > + bne t2, t3, _start_hang > > + > > Unfortunately, MISA can be wired to zero in which case > platform will provide non-standard hooks to check > extensions. > > How about this ? > > li t0, 1 > slli t0, t0, 32 > #if __riscv_xlen == 32 > bne t0, zero, _start_hang > #else > beq t0, zero, _start_hang > #endif > > Probably above can be ASM macro so that we can call > this macro at start of both _start() and _start_warm() > Sure. I will do that. Just to clarify, we don't need this for Beagleboard. After talking to StarFive engineers, it was confirmed that the U74 and E24 are in different clusters and both contain hart0. However, E24 never enters OpenSBI as it executes code compiled for RV32 only. This patch will just serve as an improved sanity check. > > /* Find preferred boot HART id */ > > MOV_3R s0, a0, s1, a1, s2, a2 > > call fw_boot_hart > > -- > > 2.31.1 > > Regards, > Anup > > > -- > opensbi mailing list > opensbi@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/opensbi
diff --git a/firmware/fw_base.S b/firmware/fw_base.S index a5ce946aa3fe..ae12a126ca66 100644 --- a/firmware/fw_base.S +++ b/firmware/fw_base.S @@ -47,6 +47,16 @@ .globl _start .globl _start_warm _start: + csrr t0, misa + slti t1, t0, 0 + slli t1, t1, 1 + slli t0, t0, 1 + slti t0, t0, 0 + add t2, t0, t1 + li t3, __riscv_xlen + srli t3, t3, 5 + bne t2, t3, _start_hang + /* Find preferred boot HART id */ MOV_3R s0, a0, s1, a1, s2, a2 call fw_boot_hart
OpenSBI is built separately for RV32/RV64. Thus, it shouldn't try to boot harts with a different XLEN compared to what it is compiled with. This patch fixes the issue observed in beagleV which has two U74(RV64) and one E24 (RV32) harts. Signed-off-by: Atish Patra <atish.patra@wdc.com> --- firmware/fw_base.S | 10 ++++++++++ 1 file changed, 10 insertions(+)