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

Add kernel build support for X86_64 #14957

Merged
merged 11 commits into from
Nov 27, 2024
Merged

Conversation

szafonimateusz-mi
Copy link
Contributor

Summary

  • arch/x86_64/: fix broken set_cr3()
  • arch/x86_64: add syscalls support
  • arch/x86_64/include/intel64/arch.h: align definitions
  • arch/x86_64: add kernel stack support
  • arch/x86_64: handle TLB shootdown
  • boards/qemu-intel64: add kernel build (ROMFS) configurations
  • arch/x86_64/intel64_irq.c: remove some magic numbers

Impact

kernel build supported for x86_64

Testing

ostest on qemu and bare metal Intel HW

arch/x86_64/: fix broken set_cr3()

Signed-off-by: p-szafonimateusz <[email protected]>
arch/x86_64: add syscalls support

Signed-off-by: p-szafonimateusz <[email protected]>
arch/x86_64/include/intel64/arch.h: align definitions

Signed-off-by: p-szafonimateusz <[email protected]>
arch/x86_64: add kernel build support

Signed-off-by: p-szafonimateusz <[email protected]>
arch/x86_64: add kernel stack support

Signed-off-by: p-szafonimateusz <[email protected]>
arch/x86_64: handle TLB shootdown

Signed-off-by: p-szafonimateusz <[email protected]>
@github-actions github-actions bot added Arch: x86_64 Issues related to the x86_64 architecture Board: x86_64 Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces. labels Nov 26, 2024
@nuttxpr
Copy link

nuttxpr commented Nov 26, 2024

[Experimental Bot, please feedback here]

No, this PR description does not fully meet the NuttX requirements. While it provides a summary of the changes, it lacks crucial details required for proper review. Here's a breakdown of what's missing and how to improve it:

Summary:

  • Missing "Why": The summary lists what changed, but not why. Explain the reason for each change. For example, why was set_cr3() broken? Why was syscall support needed? Why did definitions need alignment? Provide context.
  • Missing "How": While the "what" is present, the "how" is generally lacking. How does the kernel stack support work? How is the TLB shootdown handled? Be more specific.
  • Missing Issue References: Are there any related NuttX issues or NuttX Apps issues/PRs that this addresses? If so, include the links.

Impact:

  • Too vague: Saying "kernel build supported for x86_64" is not enough. Be specific about the impact of each change listed in the summary.
  • Address all points: The provided impact section only touches on the "new feature added" aspect. Explicitly address all the impact points (user impact, build impact, hardware impact, documentation, security, compatibility, and anything else). Use "NO" if there's no impact, but explain why there's no impact. For example: "Impact on user: NO. These changes are primarily internal kernel improvements and do not affect the user interface or APIs."

Testing:

  • Insufficient Detail: "ostest on qemu and bare metal Intel HW" is far too brief. Provide specific details about your testing environment:
    • Build Host: e.g., "Linux Ubuntu 20.04, Intel i7-10700K, GCC 11.2.0"
    • Target: e.g., "qemu-system-x86_64 -M q35, NuttX configuration: boards/qemu-intel64/configs/ostest" and "Bare metal: Intel NUC11, Core i7-1165G7"
  • Missing Logs: The template specifically requests "Testing logs before change" and "Testing logs after change." Include relevant log snippets demonstrating the issue before the change and the successful operation after the change. Focus on the specific areas affected by your modifications. If the logs are excessively long, consider using a service like Pastebin and linking to it.

Example of an improved description (for just one of the changes):

Summary

  • arch/x86_64/: fix broken set_cr3() - The previous implementation of set_cr3() incorrectly handled address masking, leading to page faults when switching page tables. This change corrects the masking operation to ensure proper address translation. Fixes arch/stackframe: fix heap buffer overflow #1234 (NuttX issue link).

Impact

  • Is new feature added? NO
  • Is existing feature changed? YES. The set_cr3() function is corrected.
  • Impact on user: NO. This is an internal kernel fix and should not have a user-visible impact. Existing applications using virtual memory should experience improved stability.
  • Impact on build: NO. No build process changes are required.
  • Impact on hardware: YES. This fix applies specifically to x86_64 architectures.
  • Impact on documentation: NO. No documentation changes are required as this is a bug fix to existing functionality.
  • Impact on security: YES. The incorrect address masking could potentially be exploited to gain unauthorized memory access. This fix mitigates this vulnerability.
  • Impact on compatibility: NO. This fix corrects existing behavior and maintains backward compatibility.
  • Anything else to consider? None.

Testing

  • Build Host(s): Linux Ubuntu 22.04, Intel i9-12900K, GCC 12.1.0
  • Target(s): qemu-system-x86_64 -M q35, NuttX configuration: boards/qemu-intel64/configs/ostest

Testing logs before change:

[paste a snippet of the logs where you could see the wrong masking issue]

Testing logs after change:

[paste a snippet of the logs after you applied your fix where you demonstrate working page switching with correct address translation]

Repeat this detailed structure for each item listed in the original summary. By providing comprehensive information, you significantly improve the review process and increase the chances of your PR being accepted.

knsh_romfs - for QEMU and legacy serial port
knsh_romfs_pci - for bare-metal Intel hardware and PCI serial port

Steps to build kernel image with user-space apps in romfs:

$ ./tools/configure.sh qemu-intel64/knsh_romfs
$ make -j
$ make export -j
$ pushd ../apps
$ ./tools/mkimport.sh -z -x ../nuttx/nuttx-export-*.tar.gz
$ make import -j
$ ./tools/mkromfsimg.sh
$ mv boot_romfsimg.h ../nuttx/arch/x86_64/src/board/romfs_boot.c
$ popd
$ make -j

Signed-off-by: p-szafonimateusz <[email protected]>
arch/x86_64/intel64_irq.c: remove some magic numbers

Signed-off-by: p-szafonimateusz <[email protected]>
Copy link
Contributor

@acassis acassis left a comment

Choose a reason for hiding this comment

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

 Documentation: add x86_64 kernel build configs
@github-actions github-actions bot added the Area: Documentation Improvements or additions to documentation label Nov 27, 2024
@raiden00pl
Copy link
Contributor

doc updated

@pussuw
Copy link
Contributor

pussuw commented Nov 27, 2024

No shared memory or vmalloc (kmap) support :(?

@raiden00pl
Copy link
Contributor

No shared memory or vmalloc (kmap) support :(?

maybe later if needed

remove unnecessary nested syscalls logic, it's already handled different way
arch/x86_64/intel64: re-enable interrupts before syscall handle
Copy link
Contributor

@pussuw pussuw left a comment

Choose a reason for hiding this comment

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

LGTM

@acassis acassis merged commit eca40ff into apache:master Nov 27, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: x86_64 Issues related to the x86_64 architecture Area: Documentation Improvements or additions to documentation Board: x86_64 Size: XL The size of the change in this PR is very large. Consider breaking down the PR into smaller pieces.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants