Re: [U-Boot] [PATCH 1/7] riscv: add infrastructure for calling functions on other harts

On Mon, Feb 18, 2019 at 5:11 PM Auer, Lukas lukas.auer@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@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@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@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@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@wdc.com

Please pull latest OpenSBI repo.

Thanks, Anup