[U-Boot] [PATCH 4/8] riscv: andes_plic: Fix some wrong configurations

[Anup Patel]

On Wed, Nov 6, 2019 at 2:18 PM Anup Patel <anup at brainfault.org> wrote:
>
> On Wed, Nov 6, 2019 at 12:14 PM Rick Chen <rickchen36 at gmail.com> wrote:
> >
> > Hi Anup
> >
> > >
> > > On Tue, Nov 5, 2019 at 7:19 AM Rick Chen <rickchen36 at gmail.com> wrote:
> > > >
> > > > Hi Anup
> > > >
> > > > > > On Thu, Oct 31, 2019 at 1:42 PM Anup Patel <anup at brainfault.org> wrote:
> > > > > > >
> > > > > > > On Thu, Oct 31, 2019 at 6:30 AM Alan Kao <alankao at 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 <rickchen36 at gmail.com> wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Bin
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Hi Rick,
> > > > > > > > > > >
> > > > > > > > > > > On Fri, Oct 25, 2019 at 2:18 PM Andes <uboot at andestech.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > From: Rick Chen <rick at 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 ?

Regards,
Anup

>
> Regards,
> Anup