`ResourceSize` was a bare type alias for `resource_size_t`, so it inherited every integer operation and `as` cast. That silently permits operations that are meaningless for the size of a hardware resource: mixing it with unrelated integers and truncating casts.
Wrap it in a `#[repr(transparent)]` newtype so each conversion at a boundary becomes explicit and reviewable. The representation is unchanged, so this is ABI-identical; only the spelling at the FFI boundary changes. Provide the conversion surface `from_raw`/`into_raw`, `From` in both directions, and a fallible `TryFrom<ResourceSize> for usize` for checked narrowing. Update the two producers, `Resource::size` and `SGEntry::dma_len`, and the `request_region` consumer. In nova-core, add an `impl FromSafeCast<ResourceSize> for usize` so its page-count conversion keeps the infallible `usize::from_safe_cast(sg_entry.dma_len())` unchanged. Suggested-by: Miguel Ojeda <[email protected]> Link: https://github.com/Rust-for-Linux/linux/issues/1203 Signed-off-by: Lorenzo Delgado <[email protected]> --- v2: keep the nova-core page-count conversion infallible, per review (Danilo Krummrich). v1 changed it to a fallible usize::try_from(sg_entry.dma_len())?; instead of touching the call site, add an impl FromSafeCast<ResourceSize> for usize in nova-core's num.rs, so drivers/gpu/nova-core/firmware/gsp.rs keeps usize::from_safe_cast(sg_entry.dma_len()) unchanged. FromSafeCast is to move to the kernel crate subsequently. io.rs is unchanged from v1. v1: https://lore.kernel.org/r/[email protected] drivers/gpu/nova-core/num.rs | 10 +++++ rust/kernel/io.rs | 73 ++++++++++++++++++++++++++++++++++-- rust/kernel/io/resource.rs | 6 +-- rust/kernel/scatterlist.rs | 2 +- 4 files changed, 83 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/nova-core/num.rs b/drivers/gpu/nova-core/num.rs index 6eb174d136ab..7c0e878bbfb2 100644 --- a/drivers/gpu/nova-core/num.rs +++ b/drivers/gpu/nova-core/num.rs @@ -6,6 +6,7 @@ //! crate. use kernel::{ + io::ResourceSize, macros::paste, prelude::*, // }; @@ -137,6 +138,15 @@ fn from_safe_cast(value: u64) -> Self { } } +// A `resource_size_t` fits into a `usize` on 64-bit platforms, where a `usize` is a `u64` and a +// `resource_size_t` is at most a `u64`. Delegates to the primitive impl for the underlying type. +#[cfg(CONFIG_64BIT)] +impl FromSafeCast<ResourceSize> for usize { + fn from_safe_cast(value: ResourceSize) -> Self { + Self::from_safe_cast(value.into_raw()) + } +} + /// Counterpart to the [`FromSafeCast`] trait, i.e. this trait is to [`FromSafeCast`] what [`Into`] /// is to [`From`]. /// diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs index fcc7678fd9e3..ebf00b4536d6 100644 --- a/rust/kernel/io.rs +++ b/rust/kernel/io.rs @@ -6,9 +6,12 @@ use crate::{ bindings, + fmt, prelude::*, // }; +use core::num::TryFromIntError; + pub mod mem; pub mod poll; pub mod register; @@ -25,11 +28,73 @@ /// `CONFIG_PHYS_ADDR_T_64BIT`, and it can be a u64 even on 32-bit architectures. pub type PhysAddr = bindings::phys_addr_t; -/// Resource Size type. +/// Resource size type. /// -/// This is a type alias to either `u32` or `u64` depending on the config option -/// `CONFIG_PHYS_ADDR_T_64BIT`, and it can be a u64 even on 32-bit architectures. -pub type ResourceSize = bindings::resource_size_t; +/// This wraps either `u32` or `u64` depending on the config option +/// `CONFIG_PHYS_ADDR_T_64BIT`, and it can be a `u64` even on 32-bit architectures. +/// +/// # Examples +/// +/// ``` +/// use kernel::io::ResourceSize; +/// +/// let size = ResourceSize::from_raw(0x1000); +/// assert_eq!(size.into_raw(), 0x1000); +/// +/// // Round-trips through the raw C type. +/// let raw: kernel::bindings::resource_size_t = size.into(); +/// assert_eq!(ResourceSize::from(raw), size); +/// +/// // Fallible conversion to `usize` (can truncate on 32-bit). +/// assert_eq!(usize::try_from(size)?, 0x1000); +/// # Ok::<(), core::num::TryFromIntError>(()) +/// ``` +#[repr(transparent)] +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub struct ResourceSize(bindings::resource_size_t); + +impl ResourceSize { + /// Creates a resource size from the raw C type. + #[inline] + pub const fn from_raw(value: bindings::resource_size_t) -> Self { + Self(value) + } + + /// Turns this resource size into the raw C type. + #[inline] + pub const fn into_raw(self) -> bindings::resource_size_t { + self.0 + } +} + +impl fmt::Debug for ResourceSize { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{:#x}", self.0) + } +} + +impl From<bindings::resource_size_t> for ResourceSize { + #[inline] + fn from(value: bindings::resource_size_t) -> Self { + Self::from_raw(value) + } +} + +impl From<ResourceSize> for bindings::resource_size_t { + #[inline] + fn from(value: ResourceSize) -> Self { + value.into_raw() + } +} + +impl TryFrom<ResourceSize> for usize { + type Error = TryFromIntError; + + #[inline] + fn try_from(value: ResourceSize) -> Result<Self, Self::Error> { + Self::try_from(value.into_raw()) + } +} /// Raw representation of an MMIO region. /// diff --git a/rust/kernel/io/resource.rs b/rust/kernel/io/resource.rs index 17b0c174cfc5..a33a416b289a 100644 --- a/rust/kernel/io/resource.rs +++ b/rust/kernel/io/resource.rs @@ -58,7 +58,7 @@ fn drop(&mut self) { }; // SAFETY: Safe as per the invariant of `Region`. - unsafe { release_fn(start, size) }; + unsafe { release_fn(start, size.into_raw()) }; } } @@ -114,7 +114,7 @@ pub fn request_region( bindings::__request_region( self.0.get(), start, - size, + size.into_raw(), name.as_char_ptr(), flags.0 as c_int, ) @@ -130,7 +130,7 @@ pub fn request_region( pub fn size(&self) -> ResourceSize { let inner = self.0.get(); // SAFETY: Safe as per the invariants of `Resource`. - unsafe { bindings::resource_size(inner) } + ResourceSize::from_raw(unsafe { bindings::resource_size(inner) }) } /// Returns the start address of the resource. diff --git a/rust/kernel/scatterlist.rs b/rust/kernel/scatterlist.rs index b83c468b5c63..5d67242befd8 100644 --- a/rust/kernel/scatterlist.rs +++ b/rust/kernel/scatterlist.rs @@ -93,7 +93,7 @@ pub fn dma_address(&self) -> dma::DmaAddress { pub fn dma_len(&self) -> ResourceSize { #[allow(clippy::useless_conversion)] // SAFETY: `self.as_raw()` is a valid pointer to a `struct scatterlist`. - unsafe { bindings::sg_dma_len(self.as_raw()) }.into() + ResourceSize::from_raw(unsafe { bindings::sg_dma_len(self.as_raw()) }.into()) } } base-commit: 7059bdf4f04a3e14f4fafb3ac35fdca913e3e21a -- 2.54.0
