Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib/libc: Add picolibc support (aarch32, aarch64 and RISC-V) [v14] #39563

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -875,6 +875,9 @@ if(CONFIG_USERSPACE)
if(CONFIG_NEWLIB_LIBC_NANO)
set(NEWLIB_PART -l libc_nano.a z_libc_partition)
endif()
if(CONFIG_PICOLIBC)
set(NEWLIB_PART -l libc.a z_libc_partition)
endif()

add_custom_command(
OUTPUT ${APP_SMEM_UNALIGNED_LD} ${APP_SMEM_PINNED_UNALIGNED_LD}
Expand Down
2 changes: 1 addition & 1 deletion arch/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ config ARM
# FIXME: current state of the code for all ARM requires this, but
# is really only necessary for Cortex-M with ARM MPU!
select GEN_PRIV_STACKS
select ARCH_HAS_THREAD_LOCAL_STORAGE if CPU_CORTEX_R || CPU_CORTEX_M
select ARCH_HAS_THREAD_LOCAL_STORAGE
help
ARM architecture

Expand Down
1 change: 1 addition & 0 deletions arch/arm/core/aarch32/cortex_a_r/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ zephyr_library_sources(
)

zephyr_library_sources_ifdef(CONFIG_USERSPACE thread.c)
zephyr_library_sources_ifdef(CONFIG_THREAD_LOCAL_STORAGE __aeabi_read_tp.S)
20 changes: 20 additions & 0 deletions arch/arm/core/aarch32/cortex_a_r/__aeabi_read_tp.S
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright (c) 2020 Intel Corporation
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <toolchain.h>

_ASM_FILE_PROLOGUE

GTEXT(__aeabi_read_tp)

SECTION_FUNC(text, __aeabi_read_tp)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any chance that you could instead make this as a cmsis function, probably putting it in include/arch/arm/aarch32/cortex_a_r/cmsis.h until it could be moved to the true arm cmsis module? I assume you probably copied this structure from Cortex-M, but since this uses a mrc, it makes sense to abstract this access.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other side of this operation, storing the TLS value into the register, is done in swap_helper.S without any abstraction, so I'm not sure it makes sense to create an abstraction here?

I also don't think we should use this function anywhere; it's a part of EABI, but should only be used in code compiled with -mtp=soft. However, as Zephyr is the code base storing the TLS base address, it makes sense for Zephyr to offer the standard ABI that also fetches it, so I went ahead and added it.

/*
* Load TLS base address, which is stored in the TPIDRURO register, also
* known as the "Process ID" register. Refer to the code in z_arm_pendsv
* to see where this register is set.
*/
mrc 15, 0, r0, c13, c0, 3
bx lr
2 changes: 1 addition & 1 deletion arch/arm/core/aarch32/swap_helper.S
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ out_fp_endif:
adds r4, r2, r4
ldr r0, [r4]

#if defined(CONFIG_CPU_CORTEX_R)
#if defined(CONFIG_CPU_CORTEX_R) || defined(CONFIG_CPU_CORTEX_A)
/* Store TLS pointer in the "Process ID" register.
* This register is used as a base pointer to all
* thread variables with offsets added by toolchain.
Expand Down
1 change: 1 addition & 0 deletions cmake/toolchain/gnuarmemb/generic.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,6 @@ set(SYSROOT_TARGET arm-none-eabi)
set(CROSS_COMPILE ${TOOLCHAIN_HOME}/bin/${CROSS_COMPILE_TARGET}-)
set(SYSROOT_DIR ${TOOLCHAIN_HOME}/${SYSROOT_TARGET})
set(TOOLCHAIN_HAS_NEWLIB ON CACHE BOOL "True if toolchain supports newlib")
set(TOOLCHAIN_HAS_PICOLIBC ON CACHE BOOL "True if toolchain supports picolibc")

message(STATUS "Found toolchain: gnuarmemb (${GNUARMEMB_TOOLCHAIN_PATH})")
1 change: 1 addition & 0 deletions cmake/toolchain/xtools/generic.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ set(SYSROOT_TARGET ${CROSS_COMPILE_TARGET})
set(CROSS_COMPILE ${some_toolchain_root}/${CROSS_COMPILE_TARGET}/bin/${CROSS_COMPILE_TARGET}-)
set(SYSROOT_DIR ${some_toolchain_root}/${SYSROOT_TARGET}/${SYSROOT_TARGET})
set(TOOLCHAIN_HAS_NEWLIB ON CACHE BOOL "True if toolchain supports newlib")
set(TOOLCHAIN_HAS_PICOLIBC ON CACHE BOOL "True if toolchain supports picolibc")

unset(some_toolchain_root)
unset(some_toolchain)
Expand Down
20 changes: 17 additions & 3 deletions include/sys/libc-hooks.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* that need to call into the kernel as system calls
*/

#if defined(CONFIG_NEWLIB_LIBC) || defined(CONFIG_ARCMWDT_LIBC)
#if defined(CONFIG_NEWLIB_LIBC) || defined(CONFIG_ARCMWDT_LIBC) || defined(CONFIG_PICOLIBC)

/* syscall generation ignores preprocessor, ensure this is defined to ensure
* we don't have compile errors
Expand Down Expand Up @@ -56,7 +56,21 @@ extern struct k_mem_partition z_malloc_partition;
*/
#define Z_MALLOC_PARTITION_EXISTS 1
#endif /* CONFIG_MINIMAL_LIBC_MALLOC_ARENA_SIZE > 0 */
#endif /* CONFIG_MINIMAL_LIBC */
#elif defined(CONFIG_PICOLIBC)
/* If we are using picolibc, the heap arena is in one of two areas:
* - If we have an MPU that requires power of two alignment, the heap bounds
* must be specified in Kconfig via CONFIG_PICOLIBC_ALIGNED_HEAP_SIZE.
* - Otherwise, the heap arena on most arches starts at a suitably
* aligned base addreess after the `_end` linker symbol, through to the end
* of system RAM.
*/
#if (!defined(CONFIG_MPU_REQUIRES_POWER_OF_TWO_ALIGNMENT) || \
(defined(CONFIG_MPU_REQUIRES_POWER_OF_TWO_ALIGNMENT) && \
CONFIG_PICOLIBC_ALIGNED_HEAP_SIZE))
#define Z_MALLOC_PARTITION_EXISTS 1
extern struct k_mem_partition z_malloc_partition;
#endif
#endif /* CONFIG_PICOLIBC */

#ifdef Z_MALLOC_PARTITION_EXISTS
/* Memory partition containing the libc malloc arena. Configuration controls
Expand All @@ -66,7 +80,7 @@ extern struct k_mem_partition z_malloc_partition;
#endif

#if defined(CONFIG_NEWLIB_LIBC) || defined(CONFIG_STACK_CANARIES) || \
defined(CONFIG_NEED_LIBC_MEM_PARTITION)
defined(CONFIG_PICOLIBC) || defined(CONFIG_NEED_LIBC_MEM_PARTITION)
/* - All newlib globals will be placed into z_libc_partition.
* - Minimal C library globals, if any, will be placed into
* z_libc_partition.
Expand Down
8 changes: 8 additions & 0 deletions kernel/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,17 @@ config THREAD_USERSPACE_LOCAL_DATA
depends on USERSPACE
default y if ERRNO && !ERRNO_IN_TLS

config LIBC_ERRNO
bool
help
Use external libc errno, not the internal one. This makes sure that
ERRNO is not set for libc configurations that provide their own
implementation.

config ERRNO
bool "Enable errno support"
default y
depends on !LIBC_ERRNO
help
Enable per-thread errno in the kernel. Application and library code must
include errno.h provided by the C library (libc) to use the errno
Expand Down
3 changes: 3 additions & 0 deletions kernel/errno.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,13 @@ static inline int *z_vrfy_z_errno(void)
#include <syscalls/z_errno_mrsh.c>

#else
#ifndef CONFIG_LIBC_ERRNO
int *z_impl_z_errno(void)
{
return &_current->errno_var;
}
#endif /* CONFIG_PICOLIBC */

#endif /* CONFIG_USERSPACE */

#endif /* CONFIG_ERRNO_IN_TLS */
Expand Down
2 changes: 2 additions & 0 deletions lib/libc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,6 @@ elseif(CONFIG_MINIMAL_LIBC)
add_subdirectory(minimal)
elseif(CONFIG_ARMCLANG_STD_LIBC)
add_subdirectory(armstdc)
elseif(CONFIG_PICOLIBC)
add_subdirectory(picolibc)
endif()
33 changes: 33 additions & 0 deletions lib/libc/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@ config MINIMAL_LIBC
help
Build with minimal C library.

config PICOLIBC
bool "Picolibc library"
depends on !NATIVE_APPLICATION
select THREAD_LOCAL_STORAGE if ARCH_HAS_THREAD_LOCAL_STORAGE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering why TLS is always enabled. Can picolib work without TLS if choose to do so?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TLS must be enabled in zephyr if picolibc is built with TLS support. Otherwise, any TLS variables in picolibc will not work. I'm using 'ARCH_HAS_THREAD_LOCAL_STORAGE' as a proxy for whether picolibc was built with TLS support as I don't know how to check for that at configure time (picolibc has settings in picolibc.h which could be used, is there some way to look at those?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If picolibc TLS is also always enabled when building for those architectures, this is fine.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that depends on the toolchain. ARM ships their GCC embedded toolchain with TLS disabled. As there is no impact on code not using TLS in either case, I assume the ARM people just don't realize they've disabled it. Crosstool-ng and debian both ship with toolchains that enable TLS support in the compiler and picolibc. Once the sdk-ng PR for picolibc is merged, then zephyr can rely on that toolchain for picolibc, where TLS is enabled.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, might want to add "&& !(arm toolchain)" for the if condition, since TLS is not required.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be a lot better to figure out if picolibc has TLS support automatically; anyone know enough cmake to do that?

select LIBC_ERRNO
help
Build with picolibc library. The picolibc library is expected to be
part of the SDK in this case.

config NEWLIB_LIBC
bool "Newlib C library"
depends on !NATIVE_APPLICATION
Expand All @@ -53,6 +62,30 @@ endchoice # LIBC_IMPLEMENTATION
config HAS_NEWLIB_LIBC_NANO
bool

if PICOLIBC

config PICOLIBC_INTEGER_PRINTF
bool "Build with picolibc integer-only printf"
help
Build with floating point printf disabled. This will reduce the size
of the image.

config PICOLIBC_ALIGNED_HEAP_SIZE
int "Picolibc aligned heap size (bytes)"
depends on MPU_REQUIRES_POWER_OF_TWO_ALIGNMENT
depends on USERSPACE
default 0
help
If user mode is enabled, and MPU hardware has requirements that
regions be sized to a power of two and aligned to their size,
and user mode threads need to access this heap, then this is necessary
to properly define an MPU region for the heap.

If this is left at 0, then remaining system RAM will be used for this
area and it may not be possible to program it as an MPU region.

endif # PICOLIBC

if NEWLIB_LIBC

config NEWLIB_LIBC_NANO
Expand Down
41 changes: 41 additions & 0 deletions lib/libc/picolibc/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# SPDX-License-Identifier: Apache-2.0

zephyr_library()
zephyr_library_sources(libc-hooks.c)

# Zephyr normally uses -ffreestanding, which with current GNU toolchains
# means that the flag macros used by picolibc <inttypes.h> to signal
# support for PRI.64 macros are not present. To make them available we
# need to hook into the include path before the system files and
# explicitly include the picolibc header that provides those macros.
zephyr_include_directories(include)

# define __LINUX_ERRNO_EXTENSIONS__ so we get errno defines like -ESHUTDOWN
# used by the network stack
zephyr_compile_definitions(__LINUX_ERRNO_EXTENSIONS__)

zephyr_link_libraries(
m
c
gcc # Lib C depends on libgcc.
)

# The -T/dev/null avoids pulling in picolibc.ld
zephyr_link_libraries(
--specs=picolibc.specs
-T/dev/null
)

zephyr_compile_options(
--specs=picolibc.specs
-D_GNU_SOURCE
)

if(CONFIG_PICOLIBC_INTEGER_PRINTF)
zephyr_compile_options(
-DPICOLIBC_INTEGER_PRINTF_SCANF
)
zephyr_link_libraries(
-DPICOLIBC_INTEGER_PRINTF_SCANF
)
endif()
Loading