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.

Sign up for GitHub

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

Fix a number of issues in CMSE examples #264

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix a number of issues in CMSE examples #264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 40 additions & 57 deletions cmse/cmse.md
View file
Open in desktop
Copy link

Choose a reason for hiding this comment

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

There's a couple of mentions of FPSCR that still need to be updated. EG:
"Save and clear bits [27:0] of FPSCR (Armv8.1-M Mainline only)"

I'm not sure where this has come from, but for v8.1 we should be using the FPCXT payloads to handle FPSCR. For v8.0 manual handling is required, but unless I'm missing something it the whole register must be set to a reasonable value (which may not be zero), and not just bits 27:0

Copy link
Member Author

Choose a reason for hiding this comment

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

I have replaced occurrences of FPSCR by FPCXT.

Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,13 @@ Anticipated changes to this document include:
[Arguments on the stack and floating point handling](#arguments-on-the-stack-and-floating-point-handling).
* Removed incorrect information about the floating-point ABI used in
[Arguments on the stack and floating point handling](#arguments-on-the-stack-and-floating-point-handling).
* Replaced a conditional clearing of floating-point registers by the `VSCCLRM`
instruction in the example in
[Arguments on the stack and floating point handling](#arguments-on-the-stack-and-floating-point-handling).
* Corrected description and example in section
[Non-secure function pointers](#non-secure-function-pointer).
* Corrected examples to save and restore `FPCXT` in Security state transitions.
* Replaced sequences of `mov rX, #0` by the `CLRM` instruction in examples.

## References

Expand Down Expand Up @@ -1173,7 +1178,8 @@ registers that contain secret information
> * Clear all registers and flags that have undefined values at the return of a
> procedure, according to [[AAPCS]](#AAPCS).
> * Restore all callee-saved registers as mandated by [[AAPCS]](#AAPCS).
> * Restore bits [27:0] of FPSCR (Armv8.1-M Mainline only).
> * Restore the Non-secure Floating-point context payload (`FPCXTNS`)
> (Armv8.1-M Mainline only).
Copy link

Choose a reason for hiding this comment

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

Saying "v8.1 mainline only" makes it sound like this isn't required on a v8.0 CPU. This is required on any CPU that has FP and MVE. If you have 8.1 then the new FPCXTNS payload makes it easier to do. However if you have v8.0 you still have to restore the context, but using the old approach detecting only restoring the FPSCR value if there's an active context etc.

Similar comment applies in a few other places below


You can clear the floating-point registers conditionally by checking the
`SFPA` bit of the special-purpose `CONTROL` register.
Expand Down Expand Up @@ -1266,16 +1272,17 @@ the non-secure state to restore these registers.
> * Registers that do not hold secret information.
> * Clear all registers and flags that have undefined values at the entry to a
> procedure according to the [[AAPCS]](#AAPCS).
> * Save and clear bits [27:0] of FPSCR (Armv8.1-M Mainline only).
> * Save and clear the Secure Floating-point context payload (`FPCXTS`)
> (Armv8.1-M Mainline only).

A toolchain could provide you with the means to specify that some
types of variables never hold secret information.

<span id="requirement-54" class="requirement-box"></span>

> When the non-secure function call returns, caller- and callee-saved
> registers saved before the call must be restored. This includes bits [27:0]
> of FPSCR (Armv8.1-M Mainline only).
> registers saved before the call must be restored. This includes `FPCXTS`
> (Armv8.1-M Mainline only).
> An implementation need not save and restore a register if its value is not
> live across the call. Note: callee-saved registers are live across the
> call in almost all situations. These requirements specify behaviour that is
Expand All @@ -1286,7 +1293,7 @@ types of variables never hold secret information.
> must be cleared.

The floating point registers can very efficiently be saved and cleared using
the `VLSTM`, and restored using `VLLDM` instructions.
the `VLSTM`, and restored using `VLLDM` instructions. These include `FPCXTS`.

An example instruction sequence for a non-secure call is listed in
[Example non-secure function call](#example-non-secure-function-call).
Expand Down Expand Up @@ -1595,12 +1602,8 @@ bar:
@ save callee-saved integer registers
push {r4-r12, lr}
@ clear all integer registers (except for function pointer and arguments)
mov r2, #0
mov r3, #0
…
mov r12, #0
@ clear the integer status flags
msr APSR_nzcvqg, r2
@ and the integer status flags
clrm {r2-r12, apsr}
@ perform the call to the non-secure function
bic r1, r1, #1
blxns r1
Expand Down Expand Up @@ -1642,12 +1645,8 @@ bar:
@ setup floating point arguments of the call
vmov s0, r1
@ clear all integer registers (except for function pointer and arguments)
mov r2, #0
mov r3, #0
…
mov r12, #0
@ clear the integer status flags
msr APSR_nzcvqg, r2
@ and the integer status flags
clrm {r2-r12, apsr}
@ perform the call to the non-secure function
bic r0, r0, #1
blxns r0
Expand Down Expand Up @@ -1723,12 +1722,8 @@ bar:
@ load the function pointer
ldr r4, =foo
@ clear all integer registers (except for function pointer and arguments)
mov r6, #0
mov r7, #0
…
mov r12, #0
@ clear the integer status flags
msr APSR_nzcvqg, r6
@ and the integer status flags
clrm {r6-r12, apsr}
@ perform the call to the non-secure function
bic r4, r4, #1
blxns r4
Expand Down Expand Up @@ -1810,65 +1805,53 @@ __acle_se_foo:
tst lr, #1
it eq
subeq sp, sp, #8
@ 2: push used callee-saved register onto the stack
@ 2: save Non-secure Floating-point context payload
vstr fpcxtns, [sp, #-8]!
@ 3: push used callee-saved register onto the stack
push {r4-r6, lr}
@ 3: if called from secure the arguments are already in the correct place
tst lr, #1
@ 4: if called from secure the arguments are already in the correct place
bne .LdoneARGS
@ 4: get the non-secure stack pointer
@ 5: get the non-secure stack pointer
mrs r4, SP_NS
@ 5: calculate end of range
@ 6: calculate end of range
adds r6, r4, #7
@ 6: take permissions at begin and end of range
@ 7: take permissions at begin and end of range
tta r5, r4
tta r6, r6
@ 7: check if range is in one region (this means identical permissions)
@ 8: check if range is in one region (this means identical permissions)
cmp r5, r6
it ne
blne cmse_abort
@ 8: check bit 20 of the TT result (non-secure read flag)
@ 9: check bit 20 of the TT result (non-secure read flag)
tst r5, #0x100000
it eq
bleq cmse_abort
@ 9: copy arguments from non-secure stack to secure stack
@10: copy arguments from non-secure stack to secure stack
ldr r5, [r4 ]
ldr r6, [r4, #4 ]
str r5, [sp, #16]
str r6, [sp, #20]
.LdoneARGS:
@10: function body
@11: function body
ldr r0, [sp, #16]
ldr r1, [sp, #20]
bl bar
@11: restore used callee-saved registers
@12: restore used callee-saved registers
pop {r4-r6, lr}
@12: if called from secure, we are done
@13: restore Non-secure Floating-point context payload
vldr fpcxtns, [sp], #8
@14: if called from secure, we are done
tst lr, #1
it ne
bxne lr
@13: pop secure stack space
@15: pop secure stack space
add sp, sp, #8
@14: check SFPA bit to see if FP is used
mrs r1, control
tst r1, #8
bne .LdoneFP
@15: clear floating point caller-saved registers
mov r1, #0
vmov s0, s1, r1, r1
vmov s2, s3, r1, r1
...
vmov s30, s31, r1, r1
@16: clear floating point flags
vmsr fpscr, r1
@16: clear floating point caller-saved registers if a FP context is active
vscclrm {s0-s31}
.LdoneFP:
@17: clear integer caller-saved registers except for return value
mov r1, #0
mov r2, #0
mov r3, #0
@18: clear other registers and the integer status flags
mov r12, #0
msr APSR_nzcvqg, r3
@19: return to non-secure state
@17: clear integer caller-saved registers except for return value,
@ r12 and the integer status flags
clrm {r1-r3, r12, apsr}
@18: return to non-secure state
bxns lr
```

Expand Down
7 changes: 1 addition & 6 deletions main/acle.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,12 +341,7 @@ Armv8.4-A [[ARMARMv84]](#ARMARMv84). Support is added for the Dot Product intrin
* Fixes for [Function Multi Versioning](#function-multi-versioning):
* Renamed features to `sme-f64f64` and `sme-i16i64`
* Corrected register name to `ID_AA64SMFR0_EL1.I16I64`
* Removed incorrect optimisation remark in [CMSE](#CMSE-ACLE)'s floating-point
register clearing.
* Removed incorrect information about the floating-point ABI used in
[CMSE](#CMSE-ACLE)'s Arguments on the stack and floating point handling.
* Corrected description and example in [CMSE](#CMSE-ACLE)'s section about
non-secure function pointers.
* Several fixes in [CMSE](#CMSE-ACLE).

### References

Expand Down