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 <sean...@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