Danilo Krummrich
2025-Oct-29 11:26 UTC
[PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
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 } )
Alexandre Courbot
2025-Oct-29 13:54 UTC
[PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
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) }
John Hubbard
2025-Oct-30 00:29 UTC
[PATCH v3 2/2] gpu: nova-core: add boot42 support for next-gen GPUs
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