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-1f3a425d7...@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?
