Skip to content

Add PIT Timer, IDT Fixes, and Deferred Kernel Timing Output#14

Merged
LordHerdier merged 7 commits intomainfrom
IDT-Skeleton
Apr 1, 2026
Merged

Add PIT Timer, IDT Fixes, and Deferred Kernel Timing Output#14
LordHerdier merged 7 commits intomainfrom
IDT-Skeleton

Conversation

@amorrisCS
Copy link
Copy Markdown
Collaborator

This PR introduces PIT-based timing support and integrates it with the kernel, along with fixes to IDT handling and timing correctness.

Features

  • Adds PIT (Programmable Interval Timer) initialization at configurable frequency (~1000 Hz)
  • Implements IRQ0 handler and tick counter
  • Provides kernel_get_ticks_ms() for millisecond timing

Key Fixes
IDT segment selector fix:

  • Removed hardcoded 0x08
  • Now reads %cs at runtime in idt_init() to prevent GP faults on iret

Timer overflow fix:

  • Resolved uint32_t overflow in kernel_get_ticks_ms() that caused wraparound after ~3.6 seconds

PIT divisor bug:

  • Fixed shadowed variable so timing calculations use the correct divisor

Design Improvements

  • Moved serial output out of IRQ context:
  • IRQ handler sets a print_pending flag
  • Printing handled in kernel_main
  • Keeps interrupt handler fast and avoids timing drift
  • Added safe uint32_t serial formatting via pre-buffered output

Other

  • Cleaned up unused includes
  • Rebased onto main to resolve merge conflicts

Result

  • Stable interrupt returns (no GP faults)
  • Accurate and consistent time tracking
  • Safe, minimal interrupt handler design
  • Clean base for future scheduling/timing features

amorrisCS and others added 7 commits April 1, 2026 08:47
Modified: exodoom/docker/scripts/build.sh
exodoom/src/kernel.c

New:
exodoom/src/idt.c
exodoom/src/idt.h
exodoom/src/isr.s
exodoom/src/pic.c
exodoom/src/pic.h
exodoom/src/pit.c
exodoom/src/pit.h
Improved upon idt.c such that all flags will no longer be set to present.

Improved upon pit.c such that buf is now null terminated.

Function names and calls converted to relative naming scheme
- Configured PIT to 1000hz and implemented kernel_get_ticks_ms().

- Implemented kernel_sleep_ms() w/ busy-wait on tick counter. A test case was added to kernel.c to assess the accuracy of kernel sleep timings.

- Additionally, all header files created for the IDT set up were changed to include "#pragma once", rather than ifndef -> define -> endif

- Github Desktop seems to HATE when files are not ended with a newline, so I went through and added a newline at the end of all the files I created that didn't have one.
Miscellaneous fixes to multiple files and refactored formatting to align with the rest of the project.
Compile error stemming from kernel.c:74, call to non-existent serial_printf() - now using a series of serial_print().

Shadow Variable Bug in pit.c:13 stemming from a re-declaration of divisor variable, leading to the global divisor variable becoming stale - now accurately manipulating global variable.

Unused header "serial.h" within pit.c - now removed
- Drop AOUT_KLUDGE flag from Multiboot header; zero reserved fields so
  GRUB loads the ELF correctly instead of misinterpreting load addresses
- Read %cs at runtime in idt_init() rather than hardcoding 0x08, fixing
  GP-faults on iret out of interrupt handlers
- Fix uint32_t overflow in kernel_get_ticks_ms() (ticks*divisor*1000
  wrapped at ~3.6s); use (uint64_t)ticks*1000/frequency instead
- Move qemu_exit after the ms-printing loop so monotonic counter is
  visible in CI output
- Add serial_print_u32(); remove dead timer_ticks() export
- Fix ICW1_ICW4 comment; remove stale docker-test from .PHONY
@amorrisCS amorrisCS requested a review from LordHerdier April 1, 2026 14:55
@LordHerdier
Copy link
Copy Markdown
Owner

This looks really good! Thank you for all of your work on the interrupt handling!

I'm approving this PR to merge with a couple of notes about bugs that will need to be fixed in the future:

Bugs

isr.s:12 : default_stub is unsafe for exception vectors that push an error code

default_stub just does a bare iret, but CPU exception vectors 8, 10–14, 17, 21, 29, and 30 push an error code onto the stack before entering the handler. A bare iret from those vectors pops the error code as the return %eip, misaligns the stack, and will likely triple-fault. The stub is currently installed on all 256 IDT entries, so any one of those exceptions firing (e.g. a page fault during early init) will cause a hard crash instead of a clean fault. Should at minimum have a separate error_stub that pops the error code before iret, and install it on the appropriate vectors.

Minor Issues

  1. idt_set_gate has a hidden dependency on idt_init() having been called
    idt_set_gate uses kernel_cs (idt.c:48) which is only populated inside idt_init(). Calling idt_set_gate before
    idt_init() silently installs gates with selector 0, which would GP-fault on any interrupt. The dependency isn't
    documented and isn't enforced. At minimum a comment on idt_set_gate saying "must be called after idt_init()" would
    help. A future refactor could fold gate setup into idt_init() where the call ordering is explicit.

  2. build.sh special-cases isr.s rather than handling all .s files generically
    The existing loop assembles .c files; there's a separate if [ -f src/isr.s ] block for assembly. If more .s files are
    added later, each would need another special case. A glob over src/
    .s (matching how src/*.c is handled) would be
    more maintainable.

  3. C-style () vs (void) in new function declarations
    void idt_init(), void pic_remap(), etc. — in C (not C++), empty parens mean "unspecified parameters", not "no
    parameters". All the new functions that take no arguments should use (void). This is consistent with what the existing
    codebase does (e.g. serial_init(void)).

  4. Comment style inconsistency
    The existing codebase uses /* */ block comments; new code in kernel.c uses // line comments. Minor, but worth keeping
    consistent.


I'm going to create a couple Jira tasks to tackle these that we can both work on. For now though, this implementation is good!

@LordHerdier LordHerdier merged commit 4ef9d7e into main Apr 1, 2026
2 checks passed
@LordHerdier LordHerdier deleted the IDT-Skeleton branch April 1, 2026 15:36
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.

2 participants