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 a GPU sub-device, so its lifetime should match the GPU rather than a single boot invocation. It will also be needed in other parts of the driver, for instance vGPU. 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 <[email protected]> Reviewed-by: Eliot Courtney <[email protected]> --- drivers/gpu/nova-core/fsp.rs | 21 +++++++++++++++++++-- drivers/gpu/nova-core/gpu.rs | 9 +++++++++ drivers/gpu/nova-core/gsp.rs | 2 ++ drivers/gpu/nova-core/gsp/hal/gh100.rs | 8 ++------ 4 files changed, 32 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/nova-core/fsp.rs b/drivers/gpu/nova-core/fsp.rs index f0c595175c9c..74ea20258b07 100644 --- a/drivers/gpu/nova-core/fsp.rs +++ b/drivers/gpu/nova-core/fsp.rs @@ -231,20 +231,37 @@ pub(crate) struct Fsp<'a> { } impl<'a> Fsp<'a> { + /// Attempts to create a `Fsp` instance. + /// + /// This can involve waiting for FSP secure boot completion, but should be instantaneous in + /// practice. + /// + /// If `chipset` doesn't support FSP, `Ok(None)` is returned. + pub(crate) fn try_new( + dev: &'a device::Device<device::Bound>, + bar: Bar0<'a>, + chipset: Chipset, + ) -> Result<Option<Self>> { + match hal::fsp_hal(chipset) { + None => Ok(None), + Some(hal) => Self::wait_secure_boot(dev, bar, chipset, hal).map(Option::Some), + } + } + /// Waits for FSP secure boot completion, then returns the [`Fsp`] interface. /// /// Polls the thermal scratch register until FSP signals boot completion or the timeout /// elapses. Returning an [`Fsp`] only on success guarantees, at the API level, that the /// interface is not used before secure boot has completed. - pub(crate) fn wait_secure_boot( + fn wait_secure_boot( dev: &'a device::Device<device::Bound>, bar: Bar0<'a>, chipset: Chipset, + hal: &'static dyn hal::FspHal, ) -> Result<Fsp<'a>> { /// FSP secure boot completion timeout in milliseconds. const FSP_SECURE_BOOT_TIMEOUT_MS: i64 = 5000; - let hal = hal::fsp_hal(chipset).ok_or(ENOTSUPP)?; let falcon = Falcon::<FspEngine>::new(dev, chipset, bar)?; let fsp_fw = FspFirmware::new(dev, chipset, FIRMWARE_VERSION)?; diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs index fc90069bc2fe..442c0979f9c6 100644 --- a/drivers/gpu/nova-core/gpu.rs +++ b/drivers/gpu/nova-core/gpu.rs @@ -22,6 +22,7 @@ Falcon, // }, fb::SysmemFlush, + fsp::Fsp, gsp::{ self, commands::GetGspStaticInfoReply, @@ -262,6 +263,10 @@ struct GspResources<'gpu> { gsp_falcon: Falcon<'gpu, GspFalcon>, /// SEC2 falcon instance, used for GSP boot up and cleanup. sec2_falcon: Falcon<'gpu, 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<'gpu>>, /// GSP runtime data. #[pin] gsp: Gsp, @@ -305,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, ) @@ -356,6 +362,8 @@ pub(crate) fn new( sec2_falcon: Falcon::new(dev, spec.chipset, bar)?, + fsp: Fsp::try_new(dev, bar, spec.chipset)?, + gsp <- Gsp::new(pdev), // This member must be initialized last, so the `UnloadBundle` can never be dropped @@ -367,6 +375,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 9f055a0d6cb9..d89cc3ba7c72 100644 --- a/drivers/gpu/nova-core/gsp.rs +++ b/drivers/gpu/nova-core/gsp.rs @@ -39,6 +39,7 @@ sec2::Sec2 as Sec2Falcon, Falcon, // }, + fsp::Fsp, gpu::Chipset, gsp::{ cmdq::Cmdq, @@ -65,6 +66,7 @@ pub(crate) struct GspBootContext<'ctx, 'gpu> { pub(crate) chipset: Chipset, pub(crate) gsp_falcon: &'ctx Falcon<'gpu, GspFalcon>, pub(crate) sec2_falcon: &'ctx Falcon<'gpu, Sec2Falcon>, + pub(crate) fsp: Option<&'ctx mut Fsp<'gpu>>, } impl<'ctx, 'gpu> GspBootContext<'ctx, 'gpu> { diff --git a/drivers/gpu/nova-core/gsp/hal/gh100.rs b/drivers/gpu/nova-core/gsp/hal/gh100.rs index be531df8680b..d1c4f6104701 100644 --- a/drivers/gpu/nova-core/gsp/hal/gh100.rs +++ b/drivers/gpu/nova-core/gsp/hal/gh100.rs @@ -16,10 +16,7 @@ Falcon, // }, fb::FbLayout, - fsp::{ - FmcBootArgs, - Fsp, // - }, + fsp::FmcBootArgs, gsp::{ hal::{ GspHal, @@ -142,7 +139,6 @@ fn boot( wpr_meta: &Coherent<GspFwWprMeta>, ) -> Result<Option<crate::gsp::UnloadBundle>> { let dev = ctx.dev(); - let bar = ctx.bar; let chipset = ctx.chipset; let gsp_falcon = ctx.gsp_falcon; @@ -150,7 +146,7 @@ fn boot( KBox::new(FspUnloadBundle, GFP_KERNEL)? as KBox<dyn UnloadBundle> ); - let mut fsp = Fsp::wait_secure_boot(dev, bar, chipset)?; + let fsp = ctx.fsp.as_mut().ok_or(ENODEV)?; let args = FmcBootArgs::new( dev, -- 2.55.0
