Skip to content

Commit ef76468

Browse files
rpcmerpcme
committed
armv9: fix CI checks (spelling, header, unit test, formatting)
- tasks.c: revert portSTRIP_ADDRESS_TAG in prvInitialiseNewTask — tags are not applied at task creation time so stripping is unnecessary, and the macro is not available in the CMock unit test environment - mte_port.c: add standard FreeRTOS MIT license header - include/stack_macros.h: restore upstream formatting (revert local uncrustify that diverged from CI), keep portSTRIP_ADDRESS_TAG in runtime overflow checks where MTE tags are present - .github/.cSpellWords.txt: add ARM architecture terms (ACLE, APDA, APDB, REGEN, SSVE, SVCR)
1 parent 294945f commit ef76468

5 files changed

Lines changed: 382 additions & 36 deletions

File tree

‎.github/.cSpellWords.txt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,3 +1017,10 @@ XPSR
10171017
XRAM
10181018
xtal
10191019
XTENSA
1020+
ACLE
1021+
acle
1022+
APDA
1023+
APDB
1024+
REGEN
1025+
SSVE
1026+
SVCR

‎docs/MISRA_REVIEW.md‎

Lines changed: 298 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,298 @@
1+
# MISRA C:2012 Review — Armv9 Port Changes
2+
3+
## Files Changed
4+
5+
| File | Change |
6+
|------|--------|
7+
| `include/FreeRTOS.h` | Added `portSTRIP_ADDRESS_TAG` default macro |
8+
| `include/stack_macros.h` | Applied `portSTRIP_ADDRESS_TAG` in overflow checks |
9+
| `tasks.c` | Applied `portSTRIP_ADDRESS_TAG` in stack depth assert |
10+
| `portable/GCC/ARM_AARCH64_ARMV9/port.c` | MTE stack tagging, PAC key init |
11+
| `portable/GCC/ARM_AARCH64_ARMV9/portmacro.h` | MTE/PAC declarations, `portSTRIP_ADDRESS_TAG` override |
12+
| `portable/GCC/ARM_AARCH64_ARMV9/portASM.S` | SVE2/PAC context switch (assembly — MISRA N/A) |
13+
| `portable/GCC/ARM_AARCH64_ARMV9/mte_port.c` | MTE heap wrapper (new file) |
14+
15+
## Review by Rule
16+
17+
### include/FreeRTOS.h
18+
19+
```c
20+
#ifndef portSTRIP_ADDRESS_TAG
21+
#define portSTRIP_ADDRESS_TAG( pxPointer ) ( pxPointer )
22+
#endif
23+
```
24+
25+
**No violations.** Function-like macro is consistent with existing patterns (Dir 4.9 deviation already documented). Identity macro has no side effects.
26+
27+
---
28+
29+
### include/stack_macros.h
30+
31+
```c
32+
if( portSTRIP_ADDRESS_TAG( pxCurrentTCB->pxTopOfStack ) <= pxCurrentTCB->pxStack + portSTACK_LIMIT_PADDING )
33+
```
34+
35+
**No new violations.** The macro expands to the pointer itself (no-op) for all existing ports. For our port, it performs a cast — covered by existing Rule 11.4 deviation (pointer-to-integer conversion for stack alignment).
36+
37+
---
38+
39+
### tasks.c
40+
41+
```c
42+
configASSERT( ( ( portPOINTER_SIZE_TYPE ) ( pxTopOfStack - ( StackType_t * ) portSTRIP_ADDRESS_TAG( pxNewTCB->pxTopOfStack ) ) ) < ( ( portPOINTER_SIZE_TYPE ) uxStackDepth ) );
43+
```
44+
45+
**Potential violations:**
46+
47+
| Rule | Issue | Severity | Justification |
48+
|------|-------|----------|---------------|
49+
| Rule 11.4 | Cast from pointer to integer (`uint64_t`) inside `portSTRIP_ADDRESS_TAG` | Required | Same pattern as existing `portPOINTER_SIZE_TYPE` casts in this file. Needed to strip MTE tag bits. Only active when `configARMV9_MTE_STACK=1`. |
50+
| Rule 11.3 | Cast from `void *` back to `StackType_t *` | Required | The macro strips tag and returns same type. Alignment guaranteed (16-byte aligned stack). |
51+
52+
**Action:** Add MISRA suppression comment:
53+
```c
54+
/* MISRA Ref 11.4.1 - Cast required to strip MTE address tag for pointer comparison. */
55+
```
56+
57+
---
58+
59+
### portable/GCC/ARM_AARCH64_ARMV9/portmacro.h
60+
61+
```c
62+
#define portSTRIP_ADDRESS_TAG( pxPointer ) \
63+
( ( typeof( pxPointer ) )( ( uint64_t )( pxPointer ) & 0x00FFFFFFFFFFFFFFULL ) )
64+
```
65+
66+
**Violations:**
67+
68+
| Rule | Issue | Severity | Action |
69+
|------|-------|----------|--------|
70+
| Rule 11.4 | Pointer to integer conversion | Required | Justified: needed to mask MTE tag bits. Add MISRA Ref comment. |
71+
| Rule 11.6 | Cast from integer to pointer | Required | Justified: restoring original pointer type after tag strip. Add MISRA Ref comment. |
72+
| Dir 4.9 | Function-like macro | Advisory | Already project-wide deviation in coverity_misra.config. |
73+
| Extension | `typeof` is a GCC extension, not standard C | N/A for MISRA | Port is GCC-specific (directory name says GCC). Consistent with existing port. |
74+
75+
**Action:** Replace `typeof` with explicit cast in the macro to avoid GCC extension:
76+
```c
77+
#define portSTRIP_ADDRESS_TAG( pxPointer ) \
78+
( ( void * )( ( uint64_t )( pxPointer ) & 0x00FFFFFFFFFFFFFFULL ) )
79+
```
80+
Then cast at call sites. OR document `typeof` as a port-specific GCC extension (consistent with existing `__asm volatile` usage).
81+
82+
---
83+
84+
### portable/GCC/ARM_AARCH64_ARMV9/port.c (MTE stack tagging)
85+
86+
```c
87+
uint64_t raw_top = (uint64_t)pxTopOfStack;
88+
uint64_t tagged;
89+
__asm volatile( "irg %0, %1" : "=r"(tagged) : "r"(raw_top) );
90+
uint64_t tag_only = tagged & 0xFF00000000000000ULL;
91+
size_t xStackBytes = (size_t)configMINIMAL_STACK_SIZE * sizeof( StackType_t );
92+
uint64_t base = ( raw_top - xStackBytes + 16 ) & ~0xFULL;
93+
uint64_t tagged_base = base | tag_only;
94+
for( size_t i = 0; i < xStackBytes; i += 16 )
95+
{
96+
__asm volatile( "stg %0, [%0]" :: "r"( tagged_base + i ) );
97+
}
98+
pxTopOfStack = (StackType_t *)( raw_top | tag_only );
99+
```
100+
101+
**Violations:**
102+
103+
| Rule | Issue | Severity | Action |
104+
|------|-------|----------|--------|
105+
| Rule 11.4 | `(uint64_t)pxTopOfStack` — pointer to integer | Required | Justified: MTE requires integer manipulation of pointer tag bits. |
106+
| Rule 11.6 | `(StackType_t *)(raw_top | tag_only)` — integer to pointer | Required | Justified: reconstructing tagged pointer. Alignment preserved. |
107+
| Rule 1.2 | Inline assembly (`__asm volatile`) | Advisory | Already project-wide deviation. GCC port uses inline asm extensively. |
108+
| Dir 4.11 | `sizeof` in arithmetic | Advisory | Standard pattern, no issue. |
109+
110+
**Action:** Add MISRA Ref comments at each cast.
111+
112+
---
113+
114+
### portable/GCC/ARM_AARCH64_ARMV9/port.c (PAC key init)
115+
116+
```c
117+
pxTopOfStack--; *pxTopOfStack = 0xCAFEBEEFDEADF00DULL; /* APIA Lo */
118+
```
119+
120+
**No violations.** Assigning integer constants to `StackType_t` (which is `uint64_t`). Same pattern as existing GPR initialization in this function.
121+
122+
---
123+
124+
### portable/GCC/ARM_AARCH64_ARMV9/mte_port.c
125+
126+
```c
127+
void *pvPortMallocTagged( size_t xSize )
128+
{
129+
void *pv = pvPortMalloc( xSize );
130+
if( pv == NULL ) return NULL;
131+
uint64_t tagged;
132+
__asm volatile( "irg %0, %1" : "=r"(tagged) : "r"((uint64_t)pv) );
133+
size_t xRounded = ( xSize + 15 ) & ~15UL;
134+
for( size_t i = 0; i < xRounded; i += 16 )
135+
{
136+
__asm volatile( "stg %0, [%0]" :: "r"(tagged + i) );
137+
}
138+
return (void *)tagged;
139+
}
140+
```
141+
142+
**Violations:**
143+
144+
| Rule | Issue | Severity | Action |
145+
|------|-------|----------|--------|
146+
| Rule 11.4 | `(uint64_t)pv` — pointer to integer | Required | Same justification as above. |
147+
| Rule 11.6 | `(void *)tagged` — integer to pointer | Required | Same justification. |
148+
| Rule 11.5 | Return `void *` (implicit conversion at call site) | Advisory | Already project-wide deviation (Ref 11.5.1 — pvPortMalloc pattern). |
149+
| Rule 15.5 | Multiple return points (`if NULL return NULL`) | Advisory | Already project-wide deviation. |
150+
151+
**Action:** Add MISRA Ref comments.
152+
153+
---
154+
155+
## Summary
156+
157+
| Category | Count | Action |
158+
|----------|-------|--------|
159+
| New violations requiring justification | 4 (Rule 11.4, 11.6 for MTE tag manipulation) | Add MISRA Ref comments |
160+
| Violations covered by existing deviations | 5 (Dir 4.9, Rule 1.2, 11.5, 15.5) | No action needed |
161+
| GCC extension usage (`typeof`) | 1 | Replace with `void *` cast or document |
162+
| Assembly files | N/A | MISRA applies to C only |
163+
164+
## Recommended Actions
165+
166+
1. Add `/* MISRA Ref 11.4.2 */` comments at pointer-to-integer casts for MTE tag manipulation
167+
2. Add `/* MISRA Ref 11.6.1 */` comments at integer-to-pointer casts for tagged pointer reconstruction
168+
3. Replace `typeof` with explicit `void *` in `portSTRIP_ADDRESS_TAG` macro
169+
4. Add deviation documentation to `MISRA.md` for the new Ref numbers
170+
171+
---
172+
173+
## ACLE Intrinsics — Required Refactoring
174+
175+
### Finding
176+
177+
Our port uses raw inline assembly (`__asm volatile`) for MTE operations where ARM provides
178+
MISRA-compliant C intrinsics via `<arm_acle.h>`. These intrinsics operate on `void *` pointers
179+
throughout — no pointer↔integer casts required. ARM designed them specifically so that
180+
MISRA-compliant code can use MTE without violations.
181+
182+
### MTE Intrinsics (arm_acle.h, available with -march=armv8.5-a+memtag)
183+
184+
| Intrinsic | Replaces | Current usage location |
185+
|-----------|----------|----------------------|
186+
| `__arm_mte_create_random_tag(ptr, mask)` | `IRG Xd, Xn` | mte_port.c, port.c |
187+
| `__arm_mte_set_tag(ptr)` | `STG Xt, [Xt]` | mte_port.c, port.c |
188+
| `__arm_mte_get_tag(ptr)` | `LDG Xt, [Xn]` | Not used yet |
189+
| `__arm_mte_increment_tag(ptr, offset)` | `ADDG` | Not used yet |
190+
| `__arm_mte_ptrdiff(ptr)` | Manual `& 0x00FFFFFFFFFFFFFF` | portmacro.h |
191+
192+
### PAC Intrinsics (arm_acle.h)
193+
194+
| Intrinsic | Replaces | Current usage location |
195+
|-----------|----------|----------------------|
196+
| `__builtin_arm_pacia(ptr, modifier)` | `PACIA X30, SP` | portASM.S (PAC_FRAME) |
197+
| `__builtin_arm_autia(ptr, modifier)` | `AUTIA X30, SP` | portASM.S (PAC_FRAME) |
198+
199+
Note: PAC intrinsics exist but our PAC_FRAME usage is in assembly context switch macros
200+
where C intrinsics are not applicable. Assembly is exempt from MISRA. No action needed.
201+
202+
PAC key register access (`MRS APIAKeyLo_EL1` etc.) has NO ACLE intrinsic — system register
203+
access requires inline asm. This is expected and covered by existing Rule 1.2 deviation.
204+
205+
### SVE2 Intrinsics (arm_sve.h, available with -march=armv9-a+sve2)
206+
207+
| Intrinsic | Replaces | Current usage location |
208+
|-----------|----------|----------------------|
209+
| `svcntb()` | `RDVL X0, #1` | sve2_demo.c |
210+
| `svld1_u8(pg, ptr)` | `LD1B` | sobel_demo.c, blur_demo.c |
211+
| `svst1_u8(pg, ptr, data)` | `ST1B` | sobel_demo.c, blur_demo.c |
212+
| `svwhilelt_b8(i, n)` | `WHILELT` | All SVE2 demos |
213+
| `svhistcnt_u32(pg, a, b)` | `HISTCNT` | histogram_demo.c |
214+
| `svmatch_u8(pg, a, b)` | `MATCH` | match_demo.c |
215+
| `svabd_u8_m(pg, a, b)` | `UABD` | sad_demo.c |
216+
| `svdup_u8(val)` | `DUP` | sve2_demo.c |
217+
| `svaddv_u8(pg, data)` | `UADDV` | histogram_demo.c, sad_demo.c |
218+
219+
### Impact Assessment
220+
221+
| File | Violation source | Fix | MISRA impact |
222+
|------|-----------------|-----|--------------|
223+
| `mte_port.c` | `(uint64_t)pv`, `(void *)tagged` | Use `__arm_mte_create_random_tag` + `__arm_mte_set_tag` | Eliminates Rule 11.4 and 11.6 violations entirely |
224+
| `port.c` (MTE stack) | `(uint64_t)pxTopOfStack`, `(StackType_t *)(raw_top \| tag_only)` | Use `__arm_mte_create_random_tag` + `__arm_mte_set_tag` loop | Eliminates Rule 11.4 and 11.6 violations entirely |
225+
| `portmacro.h` | `(uint64_t)(pxPointer) & 0x00FF...` | Use `__arm_mte_ptrdiff(pxPointer)` | Eliminates Rule 11.4, 11.6, and GCC `typeof` extension |
226+
| Demo SVE2 files | Inline asm | Use `arm_sve.h` intrinsics | Demo code (not upstreamed to kernel) — advisory, not blocking |
227+
228+
### Refactoring Plan
229+
230+
**Priority 1 (MISRA blocking — port files):**
231+
1. `portmacro.h`: Replace `portSTRIP_ADDRESS_TAG` with `__arm_mte_ptrdiff`
232+
2. `mte_port.c`: Replace all `IRG`/`STG` inline asm with ACLE intrinsics
233+
3. `port.c`: Replace MTE stack tagging inline asm with ACLE intrinsics
234+
235+
**Priority 2 (Quality — demo files):**
236+
4. SVE2 demo files: Replace inline asm with `arm_sve.h` intrinsics (improves readability, enables compiler optimization)
237+
238+
**Not applicable:**
239+
- `portASM.S`: Assembly file, MISRA exempt
240+
- PAC key register access: No ACLE intrinsic exists, inline asm is the only option (covered by existing deviation)
241+
242+
### After Refactoring
243+
244+
With ACLE intrinsics applied to port files:
245+
- **Zero new MISRA violations** in kernel-level code
246+
- All MTE operations use ARM's official C API
247+
- `portSTRIP_ADDRESS_TAG` becomes a single intrinsic call — no casts visible to `tasks.c` or `stack_macros.h`
248+
- The only remaining inline asm in port C files is PAC key register access (unavoidable, pre-existing deviation)
249+
250+
---
251+
252+
## Toolchain Requirement
253+
254+
### Finding
255+
256+
MTE ACLE intrinsics (`__arm_mte_create_random_tag`, `__arm_mte_set_tag`, etc.) are NOT available
257+
in the GCC bare-metal toolchain (`aarch64-none-elf-gcc`). The `arm_acle.h` header in this
258+
toolchain is a stub that defines feature-test macros only.
259+
260+
### Intrinsic Availability by Toolchain
261+
262+
| Toolchain | MTE Intrinsics | SVE Intrinsics | Status |
263+
|-----------|---------------|----------------|--------|
264+
| `aarch64-none-elf-gcc` (ARM GNU, bare-metal) | ❌ Not available | ✅ Available (`arm_sve.h`) | Current toolchain |
265+
| `aarch64-linux-gnu-gcc` (GCC, Linux) | ✅ Available (GCC 10+) | ✅ Available | Requires Linux sysroot |
266+
| `armclang` (Arm Compiler for Embedded) | ✅ Available | ✅ Available | Commercial license |
267+
| `clang` (LLVM, bare-metal) | ✅ Available (Clang 9+) | ✅ Available | Open source |
268+
269+
### Minimum Toolchain for Zero MISRA Violations
270+
271+
To use ACLE MTE intrinsics and eliminate all pointer↔integer cast violations:
272+
- **Arm Compiler for Embedded 6.x+** (armclang), OR
273+
- **LLVM/Clang 9+** with `--target=aarch64-none-elf`
274+
275+
### Current Toolchain
276+
277+
```
278+
arm-gnu-toolchain-15.2.rel1-x86_64-aarch64-none-elf
279+
GCC 15.2.1 (bare-metal)
280+
```
281+
282+
This toolchain supports SVE2 intrinsics (`arm_sve.h`) but NOT MTE intrinsics.
283+
MTE operations require inline assembly with documented MISRA deviations (Rule 11.4, 11.6).
284+
285+
### Decision
286+
287+
Until the project migrates to armclang or Clang:
288+
1. MTE port code uses inline assembly (current approach)
289+
2. MISRA deviations for Rule 11.4/11.6 are documented with justification
290+
3. SVE2 demo code CAN use `arm_sve.h` intrinsics (available in current toolchain)
291+
4. When toolchain is upgraded, refactor MTE code to ACLE intrinsics (code is ready in git history)
292+
293+
### Recommendation for Upstream
294+
295+
Document in the port README:
296+
- **Minimum toolchain (MISRA compliant)**: Arm Compiler for Embedded 6.19+ or Clang 9+
297+
- **Minimum toolchain (functional)**: GCC 10+ with `-march=armv8.5-a+memtag`
298+
- **SVE2 intrinsics**: GCC 10+ or any toolchain with `arm_sve.h`

0 commit comments

Comments
 (0)