cpio miniroot support #7
Conversation
dancrossnyc
force-pushed
the
rd
branch
4 times, most recently
from
962e2a9 to
178bf0a
Compare
dancrossnyc
force-pushed
the
spinner
branch
2 times, most recently
from
642377c to
5f0476c
Compare
Support mounting a cpio archive as a ramdisk, in lieu of UFS. The usual file commands work here, albeit without symlink support. Regardless, this is good enough to load a kernel and get it running, using the same commands and syntax as for UFS. Signed-off-by: Dan Cross <[email protected]>
iximeow
left a comment
iximeow
left a comment
There was a problem hiding this comment.
i realize you've merged this already but i only just got to taking a look :) glad this worked out! nothing i've commented on is super critical.
| impl Read for &[u8] { | ||
| fn read(&self, off: u64, dst: &mut [u8]) -> Result<usize> { | ||
| let off = off as usize; | ||
| if off > self.len() { |
There was a problem hiding this comment.
... i did not know that &slice[slice.len()..] would work, but apparently it does and produces an empty slice!
There was a problem hiding this comment.
Indeed! It occurs to me that, regardless of correctness, it's clearer all around to just use >= here.
| let entry = loader::load(&mut config.page_table, kernel.as_ref())?; | ||
| let entry = entry.try_into().unwrap(); | ||
| Ok(Value::Pointer(fs.data().with_addr(entry).cast_mut())) | ||
| Ok(Value::Pointer(fs.as_ref().with_addr(entry).cast_mut())) |
There was a problem hiding this comment.
if loader::load called page_table.try_with_addr(elf.entry) and returned that, you wouldn't need FileSystem::with_addr at all, right? and it would be more accurate anyway, the kernel entrypoint has no relationship to the filesystem at this point - we could unmap the filesystem the kernel was loaded from at this point theoretically?
There was a problem hiding this comment.
Hmm, yes.
However, the page tables don't have any special relationship to the memory that address is tied to, either. I chose that to give me the provenance of the segments, but I don't know if that's a good idea, honestly. Those addresses are constructed from whole-cloth, taken from the program headers in the ELF file, and don't really correspond to the data that go into the page tables themselves, either. I think at the time my rationale was that the page tables define the address space, but I'm not sure that's a good argument for that particular provenance.
As for the entry address... An invalid ELF binary could, for example, include a bogus address that isn't mapped at all. We could try and detect that at the point where we go to call it, but that's rather farther removed from here. Regardless, the point that it doesn't really correspond to the provenance of the filesystem anymore is a good one. So what provenance makes sense? For the entry point, I would argument that it would be the segment that the entry point lies within, if any. I've made a change here that, I think, implements that. Let me know what you think?
| let frag_off = off % fragsize; | ||
| let m = cmp::min(n - nread, fragsize - frag_off); | ||
| match self.bmap(off as u64)? { | ||
| Block::Hole => { | ||
| buf[nread..nread + m].fill(0); | ||
| } | ||
| Block::Sd(bs) => { | ||
| buf[nread..nread + m] | ||
| .copy_from_slice(&bs[frag_off..frag_off + m]); | ||
| } | ||
| } | ||
| let block = self.bmap(off as u64)?; | ||
| block.read(frag_off, &mut buf[nread..nread + m]); |
There was a problem hiding this comment.
this and Block::read are in conflict over who is responsible for sizing the read buffer appropriately. if Block::Hole was Block::Hole(fragsize) we could hold firm that Block::read can take a buffer of any size, and write up to fragsize-frag_off bytes into that buffer.
then we could pass only &mut buf[nread..], return the number of bytes read from Block::read, and use that to bump off and nread instead of ever calculating m here. what do you think?
There was a problem hiding this comment.
I don't know that that's the case. The file size is in bytes, and may not be a multiple of the fragment size. The fragment size is fixed when the filesystem is created (it's a data in the superblock), but the file size is just whatever the file size is.
There was a problem hiding this comment.
Ok, we talked about this offline (or online?) and I'm convinced. PTAL.
Treat cpio archives as ramdisks, if provided.
lsis a bit hacky here (there are no directories per se) but it basically works, at least well enough to load a kernel and jump into unix.