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

Remove lease limit from qspi_read #1911

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions drv/cosmo-hf/src/hf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,13 +268,13 @@ impl idl::InOrderHostFlashImpl for ServerImpl {
&mut self,
_: &RecvMessage,
addr: u32,
dest: LenLimit<Leased<W, [u8]>, PAGE_SIZE_BYTES>,
dest: Leased<W, [u8]>,
) -> Result<(), RequestError<HfError>> {
self.drv.check_flash_mux_state()?;
self.drv
.flash_read(
self.flash_addr(addr),
&mut LeaseBufWriter::<_, 32>::from(dest.into_inner()),
&mut LeaseBufWriter::<_, 32>::from(dest),
)
.map_err(|_| RequestError::went_away())
}
Expand Down Expand Up @@ -483,7 +483,7 @@ impl idl::InOrderHostFlashImpl for FailServer {
&mut self,
_: &RecvMessage,
_offset: u32,
_dest: LenLimit<Leased<W, [u8]>, PAGE_SIZE_BYTES>,
_dest: Leased<W, [u8]>,
) -> Result<(), RequestError<HfError>> {
Err(self.err.into())
}
Expand Down
5 changes: 3 additions & 2 deletions drv/cosmo-hf/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,11 @@ impl FlashDriver {

/// Reads data from the given address into a `BufWriter`
///
/// This function will only return an error if it fails to read from a
/// This function will only return an error if it fails to write to a
Copy link
Member

Choose a reason for hiding this comment

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

whoops :P

/// provided lease; when given a slice, it is infallible.
fn flash_read(
&mut self,
offset: u32,
mut offset: u32,
dest: &mut dyn idol_runtime::BufWriter<'_>,
) -> Result<(), ()> {
loop {
Expand All @@ -269,6 +269,7 @@ impl FlashDriver {
}
}
}
offset += len as u32;
Copy link
Member

Choose a reason for hiding this comment

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

should we be doing something to avoid a panic site from the overflow check here?

}
Ok(())
}
Expand Down
17 changes: 12 additions & 5 deletions drv/gimlet-hf-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,14 +505,21 @@ impl idl::InOrderHostFlashImpl for ServerImpl {
fn read(
&mut self,
_: &RecvMessage,
addr: u32,
dest: LenLimit<Leased<W, [u8]>, PAGE_SIZE_BYTES>,
mut addr: u32,
dest: Leased<W, [u8]>,
) -> Result<(), RequestError<HfError>> {
self.check_muxed_to_sp()?;
self.qspi.read_memory(addr, &mut self.block[..dest.len()]);
let mut offset = 0;
for i in (0..dest.len()).step_by(self.block.len()) {

This comment was marked as outdated.

Copy link
Contributor

Choose a reason for hiding this comment

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

note: While looking at my folly, I noticed that it seems like step_by is stopping some compiler work from happening: https://godbolt.org/z/6Phz5WEcG

It's a shame that such a useful API drops code off a (probably minor) cliff :(

I hope this at least is interesting and potentially useful information to you.

let n = (dest.len() - i).min(self.block.len());

dest.write_range(0..dest.len(), &self.block[..dest.len()])
.map_err(|_| RequestError::Fail(ClientError::WentAway))?;
self.qspi.read_memory(addr, &mut self.block[..n]);

dest.write_range(offset..offset + n, &self.block[..n])
.map_err(|_| RequestError::Fail(ClientError::WentAway))?;
addr += n as u32;
offset += n;
}

Ok(())
}
Expand Down
13 changes: 9 additions & 4 deletions drv/mock-gimlet-hf-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,17 @@ impl idl::InOrderHostFlashImpl for ServerImpl {
&mut self,
_: &RecvMessage,
_addr: u32,
dest: LenLimit<Leased<W, [u8]>, PAGE_SIZE_BYTES>,
dest: Leased<W, [u8]>,
) -> Result<(), RequestError<HfError>> {
let zero = [0; PAGE_SIZE_BYTES];

dest.write_range(0..dest.len(), &zero[..dest.len()])
.map_err(|_| RequestError::Fail(ClientError::WentAway))?;
let mut offset = 0;
for i in (0..dest.len()).step_by(zero.len()) {
let n = (dest.len() - i).min(zero.len());

dest.write_range(offset..offset + n, &zero[..n])
.map_err(|_| RequestError::Fail(ClientError::WentAway))?;
offset += n;
}

Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion idl/hf.idol
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ Interface(
"address": "u32",
},
leases: {
"data": (type: "[u8]", write: true, max_len: Some(256)),
"data": (type: "[u8]", write: true),
},
reply: Result(
ok: "()",
Expand Down