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.

Sign up for GitHub

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

Jump to bottom

Multiple improvements to the inline assembly code #251

Merged
merged 10 commits into from
May 13, 2021
Merged

Multiple improvements to the inline assembly code #251

merged 10 commits into from
May 13, 2021

Conversation

josephlr
Copy link
Contributor

@josephlr josephlr commented May 6, 2021 •
edited
Loading

See the individual commits for more information.

Amongst other things, this PR adds external_asm implementations for some functions that were missing it and documents those functions which still require inline_asm

Example Docs:

docs

@josephlr josephlr requested a review from phil-opp May 6, 2021 09:14
phil-opp
phil-opp approved these changes May 6, 2021
View reviewed changes
Copy link
Member

@phil-opp phil-opp left a comment

Choose a reason for hiding this comment

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

Thanks a lot! Looks all good to me. Displaying the feature requirements on docs.rs is a great idea!

src/instructions/interrupts.rs Outdated Show resolved Hide resolved
src/instructions/mod.rs Outdated Show resolved Hide resolved
@josephlr
Fix Clippy warnings
ff6cbc0 
Signed-off-by: Joe Richey <[email protected]>
@josephlr
Consistently format ASM blocks
cc7f286 
This mostly makes the code shorter and easier to read

Signed-off-by: Joe Richey <[email protected]>
@josephlr
Use correct asm! options
92920d5 
This adds some previously omitted options to our inline asm!

Note that instuctions that just push/pop the stack (but otherwise
don't touch memory) can be marked `nomem`. We also don't mark
instuctions as `nomem` if they could modify memory translation:

This includes
  - writing segment registers
  - invlpg/invpcid
  - writing cr2/cr3/cr4
  - writing any MSR

(we still mark these as `nostack` though.

The following are still marked `nomem`:
  - Reading any MSR (as this cannot have a side-effect)
  - Writing XCR0

Signed-off-by: Joe Richey <[email protected]>
@josephlr
Simplify _x86_64_asm_read_rflags
fe15d60 
Signed-off-by: Joe Richey <[email protected]>
@josephlr
Add non "inline_asm" impl for bochs_breakpoint
24b9a45 
Signed-off-by: Joe Richey <[email protected]>
@josephlr
Do wrmsr bit-twiddling in Rust
dd0b377 
Signed-off-by: Joe Richey <[email protected]>
@josephlr
Add non inline_asm XCr0 implementation
8d61248 
Signed-off-by: Joe Richey <[email protected]>
@josephlr
Add doc(cfg()) noting that some functions need nightly
951d2ba 
Signed-off-by: Joe Richey <[email protected]>
@josephlr
Copy link
Contributor Author

josephlr commented May 6, 2021 •
edited
Loading

@phil-opp it looks like using compile_error!() in read_rip() doesn't do what we want (it breaks the whole build instead of just removing that function if external_asm is used). So I removed it, I also removed the similar usage from software_interrupt!{} so we are consistent.

Finally, I changed the docs to mention nightly and inline_asm, so they look like:
docs2

If this looks good to you, feel free to merge it.

EDIT: I also updated the Changelog

@josephlr
Don't use compile_error!
8f7e5d6 
Also, update the docs attrube to point to both relevant features.

Signed-off-by: Joe Richey <[email protected]>
@josephlr
Update Changelog
2d1e4a2 
Signed-off-by: Joe Richey <[email protected]>
phil-opp
phil-opp approved these changes May 13, 2021
View reviewed changes
Copy link
Member

@phil-opp phil-opp left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

src/registers/rflags.rs
Comment on lines +122 to +123
// HACK: we mark this function as preserves_flags to prevent Rust from restoring
// saved flags after the "popf" below. See above note on safety.
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for improving this comment, it's much clearer now!

@phil-opp phil-opp merged commit 2792ba6 into master May 13, 2021
@phil-opp phil-opp deleted the asm branch May 13, 2021 20:18
@phil-opp
Copy link
Member

Published as v0.14.2

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

@phil-opp phil-opp phil-opp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@josephlr @phil-opp