From 3c48283d816926375412a7b2c52973e46e6f99c6 Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Thu, 31 Mar 2022 00:06:16 -0700 Subject: [PATCH 1/4] VirtAddr: Remove 32-bit conversion Signed-off-by: Joe Richey --- src/addr.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/addr.rs b/src/addr.rs index 46852c081..744a8acd6 100644 --- a/src/addr.rs +++ b/src/addr.rs @@ -125,11 +125,7 @@ impl VirtAddr { } /// Creates a virtual address from the given pointer - // cfg(target_pointer_width = "32") is only here for backwards - // compatibility: Earlier versions of this crate did not have any `cfg()` - // on this function. At least for 32- and 64-bit we know the `as u64` cast - // doesn't truncate. - #[cfg(any(target_pointer_width = "32", target_pointer_width = "64"))] + #[cfg(target_pointer_width = "64")] #[inline] pub fn from_ptr(ptr: *const T) -> Self { Self::new(ptr as u64) From b0d47b34541d086daede63f1b4a02be879b1f630 Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Thu, 31 Mar 2022 00:11:54 -0700 Subject: [PATCH 2/4] VirtAddr: don't succeed on non-canonical addresses Signed-off-by: Joe Richey --- src/addr.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/addr.rs b/src/addr.rs index 744a8acd6..bacf2066d 100644 --- a/src/addr.rs +++ b/src/addr.rs @@ -66,7 +66,8 @@ impl VirtAddr { /// /// ## Panics /// - /// This function panics if the bits in the range 48 to 64 contain data (i.e. are not null and no sign extension). + /// This function panics if the bits in the range 48 to 64 are invalid + /// (i.e. are not a proper sign extension of bit 47). #[inline] pub fn new(addr: u64) -> VirtAddr { Self::try_new(addr).expect( @@ -77,16 +78,16 @@ impl VirtAddr { /// Tries to create a new canonical virtual address. /// - /// This function tries to performs sign - /// extension of bit 47 to make the address canonical. It succeeds if bits 48 to 64 are - /// either a correct sign extension (i.e. copies of bit 47) or all null. Else, an error - /// is returned. + /// This function tries to performs sign extension of bit 47 to make the + /// address canonical. It succeeds if bits 48 to 64 are a correct sign + /// extension (i.e. copies of bit 47). Else, an error is returned. #[inline] pub fn try_new(addr: u64) -> Result { - match addr.get_bits(47..64) { - 0 | 0x1ffff => Ok(VirtAddr(addr)), // address is canonical - 1 => Ok(VirtAddr::new_truncate(addr)), // address needs sign extension - _ => Err(VirtAddrNotValid(addr)), + let v = Self::new_truncate(addr); + if v.0 == addr { + Ok(v) + } else { + Err(VirtAddrNotValid(addr)) } } From 49b41c8332f36177baaa17d0ca51729fbe0a4ca2 Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Thu, 31 Mar 2022 00:15:41 -0700 Subject: [PATCH 3/4] VirtAddr: make new() and try_new() const Signed-off-by: Joe Richey --- src/addr.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/addr.rs b/src/addr.rs index bacf2066d..03ed5125a 100644 --- a/src/addr.rs +++ b/src/addr.rs @@ -69,11 +69,12 @@ impl VirtAddr { /// This function panics if the bits in the range 48 to 64 are invalid /// (i.e. are not a proper sign extension of bit 47). #[inline] - pub fn new(addr: u64) -> VirtAddr { - Self::try_new(addr).expect( - "address passed to VirtAddr::new must not contain any data \ - in bits 48 to 64", - ) + pub const fn new(addr: u64) -> VirtAddr { + // TODO: Replace with .ok().expect(msg) when that works on stable. + match Self::try_new(addr) { + Ok(v) => v, + Err(_) => panic!("virtual address must be sign extended in bits 48 to 64"), + } } /// Tries to create a new canonical virtual address. @@ -82,7 +83,7 @@ impl VirtAddr { /// address canonical. It succeeds if bits 48 to 64 are a correct sign /// extension (i.e. copies of bit 47). Else, an error is returned. #[inline] - pub fn try_new(addr: u64) -> Result { + pub const fn try_new(addr: u64) -> Result { let v = Self::new_truncate(addr); if v.0 == addr { Ok(v) From 872edf7589de425a24c9aa482ded711cf38847c1 Mon Sep 17 00:00:00 2001 From: Joseph Richey Date: Thu, 31 Mar 2022 10:09:51 -0700 Subject: [PATCH 4/4] Update VirtAddr documentation and fix typos Co-authored-by: Philipp Oppermann Signed-off-by: Joe Richey --- src/addr.rs | 18 ++++++++++-------- src/instructions/interrupts.rs | 4 ++-- src/structures/idt.rs | 6 +++--- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/addr.rs b/src/addr.rs index 03ed5125a..84373160a 100644 --- a/src/addr.rs +++ b/src/addr.rs @@ -22,7 +22,7 @@ const ADDRESS_SPACE_SIZE: u64 = 0x1_0000_0000_0000; /// between `u64` and `usize`. /// /// On `x86_64`, only the 48 lower bits of a virtual address can be used. The top 16 bits need -/// to be copies of bit 47, i.e. the most significant bit. Addresses that fulfil this criterium +/// to be copies of bit 47, i.e. the most significant bit. Addresses that fulfil this criterion /// are called “canonical”. This type guarantees that it always represents a canonical address. #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] #[repr(transparent)] @@ -62,7 +62,8 @@ impl core::fmt::Debug for VirtAddrNotValid { impl VirtAddr { /// Creates a new canonical virtual address. /// - /// This function performs sign extension of bit 47 to make the address canonical. + /// The provided address should already be canonical. If you want to check + /// whether an address is canonical, use [`try_new`](Self::try_new). /// /// ## Panics /// @@ -79,9 +80,10 @@ impl VirtAddr { /// Tries to create a new canonical virtual address. /// - /// This function tries to performs sign extension of bit 47 to make the - /// address canonical. It succeeds if bits 48 to 64 are a correct sign - /// extension (i.e. copies of bit 47). Else, an error is returned. + /// This function checks wether the given address is canonical + /// and returns an error otherwise. An address is canonical + /// if bits 48 to 64 are a correct sign + /// extension (i.e. copies of bit 47). #[inline] pub const fn try_new(addr: u64) -> Result { let v = Self::new_truncate(addr); @@ -94,9 +96,9 @@ impl VirtAddr { /// Creates a new canonical virtual address, throwing out bits 48..64. /// - /// This function performs sign extension of bit 47 to make the address canonical, so - /// bits 48 to 64 are overwritten. If you want to check that these bits contain no data, - /// use `new` or `try_new`. + /// This function performs sign extension of bit 47 to make the address + /// canonical, overwriting bits 48 to 64. If you want to check whether an + /// address is canonical, use [`new`](Self::new) or [`try_new`](Self::try_new). #[inline] pub const fn new_truncate(addr: u64) -> VirtAddr { // By doing the right shift as a signed operation (on a i64), it will diff --git a/src/instructions/interrupts.rs b/src/instructions/interrupts.rs index 2176d42b1..7c5f4900e 100644 --- a/src/instructions/interrupts.rs +++ b/src/instructions/interrupts.rs @@ -109,10 +109,10 @@ where /// On some processors, the interrupt shadow of `sti` does not apply to /// non-maskable interrupts (NMIs). This means that an NMI can occur between /// the `sti` and `hlt` instruction, with the result that the CPU is put to -/// sleep even though a new interrupt occured. +/// sleep even though a new interrupt occurred. /// /// To work around this, it is recommended to check in the NMI handler if -/// the interrupt occured between `sti` and `hlt` instructions. If this is the +/// the interrupt occurred between `sti` and `hlt` instructions. If this is the /// case, the handler should increase the instruction pointer stored in the /// interrupt stack frame so that the `hlt` instruction is skipped. /// diff --git a/src/structures/idt.rs b/src/structures/idt.rs index e8cab39e0..5ad61e507 100644 --- a/src/structures/idt.rs +++ b/src/structures/idt.rs @@ -42,7 +42,7 @@ use super::gdt::SegmentSelector; /// first entry, the entry for the `divide_error` exception. Note that the index access is /// not possible for entries for which an error code is pushed. /// -/// The remaining entries are used for interrupts. They can be accesed through index +/// The remaining entries are used for interrupts. They can be accessed through index /// operations on the idt, e.g. `idt[32]` returns the first interrupt entry, which is the 32nd IDT /// entry). /// @@ -1263,7 +1263,7 @@ macro_rules! set_general_handler { #[macro_export] #[doc(hidden)] /// We can't loop in macros, but we can use recursion. -/// This macro recursivly adds one more bit to it's arguments until we have 8 bits so that we can call set_general_handler_entry. +/// This macro recursively adds one more bit to it's arguments until we have 8 bits so that we can call set_general_handler_entry. macro_rules! set_general_handler_recursive_bits { // if we have 8 all bits, construct the index from the bits, check if the entry is in range and invoke the macro that sets the handler ($idt:expr, $handler:ident, $range:expr, $bit7:tt, $bit6:tt, $bit5:tt, $bit4:tt, $bit3:tt, $bit2:tt, $bit1:tt, $bit0:tt) => {{ @@ -1274,7 +1274,7 @@ macro_rules! set_general_handler_recursive_bits { $crate::set_general_handler_entry!($idt, $handler, IDX, $bit7, $bit6, $bit5, $bit4, $bit3, $bit2, $bit1, $bit0); } }}; - // otherwise recursivly invoke the macro adding one more bit + // otherwise recursively invoke the macro adding one more bit ($idt:expr, $handler:ident, $range:expr $(, $bits:tt)*) => { $crate::set_general_handler_recursive_bits!($idt, $handler, $range $(, $bits)*, 0); $crate::set_general_handler_recursive_bits!($idt, $handler, $range $(, $bits)*, 1);