[U-Boot] [PATCH 1/7] riscv: add infrastructure for calling functions on other harts
Anup Patel
anup at brainfault.org
Mon Feb 18 13:16:18 UTC 2019
On Mon, Feb 18, 2019 at 5:11 PM Auer, Lukas
<lukas.auer at aisec.fraunhofer.de> wrote:
>
> On Mon, 2019-02-18 at 10:01 +0000, Auer, Lukas wrote:
> > On Mon, 2019-02-18 at 10:28 +0530, Anup Patel wrote:
> > > On Tue, Feb 12, 2019 at 3:44 AM Lukas Auer
> > > <lukas.auer at aisec.fraunhofer.de> wrote:
> > > > Harts on RISC-V boot independently and U-Boot is responsible for
> > > > managing them. Functions are called on other harts with
> > > > smp_call_function(), which sends inter-processor interrupts
> > > > (IPIs)
> > > > to
> > > > all other harts. Functions are specified with their address and
> > > > two
> > > > function arguments (argument 2 and 3). The first function
> > > > argument
> > > > is
> > > > always the hart ID of the hart calling the function. On the other
> > > > harts,
> > > > the IPI interrupt handler handle_ipi() must be called on software
> > > > interrupts to handle the request and call the specified function.
> > > >
> > > > Functions are stored in the ipi_data data structure. Every hart
> > > > has
> > > > its
> > > > own data structure in global data. While this is not required at
> > > > the
> > > > moment (all harts are expected to boot Linux), this does allow
> > > > future
> > > > expansion, where other harts may be used for monitoring or other
> > > > tasks.
> > > >
> > > > Signed-off-by: Lukas Auer <lukas.auer at aisec.fraunhofer.de>
> > > > ---
> > > >
> > > > arch/riscv/Kconfig | 19 +++++
> > > > arch/riscv/include/asm/global_data.h | 5 ++
> > > > arch/riscv/include/asm/smp.h | 53 +++++++++++++
> > > > arch/riscv/lib/Makefile | 1 +
> > > > arch/riscv/lib/smp.c | 110
> > > > +++++++++++++++++++++++++++
> > > > 5 files changed, 188 insertions(+)
> > > > create mode 100644 arch/riscv/include/asm/smp.h
> > > > create mode 100644 arch/riscv/lib/smp.c
> > > >
> > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > index c45e4d73a8..c0842178dd 100644
> > > > --- a/arch/riscv/Kconfig
> > > > +++ b/arch/riscv/Kconfig
> > > > @@ -116,4 +116,23 @@ config RISCV_RDTIME
> > > > config SYS_MALLOC_F_LEN
> > > > default 0x1000
> > > >
> > > > +config SMP
> > > > + bool "Symmetric Multi-Processing"
> > > > + help
> > > > + This enables support for systems with more than one
> > > > CPU.
> > > > If
> > > > + you say N here, U-Boot will run on single and
> > > > multiprocessor
> > > > + machines, but will use only one CPU of a multiprocessor
> > > > + machine. If you say Y here, U-Boot will run on many,
> > > > but
> > > > not
> > > > + all, single processor machines.
> > > > +
> > > > +config NR_CPUS
> > > > + int "Maximum number of CPUs (2-32)"
> > > > + range 2 32
> > > > + depends on SMP
> > > > + default "8"
> > > > + help
> > > > + On multiprocessor machines, U-Boot sets up a stack for
> > > > each CPU.
> > > > + Stack memory is pre-allocated. U-Boot must therefore
> > > > know
> > > > the
> > > > + maximum number of CPUs that may be present.
> > > > +
> > > > endmenu
> > > > diff --git a/arch/riscv/include/asm/global_data.h
> > > > b/arch/riscv/include/asm/global_data.h
> > > > index a3a342c6e1..23a5f35af5 100644
> > > > --- a/arch/riscv/include/asm/global_data.h
> > > > +++ b/arch/riscv/include/asm/global_data.h
> > > > @@ -10,12 +10,17 @@
> > > > #ifndef __ASM_GBL_DATA_H
> > > > #define __ASM_GBL_DATA_H
> > > >
> > > > +#include <asm/smp.h>
> > > > +
> > > > /* Architecture-specific global data */
> > > > struct arch_global_data {
> > > > long boot_hart; /* boot hart id */
> > > > #ifdef CONFIG_SIFIVE_CLINT
> > > > void __iomem *clint; /* clint base address */
> > > > #endif
> > > > +#ifdef CONFIG_SMP
> > > > + struct ipi_data ipi[CONFIG_NR_CPUS];
> > >
> > > The data passed by "main HART" via global data to other HARTs
> > > is not visible reliably and randomly few HARTs fail to enter Linux
> > > kernel on my board. I am suspecting per-HART "ipi" data in global
> > > data not being cache-line aligned as the cause of behavior but
> > > there
> > > could be other issue too.
> > >
> > > I have a hack which works reliable for me. As-per this hack, we add
> > > "mdelay(10)" just before calling riscv_send_ipi() in
> > > send_ipi_many().
> > > This hack helped me conclude that there is some sync issue in per-
> > > HART
> > > "ipi" data.
> > >
> > > The above issue is not seen on QEMU so we are fine there.
> > >
> > > I would suggest to make per-HART "ipi" data cache-line aligned
> > > (just like Linux kernel).
> > >
> >
> > Interesting, this is definitely a memory coherency issue, probably
> > inadequate fences. I am not sure if making the ipi data structure
> > cache-line aligned will reliably fix it. I'll try to replicate the
> > issue and get it fixed. Thanks for reporting it!
> >
>
> Not sure if it is connected, but I have noticed a regression with the
> current OpenSBI. Testing U-Boot with the SMP patches applied on QEMU
> with 4 harts, hart 4 will not receive software interrupts. I have
> bisected the issue to the following commit.
>
> 918c1354b75c74b62f67c4e929551d643f035443 is the first bad commit
> commit 918c1354b75c74b62f67c4e929551d643f035443
> Author: Nick Kossifidis <mickflemm at gmail.com>
> Date: Sun Feb 17 09:00:20 2019 +0200
>
> lib: Improve delivery of SBI_IPI_EVENT_HALT
>
> When sbi_ipi_send_many gets called with the current hartid
> included on pmask (or when pmask is NULL), and we send
> a HALT event, since the for loop works sequentially over
> all hartids on the mask, we may send a HALT event to the
> current hart before the loop finishes. So we will halt
> the current hart before it can deliver a HALT IPI to the
> rest and some harts will remain active.
>
> Make sure we send an IPI to the current hart after we've
> finished with everybody else.
>
> Signed-off-by: Nick Kossifidis <mick at ics.forth.gr>
>
No issue. Already fixed it with following commit:
lib: Fix mask shift in sbi_ipi_send_many()
The mask shift in for-loop of sbi_ipi_send_many() is
broken with commit 918c135
("lib: Improve delivery of SBI_IPI_EVENT_HALT")
This patch fix it.
Signed-off-by: Anup Patel <anup.patel at wdc.com>
Please pull latest OpenSBI repo.
Thanks,
Anup
More information about the U-Boot
mailing list