From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bin Meng Date: Mon, 14 Sep 2020 14:15:36 +0800 Subject: [PATCH 3/7] riscv: Use NULL as a sentinel value for smp_call_function In-Reply-To: <20200914061043.GA13185@andestech.com> References: <20200907181659.92449-1-seanga2@gmail.com> <20200907181659.92449-4-seanga2@gmail.com> <20200914015816.GA6319@andestech.com> <20200914061043.GA13185@andestech.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Leo, On Mon, Sep 14, 2020 at 2:10 PM Leo Liang wrote: > > Hi, Bin > On Mon, Sep 14, 2020 at 10:07:57AM +0800, Bin Meng wrote: > > Hi Leo, > > > > On Mon, Sep 14, 2020 at 9:58 AM Leo Liang wrote: > > > > > > On Fri, Sep 11, 2020 at 04:04:13PM +0800, Bin Meng wrote: > > > > On Tue, Sep 8, 2020 at 2:17 AM Sean Anderson wrote: > > > > > > > > > > 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. > > > > > > > > > > Signed-off-by: Sean Anderson > > > > > --- > > > > > > > > > > arch/riscv/lib/smp.c | 26 ++++++++++++++++++++++---- > > > > > 1 file changed, 22 insertions(+), 4 deletions(-) > > > > > > > > > > > > > Reviewed-by: Bin Meng > > > > > > Hi Bin, > > > > > > There is a series of follow-up discussion on this patch that you might miss reading. > > > > > > This current patch will cause AE350 to fail booting, > > > > > > so maybe we should wait until Sean's next patch comes up to consider a reviewed-by tag. > > > > > > > Do we know why AE350 fails to boot with this patch? > > > > When booting with bootm command, > boot hart will use smp_call_function(addr, arg0, arg1) to tell other harts to jump to "addr", > and other hart will use handle_ipi() to get the "addr". > > The address of bbl+kernel payload when booting with AE350 is at 0x0, > so the "addr" would be 0x0 that other harts have to jump to. > Thanks for the info! It's weird that AE350 has set the kernel boot entry at address 0. > With this patch > > + if (!ipi->addr) { > + pr_err("smp_function cannot be set to 0x0\n"); > + return -EINVAL; > + } > > + if (!smp_function) > + return; > > these two code snippets would cause u-boot to hang and thus fail the booting process. > > > If some big changes are reworked in newer patches, I believe author > > should remove the tag and re-request the review. > > > > Oh I see! Thanks for the explanation. Regards, Bin