-
Notifications
You must be signed in to change notification settings - Fork 389
Introduce enum-class for memory flags #2329
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,17 +26,10 @@ | |
| #include <sstream> | ||
| #include <expects> | ||
| #include <kernel/memmap.hpp> | ||
| #include <sys/mman.hpp> | ||
|
|
||
| namespace os::mem { | ||
|
|
||
| /** POSIX mprotect compliant access bits **/ | ||
| enum class Access : uint8_t { | ||
| none = 0, | ||
| read = 1, | ||
| write = 2, | ||
| execute = 4 | ||
| }; | ||
|
|
||
| using Raw_allocator = buddy::Alloc<false>; | ||
|
|
||
| /** Get default allocator for untyped allocations */ | ||
|
|
@@ -68,7 +61,7 @@ namespace os::mem { | |
| * Virtual to physical memory mapping. | ||
| * For interfacing with the virtual memory API, e.g. mem::map / mem::protect. | ||
| **/ | ||
| template <typename Fl = Access> | ||
| template <typename Fl = os::mem::Permission> | ||
| struct Mapping | ||
| { | ||
| static const size_t any_size; | ||
|
|
@@ -126,7 +119,7 @@ namespace os::mem { | |
| Map unmap(uintptr_t addr); | ||
|
|
||
| /** Get protection flags for page enclosing a given address */ | ||
| Access flags(uintptr_t addr); | ||
| Permission flags(uintptr_t addr); | ||
|
|
||
| /** Determine active page size of a given linear address **/ | ||
| uintptr_t active_page_size(uintptr_t addr); | ||
|
|
@@ -142,20 +135,20 @@ namespace os::mem { | |
| * might result in 513 4KiB pages or 1 2MiB page and 1 4KiB page getting | ||
| * protected. | ||
| **/ | ||
| Map protect(uintptr_t linear, size_t len, Access flags = Access::read); | ||
| Map protect(uintptr_t linear, size_t len, Permission flags = Permission::Read); // TODO(mazunki): consider whether we should default to Read here | ||
|
|
||
| /** | ||
| * Set and return access flags for a given linear address range | ||
| * The range is expected to be mapped by a previous call to map. | ||
| **/ | ||
| Access protect_range(uintptr_t linear, Access flags = Access::read); | ||
| Permission protect_range(uintptr_t linear, Permission flags = Permission::Read); // TODO(mazunki): consider whether we should default to Read here | ||
|
|
||
| /** | ||
| * Set and return access flags for a page starting at linear. | ||
| * @note : the page size can be any of the supported sizes and | ||
| * protection will apply for that whole page. | ||
| **/ | ||
| Access protect_page(uintptr_t linear, Access flags = Access::read); | ||
| Permission protect_page(uintptr_t linear, Permission flags = Permission::Read); // TODO(mazunki): consider whether we should default to Read here | ||
|
|
||
|
|
||
| /** Get the physical address to which linear address is mapped **/ | ||
|
|
@@ -176,20 +169,6 @@ namespace os::mem { | |
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
| // Enable bitwise ops on access flags | ||
| namespace util { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you remind me why you're breaking out a new header for this? The file is only 345 lines
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mainly because most of |
||
| inline namespace bitops { | ||
| template<> | ||
| struct enable_bitmask_ops<os::mem::Access> { | ||
| using type = typename std::underlying_type<os::mem::Access>::type; | ||
| static constexpr bool enable = true; | ||
| }; | ||
| } | ||
| } | ||
|
|
||
|
|
||
| namespace os::mem { | ||
|
|
||
| // | ||
|
|
@@ -333,11 +312,12 @@ namespace os::mem { | |
| virtual_move(uintptr_t src, size_t size, uintptr_t dst, const char* label) | ||
| { | ||
| using namespace util::bitops; | ||
| const auto flags = os::mem::Access::read | os::mem::Access::write; | ||
| const auto flags = os::mem::Permission::Data; // TODO(mazunki): shouldn't this inherit flags from @src? | ||
| // setup @dst as new virt area for @src | ||
| os::mem::map({dst, src, flags, size}, label); | ||
|
|
||
| // unpresent @src | ||
| os::mem::protect(src, size, os::mem::Access::none); | ||
| os::mem::protect(src, size, os::mem::Permission::Any); // TODO(mazunki): change to Permission::None when introduced | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| /* | ||
| * provides namespaced types for `sys_mman(0p)` values | ||
| */ | ||
| #ifndef _SYS_MMAN_HPP | ||
| #define _SYS_MMAN_HPP | ||
|
|
||
| #include <cstdint> | ||
| #include <sys/mman.h> | ||
| #include <util/bitops.hpp> | ||
| #include <type_traits> | ||
|
|
||
| namespace os::mem { | ||
| enum class Flags : uint8_t { | ||
| None = 0, | ||
| Shared = MAP_SHARED, | ||
| Private = MAP_PRIVATE, | ||
| Fixed = MAP_FIXED, | ||
| Anonymous = MAP_ANONYMOUS, | ||
| }; | ||
|
|
||
| enum class Permission : uint8_t { // TODO(mazunki): consider making Permission::{Read,Write,Execute} private or standalone class | ||
| Read = PROT_READ, | ||
| Write = PROT_WRITE, | ||
| Execute = PROT_EXEC, | ||
|
|
||
| Data = Read | Write, | ||
| Code = Read | Execute, | ||
|
|
||
| Any = 0, // TODO(mazunki): this should really be R|W|X; but requires some refactoring | ||
| RWX = Read|Write|Execute, // TODO(mazunki): temporary, remove me. references should use Permission::Any | ||
|
|
||
| // None = 0, // TODO(mazunki): implement this after Any is properly implemented (to avoid confusion with old Access::none which had a different meaning). should block all access (best used for unmapped stuff, potentially tests) | ||
| }; | ||
| } // os::mmap | ||
|
|
||
|
|
||
| namespace util { | ||
| inline namespace bitops { | ||
| template<> struct enable_bitmask_ops<os::mem::Flags> { | ||
| using type = std::underlying_type<os::mem::Flags>::type; | ||
| static constexpr bool enable = true; | ||
| }; | ||
| } | ||
|
|
||
| inline namespace bitops { | ||
| template<> | ||
| struct enable_bitmask_ops<os::mem::Permission> { | ||
| using type = typename std::underlying_type<os::mem::Permission>::type; | ||
| static constexpr bool enable = true; | ||
| }; | ||
| } | ||
| } | ||
| #endif // _SYS_MMAN_HPP |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| diff --git a/src/mman/mmap.c b/src/mman/mmap.c | ||
| index 43e5e029..43307692 100644 | ||
| --- a/src/mman/mmap.c | ||
| +++ b/src/mman/mmap.c | ||
| @@ -36,4 +36,15 @@ void *__mmap(void *start, size_t len, int prot, int flags, int fd, off_t off) | ||
| return (void *)__syscall_ret(ret); | ||
| } | ||
|
|
||
| -weak_alias(__mmap, mmap); | ||
| +void *__includeos_mmap(void *start, size_t len, int prot, int flags, int fd, off_t off) | ||
| +{ | ||
| + long ret; | ||
| + if (flags & MAP_FIXED) { | ||
| + __vm_wait(); | ||
| + } | ||
| + ret = __syscall(SYS_mmap, start, len, prot, flags, fd, off); | ||
| + | ||
| + return (void *) ret; | ||
| +} | ||
| + | ||
| +weak_alias(__includeos_mmap, mmap); |
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.
Let's go with
os::mem::Prot,os::mem::Protectoros::mem::Protectioninstead - which ever you prefer, aligning with Intel manual and mprotect terminology.Uh oh!
There was an error while loading. Please reload this page.
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 do prefer
os::mem::Protectionmyself, if we have to align ourselves toPROT_*values. I was hoping to use our own constexpr semantics with more extended features. I would like to distinguish between "allow-all" and "undefined protections". There is no way of doing this with a traditional Protection bitmask, sincePROT_NONE = 0. Keeping the same name could be confusing.That is, Protection is the opposite of Permission. Also, related question: do protection flags work the same on all/most architectures?