[PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits

Alexandre Courbot posted 7 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits
Posted by Alexandre Courbot 3 months, 2 weeks ago
The core library's `From` implementations do not cover conversions
that are not portable or future-proof. For instance, even though it is
safe today, `From<usize>` is not implemented for `u64` because of the
possibility to support larger-than-64bit architectures in the future.

However, the kernel supports a narrower set of architectures, and in the
case of Nova we only support 64-bit. This makes it helpful and desirable
to provide more infallible conversions, lest we need to rely on the `as`
keyword and carry the risk of silently losing data.

Thus, introduce a new module `num` that provides safe const functions
performing more conversions allowed by the build target, as well as
`FromAs` and `IntoAs` traits that are just extensions of `From` and
`Into` to conversions that are known to be lossless.

Suggested-by: Danilo Krummrich <dakr@kernel.org>
Link: https://lore.kernel.org/rust-for-linux/DDK4KADWJHMG.1FUPL3SDR26XF@kernel.org/
Acked-by: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/nova_core.rs |   1 +
 drivers/gpu/nova-core/num.rs       | 158 +++++++++++++++++++++++++++++++++++++
 2 files changed, 159 insertions(+)

diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index e130166c1086..9180ec9c27ef 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -13,6 +13,7 @@
 mod gfw;
 mod gpu;
 mod gsp;
+mod num;
 mod regs;
 mod vbios;
 
diff --git a/drivers/gpu/nova-core/num.rs b/drivers/gpu/nova-core/num.rs
new file mode 100644
index 000000000000..adb5a92f0d51
--- /dev/null
+++ b/drivers/gpu/nova-core/num.rs
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Numerical helpers functions and traits.
+//!
+//! This is essentially a staging module for code to mature until it can be moved to the `kernel`
+//! crate.
+
+use kernel::{build_error, macros::paste};
+
+macro_rules! impl_lossless_as {
+    ($from:ty as { $($into:ty),* }) => {
+        $(
+        paste! {
+            #[doc = ::core::concat!(
+                "Losslessly converts a [`",
+                ::core::stringify!($from),
+                "`] into a [`",
+                ::core::stringify!($into),
+                "`].")]
+            ///
+            /// This conversion is allowed as it is always lossless. Prefer this over the `as`
+            /// keyword to ensure no lossy conversions are performed.
+            ///
+            /// This is for use from a `const` context. For non `const` use, prefer the [`FromAs`]
+            /// and [`IntoAs`] traits.
+            ///
+            /// # Examples
+            ///
+            /// ```
+            /// use crate::num;
+            ///
+            #[doc = ::core::concat!(
+                "assert_eq!(num::",
+                ::core::stringify!($from),
+                "_as_",
+                ::core::stringify!($into),
+                "(1",
+                ::core::stringify!($from),
+                "), 1",
+                ::core::stringify!($into),
+                ");")]
+            /// ```
+            #[allow(unused)]
+            pub(crate) const fn [<$from _as_ $into>](value: $from) -> $into {
+                kernel::static_assert!(size_of::<$into>() >= size_of::<$from>());
+
+                value as $into
+            }
+        }
+        )*
+    };
+}
+
+impl_lossless_as!(u8 as { u16, u32, u64, usize });
+impl_lossless_as!(u16 as { u32, u64, usize });
+impl_lossless_as!(u32 as { u64, usize } );
+// `u64` and `usize` have the same size on 64-bit platforms.
+#[cfg(CONFIG_64BIT)]
+impl_lossless_as!(u64 as { usize } );
+
+// A `usize` fits into a `u64` on 32 and 64-bit platforms.
+#[cfg(any(CONFIG_32BIT, CONFIG_64BIT))]
+impl_lossless_as!(usize as { u64 });
+
+// A `usize` fits into a `u32` on 32-bit platforms.
+#[cfg(CONFIG_32BIT)]
+impl_lossless_as!(usize as { u32 });
+
+/// Extension trait providing guaranteed lossless conversion to `Self` from `T`.
+///
+/// The standard library's `From` implementations do not cover conversions that are not portable or
+/// future-proof. For instance, even though it is safe today, `From<usize>` is not implemented for
+/// [`u64`] because of the possibility to support larger-than-64bit architectures in the future.
+///
+/// The workaround is to either deal with the error handling of [`TryFrom`] for an operation that
+/// technically cannot fail, or to use the `as` keyword, which can silently strip data if the
+/// destination type is smaller than the source.
+///
+/// Both options are hardly acceptable for the kernel. It is also a much more architecture
+/// dependent environment, supporting only 32 and 64 bit architectures, with some modules
+/// explicitly depending on a specific bus width that could greatly benefit from infallible
+/// conversion operations.
+///
+/// Thus this extension trait that provides, for the architecture the kernel is built for, safe
+/// conversion between types for which such conversion is lossless.
+///
+/// In other words, this trait is implemented if, for the current build target and with `t: T`, the
+/// `t as Self` operation is completely lossless.
+///
+/// Prefer this over the `as` keyword to ensure no lossy conversions are performed.
+///
+/// If you need to perform a conversion in `const` context, use [`u64_as_usize`],
+/// [`u32_as_usize`], [`usize_as_u64`], etc.
+///
+/// # Examples
+///
+/// ```
+/// use crate::num::FromAs;
+///
+/// assert_eq!(usize::from_as(0xf00u32), 0xf00u32 as usize);
+/// ```
+pub(crate) trait FromAs<T> {
+    /// Create a `Self` from `value`. This operation is guaranteed to be lossless.
+    fn from_as(value: T) -> Self;
+}
+
+impl FromAs<usize> for u64 {
+    fn from_as(value: usize) -> Self {
+        usize_as_u64(value)
+    }
+}
+
+#[cfg(CONFIG_32BIT)]
+impl FromAs<usize> for u32 {
+    fn from_as(value: usize) -> Self {
+        usize_as_u32(value)
+    }
+}
+
+impl FromAs<u32> for usize {
+    fn from_as(value: u32) -> Self {
+        u32_as_usize(value)
+    }
+}
+
+#[cfg(CONFIG_64BIT)]
+impl FromAs<u64> for usize {
+    fn from_as(value: u64) -> Self {
+        u64_as_usize(value)
+    }
+}
+
+/// Counterpart to the [`FromAs`] trait, i.e. this trait is to [`FromAs`] what [`Into`] is to
+/// [`From`].
+///
+/// See the documentation of [`FromAs`] for the motivation.
+///
+/// # Examples
+///
+/// ```
+/// use crate::num::IntoAs;
+///
+/// assert_eq!(0xf00u32.into_as(), 0xf00u32 as usize);
+/// ```
+pub(crate) trait IntoAs<T> {
+    /// Convert `self` into a `T`. This operation is guaranteed to be lossless.
+    fn into_as(self) -> T;
+}
+
+/// Reverse operation for types implementing [`FromAs`].
+impl<S, T> IntoAs<T> for S
+where
+    T: FromAs<S>,
+{
+    fn into_as(self) -> T {
+        T::from_as(self)
+    }
+}

-- 
2.51.0
Re: [PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits
Posted by Joel Fernandes 3 months, 1 week ago
Hello Alex,

On 10/27/2025 8:54 AM, Alexandre Courbot wrote:
> The core library's `From` implementations do not cover conversions
> that are not portable or future-proof. For instance, even though it is
> safe today, `From<usize>` is not implemented for `u64` because of the
> possibility to support larger-than-64bit architectures in the future.
> 
> However, the kernel supports a narrower set of architectures, and in the
> case of Nova we only support 64-bit. This makes it helpful and desirable
> to provide more infallible conversions, lest we need to rely on the `as`
> keyword and carry the risk of silently losing data.
> 
> Thus, introduce a new module `num` that provides safe const functions
> performing more conversions allowed by the build target, as well as
> `FromAs` and `IntoAs` traits that are just extensions of `From` and
> `Into` to conversions that are known to be lossless.

Why not just implement `From` and `Into` for the types missing it then, with
adequate comments about why such conversions are Ok for the kernel, instead of
introducing a new trait? This is exactly what `From`/`Into` is for right?
> Suggested-by: Danilo Krummrich <dakr@kernel.org>
> Link: https://lore.kernel.org/rust-for-linux/DDK4KADWJHMG.1FUPL3SDR26XF@kernel.org/
> Acked-by: Danilo Krummrich <dakr@kernel.org>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
>  drivers/gpu/nova-core/nova_core.rs |   1 +
>  drivers/gpu/nova-core/num.rs       | 158 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 159 insertions(+)
> 
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> index e130166c1086..9180ec9c27ef 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -13,6 +13,7 @@
>  mod gfw;
>  mod gpu;
>  mod gsp;
> +mod num;
>  mod regs;
>  mod vbios;
>  
> diff --git a/drivers/gpu/nova-core/num.rs b/drivers/gpu/nova-core/num.rs
> new file mode 100644
> index 000000000000..adb5a92f0d51
> --- /dev/null
> +++ b/drivers/gpu/nova-core/num.rs
> @@ -0,0 +1,158 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Numerical helpers functions and traits.
> +//!
> +//! This is essentially a staging module for code to mature until it can be moved to the `kernel`
> +//! crate.
> +
> +use kernel::{build_error, macros::paste};
> +
> +macro_rules! impl_lossless_as {
> +    ($from:ty as { $($into:ty),* }) => {
> +        $(
> +        paste! {
> +            #[doc = ::core::concat!(
> +                "Losslessly converts a [`",
> +                ::core::stringify!($from),
> +                "`] into a [`",
> +                ::core::stringify!($into),
> +                "`].")]
> +            ///
> +            /// This conversion is allowed as it is always lossless. Prefer this over the `as`
> +            /// keyword to ensure no lossy conversions are performed.
> +            ///
> +            /// This is for use from a `const` context. For non `const` use, prefer the [`FromAs`]
> +            /// and [`IntoAs`] traits.
> +            ///
> +            /// # Examples
> +            ///
> +            /// ```
> +            /// use crate::num;
> +            ///
> +            #[doc = ::core::concat!(
> +                "assert_eq!(num::",
> +                ::core::stringify!($from),
> +                "_as_",
> +                ::core::stringify!($into),
> +                "(1",
> +                ::core::stringify!($from),
> +                "), 1",
> +                ::core::stringify!($into),
> +                ");")]
> +            /// ```
> +            #[allow(unused)]
> +            pub(crate) const fn [<$from _as_ $into>](value: $from) -> $into {
> +                kernel::static_assert!(size_of::<$into>() >= size_of::<$from>());
> +
> +                value as $into
> +            }
> +        }
> +        )*
> +    };
> +}
> +
> +impl_lossless_as!(u8 as { u16, u32, u64, usize });
> +impl_lossless_as!(u16 as { u32, u64, usize });
> +impl_lossless_as!(u32 as { u64, usize } );

I prefer consistency with the notation in num.rs in the rust standard library:
impl_from!(u16 => usize)
But I am Ok with your notation as well, which is concise.

> +// `u64` and `usize` have the same size on 64-bit platforms.
> +#[cfg(CONFIG_64BIT)]
> +impl_lossless_as!(u64 as { usize } );
> +
> +// A `usize` fits into a `u64` on 32 and 64-bit platforms.
> +#[cfg(any(CONFIG_32BIT, CONFIG_64BIT))]
> +impl_lossless_as!(usize as { u64 });
> +
> +// A `usize` fits into a `u32` on 32-bit platforms.
> +#[cfg(CONFIG_32BIT)]
> +impl_lossless_as!(usize as { u32 });
> +
> +/// Extension trait providing guaranteed lossless conversion to `Self` from `T`.
> +///
> +/// The standard library's `From` implementations do not cover conversions that are not portable or
> +/// future-proof. For instance, even though it is safe today, `From<usize>` is not implemented for
> +/// [`u64`] because of the possibility to support larger-than-64bit architectures in the future.
> +///
> +/// The workaround is to either deal with the error handling of [`TryFrom`] for an operation that
> +/// technically cannot fail, or to use the `as` keyword, which can silently strip data if the
> +/// destination type is smaller than the source.
> +///
> +/// Both options are hardly acceptable for the kernel. It is also a much more architecture
> +/// dependent environment, supporting only 32 and 64 bit architectures, with some modules
> +/// explicitly depending on a specific bus width that could greatly benefit from infallible
> +/// conversion operations.
> +///
> +/// Thus this extension trait that provides, for the architecture the kernel is built for, safe
> +/// conversion between types for which such conversion is lossless.
> +///
> +/// In other words, this trait is implemented if, for the current build target and with `t: T`, the
> +/// `t as Self` operation is completely lossless.
> +///
> +/// Prefer this over the `as` keyword to ensure no lossy conversions are performed.
> +///
> +/// If you need to perform a conversion in `const` context, use [`u64_as_usize`],
> +/// [`u32_as_usize`], [`usize_as_u64`], etc.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use crate::num::FromAs;
> +///
> +/// assert_eq!(usize::from_as(0xf00u32), 0xf00u32 as usize);

This `from_as()` syntax will be very confusing for users IMO, honestly we should
just keep it as `from()`, otherwise there is confusion and ambiguity around
whether someone should use `::from()` or `::from_as()`. Please let us just keep
all infallible conversions to use `from()`/`into()` and all infallible ones to
use `try_from()`/`try_into()`. No need to additional `_as()` prefix, as it
creates confusion. In the end of the day, the fact the conversion is lossless is
not relevant, all conversions that don't use the `as` keyword should be expected
to be lossless right?

thanks,

 - Joel
Re: [PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits
Posted by Alexandre Courbot 3 months, 1 week ago
On Tue Oct 28, 2025 at 4:09 AM JST, Joel Fernandes wrote:
> Hello Alex,
>
> On 10/27/2025 8:54 AM, Alexandre Courbot wrote:
>> The core library's `From` implementations do not cover conversions
>> that are not portable or future-proof. For instance, even though it is
>> safe today, `From<usize>` is not implemented for `u64` because of the
>> possibility to support larger-than-64bit architectures in the future.
>> 
>> However, the kernel supports a narrower set of architectures, and in the
>> case of Nova we only support 64-bit. This makes it helpful and desirable
>> to provide more infallible conversions, lest we need to rely on the `as`
>> keyword and carry the risk of silently losing data.
>> 
>> Thus, introduce a new module `num` that provides safe const functions
>> performing more conversions allowed by the build target, as well as
>> `FromAs` and `IntoAs` traits that are just extensions of `From` and
>> `Into` to conversions that are known to be lossless.
>
> Why not just implement `From` and `Into` for the types missing it then, with
> adequate comments about why such conversions are Ok for the kernel, instead of
> introducing a new trait? This is exactly what `From`/`Into` is for right?

I wish we could. :) The link Danilo sent should clarify why this is not
possible.

<snip>
>> +impl_lossless_as!(u8 as { u16, u32, u64, usize });
>> +impl_lossless_as!(u16 as { u32, u64, usize });
>> +impl_lossless_as!(u32 as { u64, usize } );
>
> I prefer consistency with the notation in num.rs in the rust standard library:
> impl_from!(u16 => usize)
> But I am Ok with your notation as well, which is concise.

You're right, something like `impl_as` is both shorter and more
consistent.

>
>> +// `u64` and `usize` have the same size on 64-bit platforms.
>> +#[cfg(CONFIG_64BIT)]
>> +impl_lossless_as!(u64 as { usize } );
>> +
>> +// A `usize` fits into a `u64` on 32 and 64-bit platforms.
>> +#[cfg(any(CONFIG_32BIT, CONFIG_64BIT))]
>> +impl_lossless_as!(usize as { u64 });
>> +
>> +// A `usize` fits into a `u32` on 32-bit platforms.
>> +#[cfg(CONFIG_32BIT)]
>> +impl_lossless_as!(usize as { u32 });
>> +
>> +/// Extension trait providing guaranteed lossless conversion to `Self` from `T`.
>> +///
>> +/// The standard library's `From` implementations do not cover conversions that are not portable or
>> +/// future-proof. For instance, even though it is safe today, `From<usize>` is not implemented for
>> +/// [`u64`] because of the possibility to support larger-than-64bit architectures in the future.
>> +///
>> +/// The workaround is to either deal with the error handling of [`TryFrom`] for an operation that
>> +/// technically cannot fail, or to use the `as` keyword, which can silently strip data if the
>> +/// destination type is smaller than the source.
>> +///
>> +/// Both options are hardly acceptable for the kernel. It is also a much more architecture
>> +/// dependent environment, supporting only 32 and 64 bit architectures, with some modules
>> +/// explicitly depending on a specific bus width that could greatly benefit from infallible
>> +/// conversion operations.
>> +///
>> +/// Thus this extension trait that provides, for the architecture the kernel is built for, safe
>> +/// conversion between types for which such conversion is lossless.
>> +///
>> +/// In other words, this trait is implemented if, for the current build target and with `t: T`, the
>> +/// `t as Self` operation is completely lossless.
>> +///
>> +/// Prefer this over the `as` keyword to ensure no lossy conversions are performed.
>> +///
>> +/// If you need to perform a conversion in `const` context, use [`u64_as_usize`],
>> +/// [`u32_as_usize`], [`usize_as_u64`], etc.
>> +///
>> +/// # Examples
>> +///
>> +/// ```
>> +/// use crate::num::FromAs;
>> +///
>> +/// assert_eq!(usize::from_as(0xf00u32), 0xf00u32 as usize);
>
> This `from_as()` syntax will be very confusing for users IMO, honestly we should
> just keep it as `from()`, otherwise there is confusion and ambiguity around
> whether someone should use `::from()` or `::from_as()`. Please let us just keep
> all infallible conversions to use `from()`/`into()` and all infallible ones to
> use `try_from()`/`try_into()`. No need to additional `_as()` prefix, as it
> creates confusion. In the end of the day, the fact the conversion is lossless is
> not relevant, all conversions that don't use the `as` keyword should be expected
> to be lossless right?

I wanted to use `from`/`into` initially but this unfortunately clashes
with the `From` and `Into` traits and forces callers to disambiguate
which trait we want to use with a turbofish, which goes against the
intent of keeping the syntax short. I'm happy to consider better names
though.
Re: [PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits
Posted by Danilo Krummrich 3 months, 1 week ago
On 10/27/25 8:09 PM, Joel Fernandes wrote:
> Why not just implement `From` and `Into` for the types missing it then, with
> adequate comments about why such conversions are Ok for the kernel, instead of
> introducing a new trait? This is exactly what `From`/`Into` is for right?

https://doc.rust-lang.org/reference/items/implementations.html#r-items.impl.trait.orphan-rule.intro
Re: [PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits
Posted by Danilo Krummrich 3 months, 1 week ago
On 10/27/25 8:23 PM, Danilo Krummrich wrote:
> On 10/27/25 8:09 PM, Joel Fernandes wrote:
>> Why not just implement `From` and `Into` for the types missing it then, with
>> adequate comments about why such conversions are Ok for the kernel, instead of
>> introducing a new trait? This is exactly what `From`/`Into` is for right?
> 
> https://doc.rust-lang.org/reference/items/implementations.html#r-items.impl.trait.orphan-rule.intro

(Sorry, I didn't mean to send the link without additional comment.)

We can't do this due to the orphan rule, but even if we could I think a separate
trait indicating the reason for the conversions to be valid is a good thing.

- Danilo
Re: [PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits
Posted by Joel Fernandes 3 months, 1 week ago
On 10/27/2025 3:28 PM, Danilo Krummrich wrote:
> On 10/27/25 8:23 PM, Danilo Krummrich wrote:
>> On 10/27/25 8:09 PM, Joel Fernandes wrote:
>>> Why not just implement `From` and `Into` for the types missing it then, with
>>> adequate comments about why such conversions are Ok for the kernel, instead of
>>> introducing a new trait? This is exactly what `From`/`Into` is for right?
>>
>> https://doc.rust-lang.org/reference/items/implementations.html#r-items.impl.trait.orphan-rule.intro
> 
> (Sorry, I didn't mean to send the link without additional comment.)
> 
> We can't do this due to the orphan rule, but even if we could I think a separate
> trait indicating the reason for the conversions to be valid is a good thing.
> 

Thanks for clarifying, yeah its not ideal then as the user of the conversion
needs to remember when to use from vs from_as. I don't think its terrible, just
a bit confusing.

Thanks.
Re: [PATCH v2 5/7] gpu: nova-core: add extra integer conversion functions and traits
Posted by Joel Fernandes 3 months, 1 week ago
On 10/27/2025 3:09 PM, Joel Fernandes wrote:
[...]
>> +/// Extension trait providing guaranteed lossless conversion to `Self` from `T`.
>> +///
>> +/// The standard library's `From` implementations do not cover conversions that are not portable or
>> +/// future-proof. For instance, even though it is safe today, `From<usize>` is not implemented for
>> +/// [`u64`] because of the possibility to support larger-than-64bit architectures in the future.
>> +///
>> +/// The workaround is to either deal with the error handling of [`TryFrom`] for an operation that
>> +/// technically cannot fail, or to use the `as` keyword, which can silently strip data if the
>> +/// destination type is smaller than the source.
>> +///
>> +/// Both options are hardly acceptable for the kernel. It is also a much more architecture
>> +/// dependent environment, supporting only 32 and 64 bit architectures, with some modules
>> +/// explicitly depending on a specific bus width that could greatly benefit from infallible
>> +/// conversion operations.
>> +///
>> +/// Thus this extension trait that provides, for the architecture the kernel is built for, safe
>> +/// conversion between types for which such conversion is lossless.
>> +///
>> +/// In other words, this trait is implemented if, for the current build target and with `t: T`, the
>> +/// `t as Self` operation is completely lossless.
>> +///
>> +/// Prefer this over the `as` keyword to ensure no lossy conversions are performed.
>> +///
>> +/// If you need to perform a conversion in `const` context, use [`u64_as_usize`],
>> +/// [`u32_as_usize`], [`usize_as_u64`], etc.
>> +///
>> +/// # Examples
>> +///
>> +/// ```
>> +/// use crate::num::FromAs;
>> +///
>> +/// assert_eq!(usize::from_as(0xf00u32), 0xf00u32 as usize);
> 
> This `from_as()` syntax will be very confusing for users IMO, honestly we should
> just keep it as `from()`, otherwise there is confusion and ambiguity around
> whether someone should use `::from()` or `::from_as()`. Please let us just keep
> all infallible conversions to use `from()`/`into()` and all infallible ones to

s/infallible ones/fallible ones/.

Sorry, thanks,

 - Joel