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

Jump to bottom

Zephyr: fix build breakage with Zephyr "main" #9199

Merged
merged 3 commits into from
Jun 12, 2024
Merged

Zephyr: fix build breakage with Zephyr "main" #9199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f53b41d
west: update to Zephyr "main", fixing disabled LLEXT builds
lyakh Jun 5, 2024
074dd67
llext: don't build if CONFIG_LLEXT=n
lyakh Jun 6, 2024
5c74c70
llext: only enable on ACE platforms
lyakh Jun 6, 2024
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
4 changes: 4 additions & 0 deletions app/boards/intel_adsp_ace15_mtpm.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ CONFIG_LL_WATCHDOG=y

CONFIG_MEMORY_WIN_2_SIZE=12288

CONFIG_LLEXT=y
CONFIG_LLEXT_STORAGE_WRITABLE=y
CONFIG_MODULES=y

# Temporary disabled options
CONFIG_TRACE=n
CONFIG_COMP_KPB=y
Expand Down
4 changes: 4 additions & 0 deletions app/boards/intel_adsp_ace20_lnl.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ CONFIG_PROBE_DMA_MAX=2

CONFIG_MEMORY_WIN_2_SIZE=12288

CONFIG_LLEXT=y
CONFIG_LLEXT_STORAGE_WRITABLE=y
CONFIG_MODULES=y


# Temporary disabled options
CONFIG_TRACE=n
Expand Down
4 changes: 0 additions & 4 deletions app/prj.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,3 @@ CONFIG_SCHED_CPU_MASK_PIN_ONLY=y
CONFIG_SYS_CLOCK_TICKS_PER_SEC=15000
CONFIG_DAI=y
CONFIG_HEAP_MEM_POOL_SIZE=2048

CONFIG_LLEXT=y
CONFIG_LLEXT_STORAGE_WRITABLE=y
CONFIG_MODULES=y
6 changes: 6 additions & 0 deletions src/include/sof/llext_manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,16 @@
struct comp_driver;
struct comp_ipc_config;

#if CONFIG_LLEXT
uintptr_t llext_manager_allocate_module(struct processing_module *proc,
const struct comp_ipc_config *ipc_config,
const void *ipc_specific_config);

int llext_manager_free_module(const uint32_t component_id);
#else
#define llext_manager_allocate_module(proc, ipc_config, ipc_specific_config) 0
#define llext_manager_free_module(component_id) 0
#define llext_unload(ext) 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wow, this looks really weird.... I think it's not just "creative", unusual and harder to read...

  • I don't think having these being either functions or macros depending on Kconfig is a good idea. Debugging Kconfig and preprocessing issues is painful enough already, so having totally different error messages depending on CONFIG_LLEXT seems wrong. Please keep these functions always: static inline uintptr_tllext_manager_free_module(component_id) { return 0; }. The generated code should be the same.

  • Naive question sorry: why are these functions needed when CONFIG_LLEXT is false? I don't know all the details sorry but returning NULLs and zeros is an anti-pattern. Ideally, these shouldn't even be needed so a short comment/pointer explaining why would not hurt.

  • where is llext_unload() defined when CONFIG_LLEXT is true?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  • Naive question sorry: why are these functions needed when CONFIG_LLEXT is false? I don't know all the details sorry but returning NULLs and zeros is an anti-pattern. Ideally, these shouldn't even be needed so a short comment/pointer explaining why would not hurt.

@marc-hb because they're called from lib_manager.c and that one is included in TGL builds too. No, I don't think it's needed there for the same reason as LLEXT - no memory mapping support.

  • where is llext_unload() defined when CONFIG_LLEXT is true?

in Zephyr

#endif

#endif
2 changes: 2 additions & 0 deletions src/library_manager/lib_manager.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@

#include <zephyr/cache.h>
#include <zephyr/drivers/mm/system_mm.h>
#if CONFIG_LLEXT
#include <zephyr/llext/llext.h>
#endif

#if CONFIG_LIBRARY_AUTH_SUPPORT
#include <auth/intel_auth_api.h>
Expand Down
2 changes: 1 addition & 1 deletion west.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ manifest:

- name: zephyr
repo-path: zephyr
revision: 53ddff639562ef68dc0a6f62b82f7505c75ebdce
revision: 0a3f2f0397a86425cc7d12fa3a0c0ab8020d80e1
remote: zephyrproject

# Import some projects listed in zephyr/west.yml@revision
Expand Down
2 changes: 1 addition & 1 deletion zephyr/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ zephyr_library_sources_ifdef(CONFIG_LIBRARY_MANAGER
${SOF_SRC_PATH}/library_manager/lib_notification.c
)

if (CONFIG_MM_DRV)
if (CONFIG_MM_DRV AND CONFIG_LLEXT)
zephyr_library_sources_ifdef(CONFIG_LIBRARY_MANAGER
${SOF_SRC_PATH}/library_manager/llext_manager.c
)
Expand Down
2 changes: 1 addition & 1 deletion zephyr/lib/cpu.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ void cpu_disable_core(int id)
return;

/* Broadcasting interrupts to other cores. */
arch_sched_ipi();
arch_sched_broadcast_ipi();

uint64_t timeout = k_cycle_get_64() +
k_ms_to_cyc_ceil64(CONFIG_SECONDARY_CORE_DISABLING_TIMEOUT);
Expand Down
4 changes: 3 additions & 1 deletion zephyr/wrapper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,10 @@ int poll_for_register_delay(uint32_t reg, uint32_t mask,
*/
volatile int *_sof_fatal_null = NULL;

struct arch_esf;

void k_sys_fatal_error_handler(unsigned int reason,
const z_arch_esf_t *esf)
const struct arch_esf *esf)
{
ARG_UNUSED(esf);

Expand Down