[PATCH v4 1/2] bsp/riscv: Work area size based on stack pointer
Daniel Cederman
cederman at gaisler.com
Wed Aug 17 08:56:23 UTC 2022
On 2022-08-15 17:19, Hesham Almatary wrote:
> 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?
>
Thank you for your comments. Thinking about this I think it would make
more sense to get the memory size from the fdt instead, and not use the
non-standard way of providing it via the stack pointer. I saw that this
was the approach used in the altera-cyclone-v BSP. I will post a patch
that uses this method instead.
>
>>>> /* 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