Implement function for creating a gdt in a const environment. #413
Conversation
src/structures/gdt.rs
Outdated
| /// Returns the length of the descriptor table | ||
| pub const fn len(&self) -> usize { | ||
| self.len | ||
| } |
There was a problem hiding this comment.
Length can have a couple of meanings here:
- The amount of entries.
- The amount of bytes
- The amount of bytes minus one (this representation is used when the descriptor table is loaded)
Can you clarify in the comment which of those is used here?
There was a problem hiding this comment.
The goal of this function is to be able to create a GdtPointer that can be used with the lgdt instruction. Currently, I am using (GDT_TABLE.len() * 8 - 1) as u16 for the size parameter of the gdtpointer, where GDT_TABLE is an instance of GlobalDescriptorTable. Maybe more beneficial to introduce a GdtPointer type to the library and a const fn to create one?
There was a problem hiding this comment.
The implementation for the GdtPointer is slightly different than the one already in this crate. I'm not sure what makes the most sense to do here. I am generating the gdt stuff with rust at compile-time, but actually loading it in some assembly bootstrap code.
/// A struct for creating a global descriptor table pointer, suitable for loading with lidtr
#[repr(C, packed)]
pub struct GdtPointer<'a> {
/// The size of the gdt table in bytes minus 1. See x86 processor manual for more information.
size: u16,
/// The address of the global descriptor table.
address: &'a GlobalDescriptorTable,
}
src/structures/gdt.rs
Outdated
| /// Descriptor::kernel_data_segment() | ||
| /// ]); | ||
| /// ``` | ||
| pub const fn from_descriptors<const N: usize>( |
There was a problem hiding this comment.
This function will panic if the capacity is exceeded. This should be mentioned in the doc comment.
src/structures/gdt.rs
Outdated
| self.len | ||
| } | ||
|
|
||
| ///Creates a filled out GDT. |
There was a problem hiding this comment.
| ///Creates a filled out GDT. | |
| /// Creates a GDT with the provided descriptors. |
src/structures/gdt.rs
Outdated
| loop { | ||
| if index < N { |
There was a problem hiding this comment.
This can be expressed using a while loop.
src/structures/gdt.rs
Outdated
| if index < N { | ||
| match descriptors[index] { | ||
| Descriptor::UserSegment(value) => { | ||
| assert!(outindex <= data.len().saturating_sub(1)); |
There was a problem hiding this comment.
Are this assert and the one below needed? Shouldn't out of bounds accesses also be caught by the index operations in line 88, 95 and 99?
Freax13
added
the
waiting-on-author
|
For some reason, I am just now seeing your comments on the pull request. I will look into it in the next week or so. |
|
@Freax13 I think the function I added with this pull request is a bit redundant now that I see the const_fn feature in the crate, however I do need a way to generate a GdtTablePointer with a const fn. |
Yeah, you're right. I'm sorry about that, we had some breaking changes in the
That won't be easy. Our DescriptorTablePointer type uses a Just out of curiosity, what's your use case for creating the pointer at compile time? |
|
No worries on the changes. |
|
Any thoughts on what the name of this function should be? |
|
|
@uglyoldbob @tepperson2 something seems to have gotten messed up on this PR, it's showing >1000 lines changed. Could you fix this PR to only include the last two commits? |
|
I wasn't quite sure the best way to fix it, so i redid the last commit and force pushed it without the merge. |
Can you rebase onto the latest master so that there aren't conflicts? |
…de the crate, use that function in pointer.
|
It should be ready. I combined it into a single commit. |
As mentioned in another pull request, the form for this function was preferred over the other implementation.
#406