Re: [PATCH v2 13/13] gpu: nova-core: store Fsp instance in Gpu

[John Hubbard]

On 6/22/26 12:10 AM, Alexandre Courbot wrote:
> The `Fsp` instance was only used in the Hopper+ boot path, and
> consequently built locally (and immediately dropped) in it.
> 
> This worked well as a temporary measure, but the FSP is also needed in
> other parts of the driver, for instance vGPU.

Or: "will be needed, in future patchsets", but not in this one. May I
suggest that we put this patch in one of those future patchsets,
instead of trying to do it here, in advance of anything using
it?

thanks,
-- 
John Hubbard

> 
> Thus, create the `Fsp` instance in the `Gpu` constructor and store it
> there, passing it to the GSP boot as a mutable reference using
> `GspBootContext`. This makes the `Fsp` available even after the GSP is
> booted.
> 
> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
> ---
>  drivers/gpu/nova-core/gpu.rs           | 15 ++++++++++++++-
>  drivers/gpu/nova-core/gsp.rs           |  2 ++
>  drivers/gpu/nova-core/gsp/hal/gh100.rs |  9 +++------
>  3 files changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> index 2e76e4bf79b2..e5ebd79c9020 100644
> --- a/drivers/gpu/nova-core/gpu.rs
> +++ b/drivers/gpu/nova-core/gpu.rs
> @@ -21,11 +21,13 @@
>          Falcon, //
>      },
>      fb::SysmemFlush,
> +    fsp::Fsp,
>      gsp::{
>          self,
>          commands::GetGspStaticInfoReply,
>          Gsp,
> -        GspBootContext, //
> +        GspBootContext,
> +        GspBootMethod, //
>      },
>      regs,
>  };
> @@ -261,6 +263,10 @@ struct GspResources<'gpu> {
>      gsp_falcon: Falcon<GspFalcon>,
>      /// SEC2 falcon instance, used for GSP boot up and cleanup.
>      sec2_falcon: Falcon<Sec2Falcon>,
> +    /// FSP instance, if on an arch that supports it.
> +    // TODO: use different resource types for each boot method, and make the relevant Gsp methods
> +    // generic against them.
> +    fsp: Option<Fsp>,
>      /// GSP runtime data.
>      #[pin]
>      gsp: Gsp,
> @@ -304,6 +310,7 @@ fn drop(self: Pin<&mut Self>) {
>                      chipset: this.spec.chipset,
>                      gsp_falcon: &*this.gsp_falcon,
>                      sec2_falcon: &*this.sec2_falcon,
> +                    fsp: this.fsp.as_mut(),
>                  },
>                  bundle,
>              )
> @@ -354,6 +361,11 @@ pub(crate) fn new(
>  
>                  sec2_falcon: Falcon::new(dev, spec.chipset)?,
>  
> +                fsp: match spec.chipset.gsp_boot_method() {
> +                    GspBootMethod::Sec2 { .. } => None,
> +                    GspBootMethod::Fsp => Some(Fsp::wait_secure_boot(dev, bar, spec.chipset)?),
> +                },
> +
>                  gsp <- Gsp::new(pdev),
>  
>                  // This member must be initialized last, so the `UnloadBundle` can never be dropped
> @@ -365,6 +377,7 @@ pub(crate) fn new(
>                      chipset: spec.chipset,
>                      gsp_falcon,
>                      sec2_falcon,
> +                    fsp: fsp.as_mut(),
>                  })?,
>              }),
>  
> diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
> index 771b38e6335d..ff438506070a 100644
> --- a/drivers/gpu/nova-core/gsp.rs
> +++ b/drivers/gpu/nova-core/gsp.rs
> @@ -38,6 +38,7 @@
>          sec2::Sec2 as Sec2Falcon,
>          Falcon, //
>      },
> +    fsp::Fsp,
>      gpu::{
>          Architecture,
>          Chipset, //
> @@ -62,6 +63,7 @@ pub(crate) struct GspBootContext<'a> {
>      pub(crate) chipset: Chipset,
>      pub(crate) gsp_falcon: &'a Falcon<GspFalcon>,
>      pub(crate) sec2_falcon: &'a Falcon<Sec2Falcon>,
> +    pub(crate) fsp: Option<&'a mut Fsp>,
>  }
>  
>  impl<'a> GspBootContext<'a> {
> diff --git a/drivers/gpu/nova-core/gsp/hal/gh100.rs b/drivers/gpu/nova-core/gsp/hal/gh100.rs
> index 7bba18ba2f75..8673a749dbac 100644
> --- a/drivers/gpu/nova-core/gsp/hal/gh100.rs
> +++ b/drivers/gpu/nova-core/gsp/hal/gh100.rs
> @@ -17,10 +17,7 @@
>          Falcon, //
>      },
>      fb::FbLayout,
> -    fsp::{
> -        FmcBootArgs,
> -        Fsp, //
> -    },
> +    fsp::FmcBootArgs,
>      gsp::{
>          hal::{
>              GspHal,
> @@ -152,12 +149,12 @@ fn boot(
>          let mut unload_bundle = Err(EAGAIN);
>  
>          let res = (|| {
> +            let fsp = ctx.fsp.as_mut().ok_or(ENODEV)?;
> +
>              unload_bundle = Ok(crate::gsp::UnloadBundle(
>                  KBox::new(FspUnloadBundle, GFP_KERNEL)? as KBox<dyn UnloadBundle>,
>              ));
>  
> -            let mut fsp = Fsp::wait_secure_boot(dev, bar, chipset)?;
> -
>              let args = FmcBootArgs::new(
>                  dev,
>                  chipset,
>