Add block/disk IO functionality #18
Conversation
This is immediately useful for checking partition types/GUIDs for block and disk I/O, but is also useful elsewhere.
Some function calls require image handles in place of the LoadedImageProtocol struct, so track it for library consumers that may want to use it.
src/protocol/block.rs
Outdated
|
|
||
| // The read buffer must be aligned to the value of `media.io_align`. UEFI doesn't provide | ||
| // any sort of memalign, so in order to be safe, if we need to be aligned on any boundary | ||
| // greater than 2, use `allocate_pages` to obtain a 4K-aligned address instead of |
src/protocol/block.rs
Outdated
| ) { | ||
| Status::Success => Ok(slice::from_raw_parts_mut(buffer, num_bytes)), | ||
| e => { | ||
| bs.free_pool(buffer); |
There was a problem hiding this comment.
This will presumably be unhappy if the buffer was allocated with allocate_pages.
src/protocol/block.rs
Outdated
| num_pages += 1; | ||
| } | ||
|
|
||
| buffer = bs.allocate_pages(num_pages).map(|buf| buf as *mut u8); |
There was a problem hiding this comment.
The comment block says the caller needs to free this with allocate_pool, which could end badly.
csssuf
force-pushed
the
disk-io
branch
2 times, most recently
from
dc994ca to
50e2139
Compare
src/protocol/block.rs
Outdated
|
|
||
| /// Read `num_bytes` bytes from the disk starting at block `start`. The returned slice includes | ||
| /// memory allocated with `allocate_pool` or `allocate_pages`, and it is the caller's | ||
| /// responsibility to free it with the appropriate function based on `must_align()`. |
There was a problem hiding this comment.
This does not make a great API. Is allocate_pool an important optimization? It seems like it'd be cleaner to always use allocate_pages.
There was a problem hiding this comment.
I agree. I would prefer to use allocate_pool wherever possible, simply because free_pages requires the caller to specify how many pages to free, which also isn't a great API. Mixing them definitely isn't ideal though. Realistically it shouldn't be the responsibility of the library consumer to manage memory allocated with allocate_pages, but until the new allocator APIs are more stable there's not a simple solution.
There are two ways to deal with it here in the short term:
- Write a simple allocator for
rust-uefito use internally that doesn't attempt to also be the language's allocator; or - For the block I/O protocol, always allocate memory with
allocate_pagesand just force library consumers to deal with the unpleasant deallocation.
Which would be preferable?
src/protocol/block.rs
Outdated
| } | ||
|
|
||
| /// Read `num_blocks` blocks from the disk starting at block `start`. The returned slice | ||
| /// includes memory allocated with `allocate_pool`, and it is the caller's responsibility to |
src/protocol/block.rs
Outdated
|
|
||
| /// Read `num_bytes` bytes from the disk starting at block `start`. The returned slice includes | ||
| /// memory allocated with `allocate_pages`, and it is the caller's responsibility to free it | ||
| /// with `free_pages` based on `required_pages` or `required_pages_block`. |
src/protocol/block.rs
Outdated
| if self.must_align() { | ||
| bs.free_pages(buffer, self.required_pages(read_size)); | ||
| } else { | ||
| bs.free_pool(buffer); |
The commits in this PR that aren't directly used by 281cef1 add functionality that is basically necessary for making effective use of the block/disk IO protocols.