* [PATCH v4 0/3] gpu: nova: add boot42 support for next-gen GPUs
@ 2025-11-02 1:36 John Hubbard
2025-11-02 1:36 ` [PATCH v4 1/3] gpu: nova-core: prepare Spec and Revision types for boot0/boot42 John Hubbard
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: John Hubbard @ 2025-11-02 1:36 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 v4:
1) Simplified and improved the decision logic: reads both arch_0 and
arch_1 fields in boot0, and skips the unnecessary is_nv04() logic as
well. Thanks to Timur Tabi and Danilo for noticing these issues.
2) Added a patch to represent Architecture as a u8. This simplifies a
few things. (Thanks to Alex Courbot. I added your Suggested-by to that
patch.)
3) Enhanced the Revision type to do more, which simplifies the callers.
(Thanks to Danilo.)
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 (3):
gpu: nova-core: prepare Spec and Revision types for boot0/boot42
gpu: nova-core: make Architecture behave as a u8 type
gpu: nova-core: add boot42 support for next-gen GPUs
drivers/gpu/nova-core/gpu.rs | 92 +++++++++++++++++++++++++++--------
drivers/gpu/nova-core/regs.rs | 41 ++++++++++++++++
2 files changed, 112 insertions(+), 21 deletions(-)
base-commit: 9a3c2f8a4f84960a48c056d0da88de3d09e6d622
--
2.51.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 1/3] gpu: nova-core: prepare Spec and Revision types for boot0/boot42
2025-11-02 1:36 [PATCH v4 0/3] gpu: nova: add boot42 support for next-gen GPUs John Hubbard
@ 2025-11-02 1:36 ` John Hubbard
2025-11-02 1:36 ` [PATCH v4 2/3] gpu: nova-core: make Architecture behave as a u8 type John Hubbard
2025-11-02 1:36 ` [PATCH v4 3/3] gpu: nova-core: add boot42 support for next-gen GPUs John Hubbard
2 siblings, 0 replies; 7+ messages in thread
From: John Hubbard @ 2025-11-02 1:36 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) Enhance Revision, which in turn simplifies Spec::new().
4) 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 | 45 +++++++++++++++++++----------------
drivers/gpu/nova-core/regs.rs | 8 +++++++
2 files changed, 33 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 9d182bffe8b4..8173cdcd8378 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -130,16 +130,18 @@ fn try_from(value: u8) -> Result<Self> {
}
pub(crate) struct Revision {
- major: u8,
- minor: u8,
+ pub(crate) major: u8,
+ pub(crate) minor: u8,
}
-impl Revision {
- fn from_boot0(boot0: regs::NV_PMC_BOOT_0) -> Self {
- Self {
- major: boot0.major_revision(),
- minor: boot0.minor_revision(),
- }
+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: boot0.revision(),
+ })
}
}
@@ -149,7 +151,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
}
}
-/// 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.
@@ -160,10 +162,19 @@ impl Spec {
fn new(bar: &Bar0) -> Result<Spec> {
let boot0 = regs::NV_PMC_BOOT_0::read(bar);
- Ok(Self {
- chipset: boot0.chipset()?,
- revision: Revision::from_boot0(boot0),
- })
+ Spec::try_from(boot0)
+ }
+}
+
+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
+ )
}
}
@@ -193,13 +204,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.
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 206dab2e1335..207b865335af 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -41,6 +41,14 @@ pub(crate) fn chipset(self) -> Result<Chipset> {
})
.and_then(Chipset::try_from)
}
+
+ /// Returns the revision information of the chip.
+ pub(crate) fn revision(self) -> crate::gpu::Revision {
+ crate::gpu::Revision {
+ major: self.major_revision(),
+ minor: self.minor_revision(),
+ }
+ }
}
// PBUS
--
2.51.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4 2/3] gpu: nova-core: make Architecture behave as a u8 type
2025-11-02 1:36 [PATCH v4 0/3] gpu: nova: add boot42 support for next-gen GPUs John Hubbard
2025-11-02 1:36 ` [PATCH v4 1/3] gpu: nova-core: prepare Spec and Revision types for boot0/boot42 John Hubbard
@ 2025-11-02 1:36 ` John Hubbard
2025-11-02 1:36 ` [PATCH v4 3/3] gpu: nova-core: add boot42 support for next-gen GPUs John Hubbard
2 siblings, 0 replies; 7+ messages in thread
From: John Hubbard @ 2025-11-02 1:36 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
This allows Architecture to be passed into register!() and bitfield!()
macro calls. That in turn requires a default implementation for
Architecture.
This simplifies transforming BOOT0 (and later, BOOT42) register values
into GPU architectures.
Suggested-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
drivers/gpu/nova-core/gpu.rs | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 8173cdcd8378..27b8926977da 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -109,8 +109,10 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
}
/// Enum representation of the GPU generation.
-#[derive(fmt::Debug)]
+#[derive(fmt::Debug, Default)]
+#[repr(u8)]
pub(crate) enum Architecture {
+ #[default]
Turing = 0x16,
Ampere = 0x17,
Ada = 0x19,
@@ -129,6 +131,13 @@ fn try_from(value: u8) -> Result<Self> {
}
}
+impl From<Architecture> for u8 {
+ fn from(value: Architecture) -> Self {
+ // CAST: `Architecture` is `repr(u8)`, so this cast is always lossless.
+ value as u8
+ }
+}
+
pub(crate) struct Revision {
pub(crate) major: u8,
pub(crate) minor: u8,
--
2.51.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4 3/3] gpu: nova-core: add boot42 support for next-gen GPUs
2025-11-02 1:36 [PATCH v4 0/3] gpu: nova: add boot42 support for next-gen GPUs John Hubbard
2025-11-02 1:36 ` [PATCH v4 1/3] gpu: nova-core: prepare Spec and Revision types for boot0/boot42 John Hubbard
2025-11-02 1:36 ` [PATCH v4 2/3] gpu: nova-core: make Architecture behave as a u8 type John Hubbard
@ 2025-11-02 1:36 ` John Hubbard
2025-11-02 18:14 ` Timur Tabi
2 siblings, 1 reply; 7+ messages in thread
From: John Hubbard @ 2025-11-02 1:36 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 | 38 ++++++++++++++++++++++++++++++++++-
drivers/gpu/nova-core/regs.rs | 33 ++++++++++++++++++++++++++++++
2 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 27b8926977da..33d96eb1d887 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -154,6 +154,17 @@ fn try_from(boot0: regs::NV_PMC_BOOT_0) -> Result<Self> {
}
}
+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: boot42.revision(),
+ })
+ }
+}
+
impl fmt::Display for Revision {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{:x}.{:x}", self.major, self.minor)
@@ -169,9 +180,34 @@ pub(crate) struct Spec {
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);
- Spec::try_from(boot0)
+ if boot0.use_boot42_instead() {
+ Spec::try_from(regs::NV_PMC_BOOT_42::read(bar))
+ } else {
+ Spec::try_from(boot0)
+ }
}
}
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index 207b865335af..8b5ff3858210 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -25,6 +25,13 @@
});
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
+ }
+
/// Combines `architecture_0` and `architecture_1` to obtain the architecture of the chip.
pub(crate) fn architecture(self) -> Result<Architecture> {
Architecture::try_from(
@@ -51,6 +58,32 @@ pub(crate) fn revision(self) -> crate::gpu::Revision {
}
}
+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, "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> {
+ self.architecture()
+ .map(|arch| {
+ ((arch as u32) << Self::IMPLEMENTATION_RANGE.len())
+ | u32::from(self.implementation())
+ })
+ .and_then(Chipset::try_from)
+ }
+
+ /// Returns the revision information of the chip.
+ pub(crate) fn revision(self) -> crate::gpu::Revision {
+ crate::gpu::Revision {
+ major: self.major_revision(),
+ minor: self.minor_revision(),
+ }
+ }
+}
+
// PBUS
register!(NV_PBUS_SW_SCRATCH @ 0x00001400[64] {});
--
2.51.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4 3/3] gpu: nova-core: add boot42 support for next-gen GPUs
2025-11-02 1:36 ` [PATCH v4 3/3] gpu: nova-core: add boot42 support for next-gen GPUs John Hubbard
@ 2025-11-02 18:14 ` Timur Tabi
2025-11-03 1:04 ` John Hubbard
0 siblings, 1 reply; 7+ messages in thread
From: Timur Tabi @ 2025-11-02 18:14 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 Sat, 2025-11-01 at 18:36 -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.
You missed this one. Boot0 will not be completely zeroed out.
>
>
> +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: boot42.revision(),
> + })
> + }
> +}
> +
> impl fmt::Display for Revision {
> fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> write!(f, "{:x}.{:x}", self.major, self.minor)
> @@ -169,9 +180,34 @@ pub(crate) struct Spec {
>
> 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
Did you intend to write more than just "boot42" at the end here?
> + //
> + // 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);
>
> - Spec::try_from(boot0)
> + if boot0.use_boot42_instead() {
> + Spec::try_from(regs::NV_PMC_BOOT_42::read(bar))
> + } else {
> + Spec::try_from(boot0)
> + }
> }
> }
>
> diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
> index 207b865335af..8b5ff3858210 100644
> --- a/drivers/gpu/nova-core/regs.rs
> +++ b/drivers/gpu/nova-core/regs.rs
> @@ -25,6 +25,13 @@
> });
>
> 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
> + }
So this was the crux of my initial objection, and I just don't think this is truly "forward
looking". The code is using boot42 only if boot0 is "zeroed out". So sometimes Nova will use
boot0 and sometimes it will use boot42, depending on the GPU. It's this inconsistency that
bothers me.
Instead, I think Nova should use only boot42, so that we have consistent information across all
GPUs. boot0 should only be used to avoid accidentally reading boot42 when it doesn't exist.
Previously, Danilo said this:
> 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.
This I agree with. The first thing that Spec::new() should do is check whether we're on an
ancient GPU that does not even have boot42. If so, return Err(ENODEV). Otherwise, from that
point onward, no code will ever look at boot0 again. boot0 should never be used to return the
actual architecture/gpu information.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 3/3] gpu: nova-core: add boot42 support for next-gen GPUs
2025-11-02 18:14 ` Timur Tabi
@ 2025-11-03 1:04 ` John Hubbard
2025-11-03 4:59 ` Timur Tabi
0 siblings, 1 reply; 7+ messages in thread
From: John Hubbard @ 2025-11-03 1:04 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 11/2/25 10:14 AM, Timur Tabi wrote:
> On Sat, 2025-11-01 at 18:36 -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.
>
> You missed this one. Boot0 will not be completely zeroed out.
>
Thanks for catching that, I'll write it like the other case.
>>
>>
>> +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: boot42.revision(),
>> + })
>> + }
>> +}
>> +
>> impl fmt::Display for Revision {
>> fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>> write!(f, "{:x}.{:x}", self.major, self.minor)
>> @@ -169,9 +180,34 @@ pub(crate) struct Spec {
>>
>> 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
>
> Did you intend to write more than just "boot42" at the end here?
Nope, that's just an odd typo fragment that I need to delete, thanks
for spotting it.
...
>> let boot0 = regs::NV_PMC_BOOT_0::read(bar);
>>
>> - Spec::try_from(boot0)
>> + if boot0.use_boot42_instead() {
>> + Spec::try_from(regs::NV_PMC_BOOT_42::read(bar))
>> + } else {
>> + Spec::try_from(boot0)
>> + }
>> }
>> }
>>
>> diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
>> index 207b865335af..8b5ff3858210 100644
>> --- a/drivers/gpu/nova-core/regs.rs
>> +++ b/drivers/gpu/nova-core/regs.rs
>> @@ -25,6 +25,13 @@
>> });
>>
>> 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
>> + }
>
> So this was the crux of my initial objection, and I just don't think this is truly "forward
> looking". The code is using boot42 only if boot0 is "zeroed out". So sometimes Nova will use
To put it another way: the code is only using boot42 if boot0 is
encoded, by the HW team, to go read boot42. As you know, the future
ref manual literally says "go read boot42."
> boot0 and sometimes it will use boot42, depending on the GPU. It's this inconsistency that
> bothers me.
>
> Instead, I think Nova should use only boot42, so that we have consistent information across all
> GPUs. boot0 should only be used to avoid accidentally reading boot42 when it doesn't exist.
I am convinced that the most appropriate thing for a device driver
to do is to match what the HW configuration says. We should draw
the dividing line at the changeover point, which is in an upcoming
ref manual.
Once boot0 has the encoding set to "go read boot42", the driver
does that. Until then, HW promises that boot0 is correct.
It may look all nice and neat to use "Nova is a new driver" to
pick the point to change, but again, it's more accurate and
appropriate for a device driver to follow HW's lead, and use
what boot0 says to do.
>
> Previously, Danilo said this:
>
>> 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.
>
> This I agree with. The first thing that Spec::new() should do is check whether we're on an
> ancient GPU that does not even have boot42. If so, return Err(ENODEV). Otherwise, from that
> point onward, no code will ever look at boot0 again. boot0 should never be used to return the
> actual architecture/gpu information.
>
I don't think we have a conflict on this point, if you read through how
the code works. The only difference is the point I wrote about above.
I'm hoping you'll allow me to proceed with that.
thanks,
--
John Hubbard
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 3/3] gpu: nova-core: add boot42 support for next-gen GPUs
2025-11-03 1:04 ` John Hubbard
@ 2025-11-03 4:59 ` Timur Tabi
0 siblings, 0 replies; 7+ messages in thread
From: Timur Tabi @ 2025-11-03 4:59 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 Sun, 2025-11-02 at 17:04 -0800, John Hubbard wrote:
> To put it another way: the code is only using boot42 if boot0 is
> encoded, by the HW team, to go read boot42. As you know, the future
> ref manual literally says "go read boot42."
Sure, but that does not mean that older GPUs cannot use boot42.
> I don't think we have a conflict on this point, if you read through how
> the code works. The only difference is the point I wrote about above.
>
> I'm hoping you'll allow me to proceed with that.
Yes, please.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-11-03 4:59 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-02 1:36 [PATCH v4 0/3] gpu: nova: add boot42 support for next-gen GPUs John Hubbard
2025-11-02 1:36 ` [PATCH v4 1/3] gpu: nova-core: prepare Spec and Revision types for boot0/boot42 John Hubbard
2025-11-02 1:36 ` [PATCH v4 2/3] gpu: nova-core: make Architecture behave as a u8 type John Hubbard
2025-11-02 1:36 ` [PATCH v4 3/3] gpu: nova-core: add boot42 support for next-gen GPUs John Hubbard
2025-11-02 18:14 ` Timur Tabi
2025-11-03 1:04 ` John Hubbard
2025-11-03 4:59 ` Timur Tabi
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).