[PATCH 1/1] riscv: don't jump to 0x0 in handle_ipi()

[Heinrich Schuchardt]

On 11.08.20 08:20, Rick Chen wrote:
> Hi Heinrich
>
>> From: Heinrich Schuchardt [mailto:xypron.glpk at gmx.de]
>> Sent: Tuesday, August 11, 2020 11:57 AM
>> To: Rick Jian-Zhi Chen(陳建志)
>> Cc: Sean Anderson; Lukas Auer; Simon Glass; Anup Patel; Daniel Schwierzeck; u-boot at lists.denx.de; Heinrich Schuchardt
>> Subject: [PATCH 1/1] riscv: don't jump to 0x0 in handle_ipi()
>>
>> At least on the Kendryte K210:
>>
>> gd->arch.available_harts= 0x0000000000000003 arch.ipi[0].addr=
>> gd->0x0000000000000000 arch.ipi[0].arg0= 0x0000000000000000
>> gd->arch.ipi[0].arg1= 0x0000000000000000 arch.ipi[1].addr=
>> gd->0x0000000000000000 arch.ipi[1].arg0= 0x0000000000000000
>> gd->arch.ipi[1].arg1= 0x0000000000000000
>>
>> We should not jump to 0x0 to handle an interrupt.
>
> Can you explain why K210 be affected by it ?

I have been running U-Boot on the MaixDuino.

Without this patch secondary_hart_loop() is reached only once. With the
patch it is reached several thousand times.

I would not expect NULL to contain any code that should be executed by
the secondary hart. See doc/board/sipeed/maix.rst:

Address    Size      Description
========== ========= ===========
0x00000000 0x1000    debug
0x00001000 0x1000    rom
0x02000000 0xC000    clint

>
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>>  arch/riscv/lib/smp.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/riscv/lib/smp.c b/arch/riscv/lib/smp.c index ac22136314..d725fa32e8 100644
>> --- a/arch/riscv/lib/smp.c
>> +++ b/arch/riscv/lib/smp.c
>> @@ -96,6 +96,8 @@ void handle_ipi(ulong hart)
>>                 return;
>>         }
>>
>> +       if (!smp_function)
>> +               return;
>>         smp_function(hart, gd->arch.ipi[hart].arg0, gd->arch.ipi[hart].arg1);  }
>
> I remember Sean add this check in
> [v10,14/21] riscv: Clean up IPI initialization code
> . And I ask him to remove.
> https://patchwork.ozlabs.org/project/uboot/patch/20200503024637.327733-15-seanga2@gmail.com/

Your comment was: "Please remove the sanity check. If zero address is
the intended jump point, then system will work abnormal."

The only place where gd->arch.ipi[hart].addr is set is:

arch/riscv/lib/smp.c:53 send_ipi_many():
gd->arch.ipi[reg].addr = ipi->addr;

send_ipi_many() is only called in smp_call_function().

So the line

smp_function(hart, gd->arch.ipi[hart].arg0, gd->arch.ipi[hart].arg1);

can only work if smp_function() has been called before this point at any
time. The start code only calls it in spl_secondary_hart_stack_gd_setup().

Why do we call handle_ipi() on the secondary hart at all?

Where do you expect it to jump if U-Boot is the first binary loaded?

Even if spl_secondary_hart_stack_gd_setup() has initialized the jump
address to secondary_hart_relocate(), do we want the secondary hart to
execute it again and again?

Best regards

Heinrich