Development #70

coderabbitai · 2025-08-11T08:48:01Z

🛠️ Refactor suggestion

Polish the memset wrapper; keep signature conventional and remove unprofessional comment

Replace the comment with a neutral rationale.

Prefer size_t for the size parameter to match the header and conventional signatures.

-void* memset(void* dest, int value, unsigned long size) { // FCK GCC - return FastMemset(dest, value, size); -} +void* memset(void* dest, int value, size_t size) { // Provide standard-compatible memset symbol in freestanding kernel + return FastMemset(dest, value, (uint64_t)size); +}

Additionally, ensure FastMemset masks the byte value to 0xFF in the SSE path (outside the changed lines); otherwise, non-8-bit values or negative ints can produce incorrect fill patterns.

Outside selected lines, suggested fix in FastMemset:

// before building val64 uint8_t b = (uint8_t)value; uint64_t val64 = ((uint64_t)b << 56) | ((uint64_t)b << 48) | ((uint64_t)b << 40) | ((uint64_t)b << 32) | ((uint64_t)b << 24) | ((uint64_t)b << 16) | ((uint64_t)b << 8) | (uint64_t)b;

🤖 Prompt for AI Agents

In kernel/memory/MemOps.c around lines 41 to 43, the memset wrapper uses a nonstandard signature and an unprofessional comment; change the function signature to the conventional form using size_t for the size parameter (i.e. void *memset(void *dest, int value, size_t size)), replace the informal comment with a short neutral rationale (e.g. "Wrapper forwarding to optimized FastMemset"), and update call sites if necessary; additionally, outside these lines update FastMemset's SSE path to mask the incoming int value to a single byte (0xFF) before replicating into wider words so non-8-bit or negative ints produce correct fill patterns.

coderabbitai · 2025-08-11T08:48:01Z

💡 Verification agent

🧩 Analysis chain

Align memset signature with conventional size_t and internal usage

Using unsigned long works on x86_64 (size_t == unsigned long), but declaring with size_t communicates intent and matches conventional prototypes and compiler expectations.

Minimal change (keeping freestanding constraints):

-void* memset(void* dest, int value, unsigned long size); +// Use size_t for better compatibility with compiler-emitted calls. +void* memset(void* dest, int value, size_t size);

If stddef.h is unavailable in your freestanding environment, ensure size_t is defined in a kernel-provided header before this declaration.

Do we have a kernel-provided typedef for size_t (e.g., in a local stddef.h)? If not, I can add one where appropriate.

🏁 Script executed:

#!/bin/bash # Search for an existing typedef of size_t in the kernel or common headers echo "=== Searching for 'typedef.*size_t' ===" rg -n "typedef\s+.*size_t" || echo "No typedef of size_t found." echo "" # Search for stddef.h includes that might define size_t echo "=== Searching for '#include.*stddef.h' ===" rg -n "#include.*stddef.h" || echo "No stddef.h includes found." echo "" # Search for any usage of size_t to see where it’s expected echo "=== Searching for 'size_t' usage ===" rg -n "size_t" || echo "No uses of size_t found."

Length of output: 3278

🏁 Script executed:

#!/bin/bash # Display include/stdint.h around size_t definition echo "=== include/stdint.h ===" sed -n '1,50p' include/stdint.h || echo "include/stdint.h not found" echo "" # Display include/stddef.h content echo "=== include/stddef.h ===" sed -n '1,50p' include/stddef.h || echo "include/stddef.h not found" echo "" # Display kernel/memory/MemOps.h context echo "=== kernel/memory/MemOps.h ===" sed -n '1,200p' kernel/memory/MemOps.h || echo "kernel/memory/MemOps.h not found"

Length of output: 2116

Use size_t for memset’s size parameter in MemOps.h

To match the freestanding size_t (which is defined as uint64_t in include/stddef.h) and keep consistency with your other memory APIs, update the signature and include the proper header:

• File: kernel/memory/MemOps.h
– Replace the unsigned long parameter with size_t.
– Add #include "stddef.h" to pull in the kernel’s size_t definition.

Minimal diff:

--- a/kernel/memory/MemOps.h +++ b/kernel/memory/MemOps.h @@ -#include "stdint.h" +#include "stddef.h" // for size_t (uint64_t) +#include "stdint.h" -void* memset(void* dest, int value, unsigned long size); +void* memset(void* dest, int value, size_t size);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

void* memset(void* dest, int value, unsigned long size);

#include "stddef.h" // for size_t (uint64_t)

#include "stdint.h"

void* memset(void* dest, int value, size_t size);

🤖 Prompt for AI Agents

In kernel/memory/MemOps.h around line 10, the memset declaration uses unsigned long for the size parameter which should be the freestanding size_t; update the function signature to use size_t for the size parameter and add #include "stddef.h" at the top of the header to pull in the kernel's size_t definition so the declaration matches other memory APIs and the architecture-wide typedef.

coderabbitai · 2025-08-11T08:48:01Z

💡 Verification agent

🧩 Analysis chain

PitSetFrequency usage: API unconditionally cli/sti — risks re-enabling IRQs unexpectedly

Calling PitSetFrequency(new_freq) here is fine, but the implementation shown in drivers/Pic.c does cli/sti without saving/restoring prior flags. If any caller invokes it with IRQs disabled, it will inadvertently re-enable interrupts on return. Make PitSetFrequency preserve and restore IF.

Also compute the PIT divisor in wider width, then narrow to 16-bit, for safety.

Proposed fix (in drivers/Pic.c):

-void PitSetFrequency(uint16_t hz) { - // Disable interrupts to ensure atomic update - asm volatile("cli"); - PIT_FREQUENCY_HZ = hz; // Update the global state - const uint16_t divisor = 1193180 / hz; - outb(0x43, 0x36); - outb(0x40, divisor & 0xFF); - outb(0x40, (divisor >> 8) & 0xFF); - // Re-enable interrupts - asm volatile("sti"); -} +void PitSetFrequency(uint16_t hz) { + irq_flags_t flags = save_irq_flags(); + cli(); + PIT_FREQUENCY_HZ = hz; // Update the global state (see note on volatile below) + const uint32_t div32 = 1193180u / (hz ? hz : 1u); + const uint16_t divisor = (uint16_t)div32; // clamped by caller; ensure 16-bit + outb(0x43, 0x36); + outb(0x40, (uint8_t)(divisor & 0xFF)); + outb(0x40, (uint8_t)((divisor >> 8) & 0xFF)); + restore_irq_flags(flags); +}

Additional suggestions:

Declare PIT_FREQUENCY_HZ as volatile in a shared header to avoid stale reads in reporting paths.

In Pit.h:

extern volatile uint16_t PIT_FREQUENCY_HZ;

Keep the process layer including Pit.h (not Pic.h) to reflect ownership.

To ensure there are no unsafe call sites that expect IRQs to remain disabled after PitSetFrequency returns, scan the repo for usages:

🏁 Script executed:

#!/bin/bash set -euo pipefail rg -n -A3 -B3 $'\\bPitSetFrequency\\s*\\('

Length of output: 1073

🏁 Script executed:

#!/bin/bash set -euo pipefail # Display the full PitSetFrequency implementation for review rg -n -A10 -B2 'void PitSetFrequency' drivers/Pic.c

Length of output: 471

Preserve and restore IRQ flags in PitSetFrequency

PitSetFrequency currently does an unconditional cli/sti pair, which will re-enable interrupts even if they were already disabled. It also computes the PIT divisor in 16-bit width (and panics on hz==0). Please update as follows:

• drivers/Pic.c

void PitSetFrequency(uint16_t hz) { - // Disable interrupts to ensure atomic update - asm volatile("cli"); - PIT_FREQUENCY_HZ = hz; // Update the global state - const uint16_t divisor = 1193180 / hz; + // Preserve IRQ state, disable interrupts, then restore + irq_flags_t flags = save_irq_flags(); + cli(); + PIT_FREQUENCY_HZ = hz; // Update the global state + // Compute divisor in 32-bit, avoid divide-by-zero, then narrow + uint32_t div32 = 1193180u / (hz ? hz : 1u); + uint16_t divisor = (uint16_t)div32; outb(0x43, 0x36); outb(0x40, divisor & 0xFF); outb(0x40, (divisor >> 8) & 0xFF); - // Re-enable interrupts - asm volatile("sti"); + restore_irq_flags(flags); }

• drivers/Pic.h

- void PitSetFrequency(uint16_t hz); + extern volatile uint16_t PIT_FREQUENCY_HZ; + void PitSetFrequency(uint16_t hz);

• Call sites checked:
– kernel/process/Process.c:1226 (no callers expect IRQs to remain disabled)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

// Hysteresis - only change if difference is significant

if (ABSi(new_freq - controller.current_freq) > 10) {

UpdatePIT(new_freq);

PitSetFrequency(new_freq);

controller.current_freq = new_freq;

--- a/drivers/Pic.c

+++ b/drivers/Pic.c

@@

void PitSetFrequency(uint16_t hz) {

- // Disable interrupts to ensure atomic update

- asm volatile("cli");

- PIT_FREQUENCY_HZ = hz; // Update the global state

- const uint16_t divisor = 1193180 / hz;

+ // Preserve IRQ state, disable interrupts, then restore

+ irq_flags_t flags = save_irq_flags();

+ cli();

+ PIT_FREQUENCY_HZ = hz; // Update the global state

+ // Compute divisor in 32-bit, avoid divide-by-zero, then narrow

+ uint32_t div32 = 1193180u / (hz ? hz : 1u);

+ uint16_t divisor = (uint16_t)div32;

outb(0x43, 0x36);

outb(0x40, divisor & 0xFF);

outb(0x40, (divisor >> 8) & 0xFF);

- // Re-enable interrupts

- asm volatile("sti");

+ restore_irq_flags(flags);

}

Suggested change

// Hysteresis - only change if difference is significant

if (ABSi(new_freq - controller.current_freq) > 10) {

UpdatePIT(new_freq);

PitSetFrequency(new_freq);

controller.current_freq = new_freq;

--- a/drivers/Pic.h

+++ b/drivers/Pic.h

@@

- void PitSetFrequency(uint16_t hz);

+ extern volatile uint16_t PIT_FREQUENCY_HZ;

+ void PitSetFrequency(uint16_t hz);

🤖 Prompt for AI Agents

In kernel/process/Process.c around lines 1224 to 1228, the call to PitSetFrequency(new_freq) must not rely on PitSetFrequency unconditionally disabling and re-enabling IRQs; update drivers/Pic.c and drivers/Pic.h so PitSetFrequency preserves and restores IRQ flags instead of doing a plain cli/sti pair, i.e. save CPU interrupt flag state at entry and restore it on exit; also change the PIT divisor computation to use an unsigned 16-bit width and avoid panicking on hz==0 (handle zero by clamping to a valid minimum or returning an error), and ensure the function prototype in Pic.h reflects the same behavior; note that the caller at kernel/process/Process.c:1226 does not expect IRQs to remain disabled so the saved/restored flags approach will preserve caller semantics.

hmm interesting the freeze after kill came back but its only happen in ArpRequestTestProcess, can you check?

🧩 Analysis chain

@assembler-0, let me investigate the ArpRequestTestProcess freeze issue.

🏁 Script executed:

#!/bin/bash # Search for ArpRequestTestProcess definition and usage echo "=== Searching for ArpRequestTestProcess ===" rg -n "ArpRequestTestProcess" -A 10 -B 2 echo -e "\n=== Checking for ARP-related code ===" rg -n "ARP|Arp" --type c -l | head -10 echo -e "\n=== Looking for network test processes ===" rg -n "TestProcess|test.*process" -i --type c | grep -E "(void|Process|create)" -A 2 | head -20

Length of output: 2070

🏁 Script executed:

#!/bin/bash # Get the complete ArpRequestTestProcess implementation echo "=== Full ArpRequestTestProcess implementation ===" rg -n "^void ArpRequestTestProcess" -A 50 kernel/etc/Shell.c echo -e "\n=== Check for RTL8139 blocking operations ===" rg -n "GetRtl8139Device|Rtl8139Send|Rtl8139" --type c | grep -E "(while|wait|block|spin)" -B 2 -A 2 | head -20 echo -e "\n=== Check if ArpRequestTestProcess has loops or waits ===" rg -n "ArpRequestTestProcess" -A 30 kernel/etc/Shell.c | grep -E "(while|for|wait|sleep|Yield)"

Length of output: 2560

🏁 Script executed:

#!/bin/bash # Get more lines of ArpRequestTestProcess to see the send operation echo "=== Complete ArpRequestTestProcess function ===" rg -n "^void ArpRequestTestProcess" -A 70 kernel/etc/Shell.c echo -e "\n=== Search for Rtl8139Send function implementation ===" rg -n "Rtl8139Send|RTL8139.*Send" -B 2 -A 10 --type c echo -e "\n=== Check process termination with network operations ===" rg -n "io_operations|IO_BOOST" kernel/process/Process.c | head -10

Length of output: 3457

🏁 Script executed:

#!/bin/bash # Find Rtl8139_SendPacket implementation echo "=== Searching for Rtl8139_SendPacket implementation ===" fd -e c -e h | xargs rg -l "Rtl8139_SendPacket" 2>/dev/null echo -e "\n=== Get Rtl8139_SendPacket full implementation ===" rg -n "Rtl8139_SendPacket" -B 5 -A 30 --type c echo -e "\n=== Check for spinlocks or waits in RTL8139 driver ===" fd "rtl8139" -i -e c | xargs rg -n "while|SpinLock|wait" 2>/dev/null | head -20

Length of output: 5510

@assembler-0, I found the likely cause of the freeze when killing ArpRequestTestProcess!

The issue is with the PitSetFrequency function's interrupt handling. Here's what's happening:

ArpRequestTestProcess sends an ARP packet and completes normally (no blocking operations found)

When you kill it, the termination path likely triggers a PIT frequency adjustment (via SystemService)

PitSetFrequency uses unconditional cli/sti which can corrupt the interrupt state

The problem occurs because PitSetFrequency (drivers/Pic.c:23-33) does:

asm volatile("cli"); // Disables interrupts // ... PIT programming ... asm volatile("sti"); // ALWAYS re-enables interrupts

If the caller had interrupts disabled (like during process termination), this incorrectly re-enables interrupts, causing race conditions and freezes.

Fix: Update PitSetFrequency to preserve interrupt state:

void PitSetFrequency(uint16_t hz) { // Save current interrupt state irq_flags_t flags = save_irq_flags(); cli(); PIT_FREQUENCY_HZ = hz; // Safer divisor calculation uint32_t div32 = 1193180u / (hz ? hz : 1u); uint16_t divisor = (uint16_t)div32; outb(0x43, 0x36); outb(0x40, divisor & 0xFF); outb(0x40, (divisor >> 8) & 0xFF); // Restore previous interrupt state restore_irq_flags(flags); }

This preserves the interrupt flag state across the PIT frequency change, preventing the corruption that causes the freeze. The ArpRequestTestProcess termination likely triggers a specific timing that exposes this bug.

Would you like me to verify if there are any other unsafe interrupt manipulations in the codebase?

🧬 Code Graph Analysis Results

kernel/atomic/Atomics.c: lines 6-8

void AtomicInc(volatile uint32_t* ptr) { asm volatile("lock incl %0" : "+m" (*ptr)); }

kernel/atomic/Atomics.c: lines 9-11

void AtomicDec(volatile uint32_t* ptr) { asm volatile("lock decl %0" : "+m" (*ptr)); }

kernel/atomic/Atomics.c: lines 12-21

int AtomicCmpxchg(volatile uint32_t* ptr, int expected, int desired) { int old; asm volatile( "lock cmpxchgl %2, %1" : "=a" (old), "+m" (*ptr) : "r" (desired), "0" (expected) : "memory" ); return old; }

kernel/atomic/Atomics.c: lines 22-24

uint32_t AtomicRead(volatile uint32_t* ptr) { return __atomic_load_n(ptr, __ATOMIC_SEQ_CST); }

kernel/atomic/Spinlock.h: lines 19-26

static inline irq_flags_t SpinLockIrqSave(volatile int* lock) { irq_flags_t flags = save_irq_flags(); cli(); while (__sync_lock_test_and_set(lock, 1)) { while (*lock) __builtin_ia32_pause(); } return flags; }

kernel/atomic/Spinlock.h: lines 28-31

static inline void SpinUnlockIrqRestore(volatile int* lock, irq_flags_t flags) { __sync_lock_release(lock); restore_irq_flags(flags); }

kernel/atomic/Spinlock.h: lines 8-12

static inline void SpinLock(volatile int* lock) { while (__sync_lock_test_and_set(lock, 1)) { while (*lock) __builtin_ia32_pause(); } }

kernel/atomic/Spinlock.h: lines 14-16

static inline void SpinUnlock(volatile int* lock) { __sync_lock_release(lock); }

kernel/ memory/VMem.c: lines 336-356

void* VMemAllocWithGuards(uint64_t size) { if (size == 0) return NULL; // Add space for guard pages (one before, one after) uint64_t total_size = size + (2 * PAGE_SIZE); void* base = VMemAlloc(total_size); if (!base) return NULL; uint64_t base_addr = (uint64_t)base; // Map guard pages with no permissions (just present bit) VMemMap(base_addr, VMemGetPhysAddr(base_addr), PAGE_PRESENT); // First guard VMemMap(base_addr + PAGE_SIZE + size, VMemGetPhysAddr(base_addr + PAGE_SIZE + size), PAGE_PRESENT); // Last guard // Fill guard pages with pattern *(uint64_t*)base_addr = GUARD_PAGE_PATTERN; *(uint64_t*)(base_addr + PAGE_SIZE + size) = GUARD_PAGE_PATTERN; return (void*)(base_addr + PAGE_SIZE); // Return user-accessible area }

kernel/ memory/VMem.c: lines 358-374

void VMemFreeWithGuards(void* ptr, uint64_t size) { if (!ptr) return; uint64_t base_addr = (uint64_t)ptr - PAGE_SIZE; // Check guard pages for corruption if (*(uint64_t*)base_addr != GUARD_PAGE_PATTERN) { PrintKernelError("[VMEM] Guard page corruption detected at start!\n"); PANIC("Guard page corruption detected at start"); } if (*(uint64_t*)(base_addr + PAGE_SIZE + size) != GUARD_PAGE_PATTERN) { PrintKernelError("[VMEM] Guard page corruption detected at end!\n"); PANIC("Guard page corruption detected at end"); } VMemFree((void*)base_addr, size + (2 * PAGE_SIZE)); }

kernel/memory/MemOps.c: lines 45-76

void* FastMemset(void* dest, int value, uint64_t size) { ASSERT(dest != NULL); CpuFeatures* features = GetCpuFeatures(); uint8_t* d = (uint8_t*)dest; if (features->sse2 && size >= 16) { // Create a 128-bit value where all bytes are 'value' uint64_t val64 = ((uint64_t)value << 56) | ((uint64_t)value << 48) | ((uint64_t)value << 40) | ((uint64_t)value << 32) | ((uint64_t)value << 24) | ((uint64_t)value << 16) | ((uint64_t)value << 8) | value; asm volatile( "movq %0, %%xmm0\n" "punpcklqdq %%xmm0, %%xmm0\n" "punpcklqdq %%xmm0, %%xmm0\n" : : "r"(val64) : "xmm0" ); while (size >= 16) { asm volatile("movdqu %%xmm0, (%0)" : : "r"(d) : "memory"); d += 16; size -= 16; } } // Handle remaining bytes while (size--) *d++ = value; return dest; }

kernel/memory/MemOps.c: lines 78-101

void* FastMemcpy(void* dest, const void* src, uint64_t size) { ASSERT(dest != NULL && src != NULL); CpuFeatures* features = GetCpuFeatures(); uint8_t* d = (uint8_t*)dest; const uint8_t* s = (const uint8_t*)src; if (features->sse2 && size >= 16) { while (size >= 16) { asm volatile( "movdqu (%1), %%xmm0\n" "movdqu %%xmm0, (%0)\n" : : "r"(d), "r"(st) : "xmm0", "memory" ); d += 16; s += 16; size -= 16; } } while (size--) *d++ = *s++; return dest; }

kernel/etc/Console.c: lines 173-189

void PrintKernel(const char* str) { if (!str) return; SpinLock(&lock); if (use_vbe) { VBEConsolePrint(str); } else { const uint8_t original_color = console.color; for (const char* p = str; *p; p++) { ConsolePutchar(*p); } console.color = original_color; } SpinUnlock(&lock); SerialWrite(str); }

kernel/etc/Console.c: lines 199-203

void PrintKernelSuccess(const char* str) { ConsoleSetColor(VGA_COLOR_WHITE); PrintKernel(str); ConsoleSetColor(VGA_COLOR_DEFAULT); }

kernel/etc/Console.c: lines 205-209

void PrintKernelError(const char* str) { ConsoleSetColor(VGA_COLOR_ERROR); PrintKernel(str); ConsoleSetColor(VGA_COLOR_DEFAULT); }

kernel/etc/Console.c: lines 211-215

void PrintKernelWarning(const char* str) { ConsoleSetColor(VGA_COLOR_WARNING); PrintKernel(str); ConsoleSetColor(VGA_COLOR_DEFAULT); }

kernel/etc/Console.c: lines 218-241

void PrintKernelHex(uint64_t num) { static const char hex_chars[] = "0123456789ABCDEF"; char buffer[19]; buffer[0] = '0'; buffer[1] = 'x'; if (num == 0) { buffer[2] = '0'; buffer[3] = '\0'; PrintKernel(buffer); return; } int pos = 18; buffer[pos--] = '\0'; while (num > 0 && pos >= 2) { buffer[pos--] = hex_chars[num & 0xF]; num >>= 4; } PrintKernel(&buffer[pos + 1]); }

kernel/etc/Shell.c: lines 433-462

void ShellProcess(void) { PrintKernelSuccess("[SYSTEM] VoidFrame Shell v0.0.1-beta\n"); show_help(); while (1) { if (HasInput()) { char c = GetChar(); if (c == '\n') { PrintKernel("\n"); // Move to next line command_buffer[cmd_pos] = 0; ExecuteCommand(command_buffer); cmd_pos = 0; PrintKernel(current_dir); PrintKernel("> "); } else if (c == '\b') { if (cmd_pos > 0) { cmd_pos--; PrintKernel("\b \b"); // Visual backspace } } else if (cmd_pos < 255) { command_buffer[cmd_pos++] = c; char str[2] = {c, 0}; PrintKernel(str); // Echo character } } else { // Yield CPU when no input available Yield(); } } }

drivers/Pic.c: lines 23-33

void PitSetFrequency(uint16_t hz) { // Disable interrupts to ensure atomic update asm volatile("cli"); PIT_FREQUENCY_HZ = hz; // Update the global state const uint16_t divisor = 1193180 / hz; outb(0x43, 0x36); outb(0x40, divisor & 0xFF); outb(0x40, (divisor >> 8) & 0xFF); // Re-enable interrupts asm volatile("sti"); }

include/stdlib.h: lines 4-9

static inline int ABSi(const int x) { if (x < 0) { return -x; } return x; }

oh that fixes can can you consider use fixed precision rather than floating point since floats are 🔥

in the systemservice

@assembler-0, absolutely! Floating-point in kernel space is indeed problematic. Here's a fixed-point implementation that replaces all the float operations:

void SystemService(void) { // Fixed-point arithmetic using scale factor of 1000 (3 decimal precision) #define FP_SCALE 1000 #define FP_HALF (FP_SCALE / 2) PITController controller = { .min_freq = 100, .max_freq = 1000, .current_freq = PIT_FREQUENCY_HZ, .learning_rate = 100, // 0.1 * 1000 .momentum = 700, // 0.7 * 1000 .last_adjustment = 0, .power_state = 0 }; uint64_t last_sample_time = GetSystemTicks(); uint64_t last_context_switches = context_switches; PrintKernelSuccess("[SYSTEM] Advanced frequency controller active (fixed-point)\n"); while (1) { uint64_t current_time = GetSystemTicks(); uint64_t time_delta = current_time - last_sample_time; if (time_delta >= 50) { // Sample every 50 ticks // Gather system metrics int process_count = __builtin_popcountll(active_process_bitmap); int ready_count = __builtin_popcountll(ready_process_bitmap); uint64_t cs_delta = context_switches - last_context_switches; // Calculate system load (0-1000 instead of 0.0-1.0) int32_t load = (ready_count * FP_SCALE) / MAX_PROCESSES; // Calculate context switch rate (scaled by 1000) int32_t cs_rate = (cs_delta * FP_SCALE) / time_delta; // Analyze queue depths uint32_t total_queue_depth = 0; uint32_t max_queue_depth = 0; for (int i = 0; i < MAX_PRIORITY_LEVELS; i++) { uint32_t depth = MLFQscheduler.queues[i].count; total_queue_depth += depth; if (depth > max_queue_depth) max_queue_depth = depth; } // Predictive frequency calculation int32_t target_freq = controller.min_freq; // Factor 1: Process count target_freq += (process_count - 1) * 30; // Factor 2: Queue pressure if (max_queue_depth > 3) { target_freq += max_queue_depth * 20; } // Factor 3: Context switch rate (thrashing detection) if (cs_rate > 10 * FP_SCALE) { // cs_rate > 10.0 // Multiply by 1.2: (freq * 1200) / 1000 target_freq = (target_freq * 1200) / FP_SCALE; } else if (cs_rate < 2 * FP_SCALE && process_count > 2) { // cs_rate < 2.0 // Multiply by 0.9: (freq * 900) / 1000 target_freq = (target_freq * 900) / FP_SCALE; } // Factor 4: Power-aware scaling if (total_queue_depth == 0 && process_count <= 2) { controller.power_state = 0; // Low power target_freq = controller.min_freq; } else if (load > 700) { // load > 0.7 (70%) controller.power_state = 2; // Turbo // Multiply by 1.3: (freq * 1300) / 1000 target_freq = (target_freq * 1300) / FP_SCALE; } else { controller.power_state = 1; // Normal } // Apply momentum for smooth transitions (fixed-point) // adjustment = (target_freq - current_freq) * learning_rate int32_t freq_diff = target_freq - controller.current_freq; int32_t adjustment = (freq_diff * controller.learning_rate) / FP_SCALE; // Add momentum: adjustment + momentum * last_adjustment adjustment = adjustment + (controller.momentum * controller.last_adjustment) / FP_SCALE; controller.last_adjustment = adjustment; int32_t new_freq = controller.current_freq + adjustment; // Clamp to limits if (new_freq < controller.min_freq) new_freq = controller.min_freq; if (new_freq > controller.max_freq) new_freq = controller.max_freq; // Hysteresis - only change if difference is significant if (ABSi(new_freq - controller.current_freq) > 10) { PitSetFrequency((uint16_t)new_freq); controller.current_freq = new_freq; // Log the change if (current_time % 500 == 0) { PrintKernel("[TRACER] Freq: "); PrintKernelInt(new_freq); PrintKernel("Hz, Load: "); PrintKernelInt((load * 100) / FP_SCALE); // Convert back to percentage PrintKernel("%, Power: "); PrintKernelInt(controller.power_state); PrintKernel("\n"); } } // Update history uint32_t idx = controller.history_index % FREQ_HISTORY_SIZE; controller.history[idx] = (FrequencyHistory){ .timestamp = current_time, .process_count = process_count, .frequency = controller.current_freq, .context_switches = cs_delta, .avg_latency = total_queue_depth }; controller.history_index++; last_sample_time = current_time; last_context_switches = context_switches; } CleanupTerminatedProcesses(); Yield(); } #undef FP_SCALE #undef FP_HALF }

Also update the PITController struct definition:

typedef struct { uint16_t min_freq; uint16_t max_freq; int32_t current_freq; int32_t learning_rate; // Fixed-point (scaled by 1000) int32_t momentum; // Fixed-point (scaled by 1000) int32_t last_adjustment; // Fixed-point uint8_t power_state; FrequencyHistory history[FREQ_HISTORY_SIZE]; uint32_t history_index; } PITController;

This eliminates all floating-point operations while maintaining the same behavior with 3-decimal precision using integer math. Much more efficient and kernel-safe! 🚀

✏️ Learnings added

Learnt from: assembler-0 PR: assembler-0/VoidFrame#70 File: kernel/process/Process.c:1224-1228 Timestamp: 2025-08-11T09:51:32.692Z Learning: In the VoidFrame kernel (kernel/process/Process.c), avoid using floating-point arithmetic in kernel space. Use fixed-point arithmetic instead with integer scaling (e.g., multiplying by 1000 for 3 decimal places) to avoid FPU state management overhead and potential exceptions.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@assembler-0, absolutely right! Floating point in kernel space is dangerous and can corrupt FPU state. Here's a fixed-point conversion for SystemService:

void SystemService(void) { // Fixed-point arithmetic using 16.16 format (16 bits integer, 16 bits fraction) #define FP_SHIFT 16 #define FP_ONE (1 << FP_SHIFT) #define FP_HALF (1 << (FP_SHIFT - 1)) typedef struct { uint16_t min_freq; uint16_t max_freq; uint16_t current_freq; int32_t learning_rate; // 0.1 = 6554 in 16.16 fixed point int32_t momentum; // 0.7 = 45875 in 16.16 fixed point int32_t last_adjustment; uint8_t power_state; FrequencyHistory history[FREQ_HISTORY_SIZE]; uint32_t history_index; } PITControllerFixed; PITControllerFixed controller = { .min_freq = 100, .max_freq = 1000, .current_freq = PIT_FREQUENCY_HZ, .learning_rate = 6554, // 0.1 * 65536 .momentum = 45875, // 0.7 * 65536 .last_adjustment = 0, .power_state = 0, .history_index = 0 }; uint64_t last_sample_time = GetSystemTicks(); uint64_t last_context_switches = context_switches; PrintKernelSuccess("[SYSTEM] Advanced frequency controller active (fixed-point)\n"); while (1) { uint64_t current_time = GetSystemTicks(); uint64_t time_delta = current_time - last_sample_time; if (time_delta >= 50) { // Gather system metrics int process_count = __builtin_popcountll(active_process_bitmap); int ready_count = __builtin_popcountll(ready_process_bitmap); uint64_t cs_delta = context_switches - last_context_switches; // Calculate system load (0-100 represented as 0-100) uint32_t load_percent = (ready_count * 100) / MAX_PROCESSES; // Calculate context switch rate (switches per tick * 256 for precision) uint32_t cs_rate_x256 = (cs_delta << 8) / time_delta; // Analyze queue depths uint32_t total_queue_depth = 0; uint32_t max_queue_depth = 0; for (int i = 0; i < MAX_PRIORITY_LEVELS; i++) { uint32_t depth = MLFQscheduler.queues[i].count; total_queue_depth += depth; if (depth > max_queue_depth) max_queue_depth = depth; } // Calculate target frequency using integer math int32_t target_freq = controller.min_freq; // Factor 1: Process count target_freq += (process_count - 1) * 30; // Factor 2: Queue pressure if (max_queue_depth > 3) { target_freq += max_queue_depth * 20; } // Factor 3: Context switch rate (threshold at 10 switches/tick) if (cs_rate_x256 > (10 << 8)) { // High switching target_freq = (target_freq * 6) / 5; // 1.2x boost } else if (cs_rate_x256 < (2 << 8) && process_count > 2) { target_freq = (target_freq * 9) / 10; // 0.9x reduction } // Factor 4: Power-aware scaling if (total_queue_depth == 0 && process_count <= 2) { controller.power_state = 0; target_freq = controller.min_freq; } else if (load_percent > 70) { controller.power_state = 2; target_freq = (target_freq * 13) / 10; // 1.3x turbo } else { controller.power_state = 1; } // Apply momentum using fixed-point math // adjustment = (target - current) * learning_rate + momentum * last_adjustment int32_t freq_diff = target_freq - controller.current_freq; int32_t adjustment_fp = (freq_diff * controller.learning_rate) >> FP_SHIFT; adjustment_fp += (controller.momentum * controller.last_adjustment) >> FP_SHIFT; controller.last_adjustment = adjustment_fp; int16_t new_freq = controller.current_freq + adjustment_fp; // Clamp to limits if (new_freq < controller.min_freq) new_freq = controller.min_freq; if (new_freq > controller.max_freq) new_freq = controller.max_freq; // Hysteresis - only change if difference is significant if (ABSi(new_freq - controller.current_freq) > 10) { PitSetFrequency(new_freq); controller.current_freq = new_freq; // Log changes periodically if (current_time % 500 == 0) { PrintKernel("[TRACER] Freq: "); PrintKernelInt(new_freq); PrintKernel("Hz, Load: "); PrintKernelInt(load_percent); PrintKernel("%, Power: "); PrintKernelInt(controller.power_state); PrintKernel("\n"); } } // Update history uint32_t idx = controller.history_index % FREQ_HISTORY_SIZE; controller.history[idx] = (FrequencyHistory){ .timestamp = current_time, .process_count = process_count, .frequency = controller.current_freq, .context_switches = cs_delta, .avg_latency = total_queue_depth }; controller.history_index++; last_sample_time = current_time; last_context_switches = context_switches; } CleanupTerminatedProcesses(); Yield(); } }

This fixed-point implementation:

Uses 16.16 fixed-point format for fractional values

Replaces all float operations with integer arithmetic

Maintains the same control logic without FPU usage

Is safer and more predictable in kernel space

Avoids potential FPU state corruption issues

✏️ Learnings added

Learnt from: assembler-0 PR: assembler-0/VoidFrame#70 File: kernel/process/Process.c:1224-1228 Timestamp: 2025-08-11T09:51:51.116Z Learning: In VoidFrame kernel, avoid using floating-point arithmetic in kernel space functions like SystemService. Use fixed-point arithmetic instead (e.g., 16.16 format with integer operations) to prevent FPU state corruption, improve performance, and ensure predictable behavior. This is especially important for system-level processes running with PROC_PRIV_SYSTEM privileges.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used

Learnt from: assembler-0 PR: assembler-0/VoidFrame#62 File: kernel/core/Panic.c:69-79 Timestamp: 2025-08-10T06:22:28.788Z Learning: In VoidFrame kernel, panic functions must call "asm volatile("cli")" as the very first instruction to disable interrupts immediately. Without this, if interrupts are enabled (after "sti"), an interrupt can fire during panic handling causing the panic to fail, hang, or behave unpredictably. This must be done in ALL panic entry points: Panic(), PanicWithCode(), PanicWithContext(), PanicFromInterrupt(), and at the start of KernelPanicHandler().

coderabbitai · 2025-08-11T08:48:01Z

💡 Verification agent

🧩 Analysis chain

-nostdlib in compile flags is redundant; verify runtime symbol coverage

With 'clang -c', -nostdlib has no effect (linking is done by ld later). Keeping it is harmless but misleading.

Ensure kernel provides all runtime symbols that would otherwise come from libgcc/libssp (e.g., __stack_chk_fail/guard) given -fstack-protector-strong and no standard libs.

Consider:

Either remove -nostdlib here, or move it to the link step if you ever drive linking via clang in the future.

Run to confirm required symbols are implemented in-tree:

If missing, add minimal implementations (StackGuard.c looks likely to provide these).

🏁 Script executed:

#!/bin/bash set -euo pipefail echo "Checking stack protector symbols..." rg -n --no-heading -S '__stack_chk_fail|__stack_chk_guard' -g '!**/node_modules/**' || true echo echo "Checking memset symbol is provided (to satisfy compiler-emitted calls under -ffreestanding/-fno-builtin)..." rg -n --no-heading -S $'^void\\s*\\*\\s*memset\\s*\\(' || true

Length of output: 968

Remove redundant -nostdlib from compile flags in meson.build

The -nostdlib flag has no effect when you invoke clang with -c —it only matters at link time. Since all runtime symbols (stack protector symbols and memset) are already implemented in-tree, this flag is misleading here.

• File: meson.build – remove '-nostdlib', at line 28
• If you ever switch to driving linking via clang, you can re-introduce -nostdlib in your link flags instead.

🤖 Prompt for AI Agents

In meson.build around line 28, the compile flags incorrectly include '-nostdlib' which is redundant when invoking clang with -c and misleading here; remove the literal '-nostdlib' entry from the compile flags list so it is no longer passed during compilation, and if you later control linking with clang add '-nostdlib' to the link flags instead.

-Original file line number
+Diff line change
@@ -1,7 +1,7 @@
     #ifndef PIC_H
     #define PIC_H
+    #include "stdint.h"
     void PicInstall();
     void PitInstall();
+    void PitSetFrequency(uint16_t hz);
     #endif

-Original file line number
+Diff line change
@@ Expand Up @@
     void VBEShowSplash(void) {
         if (!vbe_initialized) return;
-        for (unsigned int i = 0; i < num_splash_images * 6; i++) { // Loop
+        for (unsigned int i = 0; i < num_splash_images * 4; i++) { // Loop
             const uint32_t* image_data = (const uint32_t*)splash_images[i % num_splash_images];
             for (uint32_t y = 0; y < vbe_info.height; y++) {
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -321,9 +321,10 @@ static InitResultT SystemInitS2(void) { @@
         PrintKernelSuccess("[SYSTEM] System calls initialized\n");
         // Initialize PIC
-        PrintKernel("[INFO] Initializing PIC...\n");
+        PrintKernel("[INFO] Initializing PIC & PIT...\n");
         PicInstall();
-        PrintKernelSuccess("[SYSTEM] PIC initialized\n");
+        PitInstall();
+        PrintKernelSuccess("[SYSTEM] PIC & PIT initialized\n");
         // Initialize keyboard
         PrintKernel("[INFO] Initializing keyboard...\n");
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -7,15 +7,15 @@ @@
     #include "Memory.h"
     #include "PCI/PCI.h"
     #include "PS2.h"
+    #include "Packet.h"
     #include "Panic.h"
     #include "Process.h"
+    #include "RTL8139.h"
     #include "StringOps.h"
     #include "VFS.h"
     #include "VMem.h"
     #include "VesaBIOSExtension.h"
     #include "stdlib.h"
-    #include "Packet.h"
-    #include "RTL8139.h"
     static char command_buffer[256];
     static int cmd_pos = 0;
@@ Expand All / @@ -29,10 +29,7 @@ static void Version() { @@
     void info(void) {
         if (!VBEIsInitialized()) return;
         VBEShowInfo();
-        int count = 10000000;
-        while (--count) {
-            Yield();
-        }
+        Yield();
     }
     static char* GetArg(const char* cmd, int arg_num) {
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -5,9 +5,8 @@ @@
     #include "Ipc.h"
     #include "Cpu.h"
     // =============================================================================
-    // MLFQ SCHEDULER TUNING PARAMETERS
+    // MLFQ SCHEDULER PARAMETERS
     // =============================================================================
     // Core Queue Configuration
@@ Expand Down Expand Up / @@ -83,8 +82,8 @@ @@
     #define FAIRNESS_BOOST_ACTUAL_INTERVAL (FAIRNESS_BOOST_INTERVAL * FAIRNESS_BOOST_MULTIPLIER)
     #define LOAD_BALANCE_ACTUAL_THRESHOLD (LOAD_BALANCE_THRESHOLD * LOAD_BALANCE_MULTIPLIER)
-    #define PROC_PRIV_SYSTEM    0   // Highest privilege (kernel services)
-    #define PROC_PRIV_USER      1   // User processes
+    #define PROC_PRIV_SYSTEM     0   // Highest privilege (kernel services)
+    #define PROC_PRIV_USER       1   // User processes
     #define PROC_PRIV_RESTRICTED 2  // Restricted processes
     #define PIT_BUFF 20
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Development #70

Uh oh!

Diff view

Diff view

There are no files selected for viewing

coderabbitai bot Aug 11, 2025

Uh oh!

coderabbitai bot Aug 11, 2025

Uh oh!

coderabbitai bot Aug 11, 2025 •

edited

Loading

Uh oh!

assembler-0 Aug 11, 2025

Uh oh!

coderabbitai bot Aug 11, 2025

Uh oh!

assembler-0 Aug 11, 2025

Uh oh!

assembler-0 Aug 11, 2025

Uh oh!

coderabbitai bot Aug 11, 2025

Uh oh!

coderabbitai bot Aug 11, 2025

Uh oh!

coderabbitai bot Aug 11, 2025

Uh oh!

-Original file line number
+Diff line change
@@ Expand Up / @@ -20,7 +20,23 @@ void PitInstall() { @@
         outb(0x40, (divisor >> 8) & 0xFF); // High byte
     }
+    void PitSetFrequency(uint16_t hz) {
+        // Save current interrupt state
+        irq_flags_t flags = save_irq_flags();
+        cli();
+        PIT_FREQUENCY_HZ = hz;
+        // Safer divisor calculation
+        uint32_t div32 = 1193180u / (hz ? hz : 1u);
+        uint16_t divisor = (uint16_t)div32;
+        outb(0x43, 0x36);
+        outb(0x40, divisor & 0xFF);
+        outb(0x40, (divisor >> 8) & 0xFF);
+        // Restore previous interrupt state
+        restore_irq_flags(flags);
+    }
     void PicInstall() {
         uint8_t a1, a2;
@@ Expand Down Expand Up / @@ -52,7 +68,4 @@ void PicInstall() { @@
         // Read current mask, clear bit 0, write back
         uint8_t pic1_mask = inb(PIC1_DATA);
         outb(PIC1_DATA, pic1_mask & ~0x01); // Clear bit 0 (IRQ0)
-        // Initialize PIT after PIC
-        PitInstall();
     }

-Original file line number
+Diff line change
@@ Expand Up / @@ -38,6 +38,10 @@ void itoa(uint64_t n, char* buffer) { @@
         strcpy(buffer, p);
     }
+    void* memset(void* dest, int value, unsigned long size) { // FCK GCC
+        return FastMemset(dest, value, size);
+    }
     void* FastMemset(void* dest, int value, uint64_t size) {
         ASSERT(dest != NULL);
         CpuFeatures* features = GetCpuFeatures();
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -25,6 +25,7 @@ c_flags = [ @@
         '-finline-functions',
         '-foptimize-sibling-calls',
         '-nostdinc',
+        '-nostdlib',
         '-fno-builtin',
         '-ffreestanding',
         '-fstack-protector-strong',
@@ Expand Down @@

Development #70

Uh oh!

Development #70

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

coderabbitai bot Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

assembler-0 Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

assembler-0 Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

assembler-0 Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Aug 11, 2025 •

edited

Loading