[PATCH] gpu: nova-core: use CStr::from_bytes_until_nul() and remove util.rs

John Hubbard posted 1 patch 1 month ago
There is a newer version of this series
drivers/gpu/nova-core/gsp/commands.rs |  5 +++--
drivers/gpu/nova-core/nova_core.rs    |  1 -
drivers/gpu/nova-core/util.rs         | 16 ----------------
3 files changed, 3 insertions(+), 19 deletions(-)
delete mode 100644 drivers/gpu/nova-core/util.rs
[PATCH] gpu: nova-core: use CStr::from_bytes_until_nul() and remove util.rs
Posted by John Hubbard 1 month ago
The util.rs module contained a single helper function,
str_from_null_terminated(), which duplicated functionality that is now
available in core::ffi::CStr.

Specifically, CStr::from_bytes_until_nul() is available in the kernel's
minimum supported Rust version (1.78.0), so it time to stop using this
custom workaround.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 drivers/gpu/nova-core/gsp/commands.rs |  5 +++--
 drivers/gpu/nova-core/nova_core.rs    |  1 -
 drivers/gpu/nova-core/util.rs         | 16 ----------------
 3 files changed, 3 insertions(+), 19 deletions(-)
 delete mode 100644 drivers/gpu/nova-core/util.rs

diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index 0425c65b5d6f..a11fe6018091 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -30,7 +30,6 @@
         },
     },
     sbuffer::SBufferIter,
-    util,
 };
 
 /// The `GspSetSystemInfo` command.
@@ -209,7 +208,9 @@ impl GetGspStaticInfoReply {
     /// Returns the name of the GPU as a string, or `None` if the string given by the GSP was
     /// invalid.
     pub(crate) fn gpu_name(&self) -> Option<&str> {
-        util::str_from_null_terminated(&self.gpu_name)
+        CStr::from_bytes_until_nul(&self.gpu_name)
+            .ok()
+            .and_then(|cstr| cstr.to_str().ok())
     }
 }
 
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index b98a1c03f13d..c1121e7c64c5 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -16,7 +16,6 @@
 mod num;
 mod regs;
 mod sbuffer;
-mod util;
 mod vbios;
 
 pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
diff --git a/drivers/gpu/nova-core/util.rs b/drivers/gpu/nova-core/util.rs
deleted file mode 100644
index 4b503249a3ef..000000000000
--- a/drivers/gpu/nova-core/util.rs
+++ /dev/null
@@ -1,16 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-
-/// Converts a null-terminated byte slice to a string, or `None` if the array does not
-/// contains any null byte or contains invalid characters.
-///
-/// Contrary to [`kernel::str::CStr::from_bytes_with_nul`], the null byte can be anywhere in the
-/// slice, and not only in the last position.
-pub(crate) fn str_from_null_terminated(bytes: &[u8]) -> Option<&str> {
-    use kernel::str::CStr;
-
-    bytes
-        .iter()
-        .position(|&b| b == 0)
-        .and_then(|null_pos| CStr::from_bytes_with_nul(&bytes[..=null_pos]).ok())
-        .and_then(|cstr| cstr.to_str().ok())
-}

base-commit: 7acc70476f14661149774ab88d3fe23d83ba4249
-- 
2.52.0
Re: [PATCH] gpu: nova-core: use CStr::from_bytes_until_nul() and remove util.rs
Posted by Danilo Krummrich 1 month ago
On Sat Jan 3, 2026 at 2:34 AM CET, John Hubbard wrote:
> @@ -209,7 +208,9 @@ impl GetGspStaticInfoReply {
>      /// Returns the name of the GPU as a string, or `None` if the string given by the GSP was
>      /// invalid.
>      pub(crate) fn gpu_name(&self) -> Option<&str> {
> -        util::str_from_null_terminated(&self.gpu_name)
> +        CStr::from_bytes_until_nul(&self.gpu_name)
> +            .ok()
> +            .and_then(|cstr| cstr.to_str().ok())
>      }
>  }

Did you see my reply in [1]? The question is orthogonal to this change, but
perhaps it can be addressed with a subsequent patch?

[1] https://lore.kernel.org/lkml/DFEVITW4O9DW.P4ITE1PWIDY6@kernel.org/
Re: [PATCH] gpu: nova-core: use CStr::from_bytes_until_nul() and remove util.rs
Posted by John Hubbard 1 month ago
On 1/6/26 4:02 AM, Danilo Krummrich wrote:
> On Sat Jan 3, 2026 at 2:34 AM CET, John Hubbard wrote:
>> @@ -209,7 +208,9 @@ impl GetGspStaticInfoReply {
>>      /// Returns the name of the GPU as a string, or `None` if the string given by the GSP was
>>      /// invalid.
>>      pub(crate) fn gpu_name(&self) -> Option<&str> {
>> -        util::str_from_null_terminated(&self.gpu_name)
>> +        CStr::from_bytes_until_nul(&self.gpu_name)
>> +            .ok()
>> +            .and_then(|cstr| cstr.to_str().ok())
>>      }
>>  }
> 
> Did you see my reply in [1]? The question is orthogonal to this change, but

I did, but momentarily forgot it during a miserable day in which
Thunderbird and Wayland started violently disagreeing, and every
email turned into a troubleshooting effort. :) Glad to be past that.

> perhaps it can be addressed with a subsequent patch?
> 
> [1] https://lore.kernel.org/lkml/DFEVITW4O9DW.P4ITE1PWIDY6@kernel.org/

Yes, so that would look approximately like this, I can send this as
another patch if it looks reasonable:

diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index a53d80620468..71fca7350b94 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -238,11 +238,11 @@ pub(crate) fn boot(
 
         // Obtain and display basic GPU information.
         let info = commands::get_gsp_info(&mut self.cmdq, bar)?;
-        dev_info!(
-            pdev.as_ref(),
-            "GPU name: {}\n",
-            info.gpu_name().unwrap_or("invalid GPU name")
-        );
+        let gpu_name = info
+            .gpu_name()
+            .inspect_err(|e| dev_warn!(pdev.as_ref(), "GPU name: {}\n", e))
+            .unwrap_or("<unavailable>");
+        dev_info!(pdev.as_ref(), "GPU name: {}\n", gpu_name);
 
         Ok(())
     }
diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index a11fe6018091..426441978b4b 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -2,7 +2,9 @@
 
 use core::{
     array,
-    convert::Infallible, //
+    convert::Infallible,
+    ffi::FromBytesUntilNulError,
+    str::Utf8Error, //
 };
 
 use kernel::{
@@ -204,13 +206,35 @@ fn read(
     }
 }
 
+/// Error type for [`GetGspStaticInfoReply::gpu_name`].
+#[derive(Debug)]
+pub(crate) enum GpuNameError {
+    /// The GPU name string does not contain a null terminator.
+    NoNullTerminator(FromBytesUntilNulError),
+
+    /// The GPU name string contains invalid UTF-8.
+    InvalidUtf8(Utf8Error),
+}
+
+impl kernel::fmt::Display for GpuNameError {
+    fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
+        match self {
+            Self::NoNullTerminator(_) => write!(f, "no null terminator"),
+            Self::InvalidUtf8(e) => write!(f, "invalid UTF-8 at byte {}", e.valid_up_to()),
+        }
+    }
+}
+
 impl GetGspStaticInfoReply {
-    /// Returns the name of the GPU as a string, or `None` if the string given by the GSP was
-    /// invalid.
-    pub(crate) fn gpu_name(&self) -> Option<&str> {
+    /// Returns the name of the GPU as a string.
+    ///
+    /// Returns an error if the string given by the GSP does not contain a null terminator or
+    /// contains invalid UTF-8.
+    pub(crate) fn gpu_name(&self) -> core::result::Result<&str, GpuNameError> {
         CStr::from_bytes_until_nul(&self.gpu_name)
-            .ok()
-            .and_then(|cstr| cstr.to_str().ok())
+            .map_err(GpuNameError::NoNullTerminator)?
+            .to_str()
+            .map_err(GpuNameError::InvalidUtf8)
     }
 }



thanks,
-- 
John Hubbard
Re: [PATCH] gpu: nova-core: use CStr::from_bytes_until_nul() and remove util.rs
Posted by Danilo Krummrich 1 month ago
On 1/6/26 11:09 PM, John Hubbard wrote:
> Yes, so that would look approximately like this, I can send this as
> another patch if it looks reasonable:

Thanks, looks good! Two comments below.
> diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
> index a53d80620468..71fca7350b94 100644
> --- a/drivers/gpu/nova-core/gsp/boot.rs
> +++ b/drivers/gpu/nova-core/gsp/boot.rs
> @@ -238,11 +238,11 @@ pub(crate) fn boot(
>  
>          // Obtain and display basic GPU information.
>          let info = commands::get_gsp_info(&mut self.cmdq, bar)?;
> -        dev_info!(
> -            pdev.as_ref(),
> -            "GPU name: {}\n",
> -            info.gpu_name().unwrap_or("invalid GPU name")
> -        );
> +        let gpu_name = info
> +            .gpu_name()
> +            .inspect_err(|e| dev_warn!(pdev.as_ref(), "GPU name: {}\n", e))
> +            .unwrap_or("<unavailable>");
> +        dev_info!(pdev.as_ref(), "GPU name: {}\n", gpu_name);

I'd probably only print one or the other. Also, I think this should be
dev_dbg!() instead of dev_info!().

> +/// Error type for [`GetGspStaticInfoReply::gpu_name`].
> +#[derive(Debug)]
> +pub(crate) enum GpuNameError {
> +    /// The GPU name string does not contain a null terminator.
> +    NoNullTerminator(FromBytesUntilNulError),
> +
> +    /// The GPU name string contains invalid UTF-8.
> +    InvalidUtf8(Utf8Error),
> +}
> +
> +impl kernel::fmt::Display for GpuNameError {
> +    fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
> +        match self {
> +            Self::NoNullTerminator(_) => write!(f, "no null terminator"),
> +            Self::InvalidUtf8(e) => write!(f, "invalid UTF-8 at byte {}", e.valid_up_to()),
> +        }
> +    }
> +}

Do we need this Display impl, or is the derive(Debug) you have already good
enough for the warning print?
Re: [PATCH] gpu: nova-core: use CStr::from_bytes_until_nul() and remove util.rs
Posted by John Hubbard 1 month ago
On 1/6/26 2:44 PM, Danilo Krummrich wrote:
> On 1/6/26 11:09 PM, John Hubbard wrote:
...
>> +        let gpu_name = info
>> +            .gpu_name()
>> +            .inspect_err(|e| dev_warn!(pdev.as_ref(), "GPU name: {}\n", e))
>> +            .unwrap_or("<unavailable>");
>> +        dev_info!(pdev.as_ref(), "GPU name: {}\n", gpu_name);
> 
> I'd probably only print one or the other. Also, I think this should be

Done.

> dev_dbg!() instead of dev_info!().

We have been *very* sparing with the dev_info(), and at this point,
there are precisely two places where Nova logs at info level: at first
probe, and after finding the true GPU marketing name (buried in the
firmware).

I think we've found a nice balance now. The output looks like this:

$  dmesg -t --level=info|grep NovaCore
NovaCore 0000:e1:00.0: NVIDIA (Chipset: GA104, Architecture: Ampere, Revision: a.1)
NovaCore 0000:e1:00.0: GPU name: NVIDIA RTX A4000
[drm] Initialized nova 0.0.0 for NovaCore.nova-drm.0 on minor 0

So I'd love to leave the GPU name at info level, if you agree that
this is about right.

> 
>> +/// Error type for [`GetGspStaticInfoReply::gpu_name`].
>> +#[derive(Debug)]
>> +pub(crate) enum GpuNameError {
>> +    /// The GPU name string does not contain a null terminator.
>> +    NoNullTerminator(FromBytesUntilNulError),
>> +
>> +    /// The GPU name string contains invalid UTF-8.
>> +    InvalidUtf8(Utf8Error),
>> +}
>> +
>> +impl kernel::fmt::Display for GpuNameError {
>> +    fn fmt(&self, f: &mut kernel::fmt::Formatter<'_>) -> kernel::fmt::Result {
>> +        match self {
>> +            Self::NoNullTerminator(_) => write!(f, "no null terminator"),
>> +            Self::InvalidUtf8(e) => write!(f, "invalid UTF-8 at byte {}", e.valid_up_to()),
>> +        }
>> +    }
>> +}
> 
> Do we need this Display impl, or is the derive(Debug) you have already good
> enough for the warning print?
> 

Good point. Prettier printing is not worth it for such a rare corner case.
The Debug printer still provides the key information that one would need.


thanks,
-- 
John Hubbard
Re: [PATCH] gpu: nova-core: use CStr::from_bytes_until_nul() and remove util.rs
Posted by Danilo Krummrich 1 month ago
On 1/7/26 1:24 AM, John Hubbard wrote:
> $  dmesg -t --level=info|grep NovaCore
> NovaCore 0000:e1:00.0: NVIDIA (Chipset: GA104, Architecture: Ampere, Revision: a.1)
> NovaCore 0000:e1:00.0: GPU name: NVIDIA RTX A4000
> [drm] Initialized nova 0.0.0 for NovaCore.nova-drm.0 on minor 0
> 
> So I'd love to leave the GPU name at info level, if you agree that
> this is about right.

Since it adds additional information about the specific board, which is not
covered by the chipset information already I think it's fine.
Re: [PATCH] gpu: nova-core: use CStr::from_bytes_until_nul() and remove util.rs
Posted by Joel Fernandes 1 month ago

On 1/2/2026 8:34 PM, John Hubbard wrote:
> The util.rs module contained a single helper function,
> str_from_null_terminated(), which duplicated functionality that is now
> available in core::ffi::CStr.
> 
> Specifically, CStr::from_bytes_until_nul() is available in the kernel's
> minimum supported Rust version (1.78.0), so it time to stop using this
> custom workaround.
> 
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>

Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>

Probably elf64_section() should also use CStr::from_bytes_until_nul() in gsp.rs?

Right now it does:
         elf.get(name_idx..)
             .and_then(|nstr| nstr.get(0..=nstr.iter().position(|b| *b == 0)?))
             .and_then(|nstr| CStr::from_bytes_with_nul(nstr).ok())

thanks,

 - Joel





> ---
>  drivers/gpu/nova-core/gsp/commands.rs |  5 +++--
>  drivers/gpu/nova-core/nova_core.rs    |  1 -
>  drivers/gpu/nova-core/util.rs         | 16 ----------------
>  3 files changed, 3 insertions(+), 19 deletions(-)
>  delete mode 100644 drivers/gpu/nova-core/util.rs
> 
> diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
> index 0425c65b5d6f..a11fe6018091 100644
> --- a/drivers/gpu/nova-core/gsp/commands.rs
> +++ b/drivers/gpu/nova-core/gsp/commands.rs
> @@ -30,7 +30,6 @@
>          },
>      },
>      sbuffer::SBufferIter,
> -    util,
>  };
>  
>  /// The `GspSetSystemInfo` command.
> @@ -209,7 +208,9 @@ impl GetGspStaticInfoReply {
>      /// Returns the name of the GPU as a string, or `None` if the string given by the GSP was
>      /// invalid.
>      pub(crate) fn gpu_name(&self) -> Option<&str> {
> -        util::str_from_null_terminated(&self.gpu_name)
> +        CStr::from_bytes_until_nul(&self.gpu_name)
> +            .ok()
> +            .and_then(|cstr| cstr.to_str().ok())
>      }
>  }
>  
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> index b98a1c03f13d..c1121e7c64c5 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -16,7 +16,6 @@
>  mod num;
>  mod regs;
>  mod sbuffer;
> -mod util;
>  mod vbios;
>  
>  pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
> diff --git a/drivers/gpu/nova-core/util.rs b/drivers/gpu/nova-core/util.rs
> deleted file mode 100644
> index 4b503249a3ef..000000000000
> --- a/drivers/gpu/nova-core/util.rs
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -
> -/// Converts a null-terminated byte slice to a string, or `None` if the array does not
> -/// contains any null byte or contains invalid characters.
> -///
> -/// Contrary to [`kernel::str::CStr::from_bytes_with_nul`], the null byte can be anywhere in the
> -/// slice, and not only in the last position.
> -pub(crate) fn str_from_null_terminated(bytes: &[u8]) -> Option<&str> {
> -    use kernel::str::CStr;
> -
> -    bytes
> -        .iter()
> -        .position(|&b| b == 0)
> -        .and_then(|null_pos| CStr::from_bytes_with_nul(&bytes[..=null_pos]).ok())
> -        .and_then(|cstr| cstr.to_str().ok())
> -}
> 
> base-commit: 7acc70476f14661149774ab88d3fe23d83ba4249
Re: [PATCH] gpu: nova-core: use CStr::from_bytes_until_nul() and remove util.rs
Posted by John Hubbard 1 month ago
On 1/3/26 1:21 PM, Joel Fernandes wrote:
> 
> 
> On 1/2/2026 8:34 PM, John Hubbard wrote:
>> The util.rs module contained a single helper function,
>> str_from_null_terminated(), which duplicated functionality that is now
>> available in core::ffi::CStr.
>>
>> Specifically, CStr::from_bytes_until_nul() is available in the kernel's
>> minimum supported Rust version (1.78.0), so it time to stop using this
>> custom workaround.
>>
>> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> 
> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>

Thanks for the review!

> 
> Probably elf64_section() should also use CStr::from_bytes_until_nul() in gsp.rs?
> 
> Right now it does:
>          elf.get(name_idx..)
>              .and_then(|nstr| nstr.get(0..=nstr.iter().position(|b| *b == 0)?))
>              .and_then(|nstr| CStr::from_bytes_with_nul(nstr).ok())
> 

Yes, absolutely! Thanks for spotting that. I'll re-send this out as
a two-patch series that includes that change too.


thanks,
-- 
John Hubbard

> 
>> ---
>>  drivers/gpu/nova-core/gsp/commands.rs |  5 +++--
>>  drivers/gpu/nova-core/nova_core.rs    |  1 -
>>  drivers/gpu/nova-core/util.rs         | 16 ----------------
>>  3 files changed, 3 insertions(+), 19 deletions(-)
>>  delete mode 100644 drivers/gpu/nova-core/util.rs
>>
>> diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
>> index 0425c65b5d6f..a11fe6018091 100644
>> --- a/drivers/gpu/nova-core/gsp/commands.rs
>> +++ b/drivers/gpu/nova-core/gsp/commands.rs
>> @@ -30,7 +30,6 @@
>>          },
>>      },
>>      sbuffer::SBufferIter,
>> -    util,
>>  };
>>  
>>  /// The `GspSetSystemInfo` command.
>> @@ -209,7 +208,9 @@ impl GetGspStaticInfoReply {
>>      /// Returns the name of the GPU as a string, or `None` if the string given by the GSP was
>>      /// invalid.
>>      pub(crate) fn gpu_name(&self) -> Option<&str> {
>> -        util::str_from_null_terminated(&self.gpu_name)
>> +        CStr::from_bytes_until_nul(&self.gpu_name)
>> +            .ok()
>> +            .and_then(|cstr| cstr.to_str().ok())
>>      }
>>  }
>>  
>> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
>> index b98a1c03f13d..c1121e7c64c5 100644
>> --- a/drivers/gpu/nova-core/nova_core.rs
>> +++ b/drivers/gpu/nova-core/nova_core.rs
>> @@ -16,7 +16,6 @@
>>  mod num;
>>  mod regs;
>>  mod sbuffer;
>> -mod util;
>>  mod vbios;
>>  
>>  pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
>> diff --git a/drivers/gpu/nova-core/util.rs b/drivers/gpu/nova-core/util.rs
>> deleted file mode 100644
>> index 4b503249a3ef..000000000000
>> --- a/drivers/gpu/nova-core/util.rs
>> +++ /dev/null
>> @@ -1,16 +0,0 @@
>> -// SPDX-License-Identifier: GPL-2.0
>> -
>> -/// Converts a null-terminated byte slice to a string, or `None` if the array does not
>> -/// contains any null byte or contains invalid characters.
>> -///
>> -/// Contrary to [`kernel::str::CStr::from_bytes_with_nul`], the null byte can be anywhere in the
>> -/// slice, and not only in the last position.
>> -pub(crate) fn str_from_null_terminated(bytes: &[u8]) -> Option<&str> {
>> -    use kernel::str::CStr;
>> -
>> -    bytes
>> -        .iter()
>> -        .position(|&b| b == 0)
>> -        .and_then(|null_pos| CStr::from_bytes_with_nul(&bytes[..=null_pos]).ok())
>> -        .and_then(|cstr| cstr.to_str().ok())
>> -}
>>
>> base-commit: 7acc70476f14661149774ab88d3fe23d83ba4249
>