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 missing IDT entries #CP and #HV #387

Merged
merged 8 commits into from
Jul 21, 2022
Merged

Add missing IDT entries #CP and #HV #387

merged 8 commits into from
Jul 21, 2022

Conversation

Zildj1an
Copy link
Contributor

Add missing IDT entries #CP (vector number 21) and #HV (vector number 28) in the InterruptDescriptorTable. Both entries are documented in AMD's APM Volume 2 (here), section 8.2.

This pull request addresses issues #377 and #378.

Signed-off-by: Carlos Bilbao [email protected]

Zildj1an added 2 commits July 20, 2022 11:17
Currently the InterruptDescriptorTable is missing entry with vector number
28, #HV. More information at AMD's APM Volume 2, subsection 8.2.21. See
issue #377.

Signed-off-by: Carlos Bilbao <[email protected]>
Currently the InterruptDescriptorTable is missing entry with vector number
21, #CP. More information at AMD's APM Volume 2, subsection 8.2.20. See
issue #378.

Signed-off-by: Carlos Bilbao <[email protected]>
Copy link
Member

@Freax13 Freax13 left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to implement this!

Good job on the doc comments, I really like them!

A few more steps are required to fully integrate the new exception vectors:

  • For the Index and IndexMut implementations: add cases for hv_injection_exception here and here and cases for cp_protection_exception here and here
  • For set_general_handler_entry!: this and this arm must be removed. An arm similar to this must be added for cp_protection_exception (hv_injection_exception will be handled here and doesn't need to be explicitly added)
  • For the tests: changes are needed here and here

src/structures/idt.rs Show resolved Hide resolved
Zildj1an added 5 commits July 21, 2022 09:40
Include the cases for #CP (cp_protection_exception) and #HV
(hv_injection_exception) in the Index and IndexMut implementations.

Signed-off-by: Carlos Bilbao <[email protected]>
Add the following changes to macro set_general_handler_entry! to account
for the newly added vector entries:
For #HV (vector 28) we need to remove the empty arm.
For #CP (vector 21) we need to remove the empty arm, and include a new arm.

Signed-off-by: Carlos Bilbao <[email protected]>
Update tests for IDTs considering newly added vector entries #CP and #HV.

Signed-off-by: Carlos Bilbao <[email protected]>
Fix typo in #CP definition.

Signed-off-by: Carlos Bilbao <[email protected]>
The #CP (vector 21) exception can also be generated due to a missing
ENDBRANCH instruction if indirect branch tracking is enabled. Include this
in the comments.

Signed-off-by: Carlos Bilbao <[email protected]>
Copy link
Member

@Freax13 Freax13 left a comment

Choose a reason for hiding this comment

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

Two of the range indices need to be fixed, but other than that this looks good to me!

src/structures/idt.rs Outdated Show resolved Hide resolved
src/structures/idt.rs Outdated Show resolved Hide resolved
Make case #CP (21) reachable for Index and IndexMut implementations.

Signed-off-by: Carlos Bilbao <[email protected]>
@Zildj1an
Copy link
Contributor Author

Thanks for all your feedback, @Freax13. It was very helpful. I think this is ready now :)

Copy link
Member

@Freax13 Freax13 left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!

@Freax13 Freax13 merged commit 922c557 into rust-osdev:master Jul 21, 2022
@Freax13 Freax13 mentioned this pull request Sep 15, 2023
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.

Add Missing IDT entry #CP (vector 21) Add missing IDT entry #HV (vector 28)
2 participants