Skip to content

Conversation

@lipengfei28
Copy link
Contributor

@lipengfei28 lipengfei28 commented Sep 25, 2024

  1. Summary
    The original implement for exception handler is very simple and
    haven't framework for breakpoint/watchpoint routine or brk instruction.
    I refine the fatal handler and add framework for debug handler to
    register or unregister. this is a prepare for watchpoint/breakpoint implement
  2. add arm64_current_el to obtain current EL]

Summary

Impact

Testing

@github-actions github-actions bot added Arch: arm64 Issues related to ARM64 (64-bit) architecture Size: L The size of the change in this PR is large labels Sep 25, 2024
@nuttxpr
Copy link

nuttxpr commented Sep 25, 2024

[Experimental Bot, please feedback here]

NuttX PR Requirements Review:

Meets Requirements?: Partially

Reasoning:

  • Summary: The summary provides a high-level overview but lacks specifics.

    • Needs Improvement: Clearly state the functional part of the code being changed (e.g., exception handling mechanism) and explicitly describe how the change works (e.g., "introduces a registration mechanism for debug handlers, allowing for breakpoint/watchpoint functionality in the future").
  • Impact: The "Impact" section is entirely missing.

    • Needs Addition: Address all the bullet points in the "Impact" section of the requirements. Pay particular attention to:
      • New/changed features: Describe the framework for debug handlers.
      • Impact on users: Will users need to modify their code or configuration?
      • Impact on hardware/boards: Are there specific architectures or boards affected?
      • Documentation: Does this change necessitate documentation updates? If so, are they included in the PR?
  • Testing: The "Testing" section is incomplete.

    • Needs Improvement:
      • Specify the build host OS, CPU, compiler, and versions used.
      • List the target architectures, boards, and configurations tested.
      • Provide actual testing logs (or links to them) before and after the change.

Recommendations:

  1. Expand the Summary: Provide more detailed information about the changes made.
  2. Complete the Impact Section: Address each bullet point with specific details relevant to your changes.
  3. Fill out the Testing Section: Include concrete information about your testing environment and results.

By addressing these points, you'll make your PR clearer, easier to review, and more likely to be merged.

@lipengfei28 lipengfei28 requested a review from acassis September 25, 2024 13:47
Summary
  The original implement for exception handler is very simple and
haven't framework for breakpoint/watchpoint routine or brk instruction.
  I refine the fatal handler and add framework for debug handler to
register or unregister. this is a prepare for watchpoint/breakpoint
implement

Signed-off-by: qinwei1 <qinwei1@xiaomi.com>
Summary
  Add a macro to obtain current execute level

Signed-off-by: qinwei1 <qinwei1@xiaomi.com>
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!

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

Labels

Arch: arm64 Issues related to ARM64 (64-bit) architecture Size: L The size of the change in this PR is large

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants