[PATCH v4 1/2] bsp/riscv: Work area size based on stack pointer

Hesham Almatary Hesham.Almatary at cl.cam.ac.uk
Mon Aug 15 15:19:57 UTC 2022


On Mon, 15 Aug 2022 at 15:35, Daniel Cederman <cederman at gaisler.com> wrote:
>
> On 2022-08-15 15:43, Hesham Almatary wrote:
> > On Mon, 15 Aug 2022 at 08:16, Daniel Cederman <cederman at gaisler.com> wrote:
> >> From: Martin Aberg <maberg at gaisler.com>
> >>
> >> Remember the initial stack pointer in start.S. It can later be used to
> >> determine top of RAM.
> >> ---
> >>   bsps/riscv/include/bsp/start.h                | 67 ++++++++++++++++
> >>   .../shared/start/bspgetworkarea-fromstack.c   | 76 +++++++++++++++++++
> >>   bsps/riscv/shared/start/start.S               | 15 ++++
> >>   cpukit/score/cpu/riscv/include/rtems/asm.h    | 12 +++
> >>   4 files changed, 170 insertions(+)
> >>   create mode 100644 bsps/riscv/include/bsp/start.h
> >>   create mode 100644 bsps/riscv/shared/start/bspgetworkarea-fromstack.c
> >>
> >> diff --git a/bsps/riscv/include/bsp/start.h b/bsps/riscv/include/bsp/start.h
> >> new file mode 100644
> >> index 0000000000..f40f1873a7
> >> --- /dev/null
> >> +++ b/bsps/riscv/include/bsp/start.h
> >> @@ -0,0 +1,67 @@
> >> +/* SPDX-License-Identifier: BSD-2-Clause */
> >> +
> >> +/**
> >> + * @file
> >> + *
> >> + * @ingroup RTEMSBSPsRISCVSharedStart
> >> + *
> >> + * @brief RISC-V start definitions.
> >> + */
> >> +
> >> +/*
> >> + * Copyright (c) 2021 Cobham Gaisler AB.
> >> + *
> >> + * Redistribution and use in source and binary forms, with or without
> >> + * modification, are permitted provided that the following conditions
> >> + * are met:
> >> + * 1. Redistributions of source code must retain the above copyright
> >> + *    notice, this list of conditions and the following disclaimer.
> >> + * 2. Redistributions in binary form must reproduce the above copyright
> >> + *    notice, this list of conditions and the following disclaimer in the
> >> + *    documentation and/or other materials provided with the distribution.
> >> + *
> >> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> >> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> >> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> >> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> >> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> >> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> >> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> >> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> >> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> >> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> >> + * POSSIBILITY OF SUCH DAMAGE.
> >> + */
> >> +
> >> +#ifndef LIBBSP_RISCV_SHARED_START_H
> >> +#define LIBBSP_RISCV_SHARED_START_H
> >> +
> >> +/**
> >> + * @defgroup RTEMSBSPsRISCVSharedStart Start Support
> >> + *
> >> + * @ingroup RTEMSBSPsRISCVShared
> >> + *
> >> + * @brief Start support.
> >> + *
> >> + * @{
> >> + */
> >> +
> >> +#include <stdint.h>
> >> +
> >> +#ifdef __cplusplus
> >> +extern "C" {
> >> +#endif
> >> +
> >> +/*
> >> + * This variable is initialized by the first CPU entering the BSP start code.
> >> + * The value is the stack pointer at entry.
> >> + */
> >> +extern const uintptr_t riscv_start_stack_pointer;
> >> +
> >> +#ifdef __cplusplus
> >> +}
> >> +#endif
> >> +
> >> +/** @} */
> >> +
> >> +#endif /* LIBBSP_RISCV_SHARED_START_H */
> >> diff --git a/bsps/riscv/shared/start/bspgetworkarea-fromstack.c b/bsps/riscv/shared/start/bspgetworkarea-fromstack.c
> >> new file mode 100644
> >> index 0000000000..d55a876346
> >> --- /dev/null
> >> +++ b/bsps/riscv/shared/start/bspgetworkarea-fromstack.c
> >> @@ -0,0 +1,76 @@
> >> +/* SPDX-License-Identifier: BSD-2-Clause */
> >> +
> >> +/**
> >> + * @file
> >> + *
> >> + * @brief BSP specific initialization support routines
> >> + *
> >> + */
> >> +
> >> +/*
> >> + * COPYRIGHT (c) 1989-2020.
> >> + * On-Line Applications Research Corporation (OAR).
> >> + * Cobham Gaisler AB.
> >> + *
> >> + * Redistribution and use in source and binary forms, with or without
> >> + * modification, are permitted provided that the following conditions
> >> + * are met:
> >> + * 1. Redistributions of source code must retain the above copyright
> >> + *    notice, this list of conditions and the following disclaimer.
> >> + * 2. Redistributions in binary form must reproduce the above copyright
> >> + *    notice, this list of conditions and the following disclaimer in the
> >> + *    documentation and/or other materials provided with the distribution.
> >> + *
> >> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> >> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> >> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> >> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> >> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> >> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> >> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> >> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> >> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> >> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> >> + * POSSIBILITY OF SUCH DAMAGE.
> >> + */
> >> +
> >> +#include <bsp.h>
> >> +#include <bsp/start.h>
> >> +#include <bsp/bootcard.h>
> >> +
> >> +#include <rtems/sysinit.h>
> >> +
> >> +/*
> >> + *  These are provided by the linkcmds for ALL of the BSPs which use this file.
> >> + */
> >> +extern char WorkAreaBase[];
> >> +extern char RamEnd[];
> >> +
> >> +static Memory_Area _Memory_Areas[ 1 ];
> >> +
> >> +static void bsp_memory_initialize( void )
> >> +{
> >> +  char *end;
> >> +
> >> +  /* top of RAM inidicated by initial stack pointer */
> >> +  end = (char *) riscv_start_stack_pointer;
Do you need to extern riscv_start_stack_pointer?


> >> +  if (end == 0) {
> >> +    /* fall back to linker symbol if not set */
> >> +    end = RamEnd;
> >> +  }
> >> +  _Memory_Initialize( &_Memory_Areas[ 0 ], WorkAreaBase, end );
> >> +}
> >> +
> >> +RTEMS_SYSINIT_ITEM(
> >> +  bsp_memory_initialize,
> >> +  RTEMS_SYSINIT_MEMORY,
> >> +  RTEMS_SYSINIT_ORDER_MIDDLE
> >> +);
> >> +
> >> +static const Memory_Information _Memory_Information =
> >> +  MEMORY_INFORMATION_INITIALIZER( _Memory_Areas );
> >> +
> >> +const Memory_Information *_Memory_Get( void )
> >> +{
> >> +  return &_Memory_Information;
> >> +}
> >> diff --git a/bsps/riscv/shared/start/start.S b/bsps/riscv/shared/start/start.S
> >> index 3702f8ac2f..d87f41a3dd 100644
> >> --- a/bsps/riscv/shared/start/start.S
> >> +++ b/bsps/riscv/shared/start/start.S
> >> @@ -60,6 +60,9 @@ SYM(_start):
> >>          LADDR   t0, _RISCV_Exception_handler
> >>          csrw    mtvec, t0
> >>
> >> +       /* Save stack pointer so it can mark end of work area later on */
> >> +       mv      t3, sp
> >> +
> > I might be missing something, but I assume at this point of the code
> > execution, $sp is undefined and it depends on hardware/firmware that
> > runs before RTEMS what to reset $sp to. Isn't that the case?
> That is correct. It allows the debugger/simulator/bootloader to provide
> the size of the memory to the application without having it hardcoded in
> the image. It is the same approach that we use for LEON.

Thanks for explaining that. Could you also add that explanation in the
code as a comment (and perhaps also in the commit message)? Otherwise
someone who's reading that code would have no clue why this is being
done, especially that it's not part of the RISC-V specs or ABIs.

Have you also tested that patch on existing RISC-V platforms such as
Spike and/or QEMU? What happens if $sp is set to a "non-zero"
undefined value by the hardware on reset (which is likely the case on
FPGAs and real hardware), and no debugger/bootloader sets it?



> >>          /* Load stack pointer and branch to secondary processor start if necessary */
> >>   #ifdef RTEMS_SMP
> >>          LADDR   sp, _ISR_Stack_area_begin
> >> @@ -75,6 +78,9 @@ SYM(_start):
> >>          LADDR   sp, _ISR_Stack_area_end
> >>   #endif
> >>
> >> +       LADDR   t0, riscv_start_stack_pointer
> >> +       SREG    t3, 0(t0)
> >> +
> >>   #ifdef BSP_START_COPY_FDT_FROM_U_BOOT
> >>          mv      a0, a1
> >>          call    bsp_fdt_copy
> >> @@ -146,3 +152,12 @@ SYM(_start):
> >>   #endif
> >>
> >>   #endif /* RTEMS_SMP */
> >> +
> >> +       .section        .bsp_start_data, "aw"
> >> +       .align  PTR_ALIGN
> >> +
> >> +       .globl  riscv_start_stack_pointer
> >> +       .type   riscv_start_stack_pointer, @object
> >> +       .size   riscv_start_stack_pointer, PTR_SIZE
> >> +riscv_start_stack_pointer:
> >> +       PTR_VALUE       0
> >> diff --git a/cpukit/score/cpu/riscv/include/rtems/asm.h b/cpukit/score/cpu/riscv/include/rtems/asm.h
> >> index 4e09d16410..b20e7f49ba 100644
> >> --- a/cpukit/score/cpu/riscv/include/rtems/asm.h
> >> +++ b/cpukit/score/cpu/riscv/include/rtems/asm.h
> >> @@ -133,12 +133,24 @@
> >>
> >>   #define SREG sw
> >>
> >> +#define PTR_ALIGN 2
> >> +
> >> +#define PTR_SIZE 4
> >> +
> >> +#define PTR_VALUE .word
> >> +
> >>   #elif __riscv_xlen == 64
> >>
> >>   #define LREG ld
> >>
> >>   #define SREG sd
> >>
> >> +#define PTR_ALIGN 3
> >> +
> >> +#define PTR_SIZE 8
> >> +
> >> +#define PTR_VALUE .dword
> >> +
> >>   #endif /* __riscv_xlen */
> >>
> >>   #ifdef __riscv_cmodel_medany
> >> --
> >> 2.34.1
> >>
> >> _______________________________________________
> >> devel mailing list
> >> devel at rtems.org
> >> http://lists.rtems.org/mailman/listinfo/devel
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel


More information about the devel mailing list