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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DCFCSPF1PTLT.2A4LKV4TAF0JU@nvidia.com>
Date: Sat, 30 Aug 2025 09:59:13 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "John Hubbard" <jhubbard@...dia.com>, "Miguel Ojeda" <ojeda@...nel.org>,
 "Alex Gaynor" <alex.gaynor@...il.com>, "Boqun Feng" <boqun.feng@...il.com>,
 "Gary Guo" <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, "Benno Lossin" <lossin@...nel.org>, "Andreas
 Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
 "Trevor Gross" <tmgross@...ch.edu>, "Danilo Krummrich" <dakr@...nel.org>,
 "David Airlie" <airlied@...il.com>, "Simona Vetter" <simona@...ll.ch>,
 "Maarten Lankhorst" <maarten.lankhorst@...ux.intel.com>, "Maxime Ripard"
 <mripard@...nel.org>, "Thomas Zimmermann" <tzimmermann@...e.de>
Cc: "Alistair Popple" <apopple@...dia.com>, "Joel Fernandes"
 <joelagnelf@...dia.com>, "Timur Tabi" <ttabi@...dia.com>,
 <rust-for-linux@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
 <nouveau@...ts.freedesktop.org>, <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH v2 8/8] gpu: nova-core: compute layout of more
 framebuffer regions required for GSP

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?