Update things so that Luma builds again on recent nightly #26
Conversation
Current nightly rejects the one we had until now due to the lack of -Fn32, add it so it is happy again. Here is the error we get without this commit: ``` error: data-layout for target `powerpc-unknown-eabi-2922781650972509521`, `E-m:e-p:32:32-i64:64-n32`, differs from LLVM target's `powerpc-unknown-eabi` default layout, `E-m:e-p:32:32-Fn32-i64:64-n32` ```
There was a problem hiding this comment.
Move without_provenance_mut to with_provenance_mut MMIO is always considered to be exposed from what I remember https://discord.com/channels/273534239310479360/592856094527848449/1317772050344972318
luma_core/src/ipc.rs
Outdated
|
|
||
| //bit 0 = X1 | Execute IPC request | ||
| //bit 1 = Y2 | Acknowledge IPC request | ||
| //bit 2 = Y1 | IPC request reply available | ||
| //bit 3 = X2 | Relaunch IPC | ||
| //bit 4 = IY1 | IPC request reply send out IPC interrupt | ||
| //bit 5 = IY2 | IPC request acknowledge sends out IPC interrupt | ||
| const HW_IPC_PPCCTRL: usize = 0xCD00_0004usize; //from_exposed_addr_mut(0xCD00_0004); | ||
| const HW_IPC_PPCCTRL: *mut u32 = ptr::without_provenance_mut(0xCD00_0004); |
There was a problem hiding this comment.
we want with_provenance_mut here instead of without_provenance_mut otherwise writing to value is UB
There was a problem hiding this comment.
with_provenance_mut() is also UB, since we haven’t converted any pointer to an address right before, because we don’t have any pointer to convert. In addition, it isn’t const so we couldn’t use it here.
Oh and that link seems to require an account, could you copy/paste the relevant discussion items to some usable public website please?
There was a problem hiding this comment.
with_provenance_mut() is also UB, since we haven’t converted any pointer to an address right before, because we don’t have any pointer to convert.
That's not correct; see the docs:
"In addition, memory which is outside the control of the Rust abstract machine (MMIO registers, for example) is always considered to be accessible with an exposed provenance, so long as this memory is disjoint from memory that will be used by the abstract machine such as the stack, heap, and statics."
So on first sight, what you are doing here seems entirely correct to me. I might be missing some subtle detail though.
There was a problem hiding this comment.
Thanks for the review. :)
luma_core/src/ipc.rs
Outdated
|
|
||
| //bits 0..=31 = physical address of ipc request | ||
| const HW_IPC_PPCMSG: usize = 0xCD00_0000usize; //from_exposed_addr_mut(0xCD00_0000); | ||
| const HW_IPC_PPCMSG: *mut u32 = ptr::without_provenance_mut(0xCD00_0000); |
There was a problem hiding this comment.
we want with_provenance_mut here instead of without_provenance_mut otherwise writing to value is UB
luma_core/src/ipc.rs
Outdated
|
|
||
| //bit 0 = Y1 | IPC request reply available | ||
| //bit 1 = X2 | Relauch IPC | ||
| //bit 2 = X1 | Execute IPC request | ||
| //bit 3 = Y2 | Acknowledge IPC request | ||
| //bit 4 = IX1 | Execute ipc request send IPC interrupt | ||
| //bit 5 = IX2 | Relaunch IPC sends IPC interrupt | ||
| const HW_IPC_ARMCTRL: usize = 0xCD00_000Cusize; //from_exposed_addr_mut(0xCD00_000C); | ||
| const HW_IPC_ARMCTRL: *mut u32 = ptr::without_provenance_mut(0xCD00_000C); |
There was a problem hiding this comment.
we want with_provenance_mut here instead of without_provenance_mut otherwise writing to value is UB
Now that the `exposed_provenance` and `strict_provenance` features have been stabilized, but `with_exposed_provenance_mut()` isn’t const, and `without_provenance_mut()` is UB to dereference. `core::ptr`’s documentation says: > Situations where a valid pointer must be created from just an address, > such as baremetal code accessing a memory-mapped interface at a fixed > address, are an open question on how to support. These situations will > still be allowed, but we might require some kind of “I know what I’m > doing” annotation to explain the situation to the compiler. It’s also > possible they need no special attention at all, because they’re > generally accessing memory outside the scope of “the abstract machine”, > or already using “I know what I’m doing” annotations like “volatile”. See rust-lang/rust#98593 for more information about future development around converting MMIO addresses to pointers.
c107b6a to
56fc048
Compare
|
The documentation of |
The first commit is for a simple LLVM change, the data layout is more descriptive now.
The second one is about an issue we’ve had since basically forever, but which is becoming worse with provenance support being added to the language. Basically, it’s always been UB to cast a random integer into a pointer, but that works on von Neumann machines like ours. The
exposed_provenanceandstrict_provenancefeatures have been stabilized, so let’s migrate to their functions at least, even though it’s still UB to use them in our case, see the commit message for why. rust-lang/rust#98593 might end up giving us a function we can use for that usecase, but until then let’s keep one UB which currently works.