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.
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.
or it will cause IPI sending errors.
Regards, Bin