Re: [PATCH 21/27] gpu: nova-core: gsp: add the GMC boot event dispatcher

On Tue, 18 Aug 2026 20:52:14 -0700
John Hubbard <[email protected]> wrote:

Looks good to me besides a nit below.

Reviewed-by: Zhi Wang <[email protected]>

> The r000 GSP boot protocol sends its two load-and-execute events as
> GMC commands, each identified by a command id.
> 
> Nova-core has a handler for each of the two events, but nothing that
> reads a command id and selects between them.
> 
> Add the dispatcher, keyed on the two command ids from the r000
> bindings.
> 
> Assisted-by: Cursor:claude-opus-5
> Signed-off-by: John Hubbard <[email protected]>
> ---
>  drivers/gpu/nova-core/gsp/boot.rs | 62
> +++++++++++++++++++++++++++++-- drivers/gpu/nova-core/gsp/fw.rs   |
> 10 +++++ 2 files changed, 69 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/gsp/boot.rs
> b/drivers/gpu/nova-core/gsp/boot.rs index d62a5c834ccf..15933ecc061b
> 100644 --- a/drivers/gpu/nova-core/gsp/boot.rs
> +++ b/drivers/gpu/nova-core/gsp/boot.rs
> @@ -36,7 +36,11 @@
>      },
>      gsp::{
>          cmdq::Cmdq,
> -        commands, //
> +        commands,
> +        fw::{
> +            GMCAPI_CMD_EXEC_GENERIC_BOOTLOADER,
> +            GMCAPI_CMD_EXEC_HS_BINARY, //
> +        }, //
>      },
>      regs, //
>  };
> @@ -162,6 +166,60 @@ fn core_resume(
>          Ok(())
>      }
>  
> +    /// Dispatch one GMC boot event to its load-and-execute handler.
> +    ///
> +    /// LIBOS2 chipsets send `GMCAPI_CMD_EXEC_GENERIC_BOOTLOADER`
> and LIBOS3 chipsets send
> +    /// `GMCAPI_CMD_EXEC_HS_BINARY`, so a given GPU only ever
> reaches one of the two handlers.
> +    ///
> +    /// # Errors
> +    ///
> +    /// - `EINVAL` if `command_id` is not a load-and-execute command.
> +    ///
> +    /// Errors from the handlers are propagated as-is.
> +    #[expect(dead_code)]
> +    #[allow(clippy::too_many_arguments)]

Should this be expect? So that if we refactored this arugment list in
future and it gets shorter then CLIPPY will tell us to remove the tag
above.

> +    fn dispatch_gmc_boot_event(
> +        command_id: u32,
> +        payload: &[u8],
> +        bootloader: &GenericBootloader,
> +        gsp_falcon: &Falcon<'_, Gsp>,
> +        sec2_falcon: &Falcon<'_, Sec2>,
> +        bar: Bar0<'_>,
> +        dev: &device::Device,
> +        bootloader_app_version: u32,
> +        libos_dma_handle: u64,
> +    ) -> Result {
> +        match command_id {
> +            GMCAPI_CMD_EXEC_GENERIC_BOOTLOADER =>
> Self::handle_load_exec_bootloader(
> +                payload,
> +                bootloader,
> +                gsp_falcon,
> +                sec2_falcon,
> +                bar,
> +                dev,
> +                bootloader_app_version,
> +                libos_dma_handle,
> +            ),
> +            GMCAPI_CMD_EXEC_HS_BINARY =>
> Self::handle_load_exec_hs_binary(
> +                payload,
> +                gsp_falcon,
> +                sec2_falcon,
> +                bar,
> +                dev,
> +                bootloader_app_version,
> +                libos_dma_handle,
> +            ),
> +            _ => {
> +                dev_err!(
> +                    dev,
> +                    "Unexpected GMC boot event:
> command_id={:#010x}\n",
> +                    command_id
> +                );
> +                Err(EINVAL)
> +            }
> +        }
> +    }
> +
>      /// Handle a `GSP_LOAD_EXEC_GENERIC_BOOTLOADER` event.
>      ///
>      /// The driver does not copy the image the GSP asks for. It
> writes the descriptor the event @@ -175,7 +233,6 @@ fn core_resume(
>      ///   size this driver mirrors, or the event names a context DMA
> slot or an aperture that does ///   not exist.
>      /// - `ETIMEDOUT` if the GSP does not suspend, or the image does
> not halt, in time.
> -    #[expect(dead_code)]
>      #[allow(clippy::too_many_arguments)]
>      fn handle_load_exec_bootloader(
>          payload: &[u8],
> @@ -268,7 +325,6 @@ fn handle_load_exec_bootloader(
>      /// - `EINVAL` if the payload is shorter than the parameter
> block, or the ucode id does not ///   fit the BROM register field.
>      /// - `ETIMEDOUT` if the GSP does not suspend, or the binary
> does not halt, in time.
> -    #[expect(dead_code)]
>      #[allow(clippy::too_many_arguments)]
>      fn handle_load_exec_hs_binary(
>          payload: &[u8],
> diff --git a/drivers/gpu/nova-core/gsp/fw.rs
> b/drivers/gpu/nova-core/gsp/fw.rs index 83f7d2042aa1..4772af362117
> 100644 --- a/drivers/gpu/nova-core/gsp/fw.rs
> +++ b/drivers/gpu/nova-core/gsp/fw.rs
> @@ -979,6 +979,16 @@ pub(crate) struct GmcApiHeader {
>  /// `GMCAPI_HEADER_COMMAND_ID_MASK`. The remaining byte carries
> flags. const GMCAPI_COMMAND_ID_MASK: u32 = 0x00ff_ffff;
>  
> +/// GMC command asking the driver to run the generic falcon
> bootloader against a descriptor the +/// GSP supplies.
> +pub(crate) const GMCAPI_CMD_EXEC_GENERIC_BOOTLOADER: u32 =
> +    r000_00::GMCAPI_COMMANDS_GMCAPI_CMD_EXEC_GENERIC_BOOTLOADER;
> +
> +/// GMC command asking the driver to run a high-security binary the
> GSP has placed in the +/// framebuffer.
> +pub(crate) const GMCAPI_CMD_EXEC_HS_BINARY: u32 =
> +    r000_00::GMCAPI_COMMANDS_GMCAPI_CMD_EXEC_HS_BINARY;
> +
>  static_assert!(size_of::<GmcApiHeader>() == 40);
>  
>  impl GmcApiHeader {