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. 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 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. :) Thanks Rick > > Meanwhile, I will try above patch on QEMU and SiFive Unleashed. > > > > > It will be convenient to change U-Boot SPL text base currently. > > Let's fix this correctly now itself. > > Regards, > Anup _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
