Decouple instructions into a separate feature flag.

[h33p]

This would implement what was proposed in #177.

There is one issue, however - array_init dependency. The crate currently does not compile with no features on. From what I could tell, the dep is only used in PageTable::new, and it very well could be dropped, by replacing PageTable::new with an identical implementation as the const variant, just kept as non-const. However, I did not implement this change, as I didn't know if there was something else at play that needed the function to be the way it is. If this change would be fine, I can go ahead and add it in this PR.

Other than that, if there are any other changes that may need to be implemented, please let me know!

Edit(phil-opp): This is a breaking change.

[h33p]

I'm also not sure why the workflows failed

[phil-opp]

Thanks a lot! Looks good to me overall, except for a few minor things (see my inline comments).

The CI failures happened because yesterday's Rust nightly was broken. I just restarted the checks and it seems like it's now working again. There is still a small CI error because one check tries to enable the stable feature, which no longer exists. To fix this, you need to update the check command in .github/workflows/build.yml.

[phil-opp]

Regarding the const-ness of PageTable::new() on stable: I think we can make the function non-const for now and drop the array-init dependency. This is going to be a breaking change anyway because of the new feature gate names, so I think that's ok for now.

[h33p]

Final nail in the coffin is the addr module, which is dependent on by the OffsetPageTable. Lots of functions are marked for only 64-bit pointer width, and that is very fair - lots of silent trunction errors can lead to the oddest of behaviours.

I am wondering, however, if it wouldn't make sense to allow to work with 64-bit addresses on 32-bit machines? At the last as_ptr step it would be possible to inject a truncation debug assertion, given this brings enough breaking changes as it stands, and if I'm not mistaken adding extra functionality wouldn't be a bad thing.

From what I can tell there are 2 options to go about the issue - either mark OffsetPageTable as a structure that depends on 64-bit pointer width, or making the addresses function on non-64-bit architectures. Of course, there can be a third solution, but I don't see it right now. Please tell me what you think about it. And how should I proceed?

[phil-opp]

I am wondering, however, if it wouldn't make sense to allow to work with 64-bit addresses on 32-bit machines? At the last as_ptr step it would be possible to inject a truncation debug assertion, given this brings enough breaking changes as it stands, and if I'm not mistaken adding extra functionality wouldn't be a bad thing.

I don't think that that panicking at runtime is a good idea. People can use the as_u64 method and a manual cast if they want this behavior, but it should not be the default.

From what I can tell there are 2 options to go about the issue - either mark OffsetPageTable as a structure that depends on 64-bit pointer width, or making the addresses function on non-64-bit architectures.

The OffsetPageTable type is already only available on x86_64 in the current implementation:

x86_64/src/structures/paging/mapper/offset_page_table.rs

Line 1 in 3ce339e

#![cfg(target_arch = "x86_64")]

So I think this is the way to go. OffsetPageTable is just a thin wrapper around MappedPageTable, which is available on all architectures. Users on non-x86_64 architectures can use MappedPageTable directly with a custom PhysToVirt implementation.

[h33p]

Alright, I added a pointer width check. The macos testing seems to have failed due to lld missing (I think we had similar issue a few days back on our CI as well), but overall the code seems to be somewhat ready from my end. If there are any additional changes needed, please let me know!

[phil-opp]

Thanks a lot, looks very good! I left two more inline comments above, maybe you could look into them? Otherwise I think this should be ready for merging.

The rust-lld failure is caused by rust-lang/rust#76698, I hope that it gets fixed soon.

[h33p]

Alright, I added in the requested PageTable change, and clarified the decision made in the build script. I hope it is all fine!

[phil-opp]

Looks good to me, thanks!

I'll probably wait until the macOS nightly is fixed until merging, to ensure that everything works there as well. I hope that this will happen in the next few days.

[phil-opp]

Published as v0.12.0.