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.

Sign up for GitHub

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

Kconfig: don't fall back on CONFIG_TIGERLAKE #7763

Merged
merged 4 commits into from
Jun 13, 2023
Merged

Kconfig: don't fall back on CONFIG_TIGERLAKE #7763

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eaedcec
xtensa/CMakeLists.txt: simplify CONFIG_RENOIR/REMBRANDT logic
marc-hb Jun 8, 2023
3c323c3
scripts/fuzz.sh: cosmetic move list of CONFIG_s to a bash array
marc-hb Jun 8, 2023
71f5fa7
scripts/fuzz.sh: add -DCONFIG_ZEPHYR_POSIX
marc-hb Jun 8, 2023
13ae274
Kconfig: don't fall back on CONFIG_TIGERLAKE
marc-hb Jun 8, 2023
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
19 changes: 12 additions & 7 deletions scripts/fuzz.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,18 @@ main()
: "${TEST_DURATION:=3}"
: "${FUZZER_STDOUT:=/dev/stdout}" # bashism

west build -d build-fuzz -b native_posix "$SOF_TOP"/app/ -- \
-DCONFIG_ASSERT=y \
-DCONFIG_SYS_HEAP_BIG_ONLY=y \
-DCONFIG_ZEPHYR_NATIVE_DRIVERS=y \
-DCONFIG_ARCH_POSIX_LIBFUZZER=y \
-DCONFIG_ARCH_POSIX_FUZZ_TICKS=100 \
-DCONFIG_ASAN=y "$@"
# Move this to a new fuzz.conf overlay file if it grows bigger
local fuzz_configs=(
-DCONFIG_ZEPHYR_POSIX=y
-DCONFIG_ASSERT=y
-DCONFIG_SYS_HEAP_BIG_ONLY=y
-DCONFIG_ZEPHYR_NATIVE_DRIVERS=y
-DCONFIG_ARCH_POSIX_LIBFUZZER=y
-DCONFIG_ARCH_POSIX_FUZZ_TICKS=100
-DCONFIG_ASAN=y
)

west build -d build-fuzz -b native_posix "$SOF_TOP"/app/ -- "${fuzz_configs[@]}" "$@"

mkdir -p ./fuzz_corpus

Expand Down
19 changes: 9 additions & 10 deletions src/arch/xtensa/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,27 @@ elseif(CONFIG_IMX8M)
set(platform_folder imx8m)
elseif(CONFIG_IMX8ULP)
set(platform_folder imx8ulp)
elseif(CONFIG_RENOIR)
set(platform_folder amd/renoir)
elseif(CONFIG_REMBRANDT)
set(platform_folder amd/rembrandt)
elseif(CONFIG_MT8186)
set(platform_folder mt8186)
elseif(CONFIG_MT8188)
set(platform_folder mt8188)
elseif(CONFIG_MT8195)
set(platform_folder mt8195)
else()
message(FATAL_ERROR "Platform not defined, check your Kconfiguration?")
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW: you could move this to a build-time #error in a C file somewhere to avoid having to do this in two different cmake schemes.

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 respectfully but strongly disagree. I think it's better to check and fail as early as possible. Also, C compilation is parallelized and typically unreadable before adding -j1 whereas CMake configuration is single-threaded. I think it's also better to always have a "default" error clause in a case/switch, and/or to check a variable is not empty before using it (the -I platform/${EMPTY}/include wasted a large amount of my time!)

I avoid CMake as much as possible but the duplication here is only one line. It's only because Zephyr versus non-Zephyr so it won't grow larger.

Finally, in which C file?

All this being said, if there is some C file where this check could be useful TOO then why not; "defence in depth".

endif()

set(fw_name ${CONFIG_RIMAGE_SIGNING_SCHEMA})

set(platform_ld_script ${platform_folder}.x)
# File name without directory
get_filename_component(_plf_ld_script ${platform_folder} NAME)
set(platform_ld_script ${_plf_ld_script}.x)
set(platform_rom_ld_script rom.x)

if(CONFIG_RENOIR)
set(platform_folder amd/renoir)
set(platform_ld_script renoir.x)
endif()

if(CONFIG_REMBRANDT)
set(platform_folder amd/rembrandt)
set(platform_ld_script rembrandt.x)
endif()
# includes

# None of these should be included if Zephyr strict headers are used.
Expand Down
4 changes: 3 additions & 1 deletion src/platform/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ menu "Platform"
choice
prompt "Platform"
default ZEPHYR_POSIX if ARCH_POSIX
default TIGERLAKE
# It's not really 'optional' but no value is much less confusing
# than falling back on a totally random value.
optional

config TIGERLAKE
bool "Build for Tigerlake"
Expand Down
3 changes: 3 additions & 0 deletions zephyr/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,9 @@ zephyr_library_sources_ifdef(CONFIG_LIBRARY
${SOF_DRIVERS_PATH}/host/ipc.c
)

if(NOT DEFINED PLATFORM)
message(FATAL_ERROR "PLATFORM is not defined, check your Kconfiguration?")
endif()
zephyr_include_directories(${SOF_PLATFORM_PATH}/${PLATFORM}/include)

# Mandatory Files used on all platforms.
Expand Down