Skip to content

Conversation

@kisum9
Copy link
Contributor

@kisum9 kisum9 commented Dec 3, 2025

TrappedFP also only available for FEAT_AA32.

Originally posted by @thejpster in #60 (comment)

Fixed issue #62

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates documentation comments for Exception Class (EC) values in the ESR (Exception Syndrome Register) definitions to correctly annotate feature dependencies. The primary change adds the (FEAT_AA32) annotation to the TrappedFP exception class, clarifying that trapping of floating-point/SIMD functionality is only available in AArch32 state.

Key Changes

  • Added (FEAT_AA32) annotation to TrappedFP exception class comments in both ESR_EL1 and ESR_EL2 registers
  • Added (FEAT_BTI) annotation to IllegalExecutionState exception class comment in ESR_EL2 register

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/registers/esr_el1.rs Updated TrappedFP documentation to indicate FEAT_AA32 dependency
src/registers/esr_el2.rs Updated TrappedFP documentation to indicate FEAT_AA32 dependency and IllegalExecutionState to indicate FEAT_BTI dependency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

/// Branch Target Exception (FEAT_BTI)
BranchTarget = 0b00_1101,
/// Illegal Execution state
/// Illegal Execution state (FEAT_BTI)
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

The IllegalExecutionState exception (EC=0b001110) is not specific to FEAT_BTI. According to ARM architecture specifications, this exception class is used when an attempt is made to execute in an illegal execution state (e.g., attempting to execute AArch32 code when AArch32 is not supported at the current exception level). This is a general architectural exception that exists independently of FEAT_BTI.

While BranchTarget (EC=0b001101) is correctly annotated with FEAT_BTI, IllegalExecutionState should not have this annotation. Consider removing the (FEAT_BTI) annotation from this line.

Suggested change
/// Illegal Execution state (FEAT_BTI)
/// Illegal Execution state

Copilot uses AI. Check for mistakes.
@berkus
Copy link
Member

berkus commented Dec 7, 2025

@kisum9 I think copilot is right here, FEAT_BTI is not related to IllegalStateException, it's just unfortunate markup errors in the ARM ARM.

Copy link
Member

@berkus berkus left a comment

Choose a reason for hiding this comment

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

Please remove FEAT_BTI.

@thejpster
Copy link

So I think both of these only apply when FEAT_BTI is set. They've elided the second row in that column because it's the same as the one above.

image

@kisum9
Copy link
Contributor Author

kisum9 commented Dec 8, 2025

@berkus I agree with @thejpster. ESR_EL1 and ESR_EL2 both set FEAT_BTI for IllegalExecutionState,maybe add comment like “With FEAT_BTI implemented, this EC may also be used for BTI-related faults“

@kisum9 kisum9 force-pushed the fix-TrappedFP-annotation branch from 2c05616 to 704513f Compare December 8, 2025 03:37
@berkus
Copy link
Member

berkus commented Dec 9, 2025

So I think both of these only apply when FEAT_BTI is set. They've elided the second row in that column because it's the same as the one above.

hmm I've looked in other places and it doesn't seem elided to me, e.g.

image

Are instruction aborts only supported when scalable matrix extension is enabled? I don't think so.

@berkus
Copy link
Member

berkus commented Dec 9, 2025

This document might look more usable.

@berkus
Copy link
Member

berkus commented Dec 9, 2025

In the ARM ARM markup they just literally missed to fill in the <td> when there is no text, leading to this weird look.

image

@kisum9
Copy link
Contributor Author

kisum9 commented Dec 9, 2025

Got it ;)

@kisum9 kisum9 force-pushed the fix-TrappedFP-annotation branch from 704513f to 3282b2a Compare December 9, 2025 01:18
@berkus berkus merged commit d40bea9 into rust-embedded:main Dec 10, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants