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

[Atish Patra]

On Thu, Aug 13, 2020 at 3:52 PM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 14.08.20 00:25, Atish Patra wrote:
> >>> Probably, the hart executing warmboot invokes sbi_platform_ipi_clear
> >>> even before sbi_ipi_init is being called from the hart running coldboot.
> >> There are two calls to sbi_ipi_init(). Is only the one in coldboot relevant?
> >>
> > Yes. That actually setups the clint base address.
> >
> >> Do you mean something like:
> >>
> > No. I meant like this.
> >
> > +#define UART_BASE 0x38000000ULL
> > +static void sbi_print_early(char ch) {
> > +    int32_t r;
> > +    do {
> > +      __asm__ __volatile__ (
> > +        "amoor.w %0, %2, %1\n"
> > +        : "=r" (r), "+A" (*(uint32_t*)(UART_BASE))
> > +        : "r" (ch));
> > +    } while (r < 0);
> > +}
> > +
> >  static void sbi_boot_prints(struct sbi_scratch *scratch, u32 hartid)
> >  {
> >         int xlen;
> > @@ -112,6 +123,8 @@ static void wait_for_coldboot(struct sbi_scratch
> > *scratch, u32 hartid)
> >                         wfi();
> >                         cmip = csr_read(CSR_MIP);
> >                  } while (!(cmip & MIP_MSIP));
> > +               sbi_print_early('T');
> > +               sbi_printf("mip value %lx\n", cmip);
> >                 spin_lock(&coldboot_lock);
> >         };
> >
> >
> > Note: sbi_printf may not work if sbi_console_init from the cold boot
> > path is not invoked.
> > That's why I added a "sbi_print_early".  If sbi_printf doesn't work,
> > please remove it.
> >
> > Thanks for your time in figuring out the issue with Kendryte.
> > Unfortunately, I can't test these changes as I don't have
> > a kendryte board at home.
> >
> >
>
> This now booted to U-Boot. The extra time for printing was enough to
> break the livelock:
>
Thanks again for verification.

>
> TTTTTTTTTT

This confirms that MSIP bit is set even if there was no IPI sent and MIP was
cleared in fw_base.S [1]. I guess there is a hardware bug with Kendryte where
MSIP is not getting reset properly.

[1] https://github.com/riscv/opensbi/blob/master/firmware/fw_base.S#L370

If that's the case, we can convert coldboot_done to an atomic variable
as suggested by
Anup & Jessica.

>           OpenSBI v0.8-11-g749e10e
> mip value 8
> T  ____                    _____ ____ _____
>   / __ \                  / ____|  _ \_   _|
>  | |  | |_ __   ___ _ __ | (___ | |_) || |
>  | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
>  | |__| | |_) |  __/ | | |____) | |_) || |_
>   \____/| .__/ \___|_| |_|_____/|____/_____|
>         | |
>         |_|
>
> mip value 8
> Tlatform Name       : Kendryte K210
> mip value 8
> Tlatform Features   : timer
> mip value 8
> Tlatform HART Count : 2
> Boot HART ID        : 0
> mip value 8
> Toot HART ISA       : rv64imafdcsu
> mip value 8
> TOOT HART Features  : none
> mip value 8
> TOOT HART PMP Count : 0
> mip value 8
> Tirmware Base       : 0x80000000
> mip value 8
> Tirmware Size       : 72 KB
> mip value 8
> TTuntime SBI Version : 0.2
> mip value 8
> T
>  mip value 8
> TIDELEG : 0x0000000000000222
> mip value 8
> TEDELEG : 0x0000000000000109
> mip value 8
>
>
> U-Boot 2020.10-rc2-00127-g795d62af44-dirty (Aug 13 2020 - 22:00:56 +0200)
>
> DRAM:  8 MiB
> In:    serial at 38000000
> Out:   serial at 38000000
> Err:   serial at 38000000
> =>
>
> Best regards
>
> Heinrich



-- 
Regards,
Atish