-
-
Notifications
You must be signed in to change notification settings - Fork 176
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
Introduce pte_flags
crate with support for aarch64
and x86_64
#696
Conversation
pte_flags
crate replaces entryflags_x86_64
and supports both aarch64
& x86_64
pte_flags
crate with support for aarch64
and x86_64
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.
Thanks for this PR!
There are a few design issues with this:
- The main point of this redesign is for you (and me jointly) to determine which flags are shared across the two architectures, and also how to express x86-specific terminology/concepts like "global", "cacheable", and "writeback/write-through" flags in aarch64-specific terms (which may also depend on how we intend to use them in Theseus). For example,
x86
has a single concept of "global" or not, whileaarch64
has a different concept of "shareable". The purpose of this is to determine how they are similar or different and to have a single - The existing conversion routine from "software" flags to hardware flags is quite slow and overly complex, making it hard to understand what the similarities and differences are across the architectures.
- There aren't dedicated structs that define the arch-specific PTE flags, making it hard to understand the actual physical layout of PTE flags on each architecture. The main idea was to have 3 structs: one "software" (arch-generic), one specific to
aarch64
, and one specific tox86_64
. Ifaarch64
andx86_64
were similar enough, the top-level arch-generic "software" PTE flags could be theoretically removed entirely, though it'd be nice to have that top-level flag struct to make future architectures easier to integrate. Then, the idea would be to implement conversion routines to and from those structs.- This is also required in case we need to set a arch-specific flag bit that doesn't have a corresponding representation in the "software" flags or on another arch.
- Converting from hardware flags to "software" flags isn't supported, but that's a fundamental requirement since we sometimes need to read and interpret the actual flag values from the hardware PTEs. This is also the reason for the above point about needing actual concrete struct definitions for the arch-specific
PteFlags
- Please take my descriptive comments from the existing
EntryFlags
and use that as a model for these. Theaarch64
PteFlags
should include equally detailed comments - (minor) these flags are used in
MappedPages
, yes, but they don't strictly have anything to do with that, so the name shouldn't reflect that.PteFlags
orPagingFlags
is better.
What i had in mind after our chat was a significantly different design (described above); I can either mock it up now or wait for you to take a second crack at it first, but be sure to deeply consider the above points and also how others would use your new types.
I can share with you my understanding of these in aarch64:
I don't think these shareability/cacheability attributes map directly to the GLOBAL bit on x86_64.
After having approximatively understood that shareability mechanism, I wonder why
I must say I don't know for sure what's slow and what's fast when dealing with bit flags in Rust; I had assumed the ternary
No problem, will do.
Yes
OK
I will try to solve the problems you mentioned (but I don't know how to improve the speed of the conversion) and improve the crate. |
…ation, conversion routines between arch-specific flags and software ones.
Okay great, this is a huge improvement over the first iteration! There's still lots of room for improvement, but now we have a good base from which to start.
Well, for most things, it's best to actually profile (benchmark) the routines you're writing to see if they're actually slow. Alternatively, take a look at the generated assembly to see how well the compiler was able to optimize your code. The main way to make things fast is ... do less work. I know it sounds simple, but here's a concrete example. In your design above, you have multiple conditional branches for each and every bit in PteFlags ... that's a lot of conditionals, which are slow. Instead, one could design the struct to maximize the similarities between the two architectures, i.e., to fully take advantage of the bits that have the same bit position and meaning in both x86_64 and aarch64. For the sake of learning, here is my thought process when undertaking this design:
Another (potential) issue is with how you defined the bitflags constants; that crate doesn't really expect 0-value bit consts to be defined and it's various accessor functions may not work properly with those present... see the bitflags docs for more details and gotchas about how that works. I am working on a redesign which I will push later this weekend. Then, we can discuss it to ensure our understanding of the various PTE flags on aarch64 is correct. |
See #699 for my attempt at this design. Hopefully that will serve as a model for future strategies on implementing things for aarch64 and x86_64 in a simple, compatible, and efficient manner. Feel free to leave comments on that PR; we can also discuss both PRs comparatively on discord or in person. |
Can I close this, since #699 replaces it? |
Closing in favor of #699 now that we have discussed things in depth. |
This introduces a replacement for
entryflags_x86_64
which supports aarch64 and x86_64; support for other architectures can be added easily.Currently the only documentation is on following methods of
MappedPageAttributes
:That documentation describes the semantics of the corresponding flags. Feel free to suggest other things to document.