linux-kernel - Re: [PATCH v2 1/2] gpu: nova-core: merge the Revision type into the Spec type

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: <DDTYCAZWUZL5.36Y1LKLB8EUD6@kernel.org>
Date: Tue, 28 Oct 2025 13:25:30 +0100
From: "Danilo Krummrich" <dakr@...nel.org>
To: "John Hubbard" <jhubbard@...dia.com>
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>
Subject: Re: [PATCH v2 1/2] gpu: nova-core: merge the Revision type into the
 Spec type

On Tue Oct 28, 2025 at 3:39 AM CET, John Hubbard wrote:
> -pub(crate) struct Revision {
> -    major: u8,
> -    minor: u8,
> -}
> -
> -impl Revision {
> -    fn from_boot0(boot0: regs::NV_PMC_BOOT_0) -> Self {
> -        Self {
> -            major: boot0.major_revision(),
> -            minor: boot0.minor_revision(),
> -        }
> -    }
> -}
> -
> -impl fmt::Display for Revision {
> -    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> -        write!(f, "{:x}.{:x}", self.major, self.minor)
> -    }
> -}
> -
> -/// Structure holding the metadata of the GPU.
> +/// Structure holding a basic description of the GPU: Architecture, Chipset and Revision.
>  pub(crate) struct Spec {
>      chipset: Chipset,
> -    /// The revision of the chipset.
> -    revision: Revision,
> +    major_revision: u8,
> +    minor_revision: u8,
>  }

Why not keep the Revision type and its Display impl as well?

>  
>  impl Spec {
> @@ -162,11 +142,25 @@ fn new(bar: &Bar0) -> Result<Spec> {
>  
>          Ok(Self {
>              chipset: boot0.chipset()?,
> -            revision: Revision::from_boot0(boot0),
> +            major_revision: boot0.major_revision(),
> +            minor_revision: boot0.minor_revision(),
>          })
>      }
>  }
>  
> +impl fmt::Display for Spec {
> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> +        write!(
> +            f,
> +            "Chipset: {}, Architecture: {:?}, Revision: {:x}.{:x}",
> +            self.chipset,
> +            self.chipset.arch(),
> +            self.major_revision,
> +            self.minor_revision
> +        )

This could just be:

	write!(
	    f,
	    "Chipset: {}, Architecture: {:?}, Revision: {}",
	    self.chipset,
	    self.chipset.arch(),
	    self.revision,
	)

> +    }
> +}
> +
>  /// Structure holding the resources required to operate the GPU.
>  #[pin_data]
>  pub(crate) struct Gpu {
> @@ -193,13 +187,7 @@ pub(crate) fn new<'a>(
>      ) -> impl PinInit<Self, Error> + 'a {
>          try_pin_init!(Self {
>              spec: Spec::new(bar).inspect(|spec| {
> -                dev_info!(
> -                    pdev.as_ref(),
> -                    "NVIDIA (Chipset: {}, Architecture: {:?}, Revision: {})\n",
> -                    spec.chipset,
> -                    spec.chipset.arch(),
> -                    spec.revision
> -                );
> +                dev_info!(pdev.as_ref(),"NVIDIA ({})\n", spec);
>              })?,
>  
>              // We must wait for GFW_BOOT completion before doing any significant setup on the GPU.
> -- 
> 2.51.1