-
Notifications
You must be signed in to change notification settings - Fork 123
TDX: Pull TLB flush ringbuf struct into support/, add tests #1387
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
base: main
Are you sure you want to change the base?
Conversation
@@ -1817,6 +1817,18 @@ pub mod hypercall { | |||
#[repr(transparent)] | |||
pub struct HvGvaRange(pub u64); | |||
|
|||
impl From<u64> for HvGvaRange { |
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.
why
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.
So that the new support crate can return a T rather than a raw u64
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 seems error prone for other users of HvGvaRange
.
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.
They could always do .0
or HvGvaRange(foo)
before, this just works with generics.
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 just worry that things like gpa.into()
are going to creep in, which will work as long as gpa
doesn't have any low bits set... but will cause random failures if there are low bits set.
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.
That's fair, but From + Into definitely feels like the right API to be exposing from the ringbuf itself so that you don't lose the type of whatever you're storing, right? Unless you'd rather the callers had to do the translations, but that just feels annoying.
Looks like loom doesn't build for Windows-aarch64. |
Yeah, this |
The added tests use loom to ensure that all possible order of operations work correctly. This required tweaking the orderings we use. Can be reviewed commit-by-commit.