linux-kernel - Re: [PATCH 1/2] gpu: nova: remove Spec and Revision types

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: <DDRBED7J0QGR.2LOZZJOYYONIS@kernel.org>
Date: Sat, 25 Oct 2025 12:01:26 +0200
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 1/2] gpu: nova: remove Spec and Revision types

On Sat Oct 25, 2025 at 2:14 AM CEST, John Hubbard wrote:
> In the fullness of time, it has become clear that these two types are
> not actually needed for anything, after all. Remove them both, and use
> boot0.chipset directly, instead.

What has become clear is that we don't use the spec field in struct Gpu so far,
which is not a surprise given that all efforts focus on the initialization path.

But even if we forsee that we do not have a reason to store the spec field in
struct Gpu, it doesn't mean that the structure itself is useless, see more
below.

>  /// Structure holding the resources required to operate the GPU.
>  #[pin_data]
>  pub(crate) struct Gpu {
> -    spec: Spec,
> +    chipset: Chipset,
>      /// MMIO mapping of PCI BAR 0
>      bar: Arc<Devres<Bar0>>,
>      /// System memory page required for flushing all pending GPU-side memory writes done through
> @@ -191,16 +153,21 @@ pub(crate) fn new<'a>(
>          devres_bar: Arc<Devres<Bar0>>,
>          bar: &'a Bar0,
>      ) -> impl PinInit<Self, Error> + 'a {
> +        let boot0 = regs::NV_PMC_BOOT_0::read(bar);
> +
>          try_pin_init!(Self {
> -            spec: Spec::new(bar).inspect(|spec| {
> +            chipset: {
> +                let chipset = boot0.chipset()?;
>                  dev_info!(
>                      pdev.as_ref(),
> -                    "NVIDIA (Chipset: {}, Architecture: {:?}, Revision: {})\n",
> -                    spec.chipset,
> -                    spec.chipset.arch(),
> -                    spec.revision
> +                    "NVIDIA (Chipset: {}, Architecture: {:?}, Revision: {:x}.{:x})\n",
> +                    chipset,
> +                    chipset.arch(),
> +                    boot0.major_revision(),
> +                    boot0.minor_revision()

Now you have to open code reading the register, get the Chipset instance and
manually format the revision, which was previously done through a Display impl
of Revision. And the subsequent patch introduces even more open coded logic in
the constructor of struct Gpu.

Instead of removing Spec, we should improve it by giving it its own Display
impl, such that this code can become:

	dev_info!(pdev.as_ref(), "{}\n", spec);