* [PATCH v5 01/13] gpu: nova-core: gsp: sequencer: use GspBootContext
2026-07-07 7:21 [PATCH v5 00/13] gpu: nova-core: consolidate and streamline GSP boot process Alexandre Courbot
@ 2026-07-07 7:21 ` Alexandre Courbot
2026-07-07 7:21 ` [PATCH v5 02/13] gpu: nova-core: gsp: sequencer: do not store sequence into GspSequencer Alexandre Courbot
` (11 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-07-07 7:21 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Gary Guo, John Hubbard, Alistair Popple, Timur Tabi,
Eliot Courtney, Zhi Wang
Cc: nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
`GspBootContext` contains all the resources currently carried by
`GspSequencerParams`, so replace the latter with the former for better
integration with the boot process and less code.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/gsp/hal/tu102.rs | 21 +++++++-------------
drivers/gpu/nova-core/gsp/sequencer.rs | 36 ++++++++++++----------------------
2 files changed, 20 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/hal/tu102.rs b/drivers/gpu/nova-core/gsp/hal/tu102.rs
index ff71b45b5432..87ceb8878f01 100644
--- a/drivers/gpu/nova-core/gsp/hal/tu102.rs
+++ b/drivers/gpu/nova-core/gsp/hal/tu102.rs
@@ -37,10 +37,7 @@
GspHal,
UnloadBundle, //
},
- sequencer::{
- GspSequencer,
- GspSequencerParams, //
- },
+ sequencer::GspSequencer,
Gsp,
GspBootContext,
GspFwWprMeta, //
@@ -326,16 +323,12 @@ fn boot<'a>(
}
fn post_boot(&self, gsp: &Gsp, ctx: &GspBootContext<'_>, gsp_fw: &GspFirmware) -> Result {
- // Create and run the GSP sequencer.
- let seq_params = GspSequencerParams {
- bootloader_app_version: gsp_fw.bootloader.app_version,
- libos_dma_handle: gsp.libos.dma_handle(),
- gsp_falcon: ctx.gsp_falcon,
- sec2_falcon: ctx.sec2_falcon,
- dev: ctx.dev(),
- bar: ctx.bar,
- };
- GspSequencer::run(&gsp.cmdq, seq_params)?;
+ GspSequencer::run(
+ &gsp.cmdq,
+ ctx,
+ gsp.libos.dma_handle(),
+ gsp_fw.bootloader.app_version,
+ )?;
Ok(())
}
diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs b/drivers/gpu/nova-core/gsp/sequencer.rs
index 13983d42b12b..f55205bd61f3 100644
--- a/drivers/gpu/nova-core/gsp/sequencer.rs
+++ b/drivers/gpu/nova-core/gsp/sequencer.rs
@@ -31,6 +31,7 @@
MessageFromGsp, //
},
fw,
+ GspBootContext, //
},
num::FromSafeCast,
sbuffer::SBufferIter,
@@ -335,24 +336,13 @@ fn iter(&self) -> GspSeqIter<'_> {
}
}
-/// Parameters for running the GSP sequencer.
-pub(crate) struct GspSequencerParams<'a> {
- /// Bootloader application version.
- pub(crate) bootloader_app_version: u32,
- /// LibOS DMA handle address.
- pub(crate) libos_dma_handle: u64,
- /// GSP falcon for core operations.
- pub(crate) gsp_falcon: &'a Falcon<'a, Gsp>,
- /// SEC2 falcon for core operations.
- pub(crate) sec2_falcon: &'a Falcon<'a, Sec2>,
- /// Device for logging.
- pub(crate) dev: &'a device::Device,
- /// BAR0 for register access.
- pub(crate) bar: Bar0<'a>,
-}
-
impl<'a> GspSequencer<'a> {
- pub(crate) fn run(cmdq: &Cmdq, params: GspSequencerParams<'a>) -> Result {
+ pub(crate) fn run(
+ cmdq: &Cmdq,
+ ctx: &'a GspBootContext<'_>,
+ libos_dma_handle: u64,
+ bootloader_app_version: u32,
+ ) -> Result {
let seq_info = loop {
match cmdq.receive_msg::<GspSequence>(Cmdq::RECEIVE_TIMEOUT) {
Ok(seq_info) => break seq_info,
@@ -363,12 +353,12 @@ pub(crate) fn run(cmdq: &Cmdq, params: GspSequencerParams<'a>) -> Result {
let sequencer = GspSequencer {
seq_info,
- bar: params.bar,
- sec2_falcon: params.sec2_falcon,
- gsp_falcon: params.gsp_falcon,
- libos_dma_handle: params.libos_dma_handle,
- bootloader_app_version: params.bootloader_app_version,
- dev: params.dev,
+ bar: ctx.bar,
+ sec2_falcon: ctx.sec2_falcon,
+ gsp_falcon: ctx.gsp_falcon,
+ libos_dma_handle,
+ bootloader_app_version,
+ dev: ctx.dev(),
};
dev_dbg!(sequencer.dev, "Running CPU Sequencer commands\n");
--
2.55.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v5 02/13] gpu: nova-core: gsp: sequencer: do not store sequence into GspSequencer
2026-07-07 7:21 [PATCH v5 00/13] gpu: nova-core: consolidate and streamline GSP boot process Alexandre Courbot
2026-07-07 7:21 ` [PATCH v5 01/13] gpu: nova-core: gsp: sequencer: use GspBootContext Alexandre Courbot
@ 2026-07-07 7:21 ` Alexandre Courbot
2026-07-07 7:21 ` [PATCH v5 03/13] gpu: nova-core: gsp: replace BootUnloadGuard with local handlers Alexandre Courbot
` (10 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-07-07 7:21 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Gary Guo, John Hubbard, Alistair Popple, Timur Tabi,
Eliot Courtney, Zhi Wang
Cc: nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
The sequence is currently stored in the `GspSequencer` even though its
lifetime is limited to `GspSequencer::run`. This object-oriented design
does not play well with the borrow-checker, as `GspSequencer::iter`
borrows the `GspSequencer`, which makes it difficult to introduce
mutable references in `GspBootContext`, as we want to do in order to
make the `Falcon` references mutable.
Thus, store the sequence locally in `GspSequencer::run`, and move
iterator creation to `GspSeqIter::new` so it no longer needs to borrow
the whole `GspSequencer`.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/gsp/sequencer.rs | 35 +++++++++++++++-------------------
1 file changed, 15 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs b/drivers/gpu/nova-core/gsp/sequencer.rs
index f55205bd61f3..ddce32cc4e30 100644
--- a/drivers/gpu/nova-core/gsp/sequencer.rs
+++ b/drivers/gpu/nova-core/gsp/sequencer.rs
@@ -129,8 +129,6 @@ pub(crate) fn new(data: &[u8], dev: &device::Device) -> Result<(Self, usize)> {
/// GSP Sequencer for executing firmware commands during boot.
pub(crate) struct GspSequencer<'a> {
- /// Sequencer information with command data.
- seq_info: GspSequence,
/// `Bar0` for register access.
bar: Bar0<'a>,
/// SEC2 falcon for core operations.
@@ -268,7 +266,7 @@ fn run(&self, seq: &GspSequencer<'_>) -> Result {
}
/// Iterator over GSP sequencer commands.
-pub(crate) struct GspSeqIter<'a> {
+struct GspSeqIter<'a> {
/// Command data buffer.
cmd_data: &'a [u8],
/// Current position in the buffer.
@@ -281,6 +279,18 @@ pub(crate) struct GspSeqIter<'a> {
dev: &'a device::Device,
}
+impl<'a> GspSeqIter<'a> {
+ fn new(seq: &'a GspSequence, dev: &'a device::Device) -> Self {
+ Self {
+ cmd_data: &seq.cmd_data,
+ current_offset: 0,
+ total_cmds: seq.cmd_index,
+ cmds_processed: 0,
+ dev,
+ }
+ }
+}
+
impl<'a> Iterator for GspSeqIter<'a> {
type Item = Result<GspSeqCmd>;
@@ -322,20 +332,6 @@ fn next(&mut self) -> Option<Self::Item> {
}
}
-impl<'a> GspSequencer<'a> {
- fn iter(&self) -> GspSeqIter<'_> {
- let cmd_data = &self.seq_info.cmd_data[..];
-
- GspSeqIter {
- cmd_data,
- current_offset: 0,
- total_cmds: self.seq_info.cmd_index,
- cmds_processed: 0,
- dev: self.dev,
- }
- }
-}
-
impl<'a> GspSequencer<'a> {
pub(crate) fn run(
cmdq: &Cmdq,
@@ -352,7 +348,6 @@ pub(crate) fn run(
};
let sequencer = GspSequencer {
- seq_info,
bar: ctx.bar,
sec2_falcon: ctx.sec2_falcon,
gsp_falcon: ctx.gsp_falcon,
@@ -363,14 +358,14 @@ pub(crate) fn run(
dev_dbg!(sequencer.dev, "Running CPU Sequencer commands\n");
- for cmd_result in sequencer.iter() {
+ for cmd_result in GspSeqIter::new(&seq_info, sequencer.dev) {
match cmd_result {
Ok(cmd) => cmd.run(&sequencer)?,
Err(e) => {
dev_err!(
sequencer.dev,
"Error running command at index {}\n",
- sequencer.seq_info.cmd_index
+ seq_info.cmd_index
);
return Err(e);
}
--
2.55.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v5 03/13] gpu: nova-core: gsp: replace BootUnloadGuard with local handlers
2026-07-07 7:21 [PATCH v5 00/13] gpu: nova-core: consolidate and streamline GSP boot process Alexandre Courbot
2026-07-07 7:21 ` [PATCH v5 01/13] gpu: nova-core: gsp: sequencer: use GspBootContext Alexandre Courbot
2026-07-07 7:21 ` [PATCH v5 02/13] gpu: nova-core: gsp: sequencer: do not store sequence into GspSequencer Alexandre Courbot
@ 2026-07-07 7:21 ` Alexandre Courbot
2026-07-07 8:04 ` Eliot Courtney
2026-07-07 7:21 ` [PATCH v5 04/13] gpu: nova-core: gsp: pass GspBootContext to unload methods Alexandre Courbot
` (9 subsequent siblings)
12 siblings, 1 reply; 20+ messages in thread
From: Alexandre Courbot @ 2026-07-07 7:21 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Gary Guo, John Hubbard, Alistair Popple, Timur Tabi,
Eliot Courtney, Zhi Wang
Cc: nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
When adding the GSP unload capability, we introduced `BootUnloadGuard`
to automatically call `Gsp::unload` whenever an error occurred during
the boot process, in order to try to reset the GSP to a valid state.
This approach is not well-suited to the errors that may occur in HALs:
by definition, an error occurring in the HAL means that the GSP is not
booted; yet the first thing that `Gsp::unload` does is queue a shutdown
message to the GSP, which will inevitably result in a timeout when done
from a HAL.
Furthermore, `BootUnloadGuard` is problematic because it holds
additional references to the boot context, notably the `Falcon`s. These
extra references stand in the way of making some of the `Falcon`'s
methods mutable, since those methods would require exclusive access. As
this behavior is only needed in one place, introducing dedicated types
for it is distracting and unnecessary.
Thus, remove `BootUnloadGuard` and adopt a two-level error handling
strategy:
- HALs are free to handle their errors as they see fit (most likely, by
running their unload bundle if it is ready by the time of the error),
- `Gsp::boot` uses a `ScopeGuard` that runs `Gsp::unload`, since the
GSP should be up and running by the time `GspHal::boot` has returned.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/gsp/boot.rs | 67 +++-------------------------------
drivers/gpu/nova-core/gsp/hal.rs | 13 +++----
drivers/gpu/nova-core/gsp/hal/gh100.rs | 31 ++++++++++------
drivers/gpu/nova-core/gsp/hal/tu102.rs | 23 +++++++-----
4 files changed, 44 insertions(+), 90 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index ab0491b57944..536f2e341c01 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -30,66 +30,6 @@
},
};
-/// Arguments required to call [`Gsp::unload`](super::Gsp::unload).
-///
-/// Stored as their own type to avoid repeating a long and tedious list in [`BootUnloadGuard`].
-pub(super) struct BootUnloadArgs<'a> {
- gsp: &'a super::Gsp,
- dev: &'a device::Device<device::Bound>,
- bar: Bar0<'a>,
- gsp_falcon: &'a Falcon<'a, Gsp>,
- sec2_falcon: &'a Falcon<'a, Sec2>,
- unload_bundle: Option<super::UnloadBundle>,
-}
-
-/// Guard that calls [`Gsp::unload`](super::Gsp::unload) with a
-/// [`UnloadBundle`](super::UnloadBundle) when dropped.
-///
-/// Used to ensure the `UnloadBundle` is run during failure paths.
-pub(super) struct BootUnloadGuard<'a> {
- guard: ScopeGuard<BootUnloadArgs<'a>, fn(BootUnloadArgs<'a>)>,
-}
-
-impl<'a> BootUnloadGuard<'a> {
- /// Wraps `unload_bundle` into a guard that executes it when dropped.
- pub(super) fn new(
- gsp: &'a super::Gsp,
- dev: &'a device::Device<device::Bound>,
- bar: Bar0<'a>,
- gsp_falcon: &'a Falcon<'a, Gsp>,
- sec2_falcon: &'a Falcon<'a, Sec2>,
- unload_bundle: Option<super::UnloadBundle>,
- ) -> Self {
- Self {
- guard: ScopeGuard::new_with_data(
- BootUnloadArgs {
- gsp,
- dev,
- bar,
- gsp_falcon,
- sec2_falcon,
- unload_bundle,
- },
- |args| {
- let _ = super::Gsp::unload(
- args.gsp,
- args.dev,
- args.bar,
- args.gsp_falcon,
- args.sec2_falcon,
- args.unload_bundle,
- );
- },
- ),
- }
- }
-
- /// Disarms the guard and returns the [`UnloadBundle`](super::UnloadBundle) it contains.
- pub(super) fn dismiss(self) -> Option<super::UnloadBundle> {
- self.guard.dismiss().unload_bundle
- }
-}
-
impl super::Gsp {
/// Attempt to boot the GSP.
///
@@ -107,6 +47,7 @@ pub(crate) fn boot(
let bar = ctx.bar;
let chipset = ctx.chipset;
let gsp_falcon = ctx.gsp_falcon;
+ let sec2_falcon = ctx.sec2_falcon;
let dev = pdev.as_ref();
let hal = super::hal::gsp_hal(chipset);
@@ -118,7 +59,11 @@ pub(crate) fn boot(
let wpr_meta = Coherent::init(dev, GFP_KERNEL, GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
// Perform the chipset-specific boot sequence, and retrieve the unload bundle.
- let unload_guard = hal.boot(&self, &ctx, &fb_layout, &wpr_meta)?;
+ let unload_bundle = hal.boot(&self, &ctx, &fb_layout, &wpr_meta)?;
+
+ let unload_guard = ScopeGuard::new_with_data(unload_bundle, |unload_bundle| {
+ let _ = self.unload(dev, bar, gsp_falcon, sec2_falcon, unload_bundle);
+ });
gsp_falcon.write_os_version(gsp_fw.bootloader.app_version);
diff --git a/drivers/gpu/nova-core/gsp/hal.rs b/drivers/gpu/nova-core/gsp/hal.rs
index d3e47ef206de..851d1f24c137 100644
--- a/drivers/gpu/nova-core/gsp/hal.rs
+++ b/drivers/gpu/nova-core/gsp/hal.rs
@@ -24,7 +24,6 @@
Chipset, //
},
gsp::{
- boot::BootUnloadGuard,
Gsp,
GspBootContext,
GspFwWprMeta, //
@@ -51,15 +50,15 @@ fn run(
pub(super) trait GspHal: Send {
/// Performs the GSP boot process, loading and running the required firmwares as needed.
///
- /// Upon success, returns a guard that runs the GSP unload sequence if GSP boot does not
- /// complete.
- fn boot<'a>(
+ /// Upon success, returns the [`crate::gsp::UnloadBundle`] to use with [`Gsp::unload`], if one
+ /// could be created.
+ fn boot(
&self,
- gsp: &'a Gsp,
- ctx: &GspBootContext<'a>,
+ gsp: &Gsp,
+ ctx: &GspBootContext<'_>,
fb_layout: &FbLayout,
wpr_meta: &Coherent<GspFwWprMeta>,
- ) -> Result<BootUnloadGuard<'a>>;
+ ) -> Result<Option<crate::gsp::UnloadBundle>>;
/// Performs HAL-specific post-GSP boot tasks.
///
diff --git a/drivers/gpu/nova-core/gsp/hal/gh100.rs b/drivers/gpu/nova-core/gsp/hal/gh100.rs
index 1d06405a32f6..18c889f9f413 100644
--- a/drivers/gpu/nova-core/gsp/hal/gh100.rs
+++ b/drivers/gpu/nova-core/gsp/hal/gh100.rs
@@ -23,7 +23,6 @@
Fsp, //
},
gsp::{
- boot::BootUnloadGuard,
hal::{
GspHal,
UnloadBundle, //
@@ -143,13 +142,13 @@ impl GspHal for Gh100 {
///
/// This path uses FSP to establish a chain of trust and boot GSP-FMC. FSP handles
/// the GSP boot internally - no manual GSP reset/boot is needed.
- fn boot<'a>(
+ fn boot(
&self,
- gsp: &'a Gsp,
- ctx: &GspBootContext<'a>,
+ gsp: &Gsp,
+ ctx: &GspBootContext<'_>,
fb_layout: &FbLayout,
wpr_meta: &Coherent<GspFwWprMeta>,
- ) -> Result<BootUnloadGuard<'a>> {
+ ) -> Result<Option<crate::gsp::UnloadBundle>> {
let dev = ctx.dev();
let bar = ctx.bar;
let chipset = ctx.chipset;
@@ -160,10 +159,6 @@ fn boot<'a>(
KBox::new(FspUnloadBundle, GFP_KERNEL)? as KBox<dyn UnloadBundle>
);
- // Wrap the unload bundle into a drop guard so it is automatically run upon failure.
- let unload_guard =
- BootUnloadGuard::new(gsp, dev, bar, gsp_falcon, sec2_falcon, Some(unload_bundle));
-
let mut fsp = Fsp::wait_secure_boot(dev, bar, chipset)?;
let args = FmcBootArgs::new(
@@ -174,11 +169,23 @@ fn boot<'a>(
false,
)?;
- fsp.boot_fmc(dev, fb_layout, &args)?;
+ // Keep the result as we want to wait for lockdown release even in case of error, to make
+ // sure `args` is not accessed by the GSP anymore.
+ let fsp_res = fsp.boot_fmc(dev, fb_layout, &args);
- wait_for_gsp_lockdown_release(dev, gsp_falcon, args.boot_params_dma_handle())?;
+ // Wait for GSP-FMC to release the GSP lockdown, indicating that `args` is not accessed
+ // anymore.
+ let lockdown_res =
+ wait_for_gsp_lockdown_release(dev, gsp_falcon, args.boot_params_dma_handle());
- Ok(unload_guard)
+ match fsp_res.and(lockdown_res) {
+ Ok(()) => Ok(Some(unload_bundle)),
+ Err(e) => {
+ // Wait for the GSP RISC-V core to halt in case of error.
+ let _ = unload_bundle.0.run(dev, bar, gsp_falcon, sec2_falcon);
+ Err(e)
+ }
+ }
}
}
diff --git a/drivers/gpu/nova-core/gsp/hal/tu102.rs b/drivers/gpu/nova-core/gsp/hal/tu102.rs
index 87ceb8878f01..f78e2489f5a6 100644
--- a/drivers/gpu/nova-core/gsp/hal/tu102.rs
+++ b/drivers/gpu/nova-core/gsp/hal/tu102.rs
@@ -6,7 +6,8 @@
use kernel::{
device,
dma::Coherent,
- io::Io, //
+ io::Io,
+ types::ScopeGuard, //
};
use crate::{
@@ -32,7 +33,6 @@
},
gpu::Chipset,
gsp::{
- boot::BootUnloadGuard,
hal::{
GspHal,
UnloadBundle, //
@@ -259,13 +259,13 @@ fn run_fwsec_frts(
struct Tu102;
impl GspHal for Tu102 {
- fn boot<'a>(
+ fn boot(
&self,
- gsp: &'a Gsp,
- ctx: &GspBootContext<'a>,
+ gsp: &Gsp,
+ ctx: &GspBootContext<'_>,
fb_layout: &FbLayout,
wpr_meta: &Coherent<GspFwWprMeta>,
- ) -> Result<BootUnloadGuard<'a>> {
+ ) -> Result<Option<crate::gsp::UnloadBundle>> {
let dev = ctx.dev();
let bar = ctx.bar;
let chipset = ctx.chipset;
@@ -290,9 +290,12 @@ fn boot<'a>(
.ok()
.map(crate::gsp::UnloadBundle);
- // Wrap the unload bundle into a drop guard so it is automatically run upon failure.
- let unload_guard =
- BootUnloadGuard::new(gsp, dev, bar, gsp_falcon, sec2_falcon, unload_bundle);
+ // Run the unload bundle to try and recover the GSP if an error occurs.
+ let unload_guard = ScopeGuard::new_with_data(unload_bundle, |unload_bundle| {
+ if let Some(unload_bundle) = unload_bundle {
+ let _ = unload_bundle.0.run(dev, bar, gsp_falcon, sec2_falcon);
+ }
+ });
// FWSEC-FRTS is not executed on chips where the FRTS region size is 0 (e.g. GA100).
if !fb_layout.frts.is_empty() {
@@ -319,7 +322,7 @@ fn boot<'a>(
)?
.run(dev, sec2_falcon, wpr_meta)?;
- Ok(unload_guard)
+ Ok(unload_guard.dismiss())
}
fn post_boot(&self, gsp: &Gsp, ctx: &GspBootContext<'_>, gsp_fw: &GspFirmware) -> Result {
--
2.55.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 03/13] gpu: nova-core: gsp: replace BootUnloadGuard with local handlers
2026-07-07 7:21 ` [PATCH v5 03/13] gpu: nova-core: gsp: replace BootUnloadGuard with local handlers Alexandre Courbot
@ 2026-07-07 8:04 ` Eliot Courtney
2026-07-07 12:56 ` Alexandre Courbot
0 siblings, 1 reply; 20+ messages in thread
From: Eliot Courtney @ 2026-07-07 8:04 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, David Airlie,
Simona Vetter, Gary Guo, John Hubbard, Alistair Popple,
Timur Tabi, Eliot Courtney, Zhi Wang
Cc: nova-gpu, dri-devel, linux-kernel, rust-for-linux, dri-devel
On Tue Jul 7, 2026 at 4:21 PM JST, Alexandre Courbot wrote:
> When adding the GSP unload capability, we introduced `BootUnloadGuard`
> to automatically call `Gsp::unload` whenever an error occurred during
> the boot process, in order to try to reset the GSP to a valid state.
>
> This approach is not well-suited to the errors that may occur in HALs:
> by definition, an error occurring in the HAL means that the GSP is not
> booted; yet the first thing that `Gsp::unload` does is queue a shutdown
> message to the GSP, which will inevitably result in a timeout when done
> from a HAL.
>
> Furthermore, `BootUnloadGuard` is problematic because it holds
> additional references to the boot context, notably the `Falcon`s. These
> extra references stand in the way of making some of the `Falcon`'s
> methods mutable, since those methods would require exclusive access. As
> this behavior is only needed in one place, introducing dedicated types
> for it is distracting and unnecessary.
>
> Thus, remove `BootUnloadGuard` and adopt a two-level error handling
> strategy:
>
> - HALs are free to handle their errors as they see fit (most likely, by
> running their unload bundle if it is ready by the time of the error),
> - `Gsp::boot` uses a `ScopeGuard` that runs `Gsp::unload`, since the
> GSP should be up and running by the time `GspHal::boot` has returned.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> drivers/gpu/nova-core/gsp/boot.rs | 67 +++-------------------------------
> drivers/gpu/nova-core/gsp/hal.rs | 13 +++----
> drivers/gpu/nova-core/gsp/hal/gh100.rs | 31 ++++++++++------
> drivers/gpu/nova-core/gsp/hal/tu102.rs | 23 +++++++-----
> 4 files changed, 44 insertions(+), 90 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
> index ab0491b57944..536f2e341c01 100644
> --- a/drivers/gpu/nova-core/gsp/boot.rs
> +++ b/drivers/gpu/nova-core/gsp/boot.rs
> @@ -30,66 +30,6 @@
> },
> };
>
> -/// Arguments required to call [`Gsp::unload`](super::Gsp::unload).
> -///
> -/// Stored as their own type to avoid repeating a long and tedious list in [`BootUnloadGuard`].
> -pub(super) struct BootUnloadArgs<'a> {
> - gsp: &'a super::Gsp,
> - dev: &'a device::Device<device::Bound>,
> - bar: Bar0<'a>,
> - gsp_falcon: &'a Falcon<'a, Gsp>,
> - sec2_falcon: &'a Falcon<'a, Sec2>,
> - unload_bundle: Option<super::UnloadBundle>,
> -}
> -
> -/// Guard that calls [`Gsp::unload`](super::Gsp::unload) with a
> -/// [`UnloadBundle`](super::UnloadBundle) when dropped.
> -///
> -/// Used to ensure the `UnloadBundle` is run during failure paths.
> -pub(super) struct BootUnloadGuard<'a> {
> - guard: ScopeGuard<BootUnloadArgs<'a>, fn(BootUnloadArgs<'a>)>,
> -}
> -
> -impl<'a> BootUnloadGuard<'a> {
> - /// Wraps `unload_bundle` into a guard that executes it when dropped.
> - pub(super) fn new(
> - gsp: &'a super::Gsp,
> - dev: &'a device::Device<device::Bound>,
> - bar: Bar0<'a>,
> - gsp_falcon: &'a Falcon<'a, Gsp>,
> - sec2_falcon: &'a Falcon<'a, Sec2>,
> - unload_bundle: Option<super::UnloadBundle>,
> - ) -> Self {
> - Self {
> - guard: ScopeGuard::new_with_data(
> - BootUnloadArgs {
> - gsp,
> - dev,
> - bar,
> - gsp_falcon,
> - sec2_falcon,
> - unload_bundle,
> - },
> - |args| {
> - let _ = super::Gsp::unload(
> - args.gsp,
> - args.dev,
> - args.bar,
> - args.gsp_falcon,
> - args.sec2_falcon,
> - args.unload_bundle,
> - );
> - },
> - ),
> - }
> - }
> -
> - /// Disarms the guard and returns the [`UnloadBundle`](super::UnloadBundle) it contains.
> - pub(super) fn dismiss(self) -> Option<super::UnloadBundle> {
> - self.guard.dismiss().unload_bundle
> - }
> -}
> -
> impl super::Gsp {
> /// Attempt to boot the GSP.
> ///
> @@ -107,6 +47,7 @@ pub(crate) fn boot(
> let bar = ctx.bar;
> let chipset = ctx.chipset;
> let gsp_falcon = ctx.gsp_falcon;
> + let sec2_falcon = ctx.sec2_falcon;
> let dev = pdev.as_ref();
> let hal = super::hal::gsp_hal(chipset);
>
> @@ -118,7 +59,11 @@ pub(crate) fn boot(
> let wpr_meta = Coherent::init(dev, GFP_KERNEL, GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
>
> // Perform the chipset-specific boot sequence, and retrieve the unload bundle.
> - let unload_guard = hal.boot(&self, &ctx, &fb_layout, &wpr_meta)?;
> + let unload_bundle = hal.boot(&self, &ctx, &fb_layout, &wpr_meta)?;
> +
> + let unload_guard = ScopeGuard::new_with_data(unload_bundle, |unload_bundle| {
> + let _ = self.unload(dev, bar, gsp_falcon, sec2_falcon, unload_bundle);
> + });
>
> gsp_falcon.write_os_version(gsp_fw.bootloader.app_version);
>
> diff --git a/drivers/gpu/nova-core/gsp/hal.rs b/drivers/gpu/nova-core/gsp/hal.rs
> index d3e47ef206de..851d1f24c137 100644
> --- a/drivers/gpu/nova-core/gsp/hal.rs
> +++ b/drivers/gpu/nova-core/gsp/hal.rs
> @@ -24,7 +24,6 @@
> Chipset, //
> },
> gsp::{
> - boot::BootUnloadGuard,
> Gsp,
> GspBootContext,
> GspFwWprMeta, //
> @@ -51,15 +50,15 @@ fn run(
> pub(super) trait GspHal: Send {
> /// Performs the GSP boot process, loading and running the required firmwares as needed.
> ///
> - /// Upon success, returns a guard that runs the GSP unload sequence if GSP boot does not
> - /// complete.
> - fn boot<'a>(
> + /// Upon success, returns the [`crate::gsp::UnloadBundle`] to use with [`Gsp::unload`], if one
> + /// could be created.
> + fn boot(
> &self,
> - gsp: &'a Gsp,
> - ctx: &GspBootContext<'a>,
> + gsp: &Gsp,
> + ctx: &GspBootContext<'_>,
> fb_layout: &FbLayout,
> wpr_meta: &Coherent<GspFwWprMeta>,
> - ) -> Result<BootUnloadGuard<'a>>;
> + ) -> Result<Option<crate::gsp::UnloadBundle>>;
>
> /// Performs HAL-specific post-GSP boot tasks.
> ///
> diff --git a/drivers/gpu/nova-core/gsp/hal/gh100.rs b/drivers/gpu/nova-core/gsp/hal/gh100.rs
> index 1d06405a32f6..18c889f9f413 100644
> --- a/drivers/gpu/nova-core/gsp/hal/gh100.rs
> +++ b/drivers/gpu/nova-core/gsp/hal/gh100.rs
> @@ -23,7 +23,6 @@
> Fsp, //
> },
> gsp::{
> - boot::BootUnloadGuard,
> hal::{
> GspHal,
> UnloadBundle, //
> @@ -143,13 +142,13 @@ impl GspHal for Gh100 {
> ///
> /// This path uses FSP to establish a chain of trust and boot GSP-FMC. FSP handles
> /// the GSP boot internally - no manual GSP reset/boot is needed.
> - fn boot<'a>(
> + fn boot(
> &self,
> - gsp: &'a Gsp,
> - ctx: &GspBootContext<'a>,
> + gsp: &Gsp,
> + ctx: &GspBootContext<'_>,
> fb_layout: &FbLayout,
> wpr_meta: &Coherent<GspFwWprMeta>,
> - ) -> Result<BootUnloadGuard<'a>> {
> + ) -> Result<Option<crate::gsp::UnloadBundle>> {
> let dev = ctx.dev();
> let bar = ctx.bar;
> let chipset = ctx.chipset;
> @@ -160,10 +159,6 @@ fn boot<'a>(
> KBox::new(FspUnloadBundle, GFP_KERNEL)? as KBox<dyn UnloadBundle>
> );
>
> - // Wrap the unload bundle into a drop guard so it is automatically run upon failure.
> - let unload_guard =
> - BootUnloadGuard::new(gsp, dev, bar, gsp_falcon, sec2_falcon, Some(unload_bundle));
> -
> let mut fsp = Fsp::wait_secure_boot(dev, bar, chipset)?;
>
> let args = FmcBootArgs::new(
> @@ -174,11 +169,23 @@ fn boot<'a>(
> false,
> )?;
>
> - fsp.boot_fmc(dev, fb_layout, &args)?;
> + // Keep the result as we want to wait for lockdown release even in case of error, to make
> + // sure `args` is not accessed by the GSP anymore.
> + let fsp_res = fsp.boot_fmc(dev, fb_layout, &args);
>
> - wait_for_gsp_lockdown_release(dev, gsp_falcon, args.boot_params_dma_handle())?;
> + // Wait for GSP-FMC to release the GSP lockdown, indicating that `args` is not accessed
> + // anymore.
> + let lockdown_res =
> + wait_for_gsp_lockdown_release(dev, gsp_falcon, args.boot_params_dma_handle());
>
> - Ok(unload_guard)
> + match fsp_res.and(lockdown_res) {
> + Ok(()) => Ok(Some(unload_bundle)),
> + Err(e) => {
> + // Wait for the GSP RISC-V core to halt in case of error.
> + let _ = unload_bundle.0.run(dev, bar, gsp_falcon, sec2_falcon);
> + Err(e)
> + }
> + }
IMO we should just ScopeGuard here too (after args creation) to run the
unload bundle. It will avoid manual manipulation of results which is
error prone (since a ? added later will break things) and also halt
happens-after gsp lockdown, so it's a natural sequence point. Also I am
not 100% sure but I am not fully convinced that
wait_for_gsp_lockdown_release will correctly detect all errors if they
occur.
With ^ change,
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
> }
> }
>
> diff --git a/drivers/gpu/nova-core/gsp/hal/tu102.rs b/drivers/gpu/nova-core/gsp/hal/tu102.rs
> index 87ceb8878f01..f78e2489f5a6 100644
> --- a/drivers/gpu/nova-core/gsp/hal/tu102.rs
> +++ b/drivers/gpu/nova-core/gsp/hal/tu102.rs
> @@ -6,7 +6,8 @@
> use kernel::{
> device,
> dma::Coherent,
> - io::Io, //
> + io::Io,
> + types::ScopeGuard, //
> };
>
> use crate::{
> @@ -32,7 +33,6 @@
> },
> gpu::Chipset,
> gsp::{
> - boot::BootUnloadGuard,
> hal::{
> GspHal,
> UnloadBundle, //
> @@ -259,13 +259,13 @@ fn run_fwsec_frts(
> struct Tu102;
>
> impl GspHal for Tu102 {
> - fn boot<'a>(
> + fn boot(
> &self,
> - gsp: &'a Gsp,
> - ctx: &GspBootContext<'a>,
> + gsp: &Gsp,
> + ctx: &GspBootContext<'_>,
> fb_layout: &FbLayout,
> wpr_meta: &Coherent<GspFwWprMeta>,
> - ) -> Result<BootUnloadGuard<'a>> {
> + ) -> Result<Option<crate::gsp::UnloadBundle>> {
> let dev = ctx.dev();
> let bar = ctx.bar;
> let chipset = ctx.chipset;
> @@ -290,9 +290,12 @@ fn boot<'a>(
> .ok()
> .map(crate::gsp::UnloadBundle);
>
> - // Wrap the unload bundle into a drop guard so it is automatically run upon failure.
> - let unload_guard =
> - BootUnloadGuard::new(gsp, dev, bar, gsp_falcon, sec2_falcon, unload_bundle);
> + // Run the unload bundle to try and recover the GSP if an error occurs.
> + let unload_guard = ScopeGuard::new_with_data(unload_bundle, |unload_bundle| {
> + if let Some(unload_bundle) = unload_bundle {
> + let _ = unload_bundle.0.run(dev, bar, gsp_falcon, sec2_falcon);
> + }
> + });
>
> // FWSEC-FRTS is not executed on chips where the FRTS region size is 0 (e.g. GA100).
> if !fb_layout.frts.is_empty() {
> @@ -319,7 +322,7 @@ fn boot<'a>(
> )?
> .run(dev, sec2_falcon, wpr_meta)?;
>
> - Ok(unload_guard)
> + Ok(unload_guard.dismiss())
> }
>
> fn post_boot(&self, gsp: &Gsp, ctx: &GspBootContext<'_>, gsp_fw: &GspFirmware) -> Result {
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 03/13] gpu: nova-core: gsp: replace BootUnloadGuard with local handlers
2026-07-07 8:04 ` Eliot Courtney
@ 2026-07-07 12:56 ` Alexandre Courbot
2026-07-07 13:34 ` Eliot Courtney
0 siblings, 1 reply; 20+ messages in thread
From: Alexandre Courbot @ 2026-07-07 12:56 UTC (permalink / raw)
To: Eliot Courtney
Cc: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Gary Guo, John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
nova-gpu, dri-devel, linux-kernel, rust-for-linux, dri-devel
On Tue Jul 7, 2026 at 5:04 PM JST, Eliot Courtney wrote:
> On Tue Jul 7, 2026 at 4:21 PM JST, Alexandre Courbot wrote:
>> When adding the GSP unload capability, we introduced `BootUnloadGuard`
>> to automatically call `Gsp::unload` whenever an error occurred during
>> the boot process, in order to try to reset the GSP to a valid state.
>>
>> This approach is not well-suited to the errors that may occur in HALs:
>> by definition, an error occurring in the HAL means that the GSP is not
>> booted; yet the first thing that `Gsp::unload` does is queue a shutdown
>> message to the GSP, which will inevitably result in a timeout when done
>> from a HAL.
>>
>> Furthermore, `BootUnloadGuard` is problematic because it holds
>> additional references to the boot context, notably the `Falcon`s. These
>> extra references stand in the way of making some of the `Falcon`'s
>> methods mutable, since those methods would require exclusive access. As
>> this behavior is only needed in one place, introducing dedicated types
>> for it is distracting and unnecessary.
>>
>> Thus, remove `BootUnloadGuard` and adopt a two-level error handling
>> strategy:
>>
>> - HALs are free to handle their errors as they see fit (most likely, by
>> running their unload bundle if it is ready by the time of the error),
>> - `Gsp::boot` uses a `ScopeGuard` that runs `Gsp::unload`, since the
>> GSP should be up and running by the time `GspHal::boot` has returned.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>> drivers/gpu/nova-core/gsp/boot.rs | 67 +++-------------------------------
>> drivers/gpu/nova-core/gsp/hal.rs | 13 +++----
>> drivers/gpu/nova-core/gsp/hal/gh100.rs | 31 ++++++++++------
>> drivers/gpu/nova-core/gsp/hal/tu102.rs | 23 +++++++-----
>> 4 files changed, 44 insertions(+), 90 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
>> index ab0491b57944..536f2e341c01 100644
>> --- a/drivers/gpu/nova-core/gsp/boot.rs
>> +++ b/drivers/gpu/nova-core/gsp/boot.rs
>> @@ -30,66 +30,6 @@
>> },
>> };
>>
>> -/// Arguments required to call [`Gsp::unload`](super::Gsp::unload).
>> -///
>> -/// Stored as their own type to avoid repeating a long and tedious list in [`BootUnloadGuard`].
>> -pub(super) struct BootUnloadArgs<'a> {
>> - gsp: &'a super::Gsp,
>> - dev: &'a device::Device<device::Bound>,
>> - bar: Bar0<'a>,
>> - gsp_falcon: &'a Falcon<'a, Gsp>,
>> - sec2_falcon: &'a Falcon<'a, Sec2>,
>> - unload_bundle: Option<super::UnloadBundle>,
>> -}
>> -
>> -/// Guard that calls [`Gsp::unload`](super::Gsp::unload) with a
>> -/// [`UnloadBundle`](super::UnloadBundle) when dropped.
>> -///
>> -/// Used to ensure the `UnloadBundle` is run during failure paths.
>> -pub(super) struct BootUnloadGuard<'a> {
>> - guard: ScopeGuard<BootUnloadArgs<'a>, fn(BootUnloadArgs<'a>)>,
>> -}
>> -
>> -impl<'a> BootUnloadGuard<'a> {
>> - /// Wraps `unload_bundle` into a guard that executes it when dropped.
>> - pub(super) fn new(
>> - gsp: &'a super::Gsp,
>> - dev: &'a device::Device<device::Bound>,
>> - bar: Bar0<'a>,
>> - gsp_falcon: &'a Falcon<'a, Gsp>,
>> - sec2_falcon: &'a Falcon<'a, Sec2>,
>> - unload_bundle: Option<super::UnloadBundle>,
>> - ) -> Self {
>> - Self {
>> - guard: ScopeGuard::new_with_data(
>> - BootUnloadArgs {
>> - gsp,
>> - dev,
>> - bar,
>> - gsp_falcon,
>> - sec2_falcon,
>> - unload_bundle,
>> - },
>> - |args| {
>> - let _ = super::Gsp::unload(
>> - args.gsp,
>> - args.dev,
>> - args.bar,
>> - args.gsp_falcon,
>> - args.sec2_falcon,
>> - args.unload_bundle,
>> - );
>> - },
>> - ),
>> - }
>> - }
>> -
>> - /// Disarms the guard and returns the [`UnloadBundle`](super::UnloadBundle) it contains.
>> - pub(super) fn dismiss(self) -> Option<super::UnloadBundle> {
>> - self.guard.dismiss().unload_bundle
>> - }
>> -}
>> -
>> impl super::Gsp {
>> /// Attempt to boot the GSP.
>> ///
>> @@ -107,6 +47,7 @@ pub(crate) fn boot(
>> let bar = ctx.bar;
>> let chipset = ctx.chipset;
>> let gsp_falcon = ctx.gsp_falcon;
>> + let sec2_falcon = ctx.sec2_falcon;
>> let dev = pdev.as_ref();
>> let hal = super::hal::gsp_hal(chipset);
>>
>> @@ -118,7 +59,11 @@ pub(crate) fn boot(
>> let wpr_meta = Coherent::init(dev, GFP_KERNEL, GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
>>
>> // Perform the chipset-specific boot sequence, and retrieve the unload bundle.
>> - let unload_guard = hal.boot(&self, &ctx, &fb_layout, &wpr_meta)?;
>> + let unload_bundle = hal.boot(&self, &ctx, &fb_layout, &wpr_meta)?;
>> +
>> + let unload_guard = ScopeGuard::new_with_data(unload_bundle, |unload_bundle| {
>> + let _ = self.unload(dev, bar, gsp_falcon, sec2_falcon, unload_bundle);
>> + });
>>
>> gsp_falcon.write_os_version(gsp_fw.bootloader.app_version);
>>
>> diff --git a/drivers/gpu/nova-core/gsp/hal.rs b/drivers/gpu/nova-core/gsp/hal.rs
>> index d3e47ef206de..851d1f24c137 100644
>> --- a/drivers/gpu/nova-core/gsp/hal.rs
>> +++ b/drivers/gpu/nova-core/gsp/hal.rs
>> @@ -24,7 +24,6 @@
>> Chipset, //
>> },
>> gsp::{
>> - boot::BootUnloadGuard,
>> Gsp,
>> GspBootContext,
>> GspFwWprMeta, //
>> @@ -51,15 +50,15 @@ fn run(
>> pub(super) trait GspHal: Send {
>> /// Performs the GSP boot process, loading and running the required firmwares as needed.
>> ///
>> - /// Upon success, returns a guard that runs the GSP unload sequence if GSP boot does not
>> - /// complete.
>> - fn boot<'a>(
>> + /// Upon success, returns the [`crate::gsp::UnloadBundle`] to use with [`Gsp::unload`], if one
>> + /// could be created.
>> + fn boot(
>> &self,
>> - gsp: &'a Gsp,
>> - ctx: &GspBootContext<'a>,
>> + gsp: &Gsp,
>> + ctx: &GspBootContext<'_>,
>> fb_layout: &FbLayout,
>> wpr_meta: &Coherent<GspFwWprMeta>,
>> - ) -> Result<BootUnloadGuard<'a>>;
>> + ) -> Result<Option<crate::gsp::UnloadBundle>>;
>>
>> /// Performs HAL-specific post-GSP boot tasks.
>> ///
>> diff --git a/drivers/gpu/nova-core/gsp/hal/gh100.rs b/drivers/gpu/nova-core/gsp/hal/gh100.rs
>> index 1d06405a32f6..18c889f9f413 100644
>> --- a/drivers/gpu/nova-core/gsp/hal/gh100.rs
>> +++ b/drivers/gpu/nova-core/gsp/hal/gh100.rs
>> @@ -23,7 +23,6 @@
>> Fsp, //
>> },
>> gsp::{
>> - boot::BootUnloadGuard,
>> hal::{
>> GspHal,
>> UnloadBundle, //
>> @@ -143,13 +142,13 @@ impl GspHal for Gh100 {
>> ///
>> /// This path uses FSP to establish a chain of trust and boot GSP-FMC. FSP handles
>> /// the GSP boot internally - no manual GSP reset/boot is needed.
>> - fn boot<'a>(
>> + fn boot(
>> &self,
>> - gsp: &'a Gsp,
>> - ctx: &GspBootContext<'a>,
>> + gsp: &Gsp,
>> + ctx: &GspBootContext<'_>,
>> fb_layout: &FbLayout,
>> wpr_meta: &Coherent<GspFwWprMeta>,
>> - ) -> Result<BootUnloadGuard<'a>> {
>> + ) -> Result<Option<crate::gsp::UnloadBundle>> {
>> let dev = ctx.dev();
>> let bar = ctx.bar;
>> let chipset = ctx.chipset;
>> @@ -160,10 +159,6 @@ fn boot<'a>(
>> KBox::new(FspUnloadBundle, GFP_KERNEL)? as KBox<dyn UnloadBundle>
>> );
>>
>> - // Wrap the unload bundle into a drop guard so it is automatically run upon failure.
>> - let unload_guard =
>> - BootUnloadGuard::new(gsp, dev, bar, gsp_falcon, sec2_falcon, Some(unload_bundle));
>> -
>> let mut fsp = Fsp::wait_secure_boot(dev, bar, chipset)?;
>>
>> let args = FmcBootArgs::new(
>> @@ -174,11 +169,23 @@ fn boot<'a>(
>> false,
>> )?;
>>
>> - fsp.boot_fmc(dev, fb_layout, &args)?;
>> + // Keep the result as we want to wait for lockdown release even in case of error, to make
>> + // sure `args` is not accessed by the GSP anymore.
>> + let fsp_res = fsp.boot_fmc(dev, fb_layout, &args);
>>
>> - wait_for_gsp_lockdown_release(dev, gsp_falcon, args.boot_params_dma_handle())?;
>> + // Wait for GSP-FMC to release the GSP lockdown, indicating that `args` is not accessed
>> + // anymore.
>> + let lockdown_res =
>> + wait_for_gsp_lockdown_release(dev, gsp_falcon, args.boot_params_dma_handle());
>>
>> - Ok(unload_guard)
>> + match fsp_res.and(lockdown_res) {
>> + Ok(()) => Ok(Some(unload_bundle)),
>> + Err(e) => {
>> + // Wait for the GSP RISC-V core to halt in case of error.
>> + let _ = unload_bundle.0.run(dev, bar, gsp_falcon, sec2_falcon);
>> + Err(e)
>> + }
>> + }
>
> IMO we should just ScopeGuard here too (after args creation) to run the
> unload bundle. It will avoid manual manipulation of results which is
> error prone (since a ? added later will break things) and also halt
> happens-after gsp lockdown, so it's a natural sequence point. Also I am
ScopedGuard here is reasonable - do you mean only for running the unload
bundle, or would you also duplicate the lockdown release there so we can
avoid using `and`?
> not 100% sure but I am not fully convinced that
> wait_for_gsp_lockdown_release will correctly detect all errors if they
> occur.
The failure path is a best effort. I don't think we can guarantee
anything as it doesn't appear to have been designed with recovery in mind.
>
> With ^ change,
> Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
Thanks!
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 03/13] gpu: nova-core: gsp: replace BootUnloadGuard with local handlers
2026-07-07 12:56 ` Alexandre Courbot
@ 2026-07-07 13:34 ` Eliot Courtney
0 siblings, 0 replies; 20+ messages in thread
From: Eliot Courtney @ 2026-07-07 13:34 UTC (permalink / raw)
To: Alexandre Courbot, Eliot Courtney
Cc: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Gary Guo, John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
nova-gpu, dri-devel, linux-kernel, rust-for-linux, dri-devel
On Tue Jul 7, 2026 at 9:56 PM JST, Alexandre Courbot wrote:
> On Tue Jul 7, 2026 at 5:04 PM JST, Eliot Courtney wrote:
>> On Tue Jul 7, 2026 at 4:21 PM JST, Alexandre Courbot wrote:
>>> When adding the GSP unload capability, we introduced `BootUnloadGuard`
>>> to automatically call `Gsp::unload` whenever an error occurred during
>>> the boot process, in order to try to reset the GSP to a valid state.
>>>
>>> This approach is not well-suited to the errors that may occur in HALs:
>>> by definition, an error occurring in the HAL means that the GSP is not
>>> booted; yet the first thing that `Gsp::unload` does is queue a shutdown
>>> message to the GSP, which will inevitably result in a timeout when done
>>> from a HAL.
>>>
>>> Furthermore, `BootUnloadGuard` is problematic because it holds
>>> additional references to the boot context, notably the `Falcon`s. These
>>> extra references stand in the way of making some of the `Falcon`'s
>>> methods mutable, since those methods would require exclusive access. As
>>> this behavior is only needed in one place, introducing dedicated types
>>> for it is distracting and unnecessary.
>>>
>>> Thus, remove `BootUnloadGuard` and adopt a two-level error handling
>>> strategy:
>>>
>>> - HALs are free to handle their errors as they see fit (most likely, by
>>> running their unload bundle if it is ready by the time of the error),
>>> - `Gsp::boot` uses a `ScopeGuard` that runs `Gsp::unload`, since the
>>> GSP should be up and running by the time `GspHal::boot` has returned.
>>>
>>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>>> ---
>>> drivers/gpu/nova-core/gsp/boot.rs | 67 +++-------------------------------
>>> drivers/gpu/nova-core/gsp/hal.rs | 13 +++----
>>> drivers/gpu/nova-core/gsp/hal/gh100.rs | 31 ++++++++++------
>>> drivers/gpu/nova-core/gsp/hal/tu102.rs | 23 +++++++-----
>>> 4 files changed, 44 insertions(+), 90 deletions(-)
>>>
>>> diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
>>> index ab0491b57944..536f2e341c01 100644
>>> --- a/drivers/gpu/nova-core/gsp/boot.rs
>>> +++ b/drivers/gpu/nova-core/gsp/boot.rs
>>> @@ -30,66 +30,6 @@
>>> },
>>> };
>>>
>>> -/// Arguments required to call [`Gsp::unload`](super::Gsp::unload).
>>> -///
>>> -/// Stored as their own type to avoid repeating a long and tedious list in [`BootUnloadGuard`].
>>> -pub(super) struct BootUnloadArgs<'a> {
>>> - gsp: &'a super::Gsp,
>>> - dev: &'a device::Device<device::Bound>,
>>> - bar: Bar0<'a>,
>>> - gsp_falcon: &'a Falcon<'a, Gsp>,
>>> - sec2_falcon: &'a Falcon<'a, Sec2>,
>>> - unload_bundle: Option<super::UnloadBundle>,
>>> -}
>>> -
>>> -/// Guard that calls [`Gsp::unload`](super::Gsp::unload) with a
>>> -/// [`UnloadBundle`](super::UnloadBundle) when dropped.
>>> -///
>>> -/// Used to ensure the `UnloadBundle` is run during failure paths.
>>> -pub(super) struct BootUnloadGuard<'a> {
>>> - guard: ScopeGuard<BootUnloadArgs<'a>, fn(BootUnloadArgs<'a>)>,
>>> -}
>>> -
>>> -impl<'a> BootUnloadGuard<'a> {
>>> - /// Wraps `unload_bundle` into a guard that executes it when dropped.
>>> - pub(super) fn new(
>>> - gsp: &'a super::Gsp,
>>> - dev: &'a device::Device<device::Bound>,
>>> - bar: Bar0<'a>,
>>> - gsp_falcon: &'a Falcon<'a, Gsp>,
>>> - sec2_falcon: &'a Falcon<'a, Sec2>,
>>> - unload_bundle: Option<super::UnloadBundle>,
>>> - ) -> Self {
>>> - Self {
>>> - guard: ScopeGuard::new_with_data(
>>> - BootUnloadArgs {
>>> - gsp,
>>> - dev,
>>> - bar,
>>> - gsp_falcon,
>>> - sec2_falcon,
>>> - unload_bundle,
>>> - },
>>> - |args| {
>>> - let _ = super::Gsp::unload(
>>> - args.gsp,
>>> - args.dev,
>>> - args.bar,
>>> - args.gsp_falcon,
>>> - args.sec2_falcon,
>>> - args.unload_bundle,
>>> - );
>>> - },
>>> - ),
>>> - }
>>> - }
>>> -
>>> - /// Disarms the guard and returns the [`UnloadBundle`](super::UnloadBundle) it contains.
>>> - pub(super) fn dismiss(self) -> Option<super::UnloadBundle> {
>>> - self.guard.dismiss().unload_bundle
>>> - }
>>> -}
>>> -
>>> impl super::Gsp {
>>> /// Attempt to boot the GSP.
>>> ///
>>> @@ -107,6 +47,7 @@ pub(crate) fn boot(
>>> let bar = ctx.bar;
>>> let chipset = ctx.chipset;
>>> let gsp_falcon = ctx.gsp_falcon;
>>> + let sec2_falcon = ctx.sec2_falcon;
>>> let dev = pdev.as_ref();
>>> let hal = super::hal::gsp_hal(chipset);
>>>
>>> @@ -118,7 +59,11 @@ pub(crate) fn boot(
>>> let wpr_meta = Coherent::init(dev, GFP_KERNEL, GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
>>>
>>> // Perform the chipset-specific boot sequence, and retrieve the unload bundle.
>>> - let unload_guard = hal.boot(&self, &ctx, &fb_layout, &wpr_meta)?;
>>> + let unload_bundle = hal.boot(&self, &ctx, &fb_layout, &wpr_meta)?;
>>> +
>>> + let unload_guard = ScopeGuard::new_with_data(unload_bundle, |unload_bundle| {
>>> + let _ = self.unload(dev, bar, gsp_falcon, sec2_falcon, unload_bundle);
>>> + });
>>>
>>> gsp_falcon.write_os_version(gsp_fw.bootloader.app_version);
>>>
>>> diff --git a/drivers/gpu/nova-core/gsp/hal.rs b/drivers/gpu/nova-core/gsp/hal.rs
>>> index d3e47ef206de..851d1f24c137 100644
>>> --- a/drivers/gpu/nova-core/gsp/hal.rs
>>> +++ b/drivers/gpu/nova-core/gsp/hal.rs
>>> @@ -24,7 +24,6 @@
>>> Chipset, //
>>> },
>>> gsp::{
>>> - boot::BootUnloadGuard,
>>> Gsp,
>>> GspBootContext,
>>> GspFwWprMeta, //
>>> @@ -51,15 +50,15 @@ fn run(
>>> pub(super) trait GspHal: Send {
>>> /// Performs the GSP boot process, loading and running the required firmwares as needed.
>>> ///
>>> - /// Upon success, returns a guard that runs the GSP unload sequence if GSP boot does not
>>> - /// complete.
>>> - fn boot<'a>(
>>> + /// Upon success, returns the [`crate::gsp::UnloadBundle`] to use with [`Gsp::unload`], if one
>>> + /// could be created.
>>> + fn boot(
>>> &self,
>>> - gsp: &'a Gsp,
>>> - ctx: &GspBootContext<'a>,
>>> + gsp: &Gsp,
>>> + ctx: &GspBootContext<'_>,
>>> fb_layout: &FbLayout,
>>> wpr_meta: &Coherent<GspFwWprMeta>,
>>> - ) -> Result<BootUnloadGuard<'a>>;
>>> + ) -> Result<Option<crate::gsp::UnloadBundle>>;
>>>
>>> /// Performs HAL-specific post-GSP boot tasks.
>>> ///
>>> diff --git a/drivers/gpu/nova-core/gsp/hal/gh100.rs b/drivers/gpu/nova-core/gsp/hal/gh100.rs
>>> index 1d06405a32f6..18c889f9f413 100644
>>> --- a/drivers/gpu/nova-core/gsp/hal/gh100.rs
>>> +++ b/drivers/gpu/nova-core/gsp/hal/gh100.rs
>>> @@ -23,7 +23,6 @@
>>> Fsp, //
>>> },
>>> gsp::{
>>> - boot::BootUnloadGuard,
>>> hal::{
>>> GspHal,
>>> UnloadBundle, //
>>> @@ -143,13 +142,13 @@ impl GspHal for Gh100 {
>>> ///
>>> /// This path uses FSP to establish a chain of trust and boot GSP-FMC. FSP handles
>>> /// the GSP boot internally - no manual GSP reset/boot is needed.
>>> - fn boot<'a>(
>>> + fn boot(
>>> &self,
>>> - gsp: &'a Gsp,
>>> - ctx: &GspBootContext<'a>,
>>> + gsp: &Gsp,
>>> + ctx: &GspBootContext<'_>,
>>> fb_layout: &FbLayout,
>>> wpr_meta: &Coherent<GspFwWprMeta>,
>>> - ) -> Result<BootUnloadGuard<'a>> {
>>> + ) -> Result<Option<crate::gsp::UnloadBundle>> {
>>> let dev = ctx.dev();
>>> let bar = ctx.bar;
>>> let chipset = ctx.chipset;
>>> @@ -160,10 +159,6 @@ fn boot<'a>(
>>> KBox::new(FspUnloadBundle, GFP_KERNEL)? as KBox<dyn UnloadBundle>
>>> );
>>>
>>> - // Wrap the unload bundle into a drop guard so it is automatically run upon failure.
>>> - let unload_guard =
>>> - BootUnloadGuard::new(gsp, dev, bar, gsp_falcon, sec2_falcon, Some(unload_bundle));
>>> -
>>> let mut fsp = Fsp::wait_secure_boot(dev, bar, chipset)?;
>>>
>>> let args = FmcBootArgs::new(
>>> @@ -174,11 +169,23 @@ fn boot<'a>(
>>> false,
>>> )?;
>>>
>>> - fsp.boot_fmc(dev, fb_layout, &args)?;
>>> + // Keep the result as we want to wait for lockdown release even in case of error, to make
>>> + // sure `args` is not accessed by the GSP anymore.
>>> + let fsp_res = fsp.boot_fmc(dev, fb_layout, &args);
>>>
>>> - wait_for_gsp_lockdown_release(dev, gsp_falcon, args.boot_params_dma_handle())?;
>>> + // Wait for GSP-FMC to release the GSP lockdown, indicating that `args` is not accessed
>>> + // anymore.
>>> + let lockdown_res =
>>> + wait_for_gsp_lockdown_release(dev, gsp_falcon, args.boot_params_dma_handle());
>>>
>>> - Ok(unload_guard)
>>> + match fsp_res.and(lockdown_res) {
>>> + Ok(()) => Ok(Some(unload_bundle)),
>>> + Err(e) => {
>>> + // Wait for the GSP RISC-V core to halt in case of error.
>>> + let _ = unload_bundle.0.run(dev, bar, gsp_falcon, sec2_falcon);
>>> + Err(e)
>>> + }
>>> + }
>>
>> IMO we should just ScopeGuard here too (after args creation) to run the
>> unload bundle. It will avoid manual manipulation of results which is
>> error prone (since a ? added later will break things) and also halt
>> happens-after gsp lockdown, so it's a natural sequence point. Also I am
>
> ScopedGuard here is reasonable - do you mean only for running the unload
> bundle, or would you also duplicate the lockdown release there so we can
> avoid using `and`?
I would skip the lockdown release path on the error path since AFAICT in
every case halt happens, we would have also gotten
`wait_for_gsp_lockdown_release` to return if we had executed it. So it
doesn't buy us any additional waiting/sequencing, and we still need to
wait for halt anyway. So concretely I think something like this is
simpler and as robust (conceptually to me it also seems odd to wait for
"gsp lockdown release" when boot has failed):
```
diff --git a/drivers/gpu/nova-core/gsp/hal/gh100.rs b/drivers/gpu/nova-core/gsp/hal/gh100.rs
--- a/drivers/gpu/nova-core/gsp/hal/gh100.rs
+++ b/drivers/gpu/nova-core/gsp/hal/gh100.rs
@@
use kernel::{
device,
dma::Coherent,
io::poll::read_poll_timeout,
- time::Delta, //
+ time::Delta,
+ types::ScopeGuard, //
};
@@
let args = FmcBootArgs::new(
dev,
chipset,
wpr_meta.dma_handle(),
gsp.libos.dma_handle(),
false,
)?;
- // Keep the result as we want to wait for lockdown release even in case of error, to make
- // sure `args` is not accessed by the GSP anymore.
- let fsp_res = fsp.boot_fmc(dev, fb_layout, &args);
+ // Wait for the GSP RISC-V core to halt in case of error. We create this guard after `args`
+ // to make sure that boot args are kept alive until halt, in case they are still being
+ // accessed.
+ let unload_guard = ScopeGuard::new_with_data(unload_bundle, |unload_bundle| {
+ let _ = unload_bundle.0.run(dev, bar, gsp_falcon, sec2_falcon);
+ });
+
+ fsp.boot_fmc(dev, fb_layout, &args)?;
// Wait for GSP-FMC to release the GSP lockdown, indicating that `args` is not accessed
// anymore.
- let lockdown_res =
- wait_for_gsp_lockdown_release(dev, gsp_falcon, args.boot_params_dma_handle());
+ wait_for_gsp_lockdown_release(dev, gsp_falcon, args.boot_params_dma_handle())?;
- match fsp_res.and(lockdown_res) {
- Ok(()) => Ok(Some(unload_bundle)),
- Err(e) => {
- // Wait for the GSP RISC-V core to halt in case of error.
- let _ = unload_bundle.0.run(dev, bar, gsp_falcon, sec2_falcon);
- Err(e)
- }
- }
+ Ok(Some(unload_guard.dismiss()))
}
}
```
WDYT?
>
>> not 100% sure but I am not fully convinced that
>> wait_for_gsp_lockdown_release will correctly detect all errors if they
>> occur.
>
> The failure path is a best effort. I don't think we can guarantee
> anything as it doesn't appear to have been designed with recovery in mind.
Yeah I agree, what I meant is that I think that I think that the wait
until halt is a better signal that we are definitely done than the
lockdown release one, and it's easier to just only wait for halt in the
error case than to also manipulate results to wait for the lockdown
release path too.
>
>>
>> With ^ change,
>> Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
>
> Thanks!
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 04/13] gpu: nova-core: gsp: pass GspBootContext to unload methods
2026-07-07 7:21 [PATCH v5 00/13] gpu: nova-core: consolidate and streamline GSP boot process Alexandre Courbot
` (2 preceding siblings ...)
2026-07-07 7:21 ` [PATCH v5 03/13] gpu: nova-core: gsp: replace BootUnloadGuard with local handlers Alexandre Courbot
@ 2026-07-07 7:21 ` Alexandre Courbot
2026-07-07 7:21 ` [PATCH v5 05/13] gpu: nova-core: gsp: centralize missing unload bundle warnings Alexandre Courbot
` (8 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-07-07 7:21 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Gary Guo, John Hubbard, Alistair Popple, Timur Tabi,
Eliot Courtney, Zhi Wang
Cc: nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
`GspBootContext` contains the resources required to boot the GSP. As it
turns out, this is also the context required for unloading it.
Reflect that fact by replacing the arguments of `Gsp::unload` and
`UnloadBundle::run` with the `GspBootContext`. This symmetry between
`Gsp::boot` and `Gsp::unload` will also be convenient when we want to
make these methods generic over the boot context corresponding to the
boot method used.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/gpu.rs | 19 ++++++++++++++++---
drivers/gpu/nova-core/gsp/boot.rs | 28 +++++++++++++---------------
drivers/gpu/nova-core/gsp/hal.rs | 15 +--------------
drivers/gpu/nova-core/gsp/hal/gh100.rs | 17 ++++-------------
drivers/gpu/nova-core/gsp/hal/tu102.rs | 23 +++++++++++------------
5 files changed, 45 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 43c3f4f8df71..32bfa0be2357 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -269,7 +269,9 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
#[pin_data(PinnedDrop)]
struct GspResources<'gpu> {
/// Device owning the GPU.
- device: &'gpu device::Device<device::Bound>,
+ device: &'gpu pci::Device<device::Bound>,
+ /// Details about the chipset.
+ spec: Spec,
/// MMIO mapping of PCI BAR 0.
bar: Bar0<'gpu>,
/// GSP falcon instance, used for GSP boot up and cleanup.
@@ -312,7 +314,16 @@ fn drop(self: Pin<&mut Self>) {
.gsp
.as_ref()
.get_ref()
- .unload(device, bar, &*this.gsp_falcon, &*this.sec2_falcon, bundle)
+ .unload(
+ GspBootContext {
+ pdev: device,
+ bar,
+ chipset: this.spec.chipset,
+ gsp_falcon: &*this.gsp_falcon,
+ sec2_falcon: &*this.sec2_falcon,
+ },
+ bundle,
+ )
.inspect_err(|e| dev_err!(device, "failed to unload GSP: {:?}\n", e));
}
}
@@ -344,7 +355,9 @@ pub(crate) fn new(
sysmem_flush: SysmemFlush::register(pdev.as_ref(), bar, spec.chipset)?,
gsp_resources <- try_pin_init!(GspResources {
- device: pdev.as_ref(),
+ device: pdev,
+
+ spec: *spec,
bar,
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 536f2e341c01..75054424a951 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -3,7 +3,6 @@
use kernel::{
bits,
- device,
dma::Coherent,
io::poll::read_poll_timeout,
prelude::*,
@@ -15,7 +14,6 @@
driver::Bar0,
falcon::{
gsp::Gsp,
- sec2::Sec2,
Falcon, //
},
fb::FbLayout,
@@ -47,7 +45,6 @@ pub(crate) fn boot(
let bar = ctx.bar;
let chipset = ctx.chipset;
let gsp_falcon = ctx.gsp_falcon;
- let sec2_falcon = ctx.sec2_falcon;
let dev = pdev.as_ref();
let hal = super::hal::gsp_hal(chipset);
@@ -61,9 +58,11 @@ pub(crate) fn boot(
// Perform the chipset-specific boot sequence, and retrieve the unload bundle.
let unload_bundle = hal.boot(&self, &ctx, &fb_layout, &wpr_meta)?;
- let unload_guard = ScopeGuard::new_with_data(unload_bundle, |unload_bundle| {
- let _ = self.unload(dev, bar, gsp_falcon, sec2_falcon, unload_bundle);
- });
+ let unload_guard =
+ ScopeGuard::new_with_data((ctx, unload_bundle), |(ctx, unload_bundle)| {
+ let _ = self.unload(ctx, unload_bundle);
+ });
+ let ctx = &unload_guard.0;
gsp_falcon.write_os_version(gsp_fw.bootloader.app_version);
@@ -82,12 +81,12 @@ pub(crate) fn boot(
self.cmdq
.send_command_no_wait(bar, commands::SetRegistry::new())?;
- hal.post_boot(&self, &ctx, &gsp_fw)?;
+ hal.post_boot(&self, ctx, &gsp_fw)?;
// Wait until GSP is fully initialized.
commands::wait_gsp_init_done(&self.cmdq)?;
- Ok(unload_guard.dismiss())
+ Ok(unload_guard.dismiss().1)
}
/// Shut down the GSP and wait until it is offline.
@@ -116,17 +115,16 @@ fn shutdown_gsp(
/// This stops all activity on the GSP.
pub(crate) fn unload(
&self,
- dev: &device::Device<device::Bound>,
- bar: Bar0<'_>,
- gsp_falcon: &Falcon<'_, Gsp>,
- sec2_falcon: &Falcon<'_, Sec2>,
+ ctx: super::GspBootContext<'_>,
unload_bundle: Option<super::UnloadBundle>,
) -> Result {
+ let dev = ctx.dev();
+
// Shut down the GSP. Keep going even in case of error.
let mut res = Self::shutdown_gsp(
&self.cmdq,
- bar,
- gsp_falcon,
+ ctx.bar,
+ ctx.gsp_falcon,
commands::PowerStateLevel::Level0,
)
.inspect_err(|e| dev_err!(dev, "GSP shutdown failed: {:?}\n", e));
@@ -136,7 +134,7 @@ pub(crate) fn unload(
res = res.and(
unload_bundle
.0
- .run(dev, bar, gsp_falcon, sec2_falcon)
+ .run(&ctx)
.inspect_err(|e| dev_err!(dev, "Unload bundle failed: {:?}\n", e)),
);
} else {
diff --git a/drivers/gpu/nova-core/gsp/hal.rs b/drivers/gpu/nova-core/gsp/hal.rs
index 851d1f24c137..849ca224085b 100644
--- a/drivers/gpu/nova-core/gsp/hal.rs
+++ b/drivers/gpu/nova-core/gsp/hal.rs
@@ -5,18 +5,11 @@
mod tu102;
use kernel::{
- device,
dma::Coherent,
prelude::*, //
};
use crate::{
- driver::Bar0,
- falcon::{
- gsp::Gsp as GspEngine,
- sec2::Sec2,
- Falcon, //
- },
fb::FbLayout,
firmware::gsp::GspFirmware,
gpu::{
@@ -37,13 +30,7 @@
/// required for unloading is prepared at load time, and stored here until it needs to be run.
pub(super) trait UnloadBundle: Send {
/// Performs the steps required to properly reset the GSP after it has been stopped.
- fn run(
- &self,
- dev: &device::Device<device::Bound>,
- bar: Bar0<'_>,
- gsp_falcon: &Falcon<'_, GspEngine>,
- sec2_falcon: &Falcon<'_, Sec2>,
- ) -> Result;
+ fn run(&self, ctx: &GspBootContext<'_>) -> Result;
}
/// Trait implemented by GSP HALs.
diff --git a/drivers/gpu/nova-core/gsp/hal/gh100.rs b/drivers/gpu/nova-core/gsp/hal/gh100.rs
index 18c889f9f413..aabef45202f7 100644
--- a/drivers/gpu/nova-core/gsp/hal/gh100.rs
+++ b/drivers/gpu/nova-core/gsp/hal/gh100.rs
@@ -11,10 +11,8 @@
};
use crate::{
- driver::Bar0,
falcon::{
gsp::Gsp as GspEngine,
- sec2::Sec2,
Falcon, //
},
fb::FbLayout,
@@ -116,22 +114,16 @@ fn wait_for_gsp_lockdown_release(
struct FspUnloadBundle;
impl UnloadBundle for FspUnloadBundle {
- fn run(
- &self,
- dev: &device::Device<device::Bound>,
- _bar: Bar0<'_>,
- gsp_falcon: &Falcon<'_, GspEngine>,
- _sec2_falcon: &Falcon<'_, Sec2>,
- ) -> Result {
+ fn run(&self, ctx: &GspBootContext<'_>) -> Result {
// GSP falcon does most of the work of resetting, so just wait for it to finish.
read_poll_timeout(
- || Ok(gsp_falcon.is_riscv_active()),
+ || Ok(ctx.gsp_falcon.is_riscv_active()),
|&active| !active,
Delta::from_millis(10),
Delta::from_secs(5),
)
.map(|_| ())
- .inspect_err(|_| dev_err!(dev, "GSP falcon failed to halt\n"))
+ .inspect_err(|_| dev_err!(ctx.dev(), "GSP falcon failed to halt\n"))
}
}
@@ -153,7 +145,6 @@ fn boot(
let bar = ctx.bar;
let chipset = ctx.chipset;
let gsp_falcon = ctx.gsp_falcon;
- let sec2_falcon = ctx.sec2_falcon;
let unload_bundle = crate::gsp::UnloadBundle(
KBox::new(FspUnloadBundle, GFP_KERNEL)? as KBox<dyn UnloadBundle>
@@ -182,7 +173,7 @@ fn boot(
Ok(()) => Ok(Some(unload_bundle)),
Err(e) => {
// Wait for the GSP RISC-V core to halt in case of error.
- let _ = unload_bundle.0.run(dev, bar, gsp_falcon, sec2_falcon);
+ let _ = unload_bundle.0.run(ctx);
Err(e)
}
}
diff --git a/drivers/gpu/nova-core/gsp/hal/tu102.rs b/drivers/gpu/nova-core/gsp/hal/tu102.rs
index f78e2489f5a6..c0956fb1c9cf 100644
--- a/drivers/gpu/nova-core/gsp/hal/tu102.rs
+++ b/drivers/gpu/nova-core/gsp/hal/tu102.rs
@@ -121,18 +121,15 @@ fn build(
}
impl UnloadBundle for Sec2UnloadBundle {
- fn run(
- &self,
- dev: &device::Device<device::Bound>,
- bar: Bar0<'_>,
- gsp_falcon: &Falcon<'_, GspEngine>,
- sec2_falcon: &Falcon<'_, Sec2>,
- ) -> Result {
+ fn run(&self, ctx: &GspBootContext<'_>) -> Result {
+ let dev = ctx.dev();
+ let bar = ctx.bar;
+
// Run FWSEC-SB to reset the GSP falcon to its pre-libos state.
// Log errors but keep going if it fails.
let fwsec_sb_res = self
.fwsec_sb
- .run(dev, bar, gsp_falcon)
+ .run(dev, bar, ctx.gsp_falcon)
.inspect_err(|e| dev_err!(dev, "FWSEC-SB failed to run: {:?}\n", e));
// Remove WPR2 region if set.
@@ -142,12 +139,14 @@ fn run(
return Ok(());
}
- sec2_falcon.reset()?;
- sec2_falcon.load(&self.booter_unloader)?;
+ ctx.sec2_falcon.reset()?;
+ ctx.sec2_falcon.load(&self.booter_unloader)?;
// Sentinel value to confirm that Booter Unloader has run.
const MAILBOX_SENTINEL: u32 = 0xff;
- let (mbox0, _) = sec2_falcon.boot(Some(MAILBOX_SENTINEL), Some(MAILBOX_SENTINEL))?;
+ let (mbox0, _) = ctx
+ .sec2_falcon
+ .boot(Some(MAILBOX_SENTINEL), Some(MAILBOX_SENTINEL))?;
if mbox0 != 0 {
dev_err!(dev, "Booter Unloader returned error 0x{:x}\n", mbox0);
return Err(EINVAL);
@@ -293,7 +292,7 @@ fn boot(
// Run the unload bundle to try and recover the GSP if an error occurs.
let unload_guard = ScopeGuard::new_with_data(unload_bundle, |unload_bundle| {
if let Some(unload_bundle) = unload_bundle {
- let _ = unload_bundle.0.run(dev, bar, gsp_falcon, sec2_falcon);
+ let _ = unload_bundle.0.run(ctx);
}
});
--
2.55.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v5 05/13] gpu: nova-core: gsp: centralize missing unload bundle warnings
2026-07-07 7:21 [PATCH v5 00/13] gpu: nova-core: consolidate and streamline GSP boot process Alexandre Courbot
` (3 preceding siblings ...)
2026-07-07 7:21 ` [PATCH v5 04/13] gpu: nova-core: gsp: pass GspBootContext to unload methods Alexandre Courbot
@ 2026-07-07 7:21 ` Alexandre Courbot
2026-07-07 7:21 ` [PATCH v5 06/13] gpu: nova-core: gsp: fold TU102 unload bundle construction into HAL method Alexandre Courbot
` (7 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-07-07 7:21 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Gary Guo, John Hubbard, Alistair Popple, Timur Tabi,
Eliot Courtney, Zhi Wang
Cc: nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
The warning emitted when the unload bundle cannot be constructed is
valid regardless of the boot method, but it was local to `Tu102`. Move
it to `Gsp::boot` so it applies to all boot methods.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/gsp/boot.rs | 10 +++++++++-
drivers/gpu/nova-core/gsp/hal/tu102.rs | 9 +--------
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 75054424a951..430ffc57b136 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -56,7 +56,15 @@ pub(crate) fn boot(
let wpr_meta = Coherent::init(dev, GFP_KERNEL, GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
// Perform the chipset-specific boot sequence, and retrieve the unload bundle.
- let unload_bundle = hal.boot(&self, &ctx, &fb_layout, &wpr_meta)?;
+ let unload_bundle = hal.boot(&self, &ctx, &fb_layout, &wpr_meta)?.or_else(|| {
+ dev_warn!(dev, "The GSP won't be able to unload properly on unbind.\n");
+ dev_warn!(
+ dev,
+ "The GPU will need to be reset before the driver can bind again.\n"
+ );
+
+ None
+ });
let unload_guard =
ScopeGuard::new_with_data((ctx, unload_bundle), |(ctx, unload_bundle)| {
diff --git a/drivers/gpu/nova-core/gsp/hal/tu102.rs b/drivers/gpu/nova-core/gsp/hal/tu102.rs
index c0956fb1c9cf..ef465b99af05 100644
--- a/drivers/gpu/nova-core/gsp/hal/tu102.rs
+++ b/drivers/gpu/nova-core/gsp/hal/tu102.rs
@@ -278,14 +278,7 @@ fn boot(
// If the unload bundle creation fails, the GPU will need to be reset before the driver can
// be probed again.
let unload_bundle = Sec2UnloadBundle::build(dev, chipset, &bios, gsp_falcon, sec2_falcon)
- .inspect_err(|e| {
- dev_warn!(dev, "Failed to prepare unload firmware: {:?}\n", e);
- dev_warn!(dev, "The GSP won't be able to unload properly on unbind.\n");
- dev_warn!(
- dev,
- "The GPU will need to be reset before the driver can bind again.\n"
- );
- })
+ .inspect_err(|e| dev_warn!(dev, "Failed to prepare unload firmware: {:?}\n", e))
.ok()
.map(crate::gsp::UnloadBundle);
--
2.55.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v5 06/13] gpu: nova-core: gsp: fold TU102 unload bundle construction into HAL method
2026-07-07 7:21 [PATCH v5 00/13] gpu: nova-core: consolidate and streamline GSP boot process Alexandre Courbot
` (4 preceding siblings ...)
2026-07-07 7:21 ` [PATCH v5 05/13] gpu: nova-core: gsp: centralize missing unload bundle warnings Alexandre Courbot
@ 2026-07-07 7:21 ` Alexandre Courbot
2026-07-07 7:21 ` [PATCH v5 07/13] gpu: nova-core: gsp: turn FWSEC execution " Alexandre Courbot
` (6 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-07-07 7:21 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Gary Guo, John Hubbard, Alistair Popple, Timur Tabi,
Eliot Courtney, Zhi Wang
Cc: nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
The construction of the unload bundle is currently a bit convoluted and
could be done in one function instead of two.
Additionally, turn that function into a method of `Tu102`. A following
patch will turn the "use FWSEC bootloader" property into a flag of the
TU102 HAL itself, and making this a method will allow the code to access
it instead of querying `Chipset`.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/gsp/hal/tu102.rs | 85 ++++++++++++++++------------------
1 file changed, 39 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/hal/tu102.rs b/drivers/gpu/nova-core/gsp/hal/tu102.rs
index ef465b99af05..c8c7a4f45809 100644
--- a/drivers/gpu/nova-core/gsp/hal/tu102.rs
+++ b/drivers/gpu/nova-core/gsp/hal/tu102.rs
@@ -56,22 +56,6 @@ enum FwsecUnloadFirmware {
}
impl FwsecUnloadFirmware {
- /// Loads the FWSEC SB firmware, as well as its bootloader if `chipset` requires it.
- fn new(
- dev: &device::Device<device::Bound>,
- chipset: Chipset,
- bios: &Vbios,
- gsp_falcon: &Falcon<'_, GspEngine>,
- ) -> Result<Self> {
- let fwsec_sb = FwsecFirmware::new(dev, gsp_falcon, bios, FwsecCommand::Sb)?;
-
- Ok(if chipset.needs_fwsec_bootloader() {
- Self::WithBl(FwsecFirmwareWithBl::new(fwsec_sb, dev, chipset)?)
- } else {
- Self::WithoutBl(fwsec_sb)
- })
- }
-
/// Runs the FWSEC SB firmware.
fn run(
&self,
@@ -93,33 +77,6 @@ struct Sec2UnloadBundle {
booter_unloader: BooterFirmware,
}
-impl Sec2UnloadBundle {
- /// Load and prepare the resources required to properly reset the GSP after it has been stopped.
- fn build(
- dev: &device::Device<device::Bound>,
- chipset: Chipset,
- bios: &Vbios,
- gsp_falcon: &Falcon<'_, GspEngine>,
- sec2_falcon: &Falcon<'_, Sec2>,
- ) -> Result<KBox<dyn UnloadBundle>> {
- KBox::new(
- Self {
- fwsec_sb: FwsecUnloadFirmware::new(dev, chipset, bios, gsp_falcon)?,
- booter_unloader: BooterFirmware::new(
- dev,
- BooterKind::Unloader,
- chipset,
- FIRMWARE_VERSION,
- sec2_falcon,
- )?,
- },
- GFP_KERNEL,
- )
- .map(|b| b as KBox<dyn UnloadBundle>)
- .map_err(Into::into)
- }
-}
-
impl UnloadBundle for Sec2UnloadBundle {
fn run(&self, ctx: &GspBootContext<'_>) -> Result {
let dev = ctx.dev();
@@ -257,6 +214,42 @@ fn run_fwsec_frts(
struct Tu102;
+impl Tu102 {
+ /// Load and prepare the resources required to properly reset the GSP after it has been stopped.
+ fn build_unload_bundle(
+ &self,
+ dev: &device::Device<device::Bound>,
+ chipset: Chipset,
+ bios: &Vbios,
+ gsp_falcon: &Falcon<'_, GspEngine>,
+ sec2_falcon: &Falcon<'_, Sec2>,
+ ) -> Result<crate::gsp::UnloadBundle> {
+ // Load the FWSEC SB firmware, as well as its bootloader if required.
+ let fwsec_sb = FwsecFirmware::new(dev, gsp_falcon, bios, FwsecCommand::Sb)?;
+ let fwsec_sb = if chipset.needs_fwsec_bootloader() {
+ FwsecUnloadFirmware::WithBl(FwsecFirmwareWithBl::new(fwsec_sb, dev, chipset)?)
+ } else {
+ FwsecUnloadFirmware::WithoutBl(fwsec_sb)
+ };
+
+ KBox::new(
+ Sec2UnloadBundle {
+ fwsec_sb,
+ booter_unloader: BooterFirmware::new(
+ dev,
+ BooterKind::Unloader,
+ chipset,
+ FIRMWARE_VERSION,
+ sec2_falcon,
+ )?,
+ },
+ GFP_KERNEL,
+ )
+ .map(|b| crate::gsp::UnloadBundle(b))
+ .map_err(Into::into)
+ }
+}
+
impl GspHal for Tu102 {
fn boot(
&self,
@@ -277,10 +270,10 @@ fn boot(
//
// If the unload bundle creation fails, the GPU will need to be reset before the driver can
// be probed again.
- let unload_bundle = Sec2UnloadBundle::build(dev, chipset, &bios, gsp_falcon, sec2_falcon)
+ let unload_bundle = self
+ .build_unload_bundle(dev, chipset, &bios, gsp_falcon, sec2_falcon)
.inspect_err(|e| dev_warn!(dev, "Failed to prepare unload firmware: {:?}\n", e))
- .ok()
- .map(crate::gsp::UnloadBundle);
+ .ok();
// Run the unload bundle to try and recover the GSP if an error occurs.
let unload_guard = ScopeGuard::new_with_data(unload_bundle, |unload_bundle| {
--
2.55.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v5 07/13] gpu: nova-core: gsp: turn FWSEC execution into HAL method
2026-07-07 7:21 [PATCH v5 00/13] gpu: nova-core: consolidate and streamline GSP boot process Alexandre Courbot
` (5 preceding siblings ...)
2026-07-07 7:21 ` [PATCH v5 06/13] gpu: nova-core: gsp: fold TU102 unload bundle construction into HAL method Alexandre Courbot
@ 2026-07-07 7:21 ` Alexandre Courbot
2026-07-07 7:21 ` [PATCH v5 08/13] gpu: nova-core: gsp: make use of FWSEC bootloader a property of the TU102 HAL Alexandre Courbot
` (5 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-07-07 7:21 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Gary Guo, John Hubbard, Alistair Popple, Timur Tabi,
Eliot Courtney, Zhi Wang
Cc: nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
Turn the `run_fwsec_frts` function into a method of `Tu102`. A following
patch will turn the "use FWSEC bootloader" property into a flag of the
TU102 HAL itself, and making this a method will allow the code to access
it instead of querying `Chipset`.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/gsp/hal/tu102.rs | 173 +++++++++++++++++----------------
1 file changed, 87 insertions(+), 86 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/hal/tu102.rs b/drivers/gpu/nova-core/gsp/hal/tu102.rs
index c8c7a4f45809..066f97c9908a 100644
--- a/drivers/gpu/nova-core/gsp/hal/tu102.rs
+++ b/drivers/gpu/nova-core/gsp/hal/tu102.rs
@@ -127,94 +127,95 @@ fn run(&self, ctx: &GspBootContext<'_>) -> Result {
}
}
-/// Helper function to load and run the FWSEC-FRTS firmware and confirm that it has properly
-/// created the WPR2 region.
-fn run_fwsec_frts(
- dev: &device::Device<device::Bound>,
- chipset: Chipset,
- falcon: &Falcon<'_, GspEngine>,
- bar: Bar0<'_>,
- bios: &Vbios,
- fb_layout: &FbLayout,
-) -> Result {
- // Check that the WPR2 region does not already exist - if it does, we cannot run
- // FWSEC-FRTS until the GPU is reset.
- if bar.read(regs::NV_PFB_PRI_MMU_WPR2_ADDR_HI).higher_bound() != 0 {
- dev_err!(
- dev,
- "WPR2 region already exists - GPU needs to be reset to proceed\n"
- );
- return Err(EBUSY);
- }
-
- // FWSEC-FRTS will create the WPR2 region.
- let fwsec_frts = FwsecFirmware::new(
- dev,
- falcon,
- bios,
- FwsecCommand::Frts {
- frts_addr: fb_layout.frts.start,
- frts_size: fb_layout.frts.len(),
- },
- )?;
-
- if chipset.needs_fwsec_bootloader() {
- let fwsec_frts_bl = FwsecFirmwareWithBl::new(fwsec_frts, dev, chipset)?;
- // Load and run the bootloader, which will load FWSEC-FRTS and run it.
- fwsec_frts_bl.run(dev, falcon, bar)?;
- } else {
- // Load and run FWSEC-FRTS directly.
- fwsec_frts.run(dev, falcon)?;
- }
-
- // SCRATCH_E contains the error code for FWSEC-FRTS.
- let frts_status = bar
- .read(regs::NV_PBUS_SW_SCRATCH_0E_FRTS_ERR)
- .frts_err_code();
- if frts_status != 0 {
- dev_err!(
- dev,
- "FWSEC-FRTS returned with error code {:#x}\n",
- frts_status
- );
-
- return Err(EIO);
- }
-
- // Check that the WPR2 region has been created as we requested.
- let (wpr2_lo, wpr2_hi) = (
- bar.read(regs::NV_PFB_PRI_MMU_WPR2_ADDR_LO).lower_bound(),
- bar.read(regs::NV_PFB_PRI_MMU_WPR2_ADDR_HI).higher_bound(),
- );
-
- match (wpr2_lo, wpr2_hi) {
- (_, 0) => {
- dev_err!(dev, "WPR2 region not created after running FWSEC-FRTS\n");
-
- Err(EIO)
- }
- (wpr2_lo, _) if wpr2_lo != fb_layout.frts.start => {
- dev_err!(
- dev,
- "WPR2 region created at unexpected address {:#x}; expected {:#x}\n",
- wpr2_lo,
- fb_layout.frts.start,
- );
-
- Err(EIO)
- }
- (wpr2_lo, wpr2_hi) => {
- dev_dbg!(dev, "WPR2: {:#x}-{:#x}\n", wpr2_lo, wpr2_hi);
- dev_dbg!(dev, "GPU instance built\n");
-
- Ok(())
- }
- }
-}
-
struct Tu102;
impl Tu102 {
+ /// Helper method to load and run the FWSEC-FRTS firmware and confirm that it has properly
+ /// created the WPR2 region.
+ fn run_fwsec_frts(
+ &self,
+ dev: &device::Device<device::Bound>,
+ chipset: Chipset,
+ falcon: &Falcon<'_, GspEngine>,
+ bar: Bar0<'_>,
+ bios: &Vbios,
+ fb_layout: &FbLayout,
+ ) -> Result {
+ // Check that the WPR2 region does not already exist - if it does, we cannot run
+ // FWSEC-FRTS until the GPU is reset.
+ if bar.read(regs::NV_PFB_PRI_MMU_WPR2_ADDR_HI).higher_bound() != 0 {
+ dev_err!(
+ dev,
+ "WPR2 region already exists - GPU needs to be reset to proceed\n"
+ );
+ return Err(EBUSY);
+ }
+
+ // FWSEC-FRTS will create the WPR2 region.
+ let fwsec_frts = FwsecFirmware::new(
+ dev,
+ falcon,
+ bios,
+ FwsecCommand::Frts {
+ frts_addr: fb_layout.frts.start,
+ frts_size: fb_layout.frts.len(),
+ },
+ )?;
+
+ if chipset.needs_fwsec_bootloader() {
+ let fwsec_frts_bl = FwsecFirmwareWithBl::new(fwsec_frts, dev, chipset)?;
+ // Load and run the bootloader, which will load FWSEC-FRTS and run it.
+ fwsec_frts_bl.run(dev, falcon, bar)?;
+ } else {
+ // Load and run FWSEC-FRTS directly.
+ fwsec_frts.run(dev, falcon)?;
+ }
+
+ // SCRATCH_E contains the error code for FWSEC-FRTS.
+ let frts_status = bar
+ .read(regs::NV_PBUS_SW_SCRATCH_0E_FRTS_ERR)
+ .frts_err_code();
+ if frts_status != 0 {
+ dev_err!(
+ dev,
+ "FWSEC-FRTS returned with error code {:#x}\n",
+ frts_status
+ );
+
+ return Err(EIO);
+ }
+
+ // Check that the WPR2 region has been created as we requested.
+ let (wpr2_lo, wpr2_hi) = (
+ bar.read(regs::NV_PFB_PRI_MMU_WPR2_ADDR_LO).lower_bound(),
+ bar.read(regs::NV_PFB_PRI_MMU_WPR2_ADDR_HI).higher_bound(),
+ );
+
+ match (wpr2_lo, wpr2_hi) {
+ (_, 0) => {
+ dev_err!(dev, "WPR2 region not created after running FWSEC-FRTS\n");
+
+ Err(EIO)
+ }
+ (wpr2_lo, _) if wpr2_lo != fb_layout.frts.start => {
+ dev_err!(
+ dev,
+ "WPR2 region created at unexpected address {:#x}; expected {:#x}\n",
+ wpr2_lo,
+ fb_layout.frts.start,
+ );
+
+ Err(EIO)
+ }
+ (wpr2_lo, wpr2_hi) => {
+ dev_dbg!(dev, "WPR2: {:#x}-{:#x}\n", wpr2_lo, wpr2_hi);
+ dev_dbg!(dev, "GPU instance built\n");
+
+ Ok(())
+ }
+ }
+ }
+
/// Load and prepare the resources required to properly reset the GSP after it has been stopped.
fn build_unload_bundle(
&self,
@@ -284,7 +285,7 @@ fn boot(
// FWSEC-FRTS is not executed on chips where the FRTS region size is 0 (e.g. GA100).
if !fb_layout.frts.is_empty() {
- run_fwsec_frts(dev, chipset, gsp_falcon, bar, &bios, fb_layout)?;
+ self.run_fwsec_frts(dev, chipset, gsp_falcon, bar, &bios, fb_layout)?;
}
gsp_falcon.reset()?;
--
2.55.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v5 08/13] gpu: nova-core: gsp: make use of FWSEC bootloader a property of the TU102 HAL
2026-07-07 7:21 [PATCH v5 00/13] gpu: nova-core: consolidate and streamline GSP boot process Alexandre Courbot
` (6 preceding siblings ...)
2026-07-07 7:21 ` [PATCH v5 07/13] gpu: nova-core: gsp: turn FWSEC execution " Alexandre Courbot
@ 2026-07-07 7:21 ` Alexandre Courbot
2026-07-07 7:42 ` Eliot Courtney
2026-07-07 7:21 ` [PATCH v5 09/13] gpu: nova-core: move GSP firmware files decision to GSP HAL Alexandre Courbot
` (4 subsequent siblings)
12 siblings, 1 reply; 20+ messages in thread
From: Alexandre Courbot @ 2026-07-07 7:21 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Gary Guo, John Hubbard, Alistair Popple, Timur Tabi,
Eliot Courtney, Zhi Wang
Cc: nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
By being in the TU102 HAL, we already know that the GSP boot method is
the SEC2 Booter, so the only variable is whether the FWSEC bootloader is
used or not. Since `Chipset` also includes the variants that boot FSP,
querying it for that information introduces a potential code path (a
chipset that boots via FSP) that the current code doesn't handle.
Turn the use of the FWSEC bootloader into a property of the `Tu102` HAL,
and give GA102+ chipsets their own instance with that property set to
`false`. This removes the invalid code path and the only use of
`Chipset` is now to load the correct firmware files.
This also removes some uses of the `Chipset::needs_fwsec_bootloader`
method and prepares the ground for removing it.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/gsp/hal.rs | 5 ++++-
drivers/gpu/nova-core/gsp/hal/ga102.rs | 14 ++++++++++++++
drivers/gpu/nova-core/gsp/hal/tu102.rs | 15 +++++++++++----
3 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/hal.rs b/drivers/gpu/nova-core/gsp/hal.rs
index 849ca224085b..eddb4e8bf510 100644
--- a/drivers/gpu/nova-core/gsp/hal.rs
+++ b/drivers/gpu/nova-core/gsp/hal.rs
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+mod ga102;
mod gh100;
mod tu102;
@@ -59,7 +60,9 @@ fn post_boot(&self, _gsp: &Gsp, _ctx: &GspBootContext<'_>, _gsp_fw: &GspFirmware
/// Returns the GSP HAL to be used for `chipset`.
pub(super) fn gsp_hal(chipset: Chipset) -> &'static dyn GspHal {
match chipset.arch() {
- Architecture::Turing | Architecture::Ampere | Architecture::Ada => tu102::TU102_HAL,
+ Architecture::Turing => tu102::TU102_HAL,
+ Architecture::Ampere if matches!(chipset, Chipset::GA100) => tu102::TU102_HAL,
+ Architecture::Ampere | Architecture::Ada => ga102::GA102_HAL,
Architecture::Hopper | Architecture::BlackwellGB10x | Architecture::BlackwellGB20x => {
gh100::GH100_HAL
}
diff --git a/drivers/gpu/nova-core/gsp/hal/ga102.rs b/drivers/gpu/nova-core/gsp/hal/ga102.rs
new file mode 100644
index 000000000000..ceb3eb39d138
--- /dev/null
+++ b/drivers/gpu/nova-core/gsp/hal/ga102.rs
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+
+use crate::gsp::hal::{
+ tu102::Tu102,
+ GspHal, //
+};
+
+/// The GA102 HAL is like the TU102 one, except it doesn't use the bootloader.
+const GA102: Tu102 = Tu102 {
+ needs_fwsec_bootloader: false,
+};
+
+pub(super) const GA102_HAL: &dyn GspHal = &GA102;
diff --git a/drivers/gpu/nova-core/gsp/hal/tu102.rs b/drivers/gpu/nova-core/gsp/hal/tu102.rs
index 066f97c9908a..8480e2eb456f 100644
--- a/drivers/gpu/nova-core/gsp/hal/tu102.rs
+++ b/drivers/gpu/nova-core/gsp/hal/tu102.rs
@@ -127,7 +127,10 @@ fn run(&self, ctx: &GspBootContext<'_>) -> Result {
}
}
-struct Tu102;
+pub(super) struct Tu102 {
+ /// If `true`, then the FWSEC-FRTS bootloader will be used to load the actual firmware.
+ pub(super) needs_fwsec_bootloader: bool,
+}
impl Tu102 {
/// Helper method to load and run the FWSEC-FRTS firmware and confirm that it has properly
@@ -162,7 +165,7 @@ fn run_fwsec_frts(
},
)?;
- if chipset.needs_fwsec_bootloader() {
+ if self.needs_fwsec_bootloader {
let fwsec_frts_bl = FwsecFirmwareWithBl::new(fwsec_frts, dev, chipset)?;
// Load and run the bootloader, which will load FWSEC-FRTS and run it.
fwsec_frts_bl.run(dev, falcon, bar)?;
@@ -227,7 +230,7 @@ fn build_unload_bundle(
) -> Result<crate::gsp::UnloadBundle> {
// Load the FWSEC SB firmware, as well as its bootloader if required.
let fwsec_sb = FwsecFirmware::new(dev, gsp_falcon, bios, FwsecCommand::Sb)?;
- let fwsec_sb = if chipset.needs_fwsec_bootloader() {
+ let fwsec_sb = if self.needs_fwsec_bootloader {
FwsecUnloadFirmware::WithBl(FwsecFirmwareWithBl::new(fwsec_sb, dev, chipset)?)
} else {
FwsecUnloadFirmware::WithoutBl(fwsec_sb)
@@ -323,5 +326,9 @@ fn post_boot(&self, gsp: &Gsp, ctx: &GspBootContext<'_>, gsp_fw: &GspFirmware) -
}
}
-const TU102: Tu102 = Tu102;
+/// The TU102 HAL requires the use of the FWSEC bootloader.
+const TU102: Tu102 = Tu102 {
+ needs_fwsec_bootloader: true,
+};
+
pub(super) const TU102_HAL: &dyn GspHal = &TU102;
--
2.55.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 08/13] gpu: nova-core: gsp: make use of FWSEC bootloader a property of the TU102 HAL
2026-07-07 7:21 ` [PATCH v5 08/13] gpu: nova-core: gsp: make use of FWSEC bootloader a property of the TU102 HAL Alexandre Courbot
@ 2026-07-07 7:42 ` Eliot Courtney
0 siblings, 0 replies; 20+ messages in thread
From: Eliot Courtney @ 2026-07-07 7:42 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, David Airlie,
Simona Vetter, Gary Guo, John Hubbard, Alistair Popple,
Timur Tabi, Eliot Courtney, Zhi Wang
Cc: nova-gpu, dri-devel, linux-kernel, rust-for-linux, dri-devel
On Tue Jul 7, 2026 at 4:21 PM JST, Alexandre Courbot wrote:
> By being in the TU102 HAL, we already know that the GSP boot method is
> the SEC2 Booter, so the only variable is whether the FWSEC bootloader is
> used or not. Since `Chipset` also includes the variants that boot FSP,
> querying it for that information introduces a potential code path (a
> chipset that boots via FSP) that the current code doesn't handle.
>
> Turn the use of the FWSEC bootloader into a property of the `Tu102` HAL,
> and give GA102+ chipsets their own instance with that property set to
> `false`. This removes the invalid code path and the only use of
> `Chipset` is now to load the correct firmware files.
>
> This also removes some uses of the `Chipset::needs_fwsec_bootloader`
> method and prepares the ground for removing it.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 09/13] gpu: nova-core: move GSP firmware files decision to GSP HAL
2026-07-07 7:21 [PATCH v5 00/13] gpu: nova-core: consolidate and streamline GSP boot process Alexandre Courbot
` (7 preceding siblings ...)
2026-07-07 7:21 ` [PATCH v5 08/13] gpu: nova-core: gsp: make use of FWSEC bootloader a property of the TU102 HAL Alexandre Courbot
@ 2026-07-07 7:21 ` Alexandre Courbot
2026-07-07 7:49 ` Alexandre Courbot
2026-07-07 7:21 ` [PATCH v5 10/13] gpu: nova-core: avoid repeated calls to pci::Device::as_ref Alexandre Courbot
` (3 subsequent siblings)
12 siblings, 1 reply; 20+ messages in thread
From: Alexandre Courbot @ 2026-07-07 7:21 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Gary Guo, John Hubbard, Alistair Popple, Timur Tabi,
Eliot Courtney, Zhi Wang
Cc: nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
From: Eliot Courtney <ecourtney@nvidia.com>
The files to give the `ModInfoBuilder` depend on the chipset; this is
HAL information, so move it there. Doing so lets us remove the
`uses_fsp` and `needs_fwsec_bootloader` ad-hoc methods of `Chipset`.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/firmware.rs | 25 +++++++++++--------------
drivers/gpu/nova-core/firmware/fwsec.rs | 5 ++---
drivers/gpu/nova-core/gpu.rs | 16 ----------------
drivers/gpu/nova-core/gsp.rs | 1 +
drivers/gpu/nova-core/gsp/hal.rs | 19 +++++++++++++++++++
5 files changed, 33 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index a94820a3b335..20eff987c5d6 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -21,6 +21,7 @@
FalconFirmware, //
},
gpu,
+ gsp::boot_firmware_files,
num::{
FromSafeCast,
IntoSafeCast, //
@@ -419,24 +420,20 @@ const fn make_entry_file(self, chipset: &str, fw: &str) -> Self {
const fn make_entry_chipset(self, chipset: gpu::Chipset) -> Self {
let name = chipset.name();
- let this = self
+ // GSP firmware files are always present.
+ let mut this = self
.make_entry_file(name, "bootloader")
.make_entry_file(name, "gsp");
- // FSP-based chipsets (Hopper, Blackwell and later) boot the GSP via the FMC image loaded by
- // FSP. Older chipsets use the SEC2 booter instead.
- let this = if chipset.uses_fsp() {
- this.make_entry_file(name, "fmc")
- } else {
- this.make_entry_file(name, "booter_load")
- .make_entry_file(name, "booter_unload")
- };
-
- if chipset.needs_fwsec_bootloader() {
- this.make_entry_file(name, "gen_bootloader")
- } else {
- this
+ // Add the firmware files specific to the GSP boot method of `chipset`.
+ let boot_files = boot_firmware_files(chipset);
+ let mut i = 0;
+ while i < boot_files.len() {
+ this = this.make_entry_file(name, boot_files[i]);
+ i += 1;
}
+
+ this
}
pub(crate) const fn create(
diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
index 95e0dd77746b..7a931f22f629 100644
--- a/drivers/gpu/nova-core/firmware/fwsec.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec.rs
@@ -385,9 +385,8 @@ pub(crate) fn new(
/// Loads the FWSEC firmware into `falcon` and execute it.
///
- /// This must only be called on chipsets that do not need the FWSEC bootloader (i.e., where
- /// [`Chipset::needs_fwsec_bootloader()`](crate::gpu::Chipset::needs_fwsec_bootloader) returns
- /// `false`). On chipsets that do, use [`bootloader::FwsecFirmwareWithBl`] instead.
+ /// This must only be called on chipsets that do not need the FWSEC bootloader. On chipsets
+ /// where the bootloader is required, use [`bootloader::FwsecFirmwareWithBl`] instead.
pub(crate) fn run(&self, dev: &Device<device::Bound>, falcon: &Falcon<'_, Gsp>) -> Result<()> {
// Reset falcon, load the firmware, and run it.
falcon
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 32bfa0be2357..c04706b60ba8 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -133,22 +133,6 @@ pub(crate) const fn arch(self) -> Architecture {
}
}
- /// Returns `true` if this chipset requires the PIO-loaded bootloader in order to boot FWSEC.
- ///
- /// This includes all chipsets < GA102.
- pub(crate) const fn needs_fwsec_bootloader(self) -> bool {
- matches!(self.arch(), Architecture::Turing) || matches!(self, Self::GA100)
- }
-
- /// Returns `true` if this chipset boots via FSP (Hopper and later), which requires the FMC
- /// firmware image.
- pub(crate) const fn uses_fsp(self) -> bool {
- matches!(
- self.arch(),
- Architecture::Hopper | Architecture::BlackwellGB10x | Architecture::BlackwellGB20x
- )
- }
-
/// Returns the address range of the PCI config mirror space.
pub(crate) fn pci_config_mirror_range(self) -> Range<u32> {
hal::gpu_hal(self).pci_config_mirror_range()
diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index b4ac4156056e..d5cca3c8350a 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -30,6 +30,7 @@
GspFwWprMeta,
LibosParams, //
};
+pub(crate) use hal::boot_firmware_files;
use crate::{
driver::Bar0,
diff --git a/drivers/gpu/nova-core/gsp/hal.rs b/drivers/gpu/nova-core/gsp/hal.rs
index eddb4e8bf510..9da078dd1059 100644
--- a/drivers/gpu/nova-core/gsp/hal.rs
+++ b/drivers/gpu/nova-core/gsp/hal.rs
@@ -57,6 +57,25 @@ fn post_boot(&self, _gsp: &Gsp, _ctx: &GspBootContext<'_>, _gsp_fw: &GspFirmware
}
}
+/// Returns the names of the firmware files required to boot the GSP of `chipset`, in addition to
+/// the "bootloader" and "gsp" images required by all chipsets.
+pub(crate) const fn boot_firmware_files(chipset: Chipset) -> &'static [&'static str] {
+ match chipset.arch() {
+ // Turing chipsets boot the GSP via the SEC2 Booter, and require the FWSEC bootloader.
+ Architecture::Turing => &["booter_load", "booter_unload", "gen_bootloader"],
+ // GA100 also requires the FWSEC bootloader.
+ Architecture::Ampere if matches!(chipset, Chipset::GA100) => {
+ &["booter_load", "booter_unload", "gen_bootloader"]
+ }
+ // Other Ampere chipsets, as well as Ada chipsets, run FWSEC directly.
+ Architecture::Ampere | Architecture::Ada => &["booter_load", "booter_unload"],
+ // Hopper and later chipsets boot the GSP via the FMC image loaded by FSP.
+ Architecture::Hopper | Architecture::BlackwellGB10x | Architecture::BlackwellGB20x => {
+ &["fmc"]
+ }
+ }
+}
+
/// Returns the GSP HAL to be used for `chipset`.
pub(super) fn gsp_hal(chipset: Chipset) -> &'static dyn GspHal {
match chipset.arch() {
--
2.55.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v5 09/13] gpu: nova-core: move GSP firmware files decision to GSP HAL
2026-07-07 7:21 ` [PATCH v5 09/13] gpu: nova-core: move GSP firmware files decision to GSP HAL Alexandre Courbot
@ 2026-07-07 7:49 ` Alexandre Courbot
2026-07-07 13:08 ` Eliot Courtney
0 siblings, 1 reply; 20+ messages in thread
From: Alexandre Courbot @ 2026-07-07 7:49 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Gary Guo, John Hubbard, Alistair Popple, Timur Tabi,
Eliot Courtney, Zhi Wang
Cc: nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
On Tue Jul 7, 2026 at 4:21 PM JST, Alexandre Courbot wrote:
> From: Eliot Courtney <ecourtney@nvidia.com>
>
> The files to give the `ModInfoBuilder` depend on the chipset; this is
> HAL information, so move it there. Doing so lets us remove the
> `uses_fsp` and `needs_fwsec_bootloader` ad-hoc methods of `Chipset`.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Eliot, I put you as the author of this patch since it is basically the
diff you gave with very minor changes. If it looks ok to you, can you
give your Signed-off-by?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 09/13] gpu: nova-core: move GSP firmware files decision to GSP HAL
2026-07-07 7:49 ` Alexandre Courbot
@ 2026-07-07 13:08 ` Eliot Courtney
0 siblings, 0 replies; 20+ messages in thread
From: Eliot Courtney @ 2026-07-07 13:08 UTC (permalink / raw)
To: Alexandre Courbot, Danilo Krummrich, Alice Ryhl, David Airlie,
Simona Vetter, Gary Guo, John Hubbard, Alistair Popple,
Timur Tabi, Eliot Courtney, Zhi Wang
Cc: nova-gpu, dri-devel, linux-kernel, rust-for-linux, dri-devel
On Tue Jul 7, 2026 at 4:49 PM JST, Alexandre Courbot wrote:
> On Tue Jul 7, 2026 at 4:21 PM JST, Alexandre Courbot wrote:
>> From: Eliot Courtney <ecourtney@nvidia.com>
>>
>> The files to give the `ModInfoBuilder` depend on the chipset; this is
>> HAL information, so move it there. Doing so lets us remove the
>> `uses_fsp` and `needs_fwsec_bootloader` ad-hoc methods of `Chipset`.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>
> Eliot, I put you as the author of this patch since it is basically the
> diff you gave with very minor changes. If it looks ok to you, can you
> give your Signed-off-by?
Yes, lgtm. I guess I just paste this?:
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 10/13] gpu: nova-core: avoid repeated calls to pci::Device::as_ref
2026-07-07 7:21 [PATCH v5 00/13] gpu: nova-core: consolidate and streamline GSP boot process Alexandre Courbot
` (8 preceding siblings ...)
2026-07-07 7:21 ` [PATCH v5 09/13] gpu: nova-core: move GSP firmware files decision to GSP HAL Alexandre Courbot
@ 2026-07-07 7:21 ` Alexandre Courbot
2026-07-07 7:21 ` [PATCH v5 11/13] gpu: nova-core: gsp: pass GspBootContext mutably Alexandre Courbot
` (2 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-07-07 7:21 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Gary Guo, John Hubbard, Alistair Popple, Timur Tabi,
Eliot Courtney, Zhi Wang
Cc: nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
Add a local `Device` reference created from the `pci::Device` in the
`Gpu` constructor to avoid repeatedly calling `as_ref`.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/gpu.rs | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index c04706b60ba8..fc90069bc2fe 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -317,9 +317,11 @@ pub(crate) fn new(
pdev: &'gpu pci::Device<device::Core<'_>>,
bar: Bar0<'gpu>,
) -> impl PinInit<Self, Error> + 'gpu {
+ let dev = pdev.as_ref();
+
try_pin_init!(Self {
- spec: Spec::new(pdev.as_ref(), bar).inspect(|spec| {
- dev_info!(pdev,"NVIDIA ({})\n", spec);
+ spec: Spec::new(dev, bar).inspect(|spec| {
+ dev_info!(dev,"NVIDIA ({})\n", spec);
})?,
// We must wait for GFW_BOOT completion before doing any significant setup on the GPU.
@@ -332,11 +334,11 @@ pub(crate) fn new(
unsafe { pdev.dma_set_mask_and_coherent(dma_mask)? };
hal.wait_gfw_boot_completion(bar)
- .inspect_err(|_| dev_err!(pdev, "GFW boot did not complete\n"))?;
+ .inspect_err(|_| dev_err!(dev, "GFW boot did not complete\n"))?;
},
// Initialize this early because `gsp_resources` depends on it.
- sysmem_flush: SysmemFlush::register(pdev.as_ref(), bar, spec.chipset)?,
+ sysmem_flush: SysmemFlush::register(dev, bar, spec.chipset)?,
gsp_resources <- try_pin_init!(GspResources {
device: pdev,
@@ -346,13 +348,13 @@ pub(crate) fn new(
bar,
gsp_falcon: Falcon::new(
- pdev.as_ref(),
+ dev,
spec.chipset,
bar
)
.inspect(|falcon| falcon.clear_swgen0_intr())?,
- sec2_falcon: Falcon::new(pdev.as_ref(), spec.chipset, bar)?,
+ sec2_falcon: Falcon::new(dev, spec.chipset, bar)?,
gsp <- Gsp::new(pdev),
@@ -372,18 +374,18 @@ pub(crate) fn new(
// Obtain and display basic GPU information.
let info = gsp_resources.gsp.get_static_info(bar)?;
match info.gpu_name() {
- Ok(name) => dev_info!(pdev, "GPU name: {}\n", name),
- Err(e) => dev_warn!(pdev, "GPU name unavailable: {:?}\n", e),
+ Ok(name) => dev_info!(dev, "GPU name: {}\n", name),
+ Err(e) => dev_warn!(dev, "GPU name unavailable: {:?}\n", e),
}
if !info.usable_fb_regions.is_empty() {
- dev_dbg!(pdev, "Usable FB regions:\n");
+ dev_dbg!(dev, "Usable FB regions:\n");
for region in &info.usable_fb_regions {
- dev_dbg!(pdev, " - {:#x?}\n", region);
+ dev_dbg!(dev, " - {:#x?}\n", region);
}
dev_dbg!(
- pdev,
+ dev,
"Total usable VRAM: {} MiB\n",
info.usable_fb_regions.iter().fold(0u64, |res, region| res
.saturating_add(region.end - region.start))
--
2.55.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v5 11/13] gpu: nova-core: gsp: pass GspBootContext mutably
2026-07-07 7:21 [PATCH v5 00/13] gpu: nova-core: consolidate and streamline GSP boot process Alexandre Courbot
` (9 preceding siblings ...)
2026-07-07 7:21 ` [PATCH v5 10/13] gpu: nova-core: avoid repeated calls to pci::Device::as_ref Alexandre Courbot
@ 2026-07-07 7:21 ` Alexandre Courbot
2026-07-07 7:21 ` [PATCH v5 12/13] gpu: nova-core: gsp: separate context and GPU lifetimes in GspBootContext Alexandre Courbot
2026-07-07 7:21 ` [PATCH v5 13/13] gpu: nova-core: store Fsp instance in Gpu Alexandre Courbot
12 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-07-07 7:21 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Gary Guo, John Hubbard, Alistair Popple, Timur Tabi,
Eliot Courtney, Zhi Wang
Cc: nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
We want to move the `Fsp` instance into `Gpu`, which will require
passing it as a mutable reference in `GspBootContext`, since
`Fsp::boot_fmc` is a mutable method. In order to use the mutable
references it contains, `GspBootContext` must also be mutable.
We will also follow up by making some methods of the `Falcon`s mutable,
which also requires passing them as mutable references.
Thus, make the `GspBootContext` passed to `Gsp::boot` and `Gsp::unload`
mutable, and pass mutable references to it to the GSP boot HAL methods.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/gsp/boot.rs | 28 +++++++++++++++-------------
drivers/gpu/nova-core/gsp/hal.rs | 11 ++++++++---
drivers/gpu/nova-core/gsp/hal/gh100.rs | 4 ++--
drivers/gpu/nova-core/gsp/hal/tu102.rs | 6 +++---
4 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 430ffc57b136..f093e0215b66 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -39,7 +39,7 @@ impl super::Gsp {
/// [`Self::unload`]) returned.
pub(crate) fn boot(
self: Pin<&mut Self>,
- ctx: super::GspBootContext<'_>,
+ mut ctx: super::GspBootContext<'_>,
) -> Result<Option<super::UnloadBundle>> {
let pdev = ctx.pdev;
let bar = ctx.bar;
@@ -56,21 +56,23 @@ pub(crate) fn boot(
let wpr_meta = Coherent::init(dev, GFP_KERNEL, GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
// Perform the chipset-specific boot sequence, and retrieve the unload bundle.
- let unload_bundle = hal.boot(&self, &ctx, &fb_layout, &wpr_meta)?.or_else(|| {
- dev_warn!(dev, "The GSP won't be able to unload properly on unbind.\n");
- dev_warn!(
- dev,
- "The GPU will need to be reset before the driver can bind again.\n"
- );
+ let unload_bundle = hal
+ .boot(&self, &mut ctx, &fb_layout, &wpr_meta)?
+ .or_else(|| {
+ dev_warn!(dev, "The GSP won't be able to unload properly on unbind.\n");
+ dev_warn!(
+ dev,
+ "The GPU will need to be reset before the driver can bind again.\n"
+ );
- None
- });
+ None
+ });
- let unload_guard =
+ let mut unload_guard =
ScopeGuard::new_with_data((ctx, unload_bundle), |(ctx, unload_bundle)| {
let _ = self.unload(ctx, unload_bundle);
});
- let ctx = &unload_guard.0;
+ let ctx = &mut unload_guard.0;
gsp_falcon.write_os_version(gsp_fw.bootloader.app_version);
@@ -123,7 +125,7 @@ fn shutdown_gsp(
/// This stops all activity on the GSP.
pub(crate) fn unload(
&self,
- ctx: super::GspBootContext<'_>,
+ mut ctx: super::GspBootContext<'_>,
unload_bundle: Option<super::UnloadBundle>,
) -> Result {
let dev = ctx.dev();
@@ -142,7 +144,7 @@ pub(crate) fn unload(
res = res.and(
unload_bundle
.0
- .run(&ctx)
+ .run(&mut ctx)
.inspect_err(|e| dev_err!(dev, "Unload bundle failed: {:?}\n", e)),
);
} else {
diff --git a/drivers/gpu/nova-core/gsp/hal.rs b/drivers/gpu/nova-core/gsp/hal.rs
index 9da078dd1059..7ebdeafc1432 100644
--- a/drivers/gpu/nova-core/gsp/hal.rs
+++ b/drivers/gpu/nova-core/gsp/hal.rs
@@ -31,7 +31,7 @@
/// required for unloading is prepared at load time, and stored here until it needs to be run.
pub(super) trait UnloadBundle: Send {
/// Performs the steps required to properly reset the GSP after it has been stopped.
- fn run(&self, ctx: &GspBootContext<'_>) -> Result;
+ fn run(&self, ctx: &mut GspBootContext<'_>) -> Result;
}
/// Trait implemented by GSP HALs.
@@ -43,7 +43,7 @@ pub(super) trait GspHal: Send {
fn boot(
&self,
gsp: &Gsp,
- ctx: &GspBootContext<'_>,
+ ctx: &mut GspBootContext<'_>,
fb_layout: &FbLayout,
wpr_meta: &Coherent<GspFwWprMeta>,
) -> Result<Option<crate::gsp::UnloadBundle>>;
@@ -52,7 +52,12 @@ fn boot(
///
/// This method is called by the GSP boot code after the GSP is confirmed to be running, and
/// after the initialization commands have been pushed onto its queue.
- fn post_boot(&self, _gsp: &Gsp, _ctx: &GspBootContext<'_>, _gsp_fw: &GspFirmware) -> Result {
+ fn post_boot(
+ &self,
+ _gsp: &Gsp,
+ _ctx: &mut GspBootContext<'_>,
+ _gsp_fw: &GspFirmware,
+ ) -> Result {
Ok(())
}
}
diff --git a/drivers/gpu/nova-core/gsp/hal/gh100.rs b/drivers/gpu/nova-core/gsp/hal/gh100.rs
index aabef45202f7..043a15dda5be 100644
--- a/drivers/gpu/nova-core/gsp/hal/gh100.rs
+++ b/drivers/gpu/nova-core/gsp/hal/gh100.rs
@@ -114,7 +114,7 @@ fn wait_for_gsp_lockdown_release(
struct FspUnloadBundle;
impl UnloadBundle for FspUnloadBundle {
- fn run(&self, ctx: &GspBootContext<'_>) -> Result {
+ fn run(&self, ctx: &mut GspBootContext<'_>) -> Result {
// GSP falcon does most of the work of resetting, so just wait for it to finish.
read_poll_timeout(
|| Ok(ctx.gsp_falcon.is_riscv_active()),
@@ -137,7 +137,7 @@ impl GspHal for Gh100 {
fn boot(
&self,
gsp: &Gsp,
- ctx: &GspBootContext<'_>,
+ ctx: &mut GspBootContext<'_>,
fb_layout: &FbLayout,
wpr_meta: &Coherent<GspFwWprMeta>,
) -> Result<Option<crate::gsp::UnloadBundle>> {
diff --git a/drivers/gpu/nova-core/gsp/hal/tu102.rs b/drivers/gpu/nova-core/gsp/hal/tu102.rs
index 8480e2eb456f..4e2f48c27368 100644
--- a/drivers/gpu/nova-core/gsp/hal/tu102.rs
+++ b/drivers/gpu/nova-core/gsp/hal/tu102.rs
@@ -78,7 +78,7 @@ struct Sec2UnloadBundle {
}
impl UnloadBundle for Sec2UnloadBundle {
- fn run(&self, ctx: &GspBootContext<'_>) -> Result {
+ fn run(&self, ctx: &mut GspBootContext<'_>) -> Result {
let dev = ctx.dev();
let bar = ctx.bar;
@@ -258,7 +258,7 @@ impl GspHal for Tu102 {
fn boot(
&self,
gsp: &Gsp,
- ctx: &GspBootContext<'_>,
+ ctx: &mut GspBootContext<'_>,
fb_layout: &FbLayout,
wpr_meta: &Coherent<GspFwWprMeta>,
) -> Result<Option<crate::gsp::UnloadBundle>> {
@@ -314,7 +314,7 @@ fn boot(
Ok(unload_guard.dismiss())
}
- fn post_boot(&self, gsp: &Gsp, ctx: &GspBootContext<'_>, gsp_fw: &GspFirmware) -> Result {
+ fn post_boot(&self, gsp: &Gsp, ctx: &mut GspBootContext<'_>, gsp_fw: &GspFirmware) -> Result {
GspSequencer::run(
&gsp.cmdq,
ctx,
--
2.55.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v5 12/13] gpu: nova-core: gsp: separate context and GPU lifetimes in GspBootContext
2026-07-07 7:21 [PATCH v5 00/13] gpu: nova-core: consolidate and streamline GSP boot process Alexandre Courbot
` (10 preceding siblings ...)
2026-07-07 7:21 ` [PATCH v5 11/13] gpu: nova-core: gsp: pass GspBootContext mutably Alexandre Courbot
@ 2026-07-07 7:21 ` Alexandre Courbot
2026-07-07 7:21 ` [PATCH v5 13/13] gpu: nova-core: store Fsp instance in Gpu Alexandre Courbot
12 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-07-07 7:21 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Gary Guo, John Hubbard, Alistair Popple, Timur Tabi,
Eliot Courtney, Zhi Wang
Cc: nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
`Falcon` instances retain references tied to the lifetime of the bound
GPU. `GspBootContext` currently uses that same lifetime for its own
borrows of the `Falcon` instances and other references.
But these lifetimes are independent: the references captured by a
`Falcon` remain valid for the GPU lifetime, while the context only
borrows the `Falcon` for the duration of a boot or unload operation.
This distinction is hidden for shared references by covariance, but
cannot be ignored anymore if the context carries mutable references to
GPU subdevices, as will happen for the `Fsp` and the `Falcon`s.
Thus, give `GspBootContext` separate lifetimes for its subdevice borrows
and the GPU resources captured by those subdevices, and update its users
accordingly.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/gsp.rs | 19 ++++++++++++-------
drivers/gpu/nova-core/gsp/boot.rs | 4 ++--
drivers/gpu/nova-core/gsp/hal.rs | 6 +++---
drivers/gpu/nova-core/gsp/hal/gh100.rs | 4 ++--
drivers/gpu/nova-core/gsp/hal/tu102.rs | 11 ++++++++---
drivers/gpu/nova-core/gsp/sequencer.rs | 2 +-
6 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index d5cca3c8350a..9f055a0d6cb9 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -54,16 +54,21 @@
pub(crate) const GSP_PAGE_SIZE: usize = 1 << GSP_PAGE_SHIFT;
/// Common context for the GSP boot process.
-pub(crate) struct GspBootContext<'a> {
- pub(crate) pdev: &'a pci::Device<device::Bound>,
- pub(crate) bar: Bar0<'a>,
+///
+/// It carries two distinct lifetimes:
+///
+/// - `'gpu` is the lifetime of the bound GPU device, as captured by the GPU subdevices.
+/// - `'ctx` is a shorter lifetime during which this context borrows those subdevices.
+pub(crate) struct GspBootContext<'ctx, 'gpu> {
+ pub(crate) pdev: &'gpu pci::Device<device::Bound>,
+ pub(crate) bar: Bar0<'gpu>,
pub(crate) chipset: Chipset,
- pub(crate) gsp_falcon: &'a Falcon<'a, GspFalcon>,
- pub(crate) sec2_falcon: &'a Falcon<'a, Sec2Falcon>,
+ pub(crate) gsp_falcon: &'ctx Falcon<'gpu, GspFalcon>,
+ pub(crate) sec2_falcon: &'ctx Falcon<'gpu, Sec2Falcon>,
}
-impl<'a> GspBootContext<'a> {
- pub(crate) fn dev(&self) -> &'a device::Device<device::Bound> {
+impl<'ctx, 'gpu> GspBootContext<'ctx, 'gpu> {
+ pub(crate) fn dev(&self) -> &'gpu device::Device<device::Bound> {
self.pdev.as_ref()
}
}
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index f093e0215b66..c347558aa8e5 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -39,7 +39,7 @@ impl super::Gsp {
/// [`Self::unload`]) returned.
pub(crate) fn boot(
self: Pin<&mut Self>,
- mut ctx: super::GspBootContext<'_>,
+ mut ctx: super::GspBootContext<'_, '_>,
) -> Result<Option<super::UnloadBundle>> {
let pdev = ctx.pdev;
let bar = ctx.bar;
@@ -125,7 +125,7 @@ fn shutdown_gsp(
/// This stops all activity on the GSP.
pub(crate) fn unload(
&self,
- mut ctx: super::GspBootContext<'_>,
+ mut ctx: super::GspBootContext<'_, '_>,
unload_bundle: Option<super::UnloadBundle>,
) -> Result {
let dev = ctx.dev();
diff --git a/drivers/gpu/nova-core/gsp/hal.rs b/drivers/gpu/nova-core/gsp/hal.rs
index 7ebdeafc1432..34b4bb82a999 100644
--- a/drivers/gpu/nova-core/gsp/hal.rs
+++ b/drivers/gpu/nova-core/gsp/hal.rs
@@ -31,7 +31,7 @@
/// required for unloading is prepared at load time, and stored here until it needs to be run.
pub(super) trait UnloadBundle: Send {
/// Performs the steps required to properly reset the GSP after it has been stopped.
- fn run(&self, ctx: &mut GspBootContext<'_>) -> Result;
+ fn run(&self, ctx: &mut GspBootContext<'_, '_>) -> Result;
}
/// Trait implemented by GSP HALs.
@@ -43,7 +43,7 @@ pub(super) trait GspHal: Send {
fn boot(
&self,
gsp: &Gsp,
- ctx: &mut GspBootContext<'_>,
+ ctx: &mut GspBootContext<'_, '_>,
fb_layout: &FbLayout,
wpr_meta: &Coherent<GspFwWprMeta>,
) -> Result<Option<crate::gsp::UnloadBundle>>;
@@ -55,7 +55,7 @@ fn boot(
fn post_boot(
&self,
_gsp: &Gsp,
- _ctx: &mut GspBootContext<'_>,
+ _ctx: &mut GspBootContext<'_, '_>,
_gsp_fw: &GspFirmware,
) -> Result {
Ok(())
diff --git a/drivers/gpu/nova-core/gsp/hal/gh100.rs b/drivers/gpu/nova-core/gsp/hal/gh100.rs
index 043a15dda5be..be531df8680b 100644
--- a/drivers/gpu/nova-core/gsp/hal/gh100.rs
+++ b/drivers/gpu/nova-core/gsp/hal/gh100.rs
@@ -114,7 +114,7 @@ fn wait_for_gsp_lockdown_release(
struct FspUnloadBundle;
impl UnloadBundle for FspUnloadBundle {
- fn run(&self, ctx: &mut GspBootContext<'_>) -> Result {
+ fn run(&self, ctx: &mut GspBootContext<'_, '_>) -> Result {
// GSP falcon does most of the work of resetting, so just wait for it to finish.
read_poll_timeout(
|| Ok(ctx.gsp_falcon.is_riscv_active()),
@@ -137,7 +137,7 @@ impl GspHal for Gh100 {
fn boot(
&self,
gsp: &Gsp,
- ctx: &mut GspBootContext<'_>,
+ ctx: &mut GspBootContext<'_, '_>,
fb_layout: &FbLayout,
wpr_meta: &Coherent<GspFwWprMeta>,
) -> Result<Option<crate::gsp::UnloadBundle>> {
diff --git a/drivers/gpu/nova-core/gsp/hal/tu102.rs b/drivers/gpu/nova-core/gsp/hal/tu102.rs
index 4e2f48c27368..29bb17171f56 100644
--- a/drivers/gpu/nova-core/gsp/hal/tu102.rs
+++ b/drivers/gpu/nova-core/gsp/hal/tu102.rs
@@ -78,7 +78,7 @@ struct Sec2UnloadBundle {
}
impl UnloadBundle for Sec2UnloadBundle {
- fn run(&self, ctx: &mut GspBootContext<'_>) -> Result {
+ fn run(&self, ctx: &mut GspBootContext<'_, '_>) -> Result {
let dev = ctx.dev();
let bar = ctx.bar;
@@ -258,7 +258,7 @@ impl GspHal for Tu102 {
fn boot(
&self,
gsp: &Gsp,
- ctx: &mut GspBootContext<'_>,
+ ctx: &mut GspBootContext<'_, '_>,
fb_layout: &FbLayout,
wpr_meta: &Coherent<GspFwWprMeta>,
) -> Result<Option<crate::gsp::UnloadBundle>> {
@@ -314,7 +314,12 @@ fn boot(
Ok(unload_guard.dismiss())
}
- fn post_boot(&self, gsp: &Gsp, ctx: &mut GspBootContext<'_>, gsp_fw: &GspFirmware) -> Result {
+ fn post_boot(
+ &self,
+ gsp: &Gsp,
+ ctx: &mut GspBootContext<'_, '_>,
+ gsp_fw: &GspFirmware,
+ ) -> Result {
GspSequencer::run(
&gsp.cmdq,
ctx,
diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs b/drivers/gpu/nova-core/gsp/sequencer.rs
index ddce32cc4e30..422a74f9ecbd 100644
--- a/drivers/gpu/nova-core/gsp/sequencer.rs
+++ b/drivers/gpu/nova-core/gsp/sequencer.rs
@@ -335,7 +335,7 @@ fn next(&mut self) -> Option<Self::Item> {
impl<'a> GspSequencer<'a> {
pub(crate) fn run(
cmdq: &Cmdq,
- ctx: &'a GspBootContext<'_>,
+ ctx: &'a GspBootContext<'_, '_>,
libos_dma_handle: u64,
bootloader_app_version: u32,
) -> Result {
--
2.55.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v5 13/13] gpu: nova-core: store Fsp instance in Gpu
2026-07-07 7:21 [PATCH v5 00/13] gpu: nova-core: consolidate and streamline GSP boot process Alexandre Courbot
` (11 preceding siblings ...)
2026-07-07 7:21 ` [PATCH v5 12/13] gpu: nova-core: gsp: separate context and GPU lifetimes in GspBootContext Alexandre Courbot
@ 2026-07-07 7:21 ` Alexandre Courbot
12 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-07-07 7:21 UTC (permalink / raw)
To: Danilo Krummrich, Alice Ryhl, David Airlie, Simona Vetter,
Gary Guo, John Hubbard, Alistair Popple, Timur Tabi,
Eliot Courtney, Zhi Wang
Cc: nova-gpu, dri-devel, linux-kernel, rust-for-linux,
Alexandre Courbot
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 <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
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
^ permalink raw reply related [flat|nested] 20+ messages in thread