Skip to content

Conversation

@pussuw
Copy link
Contributor

@pussuw pussuw commented Oct 18, 2024

Summary

Add boilerplate code for enabling SMP mode for MPFS target.

Impact

Add SMP build support for MPFS target. No other target is affected.

Testing

Build passes
Simple MPFS SMP target boots to nsh console

@github-actions github-actions bot added Arch: risc-v Issues related to the RISC-V (32-bit or 64-bit) architecture Size: M The size of the change in this PR is medium labels Oct 18, 2024
MPFS implements external interrupt control on a per-hart basis i.e. there
are PLIC control registers for each hart separately. This means we need
a procedure to initialize such registers for each hart individually,
instead of only for the boot hart like it is now.

Fix this by implementing mpfs_plic_init_hart which can be called by each
hart as needed.

Note: it is not a good idea to initialize all harts from the boot hart,
as the boot hart may not know which harts are used by NuttX in AMP
configuration. It is better that the hart initializes itself.

Note: The hartid must be provided as explicit parameter, as it cannot
be queried via riscv_mhartid() yet; the per-cpu structure is initialized
later on which means riscv_mhartid() would return 0 for all harts except
the boot hart.
@nuttxpr
Copy link

nuttxpr commented Oct 18, 2024

[Experimental Bot, please feedback here]

Fill In The Commit Message: This PR contains a Commit with an Empty Commit Message. Please fill in the Commit Message with the PR Summary.

Yes, the PR appears to meet the NuttX requirements.

Here's why:

  • Summary: Clearly explains the purpose (add SMP support to MPFS), what part of the code is affected (MPFS target), and how it works (adding boilerplate code).
  • Impact:
    • Accurately identifies the added feature (SMP build support for MPFS).
    • Addresses all impact areas, indicating no change or describing the specific impact.
  • Testing:
    • Confirms testing on a local setup.
    • Provides basic evidence that the build passes and the system boots.

Suggestions for Improvement (to make it even stronger):

  • Summary: Consider linking a related NuttX issue if one exists for tracking the feature request or bug.
  • Testing:
    • Specificity: Be more specific about the build host and target used for testing (e.g., "Linux, x86_64, GCC 12.2.0", "sim:qemu-rv32").
    • Functionality: Go beyond basic boot testing. Demonstrate that SMP functionality within the MPFS target is working correctly (e.g., running a multi-threaded application that interacts with the filesystem).
    • Logs (Optional): While not strictly required in this case, providing relevant snippets of logs (especially around SMP initialization) can be very helpful for reviewers.

By adding these minor enhancements, the PR would be even stronger and easier for reviewers to assess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Arch: risc-v Issues related to the RISC-V (32-bit or 64-bit) architecture Size: M The size of the change in this PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants