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
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
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/
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
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?
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
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.
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
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
>
© 2016 - 2026 Red Hat, Inc.