[PATCH v6 13/19] riscv: Fix race conditions when initializing IPI

[Sean Anderson]

On 3/10/20 9:33 PM, Rick Chen wrote:
> Hi Sean
> 
>> On 3/10/20 4:20 AM, Rick Chen wrote:
>>> Hi Sean
>>>
>>>> The IPI code could have race conditions in several places.
>>>> * Several harts could race on the value of gd->arch->clint/plic
>>>> * Non-boot harts could race with the main hart on the DM subsystem In
>>>>   addition, if an IPI was pending when U-Boot started, it would cause the
>>>>   IPI handler to jump to address 0.
>>>>
>>>> To address these problems, a new function riscv_init_ipi is introduced. It
>>>> is called once during arch_cpu_init_dm. Before this point, no riscv_*_ipi
>>>> functions may be called. Access is synchronized by gd->arch->ipi_ready.
>>>>
>>>> Signed-off-by: Sean Anderson <seanga2 at gmail.com>
>>>> ---
>>>>
>>>
>>>> Can you try to clear mip/sip in startup flow before secondary_hart_loop:
>>>> Maybe it can help to overcome the problem of calling riscv_clear_ipi()
>>>> before riscv_init_ipi() that you added.
>>>
>>> How is the verified result about trying to clear mip/sip in startup flow ?
>>> I have asked you twice in v5, but you still have no response about it.
>>>
>>> [PATCH v5 27/33] riscv: Fix race conditions when initializing IPI
>>> https://patchwork.ozlabs.org/patch/1246864/#2377119
>>>
>>> Thanks
>>> Rick
>>
>> I managed to get it working, and this patch incorporates that change.
>> However, I forgot to update the commit message. The original issue I had
>> was related to an accidental change to my config, and not to the
>> clearing of MIP.
> 
> So it is not a race condition issue, right ?

It is sort of a race condition? If the IP CSR is not cleared, then we
can have a race condition.

> 
> Maybe you shall split this into two patchs as below
> 
> patch 1
> riscv: Clear pending ipi in start code
> Describe that it can how and what it help to fix the problem you
> encounter more detail
> e.g.
> (Perhaps the prior stage sends an IPI but does not clear it) ...
> 
> patch 2
> riscv: refine ipi initialize code flow
> Describe that your motivation and intention more detail
> e.g.
> I think the macro approach is a bit confusing,
> since it's unclear at first glance what function
> will be initializing the clint/plic.
> Given U-Boot's otherwise completely SMP-unsafe design,
> I think it's better to be explicit and conservative in these areas.
> 
> It can help us to roll back and debug in the future.
> 
> Thanks,
> Rick

This will be split in the next revision.

>>
>> --Sean