Skip to content

Feature: Add boot loader #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
imsudiproy wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Feature: Add boot loader #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b2f60d
initial commit
imsudiproy Nov 21, 2025
9f32641
update project sctucture
imsudiproy Nov 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Notes for our reference. We will remove this before merging to master.
# multiboot_header.S is needed to make your binary Multiboot2-compliant.
# Use x86_64-elf-* toolchain or adapt to your toolchain names.
AS = x86_64-elf-as
CC = x86_64-elf-gcc
LD = x86_64-elf-ld
OBJCOPY = x86_64-elf-objcopy

CFLAGS = -m64 -ffreestanding -O2 -Wall -Wextra -fno-pic -fno-stack-protector -fno-builtin
LDFLAGS = -T link.ld

all: iso/boot/kernel.elf plug-os.iso
Copy link

@coderabbitai coderabbitai bot Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Inconsistent ISO filename.

The all target references plug-os.iso but the clean target removes os.iso, and the os.iso target creates os.iso. This inconsistency will cause the build to fail.

Apply this diff to use consistent naming:

-all: iso/boot/kernel.elf plug-os.iso
+all: iso/boot/kernel.elf os.iso
📝 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
all: iso/boot/kernel.elf plug-os.iso
all: iso/boot/kernel.elf os.iso
🧰 Tools
🪛 checkmake (0.2.2)

[warning] 12-12: Target "all" should be declared PHONY.

(phonydeclared)

🤖 Prompt for AI Agents 
Makefile lines ~12 (around startLine 12): the all target refers to plug-os.iso
while the iso target and clean target use os.iso, causing inconsistent
filenames; pick a single ISO name (e.g., plug-os.iso) and update the iso target,
its prerequisites/dependencies, and the clean target to produce/remove the same
filename: rename the os.iso target and any occurrences of os.iso to plug-os.iso
so all targets and clean use the identical filename.


kernel.o: kernel.c
$(CC) $(CFLAGS) -c kernel.c -o kernel.o

entry.o: entry.S
$(AS) --64 entry.S -o entry.o

multiboot_header.o: multiboot_header.S
$(AS) multiboot_header.S -o multiboot_header.o
Comment on lines +20 to +21
Copy link

@coderabbitai coderabbitai bot Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Incorrect source path for multiboot_header.S

The source file path is missing the directory prefix. Based on the project structure, the file is located at boot/multiboot/multiboot_header.S.

Apply this diff to fix the path:

-multiboot_header.o: multiboot_header.S
-	$(AS) multiboot_header.S -o multiboot_header.o
+multiboot_header.o: boot/multiboot/multiboot_header.S
+	$(AS) --64 boot/multiboot/multiboot_header.S -o multiboot_header.o

Note: Also added the --64 flag for consistency with the entry.o target at line 18.

📝 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
multiboot_header.o: multiboot_header.S
$(AS) multiboot_header.S -o multiboot_header.o
multiboot_header.o: boot/multiboot/multiboot_header.S
$(AS) --64 boot/multiboot/multiboot_header.S -o multiboot_header.o
🤖 Prompt for AI Agents 
In Makefile around lines 20 to 21, the rule for building multiboot_header.o uses
an incorrect source path and missing assembler flag; change the dependency to
boot/multiboot/multiboot_header.S and invoke the assembler with the same --64
flag used for entry.o (e.g. $(AS) --64 boot/multiboot/multiboot_header.S -o
multiboot_header.o) so the correct file is assembled in 64-bit mode.


kernel.elf: multiboot_header.o entry.o kernel.o
$(LD) $(LDFLAGS) -o kernel.elf multiboot_header.o entry.o kernel.o

iso/boot/kernel.elf: kernel.elf
mkdir -p iso/boot/grub
cp kernel.elf iso/boot/
cp grub.cfg iso/boot/grub/
Comment on lines +26 to +29
Copy link

@coderabbitai coderabbitai bot Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Incorrect path to grub.cfg

The grub.cfg file is located at boot/grub/grub.cfg according to the project structure, not at the root directory.

Apply this diff:

 iso/boot/kernel.elf: kernel.elf
 	mkdir -p iso/boot/grub
 	cp kernel.elf iso/boot/
-	cp grub.cfg iso/boot/grub/
+	cp boot/grub/grub.cfg iso/boot/grub/
📝 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
iso/boot/kernel.elf: kernel.elf
mkdir -p iso/boot/grub
cp kernel.elf iso/boot/
cp grub.cfg iso/boot/grub/
iso/boot/kernel.elf: kernel.elf
mkdir -p iso/boot/grub
cp kernel.elf iso/boot/
cp boot/grub/grub.cfg iso/boot/grub/
🤖 Prompt for AI Agents 
In Makefile around lines 26 to 29, the rule copies grub.cfg from the repository
root but the correct source path is boot/grub/grub.cfg; update the cp command to
copy from boot/grub/grub.cfg into iso/boot/grub/ (ensure destination dir exists
as already done) so the rule uses the correct source path.


os.iso:
grub-mkrescue -o os.iso iso/ # on some systems you may need grub2-mkrescue
Comment on lines +31 to +32
Copy link

@coderabbitai coderabbitai bot Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Major: Missing dependency causes unreliable builds.

The os.iso target doesn't declare its dependency on iso/boot/kernel.elf, which means the ISO might be created before the kernel is copied, resulting in an incomplete or outdated ISO image.

Apply this diff:

-os.iso:
+os.iso: iso/boot/kernel.elf
 	grub-mkrescue -o os.iso iso/    # on some systems you may need grub2-mkrescue
📝 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
os.iso:
grub-mkrescue -o os.iso iso/ # on some systems you may need grub2-mkrescue
os.iso: iso/boot/kernel.elf
grub-mkrescue -o os.iso iso/ # on some systems you may need grub2-mkrescue
🤖 Prompt for AI Agents 
In Makefile around lines 31 to 32, the os.iso target lacks an explicit
dependency on iso/boot/kernel.elf so the ISO can be built before the kernel is
copied; update the os.iso rule to depend on iso/boot/kernel.elf (and any other
files under iso/ that must exist) so Make rebuilds the ISO only after the kernel
is produced and placed into iso/boot; ensure the dependency list includes
iso/boot/kernel.elf and any other generated boot files and keep the
grub-mkrescue command as the recipe.


clean:
rm -rf *.o kernel.elf iso os.iso
Empty file added boot/entry/entry64.S
View file
Open in desktop
Empty file.
9 changes: 5 additions & 4 deletions boot/grub/grub.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
set timeout=0
set timeout=3
set default=0

menuentry "My OS" {
multiboot2 /boot/kernel.elf
boot
menuentry "plugOS (multiboot2)" {
multiboot2 /boot/kernel.elf
module2 /boot/initramfs.img # optional
boot
}
12 changes: 12 additions & 0 deletions boot/multiboot/multiboot_header.S
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
.section .multiboot, "a"
.align 8
.long 0xE85250D6 /* multiboot2 magic */
.long 0 /* architecture (0 = i386) - fine for GRUB */
.long multiboot_header_end - multiboot_header /* header length placeholder */
.long 0 /* checksum placeholder (computed at link or filled later) */
Comment on lines +3 to +6
Copy link

@coderabbitai coderabbitai bot Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Critical: Invalid checksum will prevent boot.

The checksum at line 6 is set to 0 as a placeholder, but the Multiboot2 specification requires a valid checksum calculated as -(magic + architecture + header_length). With an invalid checksum, the bootloader will reject this header and fail to boot.

Additionally, the multiboot_header label is missing before line 3, which is referenced in the header length calculation at line 5.

Apply this diff to fix both issues:

     .section .multiboot, "a"
     .align 8
+multiboot_header:
     .long 0xE85250D6           /* multiboot2 magic */
     .long 0                      /* architecture (0 = i386) - fine for GRUB */
     .long multiboot_header_end - multiboot_header /* header length placeholder */
-    .long 0                      /* checksum placeholder (computed at link or filled later) */
+    .long -(0xE85250D6 + 0 + (multiboot_header_end - multiboot_header)) /* checksum */

Run the following script to verify the multiboot2 specification requirements:

🌐 Web query:

Multiboot2 specification checksum calculation requirements

💡 Result:

Summary — Multiboot2 header checksum requirements

  • The Multiboot2 header is: magic (u32), architecture (u32), header_length (u32), checksum (u32), followed by tags. The header must be entirely within the first 32768 bytes of the image and 64‑bit aligned. [1][2]

  • The checksum is a 32‑bit unsigned value chosen so that the 32‑bit unsigned sum of magic + architecture + header_length + checksum equals 0. Equivalently:
    checksum = (−(magic + architecture + header_length)) mod 2^32. [1][2]

References
[1] Multiboot2 Specification (Header layout and magic fields). [2] GNU Multiboot2 manual — Header magic fields.

🏁 Script executed:

#!/bin/bash
# Verify the multiboot_header.S file content
fd -t f "multiboot_header.S" | head -5 | while read file; do
  echo "=== File: $file ==="
  cat "$file"
done

Length of output: 631

Critical issues confirmed: missing label and invalid checksum.

The actual file confirms both issues identified in the review:

  1. The multiboot_header label is indeed missing before line 3. Line 5 references this non-existent label, which would cause an assembler error or incorrect offset calculation.

  2. The checksum at line 6 is set to 0, which violates the Multiboot2 specification. The checksum must be a 32-bit unsigned value chosen so that the 32-bit unsigned sum of magic + architecture + header_length + checksum equals 0.

The proposed diff correctly:

  • Adds the missing multiboot_header: label before the magic number
  • Calculates the checksum as -(0xE85250D6 + 0 + (multiboot_header_end - multiboot_header)), which matches the specification requirement

Apply the provided diff to fix both issues.

🤖 Prompt for AI Agents 
In boot/multiboot/multiboot_header.S around lines 3-6, the multiboot_header
label is missing and the checksum is currently 0; add the missing label
"multiboot_header:" immediately before the magic .long entry, and replace the
zero checksum with the computed 32‑bit value equal to the two's‑complement
negation of (magic + architecture + header_length) using an assembler expression
that uses (multiboot_header_end - multiboot_header) for header_length so the sum
of magic, architecture, header_length and checksum equals 0.


/* --- tags go here (info request, entry address, etc). Example: end tag --- */
.long 0 /* tag type = end (0) */
.long 8 /* size = 8 (tag header size) */

multiboot_header_end:
1 change: 1 addition & 0 deletions boot/multiboot/multiboot_specs.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Here we will store our notes
72 changes: 72 additions & 0 deletions docs/project_structure.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
myos/
│
├── build/ # All temporary/generated build artifacts go here.
│ ├── iso/ # GRUB-ready ISO tree (boot/, grub/, kernel.elf, etc.)
│ └── obj/ # Object files (.o)
│
├── boot/ # Bootloader-related code
│ ├── grub/ # GRUB config
│ │ └── grub.cfg
│ ├── multiboot/ # Multiboot2 header and support files
│ │ ├── multiboot_header.S
│ │ └── multiboot_specs.txt # (optional notes for yourself)
│ └── entry/ # CPU entry, long-mode switch, early platform code
│ ├── entry32.S
│ └── entry64.S
│
├── kernel/ # Kernel proper
│ ├── arch/ # Architecture-specific code (x86_64)
│ │ └── x86_64/
│ │ ├── mm/ # Paging, PML4 setup, frame allocator
│ │ │ ├── paging.c
│ │ │ └── paging.h
│ │ ├── interrupts/
│ │ │ ├── idt.c
│ │ │ ├── idt.h
│ │ │ ├── isr.S
│ │ │ └── irq.c
│ │ ├── gdt/
│ │ │ ├── gdt.c
│ │ │ └── gdt.h
│ │ ├── cpufeatures.c
│ │ └── cpufeatures.h
│ │
│ ├── drivers/ # Hardware drivers (framebuffer, keyboard, rtc…)
│ │ ├── framebuffer.c
│ │ ├── keyboard.c
│ │ └── pci.c
│ │
│ ├── fs/ # Filesystems (VFS, ext2, initrd parsing)
│ │ ├── vfs.c
│ │ └── vfs.h
│ │
│ ├── lib/ # Kernel-side libs (string/mem, logging, util)
│ │ ├── string.c
│ │ ├── memory.c
│ │ └── panic.c
│ │
│ ├── scheduler/ # (Future) tasking, context switch, user mode
│ │ ├── task.c
│ │ └── task.h
│ │
│ ├── kernel.c # Main entry point (kernel_main)
│ └── kernel.h
│
├── scripts/ # Utility scripts for building/toolchain/ISO
│ ├── build_iso.sh
│ ├── run_qemu.sh
│ └── debug_gdb.sh
│
├── toolchain/ # Cross-compiler build scripts or prebuilts
│ ├── build_gcc.sh
│ ├── build_binutils.sh
│ └── README.md
│
├── include/ # Shared headers
│ ├── stdint.h
│ ├── stdbool.h
│ └── common.h
│
├── link.ld # Kernel linker script
├── Makefile # Top-level build orchestration
└── README.md