Skip to content

Conversation

assembler-0
Copy link
Owner

@assembler-0 assembler-0 commented Sep 29, 2025

Summary by CodeRabbit

  • New Features

    • Expanded CPU feature detection with clearer “System:” boot messages.
    • Added stderr support for write syscalls.
  • Bug Fixes

    • Improved system-call gate setup and interrupt handling for greater stability.
    • Refined AHCI disk I/O timing for more reliable reads/writes.
    • Enhanced FAT directory operations and listing behavior.
    • Adjusted shell behavior: cd no longer changes the current directory.
  • Documentation

    • Roadmap phrasing update; structure docs reflect renamed architecture files.
  • Chores

    • Build tweaks: added a compiler flag and switched the default CPU model.

Copy link

coderabbitai bot commented Sep 29, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Refactors CPU feature detection into arch/x86_64/features, updates many includes to x64.h, adjusts build flags/sources, modifies syscall write handling and syscall IDT gate setup, adds AHCI read/write APIs and timing tweaks, expands FAT1x directory ops, updates cli/sti barriers, tweaks SwitchToHigherHalf calling convention, simplifies Kernel ACPI messages, and alters shell cd behavior.

Changes

Cohort / File(s) Summary
CPU feature refactor & includes
arch/x86_64/features/x64.c, arch/x86_64/features/x64.h, drivers/APIC.c, drivers/TSC.c, drivers/Vesa.c, kernel/sched/MLFQ.c, kernel/sched/MLFQ.h, mm/MemOps.c, mm/PageFaultHandler.h, mm/trace/StackTrace.h, docs/STRUCTURE.md, meson.build
New CPUFeatureValidation with CPUID; expanded CpuFeatures (sse3/ssse3/sse41/sse42/bmi1/bmi2/fma); replace Cpu.h with x64.h across code; build now uses features path; run CPU model set to max; -mserialize added.
Syscall and IDT updates
arch/x86_64/idt/Idt.c, arch/x86_64/syscall/Syscall.c
Syscall gate now uses kernelCodeSegment/flags; SYS_WRITE now handles fd 2 via PrintKernelError; stdout unchanged; others return -1.
AHCI storage enhancements
drivers/storage/AHCI.c
Adds AHCI_ReadSectors/AHCI_WriteSectors; wrappers adjust port indexing (i+1 / -1); logs on IO; replaces delay() with delay_us() in waits; integrates new APIs.
FAT1x filesystem changes
fs/FAT/FAT1x.c
Adjusted detection returns/logging; adds Fat1xIsDirectory/Fat1xListDirectory; updates directory/file ops and traversal/entry-finding logic.
Low-level barriers
include/Io.h
cli()/sti() gain hot/flatten, barriers, and serialize; signatures drop void parameter list.
Switch calling convention
include/Switch.h
SwitchToHigherHalf now asmlinkage; adds Kernel.h and stdint.h includes.
Kernel init changes
kernel/core/Kernel.c
Removes CPUFeatureValidation block; simplifies ACPI init messages.
Shell behavior
kernel/etc/Shell.c
cd no longer updates current directory; only errors on non-directory.
Docs editorial
docs/ROADMAP.md
Phrasing change: “Bidirectional filesystem handling.”

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant App as User App
  participant Kernel as Syscall Handler
  participant Out as PrintKernel / PrintKernelError
  User->>App: write(fd, buf, count)
  App->>Kernel: SYS_WRITE(fd, buf, count)
  alt fd == 1 (stdout)
    Kernel->>Out: PrintKernel(buf, count)
    Out-->>Kernel: ok
    Kernel-->>App: return count
  else fd == 2 (stderr)
    Kernel->>Out: PrintKernelError(buf, count)
    Out-->>Kernel: ok
    Kernel-->>App: return count
  else other fd
    Kernel-->>App: return -1
  end
Loading
sequenceDiagram
  autonumber
  participant VFS as BlockDevice Wrapper
  participant AHCI as AHCI_Read/WriteSectors
  participant Port as AHCI Port
  participant DMA as HBA DMA Engine
  VFS->>AHCI: ReadSectors/WriteSectors(port, lba, count, buf)
  AHCI->>Port: Validate init/idle
  AHCI->>Port: AHCI_SendCommand(READ/WRITE_DMA_EX)
  Port->>DMA: Program PRDT / issue FIS
  DMA-->>Port: IRQ/complete
  Port-->>AHCI: status
  AHCI-->>VFS: result
Loading
sequenceDiagram
  autonumber
  participant Boot as CpuInit
  participant CPU as CPUID
  Boot->>CPU: Query features
  CPU-->>Boot: Set sse3/ssse3/sse41/sse42/bmi1/bmi2/fma/...
  alt OSXSAVE not supported
    Boot->>CPU: CPUFeatureValidation()
    CPU-->>Boot: Re-validated flags
  end
  Boot->>CPU: CPUFeatureValidation() (final)
  CPU-->>Boot: Confirmed feature state
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • Development #116 — Introduces AHCI driver APIs overlapping with new AHCI_ReadSectors/AHCI_WriteSectors.
  • Development #5 — Alters syscall IDT gate setup similar to this PR’s Idt.c change.
  • Development #44 — Adjusts CPU feature detection flow related to this refactor into x64.c/x64.h.

Poem

A rabbit taps the kernel’s keys,
Flips CPUIDs on autumn breeze.
AHCI carts the sectors through,
While stderr gets its crimson hue.
Directories hop, yet cd stays still—
Tiny fences guard the mill.
Onward, bits! Let’s burrow and build. 🐇💾

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch Development

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fd1f1a6 and 322511f.

📒 Files selected for processing (21)
  • arch/x86_64/features/x64.c (4 hunks)
  • arch/x86_64/features/x64.h (1 hunks)
  • arch/x86_64/idt/Idt.c (1 hunks)
  • arch/x86_64/syscall/Syscall.c (1 hunks)
  • docs/ROADMAP.md (1 hunks)
  • docs/STRUCTURE.md (1 hunks)
  • drivers/APIC.c (1 hunks)
  • drivers/TSC.c (1 hunks)
  • drivers/Vesa.c (1 hunks)
  • drivers/storage/AHCI.c (8 hunks)
  • fs/FAT/FAT1x.c (1 hunks)
  • include/Io.h (1 hunks)
  • include/Switch.h (1 hunks)
  • kernel/core/Kernel.c (1 hunks)
  • kernel/etc/Shell.c (1 hunks)
  • kernel/sched/MLFQ.c (1 hunks)
  • kernel/sched/MLFQ.h (1 hunks)
  • meson.build (5 hunks)
  • mm/MemOps.c (1 hunks)
  • mm/PageFaultHandler.h (1 hunks)
  • mm/trace/StackTrace.h (1 hunks)

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@assembler-0 assembler-0 merged commit bd86031 into main Sep 29, 2025
0 of 2 checks passed
This was referenced Oct 2, 2025
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.

1 participant