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

Auer, Lukas lukas.auer at aisec.fraunhofer.de
Mon Feb 18 09:53:06 UTC 2019


On Mon, 2019-02-18 at 09:10 +0530, Anup Patel wrote:
> On Mon, Feb 18, 2019 at 8:53 AM Rick Chen <rickchen36 at gmail.com>
> wrote:
> > <rick at andestech.com> 於 2019年2月18日 週一 上午11:00寫道:
> > > 
> > > 
> > > > -----Original Message-----
> > > > From: Auer, Lukas [mailto:lukas.auer at aisec.fraunhofer.de]
> > > > Sent: Monday, February 18, 2019 5:55 AM
> > > > To: u-boot at lists.denx.de; Anup.Patel at wdc.com
> > > > Cc: anup at brainfault.org; bmeng.cn at gmail.com; schwab at suse.de;
> > > > Rick Jian-Zhi
> > > > Chen(陳建志); palmer at sifive.com; Atish.Patra at wdc.com; 
> > > > agraf at suse.de
> > > > Subject: Re: [PATCH 1/7] riscv: add infrastructure for calling
> > > > functions on other
> > > > harts
> > > > 
> > > > On Tue, 2019-02-12 at 01:44 +0000, Anup Patel wrote:
> > > > > > -----Original Message-----
> > > > > > From: Lukas Auer [mailto:lukas.auer at aisec.fraunhofer.de]
> > > > > > Sent: Tuesday, February 12, 2019 3:44 AM
> > > > > > To: u-boot at lists.denx.de
> > > > > > Cc: Atish Patra <Atish.Patra at wdc.com>; Anup Patel
> > > > > > <Anup.Patel at wdc.com>; Bin Meng <bmeng.cn at gmail.com>;
> > > > > > Andreas
> > > > Schwab
> > > > > > <schwab at suse.de>; Palmer Dabbelt <palmer at sifive.com>;
> > > > > > Alexander Graf
> > > > > > <agraf at suse.de>; Lukas Auer <lukas.auer at aisec.fraunhofer.de
> > > > > > >; Anup
> > > > > > Patel <anup at brainfault.org>; Rick Chen <rick at andestech.com>
> > > > > > Subject: [PATCH 1/7] riscv: add infrastructure for calling
> > > > > > functions
> > > > > > on other harts
> > > > > > 
> > > > > > 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]; #endif
> > > > > >  };
> > > > > > 
> > > > > >  #include <asm-generic/global_data.h>
> > > > > > diff --git a/arch/riscv/include/asm/smp.h
> > > > > > b/arch/riscv/include/asm/smp.h
> > > > > > new file mode 100644 index 0000000000..bc863fdbaf
> > > > > > --- /dev/null
> > > > > > +++ b/arch/riscv/include/asm/smp.h
> > > > > > @@ -0,0 +1,53 @@
> > > > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > > > +/*
> > > > > > + * Copyright (C) 2019 Fraunhofer AISEC,
> > > > > > + * Lukas Auer <lukas.auer at aisec.fraunhofer.de>  */
> > > > > > +
> > > > > > +#ifndef _ASM_RISCV_SMP_H
> > > > > > +#define _ASM_RISCV_SMP_H
> > > > > > +
> > > > > > +/**
> > > > > > + * struct ipi_data - Inter-processor interrupt (IPI) data
> > > > > > structure
> > > > > > + *
> > > > > > + * IPIs are used for SMP support to communicate to other
> > > > > > harts
> > > > > > what
> > > > > > +function to
> > > > > > + * call. Functions are in the form
> > > > > > + * void (*addr)(ulong hart, ulong arg0, ulong arg1).
> > > > > > + *
> > > > > > + * The function address and the two arguments, arg0 and
> > > > > > arg1, are
> > > > > > +stored in the
> > > > > > + * IPI data structure. The hart ID is inserted by the hart
> > > > > > handling the
> > > > > > +IPI and
> > > > > > + * calling the function.
> > > > > > + *
> > > > > > + * @addr: Address of function
> > > > > > + * @arg0: First argument of function
> > > > > > + * @arg1: Second argument of function
> > > > > > + */
> > > > > > +struct ipi_data {
> > > > > > + ulong addr;
> > > > > > + ulong arg0;
> > > > > > + ulong arg1;
> > > > > > +};
> > > > > > +
> > > > > > +/**
> > > > > > + * handle_ipi() - interrupt handler for software
> > > > > > interrupts
> > > > > > + *
> > > > > > + * The IPI interrupt handler must be called to handle
> > > > > > software
> > > > > > +interrupts. It
> > > > > > + * calls the function specified in the hart's IPI data
> > > > > > structure.
> > > > > > + *
> > > > > > + * @hart: Hart ID of the current hart
> > > > > > + */
> > > > > > +void handle_ipi(ulong hart);
> > > > > > +
> > > > > > +/**
> > > > > > + * smp_call_function() - Call a function on all other
> > > > > > harts
> > > > > > + *
> > > > > > + * Send IPIs with the specified function call to all
> > > > > > harts.
> > > > > > + *
> > > > > > + * @addr: Address of function
> > > > > > + * @arg0: First argument of function
> > > > > > + * @arg1: Second argument of function
> > > > > > + * @return 0 if OK, -ve on error
> > > > > > + */
> > > > > > +int smp_call_function(ulong addr, ulong arg0, ulong arg1);
> > > > > > +
> > > > > > +#endif
> > > > > > diff --git a/arch/riscv/lib/Makefile
> > > > > > b/arch/riscv/lib/Makefile
> > > > > > index
> > > > > > edfa61690c..19370f9749 100644
> > > > > > --- a/arch/riscv/lib/Makefile
> > > > > > +++ b/arch/riscv/lib/Makefile
> > > > > > @@ -14,6 +14,7 @@ obj-$(CONFIG_SIFIVE_CLINT) +=
> > > > > > sifive_clint.o
> > > > > >  obj-y    += interrupts.o
> > > > > >  obj-y    += reset.o
> > > > > >  obj-y   += setjmp.o
> > > > > > +obj-$(CONFIG_SMP) += smp.o
> > > > > > 
> > > > > >  # For building EFI apps
> > > > > >  CFLAGS_$(EFI_CRT0) := $(CFLAGS_EFI)
> > > > > > diff --git a/arch/riscv/lib/smp.c b/arch/riscv/lib/smp.c
> > > > > > new file
> > > > > > mode 100644
> > > > > > index 0000000000..1266a2a0ef
> > > > > > --- /dev/null
> > > > > > +++ b/arch/riscv/lib/smp.c
> > > > > > @@ -0,0 +1,110 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > +/*
> > > > > > + * Copyright (C) 2019 Fraunhofer AISEC,
> > > > > > + * Lukas Auer <lukas.auer at aisec.fraunhofer.de>  */
> > > > > > +
> > > > > > +#include <common.h>
> > > > > > +#include <dm.h>
> > > > > > +#include <asm/barrier.h>
> > > > > > +#include <asm/smp.h>
> > > > > > +
> > > > > > +DECLARE_GLOBAL_DATA_PTR;
> > > > > > +
> > > > > > +/**
> > > > > > + * riscv_send_ipi() - Send inter-processor interrupt (IPI)
> > > > > > + *
> > > > > > + * Platform code must provide this function.
> > > > > > + *
> > > > > > + * @hart: Hart ID of receiving hart
> > > > > > + * @return 0 if OK, -ve on error
> > > > > > + */
> > > > > > +extern int riscv_send_ipi(int hart);
> > > > > > +
> > > > > > +/**
> > > > > > + * riscv_clear_ipi() - Clear inter-processor interrupt
> > > > > > (IPI)
> > > > > > + *
> > > > > > + * Platform code must provide this function.
> > > > > > + *
> > > > > > + * @hart: Hart ID of hart to be cleared
> > > > > > + * @return 0 if OK, -ve on error
> > > > > > + */
> > > > > > +extern int riscv_clear_ipi(int hart);
> > > > > > +
> > > > > > +static int send_ipi_many(struct ipi_data *ipi) {
> > > > > > + ofnode node, cpus;
> > > > > > + u32 reg;
> > > > > > + int ret;
> > > > > > +
> > > > > > + cpus = ofnode_path("/cpus");
> > > > > > + if (!ofnode_valid(cpus)) {
> > > > > > +         pr_err("Can't find cpus node!\n");
> > > > > > +         return -EINVAL;
> > > > > > + }
> > > > > > +
> > > > > > + ofnode_for_each_subnode(node, cpus) {
> > > > > > +         if (!ofnode_is_available(node))
> > > > > > +                 continue;
> > > > > 
> > > > > It is not correct to assume that whatever CPUs are marked
> > > > > available will come online. It is possible that certain
> > > > > available
> > > > > CPUs failed to come online due HW failure.
> > > > > 
> > > > 
> > > > This was intended so that we don't send IPIs to harts, which
> > > > have been
> > > > explicitly marked as disabled.
> > > > 
> > > > > Better approach would be keep an atomic bitmask of HARTs
> > > > > that have entered U-Boot. All HARTs that enter U-Boot will
> > > > > update the atomic HART available bitmask. We send IPI only
> > > > > to HARTs that are available as-per atomic HART bitmask.
> > > > > 
> > > > 
> > > > I'm not sure if this is required in U-Boot, since we are not
> > > > relying on
> > > > all harts to boot for U-Boot to function. We only try to boot
> > > > all harts
> > > > listed as available in the device tree.
> > > > 
> > 
> > It may also need to get information from each cpu nodes.
> > The cpu nodes is important, they must be match with the bitmask, or
> > it
> > maybe go wrong.
> > So the bitmask way seem not be necessary.
> 
> Makes sense. How about considering both HART DT nodes availability
> and HART online bitmask when sending IPI ?
> 
> My motivation behind HART online bitmask in U-Boot is because:
> 
> 1. HART DT nodes marked available in DTB might actually be in unknown
> state in case of HW failure. It is commonly seen that as silicon ages
> or
> due to continuous heating of silicon new HW failures start showing up
> over-time. This means even HARTs can fail to come online over-time.
> 
> 2. We are moving towards SBI v0.2 and SBI PM extensions. This means
> RISC-V SoCs that implement SBI PM extensions will only bring-up one
> HART at boot-time. Rest of the HARTs, will have to powerd-up using
> SBI calls. This means that even if HART DT node is marked available,
> the HART might be still be offline until someone from S-mode does
> SBI call to power-up HART.
> 
> Of course, we will be handling in above cases in OpenSBI too but it
> is better to have "HART online bitmask" in U-Boot as well so that it
> works fine if some buggy proprietary SBI runtime firmware is not
> handling above cases.
> 

Thanks for explaining! I agree, especially for case 2 we will need to
handle offline harts. I'll integrate a hart online bitmask in the next
version.

Thanks,
Lukas


More information about the U-Boot mailing list