Hi Sean
> Hi Sean
>
> > Some IPIs may already be pending when U-Boot is started. This could be a
> > problem if a secondary hart tries to handle an IPI before the boot hart has
> > initialized the IPI device.
> >
> > This commit uses NULL as a sentinel for secondary harts so they know when
> > the IPI is initialized, and it is safe to use the IPI API. The smp addr
> > parameter is initialized to NULL by board_init_f_init_reserve. Before this,
> > secondary harts wait in wait_for_gd_init.
> >
> > This imposes a minor restriction because harts may no longer jump to NULL.
> > However, given that the RISC-V debug device is likely to be located at
> > 0x400, it is unlikely for any RISC-V implementation to have usable ram
> > located at 0x0.
>
> The ram location of AE350 is at 0x0.
>
> >
> > Signed-off-by: Sean Anderson <sean...@gmail.com>
> > ---
> >
> > arch/riscv/lib/smp.c | 26 ++++++++++++++++++++++----
> > 1 file changed, 22 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/lib/smp.c b/arch/riscv/lib/smp.c
> > index ab6d8bd7fa..8c25755330 100644
> > --- a/arch/riscv/lib/smp.c
> > +++ b/arch/riscv/lib/smp.c
> > @@ -18,6 +18,12 @@ static int send_ipi_many(struct ipi_data *ipi, int wait)
> > u32 reg;
> > int ret, pending;
> >
> > + /* NULL is used as a sentinel value */
> > + if (!ipi->addr) {
> > + pr_err("smp_function cannot be set to 0x0\n");
> > + return -EINVAL;
> > + }
> > +
>
> This conflict with memory configurations of AE350.
> Please check about doc\board\AndesTech\ax25-ae350.rst, and you can
> find BBL is configured as zero address on AE350 platform.
>
> > cpus = ofnode_path("/cpus");
> > if (!ofnode_valid(cpus)) {
> > pr_err("Can't find cpus node!\n");
> > @@ -50,11 +56,16 @@ static int send_ipi_many(struct ipi_data *ipi, int wait)
> > continue;
> > #endif
> >
> > - gd->arch.ipi[reg].addr = ipi->addr;
> > gd->arch.ipi[reg].arg0 = ipi->arg0;
> > gd->arch.ipi[reg].arg1 = ipi->arg1;
> >
> > - __smp_mb();
Why do you add this in [PATCH 2/7] but remove it in [PATCH 3/7] ?
Thanks,
Rick
> > + /*
> > + * Ensure addr only becomes non-NULL when arg0 and arg1 are
> > + * valid. An IPI may already be pending on other harts, so
> > we
> > + * need a way to signal that the IPI device has been
> > + * initialized, and that it is ok to call the function.
> > + */
> > + __smp_store_release(&gd->arch.ipi[reg].addr, ipi->addr);
>
> It is too tricky and hack by using zero address to be a signal for the
> other pending harts waiting the IPI device been initialized.
>
> >
> > ret = riscv_send_ipi(reg);
> > if (ret) {
> > @@ -83,9 +94,16 @@ void handle_ipi(ulong hart)
> > if (hart >= CONFIG_NR_CPUS)
> > return;
> >
> > - __smp_mb();
> > + smp_function = (void (*)(ulong, ulong, ulong))
> > + __smp_load_acquire(&gd->arch.ipi[hart].addr);
> > + /*
> > + * If the function is NULL, then U-Boot has not requested the IPI.
> > The
> > + * IPI device may not be initialized, so all we can do is wait for
> > + * U-Boot to initialize it and send an IPI
> > + */
> > + if (!smp_function)
> > + return;
>
> It will boot BBL+Kernel payload fail here on AE350 platforms with this check.
>
> Thanks,
> Rick
>
> >
> > - smp_function = (void (*)(ulong, ulong,
> > ulong))gd->arch.ipi[hart].addr;
> > invalidate_icache_all();
> >
> > /*
> > --
> > 2.28.0
> >
