Skip to content

uefi: add basic integration test for BlockIo and BlockIo2 protocols + fix in BlockIo2 #1842

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.

Sign up for GitHub

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

Open
phip1611 wants to merge 2 commits into
base: main
Choose a base branch
from
+166 −11
Open

uefi: add basic integration test for BlockIo and BlockIo2 protocols + fix in BlockIo2 #1842

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
593f673
uefi: BlockIo2: fix unidiomatic code
phip1611 Dec 6, 2025
295ec8d
uefi-test-runner: add basic test for blockio protocols
phip1611 Dec 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions uefi/src/proto/media/block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@ impl BlockIO2 {
/// * `media_id` - The media ID that the read request is for.
/// * `lba` - The starting logical block address to read from on the device.
/// * `token` - Transaction token for asynchronous read.
/// * `len` - Buffer size.
/// * `buffer` - The target buffer of the read operation
///
/// # Safety
Expand All @@ -256,12 +255,20 @@ impl BlockIO2 {
media_id: u32,
lba: Lba,
token: Option<NonNull<BlockIO2Token>>,
len: usize,
Copy link
Member Author

@phip1611 phip1611 Dec 6, 2025

Choose a reason for hiding this comment

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

ping @Virv12 FYI. Unfortunately, I missed this in the review.

Copy link
Contributor

@Virv12 Virv12 Dec 7, 2025

Choose a reason for hiding this comment

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

I don't like this change, using a reference introduces a lifetime, implying we are going to use the buffer only until the lifetime is alive, which is wrong.

There has been a similar discussion on DiskIO2.

Copy link
Member Author

@phip1611 phip1611 Dec 7, 2025

Choose a reason for hiding this comment

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

Thanks a lot for pointing that out to me, I wasn't aware of this anymore!

buffer: *mut u8,
buffer: &mut [u8],
) -> Result {
let token = opt_nonnull_to_ptr(token);
unsafe { (self.0.read_blocks_ex)(&self.0, media_id, lba, token.cast(), len, buffer.cast()) }
.to_result()
unsafe {
(self.0.read_blocks_ex)(
&self.0,
media_id,
lba,
token.cast(),
buffer.len(),
buffer.as_mut_ptr().cast(),
)
}
.to_result()
}

/// Writes a specified number of blocks to the device.
Expand All @@ -270,7 +277,6 @@ impl BlockIO2 {
/// * `media_id` - The media ID that the write request is for.
/// * `lba` - The starting logical block address to be written.
/// * `token` - Transaction token for asynchronous write.
/// * `len` - Buffer size.
/// * `buffer` - Buffer to be written from.
///
/// # Safety
Expand All @@ -290,12 +296,18 @@ impl BlockIO2 {
media_id: u32,
lba: Lba,
token: Option<NonNull<BlockIO2Token>>,
len: usize,
buffer: *const u8,
buffer: &[u8],
) -> Result {
let token = opt_nonnull_to_ptr(token);
unsafe {
(self.0.write_blocks_ex)(&mut self.0, media_id, lba, token.cast(), len, buffer.cast())
(self.0.write_blocks_ex)(
&mut self.0,
media_id,
lba,
token.cast(),
buffer.len(),
buffer.as_ptr().cast(),
)
}
.to_result()
}
Expand Down