From bacc76c1b4a1477875d90f3f12361f7e1ab1ce76 Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Tue, 12 Jan 2021 02:40:33 -0800 Subject: [PATCH 1/2] idt: Fixup Options structure and cleanup set_handler_fn This change: - Moves the IDT entry's CS GDT selector to the options structure - It also now uses SegmentSelector - The `set_handler_fn` is now a proper generic function - This is done by adding the `InterruptFn` trait - Change behavior (and document) so `set_handler_fn` resets _all_ the options (as opposed to some). Signed-off-by: Joe Richey --- src/structures/idt.rs | 97 ++++++++++++++++++++++++++----------------- 1 file changed, 58 insertions(+), 39 deletions(-) diff --git a/src/structures/idt.rs b/src/structures/idt.rs index dd605f20a..acb4ac602 100644 --- a/src/structures/idt.rs +++ b/src/structures/idt.rs @@ -17,6 +17,8 @@ use core::marker::PhantomData; use core::ops::Bound::{Excluded, Included, Unbounded}; use core::ops::{Deref, Index, IndexMut, RangeBounds}; +use super::gdt::SegmentSelector; + /// An Interrupt Descriptor Table with 256 entries. /// /// The first 32 entries are used for CPU exceptions. These entries can be either accessed through @@ -557,13 +559,11 @@ impl IndexMut for InterruptDescriptorTable { /// An Interrupt Descriptor Table entry. /// -/// The generic parameter can either be `HandlerFunc` or `HandlerFuncWithErrCode`, depending -/// on the interrupt vector. +/// The generic parameter is some [`InterruptFn`], depending on the interrupt vector. #[derive(Debug, Clone, Copy, PartialEq)] #[repr(C)] pub struct Entry { pointer_low: u16, - gdt_selector: u16, options: EntryOptions, pointer_middle: u16, pointer_high: u32, @@ -590,7 +590,6 @@ impl Entry { #[inline] pub const fn missing() -> Self { Entry { - gdt_selector: 0, pointer_low: 0, pointer_middle: 0, pointer_high: 0, @@ -599,71 +598,91 @@ impl Entry { phantom: PhantomData, } } +} - /// Set the handler address for the IDT entry and sets the present bit. - /// - /// For the code selector field, this function uses the code segment selector currently - /// active in the CPU. +#[cfg(feature = "instructions")] +impl Entry { + /// Sets the handler function for the IDT entry and sets the following defaults: + /// - The code selector is the code segment currently active in the CPU + /// - The present bit is set + /// - Interrupts are disabled on handler invocation + /// - The privilege level (DPL) is [`PrivilegeLevel::Ring0`] + /// - No IST is configured (existing stack will be used) /// /// The function returns a mutable reference to the entry's options that allows /// further customization. - #[cfg(feature = "instructions")] - #[inline] - fn set_handler_addr(&mut self, addr: u64) -> &mut EntryOptions { + pub fn set_handler_fn(&mut self, handler: F) -> &mut EntryOptions { use crate::instructions::segmentation; + let addr = handler.addr().as_u64(); self.pointer_low = addr as u16; self.pointer_middle = (addr >> 16) as u16; self.pointer_high = (addr >> 32) as u32; - self.gdt_selector = segmentation::cs().0; - + self.options = EntryOptions::minimal(); + // SAFETY: The current CS is a valid, long-mode code segment. + unsafe { self.options.set_code_selector(segmentation::cs()) }; self.options.set_present(true); &mut self.options } } -macro_rules! impl_set_handler_fn { +/// A trait for function pointers that can be used as interrupt handlers +pub trait InterruptFn { + /// The virtual address of this function pointer + fn addr(self) -> VirtAddr; +} + +macro_rules! impl_interrupt_fn { ($h:ty) => { - #[cfg(feature = "instructions")] - impl Entry<$h> { - /// Set the handler function for the IDT entry and sets the present bit. - /// - /// For the code selector field, this function uses the code segment selector currently - /// active in the CPU. - /// - /// The function returns a mutable reference to the entry's options that allows - /// further customization. - #[inline] - pub fn set_handler_fn(&mut self, handler: $h) -> &mut EntryOptions { - self.set_handler_addr(handler as u64) + #[cfg(target_pointer_width = "64")] + impl InterruptFn for $h { + fn addr(self) -> VirtAddr { + VirtAddr::from_ptr(self as *const ()) } } }; } -impl_set_handler_fn!(HandlerFunc); -impl_set_handler_fn!(HandlerFuncWithErrCode); -impl_set_handler_fn!(PageFaultHandlerFunc); -impl_set_handler_fn!(DivergingHandlerFunc); -impl_set_handler_fn!(DivergingHandlerFuncWithErrCode); +impl_interrupt_fn!(HandlerFunc); +impl_interrupt_fn!(HandlerFuncWithErrCode); +impl_interrupt_fn!(PageFaultHandlerFunc); +impl_interrupt_fn!(DivergingHandlerFunc); +impl_interrupt_fn!(DivergingHandlerFuncWithErrCode); -/// Represents the options field of an IDT entry. -#[repr(transparent)] +/// Represents the 4 non-offset bytes of an IDT Gate Descriptor. +#[repr(C)] #[derive(Debug, Clone, Copy, PartialEq)] -pub struct EntryOptions(u16); +pub struct EntryOptions { + cs: SegmentSelector, + bits: u16, +} impl EntryOptions { /// Creates a minimal options field with all the must-be-one bits set. #[inline] const fn minimal() -> Self { - EntryOptions(0b1110_0000_0000) + EntryOptions { + cs: SegmentSelector(0), + bits: 0b1110_0000_0000, // Default to a 64-bit Interrupt Gate + } + } + + /// Set the code segment that will be used by this interrupt. By default, + /// the current code segment is used. + /// + /// ## Safety + /// This function is unsafe because the caller must ensure that the passed + /// segment selector points to a valid, long-mode code segment. + pub unsafe fn set_code_selector(&mut self, cs: SegmentSelector) -> &mut Self { + self.cs = cs; + self } /// Set or reset the preset bit. #[inline] pub fn set_present(&mut self, present: bool) -> &mut Self { - self.0.set_bit(15, present); + self.bits.set_bit(15, present); self } @@ -671,7 +690,7 @@ impl EntryOptions { /// interrupts are disabled on handler invocation. #[inline] pub fn disable_interrupts(&mut self, disable: bool) -> &mut Self { - self.0.set_bit(8, !disable); + self.bits.set_bit(8, !disable); self } @@ -681,7 +700,7 @@ impl EntryOptions { /// This function panics for a DPL > 3. #[inline] pub fn set_privilege_level(&mut self, dpl: PrivilegeLevel) -> &mut Self { - self.0.set_bits(13..15, dpl as u16); + self.bits.set_bits(13..15, dpl as u16); self } @@ -701,7 +720,7 @@ impl EntryOptions { pub unsafe fn set_stack_index(&mut self, index: u16) -> &mut Self { // The hardware IST index starts at 1, but our software IST index // starts at 0. Therefore we need to add 1 here. - self.0.set_bits(0..3, index + 1); + self.bits.set_bits(0..3, index + 1); self } } From de622b2c800f9da70b8bad27afbb56cae1438689 Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Tue, 12 Jan 2021 04:30:29 -0800 Subject: [PATCH 2/2] Update comments to clarify default behavior Signed-off-by: Joe Richey --- src/structures/idt.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/structures/idt.rs b/src/structures/idt.rs index acb4ac602..89b21d41d 100644 --- a/src/structures/idt.rs +++ b/src/structures/idt.rs @@ -659,7 +659,8 @@ pub struct EntryOptions { } impl EntryOptions { - /// Creates a minimal options field with all the must-be-one bits set. + /// Creates a minimal options field with all the must-be-one bits set. This + /// means the CS selector, IST, and DPL field are all 0. #[inline] const fn minimal() -> Self { EntryOptions { @@ -668,8 +669,7 @@ impl EntryOptions { } } - /// Set the code segment that will be used by this interrupt. By default, - /// the current code segment is used. + /// Set the code segment that will be used by this interrupt. /// /// ## Safety /// This function is unsafe because the caller must ensure that the passed