From 206fb7cc8a82a86aff4ce0d699b4663d8c4a7add Mon Sep 17 00:00:00 2001
From: acheronfail <acheronfail@gmail.com>
Date: Thu, 18 Oct 2018 08:21:58 +1100
Subject: [PATCH 1/6] refactor exception code into interrupts.rs

---
 .../posts/06-cpu-exceptions/index.md          | 49 ++++++++++++++-----
 .../posts/07-double-faults/index.md           | 14 +++---
 src/interrupts.rs                             | 32 ++++++++++++
 src/lib.rs                                    |  7 ++-
 src/main.rs                                   | 35 +------------
 5 files changed, 84 insertions(+), 53 deletions(-)
 create mode 100644 src/interrupts.rs

diff --git a/blog/content/second-edition/posts/06-cpu-exceptions/index.md b/blog/content/second-edition/posts/06-cpu-exceptions/index.md
index 25c2891b..1b57b8c3 100644
--- a/blog/content/second-edition/posts/06-cpu-exceptions/index.md
+++ b/blog/content/second-edition/posts/06-cpu-exceptions/index.md
@@ -207,7 +207,11 @@ If you are interested in more details: We also have a series of posts that expla
 Now that we've understood the theory, it's time to handle CPU exceptions in our kernel. We start by creating an `init_idt` function that creates a new `InterruptDescriptorTable`:
 
 ``` rust
-// in src/main.rs
+// in src/lib.rs
+
+pub mod interrupts;
+
+// in src/interrupts.rs
 
 extern crate x86_64;
 use x86_64::structures::idt::InterruptDescriptorTable;
@@ -228,7 +232,7 @@ The breakpoint exception is commonly used in debuggers: When the user sets a bre
 For our use case, we don't need to overwrite any instructions. Instead, we just want to print a message when the breakpoint instruction is executed and then continue the program. So let's create a simple `breakpoint_handler` function and add it to our IDT:
 
 ```rust
-/// in src/main.rs
+// in src/interrupts.rs
 
 use x86_64::structures::idt::{InterruptDescriptorTable, ExceptionStackFrame};
 
@@ -246,7 +250,7 @@ extern "x86-interrupt" fn breakpoint_handler(
 
 Our handler just outputs a message and pretty-prints the exception stack frame.
 
-When we try to compile it, the following error occurs:
+When we try to compile it, the following errors occur:
 
 ```
 error[E0658]: x86-interrupt ABI is experimental and subject to change (see issue #40180)
@@ -260,7 +264,33 @@ error[E0658]: x86-interrupt ABI is experimental and subject to change (see issue
    = help: add #![feature(abi_x86_interrupt)] to the crate attributes to enable
 ```
 
-This error occurs because the `x86-interrupt` calling convention is still unstable. To use it anyway, we have to explicitly enable it by adding `#![feature(abi_x86_interrupt)]` on the top of our `main.rs`.
+This error occurs because the `x86-interrupt` calling convention is still unstable. To use it anyway, we have to explicitly enable it by adding `#![feature(abi_x86_interrupt)]` on the top of our `lib.rs`.
+
+```
+error: cannot find macro `println!` in this scope
+  --> src/interrupts.rs:40:5
+   |
+40 |     println!("EXCEPTION: BREAKPOINT\n{:#?}", stack_frame);
+   |     ^^^^^^^
+   |
+   = help: have you added the `#[macro_use]` on the module/import?
+```
+
+And this error occurs because the `print!` and `println!` macros we created in the `vga_buffer` _must be defined_ before we use them. This is one case where import order matters in Rust. We can easily fix this by ensuring the order of our imports places the macros first:
+
+```rust
+// in src/lib.rs
+
+#[macro_use]
+pub mod vga_buffer; // import this before other modules so its macros may be used
+pub mod gdt;
+pub mod interrupts;
+pub mod serial;
+```
+
+Now we can use our `print!` and `println!` macros in `interrupts.rs`. If you'd like to know more about the ins and outs of macros and how they differ from functions [you can find more information here](in-depth-rust-macros).
+
+[in-depth-rust-macros]: https://doc.rust-lang.org/book/second-edition/appendix-04-macros.html
 
 ### Loading the IDT
 In order that the CPU uses our new interrupt descriptor table, we need to load it using the [`lidt`] instruction. The `InterruptDescriptorTable` struct of the `x86_64` provides a [`load`][InterruptDescriptorTable::load] method function for that. Let's try to use it:
@@ -269,7 +299,7 @@ In order that the CPU uses our new interrupt descriptor table, we need to load i
 [InterruptDescriptorTable::load]: https://docs.rs/x86_64/0.2.8/x86_64/structures/idt/struct.InterruptDescriptorTable.html#method.load
 
 ```rust
-// in src/main.rs
+// in src/interrupts.rs
 
 pub fn init_idt() {
     let mut idt = InterruptDescriptorTable::new();
@@ -339,10 +369,7 @@ We already imported the `lazy_static` crate when we [created an abstraction for
 [vga text buffer lazy static]: ./second-edition/posts/03-vga-text-buffer/index.md#lazy-statics
 
 ```rust
-// in src/main.rs
-
-#[macro_use]
-extern crate lazy_static;
+// in src/interrupts.rs
 
 lazy_static! {
     static ref IDT: InterruptDescriptorTable = {
@@ -370,7 +397,7 @@ Now we should be able to handle breakpoint exceptions! Let's try it in our `_sta
 pub extern "C" fn _start() -> ! {
     println!("Hello World{}", "!");
 
-    init_idt();
+    blog_os::interrupts::init_idt();
 
     // invoke a breakpoint exception
     x86_64::instructions::int3();
@@ -403,7 +430,7 @@ static BREAKPOINT_HANDLER_CALLED: AtomicUsize = AtomicUsize::new(0);
 #[cfg(not(test))]
 #[no_mangle]
 pub extern "C" fn _start() -> ! {
-    init_idt();
+    blog_os::interrupts::init_idt();
 
     // invoke a breakpoint exception
     x86_64::instructions::int3();
diff --git a/blog/content/second-edition/posts/07-double-faults/index.md b/blog/content/second-edition/posts/07-double-faults/index.md
index 39466694..fc7ff6d5 100644
--- a/blog/content/second-edition/posts/07-double-faults/index.md
+++ b/blog/content/second-edition/posts/07-double-faults/index.md
@@ -33,7 +33,7 @@ Let's provoke a double fault by triggering an exception for that we didn't defin
 pub extern "C" fn _start() -> ! {
     println!("Hello World{}", "!");
 
-    init_idt();
+    blog_os::interrupts::init_idt();
 
     // trigger a page fault
     unsafe {
@@ -60,7 +60,7 @@ So in order to prevent this triple fault, we need to either provide a handler fu
 A double fault is a normal exception with an error code, so we can specify a handler function similar to our breakpoint handler:
 
 ```rust
-// in src/main.rs
+// in src/interrupts.rs
 
 lazy_static! {
     static ref IDT: InterruptDescriptorTable = {
@@ -162,7 +162,7 @@ Let's try it ourselves! We can easily provoke a kernel stack overflow by calling
 pub extern "C" fn _start() -> ! {
     println!("Hello World{}", "!");
 
-    init_idt();
+    blog_os::interrupts::init_idt();
 
     fn stack_overflow() {
         stack_overflow(); // for each recursion, the return address is pushed
@@ -314,7 +314,7 @@ pub extern "C" fn _start() -> ! {
     println!("Hello World{}", "!");
 
     blog_os::gdt::init();
-    init_idt();
+    blog_os::interrupts::init_idt();
 
     […]
 }
@@ -380,7 +380,9 @@ We reload the code segment register using [`set_cs`] and to load the TSS using [
 Now that we loaded a valid TSS and interrupt stack table, we can set the stack index for our double fault handler in the IDT:
 
 ```rust
-// in src/main.rs
+// in src/interrupts.rs
+
+use gdt;
 
 lazy_static! {
     static ref IDT: InterruptDescriptorTable = {
@@ -388,7 +390,7 @@ lazy_static! {
         idt.breakpoint.set_handler_fn(breakpoint_handler);
         unsafe {
             idt.double_fault.set_handler_fn(double_fault_handler)
-                .set_stack_index(blog_os::gdt::DOUBLE_FAULT_IST_INDEX); // new
+                .set_stack_index(gdt::DOUBLE_FAULT_IST_INDEX); // new
         }
 
         idt
diff --git a/src/interrupts.rs b/src/interrupts.rs
new file mode 100644
index 00000000..be035e4b
--- /dev/null
+++ b/src/interrupts.rs
@@ -0,0 +1,32 @@
+use x86_64::structures::idt::{ExceptionStackFrame, InterruptDescriptorTable};
+use gdt;
+
+lazy_static! {
+    static ref IDT: InterruptDescriptorTable = {
+        let mut idt = InterruptDescriptorTable::new();
+        idt.breakpoint.set_handler_fn(breakpoint_handler);
+        unsafe {
+            idt.double_fault
+                .set_handler_fn(double_fault_handler)
+                .set_stack_index(gdt::DOUBLE_FAULT_IST_INDEX);
+        }
+
+        idt
+    };
+}
+
+pub fn init_idt() {
+    IDT.load();
+}
+
+extern "x86-interrupt" fn breakpoint_handler(stack_frame: &mut ExceptionStackFrame) {
+    println!("EXCEPTION: BREAKPOINT\n{:#?}", stack_frame);
+}
+
+extern "x86-interrupt" fn double_fault_handler(
+    stack_frame: &mut ExceptionStackFrame,
+    _error_code: u64,
+) {
+    println!("EXCEPTION: DOUBLE FAULT\n{:#?}", stack_frame);
+    loop {}
+}
diff --git a/src/lib.rs b/src/lib.rs
index 86ef1bc1..961ae1d9 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1,4 +1,5 @@
 #![no_std] // don't link the Rust standard library
+#![feature(abi_x86_interrupt)]
 
 extern crate bootloader_precompiled;
 extern crate spin;
@@ -13,9 +14,11 @@ extern crate array_init;
 #[cfg(test)]
 extern crate std;
 
-pub mod gdt;
-pub mod serial;
+#[macro_use]
 pub mod vga_buffer;
+pub mod gdt;
+pub mod interrupts;
+pub mod serial;
 
 pub unsafe fn exit_qemu() {
     use x86_64::instructions::port::Port;
diff --git a/src/main.rs b/src/main.rs
index bb7e1a92..65154cfb 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,4 +1,3 @@
-#![feature(abi_x86_interrupt)]
 #![no_std] // don't link the Rust standard library
 #![cfg_attr(not(test), no_main)] // disable all Rust-level entry points
 #![cfg_attr(test, allow(dead_code, unused_macros, unused_imports))]
@@ -19,7 +18,7 @@ pub extern "C" fn _start() -> ! {
     println!("Hello World{}", "!");
 
     blog_os::gdt::init();
-    init_idt();
+    blog_os::interrupts::init_idt();
 
     fn stack_overflow() {
         stack_overflow(); // for each recursion, the return address is pushed
@@ -39,35 +38,3 @@ fn panic(info: &PanicInfo) -> ! {
     println!("{}", info);
     loop {}
 }
-
-use x86_64::structures::idt::{ExceptionStackFrame, InterruptDescriptorTable};
-
-lazy_static! {
-    static ref IDT: InterruptDescriptorTable = {
-        let mut idt = InterruptDescriptorTable::new();
-        idt.breakpoint.set_handler_fn(breakpoint_handler);
-        unsafe {
-            idt.double_fault
-                .set_handler_fn(double_fault_handler)
-                .set_stack_index(blog_os::gdt::DOUBLE_FAULT_IST_INDEX);
-        }
-
-        idt
-    };
-}
-
-pub fn init_idt() {
-    IDT.load();
-}
-
-extern "x86-interrupt" fn breakpoint_handler(stack_frame: &mut ExceptionStackFrame) {
-    println!("EXCEPTION: BREAKPOINT\n{:#?}", stack_frame);
-}
-
-extern "x86-interrupt" fn double_fault_handler(
-    stack_frame: &mut ExceptionStackFrame,
-    _error_code: u64,
-) {
-    println!("EXCEPTION: DOUBLE FAULT\n{:#?}", stack_frame);
-    loop {}
-}

From 61397dbb07e13386ac91fb1133325851fdfe019d Mon Sep 17 00:00:00 2001
From: acheronfail <acheronfail@gmail.com>
Date: Thu, 18 Oct 2018 08:59:30 +1100
Subject: [PATCH 2/6] feedback: mention creating a new interrupts module

---
 blog/content/second-edition/posts/06-cpu-exceptions/index.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/blog/content/second-edition/posts/06-cpu-exceptions/index.md b/blog/content/second-edition/posts/06-cpu-exceptions/index.md
index 1b57b8c3..0d76dc61 100644
--- a/blog/content/second-edition/posts/06-cpu-exceptions/index.md
+++ b/blog/content/second-edition/posts/06-cpu-exceptions/index.md
@@ -204,7 +204,7 @@ If you are interested in more details: We also have a series of posts that expla
 [too-much-magic]: #too-much-magic
 
 ## Implementation
-Now that we've understood the theory, it's time to handle CPU exceptions in our kernel. We start by creating an `init_idt` function that creates a new `InterruptDescriptorTable`:
+Now that we've understood the theory, it's time to handle CPU exceptions in our kernel. We'll start by creating a new interrupts module in `src/interrupts.rs`, that first creates an `init_idt` function that creates a new `InterruptDescriptorTable`:
 
 ``` rust
 // in src/lib.rs

From e3d742c928cfdd2a36e5ffe0e36141eef56d053a Mon Sep 17 00:00:00 2001
From: acheronfail <acheronfail@gmail.com>
Date: Thu, 18 Oct 2018 14:16:17 +1100
Subject: [PATCH 3/6] remove unused crate from main.rs

---
 src/main.rs | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/main.rs b/src/main.rs
index 65154cfb..b34592c7 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -5,8 +5,6 @@
 #[macro_use]
 extern crate blog_os;
 extern crate x86_64;
-#[macro_use]
-extern crate lazy_static;
 
 use core::panic::PanicInfo;
 

From 168e2b3d89bca07d2b48115be84eee3d90db8547 Mon Sep 17 00:00:00 2001
From: acheronfail <acheronfail@gmail.com>
Date: Thu, 18 Oct 2018 18:46:36 +1100
Subject: [PATCH 4/6] feedback: add explanation for adding #[macro_use] before
 import

---
 .../posts/06-cpu-exceptions/index.md               | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/blog/content/second-edition/posts/06-cpu-exceptions/index.md b/blog/content/second-edition/posts/06-cpu-exceptions/index.md
index 0d76dc61..09df2803 100644
--- a/blog/content/second-edition/posts/06-cpu-exceptions/index.md
+++ b/blog/content/second-edition/posts/06-cpu-exceptions/index.md
@@ -276,7 +276,19 @@ error: cannot find macro `println!` in this scope
    = help: have you added the `#[macro_use]` on the module/import?
 ```
 
-And this error occurs because the `print!` and `println!` macros we created in the `vga_buffer` _must be defined_ before we use them. This is one case where import order matters in Rust. We can easily fix this by ensuring the order of our imports places the macros first:
+This happened because we forgot to add `#[macro_use]` before our import of the `vga_buffer` module.
+
+```rust
+// in src/lib.rs
+
+pub mod gdt;
+pub mod interrupts;
+pub mod serial;
+#[macro_use] // new
+pub mod vga_buffer;
+```
+
+However, after adding `#[macro_use]` before the module import, we still get the same error. Sometimes this can be confusing, but it's actually a quirk of how Rust's macro system works. Macros _must be defined_ before you can use them. This is one case where import order matters in Rust. We can easily fix this by ensuring the order of our imports places the macros first:
 
 ```rust
 // in src/lib.rs

From da09ad33620c2a68f61d51be34868599e27d9492 Mon Sep 17 00:00:00 2001
From: Philipp Oppermann <dev@phil-opp.com>
Date: Thu, 18 Oct 2018 13:50:00 +0200
Subject: [PATCH 5/6] Clarify that the exceptions tests use their own IDT

---
 .../posts/06-cpu-exceptions/index.md          | 26 ++++++++++++++-----
 src/bin/test-exception-breakpoint.rs          |  8 +++---
 ...t-exception-double-fault-stack-overflow.rs |  8 +++---
 3 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/blog/content/second-edition/posts/06-cpu-exceptions/index.md b/blog/content/second-edition/posts/06-cpu-exceptions/index.md
index 09df2803..3ba15a9c 100644
--- a/blog/content/second-edition/posts/06-cpu-exceptions/index.md
+++ b/blog/content/second-edition/posts/06-cpu-exceptions/index.md
@@ -434,15 +434,14 @@ Let's create an integration test that ensures that the above continues to work.
 ```rust
 // in src/bin/test-exception-breakpoint.rs
 
-use blog_os::exit_qemu;
+[…]
 use core::sync::atomic::{AtomicUsize, Ordering};
 
 static BREAKPOINT_HANDLER_CALLED: AtomicUsize = AtomicUsize::new(0);
 
-#[cfg(not(test))]
 #[no_mangle]
 pub extern "C" fn _start() -> ! {
-    blog_os::interrupts::init_idt();
+    init_test_idt();
 
     // invoke a breakpoint exception
     x86_64::instructions::int3();
@@ -463,16 +462,31 @@ pub extern "C" fn _start() -> ! {
     loop {}
 }
 
-extern "x86-interrupt" fn breakpoint_handler(_: &mut ExceptionStackFrame) {
+
+lazy_static! {
+    static ref TEST_IDT: InterruptDescriptorTable = {
+        let mut idt = InterruptDescriptorTable::new();
+        idt.breakpoint.set_handler_fn(breakpoint_handler);
+        idt
+    };
+}
+
+pub fn init_test_idt() {
+    TEST_IDT.load();
+}
+
+extern "x86-interrupt" fn breakpoint_handler(
+    _stack_frame: &mut ExceptionStackFrame)
+{
     BREAKPOINT_HANDLER_CALLED.fetch_add(1, Ordering::SeqCst);
 }
 
-// […]
+[…]
 ```
 
 For space reasons we don't show the full content here. You can find the full file [on Github](https://github.com/phil-opp/blog_os/blob/master/src/bin/test-exception-breakpoint.rs).
 
-It is basically a copy of our `main.rs` with some modifications to `_start` and `breakpoint_handler`. The most interesting part is the `BREAKPOINT_HANDLER_CALLER` static. It is an [`AtomicUsize`], an integer type that can be safely concurrently modifies because all of its operations are atomic. We increment it when the `breakpoint_handler` is called and verify in our `_start` function that the handler was called exactly once.
+It is similar to our `main.rs`, but uses a custom IDT called `TEST_IDT` and different `_start` and `breakpoint_handler` functions. The most interesting part is the `BREAKPOINT_HANDLER_CALLER` static. It is an [`AtomicUsize`], an integer type that can be safely concurrently modifies because all of its operations are atomic. We increment it when the `breakpoint_handler` is called and verify in our `_start` function that the handler was called exactly once.
 
 [`AtomicUsize`]: https://doc.rust-lang.org/core/sync/atomic/struct.AtomicUsize.html
 
diff --git a/src/bin/test-exception-breakpoint.rs b/src/bin/test-exception-breakpoint.rs
index e6150acf..f9de1543 100644
--- a/src/bin/test-exception-breakpoint.rs
+++ b/src/bin/test-exception-breakpoint.rs
@@ -18,7 +18,7 @@ static BREAKPOINT_HANDLER_CALLED: AtomicUsize = AtomicUsize::new(0);
 #[cfg(not(test))]
 #[no_mangle]
 pub extern "C" fn _start() -> ! {
-    init_idt();
+    init_test_idt();
 
     // invoke a breakpoint exception
     x86_64::instructions::int3();
@@ -59,15 +59,15 @@ fn panic(info: &PanicInfo) -> ! {
 use x86_64::structures::idt::{ExceptionStackFrame, InterruptDescriptorTable};
 
 lazy_static! {
-    static ref IDT: InterruptDescriptorTable = {
+    static ref TEST_IDT: InterruptDescriptorTable = {
         let mut idt = InterruptDescriptorTable::new();
         idt.breakpoint.set_handler_fn(breakpoint_handler);
         idt
     };
 }
 
-pub fn init_idt() {
-    IDT.load();
+pub fn init_test_idt() {
+    TEST_IDT.load();
 }
 
 extern "x86-interrupt" fn breakpoint_handler(_stack_frame: &mut ExceptionStackFrame) {
diff --git a/src/bin/test-exception-double-fault-stack-overflow.rs b/src/bin/test-exception-double-fault-stack-overflow.rs
index 876e4486..30125c86 100644
--- a/src/bin/test-exception-double-fault-stack-overflow.rs
+++ b/src/bin/test-exception-double-fault-stack-overflow.rs
@@ -17,7 +17,7 @@ use core::panic::PanicInfo;
 #[allow(unconditional_recursion)]
 pub extern "C" fn _start() -> ! {
     blog_os::gdt::init();
-    init_idt();
+    init_test_idt();
 
     fn stack_overflow() {
         stack_overflow(); // for each recursion, the return address is pushed
@@ -53,7 +53,7 @@ fn panic(info: &PanicInfo) -> ! {
 use x86_64::structures::idt::{ExceptionStackFrame, InterruptDescriptorTable};
 
 lazy_static! {
-    static ref IDT: InterruptDescriptorTable = {
+    static ref TEST_IDT: InterruptDescriptorTable = {
         let mut idt = InterruptDescriptorTable::new();
         unsafe {
             idt.double_fault
@@ -65,8 +65,8 @@ lazy_static! {
     };
 }
 
-pub fn init_idt() {
-    IDT.load();
+pub fn init_test_idt() {
+    TEST_IDT.load();
 }
 
 extern "x86-interrupt" fn double_fault_handler(

From a80ae0d06a748874129b9af767e7ad82fb800023 Mon Sep 17 00:00:00 2001
From: acheronfail <acheronfail@gmail.com>
Date: Thu, 18 Oct 2018 22:57:37 +1100
Subject: [PATCH 6/6] feedback: fix some typos

---
 blog/content/second-edition/posts/06-cpu-exceptions/index.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/blog/content/second-edition/posts/06-cpu-exceptions/index.md b/blog/content/second-edition/posts/06-cpu-exceptions/index.md
index 09df2803..9fd6303a 100644
--- a/blog/content/second-edition/posts/06-cpu-exceptions/index.md
+++ b/blog/content/second-edition/posts/06-cpu-exceptions/index.md
@@ -300,7 +300,7 @@ pub mod interrupts;
 pub mod serial;
 ```
 
-Now we can use our `print!` and `println!` macros in `interrupts.rs`. If you'd like to know more about the ins and outs of macros and how they differ from functions [you can find more information here](in-depth-rust-macros).
+Now we can use our `print!` and `println!` macros in `interrupts.rs`. If you'd like to know more about the ins and outs of macros and how they differ from functions [you can find more information here][in-depth-rust-macros].
 
 [in-depth-rust-macros]: https://doc.rust-lang.org/book/second-edition/appendix-04-macros.html
 
@@ -442,7 +442,7 @@ static BREAKPOINT_HANDLER_CALLED: AtomicUsize = AtomicUsize::new(0);
 #[cfg(not(test))]
 #[no_mangle]
 pub extern "C" fn _start() -> ! {
-    blog_os::interrupts::init_idt();
+    init_idt();
 
     // invoke a breakpoint exception
     x86_64::instructions::int3();