[PATCH v3 04/16] rust: io: perform conversions using `AsRepr`

Gary Guo <[email protected]>
Newsgroups org.kernel.vger.rust-for-linux,dev.linux.lists.driver-core,dev.linux.lists.nova-gpu,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci
Message-ID <[email protected]>
For types that are layout-compatible with an I/O capable type, we would
want the ability to use them directly for I/O operations. E.g.

    bitfield! {
        pub struct Foo(u32) {
            ...
        }
    }

    #[repr(C)]
    struct Bar {
        foo: Foo,
    }

    let mmio: Mmio<'_, Bar> = ...;
    io_read!(mmio, .foo)

Currently this feature is available from `register!()` macro but not
otherwise available with `io_read!`, `io_write!`. Support this by
performing conversions to I/O primitives via the `AsRepr`/`AsReprMut`
trait.

This makes the `IoLoc::IoType` and `Register::Storage` redundant; thus
remove them; also convert register methods to use the `read_val` and
`write_val` instead.

Signed-off-by: Gary Guo <[email protected]>
---
 rust/kernel/bitfield.rs    |  10 ++++
 rust/kernel/io.rs          | 135 +++++++++++++++++++++++++--------------------
 rust/kernel/io/register.rs |  15 -----
 rust/macros/io/register.rs |   2 -
 4 files changed, 86 insertions(+), 76 deletions(-)

diff --git a/rust/kernel/bitfield.rs b/rust/kernel/bitfield.rs
index a0d089423f21..619c5e2189d1 100644
--- a/rust/kernel/bitfield.rs
+++ b/rust/kernel/bitfield.rs
@@ -308,6 +308,7 @@ macro_rules! bitfield {
         $(#[$attr])*
         #[repr(transparent)]
         #[derive(Clone, Copy, PartialEq, Eq)]
+        #[derive($crate::prelude::FromBytes, $crate::prelude::IntoBytes)]
         $vis struct $name {
             inner: $storage,
         }
@@ -346,6 +347,15 @@ fn from(val: $storage) -> $name {
                 Self::from_raw(val)
             }
         }
+
+        // SAFETY: `$name` is transparent over `$storage`.
+        unsafe impl $crate::mem::AsRepr for $name {
+            // Normalize `$storage` to the canonical repr type in case it is signed.
+            type Repr = <$storage as $crate::mem::AsRepr>::Repr;
+        }
+
+        // SAFETY: `$name` is transparent over `$storage`.
+        unsafe impl $crate::mem::AsReprMut for $name {}
     };
 
     // Definitions requiring knowledge of individual fields: private and public field accessors,
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 5ce9fd129068..6c087d30a166 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -11,6 +11,10 @@
 
 use crate::{
     bindings,
+    mem::{
+        AsRepr,
+        AsReprMut, //
+    },
     prelude::*,
     ptr::{
         Alignment,
@@ -353,15 +357,12 @@ fn copy_write<T: IntoBytes>(view: Self::View<'_, T>, value: T) {
 ///
 /// - The valid `Base` to operate on. For most registers, this should be [`Region`].
 /// - The offset to access (returned by [`IoLoc::offset`]),
-/// - The width of the access (determined by [`IoLoc::IoType`]),
-/// - The type `T` in which the raw data is returned or provided.
+/// - The type `T` in which the data is returned or provided.
 ///
-/// `T` and `IoLoc::IoType` may differ: for instance, a typed register has `T` = the register type
-/// with its bitfields, and `IoType` = its backing primitive (e.g. `u32`).
+/// `T` is not necessarily the type for underlying I/O operation. Methods that take `IoLoc` have `T:
+/// AsRepr` bound and the `<T as AsRepr>::Repr` type would be used to perform I/O and converted to
+/// `T` instead.
 pub trait IoLoc<Base: ?Sized, T> {
-    /// Size ([`u8`], [`u16`], etc) of the I/O performed on the returned [`offset`](IoLoc::offset).
-    type IoType: Into<T> + From<T>;
-
     /// Consumes `self` and returns the offset of this location.
     fn offset(self) -> usize;
 }
@@ -372,8 +373,6 @@ macro_rules! impl_usize_ioloc {
     ($($ty:ty),*) => {
         $(
             impl<const SIZE: usize> IoLoc<Region<SIZE>, $ty> for usize {
-                type IoType = $ty;
-
                 #[inline(always)]
                 fn offset(self) -> usize {
                     self
@@ -498,10 +497,19 @@ fn try_cast<U>(self) -> Result<<Self::Backend as IoBackend>::View<'a, U>>
     #[inline]
     fn read_val(self) -> Self::Target
     where
-        Self::Backend: IoCapable<Self::Target>,
-        Self::Target: Sized,
+        Self::Target: AsReprMut,
+        Self::Backend: IoCapable<<Self::Target as AsRepr>::Repr>,
     {
-        Self::Backend::io_read(self.as_view())
+        let view = self.as_view();
+        // SAFETY: `AsRepr` guarantees layout compatibility.
+        let repr_view = unsafe {
+            Self::Backend::project_view(
+                view,
+                Self::Backend::as_ptr(view).cast::<<Self::Target as AsRepr>::Repr>(),
+            )
+        };
+
+        Self::Target::from_repr(Self::Backend::io_read(repr_view))
     }
 
     /// Write a value to I/O.
@@ -520,10 +528,19 @@ fn read_val(self) -> Self::Target
     #[inline]
     fn write_val(self, value: Self::Target)
     where
-        Self::Backend: IoCapable<Self::Target>,
-        Self::Target: Sized,
+        Self::Target: AsRepr,
+        Self::Backend: IoCapable<<Self::Target as AsRepr>::Repr>,
     {
-        Self::Backend::io_write(self.as_view(), value)
+        let view = self.as_view();
+        // SAFETY: `AsRepr` guarantees layout compatibility.
+        let repr_view = unsafe {
+            Self::Backend::project_view(
+                view,
+                Self::Backend::as_ptr(view).cast::<<Self::Target as AsRepr>::Repr>(),
+            )
+        };
+
+        Self::Backend::io_write(repr_view, Self::Target::into_repr(value))
     }
 
     /// Copy-read from I/O memory.
@@ -645,7 +662,7 @@ fn copy_to_slice(self, data: &mut [u8])
     #[inline(always)]
     fn try_read8(self, offset: usize) -> Result<u8>
     where
-        usize: IoLoc<Self::Target, u8, IoType = u8>,
+        usize: IoLoc<Self::Target, u8>,
         Self::Backend: IoCapable<u8>,
     {
         self.try_read(offset)
@@ -655,7 +672,7 @@ fn try_read8(self, offset: usize) -> Result<u8>
     #[inline(always)]
     fn try_read16(self, offset: usize) -> Result<u16>
     where
-        usize: IoLoc<Self::Target, u16, IoType = u16>,
+        usize: IoLoc<Self::Target, u16>,
         Self::Backend: IoCapable<u16>,
     {
         self.try_read(offset)
@@ -665,7 +682,7 @@ fn try_read16(self, offset: usize) -> Result<u16>
     #[inline(always)]
     fn try_read32(self, offset: usize) -> Result<u32>
     where
-        usize: IoLoc<Self::Target, u32, IoType = u32>,
+        usize: IoLoc<Self::Target, u32>,
         Self::Backend: IoCapable<u32>,
     {
         self.try_read(offset)
@@ -675,7 +692,7 @@ fn try_read32(self, offset: usize) -> Result<u32>
     #[inline(always)]
     fn try_read64(self, offset: usize) -> Result<u64>
     where
-        usize: IoLoc<Self::Target, u64, IoType = u64>,
+        usize: IoLoc<Self::Target, u64>,
         Self::Backend: IoCapable<u64>,
     {
         self.try_read(offset)
@@ -685,7 +702,7 @@ fn try_read64(self, offset: usize) -> Result<u64>
     #[inline(always)]
     fn try_write8(self, value: u8, offset: usize) -> Result
     where
-        usize: IoLoc<Self::Target, u8, IoType = u8>,
+        usize: IoLoc<Self::Target, u8>,
         Self::Backend: IoCapable<u8>,
     {
         self.try_write(offset, value)
@@ -695,7 +712,7 @@ fn try_write8(self, value: u8, offset: usize) -> Result
     #[inline(always)]
     fn try_write16(self, value: u16, offset: usize) -> Result
     where
-        usize: IoLoc<Self::Target, u16, IoType = u16>,
+        usize: IoLoc<Self::Target, u16>,
         Self::Backend: IoCapable<u16>,
     {
         self.try_write(offset, value)
@@ -705,7 +722,7 @@ fn try_write16(self, value: u16, offset: usize) -> Result
     #[inline(always)]
     fn try_write32(self, value: u32, offset: usize) -> Result
     where
-        usize: IoLoc<Self::Target, u32, IoType = u32>,
+        usize: IoLoc<Self::Target, u32>,
         Self::Backend: IoCapable<u32>,
     {
         self.try_write(offset, value)
@@ -715,7 +732,7 @@ fn try_write32(self, value: u32, offset: usize) -> Result
     #[inline(always)]
     fn try_write64(self, value: u64, offset: usize) -> Result
     where
-        usize: IoLoc<Self::Target, u64, IoType = u64>,
+        usize: IoLoc<Self::Target, u64>,
         Self::Backend: IoCapable<u64>,
     {
         self.try_write(offset, value)
@@ -727,7 +744,7 @@ fn try_write64(self, value: u64, offset: usize) -> Result
     #[inline(always)]
     fn read8(self, offset: usize) -> u8
     where
-        usize: IoLoc<Self::Target, u8, IoType = u8>,
+        usize: IoLoc<Self::Target, u8>,
         Self::Backend: IoCapable<u8>,
     {
         self.read(offset)
@@ -739,7 +756,7 @@ fn read8(self, offset: usize) -> u8
     #[inline(always)]
     fn read16(self, offset: usize) -> u16
     where
-        usize: IoLoc<Self::Target, u16, IoType = u16>,
+        usize: IoLoc<Self::Target, u16>,
         Self::Backend: IoCapable<u16>,
     {
         self.read(offset)
@@ -751,7 +768,7 @@ fn read16(self, offset: usize) -> u16
     #[inline(always)]
     fn read32(self, offset: usize) -> u32
     where
-        usize: IoLoc<Self::Target, u32, IoType = u32>,
+        usize: IoLoc<Self::Target, u32>,
         Self::Backend: IoCapable<u32>,
     {
         self.read(offset)
@@ -763,7 +780,7 @@ fn read32(self, offset: usize) -> u32
     #[inline(always)]
     fn read64(self, offset: usize) -> u64
     where
-        usize: IoLoc<Self::Target, u64, IoType = u64>,
+        usize: IoLoc<Self::Target, u64>,
         Self::Backend: IoCapable<u64>,
     {
         self.read(offset)
@@ -775,7 +792,7 @@ fn read64(self, offset: usize) -> u64
     #[inline(always)]
     fn write8(self, value: u8, offset: usize)
     where
-        usize: IoLoc<Self::Target, u8, IoType = u8>,
+        usize: IoLoc<Self::Target, u8>,
         Self::Backend: IoCapable<u8>,
     {
         self.write(offset, value)
@@ -787,7 +804,7 @@ fn write8(self, value: u8, offset: usize)
     #[inline(always)]
     fn write16(self, value: u16, offset: usize)
     where
-        usize: IoLoc<Self::Target, u16, IoType = u16>,
+        usize: IoLoc<Self::Target, u16>,
         Self::Backend: IoCapable<u16>,
     {
         self.write(offset, value)
@@ -799,7 +816,7 @@ fn write16(self, value: u16, offset: usize)
     #[inline(always)]
     fn write32(self, value: u32, offset: usize)
     where
-        usize: IoLoc<Self::Target, u32, IoType = u32>,
+        usize: IoLoc<Self::Target, u32>,
         Self::Backend: IoCapable<u32>,
     {
         self.write(offset, value)
@@ -811,7 +828,7 @@ fn write32(self, value: u32, offset: usize)
     #[inline(always)]
     fn write64(self, value: u64, offset: usize)
     where
-        usize: IoLoc<Self::Target, u64, IoType = u64>,
+        usize: IoLoc<Self::Target, u64>,
         Self::Backend: IoCapable<u64>,
     {
         self.write(offset, value)
@@ -843,11 +860,12 @@ fn write64(self, value: u64, offset: usize)
     #[inline(always)]
     fn try_read<T, L>(self, location: L) -> Result<T>
     where
+        T: AsReprMut,
         L: IoLoc<Self::Target, T>,
-        Self::Backend: IoCapable<L::IoType>,
+        Self::Backend: IoCapable<<T as AsRepr>::Repr>,
     {
-        let view = io_view::<Self, L::IoType>(self, location.offset())?;
-        Ok(Self::Backend::io_read(view).into())
+        let view = io_view::<Self, T>(self, location.offset())?;
+        Ok(view.read_val())
     }
 
     /// Generic fallible write with runtime bounds check.
@@ -876,12 +894,12 @@ fn try_read<T, L>(self, location: L) -> Result<T>
     #[inline(always)]
     fn try_write<T, L>(self, location: L, value: T) -> Result
     where
+        T: AsRepr,
         L: IoLoc<Self::Target, T>,
-        Self::Backend: IoCapable<L::IoType>,
+        Self::Backend: IoCapable<<T as AsRepr>::Repr>,
     {
-        let view = io_view::<Self, L::IoType>(self, location.offset())?;
-        let io_value = value.into();
-        Self::Backend::io_write(view, io_value);
+        let view = io_view::<Self, T>(self, location.offset())?;
+        view.write_val(value);
         Ok(())
     }
 
@@ -920,9 +938,10 @@ fn try_write<T, L>(self, location: L, value: T) -> Result
     #[inline(always)]
     fn try_write_reg<T, L, V>(self, value: V) -> Result
     where
+        T: AsRepr,
         L: IoLoc<Self::Target, T>,
         V: LocatedRegister<Self::Target, Location = L, Value = T>,
-        Self::Backend: IoCapable<L::IoType>,
+        Self::Backend: IoCapable<<T as AsRepr>::Repr>,
     {
         let (location, value) = value.into_io_op();
 
@@ -954,16 +973,13 @@ fn try_write_reg<T, L, V>(self, value: V) -> Result
     #[inline(always)]
     fn try_update<T, L, F>(self, location: L, f: F) -> Result
     where
+        T: AsReprMut,
         L: IoLoc<Self::Target, T>,
-        Self::Backend: IoCapable<L::IoType>,
+        Self::Backend: IoCapable<<T as AsRepr>::Repr>,
         F: FnOnce(T) -> T,
     {
-        let view = io_view::<Self, L::IoType>(self, location.offset())?;
-
-        let value: T = Self::Backend::io_read(view).into();
-        let io_value = f(value).into();
-        Self::Backend::io_write(view, io_value);
-
+        let view = io_view::<Self, T>(self, location.offset())?;
+        view.write_val(f(view.read_val()));
         Ok(())
     }
 
@@ -991,11 +1007,12 @@ fn try_update<T, L, F>(self, location: L, f: F) -> Result
     #[inline(always)]
     fn read<T, L>(self, location: L) -> T
     where
+        T: AsReprMut,
         L: IoLoc<Self::Target, T>,
-        Self::Backend: IoCapable<L::IoType>,
+        Self::Backend: IoCapable<<T as AsRepr>::Repr>,
     {
-        let view = io_view_assert::<Self, L::IoType>(self, location.offset());
-        Self::Backend::io_read(view).into()
+        let view = io_view_assert::<Self, T>(self, location.offset());
+        view.read_val()
     }
 
     /// Generic infallible write with compile-time bounds check.
@@ -1022,12 +1039,12 @@ fn read<T, L>(self, location: L) -> T
     #[inline(always)]
     fn write<T, L>(self, location: L, value: T)
     where
+        T: AsRepr,
         L: IoLoc<Self::Target, T>,
-        Self::Backend: IoCapable<L::IoType>,
+        Self::Backend: IoCapable<<T as AsRepr>::Repr>,
     {
-        let view = io_view_assert::<Self, L::IoType>(self, location.offset());
-        let io_value = value.into();
-        Self::Backend::io_write(view, io_value);
+        let view = io_view_assert::<Self, T>(self, location.offset());
+        view.write_val(value)
     }
 
     /// Generic infallible write of a fully-located register value.
@@ -1064,9 +1081,10 @@ fn write<T, L>(self, location: L, value: T)
     #[inline(always)]
     fn write_reg<T, L, V>(self, value: V)
     where
+        T: AsRepr,
         L: IoLoc<Self::Target, T>,
         V: LocatedRegister<Self::Target, Location = L, Value = T>,
-        Self::Backend: IoCapable<L::IoType>,
+        Self::Backend: IoCapable<<T as AsRepr>::Repr>,
     {
         let (location, value) = value.into_io_op();
 
@@ -1098,14 +1116,13 @@ fn write_reg<T, L, V>(self, value: V)
     #[inline(always)]
     fn update<T, L, F>(self, location: L, f: F)
     where
+        T: AsReprMut,
         L: IoLoc<Self::Target, T>,
-        Self::Backend: IoCapable<L::IoType>,
+        Self::Backend: IoCapable<<T as AsRepr>::Repr>,
         F: FnOnce(T) -> T,
     {
-        let view = io_view_assert::<Self, L::IoType>(self, location.offset());
-        let value: T = Self::Backend::io_read(view).into();
-        let io_value = f(value).into();
-        Self::Backend::io_write(view, io_value);
+        let view = io_view_assert::<Self, T>(self, location.offset());
+        view.write_val(f(view.read_val()));
     }
 }
 
diff --git a/rust/kernel/io/register.rs b/rust/kernel/io/register.rs
index 6a19552ffb95..45d2e73a92e7 100644
--- a/rust/kernel/io/register.rs
+++ b/rust/kernel/io/register.rs
@@ -117,9 +117,6 @@
 
 /// Trait implemented by all registers.
 pub trait Register: Sized {
-    /// Backing primitive type of the register.
-    type Storage: Into<Self> + From<Self>;
-
     /// Start offset of the register.
     ///
     /// The interpretation of this offset depends on the type of the register.
@@ -135,8 +132,6 @@ impl<const SIZE: usize, T> IoLoc<Region<SIZE>, T> for ()
 where
     T: FixedRegister,
 {
-    type IoType = T::Storage;
-
     #[inline(always)]
     fn offset(self) -> usize {
         T::OFFSET
@@ -149,8 +144,6 @@ impl<const SIZE: usize, T> IoLoc<Region<SIZE>, T> for T
 where
     T: FixedRegister,
 {
-    type IoType = T::Storage;
-
     #[inline(always)]
     fn offset(self) -> usize {
         T::OFFSET
@@ -174,8 +167,6 @@ impl<const SIZE: usize, T> IoLoc<Region<SIZE>, T> for FixedRegisterLoc<T>
 where
     T: FixedRegister,
 {
-    type IoType = T::Storage;
-
     #[inline(always)]
     fn offset(self) -> usize {
         T::OFFSET
@@ -246,8 +237,6 @@ impl<const SIZE: usize, T, B> IoLoc<Region<SIZE>, T> for RelativeRegisterLoc<T,
     T: RelativeRegister,
     B: RegisterBase<T::BaseFamily> + ?Sized,
 {
-    type IoType = T::Storage;
-
     #[inline(always)]
     fn offset(self) -> usize {
         RelativeRegisterLoc::offset(self)
@@ -289,8 +278,6 @@ impl<const SIZE: usize, T> IoLoc<Region<SIZE>, T> for RegisterArrayLoc<T>
 where
     T: RegisterArray,
 {
-    type IoType = T::Storage;
-
     #[inline(always)]
     fn offset(self) -> usize {
         T::OFFSET + self.0 * T::STRIDE
@@ -377,8 +364,6 @@ impl<const SIZE: usize, T, B> IoLoc<Region<SIZE>, T> for RelativeRegisterArrayLo
     T: RelativeRegisterArray,
     B: RegisterBase<T::BaseFamily> + ?Sized,
 {
-    type IoType = T::Storage;
-
     #[inline(always)]
     fn offset(self) -> usize {
         self.0.offset() + self.1 * T::STRIDE
diff --git a/rust/macros/io/register.rs b/rust/macros/io/register.rs
index 61d076ab570a..8af12caa964b 100644
--- a/rust/macros/io/register.rs
+++ b/rust/macros/io/register.rs
@@ -200,8 +200,6 @@ pub(crate) fn register(def: RegDef) -> Result<TokenStream> {
             );
 
             impl ::kernel::io::register::Register for #name {
-                type Storage = #storage;
-
                 const OFFSET: usize = #offset;
             }
         ));

-- 
2.54.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.