On Thu May 28, 2026 at 10:50 AM JST, Eliot Courtney wrote: > On Wed May 27, 2026 at 11:02 PM JST, Alexandre Courbot wrote: >> On Fri May 22, 2026 at 3:59 PM JST, Eliot Courtney wrote: >>> On Thu May 21, 2026 at 10:50 PM JST, Alexandre Courbot wrote: >>>> When probing the driver, the FWSEC-FRTS firmware creates a WPR2 secure >>>> memory region to store the GSP firmware, and the Booter Loader loads and >>>> starts that firmware into the GSP, making it run in RISC-V mode. >>>> >>>> These operations need to be reverted upon unloading, particularly the >>>> WPR2 secure region creation, as its presence prevents the driver from >>>> subsequently probing. >>>> >>>> Thus, prepare the Booter Unloader and FWSEC-SB firmwares when booting >>>> the GSP, so they can be executed at unbind time to put the GPU into a >>>> state where it can be probed again. >>>> >>>> Signed-off-by: Alexandre Courbot <[email protected]> >>>> --- >>> >>> After seeing the bundle moved outwards, I realised that it has the same >>> issue that SysmemFlush does, i.e. if probe fails it does not reset the >>> GSP. A lot of the time during development I will break things badly >>> enough that probe fails, so it would be nice if this is supported. OTOH, >>> this gets the probe suceed and unload case working which is important >>> and this is a definite improvement, so for this version and the previous >>> version of the patch: >>> >>> Reviewed-by: Eliot Courtney <[email protected]> >>> >>> I also had a brief go at making this work on Drop, here is the diff on >>> top of this series. I can send this as a follow up if you would like >>> after cleaning it up, or lmk wdyt: >> >> This is clearly better. It guarantees that the unload sequence is run >> when the `Gpu` is dropped, while preserving its one-shot nature. Also, >> no `Cell` and no awkward output parameter to `Gpu::new`. >> >> The only blind spot remaining would be to also cover the case where a >> failure occurs during `Gsp::boot`, but that's for `Gsp::boot` to handle >> itself imho. >> >> Would you be ok if I folded this into this patch for v7, with your >> `Co-developed-by` and `Signed-off-by`? Then I'll also try to tackle the >> `Gsp::boot` failure scenario using a drop wrapper or something similar. > > Yerp that is fine of course. For Gsp::boot it should handle unwinding if > there is a failure but I agree it is orthogonal to this case, so > addressing in a follow-up SGTM.
I will add a patch that takes care of that using a drop guard in v7. While doing so, I noticed that with the current state of the code we could also perfectly store that drop guard into `Gpu` itself, and get the behavior we want without implementing `PinnedDrop`. However, this is only possible because `Falcon::run` and `Falcon::load` do not take mutable references, which they should do eventually for soundness - once we do the switch, storing the drop guard won't be possible anymore as that would mean keeping mutable references for the full life of the GPU. So `PinnedDrop` is necessary. I mention this here preemptively lest it is raised in v7. :)
