Conversation
portPOINTER_SIZE_TYPE is used in FreeRTOS to store both pointer and integer values. Using a plain integer type for this purpose is unsafe on architectures that distinguish between pointers and integers, where an integer type (e.g. uint64_t) may not be able to represent all pointer values. The standard intptr_t type is explicitly defined to safely hold either a pointer or an integer. This commit changes portPOINTER_SIZE_TYPE to intptr_t, similar to the existing POSIX port, improving portability of the RISC-V port. Similarly, portSTACK_TYPE is used to store word-sized values on the stack that may represent either integers or pointers, and is often cast to pointer types. Changing it to uintptr_t makes this usage explicit and correct, improving portability, intent, and safety for capability-based extensions such as CHERI-RISC-V. Signed-off-by: Hesham Almatary <Hesham.Almatary@cl.cam.ac.uk>
xTaskReturnAddress holds a [function] pointer, never an integer address. Use a proper exclusive function pointer type for this variable. Furthermore, use NULL instead of 0 for an invalid function pointer value This typedef could be made more generic for FreeRTOS-Kernel and not just for RISC-V; vApplicationTaskExit used to be declared/defined in previous versions and ideally could just be brought back. Signed-off-by: Hesham Almatary <Hesham.Almatary@cl.cam.ac.uk>
Add a new configMTIME_INIT_IN_BSP configuration macro to allow BSPs and demo applications to initialise the MMIO MTIME pointers within the CLINT. This enables BSPs to provide custom addresses or properly typed pointer values, rather than relying on plain integer constants coming from macros as is currently the case. This change does not affect existing assumptions and usage, but allows architectures that differentiate between integers and pointers to work. Signed-off-by: Hesham Almatary <Hesham.Almatary@cl.cam.ac.uk>
cookpate
force-pushed
the
riscv_enhancments
branch
from
a6a8bac to
2fe3c61
Compare
|
|
Seems reasonable enough. |
|
With all respect, @cookpate, this changes do not seems correct . The variables: volatile uint32_t * pulTimeHigh = ( volatile uint32_t * const ) ( ( configMTIME_BASE_ADDRESS ) + 4UL );
volatile uint32_t * pulTimeLow = ( volatile uint32_t * const ) ( configMTIME_BASE_ADDRESS );should only be defined when Additionally, any modification made here should also be applied to: portable/IAR/RISC-V/port.csince it contains nearly identical logic and should remain consistent with the GCC implementation. Extract from portable/IAR/RISC-V/port.c: #if ( configMTIME_BASE_ADDRESS != 0 ) && ( configMTIMECMP_BASE_ADDRESS != 0 )
void vPortSetupTimerInterrupt( void )
{
uint32_t ulCurrentTimeHigh, ulCurrentTimeLow;
volatile uint32_t * const pulTimeHigh = ( uint32_t * ) ( ( configMTIME_BASE_ADDRESS ) + 4UL ); /* 8-byte type so high 32-bit word is 4 bytes up. */
volatile uint32_t * const pulTimeLow = ( uint32_t * ) ( configMTIME_BASE_ADDRESS );
volatile uint32_t ulHartId;I am not a maintainer, so please take my comment with caution. Sorry if I made any mistake. |
|
3 participants
RISC-V port enhancements
Description
Non-breaking, backward-compatible enhancements to the RISC-V port mostly to do with types in order to make it more portable across architectures (eg: CHERI extensions that enforce memory safety and pointer provenance by separating pointer vs integer types) and environments (eg: POSIX).
Test Steps
Normal RISC-V demos should work, tested with
main_blinky.Checklist:
Related Issue
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.