[PATCH 1/1] lib: sbi: relax scoldboot_lock spinlock

[Bin Meng]

On Fri, Aug 14, 2020 at 1:57 AM Sean Anderson <seanga2 at gmail.com> wrote:
>
> On 8/13/20 1:45 PM, Atish Patra wrote:
> > On Thu, Aug 13, 2020 at 10:03 AM Jessica Clarke <jrtc27 at jrtc27.com> wrote:
> >>
> >> On 13 Aug 2020, at 17:51, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >>> On 13.08.20 18:36, Jessica Clarke wrote:
> >>>> On 13 Aug 2020, at 17:24, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >>>>>
> >>>>> OpenSBI hangs in wake_coldboot_harts() indefinitely on the MaixDuino board
> >>>>> when trying to boot U-Boot as FW_PAYLOAD.
> >>>>
> >>>> This smells of there being a more fundamental issue that's not being
> >>>> addressed which this manages to work around, either reliably or by
> >>>> chance. What is the actual deadlock condition you observe?
> >>>
> >>> There are only two function using coldboot_lock spinlock:
> >>>
> >>> * wake_coldboot_harts()
> >>> * wait_for_coldboot()
> >>>
> >>> wake_coldboot_harts() was entered but never left without patching one of
> >>> the functions.
> >>>
> >>> The Kendryte K210 has two harts. One goes the cold boot path while the
> >>> other is sent to the warm boot path.
> >>
> >> The K210's platform definition uses clint_ipi_send, which as far as I
> >> can tell is entirely non-blocking. The only thing wake_coldboot_harts
> >> does with the lock held is set coldboot_done to 1 and send an IPI to
> >> all the other harts. Thus wake_coldboot_harts should never be able to
> >> block holding the spin lock, meaning wait_for_coldboot should
> >> eventually wake up and be able to acquire the lock (on platforms that
> >> implement WFI fully it shouldn't have to wait long for the lock, though
> >> on platforms that make it a NOP there might be a bit of .
> >>
> >
> > Yes. If wake_coldboot_harts is never able to acquire the spinlock,
> > that indicates spinlock
> > is always acquired by wait_for_coldboot. That can happen if
> >
> > 1. WFI is implemented as NOP as suggested by Jessica and MSIP in MIP
> > is set which
> > shouldn't happen unless there is an IPI sent.
> >
> > Can you try clearing the MIP just before acquiring the lock ? In qemu
> > & unleashed, modifying the MSIP bit in
> > MIP doesn't actually do anything. So we do sbi_platform_ipi_clear  to reset it.
>
> I second this suggestion. I had a similar problem when porting U-Boot.
>
> https://gitlab.denx.de/u-boot/u-boot/-/commit/9472630337e7c4ac442066b5a752aaa8c3b4d4a6

I am a bit confused. Atish said: "modifying the MSIP bit in MIP
doesn't actually do anything", but the U-Boot commit did solve the
problem? How?

Also I am lost why Heinrich is seeing the problem? I thought the
U-Boot upstream work is working on Kendyte K210, no?

Regards,
Bin