Conversation
There was a problem hiding this comment.
5 issues found across 11 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="mk/generic.mk">
<violation number="1" location="mk/generic.mk:120">
P3: `gdb-attach` uses the ELF binary but doesn’t depend on it, so `make gdb-attach` can fail if the project isn’t built. Add `$(out)/$(PROJECT).elf` as a prerequisite.</violation>
</file>
<file name="kernel/syscall.c">
<violation number="1" location="kernel/syscall.c:85">
P0: Function always returns 0, but the assembly checks for return value 1. This makes `fastpath_schedule` dead code and the immediate context switch via `do_fastpath_schedule()` never happens. Based on the function contract, this should return 1 when fastpath succeeds.</violation>
</file>
<file name="include/platform/ipc-fastpath.h">
<violation number="1" location="include/platform/ipc-fastpath.h:193">
P2: Documentation is outdated: comment says `n_untyped <= 7` but the code actually supports `n_untyped <= 39`. This mismatch could mislead developers about fastpath capacity.</violation>
<violation number="2" location="include/platform/ipc-fastpath.h:202">
P1: Casting away `volatile` from `__irq_saved_regs` is unsafe. This shared memory between interrupt context (SVC handler) and C code requires volatile semantics to prevent the compiler from caching stale values or reordering accesses. Either keep the volatile in the cast or update `ipc_fastpath_helper` to accept `volatile uint32_t*`.</violation>
</file>
<file name="kernel/user-log.c">
<violation number="1" location="kernel/user-log.c:35">
P1: Security: Format pointer validation is missing permission check. The code validates that `format` is in a valid memory pool but doesn't verify user read permissions (unlike the `va` validation which checks `MP_UR | MP_UW`). A malicious user could point `format` to kernel-only memory, causing `dbg_vprintf` to leak kernel data. Add a permission check after getting the pool.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
97c13c1 to
43a2ef7
Compare
This implements QNX-inspired IPC fastpath extending capacity from 32 bytes (MR0-MR7) to 160 bytes (MR0-MR39) by adding 128-byte msg_buffer to TCB, achieving 3-4× speedup for 64-128 byte messages with projected 95% fastpath coverage. Message Register Mapping: - MR0-MR7: ctx.regs[0-7] (32 bytes, R4-R11 registers) - MR8-MR39: msg_buffer[0-31] (128 bytes, new TCB buffer) - MR40-MR47: utcb->mr[0-7] (32 bytes, UTCB overflow for slowpath) Fastpath Eligibility: 1. Simple send (to_tid valid, from_tid == NILTHREAD) 2. No typed items (n_typed == 0, no MapItem/GrantItem) 3. Short message (n_untyped <= 39, fits in registers + buffer) 4. Receiver ready (T_RECV_BLOCKED state) 5. Receiver waiting (for caller or L4_ANYTHREAD) 6. No special threads (excludes THREAD_LOG, THREAD_IRQ_REQUEST) 7. No thread start (tag.raw != 0x00000005 init protocol)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This extends IPC fastpath capacity from 32 bytes (MR0-MR7 register only) to 160 bytes by adding a 128-byte buffer in TCB structure.
Memory layout:
Inspired by QNX Neutrino short message optimization.
Summary by cubic
Expand IPC fastpath to 160 bytes using a per-thread short message buffer and a register-safe SVC path. Most short sends now skip the slowpath.
New Features
Bug Fixes
Written for commit 3a31f91. Summary will update on new commits.