rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] gpu: nova: add boot42 support for next-gen GPUs
@ 2025-10-29  3:03 John Hubbard
  2025-10-29  3:03 ` [PATCH v3 1/2] gpu: nova-core: prepare Spec and Revision types for boot0/boot42 John Hubbard
  2025-10-29  3:03 ` [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs John Hubbard
  0 siblings, 2 replies; 27+ messages in thread
From: John Hubbard @ 2025-10-29  3:03 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Alexandre Courbot, Joel Fernandes, Timur Tabi, Alistair Popple,
	Edwin Peer, Zhi Wang, David Airlie, Simona Vetter, Bjorn Helgaas,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, nouveau, rust-for-linux, LKML, John Hubbard

Changes in v3:

1) Restored the Revision type as recommended by Danilo, but decoupled it
from boot0.

2) Applied Alex Courbot's suggestion to use TryFrom<NV_PMC_BOOT_0/42>
for Spec.

3) Reflowed the new comment documentation to 100 cols, to avoid wasting
a few vertical lines.

Changes in v2:

1) Restored the Spec type, and used that to encapsulate the subsequent
   boot42 enhancements. Thanks to Danilo Krummrich's feedback for that
   improvement.

v1 cover letter:

NVIDIA GPUs are moving away from using NV_PMC_BOOT_0 to contain
architecture and revision details, and will instead use NV_PMC_BOOT_42
in the future. NV_PMC_BOOT_0 will be zeroed out.

Change the selection logic in Nova so that it will claim Turing and
later GPUs. This will work for the foreseeable future, without any
further code changes here, because all NVIDIA GPUs are considered, from
the oldest supported on Linux (NV04), through the future GPUs.

Add some comment documentation to explain, chronologically, how boot0
and boot42 change with the GPU eras, and how that affects the selection
logic.

Also, remove the Revision type, because Revision is no longer valuable
as a stand-alone type, because we only ever want the full information
that Spec provides.

This is based on today's drm-rust-next, which in turn is based on
Linux 6.18-rc2.

John Hubbard (2):
  gpu: nova-core: prepare Spec and Revision types for boot0/boot42
  gpu: nova-core: add boot42 support for next-gen GPUs

 drivers/gpu/nova-core/gpu.rs  | 94 ++++++++++++++++++++++++++++-------
 drivers/gpu/nova-core/regs.rs | 27 ++++++++++
 2 files changed, 103 insertions(+), 18 deletions(-)


base-commit: ca16b15e78f4dee1631c0a68693f5e7d9b3bb3ec
-- 
2.51.2


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v3 1/2] gpu: nova-core: prepare Spec and Revision types for boot0/boot42
  2025-10-29  3:03 [PATCH v3 0/2] gpu: nova: add boot42 support for next-gen GPUs John Hubbard
@ 2025-10-29  3:03 ` John Hubbard
  2025-10-29  3:03 ` [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs John Hubbard
  1 sibling, 0 replies; 27+ messages in thread
From: John Hubbard @ 2025-10-29  3:03 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Alexandre Courbot, Joel Fernandes, Timur Tabi, Alistair Popple,
	Edwin Peer, Zhi Wang, David Airlie, Simona Vetter, Bjorn Helgaas,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, nouveau, rust-for-linux, LKML, John Hubbard

1) Implement Display for Spec. This simplifies the dev_info!() code for
   printing banners such as:

    NVIDIA (Chipset: GA104, Architecture: Ampere, Revision: a.1)

2) Decouple Revision from boot0.

3) Also, slightly enhance the comment about Spec, to be more precise.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 drivers/gpu/nova-core/gpu.rs | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 9d182bffe8b4..6f1486d4e9c6 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -134,22 +134,13 @@ pub(crate) struct Revision {
     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.
@@ -162,11 +153,26 @@ fn new(bar: &Bar0) -> Result<Spec> {
 
         Ok(Self {
             chipset: boot0.chipset()?,
-            revision: Revision::from_boot0(boot0),
+            revision: Revision {
+                major: boot0.major_revision(),
+                minor: boot0.minor_revision(),
+            },
         })
     }
 }
 
+impl fmt::Display for Spec {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        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 +199,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.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-29  3:03 [PATCH v3 0/2] gpu: nova: add boot42 support for next-gen GPUs John Hubbard
  2025-10-29  3:03 ` [PATCH v3 1/2] gpu: nova-core: prepare Spec and Revision types for boot0/boot42 John Hubbard
@ 2025-10-29  3:03 ` John Hubbard
  2025-10-29 11:26   ` Danilo Krummrich
                     ` (2 more replies)
  1 sibling, 3 replies; 27+ messages in thread
From: John Hubbard @ 2025-10-29  3:03 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Alexandre Courbot, Joel Fernandes, Timur Tabi, Alistair Popple,
	Edwin Peer, Zhi Wang, David Airlie, Simona Vetter, Bjorn Helgaas,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, nouveau, rust-for-linux, LKML, John Hubbard

NVIDIA GPUs are moving away from using NV_PMC_BOOT_0 to contain
architecture and revision details, and will instead use NV_PMC_BOOT_42
in the future. NV_PMC_BOOT_0 will be zeroed out.

Change the selection logic in Nova so that it will claim Turing and
later GPUs. This will work for the foreseeable future, without any
further code changes here, because all NVIDIA GPUs are considered, from
the oldest supported on Linux (NV04), through the future GPUs.

Add some comment documentation to explain, chronologically, how boot0
and boot42 change with the GPU eras, and how that affects the selection
logic.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 drivers/gpu/nova-core/gpu.rs  | 72 +++++++++++++++++++++++++++++++----
 drivers/gpu/nova-core/regs.rs | 27 +++++++++++++
 2 files changed, 92 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 6f1486d4e9c6..6762493206ec 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -134,6 +134,34 @@ pub(crate) struct Revision {
     minor: u8,
 }
 
+impl TryFrom<regs::NV_PMC_BOOT_0> for Spec {
+    type Error = Error;
+
+    fn try_from(boot0: regs::NV_PMC_BOOT_0) -> Result<Self> {
+        Ok(Self {
+            chipset: boot0.chipset()?,
+            revision: Revision {
+                major: boot0.major_revision(),
+                minor: boot0.minor_revision(),
+            },
+        })
+    }
+}
+
+impl TryFrom<regs::NV_PMC_BOOT_42> for Spec {
+    type Error = Error;
+
+    fn try_from(boot42: regs::NV_PMC_BOOT_42) -> Result<Self> {
+        Ok(Self {
+            chipset: boot42.chipset()?,
+            revision: Revision {
+                major: boot42.major_revision(),
+                minor: boot42.minor_revision(),
+            },
+        })
+    }
+}
+
 impl fmt::Display for Revision {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{:x}.{:x}", self.major, self.minor)
@@ -151,13 +179,43 @@ impl Spec {
     fn new(bar: &Bar0) -> Result<Spec> {
         let boot0 = regs::NV_PMC_BOOT_0::read(bar);
 
-        Ok(Self {
-            chipset: boot0.chipset()?,
-            revision: Revision {
-                major: boot0.major_revision(),
-                minor: boot0.minor_revision(),
-            },
-        })
+        // "next-gen" GPUs (some time after Blackwell) will zero out boot0, and put the architecture
+        // details in boot42 instead. Avoid reading boot42 unless we are in that case.
+        let boot42 = if boot0.is_next_gen() {
+            Some(regs::NV_PMC_BOOT_42::read(bar))
+        } else {
+            None
+        };
+
+        // Some brief notes about boot0 and boot42, in chronological order:
+        //
+        // NV04 through Volta:
+        //
+        //    Not supported by Nova. boot0 is necessary and sufficient to identify these GPUs.
+        //    boot42 may not even exist on some of these GPUs.boot42
+        //
+        // Turing through Blackwell:
+        //
+        //     Supported by both Nouveau and Nova. boot0 is still necessary and sufficient to
+        //     identify these GPUs. boot42 exists on these GPUs but we don't need to use it.
+        //
+        // Future "next-gen" GPUs:
+        //
+        //    Only supported by Nova. boot42 has the architecture details, boot0 is zeroed out.
+
+        // NV04, the very first NVIDIA GPU to be supported on Linux, is identified by a specific bit
+        // pattern in boot0. Although Nova does not support NV04 (see above), it is possible to
+        // confuse NV04 with a "next-gen" GPU. Therefore, return early if we specifically detect
+        // NV04, thus simplifying the remaining selection logic.
+        if boot0.is_nv04() {
+            Err(ENODEV)?
+        }
+
+        // Now that we know it is something more recent than NV04, use boot42 if we previously
+        // determined that boot42 was both valid and relevant, and boot0 otherwise.
+        boot42
+            .map(Spec::try_from)
+            .unwrap_or_else(|| Spec::try_from(boot0))
     }
 }
 
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 206dab2e1335..ed3a2c39edbc 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -25,6 +25,18 @@
 });
 
 impl NV_PMC_BOOT_0 {
+    pub(crate) fn is_nv04(self) -> bool {
+        // The very first supported GPU was NV04, and it is identified by a specific bit pattern in
+        // boot0. This provides a way to check for that, which in turn is required in order to avoid
+        // confusing future "next-gen" GPUs with NV04.
+        self.architecture_0() == 0 && (self.0 & 0xff00fff0) == 0x20004000
+    }
+
+    pub(crate) fn is_next_gen(self) -> bool {
+        // "next-gen" GPUs (some time after Blackwell) will set `architecture_0` to 0, and put the
+        // architecture details in boot42 instead.
+        self.architecture_0() == 0 && !self.is_nv04()
+    }
     /// Combines `architecture_0` and `architecture_1` to obtain the architecture of the chip.
     pub(crate) fn architecture(self) -> Result<Architecture> {
         Architecture::try_from(
@@ -43,6 +55,21 @@ pub(crate) fn chipset(self) -> Result<Chipset> {
     }
 }
 
+register!(NV_PMC_BOOT_42 @ 0x00000108, "Extended architecture information" {
+    7:0     implementation as u8, "Implementation version of the architecture";
+    15:8    architecture as u8, "Architecture value";
+    19:16   minor_revision as u8, "Minor revision of the chip";
+    23:20   major_revision as u8, "Major revision of the chip";
+});
+
+impl NV_PMC_BOOT_42 {
+    pub(crate) fn chipset(self) -> Result<Chipset> {
+        let arch = Architecture::try_from(self.architecture())?;
+        let chipset_value = ((arch as u32) << 8) | u32::from(self.implementation());
+        Chipset::try_from(chipset_value)
+    }
+}
+
 // PBUS
 
 register!(NV_PBUS_SW_SCRATCH @ 0x00001400[64]  {});
-- 
2.51.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-29  3:03 ` [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs John Hubbard
@ 2025-10-29 11:26   ` Danilo Krummrich
  2025-10-29 13:54     ` Alexandre Courbot
  2025-10-30  0:29     ` John Hubbard
  2025-10-29 14:05   ` Alexandre Courbot
  2025-10-29 15:02   ` Timur Tabi
  2 siblings, 2 replies; 27+ messages in thread
From: Danilo Krummrich @ 2025-10-29 11:26 UTC (permalink / raw)
  To: John Hubbard
  Cc: Alexandre Courbot, Joel Fernandes, Timur Tabi, Alistair Popple,
	Edwin Peer, Zhi Wang, David Airlie, Simona Vetter, Bjorn Helgaas,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, nouveau, rust-for-linux, LKML

On Wed Oct 29, 2025 at 4:03 AM CET, John Hubbard wrote:
> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> index 6f1486d4e9c6..6762493206ec 100644
> --- a/drivers/gpu/nova-core/gpu.rs
> +++ b/drivers/gpu/nova-core/gpu.rs
> @@ -134,6 +134,34 @@ pub(crate) struct Revision {
>      minor: u8,
>  }
>  
> +impl TryFrom<regs::NV_PMC_BOOT_0> for Spec {
> +    type Error = Error;
> +
> +    fn try_from(boot0: regs::NV_PMC_BOOT_0) -> Result<Self> {
> +        Ok(Self {
> +            chipset: boot0.chipset()?,
> +            revision: Revision {
> +                major: boot0.major_revision(),
> +                minor: boot0.minor_revision(),
> +            },

Actually, would be nice to handle this just like chipset for consistency, i.e.

	revision: boot0.revision()

> +        })
> +    }
> +}
> +
> +impl TryFrom<regs::NV_PMC_BOOT_42> for Spec {
> +    type Error = Error;
> +
> +    fn try_from(boot42: regs::NV_PMC_BOOT_42) -> Result<Self> {
> +        Ok(Self {
> +            chipset: boot42.chipset()?,
> +            revision: Revision {
> +                major: boot42.major_revision(),
> +                minor: boot42.minor_revision(),
> +            },

Same here, could be

	revision: boot42.revision()

> +        })
> +    }
> +}
> +
>  impl fmt::Display for Revision {
>      fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>          write!(f, "{:x}.{:x}", self.major, self.minor)
> @@ -151,13 +179,43 @@ impl Spec {
>      fn new(bar: &Bar0) -> Result<Spec> {
>          let boot0 = regs::NV_PMC_BOOT_0::read(bar);
>  
> -        Ok(Self {
> -            chipset: boot0.chipset()?,
> -            revision: Revision {
> -                major: boot0.major_revision(),
> -                minor: boot0.minor_revision(),
> -            },
> -        })
> +        // "next-gen" GPUs (some time after Blackwell) will zero out boot0, and put the architecture
> +        // details in boot42 instead. Avoid reading boot42 unless we are in that case.
> +        let boot42 = if boot0.is_next_gen() {
> +            Some(regs::NV_PMC_BOOT_42::read(bar))
> +        } else {
> +            None
> +        };
> +
> +        // Some brief notes about boot0 and boot42, in chronological order:
> +        //
> +        // NV04 through Volta:
> +        //
> +        //    Not supported by Nova. boot0 is necessary and sufficient to identify these GPUs.
> +        //    boot42 may not even exist on some of these GPUs.boot42
> +        //
> +        // Turing through Blackwell:
> +        //
> +        //     Supported by both Nouveau and Nova. boot0 is still necessary and sufficient to
> +        //     identify these GPUs. boot42 exists on these GPUs but we don't need to use it.
> +        //
> +        // Future "next-gen" GPUs:
> +        //
> +        //    Only supported by Nova. boot42 has the architecture details, boot0 is zeroed out.
> +
> +        // NV04, the very first NVIDIA GPU to be supported on Linux, is identified by a specific bit
> +        // pattern in boot0. Although Nova does not support NV04 (see above), it is possible to
> +        // confuse NV04 with a "next-gen" GPU. Therefore, return early if we specifically detect
> +        // NV04, thus simplifying the remaining selection logic.
> +        if boot0.is_nv04() {
> +            Err(ENODEV)?
> +        }
> +
> +        // Now that we know it is something more recent than NV04, use boot42 if we previously
> +        // determined that boot42 was both valid and relevant, and boot0 otherwise.
> +        boot42
> +            .map(Spec::try_from)
> +            .unwrap_or_else(|| Spec::try_from(boot0))
>      }
>  }

Without the comments this currently is:

	let boot42 = if boot0.is_next_gen() {
	    Some(regs::NV_PMC_BOOT_42::read(bar))
	} else {
	    None
	};
	
	if boot0.is_nv04() {
	    Err(ENODEV)?
	}
	
	boot42
	    .map(Spec::try_from)
	    .unwrap_or_else(|| Spec::try_from(boot0))

Which I think is a bit heavy-handed. Let's simplify this a bit:

	let boot0 = regs::NV_PMC_BOOT_0::read(bar);

	if boot0.is_nv04() {
	    return Err(ENODEV);
	}

	Spec::try_from(
	    if boot0.is_next_gen() {
	        regs::NV_PMC_BOOT_42::read(bar)
	    } else {
	        boot0
	    }
	)

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-29 11:26   ` Danilo Krummrich
@ 2025-10-29 13:54     ` Alexandre Courbot
  2025-10-30  0:37       ` John Hubbard
  2025-10-30  0:29     ` John Hubbard
  1 sibling, 1 reply; 27+ messages in thread
From: Alexandre Courbot @ 2025-10-29 13:54 UTC (permalink / raw)
  To: Danilo Krummrich, John Hubbard
  Cc: Alexandre Courbot, Joel Fernandes, Timur Tabi, Alistair Popple,
	Edwin Peer, Zhi Wang, David Airlie, Simona Vetter, Bjorn Helgaas,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, nouveau, rust-for-linux, LKML, Nouveau

On Wed Oct 29, 2025 at 8:26 PM JST, Danilo Krummrich wrote:
<snip>
>> @@ -151,13 +179,43 @@ impl Spec {
>>      fn new(bar: &Bar0) -> Result<Spec> {
>>          let boot0 = regs::NV_PMC_BOOT_0::read(bar);
>>  
>> -        Ok(Self {
>> -            chipset: boot0.chipset()?,
>> -            revision: Revision {
>> -                major: boot0.major_revision(),
>> -                minor: boot0.minor_revision(),
>> -            },
>> -        })
>> +        // "next-gen" GPUs (some time after Blackwell) will zero out boot0, and put the architecture
>> +        // details in boot42 instead. Avoid reading boot42 unless we are in that case.
>> +        let boot42 = if boot0.is_next_gen() {
>> +            Some(regs::NV_PMC_BOOT_42::read(bar))
>> +        } else {
>> +            None
>> +        };
>> +
>> +        // Some brief notes about boot0 and boot42, in chronological order:
>> +        //
>> +        // NV04 through Volta:
>> +        //
>> +        //    Not supported by Nova. boot0 is necessary and sufficient to identify these GPUs.
>> +        //    boot42 may not even exist on some of these GPUs.boot42
>> +        //
>> +        // Turing through Blackwell:
>> +        //
>> +        //     Supported by both Nouveau and Nova. boot0 is still necessary and sufficient to
>> +        //     identify these GPUs. boot42 exists on these GPUs but we don't need to use it.
>> +        //
>> +        // Future "next-gen" GPUs:
>> +        //
>> +        //    Only supported by Nova. boot42 has the architecture details, boot0 is zeroed out.
>> +
>> +        // NV04, the very first NVIDIA GPU to be supported on Linux, is identified by a specific bit
>> +        // pattern in boot0. Although Nova does not support NV04 (see above), it is possible to
>> +        // confuse NV04 with a "next-gen" GPU. Therefore, return early if we specifically detect
>> +        // NV04, thus simplifying the remaining selection logic.
>> +        if boot0.is_nv04() {
>> +            Err(ENODEV)?
>> +        }
>> +
>> +        // Now that we know it is something more recent than NV04, use boot42 if we previously
>> +        // determined that boot42 was both valid and relevant, and boot0 otherwise.
>> +        boot42
>> +            .map(Spec::try_from)
>> +            .unwrap_or_else(|| Spec::try_from(boot0))
>>      }
>>  }
>
> Without the comments this currently is:
>
> 	let boot42 = if boot0.is_next_gen() {
> 	    Some(regs::NV_PMC_BOOT_42::read(bar))
> 	} else {
> 	    None
> 	};
> 	
> 	if boot0.is_nv04() {
> 	    Err(ENODEV)?
> 	}
> 	
> 	boot42
> 	    .map(Spec::try_from)
> 	    .unwrap_or_else(|| Spec::try_from(boot0))
>
> Which I think is a bit heavy-handed. Let's simplify this a bit:
>
> 	let boot0 = regs::NV_PMC_BOOT_0::read(bar);
>
> 	if boot0.is_nv04() {
> 	    return Err(ENODEV);
> 	}
>
> 	Spec::try_from(
> 	    if boot0.is_next_gen() {
> 	        regs::NV_PMC_BOOT_42::read(bar)
> 	    } else {
> 	        boot0
> 	    }
> 	)

I don't think this will work because `NV_PMC_BOOT_0` and
`NV_PMC_BOOT_42` are different types, so we cannot alternate them in the
same call to `try_from`. But the following should:

    let boot0 = regs::NV_PMC_BOOT_0::read(bar);
    ...

    if boot0.is_nv04() {
        Err(ENODEV)?
    }

    if boot0.is_next_gen() {
        Spec::try_from(regs::NV_PMC_BOOT_42::read(bar))
    } else {
        Spec::try_from(boot0)
    }


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-29  3:03 ` [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs John Hubbard
  2025-10-29 11:26   ` Danilo Krummrich
@ 2025-10-29 14:05   ` Alexandre Courbot
  2025-10-31  0:04     ` John Hubbard
  2025-11-02  0:34     ` John Hubbard
  2025-10-29 15:02   ` Timur Tabi
  2 siblings, 2 replies; 27+ messages in thread
From: Alexandre Courbot @ 2025-10-29 14:05 UTC (permalink / raw)
  To: John Hubbard, Danilo Krummrich
  Cc: Alexandre Courbot, Joel Fernandes, Timur Tabi, Alistair Popple,
	Edwin Peer, Zhi Wang, David Airlie, Simona Vetter, Bjorn Helgaas,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, nouveau, rust-for-linux, LKML, Nouveau

On Wed Oct 29, 2025 at 12:03 PM JST, John Hubbard wrote:
<snip>
> diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
> index 206dab2e1335..ed3a2c39edbc 100644
> --- a/drivers/gpu/nova-core/regs.rs
> +++ b/drivers/gpu/nova-core/regs.rs
> @@ -25,6 +25,18 @@
>  });
>  
>  impl NV_PMC_BOOT_0 {
> +    pub(crate) fn is_nv04(self) -> bool {
> +        // The very first supported GPU was NV04, and it is identified by a specific bit pattern in
> +        // boot0. This provides a way to check for that, which in turn is required in order to avoid
> +        // confusing future "next-gen" GPUs with NV04.
> +        self.architecture_0() == 0 && (self.0 & 0xff00fff0) == 0x20004000
> +    }
> +
> +    pub(crate) fn is_next_gen(self) -> bool {
> +        // "next-gen" GPUs (some time after Blackwell) will set `architecture_0` to 0, and put the
> +        // architecture details in boot42 instead.
> +        self.architecture_0() == 0 && !self.is_nv04()
> +    }
>      /// Combines `architecture_0` and `architecture_1` to obtain the architecture of the chip.
>      pub(crate) fn architecture(self) -> Result<Architecture> {
>          Architecture::try_from(
> @@ -43,6 +55,21 @@ pub(crate) fn chipset(self) -> Result<Chipset> {
>      }
>  }
>  
> +register!(NV_PMC_BOOT_42 @ 0x00000108, "Extended architecture information" {
> +    7:0     implementation as u8, "Implementation version of the architecture";
> +    15:8    architecture as u8, "Architecture value";

Here you can directly return the `Architecture` type and spare callers
the labor of doing the conversion themselves:

  15:8    architecture as u8 ?=> Architecture, "Architecture value";

This allows the implementation of `NV_PMC_BOOT_42` to mirror that of
`NV_PMC_BOOT_0`:

  impl NV_PMC_BOOT_42 {
      pub(crate) fn chipset(self) -> Result<Chipset> {
          self.architecture()
              .map(|arch| {
                  ((arch as u32) << Self::IMPLEMENTATION_RANGE.len())
                      | u32::from(self.implementation())
              })
              .and_then(Chipset::try_from)
      }
  }

... but also requires `Architecture` to be `repr(u8)` and to implement
`Default` and `Into<u8>` so it can be used in the `register` macro:

  /// Enum representation of the GPU generation.
  #[derive(fmt::Debug, Default)]
  #[repr(u8)]
  pub(crate) enum Architecture {
      #[default]
      Turing = 0x16,
      Ampere = 0x17,
      Ada = 0x19,
  }

  impl From<Architecture> for u8 {
      fn from(value: Architecture) -> Self {
          // CAST: `Architecture` is `repr(u8)`, so this cast is always lossless.
          value as u8
      }
  }

These implementations were not needed for `NV_PMC_BOOT_0` because its
`architecture` method is manually implemented as it depends on two
distinct fields.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-29  3:03 ` [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs John Hubbard
  2025-10-29 11:26   ` Danilo Krummrich
  2025-10-29 14:05   ` Alexandre Courbot
@ 2025-10-29 15:02   ` Timur Tabi
  2025-10-29 23:15     ` John Hubbard
  2 siblings, 1 reply; 27+ messages in thread
From: Timur Tabi @ 2025-10-29 15:02 UTC (permalink / raw)
  To: dakr@kernel.org, John Hubbard
  Cc: Alexandre Courbot, lossin@kernel.org, a.hindborg@kernel.org,
	boqun.feng@gmail.com, aliceryhl@google.com, Zhi Wang,
	simona@ffwll.ch, alex.gaynor@gmail.com, ojeda@kernel.org,
	tmgross@umich.edu, nouveau@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	bjorn3_gh@protonmail.com, Edwin Peer, airlied@gmail.com,
	Joel Fernandes, bhelgaas@google.com, gary@garyguo.net,
	Alistair Popple

On Tue, 2025-10-28 at 20:03 -0700, John Hubbard wrote:
> NVIDIA GPUs are moving away from using NV_PMC_BOOT_0 to contain
> architecture and revision details, and will instead use NV_PMC_BOOT_42
> in the future. NV_PMC_BOOT_0 will be zeroed out.

I don't think this is correct.  ARCH_1 in PMC_BOOT_0 will be set to 1 in future GPUs.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-29 15:02   ` Timur Tabi
@ 2025-10-29 23:15     ` John Hubbard
  0 siblings, 0 replies; 27+ messages in thread
From: John Hubbard @ 2025-10-29 23:15 UTC (permalink / raw)
  To: Timur Tabi, dakr@kernel.org
  Cc: Alexandre Courbot, lossin@kernel.org, a.hindborg@kernel.org,
	boqun.feng@gmail.com, aliceryhl@google.com, Zhi Wang,
	simona@ffwll.ch, alex.gaynor@gmail.com, ojeda@kernel.org,
	tmgross@umich.edu, nouveau@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	bjorn3_gh@protonmail.com, Edwin Peer, airlied@gmail.com,
	Joel Fernandes, bhelgaas@google.com, gary@garyguo.net,
	Alistair Popple

On 10/29/25 8:02 AM, Timur Tabi wrote:
> On Tue, 2025-10-28 at 20:03 -0700, John Hubbard wrote:
>> NVIDIA GPUs are moving away from using NV_PMC_BOOT_0 to contain
>> architecture and revision details, and will instead use NV_PMC_BOOT_42
>> in the future. NV_PMC_BOOT_0 will be zeroed out.
> 
> I don't think this is correct.  ARCH_1 in PMC_BOOT_0 will be set to 1 in future GPUs.

ah right, thanks Timur! I can change it to:

NV_PMC_BOOT's ARCH_0 (bits 28:24) will be zeroed out, and ARCH_1 (bit 8:8)
will be set to 1, which means, "refer to NV_PMC_BOOT_42".


thanks,
-- 
John Hubbard


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-29 11:26   ` Danilo Krummrich
  2025-10-29 13:54     ` Alexandre Courbot
@ 2025-10-30  0:29     ` John Hubbard
  2025-10-30  0:31       ` Timur Tabi
  1 sibling, 1 reply; 27+ messages in thread
From: John Hubbard @ 2025-10-30  0:29 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Alexandre Courbot, Joel Fernandes, Timur Tabi, Alistair Popple,
	Edwin Peer, Zhi Wang, David Airlie, Simona Vetter, Bjorn Helgaas,
	Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, nouveau, rust-for-linux, LKML

On 10/29/25 4:26 AM, Danilo Krummrich wrote:
> On Wed Oct 29, 2025 at 4:03 AM CET, John Hubbard wrote:
...
>> +    fn try_from(boot0: regs::NV_PMC_BOOT_0) -> Result<Self> {
>> +        Ok(Self {
>> +            chipset: boot0.chipset()?,
>> +            revision: Revision {
>> +                major: boot0.major_revision(),
>> +                minor: boot0.minor_revision(),
>> +            },
> 
> Actually, would be nice to handle this just like chipset for consistency, i.e.
> 
> 	revision: boot0.revision()

Good idea, done.

...
>> +    fn try_from(boot42: regs::NV_PMC_BOOT_42) -> Result<Self> {
>> +        Ok(Self {
>> +            chipset: boot42.chipset()?,
>> +            revision: Revision {
>> +                major: boot42.major_revision(),
>> +                minor: boot42.minor_revision(),
>> +            },
> 
> Same here, could be
> 
> 	revision: boot42.revision()

Done. 

...> Without the comments this currently is:
> 
> 	let boot42 = if boot0.is_next_gen() {
> 	    Some(regs::NV_PMC_BOOT_42::read(bar))
> 	} else {
> 	    None
> 	};
> 	
> 	if boot0.is_nv04() {
> 	    Err(ENODEV)?
> 	}
> 	
> 	boot42
> 	    .map(Spec::try_from)
> 	    .unwrap_or_else(|| Spec::try_from(boot0))
> 
> Which I think is a bit heavy-handed. Let's simplify this a bit:
> 
> 	let boot0 = regs::NV_PMC_BOOT_0::read(bar);
> 
> 	if boot0.is_nv04() {
> 	    return Err(ENODEV);
> 	}
> 
> 	Spec::try_from(
> 	    if boot0.is_next_gen() {
> 	        regs::NV_PMC_BOOT_42::read(bar)
> 	    } else {
> 	        boot0
> 	    }
> 	)

That, combined with Timur's comment, made me realize that .is_next_gen()
can be made reliable enough that we don't even need is_nv04() at all.

I've shortened and fixed it up accordingly.

(I think I got a little too involved in the Nouveau-related discussions,
there: Nouveau has a different set of things to support, and to check
for.)


thanks,
-- 
John Hubbard


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-30  0:29     ` John Hubbard
@ 2025-10-30  0:31       ` Timur Tabi
  2025-10-30  0:35         ` John Hubbard
  0 siblings, 1 reply; 27+ messages in thread
From: Timur Tabi @ 2025-10-30  0:31 UTC (permalink / raw)
  To: dakr@kernel.org, John Hubbard
  Cc: Alexandre Courbot, lossin@kernel.org, a.hindborg@kernel.org,
	boqun.feng@gmail.com, aliceryhl@google.com, Zhi Wang,
	simona@ffwll.ch, alex.gaynor@gmail.com, ojeda@kernel.org,
	tmgross@umich.edu, nouveau@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	bjorn3_gh@protonmail.com, Edwin Peer, airlied@gmail.com,
	Joel Fernandes, bhelgaas@google.com, gary@garyguo.net,
	Alistair Popple

On Wed, 2025-10-29 at 17:29 -0700, John Hubbard wrote:
> That, combined with Timur's comment, made me realize that .is_next_gen()
> can be made reliable enough that we don't even need is_nv04() at all.

Please, not "next gen".  Technically speaking, BOOT42 is "current gen" and BOOT0 is "last gen".

And this all assumes that BOOT42 won't be replaced by something even bigger for
Blackwell+++++++++++.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-30  0:31       ` Timur Tabi
@ 2025-10-30  0:35         ` John Hubbard
  2025-10-30  1:01           ` Timur Tabi
  0 siblings, 1 reply; 27+ messages in thread
From: John Hubbard @ 2025-10-30  0:35 UTC (permalink / raw)
  To: Timur Tabi, dakr@kernel.org
  Cc: Alexandre Courbot, lossin@kernel.org, a.hindborg@kernel.org,
	boqun.feng@gmail.com, aliceryhl@google.com, Zhi Wang,
	simona@ffwll.ch, alex.gaynor@gmail.com, ojeda@kernel.org,
	tmgross@umich.edu, nouveau@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	bjorn3_gh@protonmail.com, Edwin Peer, airlied@gmail.com,
	Joel Fernandes, bhelgaas@google.com, gary@garyguo.net,
	Alistair Popple

On 10/29/25 5:31 PM, Timur Tabi wrote:
> On Wed, 2025-10-29 at 17:29 -0700, John Hubbard wrote:
>> That, combined with Timur's comment, made me realize that .is_next_gen()
>> can be made reliable enough that we don't even need is_nv04() at all.
> 
> Please, not "next gen".  Technically speaking, BOOT42 is "current gen" and BOOT0 is "last gen".
> 
> And this all assumes that BOOT42 won't be replaced by something even bigger for
> Blackwell+++++++++++.

OK, I think this wants to be called use_boot42_instead(), approximately.

...and I've fortified the code accordingly:

impl NV_PMC_BOOT_0 {
    pub(crate) fn use_boot42_instead(self) -> bool {
        // "Future" GPUs (some time after Rubin) will set `architecture_0`
        // to 0, and `architecture_1` to 1, and put the architecture details in
        // boot42 instead.
        self.architecture_0() == 0 && self.architecture_1() == 1
    }

thanks,
-- 
John Hubbard


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-29 13:54     ` Alexandre Courbot
@ 2025-10-30  0:37       ` John Hubbard
  2025-10-30  0:54         ` Alexandre Courbot
  0 siblings, 1 reply; 27+ messages in thread
From: John Hubbard @ 2025-10-30  0:37 UTC (permalink / raw)
  To: Alexandre Courbot, Danilo Krummrich
  Cc: Joel Fernandes, Timur Tabi, Alistair Popple, Edwin Peer, Zhi Wang,
	David Airlie, Simona Vetter, Bjorn Helgaas, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, nouveau,
	rust-for-linux, LKML, Nouveau

On 10/29/25 6:54 AM, Alexandre Courbot wrote:
> On Wed Oct 29, 2025 at 8:26 PM JST, Danilo Krummrich wrote:
> <snip>
...
> I don't think this will work because `NV_PMC_BOOT_0` and
> `NV_PMC_BOOT_42` are different types, so we cannot alternate them in the
> same call to `try_from`. But the following should:
> 
>     let boot0 = regs::NV_PMC_BOOT_0::read(bar);
>     ...
> 
>     if boot0.is_nv04() {
>         Err(ENODEV)?
>     }
> 
>     if boot0.is_next_gen() {
>         Spec::try_from(regs::NV_PMC_BOOT_42::read(bar))
>     } else {
>         Spec::try_from(boot0)
>     }
> 

Done. Final code snippet looks like this:

        let boot0 = regs::NV_PMC_BOOT_0::read(bar);

        if boot0.use_boot42_instead() {
            Spec::try_from(regs::NV_PMC_BOOT_42::read(bar))
        } else {
            Spec::try_from(boot0)
        }

thanks,
-- 
John Hubbard


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-30  0:37       ` John Hubbard
@ 2025-10-30  0:54         ` Alexandre Courbot
  2025-10-30  1:09           ` John Hubbard
  0 siblings, 1 reply; 27+ messages in thread
From: Alexandre Courbot @ 2025-10-30  0:54 UTC (permalink / raw)
  To: John Hubbard, Alexandre Courbot, Danilo Krummrich
  Cc: Joel Fernandes, Timur Tabi, Alistair Popple, Edwin Peer, Zhi Wang,
	David Airlie, Simona Vetter, Bjorn Helgaas, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, nouveau,
	rust-for-linux, LKML, Nouveau

On Thu Oct 30, 2025 at 9:37 AM JST, John Hubbard wrote:
> On 10/29/25 6:54 AM, Alexandre Courbot wrote:
>> On Wed Oct 29, 2025 at 8:26 PM JST, Danilo Krummrich wrote:
>> <snip>
> ...
>> I don't think this will work because `NV_PMC_BOOT_0` and
>> `NV_PMC_BOOT_42` are different types, so we cannot alternate them in the
>> same call to `try_from`. But the following should:
>> 
>>     let boot0 = regs::NV_PMC_BOOT_0::read(bar);
>>     ...
>> 
>>     if boot0.is_nv04() {
>>         Err(ENODEV)?
>>     }
>> 
>>     if boot0.is_next_gen() {
>>         Spec::try_from(regs::NV_PMC_BOOT_42::read(bar))
>>     } else {
>>         Spec::try_from(boot0)
>>     }
>> 
>
> Done. Final code snippet looks like this:
>
>         let boot0 = regs::NV_PMC_BOOT_0::read(bar);
>
>         if boot0.use_boot42_instead() {
>             Spec::try_from(regs::NV_PMC_BOOT_42::read(bar))
>         } else {
>             Spec::try_from(boot0)
>         }

The previous code was returning `ENODEV` in case of NV04, aren't we
losing this? Or is it moved to BOOT0's `try_from`?

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-30  0:35         ` John Hubbard
@ 2025-10-30  1:01           ` Timur Tabi
  2025-10-30  1:07             ` John Hubbard
  0 siblings, 1 reply; 27+ messages in thread
From: Timur Tabi @ 2025-10-30  1:01 UTC (permalink / raw)
  To: dakr@kernel.org, John Hubbard
  Cc: lossin@kernel.org, a.hindborg@kernel.org, boqun.feng@gmail.com,
	Zhi Wang, simona@ffwll.ch, tmgross@umich.edu,
	alex.gaynor@gmail.com, nouveau@lists.freedesktop.org,
	ojeda@kernel.org, linux-kernel@vger.kernel.org,
	rust-for-linux@vger.kernel.org, bjorn3_gh@protonmail.com,
	Edwin Peer, airlied@gmail.com, aliceryhl@google.com,
	bhelgaas@google.com, Joel Fernandes, Alexandre Courbot,
	gary@garyguo.net, Alistair Popple

On Wed, 2025-10-29 at 17:35 -0700, John Hubbard wrote:
>         // "Future" GPUs (some time after Rubin) will set `architecture_0`
>         // to 0, and `architecture_1` to 1, and put the architecture details in
>         // boot42 instead.

I don't want to kick a dead horse here, but aren't the architecture details already in boot42
for Turing?  I thought the whole point was that we don't need boot0 any more, and only Nouveau
has to worry about boot0 vs boot42.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-30  1:01           ` Timur Tabi
@ 2025-10-30  1:07             ` John Hubbard
  2025-10-30  1:13               ` John Hubbard
  2025-10-30  1:44               ` Timur Tabi
  0 siblings, 2 replies; 27+ messages in thread
From: John Hubbard @ 2025-10-30  1:07 UTC (permalink / raw)
  To: Timur Tabi, dakr@kernel.org
  Cc: lossin@kernel.org, a.hindborg@kernel.org, boqun.feng@gmail.com,
	Zhi Wang, simona@ffwll.ch, tmgross@umich.edu,
	alex.gaynor@gmail.com, nouveau@lists.freedesktop.org,
	ojeda@kernel.org, linux-kernel@vger.kernel.org,
	rust-for-linux@vger.kernel.org, bjorn3_gh@protonmail.com,
	Edwin Peer, airlied@gmail.com, aliceryhl@google.com,
	bhelgaas@google.com, Joel Fernandes, Alexandre Courbot,
	gary@garyguo.net, Alistair Popple

On 10/29/25 6:01 PM, Timur Tabi wrote:
> On Wed, 2025-10-29 at 17:35 -0700, John Hubbard wrote:
>>          // "Future" GPUs (some time after Rubin) will set `architecture_0`
>>          // to 0, and `architecture_1` to 1, and put the architecture details in
>>          // boot42 instead.
> 
> I don't want to kick a dead horse here, but aren't the architecture details already in boot42
> for Turing?  I thought the whole point was that we don't need boot0 any more, and only Nouveau
> has to worry about boot0 vs boot42.

Yes, but someone can still plug in a pre-Turing GPU and try to
boot up with nova-core.ko on the system.

So it's important to avoid getting into trouble in that case.

thanks,
John Hubbard

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-30  0:54         ` Alexandre Courbot
@ 2025-10-30  1:09           ` John Hubbard
  0 siblings, 0 replies; 27+ messages in thread
From: John Hubbard @ 2025-10-30  1:09 UTC (permalink / raw)
  To: Alexandre Courbot, Danilo Krummrich
  Cc: Joel Fernandes, Timur Tabi, Alistair Popple, Edwin Peer, Zhi Wang,
	David Airlie, Simona Vetter, Bjorn Helgaas, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, nouveau,
	rust-for-linux, LKML, Nouveau

On 10/29/25 5:54 PM, Alexandre Courbot wrote:
> On Thu Oct 30, 2025 at 9:37 AM JST, John Hubbard wrote:
>> On 10/29/25 6:54 AM, Alexandre Courbot wrote:
>>> On Wed Oct 29, 2025 at 8:26 PM JST, Danilo Krummrich wrote:
>>> <snip>
>> ...
>>
>> Done. Final code snippet looks like this:
>>
>>          let boot0 = regs::NV_PMC_BOOT_0::read(bar);
>>
>>          if boot0.use_boot42_instead() {
>>              Spec::try_from(regs::NV_PMC_BOOT_42::read(bar))
>>          } else {
>>              Spec::try_from(boot0)
>>          }
> 
> The previous code was returning `ENODEV` in case of NV04, aren't we
> losing this? Or is it moved to BOOT0's `try_from`?

Exactly, this is by intention now. We can fully determine the answers
via .use_boot42_instead(). The nv04 check is redundant.

thanks,
John Hubbard


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-30  1:07             ` John Hubbard
@ 2025-10-30  1:13               ` John Hubbard
  2025-10-30  1:44               ` Timur Tabi
  1 sibling, 0 replies; 27+ messages in thread
From: John Hubbard @ 2025-10-30  1:13 UTC (permalink / raw)
  To: Timur Tabi, dakr@kernel.org
  Cc: lossin@kernel.org, a.hindborg@kernel.org, boqun.feng@gmail.com,
	Zhi Wang, simona@ffwll.ch, tmgross@umich.edu,
	alex.gaynor@gmail.com, nouveau@lists.freedesktop.org,
	ojeda@kernel.org, linux-kernel@vger.kernel.org,
	rust-for-linux@vger.kernel.org, bjorn3_gh@protonmail.com,
	Edwin Peer, airlied@gmail.com, aliceryhl@google.com,
	bhelgaas@google.com, Joel Fernandes, Alexandre Courbot,
	gary@garyguo.net, Alistair Popple

On 10/29/25 6:07 PM, John Hubbard wrote:
> On 10/29/25 6:01 PM, Timur Tabi wrote:
>> On Wed, 2025-10-29 at 17:35 -0700, John Hubbard wrote:
>>>          // "Future" GPUs (some time after Rubin) will set 
>>> `architecture_0`
>>>          // to 0, and `architecture_1` to 1, and put the architecture 
>>> details in
>>>          // boot42 instead.
>>
>> I don't want to kick a dead horse here, but aren't the architecture 
>> details already in boot42
>> for Turing?  I thought the whole point was that we don't need boot0 
>> any more, and only Nouveau
>> has to worry about boot0 vs boot42.
> 
> Yes, but someone can still plug in a pre-Turing GPU and try to
> boot up with nova-core.ko on the system.
> 
> So it's important to avoid getting into trouble in that case.

In fact, here's what I have staged for the next posting:

impl Spec { 

     fn new(bar: &Bar0) -> Result<Spec> { 

         // Some brief notes about boot0 and boot42, in chronological 
order:
         // 

         // NV04 through Volta: 

         // 

         //    Not supported by Nova. boot0 is necessary and sufficient 
to identify these GPUs.
         //    boot42 may not even exist on some of these GPUs.boot42 

         // 

         // Turing through Blackwell: 

         // 

         //     Supported by both Nouveau and Nova. boot0 is still 
necessary and sufficient to
         //     identify these GPUs. boot42 exists on these GPUs but we 
don't need to use it.
         // 

         // Rubin: 

         // 

         //     Only supported by Nova. Need to use boot42 to fully 
identify these GPUs.
         // 

         // "Future" (after Rubin) GPUs: 

         // 

         //    Only supported by Nova. NV_PMC_BOOT's ARCH_0 (bits 28:24) 
will be zeroed out, and
         //    ARCH_1 (bit 8:8) will be set to 1, which will mean, 
"refer to NV_PMC_BOOT_42".
  

         let boot0 = regs::NV_PMC_BOOT_0::read(bar); 

  

         if boot0.use_boot42_instead() { 

             Spec::try_from(regs::NV_PMC_BOOT_42::read(bar)) 

         } else { 

             Spec::try_from(boot0) 

         } 

     } 

}


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-30  1:07             ` John Hubbard
  2025-10-30  1:13               ` John Hubbard
@ 2025-10-30  1:44               ` Timur Tabi
  2025-10-30  5:30                 ` John Hubbard
  1 sibling, 1 reply; 27+ messages in thread
From: Timur Tabi @ 2025-10-30  1:44 UTC (permalink / raw)
  To: dakr@kernel.org, John Hubbard
  Cc: lossin@kernel.org, a.hindborg@kernel.org, boqun.feng@gmail.com,
	Zhi Wang, simona@ffwll.ch, tmgross@umich.edu,
	alex.gaynor@gmail.com, linux-kernel@vger.kernel.org,
	ojeda@kernel.org, nouveau@lists.freedesktop.org,
	rust-for-linux@vger.kernel.org, bjorn3_gh@protonmail.com,
	Edwin Peer, airlied@gmail.com, aliceryhl@google.com,
	bhelgaas@google.com, Joel Fernandes, Alexandre Courbot,
	gary@garyguo.net, Alistair Popple

On Wed, 2025-10-29 at 18:07 -0700, John Hubbard wrote:
> Yes, but someone can still plug in a pre-Turing GPU and try to
> boot up with nova-core.ko on the system.
> 
> So it's important to avoid getting into trouble in that case.

Sure, but I think we don't want any real code that looks at boot0.  Nova should really just look
at boot42 to determine any architecture.  So what we really want is to avoid accidentally
reading boot42 on GPUs where it doesn't exist.  I believe that the oldest GPU that supports
boot42 is Fermi GF100, or chipset id 0x0c0.  So we can do something like this:

fn is_gpu_ancient(bar: &Bar0) -> bool {
    let boot0 = regs::NV_PMC_BOOT_0::read(bar);

    if boot0.architecture_1() == 0 && boot0.architecture_0() < 0x0c0 {
        return true;
    } else {
        return false;
}

At the beginning of probe(), if is_gpu_ancient() returns true, just exit immediately.  Then the
rest of the driver can happily ignore boot0 completely and use boot42 for everything.

Just my two cents.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-30  1:44               ` Timur Tabi
@ 2025-10-30  5:30                 ` John Hubbard
  2025-10-30 14:22                   ` Timur Tabi
  0 siblings, 1 reply; 27+ messages in thread
From: John Hubbard @ 2025-10-30  5:30 UTC (permalink / raw)
  To: Timur Tabi, dakr@kernel.org
  Cc: lossin@kernel.org, a.hindborg@kernel.org, boqun.feng@gmail.com,
	Zhi Wang, simona@ffwll.ch, tmgross@umich.edu,
	alex.gaynor@gmail.com, linux-kernel@vger.kernel.org,
	ojeda@kernel.org, nouveau@lists.freedesktop.org,
	rust-for-linux@vger.kernel.org, bjorn3_gh@protonmail.com,
	Edwin Peer, airlied@gmail.com, aliceryhl@google.com,
	bhelgaas@google.com, Joel Fernandes, Alexandre Courbot,
	gary@garyguo.net, Alistair Popple

On 10/29/25 6:44 PM, Timur Tabi wrote:
> On Wed, 2025-10-29 at 18:07 -0700, John Hubbard wrote:
>> Yes, but someone can still plug in a pre-Turing GPU and try to
>> boot up with nova-core.ko on the system.
>>
>> So it's important to avoid getting into trouble in that case.
> 
> Sure, but I think we don't want any real code that looks at boot0.  Nova should really just look
> at boot42 to determine any architecture.  So what we really want is to avoid accidentally

Oh I really understand your sentiment here. I'm all about, "use the new
hardware and never look back". :)

However, I don't want anyone to have to risk reading boot42 on some
ancient GPU (earlier than Fermi, even), with uncertain results.

And our HW team has promised to leave behind arch0==0, arch1==1 in
in boot0, more or less forever, specifically to help us out here.

With that in mind, I *do* want to read boot0 for the forseeable future,
as a guide to whether to look at boot42. I really think that is the
way to thread the needle here.


thanks,
-- 
John Hubbard


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-30  5:30                 ` John Hubbard
@ 2025-10-30 14:22                   ` Timur Tabi
  2025-10-30 14:45                     ` Danilo Krummrich
  0 siblings, 1 reply; 27+ messages in thread
From: Timur Tabi @ 2025-10-30 14:22 UTC (permalink / raw)
  To: dakr@kernel.org, John Hubbard
  Cc: lossin@kernel.org, a.hindborg@kernel.org, boqun.feng@gmail.com,
	Zhi Wang, simona@ffwll.ch, tmgross@umich.edu,
	alex.gaynor@gmail.com, nouveau@lists.freedesktop.org,
	ojeda@kernel.org, linux-kernel@vger.kernel.org,
	rust-for-linux@vger.kernel.org, bjorn3_gh@protonmail.com,
	Edwin Peer, airlied@gmail.com, aliceryhl@google.com,
	bhelgaas@google.com, Joel Fernandes, Alexandre Courbot,
	gary@garyguo.net, Alistair Popple

On Wed, 2025-10-29 at 22:30 -0700, John Hubbard wrote:
> However, I don't want anyone to have to risk reading boot42 on some
> ancient GPU (earlier than Fermi, even), with uncertain results.
> 
> And our HW team has promised to leave behind arch0==0, arch1==1 in
> in boot0, more or less forever, specifically to help us out here.
> 
> With that in mind, I *do* want to read boot0 for the forseeable future,
> as a guide to whether to look at boot42. I really think that is the
> way to thread the needle here.

Sure, and this is exactly what is_ancient_gpu() does.  It tells you whether to use boot42
instead of boot0.  It is the only function that looks at boot0.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-30 14:22                   ` Timur Tabi
@ 2025-10-30 14:45                     ` Danilo Krummrich
  0 siblings, 0 replies; 27+ messages in thread
From: Danilo Krummrich @ 2025-10-30 14:45 UTC (permalink / raw)
  To: Timur Tabi
  Cc: John Hubbard, lossin@kernel.org, a.hindborg@kernel.org,
	boqun.feng@gmail.com, Zhi Wang, simona@ffwll.ch,
	tmgross@umich.edu, alex.gaynor@gmail.com,
	nouveau@lists.freedesktop.org, ojeda@kernel.org,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	bjorn3_gh@protonmail.com, Edwin Peer, airlied@gmail.com,
	aliceryhl@google.com, bhelgaas@google.com, Joel Fernandes,
	Alexandre Courbot, gary@garyguo.net, Alistair Popple

On 10/30/25 3:22 PM, Timur Tabi wrote:
> On Wed, 2025-10-29 at 22:30 -0700, John Hubbard wrote:
>> However, I don't want anyone to have to risk reading boot42 on some
>> ancient GPU (earlier than Fermi, even), with uncertain results.
>>
>> And our HW team has promised to leave behind arch0==0, arch1==1 in
>> in boot0, more or less forever, specifically to help us out here.
>>
>> With that in mind, I *do* want to read boot0 for the forseeable future,
>> as a guide to whether to look at boot42. I really think that is the
>> way to thread the needle here.
> 
> Sure, and this is exactly what is_ancient_gpu() does.  It tells you whether to use boot42
> instead of boot0.  It is the only function that looks at boot0.

I think you're indeed talking about the same thing, but thinking differently
about the implementation details.

A standalone is_ancient_gpu() function called from probe() like

	if is_ancient_gpu(bar) {
		return Err(ENODEV);
	}

is what we would probably do in C, but in Rust we should just call

	let spec = Spec::new()?;

from probe() and Spec::new() will return Err(ENODEV) when it run into an ancient
GPU spec internally.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-29 14:05   ` Alexandre Courbot
@ 2025-10-31  0:04     ` John Hubbard
  2025-11-02  0:34     ` John Hubbard
  1 sibling, 0 replies; 27+ messages in thread
From: John Hubbard @ 2025-10-31  0:04 UTC (permalink / raw)
  To: Alexandre Courbot, Danilo Krummrich
  Cc: Joel Fernandes, Timur Tabi, Alistair Popple, Edwin Peer, Zhi Wang,
	David Airlie, Simona Vetter, Bjorn Helgaas, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, nouveau,
	rust-for-linux, LKML, Nouveau

On 10/29/25 7:05 AM, Alexandre Courbot wrote:
> On Wed Oct 29, 2025 at 12:03 PM JST, John Hubbard wrote:
> <snip>
> Here you can directly return the `Architecture` type and spare callers
> the labor of doing the conversion themselves:

Hi Alex,

I will attempt this, after updating my brain with a few missing macro-
related skills. haha :)


thanks,
-- 
John Hubbard


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-10-29 14:05   ` Alexandre Courbot
  2025-10-31  0:04     ` John Hubbard
@ 2025-11-02  0:34     ` John Hubbard
  2025-11-02  2:41       ` Alexandre Courbot
  1 sibling, 1 reply; 27+ messages in thread
From: John Hubbard @ 2025-11-02  0:34 UTC (permalink / raw)
  To: Alexandre Courbot, Danilo Krummrich
  Cc: Joel Fernandes, Timur Tabi, Alistair Popple, Edwin Peer, Zhi Wang,
	David Airlie, Simona Vetter, Bjorn Helgaas, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, nouveau,
	rust-for-linux, LKML, Nouveau

On 10/29/25 7:05 AM, Alexandre Courbot wrote:
> On Wed Oct 29, 2025 at 12:03 PM JST, John Hubbard wrote:
> <snip>
> This allows the implementation of `NV_PMC_BOOT_42` to mirror that of
> `NV_PMC_BOOT_0`:
> 
>   impl NV_PMC_BOOT_42 {
>       pub(crate) fn chipset(self) -> Result<Chipset> {
>           self.architecture()
>               .map(|arch| {
>                   ((arch as u32) << Self::IMPLEMENTATION_RANGE.len())

A quick note: _RANGE() and related functions are (I think?) deeply,
madly undocumented. Not only is bitfield a macro within a macro, but
bitfield itself leaves the user with only the following as "documentation":

    ::kernel::macros::paste!(
    const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;

The net result is that those of us who did not author or carefully
review register!() and bitfield!() are going to have a rough time
using these facilities.

I'm not sure of the best way to add documentation here, but just
thought I'd better give an early warning about this.


thanks,
-- 
John Hubbard


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-11-02  0:34     ` John Hubbard
@ 2025-11-02  2:41       ` Alexandre Courbot
  2025-11-02  3:33         ` John Hubbard
  0 siblings, 1 reply; 27+ messages in thread
From: Alexandre Courbot @ 2025-11-02  2:41 UTC (permalink / raw)
  To: John Hubbard, Alexandre Courbot, Danilo Krummrich
  Cc: Joel Fernandes, Timur Tabi, Alistair Popple, Edwin Peer, Zhi Wang,
	David Airlie, Simona Vetter, Bjorn Helgaas, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, nouveau,
	rust-for-linux, LKML, Nouveau

On Sun Nov 2, 2025 at 9:34 AM JST, John Hubbard wrote:
> On 10/29/25 7:05 AM, Alexandre Courbot wrote:
>> On Wed Oct 29, 2025 at 12:03 PM JST, John Hubbard wrote:
>> <snip>
>> This allows the implementation of `NV_PMC_BOOT_42` to mirror that of
>> `NV_PMC_BOOT_0`:
>> 
>>   impl NV_PMC_BOOT_42 {
>>       pub(crate) fn chipset(self) -> Result<Chipset> {
>>           self.architecture()
>>               .map(|arch| {
>>                   ((arch as u32) << Self::IMPLEMENTATION_RANGE.len())
>
> A quick note: _RANGE() and related functions are (I think?) deeply,
> madly undocumented. Not only is bitfield a macro within a macro, but
> bitfield itself leaves the user with only the following as "documentation":
>
>     ::kernel::macros::paste!(
>     const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
>
> The net result is that those of us who did not author or carefully
> review register!() and bitfield!() are going to have a rough time
> using these facilities.
>
> I'm not sure of the best way to add documentation here, but just
> thought I'd better give an early warning about this.

We can always add doccomments in the macro, as in the patch below. These
will be displayed by LSP when one highlights or tries to use one of
these constants.

If you think that's adequate, I will send a patch.

--- a/drivers/gpu/nova-core/bitfield.rs
+++ b/drivers/gpu/nova-core/bitfield.rs
@@ -249,7 +249,10 @@ impl $name {
             { $process:expr } $prim_type:tt $to_type:ty => $res_type:ty $(, $comment:literal)?;
     ) => {
         ::kernel::macros::paste!(
+        /// Inclusive range of the bits covered by this field.
         const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
+
+        /// Mask of the bits making up this field.
         const [<$field:upper _MASK>]: $storage = {
             // Generate mask for shifting
             match ::core::mem::size_of::<$storage>() {
@@ -260,6 +263,8 @@ impl $name {
                 _ => ::kernel::build_error!("Unsupported storage type size")
             }
         };
+
+        /// Shift to apply to a value to align it with the start of this field.
         const [<$field:upper _SHIFT>]: u32 = $lo;
         );

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-11-02  2:41       ` Alexandre Courbot
@ 2025-11-02  3:33         ` John Hubbard
  2025-11-08  1:55           ` Alexandre Courbot
  0 siblings, 1 reply; 27+ messages in thread
From: John Hubbard @ 2025-11-02  3:33 UTC (permalink / raw)
  To: Alexandre Courbot, Danilo Krummrich
  Cc: Joel Fernandes, Timur Tabi, Alistair Popple, Edwin Peer, Zhi Wang,
	David Airlie, Simona Vetter, Bjorn Helgaas, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, nouveau,
	rust-for-linux, LKML, Nouveau

On 11/1/25 7:41 PM, Alexandre Courbot wrote:
> On Sun Nov 2, 2025 at 9:34 AM JST, John Hubbard wrote:
>> On 10/29/25 7:05 AM, Alexandre Courbot wrote:
...

> We can always add doccomments in the macro, as in the patch below. These
> will be displayed by LSP when one highlights or tries to use one of
> these constants.
> 
> If you think that's adequate, I will send a patch.
> 
> --- a/drivers/gpu/nova-core/bitfield.rs
> +++ b/drivers/gpu/nova-core/bitfield.rs
> @@ -249,7 +249,10 @@ impl $name {
>               { $process:expr } $prim_type:tt $to_type:ty => $res_type:ty $(, $comment:literal)?;
>       ) => {
>           ::kernel::macros::paste!(
> +        /// Inclusive range of the bits covered by this field.
>           const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;

Will that let people know that they'll see something like
IMPLEMENTATION_RANGE() for a corresponding .implementation field?

I'm hoping we can somehow create clear and plain documentation for
the various functions that the macro generates.

thanks,
-- 
John Hubbard

> +
> +        /// Mask of the bits making up this field.
>           const [<$field:upper _MASK>]: $storage = {
>               // Generate mask for shifting
>               match ::core::mem::size_of::<$storage>() {
> @@ -260,6 +263,8 @@ impl $name {
>                   _ => ::kernel::build_error!("Unsupported storage type size")
>               }
>           };
> +
> +        /// Shift to apply to a value to align it with the start of this field.
>           const [<$field:upper _SHIFT>]: u32 = $lo;
>           );



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-11-02  3:33         ` John Hubbard
@ 2025-11-08  1:55           ` Alexandre Courbot
  2025-11-08  2:03             ` John Hubbard
  0 siblings, 1 reply; 27+ messages in thread
From: Alexandre Courbot @ 2025-11-08  1:55 UTC (permalink / raw)
  To: John Hubbard, Alexandre Courbot, Danilo Krummrich
  Cc: Joel Fernandes, Timur Tabi, Alistair Popple, Edwin Peer, Zhi Wang,
	David Airlie, Simona Vetter, Bjorn Helgaas, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, nouveau,
	rust-for-linux, LKML, Nouveau

On Sun Nov 2, 2025 at 12:33 PM JST, John Hubbard wrote:
> On 11/1/25 7:41 PM, Alexandre Courbot wrote:
>> On Sun Nov 2, 2025 at 9:34 AM JST, John Hubbard wrote:
>>> On 10/29/25 7:05 AM, Alexandre Courbot wrote:
> ...
>
>> We can always add doccomments in the macro, as in the patch below. These
>> will be displayed by LSP when one highlights or tries to use one of
>> these constants.
>> 
>> If you think that's adequate, I will send a patch.
>> 
>> --- a/drivers/gpu/nova-core/bitfield.rs
>> +++ b/drivers/gpu/nova-core/bitfield.rs
>> @@ -249,7 +249,10 @@ impl $name {
>>               { $process:expr } $prim_type:tt $to_type:ty => $res_type:ty $(, $comment:literal)?;
>>       ) => {
>>           ::kernel::macros::paste!(
>> +        /// Inclusive range of the bits covered by this field.
>>           const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
>
> Will that let people know that they'll see something like
> IMPLEMENTATION_RANGE() for a corresponding .implementation field?
>
> I'm hoping we can somehow create clear and plain documentation for
> the various functions that the macro generates.

I did try to generate better documentation for these using the `#doc`
directive, actually posted the patch by mistake so I might as well link
to it:

https://lore.kernel.org/rust-for-linux/20251108-gsp_boot-v8-16-70b762eedd50@nvidia.com/

Unfortunately, the final documentation does not appear with
rust-analyzer/LSP, which drastically limits its usefulness. :/

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
  2025-11-08  1:55           ` Alexandre Courbot
@ 2025-11-08  2:03             ` John Hubbard
  0 siblings, 0 replies; 27+ messages in thread
From: John Hubbard @ 2025-11-08  2:03 UTC (permalink / raw)
  To: Alexandre Courbot, Danilo Krummrich
  Cc: Joel Fernandes, Timur Tabi, Alistair Popple, Edwin Peer, Zhi Wang,
	David Airlie, Simona Vetter, Bjorn Helgaas, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross, nouveau,
	rust-for-linux, LKML, Nouveau

On 11/7/25 5:55 PM, Alexandre Courbot wrote:
> On Sun Nov 2, 2025 at 12:33 PM JST, John Hubbard wrote:
>> On 11/1/25 7:41 PM, Alexandre Courbot wrote:
>>> On Sun Nov 2, 2025 at 9:34 AM JST, John Hubbard wrote:
>>>> On 10/29/25 7:05 AM, Alexandre Courbot wrote:
>> ...
>>
>>> We can always add doccomments in the macro, as in the patch below. These
>>> will be displayed by LSP when one highlights or tries to use one of
>>> these constants.
>>>
>>> If you think that's adequate, I will send a patch.
>>>
>>> --- a/drivers/gpu/nova-core/bitfield.rs
>>> +++ b/drivers/gpu/nova-core/bitfield.rs
>>> @@ -249,7 +249,10 @@ impl $name {
>>>               { $process:expr } $prim_type:tt $to_type:ty => $res_type:ty $(, $comment:literal)?;
>>>       ) => {
>>>           ::kernel::macros::paste!(
>>> +        /// Inclusive range of the bits covered by this field.
>>>           const [<$field:upper _RANGE>]: ::core::ops::RangeInclusive<u8> = $lo..=$hi;
>>
>> Will that let people know that they'll see something like
>> IMPLEMENTATION_RANGE() for a corresponding .implementation field?
>>
>> I'm hoping we can somehow create clear and plain documentation for
>> the various functions that the macro generates.
> 
> I did try to generate better documentation for these using the `#doc`
> directive, actually posted the patch by mistake so I might as well link
> to it:
> 
> https://lore.kernel.org/rust-for-linux/20251108-gsp_boot-v8-16-70b762eedd50@nvidia.com/
> 
> Unfortunately, the final documentation does not appear with
> rust-analyzer/LSP, which drastically limits its usefulness. :/

Thanks for trying that out. I'm starting to believe that in 2025,
we maybe just need to fall back to writing comments directly near
the macro implementation code, and that way there is something
for people to read.

That would still be a significant improvement over visually parsing
the macro implementation, in order to deduce the new function names
that are being pasted together. I think...


thanks,
-- 
John Hubbard


^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2025-11-08  2:04 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-29  3:03 [PATCH v3 0/2] gpu: nova: add boot42 support for next-gen GPUs John Hubbard
2025-10-29  3:03 ` [PATCH v3 1/2] gpu: nova-core: prepare Spec and Revision types for boot0/boot42 John Hubbard
2025-10-29  3:03 ` [PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs John Hubbard
2025-10-29 11:26   ` Danilo Krummrich
2025-10-29 13:54     ` Alexandre Courbot
2025-10-30  0:37       ` John Hubbard
2025-10-30  0:54         ` Alexandre Courbot
2025-10-30  1:09           ` John Hubbard
2025-10-30  0:29     ` John Hubbard
2025-10-30  0:31       ` Timur Tabi
2025-10-30  0:35         ` John Hubbard
2025-10-30  1:01           ` Timur Tabi
2025-10-30  1:07             ` John Hubbard
2025-10-30  1:13               ` John Hubbard
2025-10-30  1:44               ` Timur Tabi
2025-10-30  5:30                 ` John Hubbard
2025-10-30 14:22                   ` Timur Tabi
2025-10-30 14:45                     ` Danilo Krummrich
2025-10-29 14:05   ` Alexandre Courbot
2025-10-31  0:04     ` John Hubbard
2025-11-02  0:34     ` John Hubbard
2025-11-02  2:41       ` Alexandre Courbot
2025-11-02  3:33         ` John Hubbard
2025-11-08  1:55           ` Alexandre Courbot
2025-11-08  2:03             ` John Hubbard
2025-10-29 15:02   ` Timur Tabi
2025-10-29 23:15     ` John Hubbard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).