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

[Rick Chen]

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 ?

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

>
> --Sean