Skip to content

abi/kern/userlib: Use usize-like data for addr+len - #2703

Open
jamesmunns wants to merge 7 commits into
masterfrom
james/abi-addr
Open

jamesmunns wants to merge 7 commits into
masterfrom
james/abi-addr

Conversation

@jamesmunns

Copy link
Copy Markdown
Contributor

This is the first PR in a series to be able to potentially run the kernel and tasks on a 64-bit host for testing purposes.

This PR switches some things in abi/kern/userlib to use usize instead of u32. On 32-bit targets, this is not a functional change, and in the first commit (before changing the types), I added some static asserts, which continue passing after the first change.

@jamesmunns

Copy link
Copy Markdown
Contributor Author

For reviewers, I've added some "big picture" notes in #2706 on how this PR fits into the bigger picture.

Comment thread sys/abi/src/lib.rs
Comment on lines +217 to +220
// This is icky! However, kipc uses `ssmarshal` for serialization, which
// makes the serialization of addresses a kipc binary concern, and we
// don't want to use `usize` as our wire format as serde treats that
// as a u64 in all cases, and we've always serialized addresses as u32s.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

SIGH

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We could always switch to hubpack! Last time I tried, there was a noticeable size regression, but that was literally years ago.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think if I made a jump, I'd aim for postcard or nprpc, but I would probably want to do that after we have something like #2706 to be able to smoke test it!

Comment thread sys/kern/src/arch/arm_m.rs
Comment thread sys/kern/src/arch/arm_m.rs
Comment thread sys/kern/src/descs.rs
/// requirements for this; it must meet them. (For example, on ARMv7-M, it
/// must be naturally aligned for the size.)
pub base: u32,
pub base: usize,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should base be an addr?

Comment thread sys/kern/src/descs.rs
//
// It's possible in the future we want `RegionDesc` to be arch-specific, but for
// now ensure that all 32-bit targets share the same ABI qualities.
#[cfg(target_pointer_width = "32")]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i wonder if perhaps all of this ought to just be in the arch module?

This branch has not been deployed

No deployments
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.

3 participants