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 flags for CR0, CR4 and XCR0, as well as extra checks for modification of XCR0 #272

Closed
wants to merge 17 commits into from

Conversation

ethindp
Copy link
Contributor

@ethindp ethindp commented Jun 28, 2021

This PR adds some extras to the three control registers CR0, CR4, and XCR0. It also adds some checks for XCR0. (We could extend this to other registers as well.)

Bits added

  • CR0:
    • Extension Type (ET, bit 4 of CR0)
  • CR4:
    • Key-Locker-Enable Bit (KL, bit 19 of CR4)
    • Control-Flow Enforcement Technology (CET) enable (CET, bit 23 of CR4)
    • Enable protection keys for supervisor-mode pages (PKS, bit 24 of CR4)
  • XCR0:
    • Enable XSAVE to manage the bound registers (BNDREG, bit 3 of XCR0)
    • Enable XSAVE to manage the BCFGU and BNDSTATUS registers (BNDCSR, bit 4 of XCR0)
    • Enable AVX-512 and allow XSAVE to manage the mask registers K0-K7 (OPMASK, bit 5 of XCR0)
    • Enable AVX-512 and allow XSAVE to manage the upper halves of the lower ZMM registers (ZMM_HI256, bit 6 of XCR0)
    • Enable AVX-512 and allow XSAVE to manage the upper ZMM registers (HI16_ZMM, bit 7 of XCR0)

Checks

This PR adds invariant checks that cannot be disabled via the assert! macro when using the write() function on the XCr0 type. Primarily, these invariants enforce those listed on pg. 2-20 of volume 3A of the intel SDMs; they disallow attempts to:

  • clear XCR0.x87;
  • clear XCR0.SSE and set XCR0.AVX;
  • clear XCR0.AVX and set any of XCR0.opmask, XCR0.ZMM_Hi256, and XCR0.Hi16_ZMM;
  • set either XCR0.BNDREG and XCR0.BNDCSR while not setting the other; and
  • set any of XCR0.opmask, XCR0.ZMM_Hi256, and XCR0.Hi16_ZMM while not setting all of them.

I missed the initial invariant (that setting any reserved bits causes a general-protection fault) but I will add that if you guys would like me to (somebody else can do so as well). We should probably consider enforcing all invariants on all control registers to catch problems like this before they arise and generate exceptions, since these APIs are supposed to be "safe". This PR unfortunately has a few breaking changes as well.

Breaking changes

In order for CR4.PKS to coexist with CR4.PKU and for the documentation to make sense, I was forced to rename CR4.PKU to PROTECTION_KEY_USER. I also renamed XCr0Flags::YMM to XCr0Flags::AVX to make the names match with the documentation. I hope you guys don't mind.

ethindp and others added 17 commits September 11, 2019 21:45
Signed-off-by: Ethin Probst <ethindp@protonmail.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
This function only throws a #UD, which we generally consider to be safe.
Also, add an `Exceptions` section to the `Segment64` docs (this is
similar to the `Panic` section in normal Rust docs).

Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Signed-off-by: Joe Richey <joerichey@google.com>
Add common abstractions for x86 Segments
This ensures that the arguments are passed by RDI, RSI, RDX, RCX, etc.
… as extra checks when writing to XCR0.

Signed-off-by: Ethin Probst <ethindp@protonmail.com>
@josephlr
Copy link
Contributor

If this is a breaking change, can you open this PR against the next branch?

@ethindp
Copy link
Contributor Author

ethindp commented Jun 28, 2021

Oh. Sure. Is there a way I can just shift the PR to the next branch? Or will I need to reopen it?

@josephlr
Copy link
Contributor

Oh. Sure. Is there a way I can just shift the PR to the next branch? Or will I need to reopen it?

I think I can rebase it onto the next branch

@josephlr josephlr changed the base branch from master to next June 28, 2021 07:25
@josephlr
Copy link
Contributor

Oh. Sure. Is there a way I can just shift the PR to the next branch? Or will I need to reopen it?

I think I can rebase it onto the next branch

Actually, it looks like this is your master branch, so I don't want to mess it up. I changed the base to next, I'll let you rebase things appropriately.

@josephlr
Copy link
Contributor

josephlr commented Jul 7, 2021

Closing in favor of #273

@josephlr josephlr closed this Jul 7, 2021
@josephlr josephlr mentioned this pull request Jul 7, 2021
13 tasks
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