Merge branch 'master' into next

Author: josephlr

Cargo.toml

@@ -22,7 +22,7 @@ license = "MIT/Apache-2.0"
 name = "x86_64"
 readme = "README.md"
 repository = "https://github.com/rust-osdev/x86_64"
-version = "0.14.8"
+version = "0.14.9"
 edition = "2018"
 rust-version = "1.59" # Needed to support inline asm and default const generics
 

Changelog.md

@@ -1,5 +1,37 @@
 # Unreleased
 
+# 0.14.9 - 2022-03-31
+
+## New Features
+
+- Address in `VirtAddrNotValid` and `PhysAddrNotValid` is now public ([#340](https://github.com/rust-osdev/x86_64/pull/340)).
+  - This field now contains the whole invalid address ([#347](https://github.com/rust-osdev/x86_64/pull/347)).
+- Remove all uses of external assembly ([#343](https://github.com/rust-osdev/x86_64/pull/343))
+  - `external_asm` and `inline_asm` features are deprecated and now have no effect.
+  - `instructions` feature (on by default) now requires Rust 1.59
+  - Specific MSRV now noted in `README` ([#355](https://github.com/rust-osdev/x86_64/pull/355))
+- Implement `core::iter::Step` for `VirtAddr` and `Page` ([#342](https://github.com/rust-osdev/x86_64/pull/342))
+  - This trait is only available on nightly.
+  - Gated behind `step_trait` feature flag
+- Add `UCet` and `SCet` registers ([#349](https://github.com/rust-osdev/x86_64/pull/349))
+- Use [`rustversion`](https://crates.io/crates/rustversion) to mark certian functions `const fn` on Rust 1.61 ([#353](https://github.com/rust-osdev/x86_64/pull/353))
+- `Entry::handler_addr()` is now public ([#354](https://github.com/rust-osdev/x86_64/pull/354))
+- Increase packed structure alignment ([#362](https://github.com/rust-osdev/x86_64/pull/362))
+- Make more address methods `const fn` ([#369](https://github.com/rust-osdev/x86_64/pull/369))
+  - `VirtAddr::as_ptr()`
+  - `VirtAddr::as_mut_ptr()`
+  - `PhysAddr::new()`
+  - `PhysAddr::try_new()`
+
+## Bug fixes and Documentation
+
+- Fixed overflow bug in PageRangeInclusive ([#351](https://github.com/rust-osdev/x86_64/pull/351))
+- Remove stabilized `const_fn_fn_ptr_basics` and `const_fn_trait_bound` features ([#352](https://github.com/rust-osdev/x86_64/pull/352))
+- Don't set `nomem` in `load_tss` ([#358](https://github.com/rust-osdev/x86_64/pull/358))
+- Correctly initialize TSS's IOPB to be empty ([#357](https://github.com/rust-osdev/x86_64/pull/357))
+- Improve `GlobalDescriptorTable::add_entry` error handling ([#361](https://github.com/rust-osdev/x86_64/pull/361))
+- Update `tss_segment` documentation ([#366](https://github.com/rust-osdev/x86_64/pull/366))
+
 # 0.14.8 – 2022-02-03
 
 - Add `Cr2::read_raw` ([#334](https://github.com/rust-osdev/x86_64/pull/334))

src/addr.rs

@@ -138,14 +138,14 @@ impl VirtAddr {
     /// Converts the address to a raw pointer.
     #[cfg(target_pointer_width = "64")]
     #[inline]
-    pub fn as_ptr<T>(self) -> *const T {
+    pub const fn as_ptr<T>(self) -> *const T {
         self.as_u64() as *const T
     }
 
     /// Converts the address to a mutable raw pointer.
     #[cfg(target_pointer_width = "64")]
     #[inline]
-    pub fn as_mut_ptr<T>(self) -> *mut T {
+    pub const fn as_mut_ptr<T>(self) -> *mut T {
         self.as_ptr::<T>() as *mut T
     }
 
@@ -386,13 +386,12 @@ impl PhysAddr {
     ///
     /// This function panics if a bit in the range 52 to 64 is set.
     #[inline]
-    pub fn new(addr: u64) -> PhysAddr {
-        assert_eq!(
-            addr.get_bits(52..64),
-            0,
-            "physical addresses must not have any bits in the range 52 to 64 set"
-        );
-        PhysAddr(addr)
+    pub const fn new(addr: u64) -> Self {
+        // TODO: Replace with .ok().expect(msg) when that works on stable.
+        match Self::try_new(addr) {
+            Ok(p) => p,
+            Err(_) => panic!("physical addresses must not have any bits in the range 52 to 64 set"),
+        }
     }
 
     /// Creates a new physical address, throwing bits 52..64 away.
@@ -415,10 +414,12 @@ impl PhysAddr {
     ///
     /// Fails if any bits in the range 52 to 64 are set.
     #[inline]
-    pub fn try_new(addr: u64) -> Result<PhysAddr, PhysAddrNotValid> {
-        match addr.get_bits(52..64) {
-            0 => Ok(PhysAddr(addr)), // address is valid
-            _ => Err(PhysAddrNotValid(addr)),
+    pub const fn try_new(addr: u64) -> Result<Self, PhysAddrNotValid> {
+        let p = Self::new_truncate(addr);
+        if p.0 == addr {
+            Ok(p)
+        } else {
+            Err(PhysAddrNotValid(addr))
         }
     }
 

src/instructions/tables.rs

@@ -70,14 +70,20 @@ pub fn sidt() -> DescriptorTablePointer {
 
 /// Load the task state register using the `ltr` instruction.
 ///
-/// Loading the task state register changes the type of the entry
-/// in the GDT from `Available 64-bit TSS` to `Busy 64-bit TSS`.
+/// Note that loading a TSS segment selector marks the corresponding TSS
+/// Descriptor in the GDT as "busy", preventing it from being loaded again
+/// (either on this CPU or another CPU). TSS structures (including Descriptors
+/// and Selectors) should generally be per-CPU. See
+/// [`tss_segment`](crate::structures::gdt::Descriptor::tss_segment)
+/// for more information.
+///
+/// Calling `load_tss` with a busy TSS selector results in a `#GP` exception.
 ///
 /// ## Safety
 ///
 /// This function is unsafe because the caller must ensure that the given
-/// `SegmentSelector` points to a valid TSS entry in the GDT and that loading
-/// this TSS is safe.
+/// `SegmentSelector` points to a valid TSS entry in the GDT and that the
+/// corresponding data in the TSS is valid.
 #[inline]
 pub unsafe fn load_tss(sel: SegmentSelector) {
     unsafe {

src/structures/gdt.rs

@@ -50,7 +50,7 @@ use crate::registers::segmentation::{Segment, CS, SS};
 #[derive(Debug, Clone)]
 pub struct GlobalDescriptorTable<const MAX: usize = 8> {
     table: [u64; MAX],
-    next_free: usize,
+    len: usize,
 }
 
 impl GlobalDescriptorTable {
@@ -69,7 +69,7 @@ impl<const MAX: usize> GlobalDescriptorTable<MAX> {
         assert!(MAX <= (1 << 13), "A GDT can only have at most 2^13 entries");
         Self {
             table: [0; MAX],
-            next_free: 1,
+            len: 1,
         }
     }
 
@@ -81,40 +81,48 @@ impl<const MAX: usize> GlobalDescriptorTable<MAX> {
     /// * Panics if the provided slice has more than `MAX` entries
     #[inline]
     pub const unsafe fn from_raw_slice(slice: &[u64]) -> Self {
-        let next_free = slice.len();
+        let len = slice.len();
         let mut table = [0; MAX];
         let mut idx = 0;
 
         assert!(
-            next_free <= MAX,
+            len <= MAX,
             "cannot initialize GDT with slice exceeding the maximum length"
         );
 
-        while idx != next_free {
+        while idx < len {
             table[idx] = slice[idx];
             idx += 1;
         }
 
-        Self { table, next_free }
+        Self { table, len }
     }
 
     /// Get a reference to the internal table.
     ///
     /// The resulting slice may contain system descriptors, which span two `u64`s.
     #[inline]
     pub fn as_raw_slice(&self) -> &[u64] {
-        &self.table[..self.next_free]
+        &self.table[..self.len]
     }
 
     /// Adds the given segment descriptor to the GDT, returning the segment selector.
     ///
-    /// Panics if the GDT has no free entries left.
+    /// Panics if the GDT doesn't have enough free entries to hold the Descriptor.
     #[inline]
     #[cfg_attr(feature = "const_fn", rustversion::attr(all(), const))]
     pub fn add_entry(&mut self, entry: Descriptor) -> SegmentSelector {
         let index = match entry {
-            Descriptor::UserSegment(value) => self.push(value),
+            Descriptor::UserSegment(value) => {
+                if self.len > self.table.len().saturating_sub(1) {
+                    panic!("GDT full")
+                }
+                self.push(value)
+            }
             Descriptor::SystemSegment(value_low, value_high) => {
+                if self.len > self.table.len().saturating_sub(2) {
+                    panic!("GDT requires two free spaces to hold a SystemSegment")
+                }
                 let index = self.push(value_low);
                 self.push(value_high);
                 index
@@ -170,14 +178,10 @@ impl<const MAX: usize> GlobalDescriptorTable<MAX> {
     #[inline]
     #[cfg_attr(feature = "const_fn", rustversion::attr(all(), const))]
     fn push(&mut self, value: u64) -> usize {
-        if self.next_free < self.table.len() {
-            let index = self.next_free;
-            self.table[index] = value;
-            self.next_free += 1;
-            index
-        } else {
-            panic!("GDT full");
-        }
+        let index = self.len;
+        self.table[index] = value;
+        self.len += 1;
+        index
     }
 
     /// Creates the descriptor pointer for this table. This pointer can only be
@@ -189,7 +193,7 @@ impl<const MAX: usize> GlobalDescriptorTable<MAX> {
             base: crate::VirtAddr::new(self.table.as_ptr() as u64),
             // 0 < self.next_free <= MAX <= 2^13, so the limit calculation
             // will not underflow or overflow.
-            limit: (self.next_free * size_of::<u64>() - 1) as u16,
+            limit: (self.len * size_of::<u64>() - 1) as u16,
         }
     }
 }
@@ -323,6 +327,13 @@ impl Descriptor {
     }
 
     /// Creates a TSS system descriptor for the given TSS.
+    ///
+    /// While it is possible to create multiple Descriptors that point to the
+    /// same TSS, this generally isn't recommended, as the TSS usually contains
+    /// per-CPU information such as the RSP and IST pointers. Instead, there
+    /// should be exactly one TSS and one corresponding TSS Descriptor per CPU.
+    /// Then, each of these descriptors should be placed in a GDT (which can
+    /// either be global or per-CPU).
     #[inline]
     pub fn tss_segment(tss: &'static TaskStateSegment) -> Descriptor {
         use self::DescriptorFlags as Flags;
@@ -349,6 +360,7 @@ impl Descriptor {
 #[cfg(test)]
 mod tests {
     use super::DescriptorFlags as Flags;
+    use super::*;
 
     #[test]
     #[rustfmt::skip]
@@ -362,4 +374,53 @@ mod tests {
         assert_eq!(Flags::USER_CODE32.bits(),   0x00cffb000000ffff);
         assert_eq!(Flags::USER_DATA.bits(),     0x00cff3000000ffff);
     }
+
+    // Makes a GDT that has two free slots
+    fn make_six_entry_gdt() -> GlobalDescriptorTable {
+        let mut gdt = GlobalDescriptorTable::new();
+        gdt.add_entry(Descriptor::kernel_code_segment());
+        gdt.add_entry(Descriptor::kernel_data_segment());
+        gdt.add_entry(Descriptor::UserSegment(DescriptorFlags::USER_CODE32.bits()));
+        gdt.add_entry(Descriptor::user_data_segment());
+        gdt.add_entry(Descriptor::user_code_segment());
+        assert_eq!(gdt.len, 6);
+        gdt
+    }
+
+    static TSS: TaskStateSegment = TaskStateSegment::new();
+
+    fn make_full_gdt() -> GlobalDescriptorTable {
+        let mut gdt = make_six_entry_gdt();
+        gdt.add_entry(Descriptor::tss_segment(&TSS));
+        assert_eq!(gdt.len, 8);
+        gdt
+    }
+
+    #[test]
+    pub fn push_max_segments() {
+        // Make sure we don't panic with user segments
+        let mut gdt = make_six_entry_gdt();
+        gdt.add_entry(Descriptor::user_data_segment());
+        assert_eq!(gdt.len, 7);
+        gdt.add_entry(Descriptor::user_data_segment());
+        assert_eq!(gdt.len, 8);
+        // Make sure we don't panic with system segments
+        let _ = make_full_gdt();
+    }
+
+    #[test]
+    #[should_panic]
+    pub fn panic_user_segment() {
+        let mut gdt = make_full_gdt();
+        gdt.add_entry(Descriptor::user_data_segment());
+    }
+
+    #[test]
+    #[should_panic]
+    pub fn panic_system_segment() {
+        let mut gdt = make_six_entry_gdt();
+        gdt.add_entry(Descriptor::user_data_segment());
+        // We have one free slot, but the GDT requires two
+        gdt.add_entry(Descriptor::tss_segment(&TSS));
+    }
 }

src/structures/mod.rs

@@ -13,10 +13,28 @@ pub mod tss;
 /// A struct describing a pointer to a descriptor table (GDT / IDT).
 /// This is in a format suitable for giving to 'lgdt' or 'lidt'.
 #[derive(Debug, Clone, Copy)]
-#[repr(C, packed)]
+#[repr(C, packed(2))]
 pub struct DescriptorTablePointer {
     /// Size of the DT.
     pub limit: u16,
     /// Pointer to the memory region containing the DT.
     pub base: VirtAddr,
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::mem::size_of;
+
+    #[test]
+    pub fn check_descriptor_pointer_size() {
+        // Per the SDM, a descriptor pointer has to be 2+8=10 bytes
+        assert_eq!(size_of::<DescriptorTablePointer>(), 10);
+        // Make sure that we can reference a pointer's limit
+        let p = DescriptorTablePointer {
+            limit: 5,
+            base: VirtAddr::zero(),
+        };
+        let _: &u16 = &p.limit;
+    }
+}

src/structures/tss.rs

@@ -8,7 +8,7 @@ use core::mem::size_of;
 /// but is used for finding kernel level stack
 /// if interrupts arrive while in kernel mode.
 #[derive(Debug, Clone, Copy)]
-#[repr(C, packed)]
+#[repr(C, packed(4))]
 pub struct TaskStateSegment {
     reserved_1: u32,
     /// The full 64-bit canonical forms of the stack pointers (RSP) for privilege levels 0-2.