On Wed, Nov 13, 2019 at 9:13 AM Rick Chen <rickche...@gmail.com> wrote: > > Hi Lukas > > > > > Hi Rick, > > > > On Mon, 2019-11-11 at 15:19 +0800, Rick Chen wrote: > > > Hi Lukas > > > > > > > Hi Rick, > > > > > > > > On Fri, 2019-11-08 at 15:27 +0800, Rick Chen wrote: > > > > > Hi Atish > > > > > > > > > > > Hi Atish > > > > > > > > > > > > > On Thu, 2019-11-07 at 19:41 +0800, Rick Chen wrote: > > > > > > > > Hi Anup & Lukas > > > > > > > > > > > > > > > > Anup Patel <a...@brainfault.org> 於 2019年11月7日 週四 下午6:44寫道： > > > > > > > > > On Thu, Nov 7, 2019 at 3:11 PM Auer, Lukas > > > > > > > > > <lukas.a...@aisec.fraunhofer.de> wrote: > > > > > > > > > > On Thu, 2019-11-07 at 11:48 +0530, Anup Patel wrote: > > > > > > > > > > > On Thu, Nov 7, 2019 at 11:40 AM Rick Chen > > > > > > > > > > > <rickche...@gmail.com > > > > > > > > > > > > wrote: > > > > > > > > > > > > Hi Anup > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Nov 7, 2019 at 10:45 AM Anup Patel < > > > > > > > > > > > > > a...@brainfault.org> wrote: > > > > > > > > > > > > > > On Thu, Nov 7, 2019 at 7:04 AM Rick Chen < > > > > > > > > > > > > > > rickche...@gmail.com> wrote: > > > > > > > > > > > > > > > Hi Anup > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Nov 6, 2019 at 2:51 PM Rick Chen < > > > > > > > > > > > > > > > > rickche...@gmail.com> wrote: > > > > > > > > > > > > > > > > > Hi Anup > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Nov 6, 2019 at 2:18 PM Anup Patel < > > > > > > > > > > > > > > > > > > a...@brainfault.org> wrote: > > > > > > > > > > > > > > > > > > > On Wed, Nov 6, 2019 at 12:14 PM Rick Chen > > > > > > > > > > > > > > > > > > > < > > > > > > > > > > > > > > > > > > > rickche...@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > Hi Anup > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Nov 5, 2019 at 7:19 AM Rick > > > > > > > > > > > > > > > > > > > > > Chen < > > > > > > > > > > > > > > > > > > > > > rickche...@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > Hi Anup > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Oct 31, 2019 at 1:42 PM > > > > > > > > > > > > > > > > > > > > > > > > Anup > > > > > > > > > > > > > > > > > > > > > > > > Patel <a...@brainfault.org> > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Oct 31, 2019 at 6:30 > > > > > > > > > > > > > > > > > > > > > > > > > AM > > > > > > > > > > > > > > > > > > > > > > > > > Alan Kao > > > > > > > > > > > > > > > > > > > > > > > > > <alan...@andestech.com> > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Bin, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the critics. > > > > > > > > > > > > > > > > > > > > > > > > > > Comments > > > > > > > > > > > > > > > > > > > > > > > > > > below. > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Oct 30, 2019 at > > > > > > > > > > > > > > > > > > > > > > > > > > 06:38:00PM +0800, Bin Meng > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Rick, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Oct 30, 2019 at > > > > > > > > > > > > > > > > > > > > > > > > > > > 10:50 > > > > > > > > > > > > > > > > > > > > > > > > > > > AM Rick Chen < > > > > > > > > > > > > > > > > > > > > > > > > > > > rickche...@gmail.com> > > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Bin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Rick, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Oct 25, 2019 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > at > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2:18 PM Andes < > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ub...@andestech.com> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > From: Rick Chen < > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > r...@andestech.com> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > It will work fine > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > due to > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > hart 0 always will > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > be > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > main > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > hart > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > coincidentally. When > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > develop SPL flow, I > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > try > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > force other harts > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > to be > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > main hart. And it > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > will go > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > wrong in sending IPI > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > flow. So fix it. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Fix what? Does this > > > > > > > > > > > > > > > > > > > > > > > > > > > > > commit > > > > > > > > > > > > > > > > > > > > > > > > > > > > > contain 2 fixes, or > > > > > > > > > > > > > > > > > > > > > > > > > > > > > just 1 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > fix? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, it include two > > > > > > > > > > > > > > > > > > > > > > > > > > > > fixs. But > > > > > > > > > > > > > > > > > > > > > > > > > > > > they will cause one > > > > > > > > > > > > > > > > > > > > > > > > > > > > negative > > > > > > > > > > > > > > > > > > > > > > > > > > > > result > > > > > > > > > > > > > > > > > > > > > > > > > > > > that only hart 0 can > > > > > > > > > > > > > > > > > > > > > > > > > > > > send ipi > > > > > > > > > > > > > > > > > > > > > > > > > > > > to other harts. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Having this fix, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > any hart > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > can be main hart in > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > U- > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Boot SPL > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > theoretically, but > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > it > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > still fail > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > somewhere. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > After dig in > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > and found there is > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > an > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > assumption that > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > hart 0 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > shall be > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > main hart in > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > OpenSbi. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > So does this mean > > > > > > > > > > > > > > > > > > > > > > > > > > > > > there is > > > > > > > > > > > > > > > > > > > > > > > > > > > > > a bug in OpenSBI too? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I am not sure if it is > > > > > > > > > > > > > > > > > > > > > > > > > > > > a bug. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Maybe it is a compatible > > > > > > > > > > > > > > > > > > > > > > > > > > > > issue. > > > > > > > > > > > > > > > > > > > > > > > > > > > > There is a limitation > > > > > > > > > > > > > > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > > > > > > > > > > > > only hart 0 can be main > > > > > > > > > > > > > > > > > > > > > > > > > > > > hart > > > > > > > > > > > > > > > > > > > > > > > > > > > > in OpenSBI. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I don't think OpenSBI has > > > > > > > > > > > > > > > > > > > > > > > > > > > such > > > > > > > > > > > > > > > > > > > > > > > > > > > limitation. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Please check the source. > > > > > > > > > > > > > > > > > > > > > > > > > > https://github.com/riscv/opensbi/blob/master/firmware/fw_base.S#L54 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Apparently, the FIRST TWO > > > > > > > > > > > > > > > > > > > > > > > > > > LINEs > > > > > > > > > > > > > > > > > > > > > > > > > > of the initialization are > > > > > > > > > > > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > > > > > > > > > > 1. get hart ID. > > > > > > > > > > > > > > > > > > > > > > > > > > 2. determine which route to > > > > > > > > > > > > > > > > > > > > > > > > > > take > > > > > > > > > > > > > > > > > > > > > > > > > > based on their ID > > > > > > > > > > > > > > > > > > > > > > > > > > respectively. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > So, I do think OpenSBI has > > > > > > > > > > > > > > > > > > > > > > > > > > this > > > > > > > > > > > > > > > > > > > > > > > > > > signature, if you are not > > > > > > > > > > > > > > > > > > > > > > > > > > willing > > > > > > > > > > > > > > > > > > > > > > > > > > to call it > > > > > > > > > > > > > > > > > > > > > > > > > > a limitation. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > This dependency on hart id #0 > > > > > > > > > > > > > > > > > > > > > > > > > was > > > > > > > > > > > > > > > > > > > > > > > > > not there until we added self- > > > > > > > > > > > > > > > > > > > > > > > > > relocation > > > > > > > > > > > > > > > > > > > > > > > > > in OpenSBI for FW_DYNAMIC. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I will try to fix this in > > > > > > > > > > > > > > > > > > > > > > > > > OpenSBI > > > > > > > > > > > > > > > > > > > > > > > > > but we might end-up having > > > > > > > > > > > > > > > > > > > > > > > > > boot_lottery. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I have send a patch to fix this > > > > > > > > > > > > > > > > > > > > > > > > OpenSBI: > > > > > > > > > > > > > > > > > > > > > > > > "[PATCH] firmware: Introduce > > > > > > > > > > > > > > > > > > > > > > > > relocation lottery" > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Can you try above patch and see > > > > > > > > > > > > > > > > > > > > > > > > if > > > > > > > > > > > > > > > > > > > > > > > > that helps ? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > It will be great if you can > > > > > > > > > > > > > > > > > > > > > > > > provide > > > > > > > > > > > > > > > > > > > > > > > > Tested-by to my patch as well. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I can not find this patch in mailing > > > > > > > > > > > > > > > > > > > > > > list. > > > > > > > > > > > > > > > > > > > > > > Can you provide a hyperlink ? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > You can try latest riscv/opensbi > > > > > > > > > > > > > > > > > > > > > master. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I have tested the patch on SiFive > > > > > > > > > > > > > > > > > > > > > Unleashed > > > > > > > > > > > > > > > > > > > > > multiple times. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I have tried this patch, but it fail > > > > > > > > > > > > > > > > > > > > firmware: Introduce relocation lottery( > > > > > > > > > > > > > > > > > > > > 98f4a208995b027662a7b04a25e4fa5df5f3eefe) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The scenario was as below: > > > > > > > > > > > > > > > > > > > > There are 4 harts run in U-Boot SPL, > > > > > > > > > > > > > > > > > > > > hart 0 > > > > > > > > > > > > > > > > > > > > play as main hart. > > > > > > > > > > > > > > > > > > > > The hart 1 will receive ipi and come > > > > > > > > > > > > > > > > > > > > into > > > > > > > > > > > > > > > > > > > > OpenSBI(0x1000000) from > > > > > > > > > > > > > > > > > > > > U-Boot SPL(0x0), meanwhile hart 0,2,3 > > > > > > > > > > > > > > > > > > > > still > > > > > > > > > > > > > > > > > > > > run in U-Boot SPL. > > > > > > > > > > > > > > > > > > > > Then hart 1 will do > > > > > > > > > > > > > > > > > > > > _relocate_copy_to_lower > > > > > > > > > > > > > > > > > > > > which will copy data from > > > > > > > > > > > > > > > > > > > > 0x1000000 to 0x0. > > > > > > > > > > > > > > > > > > > > And it will corrupt U-Boot SPL. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The self-relocation in OpenSBI firmwares > > > > > > > > > > > > > > > > > > > ensures that OpenSBI firmware > > > > > > > > > > > > > > > > > > > are moved to the FW_TEXT_START before > > > > > > > > > > > > > > > > > > > entering > > > > > > > > > > > > > > > > > > > C code. This helps > > > > > > > > > > > > > > > > > > > us load OpenSBI firmwares anywhere in RAM. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > However, OpenSBI firmwares don't know > > > > > > > > > > > > > > > > > > > where the > > > > > > > > > > > > > > > > > > > U-Boot SPL is running. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > In your case, both OpenSBI FW_DYNAMIC and > > > > > > > > > > > > > > > > > > > U- > > > > > > > > > > > > > > > > > > > Boot SPL are linked to > > > > > > > > > > > > > > > > > > > address same address 0x0. This means > > > > > > > > > > > > > > > > > > > secondary > > > > > > > > > > > > > > > > > > > HARTs cannot safely > > > > > > > > > > > > > > > > > > > wait while primary HART enters OpenSBI. > > > > > > > > > > > > > > > > > > > You > > > > > > > > > > > > > > > > > > > should hold secondary HARTs > > > > > > > > > > > > > > > > > > > in U-Boot SPL only till OpenSBI > > > > > > > > > > > > > > > > > > > FW_DYNAMIC and > > > > > > > > > > > > > > > > > > > U-Boot proper are > > > > > > > > > > > > > > > > > > > loaded in RAM by primary HART. All your > > > > > > > > > > > > > > > > > > > HARTs > > > > > > > > > > > > > > > > > > > should jump to OpenSBI > > > > > > > > > > > > > > > > > > > at the same time after everything is > > > > > > > > > > > > > > > > > > > loaded in > > > > > > > > > > > > > > > > > > > RAM. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I see the issue now. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The U-Boot SPL is first letting secondary > > > > > > > > > > > > > > > > > > HART > > > > > > > > > > > > > > > > > > jump to OpenSBI and primary > > > > > > > > > > > > > > > > > > HART jumps to OpenSBI at the end. > > > > > > > > > > > > > > > > > > (Refer, jump_to_image_no_args() in > > > > > > > > > > > > > > > > > > arch/riscv/lib/spl.c) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The real issue is FW_TEXT_START being same > > > > > > > > > > > > > > > > > > as U- > > > > > > > > > > > > > > > > > > Boot SPL TEXT_START. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > If possible please change TEXT base for > > > > > > > > > > > > > > > > > > U-Boot > > > > > > > > > > > > > > > > > > SPL or OpenSBI. I think > > > > > > > > > > > > > > > > > > changing U-Boot SPL TEXT_START would be > > > > > > > > > > > > > > > > > > convenient since this series > > > > > > > > > > > > > > > > > > is under review. Thoughts ? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes. > > > > > > > > > > > > > > > > > I know it can avoid corrupting issue with > > > > > > > > > > > > > > > > > changing U-Boot SPL > > > > > > > > > > > > > > > > > TEXT_START not equal to OpenSBI TEXT base. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I think this issue will be seen on U-Boot SPL > > > > > > > > > > > > > > > > running > > > > > > > > > > > > > > > > on QEMU as well. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > With the following changes, U-Boot SPL text > > > > > > > > > > > > > > > > > base > > > > > > > > > > > > > > > > > can equal to OpenSBI text base > > > > > > > > > > > > > > > > > 1 U-Boot pass main hart information (a2) when > > > > > > > > > > > > > > > > > jumping to OpenSBI > > > > > > > > > > > > > > > > > 2 OpenSBI pick up $a2 to keep playing as main > > > > > > > > > > > > > > > > > hart, > > > > > > > > > > > > > > > > > other harts go to > > > > > > > > > > > > > > > > > _wait_relocate_copy_done > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Overall it's a good suggestion but we cannot > > > > > > > > > > > > > > > > use a2 > > > > > > > > > > > > > > > > register because this > > > > > > > > > > > > > > > > will break FW_JUMP and FW_PAYLOAD. Instead, we > > > > > > > > > > > > > > > > should > > > > > > > > > > > > > > > > pass preferred > > > > > > > > > > > > > > > > boot HART id in struct fw_dynamic_info. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Sorry, what I want to say shall be a3. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I have a patch for this in > > > > > > > > > > > > > > > > preferred_boot_hart_v1 > > > > > > > > > > > > > > > > branch of > > > > > > > > > > > > > > > > https://github.com/avpatel/opensbi.git > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Can you try OpenSBI from above branch ? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > You will have to update the "struct > > > > > > > > > > > > > > > > fw_dynamic_info" > > > > > > > > > > > > > > > > passed to > > > > > > > > > > > > > > > > OpenSBI by U-Boot SPL. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Main hart will pass struct "fw_dynamic_info" to > > > > > > > > > > > > > > > OpenSBI > > > > > > > > > > > > > > > by U-Boot SPL. > > > > > > > > > > > > > > > But other harts will NOT pass struct > > > > > > > > > > > > > > > "fw_dynamic_info" > > > > > > > > > > > > > > > to OpenSBI by U-Boot SPL. > > > > > > > > > > > > > > > > > > > > > > > > > > > > That's wrong in U-Boot SPL. > > > > > > > > > > > > > > > > > > > > > > > > > > > > All HARTs have to follow FW_DYNAMIC protocol and > > > > > > > > > > > > > > pass > > > > > > > > > > > > > > "struct fw_dynamic_info" pointer in 'a2' register. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > So if U-Boot SPL can pass main hart information > > > > > > > > > > > > > > > via a3, > > > > > > > > > > > > > > > OpenSBI just > > > > > > > > > > > > > > > have the following change > > > > > > > > > > > > > > > blt zero, a6, _wait_relocate_copy_done > > > > > > > > > > > > > > > change to > > > > > > > > > > > > > > > bne a3, a6, _wait_relocate_copy_done > > > > > > > > > > > > > > > before this commit > > > > > > > > > > > > > > > 98f4a208995b027662a7b04a25e4fa5df5f3eefe > > > > > > > > > > > > > > > firmware: Introduce relocation lottery > > > > > > > > > > > > > > > > > > > > > > > > > > > > What about FW_JUMP and FW_PAYLOAD? We have no way of > > > > > > > > > > > > > > passing > > > > > > > > > > > > > > value in a3 for these firmwares because these are > > > > > > > > > > > > > > not > > > > > > > > > > > > > > booted by U-Boot > > > > > > > > > > > > > > SPL. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Also, U-Boot-2019.10 already uses U-Boot SPL support > > > > > > > > > > > > > > which does not > > > > > > > > > > > > > > pass anything in 'a3' register. > > > > > > > > > > > > > > > > > > > > > > > > > > > > We should definitely use "struct fw_dynamic_info" > > > > > > > > > > > > > > for > > > > > > > > > > > > > > this so that we can > > > > > > > > > > > > > > maintain backward compatibility as well. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Please make sure that U-Boot SPL passes "struct > > > > > > > > > > > > > > fw_dynamic_info" > > > > > > > > > > > > > > pointer in 'a2' register for all HARTs. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > But after this commit 98f4a, main hart become > > > > > > > > > > > > > > > chosen > > > > > > > > > > > > > > > from lottery mechanism. > > > > > > > > > > > > > > > Maybe I will prefer to change U-Boot SPL text > > > > > > > > > > > > > > > base not > > > > > > > > > > > > > > > overlap with > > > > > > > > > > > > > > > OpenSBI text start. :) > > > > > > > > > > > > > > > > > > > > > > > > > > > > Like I mentioned, we have this issue for U-Boot SPL > > > > > > > > > > > > > > on > > > > > > > > > > > > > > QEMU as well. It's > > > > > > > > > > > > > > just that most of us did not notice it for U-Boot > > > > > > > > > > > > > > SPL on > > > > > > > > > > > > > > QEMU. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Let's fix this in the right way from start itself. > > > > > > > > > > > > > > > > > > > > > > > > > > I double checked spl_invoke_opensbi() and it is doing > > > > > > > > > > > > > the > > > > > > > > > > > > > right thing > > > > > > > > > > > > > by passing "struct fw_dyanmic_info" pointer in 'a2' > > > > > > > > > > > > > register. > > > > > > > > > > > > > (Refer, common/spl/spl_opensbi.c) > > > > > > > > > > > > > > > > > > > > > > > > > > Not sure, why it is not passing 'a2' register > > > > > > > > > > > > > correctly for > > > > > > > > > > > > > you ?? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Yes, you are right. I reply too quickly. > > > > > > > > > > > > Other harts will pass struct fw_dyanmic_info in a2 to > > > > > > > > > > > > OpenSBI. > > > > > > > > > > > > > > > > > > > > > > > > Thanks for your corrections > > > > > > > > > > > > > > > > > > > > > > No problem, I am happy to help. > > > > > > > > > > > > > > > > > > > > > > BTW, I tried to play around with U-Boot SPL on QEMU. > > > > > > > > > > > > > > > > > > > > > > Maybe below changes can help you... > > > > > > > > > > > > > > > > > > > > Thanks for looking into this issue! I successfully tested > > > > > > > > > > it on > > > > > > > > > > QEMU, I > > > > > > > > > > had to add a short delay between sending the IPIs to > > > > > > > > > > trigger the > > > > > > > > > > problem. > > > > > > > > > > > > > > > > > > > > We might still run into problems however. Right now, we are > > > > > > > > > > assuming > > > > > > > > > > that the main hart is the last one to enter OpenSBI. If > > > > > > > > > > this is > > > > > > > > > > not the > > > > > > > > > > case (some delay when handling the IPI), we will have the > > > > > > > > > > same > > > > > > > > > > problem > > > > > > > > > > again. To fix this we could pass the hart mask, containing > > > > > > > > > > all > > > > > > > > > > harts > > > > > > > > > > that have entered U-Boot, to OpenSBI and wait for all harts > > > > > > > > > > to be > > > > > > > > > > running in OpenSBI. I am not sure how realistic this > > > > > > > > > > scenario is, > > > > > > > > > > so > > > > > > > > > > this might not be needed. > > > > > > > > > > > > > > > > > > I agree that we might still run into this issue if primary > > > > > > > > > HART > > > > > > > > > enters > > > > > > > > > OpenSBI before secondary HARTs. I think this situation can > > > > > > > > > only > > > > > > > > > happen on QEMU where each CPU is a thread running on host but > > > > > > > > > very unlikely/impossible on real HW. > > > > > > > > > > > > > > > > > > Maybe a delay on primary HART in U-Boot SPL after SMP calls to > > > > > > > > > secondary HARTs and before jumping to OpenSBI ? > > > > > > > > > > > > > > > > > > Regarding hart_mask in fw_dynamic_info, I think the issue > > > > > > > > > will be > > > > > > > > > the > > > > > > > > > size of the hart_mask. It is possible in-future SOC vendors > > > > > > > > > come-up > > > > > > > > > with SOC having huge number of HARTs OR SOC with discontinuous > > > > > > > > > HART IDs which can cause a 64bit hart_mask to be not > > > > > > > > > sufficient for > > > > > > > > > all HARTs. > > > > > > > > > > > > > > > > > > Also, waiting for all HARTs to enter OpenSBI will be one more > > > > > > > > > wait- > > > > > > > > > loop > > > > > > > > > in fw_base.S which will add to the boot-time as well. > > > > > > > > > > > > > > > > > > I still think the root cause of the issue is that TEXT_START > > > > > > > > > of > > > > > > > > > U-Boot SPL and OpenSBI FW_DYNAMIC is same. Maybe we can > > > > > > > > > insist SOC vendors to not use same TEXT_START ? > > > > > > > > > > > > > > > > I have try your changes about boot_hart for U-Boot SPL with > > > > > > > > OpenSBI, > > > > > > > > preferred_boot_hart_v2 branch > > > > > > > > It still encounter some booting problems. I try to find out the > > > > > > > > root > > > > > > > > cause but in vain. > > > > > > > > > > > > > > > > > > > > > > Just wanted to make sure that you have tried this patch. > > > > > > > > > > > > > > http://lists.infradead.org/pipermail/opensbi/2019-November/000672.html > > > > > > > > > > > > > > We should investigate the issue why it did not work for you if > > > > > > > this > > > > > > > patch did not work for you. > > > > > > > > > > > > Yes, I try with this > > > > > > commit 831aa3c1ad2546a2b35ddf5b1aa0ce91cdc7fe89 > > > > > > firmware: Add preferred boot HART field in struct fw_dynamic_info > > > > > > > > > > > > It fail randomly yesterday, but this morning I try several times it > > > > > > will pass. > > > > > > I will keep trying. > > > > > > > > > > > > > > > > I have figure out one fail case which is belong to main hart of U-Boot > > > > > SPL is not the last hart while entering OpenSBI > > > > > > > > > > > > > Can you try this branch [1]? It includes a quick implementation of the > > > > changes a mentioned yesterday, where the main hart waits until all > > > > harts have received the IPI. > > > > > > > > [1]: > > > > https://github.com/lukasauer/u-boot/commits/riscv-opensbi-boot-hart > > > > > > I have try this patch, but it seems can not guarantee main hart to be > > > the last hart while leaving U-Boot SPL. > > > Even the main hart have checked all harts have received the IPI, but > > > it still have opportunities to arrive OpenSBI before other harts. > > > > > > > Thanks for testing! Can you try again with the same branch? I added > > another patch so that clearing the IPI is the very last thing before > > jumping to the SMP function. > > If that does not help, we'll have to add a delay in > > spl_invoke_opensbi() to delay the main hart. > > > Thanks for your patch, it indeed solve the problem almost. > It always pass the booting verification under free running, I try many > times and can not hit the failure case. > > But if I do something with GDB, E.g > Set a break in 0x6c2 (c.jalr s2) in handle_ipi() and then delete the > break and free run after it hit this break > then it will hit the failure case as below:
The current booting mechanism where all HARTs jump to each booting stage (including Linux) at the same time is very fragile. Attaching a debugger at boot-time will certainly break current way of booting all HARTs because we are holding a particular HART in debug state. I am not sure if we can handle the debugger case in software but in real-world deployments debugger won't be present so maybe we can ignore this case. The SBI v0.2 HSM extension will make things better. It would be even better if HW designers provide explicit HW mechanism to power-on/off HARTs at run-time. Regards, Anup _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
