Re: [PATCH v2 8/8] gpu: nova-core: compute layout of more framebuffer regions required for GSP

["Alexandre Courbot" <acourbot@nvidia.com>]

On Sat Aug 30, 2025 at 8:30 AM JST, John Hubbard wrote:
> On 8/25/25 9:07 PM, Alexandre Courbot wrote:
>> diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
>> index b0e860498b883815b3861b8717f8ee1832d25440..a3eb063f86b3a06a7ad01e684919115abf5e28da 100644
> ...
>>          let fb = {
>> @@ -138,10 +202,54 @@ pub(crate) fn new(chipset: Chipset, bar: &Bar0) -> Result<Self> {
>>              frts_base..frts_base + FRTS_SIZE
>>          };
>>  
>> +        let boot = {
>
> A few lines earlier, not shown in these diffs because it's not part of this patch,
> there is a closely related TODO item:
>
>             // TODO[NUMM]: replace with `align_down` once it lands.
>             let frts_base = (vga_workspace.start & !(FRTS_DOWN_ALIGN - 1)) - FRTS_SIZE;
>
> ...which I think could be optionally fixed now, and added to this patch.
>
> Or it could be done later, in a different patch, but it seems convenient
> to merge it in as long as we're here, and using .align_down() in this patch.

This is taken care of by the series adding the `Alignment` type:

https://lore.kernel.org/rust-for-linux/20250821-num-v4-2-1f3a425d7244@nvidia.com/

>
> ...
>> diff --git a/drivers/gpu/nova-core/nvfw.rs b/drivers/gpu/nova-core/nvfw.rs
>> index 7c5baccc34a2387c30e51f93d3ae039b14b6b83a..11a63c3710b1aa1eec78359c15c101bdf2ad99c8 100644
>> --- a/drivers/gpu/nova-core/nvfw.rs
>> +++ b/drivers/gpu/nova-core/nvfw.rs
>> @@ -1,3 +1,42 @@
>>  // SPDX-License-Identifier: GPL-2.0
>>  
>>  mod r570_144;
>> +
>> +use core::ops::Range;
>> +
>> +use kernel::sizes::SZ_1M;
>> +
>> +/// Heap memory requirements and constraints for a given version of the GSP LIBOS.
>> +pub(crate) struct LibosParams {
>> +    /// The base amount of heap required by the GSP operating system, in bytes.
>> +    pub(crate) carveout_size: u64,
>> +    /// The minimum and maximum sizes allowed for the GSP FW heap, in bytes.
>> +    pub(crate) allowed_heap_size: Range<u64>,
>> +}
>> +
>> +/// Version 2 of the GSP LIBOS (Turing and GA100)
>> +pub(crate) const LIBOS2_PARAMS: LibosParams = LibosParams {
>> +    carveout_size: r570_144::GSP_FW_HEAP_PARAM_OS_SIZE_LIBOS2 as u64,
>> +    allowed_heap_size: r570_144::GSP_FW_HEAP_SIZE_OVERRIDE_LIBOS2_MIN_MB as u64 * SZ_1M as u64
>> +        ..r570_144::GSP_FW_HEAP_SIZE_OVERRIDE_LIBOS2_MAX_MB as u64 * SZ_1M as u64,
>
> We only support one version of the firmware. And in the coming months,
> that one version will have a different version number.
>
> Given those constraints, we should simply remove most (all?) of the "r570_144::"
> namespace qualifiers in the code, starting here.
>
> That way, we get:
>
> a) A small diff, instead of a huge one, when we update to a new firmware
>    version.
>
> b) Shorter, cleaner symbols everywhere: GSP_FW_HEAP_SIZE_OVERRIDE_LIBOS2_MAX_MB
>    instead of r570_144::GSP_FW_HEAP_SIZE_OVERRIDE_LIBOS2_MAX_MB, for example.

`nvfw` is the module that is supposed to abstract the currently
supported firmware version - but in order to provide this abstraction,
it needs to refer the items in question. :) I don't see how we could
avoid these qualifiers short of having a `use r750_144::*` which could
result into name collisions.

But maybe we can do a module alias to reduce the diff once the version
changes:

    use r570_144 as fwbindings;
    ...

    pub(crate) const LIBOS2_PARAMS: LibosParams = LibosParams {
        carveout_size: fwbindings::GSP_FW_HEAP_PARAM_OS_SIZE_LIBOS2 as u64,

Is that what you had in mind?