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

tools/mkexport: kernel mode module/elf flags #15694

Merged
merged 2 commits into from
Jan 26, 2025

Conversation

yf13
Copy link
Contributor

@yf13 yf13 commented Jan 26, 2025

Summary

This adds exported LDMODULEFLAGS so that kernel modules can build with Kernel mode NuttX. Without it, we have link errors like:

make[3]: Entering directory '/tmp/apps/examples/module/chardev'
LD:  /tmp/apps/bin/chardev arm-none-eabi-ld: warning: cannot find entry symbol _start; defaulting to 0000000000008000
arm-none-eabi-ld: chardev.c.tmp.apps.examples.module.chardev.o: in function `module_uninitialize':
/tmp/apps/examples/module/chardev/chardev.c:111: undefined reference to `unregister_driver'
arm-none-eabi-ld: chardev.c.tmp.apps.examples.module.chardev.o: in function `module_initialize':
/tmp/apps/examples/module/chardev/chardev.c:129: undefined reference to `register_driver'
arm-none-eabi-ld: /tmp/apps/import/startup/crt0.o: in function `__start':
/tmp/nuttx/arch/arm/src/armv7-a/crt0.c:133: undefined reference to `main'
make[3]: *** [/tmp/apps/Application.mk:308: /tmp/apps/bin/chardev] Error 1

It also stops including NuttX side paths in exported LDELFFLAGS. Currently we have:

$ fgrep LDELFFLAGS apps/import/scripts/Make.defs
LDELFFLAGS = -e main -T /tmp/nuttx/libs/libc/modlib/gnu-elf.ld

But by design, exported SDK shall not refer to NuttX folder any more because apps side may not have access to kernel sources in the SDK scenario.

Impacts

Projects using exported SDK

Testing

  • local checks with EXAMPLES_MODULE enabled qemu-armv7a:nsh and qemu-armv7a:knsh
  • CI checks

@github-actions github-actions bot added Area: Tooling Size: XS The size of the change in this PR is very small labels Jan 26, 2025
@yf13 yf13 force-pushed the kernel-module branch 6 times, most recently from 0093f32 to 82abf4b Compare January 26, 2025 07:56
@yf13 yf13 changed the title tools/mkexport.sh: kernel mode modules tools/mkexport.sh: kernel build modules and elf flags Jan 26, 2025
@yf13 yf13 changed the title tools/mkexport.sh: kernel build modules and elf flags tools/mkexport: kernel mode module/elf flags Jan 26, 2025
tools/mkexport.sh Outdated Show resolved Hide resolved
tools/mkexport.sh Outdated Show resolved Hide resolved
Copy link
Member

@lupyuen lupyuen left a comment

Choose a reason for hiding this comment

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

Tested OK with rv-virt:knsh64. Thanks :-)
https://gist.github.com/lupyuen/26f74ad08cc11107ba450d51501cf5f2

nsh> uname -a
NuttX 0.0.0 82abf4bc44 Jan 26 2025 16:10:38 risc-v rv-virt
nsh> ostest
ostest_main: Exiting with status 0

yf13 added 2 commits January 26, 2025 16:28
This adds LDMODULEFLAGS export so that kernel module can build
for kernel mode NuttX via `make import` approach.

Signed-off-by: Yanfeng Liu <[email protected]>
This stops exported LDELFFLAGS from referring to kernel folder
linkscripts because kernel folder might not be present in such
app building use cases.

Signed-off-by: Yanfeng Liu <[email protected]>
@github-actions github-actions bot added Size: S The size of the change in this PR is small and removed Size: XS The size of the change in this PR is very small labels Jan 26, 2025
@nuttxpr
Copy link

nuttxpr commented Jan 26, 2025

[Experimental Bot, please feedback here]

Yes, this PR meets the NuttX requirements.

While the provided summary is good, it could be slightly improved by explicitly mentioning the why. Something like: "This change is necessary to enable building kernel modules with Kernel mode NuttX in exported SDK builds. Currently, missing LDMODULEFLAGS and incorrect LDELFFLAGS cause link errors..."

The Impacts section could be more detailed following the template. Specifically, address each point:

  • Is new feature added? Is existing feature changed?: Existing feature (module building) fixed.
  • Impact on user: YES. Users building kernel modules with exported SDK will now be able to do so successfully.
  • Impact on build: YES. LDMODULEFLAGS are now exported and LDELFFLAGS corrected.
  • Impact on hardware: NO.
  • Impact on documentation: Potentially YES. If documentation exists regarding building modules with exported SDK, it should be updated. If not, consider adding documentation.
  • Impact on security: NO.
  • Impact on compatibility: YES. Improves forward compatibility with exported SDK builds. Mention if any backward compatibility concerns exist (unlikely).
  • Anything else to consider?: None apparent.

The Testing section is generally adequate but could benefit from more specific details. Instead of "local checks," specify OS, compiler version, NuttX configuration. For example: "Build Host: Linux, GCC 12.2, NuttX configured for qemu-armv7a with Kernel mode and EXAMPLES_MODULE enabled." Similarly, be more explicit with the Target details: "Targets: qemu-armv7a:nsh and qemu-armv7a:knsh."

Including snippets of the actual before/after testing logs (even if short and illustrative) would strengthen the verification claim.

@xiaoxiang781216 xiaoxiang781216 merged commit d13b745 into apache:master Jan 26, 2025
39 checks passed
@yf13 yf13 deleted the kernel-module branch January 30, 2025 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Tooling Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants