-
Notifications
You must be signed in to change notification settings - Fork 26
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 multiple registers and update fields #27
Conversation
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
TERR, TLOR, TSW, TACR, TIDCP, TID3, BSU, FB Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
Thanks @bitboom . I'll review your changes. |
|
||
/// When FEAT_GICv3_NMI is implemented: | ||
/// Indicates whether the virtual priority has the non-maskable property. | ||
NMI OFFSET(59) NUMBITS(1) [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I don't see NMI field listed in the ARM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we may need to agree on what version of the ARM we follow. This field does appear at least in Issue H of the Arm Generic Interrupt Controller (Arm IHI 0069H (ID020922)).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am good with supporting the broadest range, and it already has a note "When FEAT_GICv3_NMI is implemented" so I guess that's good.
src/registers/id_aa64dfr0_el1.rs
Outdated
register_bitfields! {u64, | ||
pub ID_AA64DFR0_EL1 [ | ||
/// Branch Record Buffer Extension. | ||
BRBE OFFSET(52) NUMBITS(4) [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be converted into an enum, only 3 valid values for 4 available bits.
src/registers/hpfar_el2.rs
Outdated
NS OFFSET(63) NUMBITS(1) [], | ||
|
||
/// Faulting Intermediate Physical Address. | ||
FIPA OFFSET(4) NUMBITS(40) [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at possible encodings it is probably safe to encode FIPA as OFFSET(0) NUMBITS(48)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After taking a closer look, FIPA could be OFFSET(4) NUMBIT(48)
. It's a bit confusing when viewed on the web due to the indentation, but it's clearer in the PDF version.
This conditions below refer to an offset within the 44 bits of FIPA[47:4].
- NS, bit [63]
- Bits [62:48]
- FIPA, bits [47:4]
- FIPA encoding when FEAT_D128 is implemented
- FIPA, bits [43:0]: Bits [55:12] of the Faulting Intermediate Physical Address.
- FIPA encoding when FEAT_LPA is implemented and FEAT_D128 is not implemented
- Bits [43:40] Reserved, RES0.
- FIPA, bits [39:0]: Bits [51:12] of the Faulting Intermediate Physical Address
- FIPA encoding when FEAT_LPA is not implemented
- Bits [43:36]: Reserved, RES0.
- FIPA, bits [35:0]: Bits[47:12] of the Faulting Intermediate Physical Address.
- FIPA encoding when FEAT_D128 is implemented
- Bits [3:0]: Reserved, RES0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, alright, yeah that makes sense, it was a bit unclear in the web version.
@bitboom some nitpicks and one factual error in bitfield offset, otherwise looking good! Please fix at least the invalid OFFSET and we can merge this. Preferably implement enum values for these wide fields, as I see you've already done in some other registers. Thanks! |
Signed-off-by: Sangwan Kwon <sangwan.kwon@samsung.com>
@berkus Thank you for the detailed review 😄 Most of the feedback has been reflected, and I've added a comment regarding the |
I added the following registers by referring here.
Added registers
Updated registers