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

Add support for the Nintendo 3DS (armv6k-nintendo-3ds) #248

Merged
merged 4 commits into from
Mar 25, 2022

Conversation

AzureMarker
Copy link
Contributor

@AzureMarker AzureMarker commented Feb 21, 2022

Overview

The Nintendo 3DS is a mostly-UNIX looking system, with the caveat that it does not come with a full libc implementation out of the box. On the homebrew side (I'm not under NDA), the libc interface is (partially) implemented by a user library like libctru. This is important because getrandom (conforming to Linux) is one of the functions that requires some work to make UNIX compatible.

Because of this, the implementation of getrandom needs to be supplied by a dependency. libctru doesn't supply a libc (linux) compatible getrandom but it does supply a similar function based on the underlying syscall. Our implementation of getrandom which uses this libctru function is here: https://github.com/Meziu/rust-linker-fix-3ds/blob/080f7af4f2193892cca69da271e7dfad2aaba4e4/src/lib.rs#L94.

This PR utilizes the libc implementation of getrandom (conforming to Linux) by linking to it via the libc crate.

Note: the 3DS's PS service needs to be initialized before calling this function, otherwise it returns an error. It is expected that the user initializes this at the start of their process.

Implementation choice

Technically we could fit the whole implementation in this getrandom crate since under the hood it's just a syscall. However, I didn't go down that path for a few reasons:

  1. The underlying implementation (when linked against rust-linker-fix-3ds) uses libctru to perform the syscall. The call itself is a little complex and requires a lot of knowledge about the platform (ex. needs to decode 3DS kernel error codes), so I didn't want to repeat that here.
  2. Taking the above into account, we probably can't simply link in libctru because of some concerns best summarized here: ARMv6K Nintendo 3DS Tier 3 target added rust-lang/rust#88529 (comment). I think this concern also prevents us from writing the syscall wrapper manually, since it would be including details about the 3DS's kernel interface in a somewhat-official/blessed crate.

I may be wrong on point 2 (since this isn't an official rust-lang crate), in which case we may be "allowed" to reimplement the syscall wrapper in this crate directly. But reusing the libctru implementation is pretty easy, and allows for alternative (homebrew or potential NDA-ed libc library) implementations by way of linking to libc::getrandom instead of libctru directly.

Building

The armv6k-nintendo-3ds tier 3 target is supported by rustc as of 1.57: rust-lang/rust#88529. However, it is currently core and alloc only. std support requires a patched rustc: https://github.com/Meziu/rust-horizon (we plan on upstreaming this soon). The target also requires an external toolchain from devkitPro to compile: https://devkitpro.org/wiki/devkitPro_pacman. This dissuaded me from adding it to the build-std tests in test.yaml, but let me know if this test should be added.

Draft notice

Update: The libc changes merged so this PR is now published.

This PR currently relies on libc::getrandom which isn't currently exported for the target (needs a patched libc). The PR will be in draft until we upstream that change and it gets released.

Question: Once the new libc version is released, will this will require changing the minimum version of libc in Cargo.toml?

@josephlr
Copy link
Member

As a heads up, this create generally doesn't utilize the libc shims unless we have to. Generally we use the random methods of the underlying OS directly (i.e. a syscall or low-level library call).

For this case, it seems like calling PS_GenerateRandomBytes would normally be the correct approach. However I'm not exactly familiar with any legal issues around supporting the 3DS. Where is the underlying syscall documented?

@josephlr
Copy link
Member

josephlr commented Feb 21, 2022

If we end up deciding to use libc, I don't think that updating to a newer version would be an issue

@josephlr
Copy link
Member

Okay, after reading all of your links, I think I understand the situation better (thank you by the way for providing all that context). Given that this project is still under development,

we should just call libc::getrandom

The API to generate randomness on 3DS seems:

  • complex, given initialization concerns
  • undocumented
  • maybe in a weird legal situation (though I don't think that would affect our implementation choice)

It seems best that such logic live in a libc-type library (so all languages can use it) rather than having a bespoke implementation live here. It should also make linking easier, as people know they have to link against libc.

Finally, if you should use the same approach here as you would on your port of libstd. This library will hopefully become a dependency of libstd, so we should definitely be doing things "in the same way". For example, if Rust's stdlib starts linking directly to libctru instead of libc, we should do the same.

bors added a commit to rust-lang/libc that referenced this pull request Mar 12, 2022
Horizon (Nintendo 3DS) pthread functions and non-portable extensions

This PR adds some standard and nonstandard pthread/threading functions to the horizon (Nintendo 3DS) operating system. This will allow us to open a PR to std for std threads support.

The Nintendo 3DS doesn't have a full libc implementation, so there are some user libraries which implement part of libc, such as libctru:
https://github.com/devkitPro/libctru
For some more context on this situation, see rust-random/getrandom#248

But std doesn't want to interface directly with anything that could be seen as using the "internal" interfaces of the 3DS or consoles in general:
rust-lang/rust#88529 (comment)

So we work around this by implementing standard pthread interfaces, plus some nonstandard ones as necessary, and expose that interface to std:
https://github.com/Meziu/pthread-3ds

Here's the justifications for the nonstandard interfaces:
* `pthread_attr_setprocessorid_np` (and the `get` version):
  The 3DS requires the core a thread runs on to be specified at thread creation time. I'm pretty sure you can't migrate threads across cores. Additionally, the cores act differently (ex. one core is cooperatively scheduled). This means we need to have an API to set the core to use in the thread attributes struct. We didn't find any relevant standard API for this, so we added a nonstandard one.
* `pthread_getprocessorid_np`:
  The 3DS lets you get the core/processor ID of the executing thread. But it doesn't have a function to get the core ID of any arbitrary thread. We didn't find any standard APIs which would match these semantics, so we added a nonportable one. Once place this API is used is to get the default core/processor ID when spawning a thread (spawn the thread on the current processor).

For more context, see:
* Meziu/rust-horizon#10
  * Especially Meziu/rust-horizon#10 (comment)
* rust-random/getrandom#248

cc: `@ian-h-chamberlain` `@Meziu`
bors added a commit to rust-lang/libc that referenced this pull request Mar 14, 2022
Horizon (Nintendo 3DS) getrandom function and fixes

This PR adds `getrandom`, conforming to the [Linux spec](https://man7.org/linux/man-pages/man2/getrandom.2.html), to the `horizon` OS (Nintendo 3DS).

The 3DS doesn't have a full libc implementation, and its randomness API is pretty complicated: rust-random/getrandom#248. For this reason (see the linked PR for more details), the randomness implementation is abstracted by using the Linux `getrandom` interface.

This PR also fixes some types on the horizon platform. See the commits and diff.

cc: `@ian-h-chamberlain` `@Meziu`
@ian-h-chamberlain
Copy link

@AzureMarker now that the libc PR has landed and published to crates.io, is this PR unblocked? Maybe it would need to bump the minimum required version of libc as well before merging?

@AzureMarker
Copy link
Contributor Author

Yeah I'll take a look at this soon and publish the PR when I've revalidated it.

Needed for armv6k-nintendo-3ds support.
@AzureMarker AzureMarker marked this pull request as ready for review March 15, 2022 02:40
src/util_libc.rs Outdated Show resolved Hide resolved
@@ -33,6 +33,7 @@
//! | Web Browser | `wasm32‑*‑unknown` | [`Crypto.getRandomValues()`][14], see [WebAssembly support][16]
//! | Node.js | `wasm32‑*‑unknown` | [`crypto.randomBytes`][15], see [WebAssembly support][16]
//! | SOLID | `*-kmc-solid_*` | `SOLID_RNG_SampleRandomBytes`
//! | Nintendo 3DS | `armv6k-nintendo-3ds` | [`getrandom`][1]
Copy link
Member

Choose a reason for hiding this comment

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

Hm, so the target triplet uses nintendo, but target_os is set to horizon for it?

Choose a reason for hiding this comment

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

Correct, similar to how target_os = "macos" for -apple-darwin targets, there seems to not always be a 1-to-1 match between target_os and the target triple...

As an aside, I believe there is separate work being done to add the Nintendo Switch as a target, which would also have target_os = "horizon" and most likely use this API as well, but I haven't followed that development too closely so I'm not sure about its status.

@josephlr
Copy link
Member

This looks reasonable, could we add a build test for this target to the CI?

@AzureMarker
Copy link
Contributor Author

Can this be merged? I think the CI failure is unrelated.

@newpavlov newpavlov merged commit d40ec2c into rust-random:master Mar 25, 2022
@AzureMarker AzureMarker deleted the feature/3ds branch March 25, 2022 23:21
henrysun007 pushed a commit to mesatee/getrandom that referenced this pull request Nov 24, 2022
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.

4 participants