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 ARM v6 syscalls and Raspberry Pi Pico / Nano Rp2040 Connect #381

Merged
merged 10 commits into from
Feb 23, 2022

Conversation

alexandruradovici
Copy link
Contributor

This PR adds support for ARM v6 and the layouts for Raspberry Pi Pico and Arduino Nano RP2040 Connect.

Due to Thumb-1, asm! cannot use registers higher than r7, which is a problem for yield1 and yield2.

I used clobber_abi("C") for Thumb-1, but I'm not sure it makes sens. It seems to me that there is no difference in the disassembly (with and without the clobber), at least in the examples that I used.

@jrvanwhy
Copy link
Collaborator

Due to Thumb-1, asm! cannot use registers higher than r7, which is a problem for yield1 and yield2.

I used clobber_abi("C") for Thumb-1, but I'm not sure it makes sens. It seems to me that there is no difference in the disassembly (with and without the clobber), at least in the examples that I used.

clobber_abi("C") is completely correct, and I think it should allow you to avoid the #[cfg()] directives entirely. The reason I didn't use clobber_abi is that clobber_abi didn't exist yet when I wrote this code.

RawSyscalls' comments for yield1 and yield2 should be updated as well.

@hudson-ayers
Copy link
Contributor

For reference, I posted rust-lang/rust#90796 (comment) because it seems off to me that asm! does not allow clobbering r8+ on cortex-m0

@alexandruradovici
Copy link
Contributor Author

clobber_abi("C") is completely correct, and I think it should allow you to avoid the #[cfg()] directives entirely. The reason I didn't use clobber_abi is that clobber_abi didn't exist yet when I wrote this code.

RawSyscalls' comments for yield1 and yield2 should be updated as well.

I think I modified the code according to your suggestions, but I am not sure.

@alexandruradovici
Copy link
Contributor Author

It seems that running an application on Cortex-M0+ faults after the first memop syscall. I modified some asm startup code just to call some other system calls and the kernel seems to lock up.

This is the fault the I get using the original asm code. It seems that registers r4-r7 are reset to 0 after the system call.

panicked at 'Process low_level_debug had a fault', kernel/src/process_standard.rs:332:17
	Kernel version release-2.0-453-g07c764031

---| Debug buffer not empty. Flushing. May repeat some of last message(s):
Initialization complete. Enter main loop
[0] function_call @0x10020069(0x10020048, 0x20004000, 0x1000, 0x20004020)
[0] memop(0, 0x20004100) = Success

---| No debug queue found. You can set it with the DebugQueue component.

---| Cortex-M Fault Status |---
No Cortex-M faults detected.

---| App Status |---
𝐀𝐩𝐩: low_level_debug   -   [Faulted]
 Events Queued: 0   Syscall Count: 1   Dropped Upcall Count: 0
 Restart Count: 0
 Last Syscall: Memop { operand: 0, arg0: 536887552 }
 Completion Code: None


 ╔═══════════╤══════════════════════════════════════════╗
 ║  Address  │ Region Name    Used | Allocated (bytes)  ║
 ╚0x20005000═╪══════════════════════════════════════════╝
             │ Grant Ptrs       64
             │ Upcalls         360
             │ Process         880
  0x20004AE8 ┼───────────────────────────────────────────
             │ ▼ Grant           0
  0x20004AE8 ┼───────────────────────────────────────────
             │ Unused
  0x20004100 ┼───────────────────────────────────────────
             │ ▲ Heap            ? |      ?               S
  ?????????? ┼─────────────────────────────────────────── R
             │ Data              ? |      ?               A
  ?????????? ┼─────────────────────────────────────────── M
             │ ▼ Stack           ? |      ?
  0x20004000 ┼───────────────────────────────────────────
             │ Unused
  0x20004000 ┴───────────────────────────────────────────
             .....
  0x10020200 ┬─────────────────────────────────────────── F
             │ App Flash       440                        L
  0x10020048 ┼─────────────────────────────────────────── A
             │ Protected        72                        S
  0x10020000 ┴─────────────────────────────────────────── H

  R0 : 0x00000000    R6 : 0x00000000
  R1 : 0x20004100    R7 : 0x00000000
  R2 : 0x00001000    R8 : 0x1002006C
  R3 : 0x20004020    R10: 0x00000000
  R4 : 0x00000000    R11: 0x00000000
  R5 : 0x00000000    R12: 0x1C85860D
  R9 : 0x10020048 (Static Base Register)
  SP : 0x200040E0 (Process Stack Pointer)
  LR : 0x00000001
  PC : 0x1002008C
 YPC : 0x10020086

 APSR: N 0 Z 1 C 1 V 0 Q 0
       GE 0 0 0 0
 EPSR: ICI.IT 0x00
       ThumbBit true 

 Total number of grant regions defined: 8
  Grant  0 : --          Grant  3 : --          Grant  6 : --        
  Grant  1 : --          Grant  4 : --          Grant  7 : --        
  Grant  2 : --          Grant  5 : --        

 Cortex-M MPU
  Region 0: [0x20004000:0x20005000], length: 4096 bytes; ReadWrite (0x3)
    Sub-region 0: [0x20004000:0x20004200], Enabled
    Sub-region 1: [0x20004200:0x20004400], Disabled
    Sub-region 2: [0x20004400:0x20004600], Disabled
    Sub-region 3: [0x20004600:0x20004800], Disabled
    Sub-region 4: [0x20004800:0x20004A00], Disabled
    Sub-region 5: [0x20004A00:0x20004C00], Disabled
    Sub-region 6: [0x20004C00:0x20004E00], Disabled
    Sub-region 7: [0x20004E00:0x20005000], Disabled
  Region 1: [0x10020000:0x10020200], length: 512 bytes; UnprivilegedReadOnly (0x2)
    Sub-region 0: [0x10020000:0x10020040], Enabled
    Sub-region 1: [0x10020040:0x10020080], Enabled
    Sub-region 2: [0x10020080:0x100200C0], Enabled
    Sub-region 3: [0x100200C0:0x10020100], Enabled
    Sub-region 4: [0x10020100:0x10020140], Enabled
    Sub-region 5: [0x10020140:0x10020180], Enabled
    Sub-region 6: [0x10020180:0x100201C0], Enabled
    Sub-region 7: [0x100201C0:0x10020200], Enabled
  Region 2: Unused
  Region 3: Unused
  Region 4: Unused
  Region 5: Unused
  Region 6: Unused
  Region 7: Unused

To debug, run `make lst` in the app's folder
and open the arch.0x10020048.0x20004000.lst file.

It seems to fault around here https://github.com/WyliodrinEmbeddedIoT/libtock-rs/blob/b4db50aec1ea0b61af90d25cfba938de0468d4fa/runtime/asm/asm_arm.S#L63.

This is a little strange as calling the following code seems to gave the correct value for r5.

mov r0, #0        /* operation: set break */
ldr r1, [r5, #4]  /* rt_header`s initial process break */
svc 5             /* call `memop` */

mov r0, #0        /* operation: set break */
ldr r1, [r5, #4]  /* rt_header`s initial process break */
svc 5             /* call `memop` */

mov r0, #0        /* operation: set break */
ldr r1, [r5, #4]  /* rt_header`s initial process break */
svc 5             /* call `memop` */

@hudson-ayers can you take a look at both the kernel and libtock-rs code asm code? I think you might have a better idea than me.

runtime/src/syscalls_impl_arm.rs Outdated Show resolved Hide resolved
runtime/src/syscalls_impl_arm.rs Outdated Show resolved Hide resolved
@hudson-ayers
Copy link
Contributor

@alexandruradovici
Copy link
Contributor Author

alexandruradovici commented Feb 16, 2022

I wonder why the instruction was compiled.

Using #383 seems to at least point out the error.

error: instruction requires: armv8m.base
        cbz r0, .Lzero_bss         /* Jump to zero_bss if remaining == 0 */
        ^
error: instruction requires: armv8m.base
        cbz r0, .Lcall_rust_start  /* Jump to call_rust_start if remaining == 0 */
        ^
error: instruction requires: armv8m.base
        cbz r0, .Lzero_bss         /* Jump to zero_bss if remaining == 0 */
        ^
error: instruction requires: armv8m.base
        cbz r0, .Lcall_rust_start  /* Jump to call_rust_start if remaining == 0 */
        ^
error: instruction requires: armv8m.base
        cbz r0, .Lzero_bss         /* Jump to zero_bss if remaining == 0 */
        ^
error: instruction requires: armv8m.base
        cbz r0, .Lcall_rust_start  /* Jump to call_rust_start if remaining == 0 */
        ^
   Compiling libtock2 v0.1.0 (/Users/alexandru/programe/tock/libtock-rs/libtock2)
error: instruction requires: armv8m.base
   |
note: instantiated into assembly here
  --> <inline asm>:66:2
   |
66 |     cbz r0, .Lzero_bss         /* Jump to zero_bss if remaining == 0 */
   |     ^

error: instruction requires: armv8m.base
   |
note: instantiated into assembly here
  --> <inline asm>:80:2
   |
80 |     cbz r0, .Lcall_rust_start  /* Jump to call_rust_start if remaining == 0 */
   |     ^

error: could not compile `libtock2` due to 2 previous errors

@jrvanwhy
Copy link
Collaborator

I wonder why the instruction was compiled.

IIRC, prior to #383 we don't even tell the assembler which version of ARM/Thumb we are compiling for, so it will presumably compile any instruction that is valid for any chip that supports Thumb.

I went ahead and merged #383, as you will need to adjust the assembly to avoid the cbz instruction (hopefully without increasing code size).

@alexandruradovici
Copy link
Contributor Author

I already did the adjustments, I just need to test them and I'll submit the PR.

@alexandruradovici
Copy link
Contributor Author

I added a separate file for the armv6 code and included it conditionally. Considering that the only changes are two cbz instructions, I'm not sure if it does not make more sens to simply replace them in the original arm file and drop the conditional includes.

@jrvanwhy
Copy link
Collaborator

I added a separate file for the armv6 code and included it conditionally. Considering that the only changes are two cbz instructions, I'm not sure if it does not make more sens to simply replace them in the original arm file and drop the conditional includes.

I think you should just change them in the original ARM file and drop the conditional include. Add a note in a comment at the top of the file (there's no room to do it inline) pointing out the waste on ARM v7, so that if someone needs to shave 4 bytes they know where to look.

@alexandruradovici
Copy link
Contributor Author

Done

@jrvanwhy
Copy link
Collaborator

bors r+

@bors
Copy link
Contributor

bors bot commented Feb 23, 2022

Build succeeded:

@bors bors bot merged commit 39fec6b into tock:master Feb 23, 2022
ppannuto added a commit that referenced this pull request Aug 8, 2022
I just learned about this `asm!` feature, which prompted me to look at whether we were using it.

Not sure if some of the others were options when #381 was written, but strictly speaking arm cores are defined to follow the `AAPCS` on exception entry/exit. In practice, I _think_ `C` and `aapcs` (and the others here) are just aliases that do the same thing. Nonetheless, just in case, we should probably put the 'most correct' name here.

Relevant doc: https://doc.rust-lang.org/nightly/reference/inline-assembly.html#abi-clobbers
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