From 711162f55bc22053eef9b4b8e0473784c9e36be2 Mon Sep 17 00:00:00 2001 From: Mark Date: Tue, 4 Mar 2025 19:14:38 -0800 Subject: [PATCH] Comments --- Makefile | 48 +++--------- README.md | 3 - bios/main.asm | 59 +++++++------- bios/print.asm | 29 +++---- bios/protected_mode.asm | 46 ----------- bios/stage1.asm | 121 +++++++++++++++-------------- bios/stage2.asm | 50 ++++++++++-- tetros/Cargo.toml | 2 +- tetros/linkers/x86-unknown-none.ld | 5 +- tetros/src/drivers/pic.rs | 30 +++++-- tetros/src/drivers/serial.rs | 7 ++ tetros/src/drivers/vga.rs | 4 +- tetros/src/game/board.rs | 46 ++++++----- tetros/src/game/falling.rs | 22 ++---- tetros/src/game/mod.rs | 3 + tetros/src/idt/stackframe.rs | 8 +- tetros/src/idt/table.rs | 20 ++--- tetros/src/idt/virtaddr.rs | 42 +--------- tetros/src/lib.rs | 51 +++++++++--- tetros/src/os/eflags.rs | 1 + tetros/src/os/panic.rs | 6 +- tetros/src/os/util.rs | 8 +- 22 files changed, 295 insertions(+), 316 deletions(-) delete mode 100644 bios/protected_mode.asm diff --git a/Makefile b/Makefile index 45f5b6c..c10cdec 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,7 @@ BUILD=./build # Default rule .PHONY: default -default: all +default: $(BUILD)/disk.img # Remove all build files .PHONY: clean @@ -10,23 +10,18 @@ clean: rm -drf $(BUILD) cd tetros; cargo clean -# Make everything -# (but don't run qemu) -.PHONY: all -all: img - # -# MARK: boot +# MARK: disk # -# Compile tetros as library +# Compile tetros as a library +# (so that we can link it with a custom linker script) LIB_SRC = ./tetros/Cargo.toml ./tetros/Cargo.lock $(shell find ./tetros/src -type f) $(BUILD)/tetros.lib: $(LIB_SRC) @mkdir -p $(BUILD) cd tetros && \ env RUSTFLAGS="-C soft-float" \ cargo rustc \ - --manifest-path="./Cargo.toml" \ -Z build-std=core \ -Z build-std-features=compiler-builtins-mem \ --target "./targets/x86-unknown-none.json" \ @@ -35,7 +30,7 @@ $(BUILD)/tetros.lib: $(LIB_SRC) -- \ --emit link="$(CURDIR)/$@" -# Link tetros +# Link tetros using custom linker script BIOS_LD = ./tetros/linkers/x86-unknown-none.ld $(BUILD)/tetros.elf: $(BUILD)/tetros.lib $(BIOS_LD) ld \ @@ -49,13 +44,13 @@ $(BUILD)/tetros.elf: $(BUILD)/tetros.lib $(BIOS_LD) objcopy --only-keep-debug "$@" "$@.sym" objcopy --strip-debug "$@" -# Wrap tetros in three-stage BIOS loader +# Wrap tetros in BIOS loader # Parameters: # - BIOS_SRC: source directory of bios assembly # - STAGE2_SECTOR: the index of the first sector of the stage 2 binary on the disk BIOS_SRC = ./bios STAGE2_SECTOR = 1 -$(BUILD)/bios.bin: $(wildcard $(BIOS_SRC)/*.asm) $(BUILD)/tetros.elf +$(BUILD)/disk.img: $(wildcard $(BIOS_SRC)/*.asm) $(BUILD)/tetros.elf @mkdir -p "$(BUILD)" nasm \ -f bin \ @@ -66,30 +61,12 @@ $(BUILD)/bios.bin: $(wildcard $(BIOS_SRC)/*.asm) $(BUILD)/tetros.elf -i "$(BIOS_SRC)" \ "$(BIOS_SRC)/main.asm" -# Extract full mbr (first 512 bytes) -$(BUILD)/mbr.bin: $(BUILD)/bios.bin - @mkdir -p "$(BUILD)" - @echo "" - dd if="$<" bs=512 count=1 of="$@" - -# Extract stage 2 (rest of file) -$(BUILD)/stage2.bin: $(BUILD)/bios.bin - @mkdir -p "$(BUILD)" - @echo "" - dd if="$<" bs=512 skip=1 of="$@" - # -# MARK: bundle +# MARK: qemu +# +# Do not use `-enable-kvm` or `-cpu host`, +# this confuses gdb. # - -# Make full disk image -.PHONY: img -img: $(BUILD)/disk.img -$(BUILD)/disk.img: $(BUILD)/mbr.bin $(BUILD)/stage2.bin - @mkdir -p $(BUILD) - @echo "" - dd if="$(BUILD)/mbr.bin" of=$@ conv=notrunc bs=512 - dd if="$(BUILD)/stage2.bin" of=$@ conv=notrunc seek=$(STAGE2_SECTOR) bs=512 .PHONY: qemu qemu: $(BUILD)/disk.img @@ -128,6 +105,3 @@ qemu-gdb: $(BUILD)/disk.img -fda "$<" \ -gdb tcp::26000 \ -S - -# Do not use `-enable-kvm` or `-cpu host`, -# this confuses gdb. \ No newline at end of file diff --git a/README.md b/README.md index 0b0b32b..f33206e 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,5 @@ # TetrOS: bare-metal tetris -## TODO: -- Fix stage 1 loader - ## Features - Compiles to a standalone disk image - Written from scratch using only Nasm and Rust diff --git a/bios/main.asm b/bios/main.asm index 50c62ba..3193bb7 100644 --- a/bios/main.asm +++ b/bios/main.asm @@ -1,30 +1,25 @@ sectalign off -; This program expects two external macros: +; The following code expects two external macros: ; STAGE3, a path to the stage3 binary ; STAGE2_SECTOR, the location of stage 2 ; on the disk, in 512-byte sectors. -; On a gpt disk, this is probably 34. - -; Stage 1 is MBR code, and should fit in LBA 0 -; (512 bytes). Layout is as follows: -; (Format is `offset, length: purpose`) -; 0, 424: x86 boot code -; 440, 4: Unique disk signature -; 444, 2: unknown -; 446, 16*4: Array of four legacy MBR records -; 510, 2: signature 0x55 0xAA -; 512 to end of logical block: reserved -; -; See https://uefi.org/specs/UEFI/2.10/05_GUID_Partition_Table_Format.html +; +; Both of these are set in the makefile. +; BIOS loads stage 1 at 0x7C00 ORG 0x7C00 SECTION .text -; stage 1 is sector 0, loaded into memory at 0x7C00 +; Stage 1 is MBR code, and should fit in LBA 0 +; (i.e, in the first 512 bytes). %include "stage1.asm" ; Stage 1 is at most 440 bytes +; This limit is set by the GPT spec. +; See https://uefi.org/specs/UEFI/2.10/05_GUID_Partition_Table_Format.html +; +; This `times` will throw an error if the subtraction is negative. times 440-($-$$) db 0 db 0xee @@ -32,30 +27,38 @@ db 0xee times 510-($-$$) db 0 ; MBR signature. -; This isn't loaded into memory, it's -; only here for debugging. +; This tells the BIOS that this disk is bootable. db 0x55 db 0xaa + +; Include stage 2. This is loaded into memory by stage 1. +; (stage 1 loads both stage 2 and stage 3) +; +; Stage 2 sets up protected mode, sets up the GDT, +; and initializes a minimal environment for stage 3. +; +; On a "real" boot disk, this data will not immediately follow stage 1. +; It would be stored in a special disk partition. +; +; We don't need this kind of complexity here, though, so we store +; stage 2 right after stage 1. (This is why STAGE2_SECTOR is 1.) +; +; This is nice, because the layout of the code on our boot disk +; matches the layout of the code in memory. THIS IS NOT USUALLY THE CASE. stage2: %include "stage2.asm" align 512, db 0 stage2.end: -; The maximum size of stage2 is 4 KiB, -; This fill will throw an error if the subtraction is negative. -times (4*1024)-($-stage2) db 0 - -; Pad to 0x9000. -; This needs to match the value configured in the stage3 linker script -times (0x9000 - 0x7c00)-($-$$) db 0 +; Pad to 0x3000. +; This makes sure that state3 is loaded at the address +; the linker expects. Must match the value in `tetros/linkers/x86-unknown-none.ld`. +times (0x8000 - 0x7c00)-($-$$) db 0 +; Include stage 3, the binary compiled from Rust sources. stage3: %defstr STAGE3_STR %[STAGE3] incbin STAGE3_STR align 512, db 0 .end: - -; TODO: why? Of the disk, or of memory? -; the maximum size of the boot loader portion is 384 KiB -times (384*1024)-($-$$) db 0 diff --git a/bios/print.asm b/bios/print.asm index 5e958a3..ac8e111 100644 --- a/bios/print.asm +++ b/bios/print.asm @@ -1,22 +1,21 @@ SECTION .text USE16 -; provide function for printing in x86 real mode - -; print a string and a newline -; CLOBBER -; ax +; Print a string and a newline +; +; Clobbers ax print_line: mov al, 13 call print_char mov al, 10 jmp print_char -; print a string -; IN +; Print a string +; +; Input: ; si: points at zero-terminated String -; CLOBBER -; si, ax +; +; Clobbers si, ax print: pushf cld @@ -30,8 +29,9 @@ print: popf ret -; print a character -; IN +; Print a character +; +; Input: ; al: character to print print_char: pusha @@ -42,10 +42,11 @@ print_char: ret ; print a number in hex -; IN +; +; Input: ; bx: the number -; CLOBBER -; al, cx +; +; Clobbers al, cx print_hex: mov cx, 4 .lp: diff --git a/bios/protected_mode.asm b/bios/protected_mode.asm deleted file mode 100644 index 1df111f..0000000 --- a/bios/protected_mode.asm +++ /dev/null @@ -1,46 +0,0 @@ -SECTION .text -USE16 - -protected_mode: - -.func: dd 0 - -.entry: - ; disable interrupts - cli - - ; load protected mode GDT - lgdt [gdtr] - - ; set protected mode bit of cr0 - mov eax, cr0 - or eax, 1 - mov cr0, eax - - ; far jump to load CS with 32 bit segment - ; (we are in 32-bit mode, but instruction pipeline - ; has 16-bit instructions. - jmp gdt.pm32_code:.inner - - ; gdt.pm32_code is a multiple of 8, so it always ends with three zero bits. - ; The GDT spec abuses this fact, and uses these last three bits to store other - ; data (table type and privilege). In this case, 000 is what we need anyway. - ; - ; Also note that CS isn't an address in protected mode---it's a GDT descriptor. - - - -USE32 - -.inner: - ; load all the other segments with 32 bit data segments - mov eax, gdt.pm32_data - mov ds, eax - mov es, eax - mov fs, eax - mov gs, eax - mov ss, eax - - ; jump to specified function - mov eax, [.func] - jmp eax diff --git a/bios/stage1.asm b/bios/stage1.asm index 5452f8e..a266f5a 100644 --- a/bios/stage1.asm +++ b/bios/stage1.asm @@ -1,37 +1,39 @@ USE16 -stage1: ; dl comes with disk - ; initialize segment registers - xor ax, ax +stage1: + ; Initialize segment registers + xor ax, ax ; Set ax to 0 mov ds, ax mov es, ax mov ss, ax - ; initialize stack + ; Initialize stack pointer + ; (stack grows up) mov sp, 0x7C00 - ; initialize CS - ; far jump sets both CS and IP to a known-good state, - ; we don't know where the BIOS put us at startup. - ; (could be 0x00:0x7C00, could be 0x7C00:0x00. - ; Not everybody follows spec.) - push ax + ; Initialize CS + ; + ; `retf` sets both CS and IP to a known-good state. + ; This is necessary because we don't know where the BIOS put us at startup. + ; (could be 0x00:0x7C00, could be 0x7C00:0x00. Not everybody follows spec.) + push ax ; `ax` is still 0 push word .set_cs retf .set_cs: - ; save disk number + ; Save disk number. + ; BIOS sets `dl` to the number of + ; the disk we're booting from. mov [disk], dl + ; Print "Stage 1" mov si, stage_msg call print mov al, '1' call print_char call print_line - - - ; read CHS gemotry + ; read CHS gemotry, save into [chs] ; CL (bits 0-5) = maximum sector number ; CL (bits 6-7) = high bits of max cylinder number ; CH = low bits of maximum cylinder number @@ -51,11 +53,10 @@ stage1: ; dl comes with disk and cl, 0x3f mov [chs.s], cl - ; disk address of stage 2 - ; (start sector) + ; First sector of stage 2 mov eax, STAGE2_SECTOR - ; where to load stage 2 + ; Where to load stage 2 mov bx, stage2 ; length of stage2 + stage3 @@ -63,36 +64,40 @@ stage1: ; dl comes with disk mov cx, (stage3.end - stage2) / 512 mov dx, 0 + ; Consume eax, bx, cx, dx + ; and load code from disk. call load jmp stage2.entry -; load some sectors from disk to a buffer in memory -; buffer has to be below 1MiB -; IN +; Load sectors from disk to memory. +; Cannot load more than 1MiB. +; +; Input: ; ax: start sector ; bx: offset of buffer ; cx: number of sectors (512 Bytes each) ; dx: segment of buffer -; CLOBBER -; ax, bx, cx, dx, si -; TODO rewrite to (eventually) move larger parts at once -; if that is done increase buffer_size_sectors in startup-common to that (max 0x80000 - startup_end) +; +; Clobbers ax, bx, cx, dx, si load: - ; replaced 127 with 1. - ; see https://stackoverflow.com/questions/58564895/problem-with-bios-int-13h-read-sectors-from-drive - ; TODO: fix later + ; Every "replace 1" comment means that the `1` + ; on that line could be bigger. + ; + ; See https://stackoverflow.com/questions/58564895/problem-with-bios-int-13h-read-sectors-from-drive + ; We have to load one sector at a time to avoid the 1K boundary error. + ; Would be nice to read more sectors at a time, though, that's faster. - cmp cx, 1 ;127 + cmp cx, 1 ; replace 1 jbe .good_size pusha - mov cx, 1; 127 + mov cx, 1 ; replace 1 call load popa - add eax, 1; 127 - add dx, 1 * 512 / 16 ; 127 - sub cx, 1;127 + add eax, 1 ; replace 1 + add dx, 1 * 512 / 16 ; replace 1 + sub cx, 1 ; replace 1 jmp load .good_size: @@ -101,44 +106,37 @@ load: mov [DAPACK.count], cx mov [DAPACK.seg], dx -; This should be a subroutine, -; but we don't call/ret to save a few bytes. -; (we only use this once) -; -;call print_dapack -;print_dapack: - mov bx, [DAPACK.addr + 2] + ; Print the data we're reading + ; Prints AAAAAAAA#BBBB CCCC:DDDD, where: + ; - A..A is the lba we're reading (printed in two parts) + ; - BBBB is the number of sectors we're reading + ; - CCCC is the index we're writing to + ; - DDDD is the buffer we're writing to + mov bx, [DAPACK.addr + 2] ; last two bytes call print_hex - - mov bx, [DAPACK.addr] + mov bx, [DAPACK.addr] ; first two bytes call print_hex - mov al, '#' call print_char - mov bx, [DAPACK.count] call print_hex - mov al, ' ' call print_char - mov bx, [DAPACK.seg] call print_hex - mov al, ':' call print_char - mov bx, [DAPACK.buf] call print_hex - call print_line - ;ret - ; End of print_dapack + ; Read from disk. + ; int13h, ah=0x42 does not work on some disks. + ; use int13h, ah=0x02 in thes case. cmp byte [chs.s], 0 jne .chs - ;INT 0x13 extended read does not work on CDROM! + mov dl, [disk] mov si, DAPACK mov ah, 0x42 @@ -188,6 +186,10 @@ load: jc error ; carry flag set on error ret +; +; MARK: errors +; + error_chs: mov ah, 0 @@ -200,13 +202,18 @@ error: mov si, stage1_error_msg call print - call print_line + call print_line +; halt after printing error details .halt: cli hlt jmp .halt +; +; MARK: data +; + %include "print.asm" stage_msg: db "Stage ",0 @@ -215,9 +222,9 @@ stage1_error_msg: db " ERROR",0 disk: db 0 chs: -.c: dd 0 -.h: dd 0 -.s: dd 0 + .c: dd 0 + .h: dd 0 + .s: dd 0 DAPACK: db 0x10 @@ -225,6 +232,4 @@ DAPACK: .count: dw 0 ; int 13 resets this to # of blocks actually read/written .buf: dw 0 ; memory buffer destination address (0:7c00) .seg: dw 0 ; in memory page zero -.addr: dq 0 ; put the lba to read in this spot - -db 0xff \ No newline at end of file +.addr: dq 0 ; put the lba to read in this spot \ No newline at end of file diff --git a/bios/stage2.asm b/bios/stage2.asm index f964884..2675ab5 100644 --- a/bios/stage2.asm +++ b/bios/stage2.asm @@ -1,6 +1,9 @@ SECTION .text USE16 +%include "gdt.asm" +%include "thunk.asm" + stage2.entry: mov si, stage_msg call print @@ -13,26 +16,57 @@ stage2.entry: or al, 2 out 0x92, al - mov dword [protected_mode.func], stage3.entry - jmp protected_mode.entry +protected_mode: + ; disable interrupts + cli -%include "gdt.asm" -%include "protected_mode.asm" -%include "thunk.asm" + ; load protected mode GDT + lgdt [gdtr] + ; set protected mode bit of cr0 + mov eax, cr0 + or eax, 1 + mov cr0, eax + + ; far jump to load CS with 32 bit segment + ; We need to do this because we are entering 32-bit mode, + ; but the instruction pipeline still has 16-bit instructions. + ; + ; gdt.pm32_code is a multiple of 8, so it always ends with three zero bits. + ; The GDT spec abuses this fact, and uses these last three bits to store other + ; data (table type and privilege). In this case, 000 is what we need anyway. + ; + ; Also note that CS isn't an address in protected mode---it's a GDT descriptor. + jmp gdt.pm32_code:protected_mode_inner + +; We can now use 32-bit instructions! USE32 -stage3.entry: - ; stage3 stack at 448 KiB (512KiB minus 64KiB disk buffer) +protected_mode_inner: + ; load all the other segments with 32 bit data segments + mov eax, gdt.pm32_data + mov ds, eax + mov es, eax + mov fs, eax + mov gs, eax + mov ss, eax + + ; Place stage 3 stack at 448 KiB + ; (512KiB minus 64KiB disk buffer) mov esp, 0x70000 ; push arguments to `start()` mov eax, thunk.int10 push eax + + ; Call `start()`. + ; 0x18 skips ELF headers. mov eax, [stage3 + 0x18] call eax - + .halt: + ; Halt if `start()` ever returns (it shouldn't, but just in case) + ; Without this, we'll try to execute whatever comes next in memory. cli hlt jmp .halt diff --git a/tetros/Cargo.toml b/tetros/Cargo.toml index 219cd2b..505856d 100644 --- a/tetros/Cargo.toml +++ b/tetros/Cargo.toml @@ -29,7 +29,7 @@ absolute_paths_not_starting_with_crate = "deny" explicit_outlives_requirements = "warn" unused_crate_dependencies = "warn" redundant_lifetimes = "warn" -missing_docs = "allow" +missing_docs = "warn" [lints.clippy] needless_return = "allow" diff --git a/tetros/linkers/x86-unknown-none.ld b/tetros/linkers/x86-unknown-none.ld index dc7e852..f3eaeb1 100644 --- a/tetros/linkers/x86-unknown-none.ld +++ b/tetros/linkers/x86-unknown-none.ld @@ -1,9 +1,10 @@ +/* This is the name of the Rust function we start in */ ENTRY(start) OUTPUT_FORMAT(elf32-i386) SECTIONS { - /* The start address must match bootloader.asm */ - . = 0x9000; + /* The start address must match main.asm */ + . = 0x8000; . += SIZEOF_HEADERS; . = ALIGN(4096); diff --git a/tetros/src/drivers/pic.rs b/tetros/src/drivers/pic.rs index afd74ac..c75b96e 100644 --- a/tetros/src/drivers/pic.rs +++ b/tetros/src/drivers/pic.rs @@ -1,18 +1,25 @@ +//! Control routines for the x86 +//! 8259 Programmable Interrupt Controller +//! +//! This helps us configure interrupts that receive +//! keyboard input and timer pulses. + use crate::os::util::outb; /// IO base address for master PIC const PIC_A: u32 = 0x20; +/// Command address for master PIC const PIC_A_COMMAND: u32 = PIC_A; +/// Data address for master PIC const PIC_A_DATA: u32 = PIC_A + 1; /// IO base address for slave PIC const PIC_B: u32 = 0xA0; +/// Command address for slave PIC const PIC_B_COMMAND: u32 = PIC_B; +/// Data address for slave PIC const PIC_B_DATA: u32 = PIC_B + 1; -/// PIC `EOI` command -const CMD_EOI: u8 = 0x20; - /// A driver for the PIC /// /// Reference: @@ -24,6 +31,7 @@ pub struct PICDriver { } impl PICDriver { + /// Create a PIC driver with the given offsets pub const fn new(offset_pic_a: u8, offset_pic_b: u8) -> Self { Self { offset_pic_a, @@ -47,14 +55,20 @@ impl PICDriver { unsafe { outb(PIC_B_DATA, cmd) } } - pub fn send_eoi(&self, irq: u8) { - if irq > 8 { - self.send_b_cmd(CMD_EOI); + /// Send an EOI for the given IRQ. + /// + /// This needs to be called at the end of each interrupt handler. + /// If `both` is true, reset both PICs. This is only necessary + /// when we handle interrupts from PIC_B. + pub fn send_eoi(&self, both: bool) { + if both { + self.send_b_cmd(0x20); } - - self.send_a_cmd(CMD_EOI); + self.send_a_cmd(0x20); } + /// Initialize this PIC driver. + /// This should be called as early as possible. pub fn init(&mut self) { const ICW1_ICW4: u8 = 0x01; /* Indicates that ICW4 will be present */ const ICW1_INIT: u8 = 0x10; /* Initialization - required! */ diff --git a/tetros/src/drivers/serial.rs b/tetros/src/drivers/serial.rs index cb5bad5..161e877 100644 --- a/tetros/src/drivers/serial.rs +++ b/tetros/src/drivers/serial.rs @@ -1,3 +1,10 @@ +//! Serial port driver, for debug. +//! +//! This file provides the usual `print` +//! and `println` macros (which are usually +//! provided by `std`) that send messages out +//! of the serial port. + use lazy_static::lazy_static; use spin::Mutex; use uart_16550::SerialPort; diff --git a/tetros/src/drivers/vga.rs b/tetros/src/drivers/vga.rs index 194882e..320a3b1 100644 --- a/tetros/src/drivers/vga.rs +++ b/tetros/src/drivers/vga.rs @@ -3,7 +3,6 @@ use rand::seq::IndexedRandom; use crate::RNG; -#[repr(u8)] #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum VgaColor { Black, @@ -73,6 +72,9 @@ impl Vga13h { pub const HEIGHT: usize = 200; pub const ADDR: usize = 0xA0000; + /// Initialize a new VGA driver. + /// + /// Only one of these should exist. pub const unsafe fn new() -> Self { Self { fb_a: [0; Vga13h::WIDTH * Vga13h::HEIGHT], diff --git a/tetros/src/game/board.rs b/tetros/src/game/board.rs index 885a138..daa96d6 100644 --- a/tetros/src/game/board.rs +++ b/tetros/src/game/board.rs @@ -2,21 +2,26 @@ use crate::drivers::vga::{Vga13h, VgaColor}; use super::FallingTetromino; -#[repr(u8)] +/// The state of a cell in the game board #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum TetrisCell { Empty, Filled { color: VgaColor }, } +/// The tetris board pub struct TetrisBoard { board: [TetrisCell; TetrisBoard::BOARD_WIDTH * TetrisBoard::BOARD_HEIGHT], } impl TetrisBoard { + /// The width of this board, in cells const BOARD_WIDTH: usize = 10; + + /// The height of this board, in cells const BOARD_HEIGHT: usize = 20; + /// The side length of a (square) cell, in pixels const CELL_SIZE: usize = 9; pub const fn new() -> Self { @@ -25,6 +30,8 @@ impl TetrisBoard { } } + /// Find and remove all filled rows, + /// shifting upper rows down. pub fn collapse(&mut self) { let mut y = Self::BOARD_HEIGHT - 1; 'outer: loop { @@ -58,6 +65,11 @@ impl TetrisBoard { } } + /// Place the given tetromino on the board, + /// filling the cells it occupies. + /// + /// If the tetromino cells that overlap + /// non-empty board cells are ignored. pub fn place_tetromino(&mut self, tetromino: FallingTetromino) { for (x, y) in tetromino.tiles() { let cell = self.get_cell_mut(x, y); @@ -82,32 +94,26 @@ impl TetrisBoard { return true; } + /// Get the value of the cell at the given position. + /// Returns [`None`] if (x, y) exceeds the board's bounds. pub fn get_cell(&self, x: usize, y: usize) -> Option<&TetrisCell> { - if y >= TetrisBoard::BOARD_HEIGHT { - return None; - } - - if x >= TetrisBoard::BOARD_WIDTH { - return None; - } - - return Some(&self.board[y * TetrisBoard::BOARD_WIDTH + x]); + return self.board.get(y * TetrisBoard::BOARD_WIDTH + x); } + /// Get a mutable reference to the cell at the given position. + /// Returns [`None`] if (x, y) exceeds the board's bounds. pub fn get_cell_mut(&mut self, x: usize, y: usize) -> Option<&mut TetrisCell> { - if y >= TetrisBoard::BOARD_HEIGHT { - return None; - } - - if x >= TetrisBoard::BOARD_WIDTH { - return None; - } - - return Some(&mut self.board[y * TetrisBoard::BOARD_WIDTH + x]); + return self.board.get_mut(y * TetrisBoard::BOARD_WIDTH + x); } } +// +// MARK: draw routines +// + impl TetrisBoard { + /// Draw a cell of the given color on `fb`. + /// (x, y) is the pixel position of the cell (NOT board coordinates). fn draw_cell(&self, fb: &mut [u8], color: VgaColor, x: usize, y: usize) { let color = color.as_u8(); for yo in 0..TetrisBoard::CELL_SIZE { @@ -117,6 +123,7 @@ impl TetrisBoard { } } + /// Draw the tetris board's frame fn draw_frame(&self, fb: &mut [u8], x: usize, y: usize) { let color = VgaColor::Gray.as_u8(); for yo in 0..TetrisBoard::CELL_SIZE { @@ -126,6 +133,7 @@ impl TetrisBoard { } } + /// Draw this tetris board using the given VGA driver. pub fn draw(&self, vga: &mut Vga13h, falling: Option<&FallingTetromino>) { let fb = vga.get_fb(); diff --git a/tetros/src/game/falling.rs b/tetros/src/game/falling.rs index b0e664b..d603d79 100644 --- a/tetros/src/game/falling.rs +++ b/tetros/src/game/falling.rs @@ -53,6 +53,7 @@ pub enum Direction { } impl Direction { + /// Rotate this direction clockwise pub fn rot_cw(self) -> Self { match self { Self::North => Self::East, @@ -61,17 +62,6 @@ impl Direction { Self::West => Self::North, } } - - /* - pub fn rot_ccw(self) -> Self { - match self { - Self::North => Self::West, - Self::West => Self::South, - Self::South => Self::East, - Self::East => Self::North, - } - } - */ } #[derive(Debug, Clone)] @@ -85,6 +75,7 @@ pub struct FallingTetromino { } impl FallingTetromino { + /// Make a new falling tetromino pub fn new(tetromino: Tetromino, color: VgaColor, center_x: usize, center_y: usize) -> Self { Self { tetromino, @@ -95,6 +86,7 @@ impl FallingTetromino { } } + /// Generate a random tetromino at the given position pub fn random(center_x: usize, center_y: usize) -> Self { Self::new( Tetromino::choose_rand(), @@ -104,6 +96,7 @@ impl FallingTetromino { ) } + // Move this tetromino pub fn translate(&mut self, x: i16, y: i16) { if x > 0 { let x = usize::try_from(x).unwrap(); @@ -122,16 +115,11 @@ impl FallingTetromino { } } + /// Rotate this tetromino clockwise pub fn rotate_cw(&mut self) { self.direction = self.direction.rot_cw() } - /* - pub fn rotate_ccw(&mut self) { - self.direction = self.direction.rot_ccw() - } - */ - /// Returns the positions of this falling tetromino's tiles. pub fn tiles(&self) -> [(usize, usize); 4] { match (&self.tetromino, self.direction) { diff --git a/tetros/src/game/mod.rs b/tetros/src/game/mod.rs index e92dd16..bb13eb6 100644 --- a/tetros/src/game/mod.rs +++ b/tetros/src/game/mod.rs @@ -1,3 +1,6 @@ +//! This crate contains all tetris game logic. +//! No low-level magic here. + mod board; pub use board::*; diff --git a/tetros/src/idt/stackframe.rs b/tetros/src/idt/stackframe.rs index 0360f0c..966ef86 100644 --- a/tetros/src/idt/stackframe.rs +++ b/tetros/src/idt/stackframe.rs @@ -1,16 +1,14 @@ use core::{fmt, ops::Deref}; -use crate::os::EFlags; - use super::VirtAddr; +use crate::os::EFlags; /// Wrapper type for the interrupt stack frame pushed by the CPU. /// /// This type derefs to an [`InterruptStackFrameValue`], which allows reading the actual values. /// -/// This wrapper type ensures that no accidental modification of the interrupt stack frame -/// occurs, which can cause undefined behavior (see the [`as_mut`](InterruptStackFrame::as_mut) -/// method for more information). +/// This wrapper ensures that the stack frame cannot be modified. +/// This prevents undefined behavior. #[repr(transparent)] pub struct InterruptStackFrame(InterruptStackFrameValue); diff --git a/tetros/src/idt/table.rs b/tetros/src/idt/table.rs index 7f520e8..5fd50d9 100644 --- a/tetros/src/idt/table.rs +++ b/tetros/src/idt/table.rs @@ -5,17 +5,6 @@ use super::{ HandlerFuncWithErrCode, PageFaultHandlerFunc, }; -// TODO: comments -#[repr(C, packed(2))] -struct Idtr { - size: u16, - offset: u32, -} - -// -// MARK: idt -// - // spell:off #[derive(Clone, Debug)] #[repr(C)] @@ -467,12 +456,17 @@ impl InterruptDescriptorTable { /// # Safety /// /// As long as it is the active IDT, you must ensure that: - /// /// - `self` is never destroyed. /// - `self` always stays at the same memory location. - /// It is recommended to wrap it in a `Box`. #[inline] pub unsafe fn load_unsafe(&self) { + /// The data we push to the IDTR register + #[repr(C, packed(2))] + struct Idtr { + size: u16, + offset: u32, + } + let idtr = { Idtr { size: (size_of::() - 1) as u16, diff --git a/tetros/src/idt/virtaddr.rs b/tetros/src/idt/virtaddr.rs index 1cc4cd7..43b90dc 100644 --- a/tetros/src/idt/virtaddr.rs +++ b/tetros/src/idt/virtaddr.rs @@ -1,7 +1,4 @@ -use core::{ - fmt::{self}, - ops::{Add, AddAssign, Sub, SubAssign}, -}; +use core::fmt::{self}; /// A canonical 32-bit virtual memory address. #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -57,40 +54,3 @@ impl fmt::UpperHex for VirtAddr { fmt::UpperHex::fmt(&self.0, f) } } - -impl fmt::Pointer for VirtAddr { - #[inline] - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Pointer::fmt(&(self.0 as *const ()), f) - } -} - -impl Add for VirtAddr { - type Output = Self; - #[inline] - fn add(self, rhs: u32) -> Self::Output { - VirtAddr(self.0.checked_add(rhs).unwrap()) - } -} - -impl AddAssign for VirtAddr { - #[inline] - fn add_assign(&mut self, rhs: u32) { - *self = *self + rhs; - } -} - -impl Sub for VirtAddr { - type Output = Self; - #[inline] - fn sub(self, rhs: u32) -> Self::Output { - VirtAddr(self.0.checked_sub(rhs).unwrap()) - } -} - -impl SubAssign for VirtAddr { - #[inline] - fn sub_assign(&mut self, rhs: u32) { - *self = *self - rhs; - } -} diff --git a/tetros/src/lib.rs b/tetros/src/lib.rs index d8c66b6..919e6ef 100644 --- a/tetros/src/lib.rs +++ b/tetros/src/lib.rs @@ -1,3 +1,5 @@ +//! The main code of tetris + #![no_std] #![feature(int_roundings)] #![feature(lang_items)] @@ -23,12 +25,18 @@ mod os; #[macro_use] mod drivers; -const PIC_OFFSET: u8 = 32; - // // MARK: globals // +// This code has no parallelism, so we don't _really_ +// need locks. The Mutexes here satisfy Rust's +// "no mutable global state" rule. +// +// They also help prevent bugs, since we get deadlocks +// instead of hard-to-debug surprising behavior. +// +const PIC_OFFSET: u8 = 32; static VGA: Mutex = Mutex::new(unsafe { Vga13h::new() }); static PIC: Mutex = Mutex::new(PICDriver::new(PIC_OFFSET, PIC_OFFSET + 8)); static TICK_COUNTER: Mutex = Mutex::new(0); @@ -36,6 +44,8 @@ static BOARD: Mutex = Mutex::new(TetrisBoard::new()); static FALLING: Mutex> = Mutex::new(None); static LAST_INPUT: Mutex> = Mutex::new(None); +// These values can't be initialized statically, +// so we cheat with `lazy_static` lazy_static! { static ref RNG: Mutex = Mutex::new(SmallRng::seed_from_u64(1337)); static ref IDT: InterruptDescriptorTable = { @@ -50,10 +60,26 @@ lazy_static! { }; } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum InputKey { + Left, + Right, + Up, + Down, +} + // // MARK: interrupts // +// These functions are called when we receive interrupts. +// This can occur between ANY two instructions---which is +// why we use `without_interrupts` when acquiring locks. +// +// Notice how we do as little work as possible in our +// interrupt handlers. All our business logic goes into +// the main loop. +#[expect(missing_docs)] #[derive(Debug, Clone, Copy)] #[repr(u8)] pub enum InterruptIndex { @@ -72,22 +98,20 @@ impl InterruptIndex { } extern "x86-interrupt" fn divide_handler(stack_frame: InterruptStackFrame) { + // Simple interrupt handler, as an example. + // This can be triggered manually using `asm!("int 0")`, + // even if interrupts are disabled. println!("DIVIDE ERROR {:?}", stack_frame); } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum InputKey { - Left, - Right, - Up, - Down, -} - extern "x86-interrupt" fn keyboard_handler(_stack_frame: InterruptStackFrame) { { // Re-seed our rng using user input. // This is a simple hack that makes our // "random" tile selector less deterministic. + // + // Getting random seeds from hardware is + // more trouble than its worth. let mut rng = RNG.lock(); let past: u64 = rng.random(); let tcr = u64::from(*TICK_COUNTER.lock()); @@ -125,13 +149,13 @@ extern "x86-interrupt" fn keyboard_handler(_stack_frame: InterruptStackFrame) { *LAST_INPUT.lock() = key; } - PIC.lock().send_eoi(InterruptIndex::Keyboard.as_u8()); + PIC.lock().send_eoi(false); } extern "x86-interrupt" fn timer_handler(_stack_frame: InterruptStackFrame) { let mut t = TICK_COUNTER.lock(); *t = (*t).wrapping_add(1); - PIC.lock().send_eoi(InterruptIndex::Timer.as_u8()); + PIC.lock().send_eoi(false); } extern "x86-interrupt" fn double_fault_handler( @@ -145,6 +169,7 @@ extern "x86-interrupt" fn double_fault_handler( // MARK: main // +#[expect(missing_docs)] #[no_mangle] pub unsafe extern "C" fn start(thunk10: extern "C" fn()) -> ! { println!("Entered Rust, serial ready."); @@ -190,6 +215,7 @@ pub unsafe extern "C" fn start(thunk10: extern "C" fn()) -> ! { } last_t = t; + // MARK: input // Handle user input without_interrupts(|| { if let Some(fall) = &mut *FALLING.lock() { @@ -236,6 +262,7 @@ pub unsafe extern "C" fn start(thunk10: extern "C" fn()) -> ! { } }); + // MARK: update board // Update board without_interrupts(|| { let mut v = VGA.lock(); diff --git a/tetros/src/os/eflags.rs b/tetros/src/os/eflags.rs index 2395a0b..d14fabf 100644 --- a/tetros/src/os/eflags.rs +++ b/tetros/src/os/eflags.rs @@ -80,6 +80,7 @@ bitflags! { } impl EFlags { + /// Read the EFLAGS register #[inline] pub fn read() -> EFlags { EFlags::from_bits_truncate(EFlags::read_raw()) diff --git a/tetros/src/os/panic.rs b/tetros/src/os/panic.rs index 1a0a2de..2dcf8ca 100644 --- a/tetros/src/os/panic.rs +++ b/tetros/src/os/panic.rs @@ -1,8 +1,12 @@ -//! Intrinsics for panic handling +//! Rust intrinsics for panic handling. +//! +//! These are usually provided by `std`, +//! but we don't have that luxury! use core::arch::asm; use core::panic::PanicInfo; +// Use serial println use crate::println; #[lang = "eh_personality"] diff --git a/tetros/src/os/util.rs b/tetros/src/os/util.rs index 31a4cff..b82cec8 100644 --- a/tetros/src/os/util.rs +++ b/tetros/src/os/util.rs @@ -18,11 +18,13 @@ pub fn sti() { } } -/// Run a closure with disabled interrupts. -/// /// Run the given closure, disabling interrupts before running it (if they aren't already disabled). /// Afterwards, interrupts are enabling again if they were enabled before. /// +/// This helps us prevent deadlocks, which can occur if +/// an interrupt handler tries to acquire a lock that was +/// locked at the time of the interrupt. +/// /// If you have other `enable` and `disable` calls _within_ the closure, things may not work as expected. #[inline] pub fn without_interrupts(f: F) -> R @@ -46,6 +48,7 @@ where ret } +/// Wraps the `in` instruction pub unsafe fn inb(port: u32) -> u8 { let mut out; @@ -58,6 +61,7 @@ pub unsafe fn inb(port: u32) -> u8 { return out; } +/// Wraps the `out` instruction pub unsafe fn outb(port: u32, value: u8) { asm!( "out dx, al",