Skip to content

Improve device path API #421

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

Merged
merged 1 commit into from
May 1, 2022

Conversation

nicholasbishop
Copy link
Member

This is an update to the device path API with two goals:

  • Clearly define the separation between device paths, device path
    instances, and device path nodes by using different types.
  • Make tests pass under Miri with -Zmiri-tag-raw-pointers enabled

In the UEFI spec, the EFI_DEVICE_PATH_PROTOCOL struct is used to
represent both full paths and individual nodes. Prior to this commit,
DevicePath was used the same way. This didn't make for the clearest
API though, since the distinction between paths and nodes mainly existed
in the documentation rather than the type system. To solve this, there
are now three main types: DevicePath, DevicePathInstance, and
DevicePathNode.

In addition, since DevicePath was only four bytes in size (i.e. the
size of the node header), Miri was unhappy when bytes were read past
those four bytes. The solution is to use DSTs that cover the appropriate
span of bytes. This required the addition of a new ProtocolPointer
trait so that when a DST protocol is opened, a fat pointer can be
constructed from the thin c_void pointer.

The device_path module's docstring has been expanded quite a bit to
describe how all the types fit together.

This commit enables -Zmiri-tag-raw-pointers, so:
Fixes #414

@GabrielMajeri
Copy link
Collaborator

I've checked the code and it looks great! A really elegant method of handling the inherent unsafety/complexity of UEFI's DevicePath.

This is an update to the device path API with two goals:
* Clearly define the separation between device paths, device path
  instances, and device path nodes by using different types.
* Make tests pass under Miri with `-Zmiri-tag-raw-pointers` enabled

In the UEFI spec, the `EFI_DEVICE_PATH_PROTOCOL` struct is used to
represent both full paths and individual nodes. Prior to this commit,
`DevicePath` was used the same way. This didn't make for the clearest
API though, since the distinction between paths and nodes mainly existed
in the documentation rather than the type system. To solve this, there
are now three main types: `DevicePath`, `DevicePathInstance`, and
`DevicePathNode`.

In addition, since `DevicePath` was only four bytes in size (i.e. the
size of the node header), Miri was unhappy when bytes were read past
those four bytes. The solution is to use DSTs that cover the appropriate
span of bytes. This required the addition of a new `ProtocolPointer`
trait so that when a DST protocol is opened, a fat pointer can be
constructed from the thin `c_void` pointer.

The `device_path` module's docstring has been expanded quite a bit to
describe how all the types fit together.

This commit enables `-Zmiri-tag-raw-pointers`, so:
Fixes rust-osdev#414
@GabrielMajeri GabrielMajeri merged commit 8ba6b19 into rust-osdev:main May 1, 2022
@nicholasbishop nicholasbishop deleted the bishop-unsized-dp branch May 1, 2022 21:30
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.

Enable Miri's -Zmiri-tag-raw-pointers feature
2 participants