linux-kernel - Re: [PATCH 1/6] gpu: nova-core: print FB sizes, along with ranges

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: <DECHWCRE819D.QVXCHPH23WTK@nvidia.com>
Date: Wed, 19 Nov 2025 16:33:01 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "John Hubbard" <jhubbard@...dia.com>, "Danilo Krummrich"
 <dakr@...nel.org>
Cc: "Alexandre Courbot" <acourbot@...dia.com>, "Joel Fernandes"
 <joelagnelf@...dia.com>, "Timur Tabi" <ttabi@...dia.com>, "Alistair Popple"
 <apopple@...dia.com>, "Edwin Peer" <epeer@...dia.com>, "Zhi Wang"
 <zhiw@...dia.com>, "David Airlie" <airlied@...il.com>, "Simona Vetter"
 <simona@...ll.ch>, "Bjorn Helgaas" <bhelgaas@...gle.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>,
 <nouveau@...ts.freedesktop.org>, <rust-for-linux@...r.kernel.org>, "LKML"
 <linux-kernel@...r.kernel.org>, "Nouveau"
 <nouveau-bounces@...ts.freedesktop.org>
Subject: Re: [PATCH 1/6] gpu: nova-core: print FB sizes, along with ranges

On Thu Nov 6, 2025 at 12:54 PM JST, John Hubbard wrote:
> For convenience of the reader: now you can directly see the sizes of
> each range. It is suprising just how much this helps.
>
> Sample output:
>
> NovaCore 0000:e1:00.0: FbLayout {
>     fb: 0x0..0x3ff800000 (16376 MB),
>     vga_workspace: 0x3ff700000..0x3ff800000 (1 MB),
>     frts: 0x3ff600000..0x3ff700000 (1 MB),
>     boot: 0x3ff5fa000..0x3ff600000 (0 MB),
>     elf: 0x3fb960000..0x3ff5f9000 (60 MB),
>     wpr2_heap: 0x3f3900000..0x3fb900000 (128 MB),
>     wpr2: 0x3f3800000..0x3ff700000 (191 MB),
>     heap: 0x3f3700000..0x3f3800000 (1 MB),
>     vf_partition_count: 0x0,
>     rsvd_size: 0x1a00000,
> }
>
> Signed-off-by: John Hubbard <jhubbard@...dia.com>
> ---
>  drivers/gpu/nova-core/fb.rs       | 33 ++++++++++++++++++++++++++++++-
>  drivers/gpu/nova-core/gsp/boot.rs |  2 +-
>  2 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
> index 10406b6f2e16..004238689f26 100644
> --- a/drivers/gpu/nova-core/fb.rs
> +++ b/drivers/gpu/nova-core/fb.rs
> @@ -87,7 +87,6 @@ pub(crate) fn unregister(&self, bar: &Bar0) {
>  /// Layout of the GPU framebuffer memory.
>  ///
>  /// Contains ranges of GPU memory reserved for a given purpose during the GSP boot process.
> -#[derive(Debug)]
>  pub(crate) struct FbLayout {
>      /// Range of the framebuffer. Starts at `0`.
>      pub(crate) fb: Range<u64>,
> @@ -107,6 +106,38 @@ pub(crate) struct FbLayout {
>      pub(crate) vf_partition_count: u8,
>  }
>  
> +struct RangeWithSize<'a>(&'a Range<u64>);
> +
> +impl core::fmt::Debug for RangeWithSize<'_> {
> +    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
> +        if self.0.start == 0 && self.0.end == 0 {
> +            write!(f, "0x0..0x0")
> +        } else {
> +            let size_mb = (self.0.end - self.0.start) >> 20;
> +            write!(f, "{:#x}..{:#x} ({} MB)", self.0.start, self.0.end, size_mb)
> +        }
> +    }
> +}
> +
> +impl core::fmt::Debug for FbLayout {
> +    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
> +        f.debug_struct("FbLayout")
> +            .field("fb", &RangeWithSize(&self.fb))
> +            .field("vga_workspace", &RangeWithSize(&self.vga_workspace))
> +            .field("frts", &RangeWithSize(&self.frts))
> +            .field("boot", &RangeWithSize(&self.boot))
> +            .field("elf", &RangeWithSize(&self.elf))
> +            .field("wpr2_heap", &RangeWithSize(&self.wpr2_heap))
> +            .field("wpr2", &RangeWithSize(&self.wpr2))
> +            .field("heap", &RangeWithSize(&self.heap))
> +            .field(
> +                "vf_partition_count",
> +                &fmt!("{:#x}", self.vf_partition_count),
> +            )
> +            .finish()
> +    }
> +}

The only concern I have is that if we add fields to `FbLayout` we will
need (and probably forget) to update its `Debug` implementation.

How about we just use this more intrusively:

    pub(crate) struct FbRange(Range<u64>);

    // Convert easily from a regular `Range`.
    impl From<Range<u64>> for FbRange {
        fn from(range: Range<u64>) -> Self {
            Self(range)
        }
    }

    // Provide transparent access to the members of `Range`.
    impl Deref for FbRange {
        type Target = Range<u64>;

        fn deref(&self) -> &Self::Target {
            &self.0
        }
    }

    impl Debug for FbRange {
        ...
    }

Then we can change the members of `FbLayout` to `FbRange`, and keep its
derived `Debug` implementation.

The initialization code would only need to marginally change, e.g:

    let fb: FbRange = {
        let fb_size = hal.vidmem_size(bar);

        (0..fb_size).into()
    };

And with this new type, we can also address one another shortcoming that
was bugging me! In e.g. `boot.rs` we have this ugly bit:

        frts_size: fb_layout.frts.end - fb_layout.frts.start,

What we want is a `len` method, but since our range uses u64, and `len`
returns a `usize`, standard Rust doesn't provide one for us. But thanks
to this dedicated type we can now implement our own! :)

Not saying this has to be done in this patch though, but it's a nice
side-effect.