Re: [PATCH 5/5] gpu: nova-core: firmware: process and prepare the GSP firmware

[Danilo Krummrich]

Hi Alex,

not a full review yet, but a few ad-hoc comments from skimming over it.

On Fri Aug 22, 2025 at 2:47 PM CEST, Alexandre Courbot wrote:
> +/// A device-mapped firmware with a set of (also device-mapped) pages tables mapping the firmware
> +/// to the start of their own address space.
> +pub(crate) struct GspFirmware {
> +    /// The GSP firmware inside a [`VVec`], device-mapped via a SG table.
> +    #[expect(unused)]

Do we expect this to change? Otherwise, just prefix the field name with an
underscore.

> +    fw: Pin<KBox<SGTable<Owned<VVec<u8>>>>>,
> +    /// The level 2 page table, mapping [`Self::fw`] at its beginning.
> +    #[expect(unused)]
> +    lvl2: Pin<KBox<SGTable<Owned<VVec<u8>>>>>,
> +    /// The level 1 page table, mapping [`Self::lvl2`] at its beginning.
> +    #[expect(unused)]
> +    lvl1: Pin<KBox<SGTable<Owned<VVec<u8>>>>>,

Instead of creating three allocations, just make struct GspFirmware pin_data by
itself. This should even propagate down to struct Gpu, which is pin_data.

So everything can be in one single allocation.

> +    /// The level 0 page table, mapping [`Self::lvl1`] at its beginning.
> +    lvl0: DmaObject,
> +    /// Size in bytes of the firmware contained in [`Self::fw`].
> +    #[expect(unused)]
> +    pub size: usize,
> +}