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

gdt: make add_entry const #191

Merged
merged 1 commit into from Oct 9, 2020
Merged

gdt: make add_entry const #191

merged 1 commit into from Oct 9, 2020

Conversation

josephlr
Copy link
Contributor

@josephlr josephlr commented Oct 9, 2020

This requires making the push method const as well, which means we need
the const_panic feature (not too much of a risk as this feature is
failly well along).

I also moved the pointer calculation (for GDT and IDT) into their own
(currently private) methods. Eventually these methods can be public, but
that's only useful if they can be const, which would require a breaking change.

The caclulation for the GDT pointer was also updated to use:

self.next_free * size_of::<u64>() - 1

instead of

self.table.len() * size_of::<u64>() - 1

Signed-off-by: Joe Richey joerichey@google.com

This requires making the push method const as well, which means we need
the `const_panic` feature (not too much of a risk as this feature is
failly well along).

I also moved the pointer calculation (for GDT and IDT) into their own
(currently private) methods. Eventually these methods can be public, but
that's only useful if they can be const, which would require a breaking change.

The caclulation for the GDT pointer was also updated to use:
```rust
self.next_free * size_of::<u64>() - 1
```
instead of
```rust
self.table.len() * size_of::<u64>() - 1
```

Signed-off-by: Joe Richey <joerichey@google.com>
@josephlr josephlr requested a review from phil-opp October 9, 2020 18:53
Copy link
Member

@phil-opp phil-opp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks all reasonable to me, thanks!

@phil-opp
Copy link
Member

phil-opp commented Oct 9, 2020

Eventually these methods can be public, but
that's only useful if they can be const, which would require a breaking change.

Could you clarify which change would be breaking?

@josephlr
Copy link
Contributor Author

josephlr commented Oct 9, 2020

Could you clarify which change would be breaking?

To make GlobalDescriptorTable::pointer() a const fn, DescriptorTablePointer.base would need to have a pointer type instead of a u64 type, and that field is public.

My current idea for a later PR:

  • Change DescriptorTablePointer.base to type VirtAddr
  • Make it so VirtAddr::from_ptr could be const fn (have some new from_ptr_unchecked method).

@josephlr josephlr merged commit 710cfa2 into rust-osdev:master Oct 9, 2020
@josephlr josephlr deleted the gdt-push branch October 9, 2020 21:17
@phil-opp
Copy link
Member

Sounds good to me! However, I think we can make the VirtAddr::new check a const fn too: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=87076d1f107a5a36970175cbb1bc9fa9

@phil-opp
Copy link
Member

Ideally, I would want to make the bit_field crate functions const at some point, but right now not all of its features are const compatible (e.g. being generic over RangeBounds).

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.

None yet

2 participants