Merge pull request #248 from dbeckwith/port-rw

Use marker types for Port read/write access

Author: josephlr

Changelog.md

@@ -1,6 +1,10 @@
 # Unreleased
 
-- The following methods no longer require the `nightly` feature to be `const fn`s`:
+- Make the following types aliases of the new `PortGeneric` type ([#248](https://github.com/rust-osdev/x86_64/pull/248)):
+  - `Port<T> = PortGeneric<T, ReadWriteAccess>`
+  - `PortReadOnly<T> = PortGeneric<T, ReadOnlyAccess>`
+  - `PortWriteOnly<T> = PortGeneric<T, WriteOnlyAccess>`
+- The following methods no longer require the `nightly` feature to be `const fn`s` ([#255](https://github.com/rust-osdev/x86_64/pull/255)):
   - `PageTable::new`
   - `GlobalDescriptorTable::from_raw_slice`
   - `MappedFrame::{start_address, size}`

src/instructions/port.rs

@@ -80,84 +80,80 @@ impl PortWrite for u32 {
     }
 }
 
-/// A read only I/O port.
-pub struct PortReadOnly<T> {
-    port: u16,
-    phantom: PhantomData<T>,
+mod sealed {
+    pub trait Access {
+        const DEBUG_NAME: &'static str;
+    }
 }
 
-/// A write only I/O port.
-pub struct PortWriteOnly<T> {
-    port: u16,
-    phantom: PhantomData<T>,
-}
+/// A marker trait for access types which allow reading port values.
+pub trait PortReadAccess: sealed::Access {}
 
-/// An I/O port.
-pub struct Port<T> {
-    port: u16,
-    phantom: PhantomData<T>,
-}
+/// A marker trait for access types which allow writing port values.
+pub trait PortWriteAccess: sealed::Access {}
 
-impl<T> PortReadOnly<T> {
-    /// Creates a read only I/O port with the given port number.
-    #[inline]
-    pub const fn new(port: u16) -> PortReadOnly<T> {
-        PortReadOnly {
-            port,
-            phantom: PhantomData,
-        }
-    }
+/// An access marker type indicating that a port is only allowed to read values.
+#[derive(Debug)]
+pub struct ReadOnlyAccess(());
+
+impl sealed::Access for ReadOnlyAccess {
+    const DEBUG_NAME: &'static str = "ReadOnly";
 }
+impl PortReadAccess for ReadOnlyAccess {}
 
-impl<T: PortRead> PortReadOnly<T> {
-    /// Reads from the port.
-    ///
-    /// ## Safety
-    ///
-    /// This function is unsafe because the I/O port could have side effects that violate memory
-    /// safety.
-    #[inline]
-    pub unsafe fn read(&mut self) -> T {
-        T::read_from_port(self.port)
-    }
+/// An access marker type indicating that a port is only allowed to write values.
+#[derive(Debug)]
+pub struct WriteOnlyAccess(());
+
+impl sealed::Access for WriteOnlyAccess {
+    const DEBUG_NAME: &'static str = "WriteOnly";
 }
+impl PortWriteAccess for WriteOnlyAccess {}
 
-impl<T> PortWriteOnly<T> {
-    /// Creates a write only I/O port with the given port number.
-    #[inline]
-    pub const fn new(port: u16) -> PortWriteOnly<T> {
-        PortWriteOnly {
-            port,
-            phantom: PhantomData,
-        }
-    }
+/// An access marker type indicating that a port is allowed to read or write values.
+#[derive(Debug)]
+pub struct ReadWriteAccess(());
+
+impl sealed::Access for ReadWriteAccess {
+    const DEBUG_NAME: &'static str = "ReadWrite";
 }
+impl PortReadAccess for ReadWriteAccess {}
+impl PortWriteAccess for ReadWriteAccess {}
 
-impl<T: PortWrite> PortWriteOnly<T> {
-    /// Writes to the port.
-    ///
-    /// ## Safety
-    ///
-    /// This function is unsafe because the I/O port could have side effects that violate memory
-    /// safety.
-    #[inline]
-    pub unsafe fn write(&mut self, value: T) {
-        T::write_to_port(self.port, value)
-    }
+/// An I/O port.
+///
+/// The port reads or writes values of type `T` and has read/write access specified by `A`.
+///
+/// Use the provided marker types or aliases to get a port type with the access you need:
+/// * `PortGeneric<T, ReadWriteAccess>` -> `Port<T>`
+/// * `PortGeneric<T, ReadOnlyAccess>` -> `PortReadOnly<T>`
+/// * `PortGeneric<T, WriteOnlyAccess>` -> `PortWriteOnly<T>`
+pub struct PortGeneric<T, A> {
+    port: u16,
+    phantom: PhantomData<(T, A)>,
 }
 
-impl<T> Port<T> {
+/// A read-write I/O port.
+pub type Port<T> = PortGeneric<T, ReadWriteAccess>;
+
+/// A read-only I/O port.
+pub type PortReadOnly<T> = PortGeneric<T, ReadOnlyAccess>;
+
+/// A write-only I/O port.
+pub type PortWriteOnly<T> = PortGeneric<T, WriteOnlyAccess>;
+
+impl<T, A> PortGeneric<T, A> {
     /// Creates an I/O port with the given port number.
     #[inline]
-    pub const fn new(port: u16) -> Port<T> {
-        Port {
+    pub const fn new(port: u16) -> PortGeneric<T, A> {
+        PortGeneric {
             port,
             phantom: PhantomData,
         }
     }
 }
 
-impl<T: PortRead> Port<T> {
+impl<T: PortRead, A: PortReadAccess> PortGeneric<T, A> {
     /// Reads from the port.
     ///
     /// ## Safety
@@ -170,7 +166,7 @@ impl<T: PortRead> Port<T> {
     }
 }
 
-impl<T: PortWrite> Port<T> {
+impl<T: PortWrite, A: PortWriteAccess> PortGeneric<T, A> {
     /// Writes to the port.
     ///
     /// ## Safety
@@ -183,35 +179,29 @@ impl<T: PortWrite> Port<T> {
     }
 }
 
-macro_rules! impl_port_util_traits {
-    ($struct_name:ident) => {
-        impl<T> fmt::Debug for $struct_name<T> {
-            fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-                f.debug_struct(stringify!($struct_name))
-                    .field("port", &self.port)
-                    .finish()
-            }
-        }
-
-        impl<T> Clone for $struct_name<T> {
-            fn clone(&self) -> Self {
-                Self {
-                    port: self.port,
-                    phantom: PhantomData,
-                }
-            }
-        }
+impl<T, A: sealed::Access> fmt::Debug for PortGeneric<T, A> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("PortGeneric")
+            .field("port", &self.port)
+            .field("size", &core::mem::size_of::<T>())
+            .field("access", &format_args!("{}", A::DEBUG_NAME))
+            .finish()
+    }
+}
 
-        impl<T> PartialEq for $struct_name<T> {
-            fn eq(&self, other: &Self) -> bool {
-                self.port == other.port
-            }
+impl<T, A> Clone for PortGeneric<T, A> {
+    fn clone(&self) -> Self {
+        Self {
+            port: self.port,
+            phantom: PhantomData,
         }
+    }
+}
 
-        impl<T> Eq for $struct_name<T> {}
-    };
+impl<T, A> PartialEq for PortGeneric<T, A> {
+    fn eq(&self, other: &Self) -> bool {
+        self.port == other.port
+    }
 }
 
-impl_port_util_traits!(Port);
-impl_port_util_traits!(PortReadOnly);
-impl_port_util_traits!(PortWriteOnly);
+impl<T, A> Eq for PortGeneric<T, A> {}

testing/src/lib.rs

@@ -19,10 +19,10 @@ pub enum QemuExitCode {
 }
 
 pub fn exit_qemu(exit_code: QemuExitCode) {
-    use x86_64::instructions::port::Port;
+    use x86_64::instructions::port::PortWriteOnly;
 
     unsafe {
-        let mut port = Port::new(0xf4);
+        let mut port = PortWriteOnly::new(0xf4);
         port.write(exit_code as u32);
     }
 }

testing/tests/port_read_write.rs

@@ -39,7 +39,7 @@ pub extern "C" fn _start() -> ! {
         // Read the test value using PortReadOnly
         let read_only_test_value = crt_data_read_only_port.read() & 0xFF;
 
-        // Read the test value using PortReadWrite
+        // Read the test value using Port
         let read_write_test_value = crt_read_write_data_port.read() & 0xFF;
 
         if read_only_test_value != TEST_VALUE {
@@ -51,7 +51,7 @@ pub extern "C" fn _start() -> ! {
 
         if read_write_test_value != TEST_VALUE {
             panic!(
-                "PortReadWrite: {} does not match expected value {}",
+                "Port: {} does not match expected value {}",
                 read_write_test_value, TEST_VALUE
             );
         }