Hi Anup
On Thu, Oct 31, 2019 at 1:42 PM Anup Patel anup@brainfault.org wrote:
On Thu, Oct 31, 2019 at 6:30 AM Alan Kao alankao@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@gmail.com wrote:
Hi Bin
Hi Rick,
On Fri, Oct 25, 2019 at 2:18 PM Andes uboot@andestech.com wrote: > > From: Rick Chen rick@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
- get hart ID.
- 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.
OK
Thanks Rick
Regards, Anup
But any hart can be main hart in U-Boot.
In general case, hart 0 will be main and it is fine when U-Boot jump ot OpenSBI. But if we force hart 1 to be main hart, when hart 0 jump to OPenSBI from U-Boot, It will do relocation flow in OpenSBI which willcorrupt U-Boot SPL, but hart 0 still in U-Boot SPL.
> > After some work-arounds, it can pass the verifications > that any hart can be main hart and boots U-Boot SPL -> > OpenSbi -> U-Boot proper -> Linux Kernel successfully. >
It's a bit hard for me to understand what was described here in the commit message. Maybe you need rewrite something.
OK. I will rewrite this commit message.
> Signed-off-by: Rick Chen rick@andestech.com > Cc: KC Lin kclin@andestech.com > Cc: Alan Kao alankao@andestech.com > --- > arch/riscv/lib/andes_plic.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/arch/riscv/lib/andes_plic.c b/arch/riscv/lib/andes_plic.c > index 28568e4..42394b9 100644 > --- a/arch/riscv/lib/andes_plic.c > +++ b/arch/riscv/lib/andes_plic.c > @@ -19,7 +19,7 @@ > #include <cpu.h> > > /* pending register */ > -#define PENDING_REG(base, hart) ((ulong)(base) + 0x1000 + (hart) * 8) > +#define PENDING_REG(base, hart) ((ulong)(base) + 0x1000 + ((hart) / 4) * 4) > /* enable register */ > #define ENABLE_REG(base, hart) ((ulong)(base) + 0x2000 + (hart) * 0x80) > /* claim register */ > @@ -46,7 +46,7 @@ static int init_plic(void); > > static int enable_ipi(int hart) > { > - int en; > + unsigned int en;
Is this for some compiler warning fix?
No, it is not a warning fix. It is a bug fix. I hope en can be 0x0000000080808080 instead of 0xffffffff80808080,
But it is int, which is only 32-bit. The example you gave was a 64-bit number.
Please consider the following simple program:
#define MASK 0x80808080 int main(){ int en; en = MASK; printf("%x, shifted %x\n", en, en >> 1); return 0; }
Would you mind sharing what you get after running this on your x86_64 (if you have one) computer? Really appreiciate that.
The almost identical episode is in the patch, specifically,
en = ENABLE_HART_IPI >> hart
or it will cause IPI sending errors.
Regards, Bin
Best, Alan
Regards, Anup