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

boards/qemu-armv7a: revises knsh config #15626

Merged
merged 1 commit into from
Jan 21, 2025
Merged

Conversation

yf13
Copy link
Contributor

@yf13 yf13 commented Jan 21, 2025

Summary

This adjusts qemu-armv7a:knsh with following points:

  • use smaller memory so that QEMU option -m 1024 can be skipped.
  • use ELF_EXECUTABLE for apps to allow easier debugging.
  • enable graceful shutdown from NSH command.
  • enable more cases of ostest (e.g. vfork, pthread_exit etc).

Impacts

The qemu-armv7a:knsh target.

Testing

  • local checks with qemu-armv7a:nsh
  • CI checks.

@yf13 yf13 changed the title boards/qemu-armv7a: revise kernel build sample config boards/qemu-armv7a: revises knsh config Jan 21, 2025
@github-actions github-actions bot added Board: arm Size: S The size of the change in this PR is small labels Jan 21, 2025
@nuttxpr
Copy link

nuttxpr commented Jan 21, 2025

[Experimental Bot, please feedback here]

The PR summary is good, but lacks some detail on how the changes work. For instance, how is memory reduced? What specific changes enable graceful shutdown and more ostest cases? The Impact and Testing sections are too brief and don't meet the requirements.

Here's a revised version suggesting how to improve the PR description:

Summary

This PR adjusts the qemu-armv7a:knsh configuration to improve testing and resource usage. Specifically:

  • Reduced memory usage: The default configuration's memory allocation is reduced [explain how, e.g., by adjusting the configuration variable X to Y] allowing the QEMU -m 1024 option to be omitted. This streamlines execution and reduces resource requirements.
  • ELF Executables for apps: Applications are now built as ELF executables instead of [previous format]. This change enables easier debugging with tools like GDB.
  • Graceful Shutdown from NSH: A new command/mechanism [explain what command or mechanism was added] has been implemented to allow graceful shutdown of the simulated system from the NSH console. This facilitates cleaner testing and avoids abrupt termination.
  • Expanded ostest coverage: The configuration is updated to enable more ostest cases, including vfork, pthread_exit [and any others]. This provides more thorough testing of core OS functionalities.

Impact

  • Is new feature added? YES (Graceful shutdown from NSH, Expanded ostest coverage)
  • Is existing feature changed? YES (Memory usage, App executable format)
  • Impact on user: YES. Users running the qemu-armv7a:knsh target will experience reduced memory usage and have access to the new shutdown command [explain the command and how to use it]. They can also leverage debugging tools more effectively with ELF executables.
  • Impact on build: NO.
  • Impact on hardware: NO (Only impacts the qemu-armv7a simulated hardware).
  • Impact on documentation: YES. Documentation should be updated to reflect the new shutdown command and explain the benefits of the ELF executable change. (A documentation update is included in this PR).
  • Impact on security: NO.
  • Impact on compatibility: NO known compatibility issues.
  • Anything else to consider? None.

Testing

I confirm that changes are verified on local setup and works as intended:

  • Build Host: macOS Ventura, Intel Core i9, clang-14
  • Target: qemu-armv7a:knsh

Testing logs before change:

[Show output of relevant commands demonstrating the previous behavior, e.g., memory usage with `free`, trying to debug an application, attempting to shut down, and running ostest showing missing tests.]

Testing logs after change:

[Show output of the same commands demonstrating the improved behavior, e.g., reduced memory usage, successful debugging session, clean shutdown using the new command, and ostest output including the newly enabled tests.]

This adjusts `qemu-armv7a:knsh` with following changes:

- use smaller flash/memory so that `-m 1024` can be omitted.
- use ELF_EXECUTABLE for apps to allow easier app debugging.
- enable graceful shutdown from NSH command.
- enable more cases of ostest (e.g. vfork, pthread_exit etc).

Signed-off-by: Yanfeng Liu <[email protected]>
@yf13 yf13 force-pushed the armv7a-knsh-config branch from b8318ce to fb2109a Compare January 21, 2025 09:38
@anchao
Copy link
Contributor

anchao commented Jan 21, 2025

@yf13 You could add partner email to your personal github account

@anchao anchao merged commit 1828d09 into apache:master Jan 21, 2025
25 checks passed
@yf13 yf13 deleted the armv7a-knsh-config branch January 22, 2025 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Board: arm 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