This one needed some rework.
On Sat Nov 15, 2025 at 4:55 AM JST, Joel Fernandes wrote:
> From: Alistair Popple <[email protected]>
>
> After GSP initialization is complete, retrieve the static configuration
> information from GSP-RM. This information includes GPU name, capabilities,
> memory configuration, and other properties. On some GPU variants, it is
> also required to do this for initialization to complete.
>
> Signed-off-by: Alistair Popple <[email protected]>
> Co-developed-by: Joel Fernandes <[email protected]>
> Signed-off-by: Joel Fernandes <[email protected]>
> ---
> drivers/gpu/nova-core/gsp/boot.rs | 7 +
> drivers/gpu/nova-core/gsp/commands.rs | 65 +++++++
> drivers/gpu/nova-core/gsp/fw.rs | 5 +
> .../gpu/nova-core/gsp/fw/r570_144/bindings.rs | 163 ++++++++++++++++++
> drivers/gpu/nova-core/nova_core.rs | 1 +
> drivers/gpu/nova-core/util.rs | 16 ++
> 6 files changed, 257 insertions(+)
> create mode 100644 drivers/gpu/nova-core/util.rs
>
> diff --git a/drivers/gpu/nova-core/gsp/boot.rs
> b/drivers/gpu/nova-core/gsp/boot.rs
> index c0afafbf35f6..42a3abb9243d 100644
> --- a/drivers/gpu/nova-core/gsp/boot.rs
> +++ b/drivers/gpu/nova-core/gsp/boot.rs
> @@ -40,6 +40,7 @@
> GspFwWprMeta, //
> },
> regs,
> + util, //
> vbios::Vbios,
> };
>
> @@ -237,6 +238,12 @@ pub(crate) fn boot(
> GspSequencer::run(&mut self.cmdq, seq_params, Delta::from_secs(10))?;
>
> commands::gsp_init_done(&mut self.cmdq, Delta::from_secs(10))?;
> + let info = commands::get_gsp_info(&mut self.cmdq, bar)?;
> + dev_info!(
> + pdev.as_ref(),
> + "GPU name: {}\n",
> + util::str_from_null_terminated(&info.gpu_name)
> + );
>
> Ok(())
> }
> diff --git a/drivers/gpu/nova-core/gsp/commands.rs
> b/drivers/gpu/nova-core/gsp/commands.rs
> index 07abfb54f9d7..6cb32e7d3436 100644
> --- a/drivers/gpu/nova-core/gsp/commands.rs
> +++ b/drivers/gpu/nova-core/gsp/commands.rs
> @@ -17,6 +17,7 @@
> };
>
> use crate::{
> + driver::Bar0,
> gsp::{
> cmdq::{
> Cmdq,
> @@ -25,12 +26,25 @@
> },
> fw::{
> commands::*,
> + GspStaticConfigInfo_t,
> MsgFunction, //
> },
> },
> sbuffer::SBufferIter,
> + util,
> };
>
> +// SAFETY: Padding is explicit and will not contain uninitialized data.
> +unsafe impl AsBytes for GspStaticConfigInfo_t {}
> +
> +// SAFETY: This struct only contains integer types for which all bit patterns
> +// are valid.
> +unsafe impl FromBytes for GspStaticConfigInfo_t {}
> +
> +pub(crate) struct GspStaticConfigInfo {
> + pub gpu_name: [u8; 40],
This is probably meant to be `0x40`, as the `gpuNameString` bindings
member this is built from is 64-bytes long. Changing it to be 64-bytes
long.
> +}
> +
> /// Message type for GSP initialization done notification.
> struct GspInitDone {}
>
> @@ -62,6 +76,57 @@ pub(crate) fn gsp_init_done(cmdq: &mut Cmdq, timeout:
> Delta) -> Result {
> }
> }
>
> +impl MessageFromGsp for GspStaticConfigInfo {
> + const FUNCTION: MsgFunction = MsgFunction::GetGspStaticInfo;
> + type InitError = Infallible;
> + type Message = GspStaticConfigInfo_t;
> +
> + fn read(
> + msg: &Self::Message,
> + _sbuffer: &mut SBufferIter<array::IntoIter<&[u8], 2>>,
> + ) -> Result<Self, Self::InitError> {
> + let gpu_name_str =
> util::str_from_null_terminated(&msg.gpuNameString);
> +
> + let mut gpu_name = [0u8; 40];
> + let bytes = gpu_name_str.as_bytes();
> + let copy_len = core::cmp::min(bytes.len(), gpu_name.len());
> + gpu_name[..copy_len].copy_from_slice(&bytes[..copy_len]);
> + gpu_name[copy_len] = b'\0';
We will want to move this into an `impl` block of `fw::commands`, since
`gpuNameString` is supposed to be firmware-specific and thus private.
But actually, do we even need this? When we print the GPU name in
`boot.rs`, we call `util::str_from_null_terminated` again on the bytes
array that we copied from here, but it is guaranteed to succeed if this
call succeeded as well. So I think (and successfully tested that) we can
just expose the raw bytes array as-is here, and extract the name from a
dedicated method.
> +
> + Ok(GspStaticConfigInfo { gpu_name })
> + }
> +}
> +
> +// SAFETY: This struct only contains integer types and fixed-size arrays for
> which
> +// all bit patterns are valid.
> +unsafe impl Zeroable for GspStaticConfigInfo_t {}
> +
> +struct GetGspInfo;
> +
> +impl CommandToGsp for GetGspInfo {
> + const FUNCTION: MsgFunction = MsgFunction::GetGspStaticInfo;
> + type Command = GspStaticConfigInfo_t;
> + type InitError = Infallible;
> +
> + fn init(&self) -> impl Init<Self::Command, Self::InitError> {
> + init!(GspStaticConfigInfo_t {
> + ..Zeroable::init_zeroed()
> + })
> + }
> +}
> +
> +pub(crate) fn get_gsp_info(cmdq: &mut Cmdq, bar: &Bar0) ->
> Result<GspStaticConfigInfo> {
> + cmdq.send_command(bar, GetGspInfo)?;
> +
> + loop {
> + match cmdq.receive_msg::<GspStaticConfigInfo>(Delta::from_secs(5)) {
> + Ok(info) => return Ok(info),
> + Err(ERANGE) => continue,
> + Err(e) => return Err(e),
> + }
> + }
> +}
> +
> /// The `GspSetSystemInfo` command.
> pub(crate) struct SetSystemInfo<'a> {
> pdev: &'a pci::Device<device::Bound>,
> diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
> index 0cce54310c35..5b6a906ff5dc 100644
> --- a/drivers/gpu/nova-core/gsp/fw.rs
> +++ b/drivers/gpu/nova-core/gsp/fw.rs
> @@ -882,6 +882,11 @@ pub(crate) fn element_count(&self) -> u32 {
> }
> }
>
> +pub(crate) use r570_144::{
> + // GSP static configuration information.
> + GspStaticConfigInfo_t, //
> +};
I've replaced this with a dedicated wrapping type and supporting code in
`fw::commands`, as is done for the other commands.
<snip>
> diff --git a/drivers/gpu/nova-core/util.rs b/drivers/gpu/nova-core/util.rs
> new file mode 100644
> index 000000000000..f1a4dea44c10
> --- /dev/null
> +++ b/drivers/gpu/nova-core/util.rs
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/// Converts a null-terminated byte array to a string slice.
> +///
> +/// Returns "invalid" if the bytes are not valid UTF-8 or not
> null-terminated.
> +pub(crate) fn str_from_null_terminated(bytes: &[u8]) -> &str {
> + use kernel::str::CStr;
> +
> + // Find the first null byte, then create a slice that includes it.
> + 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())
> + .unwrap_or("invalid")
> +}
Let's not decide what the behavior on an invalid string is for the
caller, and let's make these cases detectable by returning an `Option`.
The caller can then `unwrap_or` it as it wants.
I'll apply these changes locally before pushing, as they are mostly
about harmonizing with the design of other existing commands and don't
change the behavior in any way.
