The GSP requires some pieces of metadata to boot. These are passed in a
struct which the GSP transfers via DMA. Create this struct and get a
handle to it for future use when booting the GSP.
Signed-off-by: Alistair Popple <apopple@nvidia.com>
---
Changes for v5:
- Make member visibility match the struct visibility (thanks Danilo)
Changes for v3:
- Don't re-export WPR constants (thanks Alex)
Changes for v2:
- Rebased on Alex's latest version
---
drivers/gpu/nova-core/fb.rs | 1 -
drivers/gpu/nova-core/firmware/gsp.rs | 3 +-
drivers/gpu/nova-core/firmware/riscv.rs | 6 +-
drivers/gpu/nova-core/gsp.rs | 2 +
drivers/gpu/nova-core/gsp/boot.rs | 7 +++
drivers/gpu/nova-core/gsp/fw.rs | 55 ++++++++++++++++++-
.../gpu/nova-core/gsp/fw/r570_144/bindings.rs | 2 +
7 files changed, 69 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
index 4d6a1f452183..5580498ba2fb 100644
--- a/drivers/gpu/nova-core/fb.rs
+++ b/drivers/gpu/nova-core/fb.rs
@@ -87,7 +87,6 @@ pub(crate) fn unregister(&self, bar: &Bar0) {
///
/// Contains ranges of GPU memory reserved for a given purpose during the GSP boot process.
#[derive(Debug)]
-#[expect(dead_code)]
pub(crate) struct FbLayout {
/// Range of the framebuffer. Starts at `0`.
pub(crate) fb: Range<u64>,
diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
index 3a1cf0607de7..c9ad912a3150 100644
--- a/drivers/gpu/nova-core/firmware/gsp.rs
+++ b/drivers/gpu/nova-core/firmware/gsp.rs
@@ -131,7 +131,7 @@ pub(crate) struct GspFirmware {
/// Size in bytes of the firmware contained in [`Self::fw`].
pub size: usize,
/// Device-mapped GSP signatures matching the GPU's [`Chipset`].
- signatures: DmaObject,
+ pub signatures: DmaObject,
/// GSP bootloader, verifies the GSP firmware before loading and running it.
pub bootloader: RiscvFirmware,
}
@@ -216,7 +216,6 @@ pub(crate) fn new<'a, 'b>(
}))
}
- #[expect(unused)]
/// Returns the DMA handle of the radix3 level 0 page table.
pub(crate) fn radix3_dma_handle(&self) -> DmaAddress {
self.level0.dma_handle()
diff --git a/drivers/gpu/nova-core/firmware/riscv.rs b/drivers/gpu/nova-core/firmware/riscv.rs
index 04f1283abb72..115b5f5355a1 100644
--- a/drivers/gpu/nova-core/firmware/riscv.rs
+++ b/drivers/gpu/nova-core/firmware/riscv.rs
@@ -55,11 +55,11 @@ fn new(bin_fw: &BinFirmware<'_>) -> Result<Self> {
#[expect(unused)]
pub(crate) struct RiscvFirmware {
/// Offset at which the code starts in the firmware image.
- code_offset: u32,
+ pub(crate) code_offset: u32,
/// Offset at which the data starts in the firmware image.
- data_offset: u32,
+ pub(crate) data_offset: u32,
/// Offset at which the manifest starts in the firmware image.
- manifest_offset: u32,
+ pub(crate) manifest_offset: u32,
/// Application version.
app_version: u32,
/// Device-mapped firmware image.
diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index f1727173bd42..554eb1a34ee7 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -10,6 +10,8 @@
use kernel::prelude::*;
use kernel::transmute::AsBytes;
+use crate::fb::FbLayout;
+
pub(crate) use fw::{GspFwWprMeta, LibosParams};
mod fw;
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index fb22508128c4..1d2448331d7a 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: GPL-2.0
use kernel::device;
+use kernel::dma::CoherentAllocation;
+use kernel::dma_write;
use kernel::pci;
use kernel::prelude::*;
@@ -14,6 +16,7 @@
FIRMWARE_VERSION,
};
use crate::gpu::Chipset;
+use crate::gsp::GspFwWprMeta;
use crate::regs;
use crate::vbios::Vbios;
@@ -132,6 +135,10 @@ pub(crate) fn boot(
bar,
)?;
+ let wpr_meta =
+ CoherentAllocation::<GspFwWprMeta>::alloc_coherent(dev, 1, GFP_KERNEL | __GFP_ZERO)?;
+ dma_write!(wpr_meta[0] = GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
+
Ok(())
}
}
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index c3bececc29cd..1cc992ca492c 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -10,10 +10,12 @@
use kernel::dma::CoherentAllocation;
use kernel::prelude::*;
use kernel::ptr::{Alignable, Alignment};
-use kernel::sizes::SZ_1M;
+use kernel::sizes::{SZ_128K, SZ_1M};
use kernel::transmute::{AsBytes, FromBytes};
+use crate::firmware::gsp::GspFirmware;
use crate::gpu::Chipset;
+use crate::gsp::FbLayout;
/// Dummy type to group methods related to heap parameters for running the GSP firmware.
pub(crate) struct GspFwHeapParams(());
@@ -105,6 +107,57 @@ pub(crate) fn wpr_heap_size(&self, chipset: Chipset, fb_size: u64) -> u64 {
#[repr(transparent)]
pub(crate) struct GspFwWprMeta(bindings::GspFwWprMeta);
+// SAFETY: Padding is explicit and will not contain uninitialized data.
+unsafe impl AsBytes for GspFwWprMeta {}
+
+// SAFETY: This struct only contains integer types for which all bit patterns
+// are valid.
+unsafe impl FromBytes for GspFwWprMeta {}
+
+type GspFwWprMetaBootResumeInfo = r570_144::GspFwWprMeta__bindgen_ty_1;
+type GspFwWprMetaBootInfo = r570_144::GspFwWprMeta__bindgen_ty_1__bindgen_ty_1;
+
+impl GspFwWprMeta {
+ pub(crate) fn new(gsp_firmware: &GspFirmware, fb_layout: &FbLayout) -> Self {
+ Self(bindings::GspFwWprMeta {
+ magic: r570_144::GSP_FW_WPR_META_MAGIC as u64,
+ revision: u64::from(r570_144::GSP_FW_WPR_META_REVISION),
+ sysmemAddrOfRadix3Elf: gsp_firmware.radix3_dma_handle(),
+ sizeOfRadix3Elf: gsp_firmware.size as u64,
+ sysmemAddrOfBootloader: gsp_firmware.bootloader.ucode.dma_handle(),
+ sizeOfBootloader: gsp_firmware.bootloader.ucode.size() as u64,
+ bootloaderCodeOffset: u64::from(gsp_firmware.bootloader.code_offset),
+ bootloaderDataOffset: u64::from(gsp_firmware.bootloader.data_offset),
+ bootloaderManifestOffset: u64::from(gsp_firmware.bootloader.manifest_offset),
+ __bindgen_anon_1: GspFwWprMetaBootResumeInfo {
+ __bindgen_anon_1: GspFwWprMetaBootInfo {
+ sysmemAddrOfSignature: gsp_firmware.signatures.dma_handle(),
+ sizeOfSignature: gsp_firmware.signatures.size() as u64,
+ },
+ },
+ gspFwRsvdStart: fb_layout.heap.start,
+ nonWprHeapOffset: fb_layout.heap.start,
+ nonWprHeapSize: fb_layout.heap.end - fb_layout.heap.start,
+ gspFwWprStart: fb_layout.wpr2.start,
+ gspFwHeapOffset: fb_layout.wpr2_heap.start,
+ gspFwHeapSize: fb_layout.wpr2_heap.end - fb_layout.wpr2_heap.start,
+ gspFwOffset: fb_layout.elf.start,
+ bootBinOffset: fb_layout.boot.start,
+ frtsOffset: fb_layout.frts.start,
+ frtsSize: fb_layout.frts.end - fb_layout.frts.start,
+ gspFwWprEnd: fb_layout
+ .vga_workspace
+ .start
+ .align_down(Alignment::new::<SZ_128K>()),
+ gspFwHeapVfPartitionCount: fb_layout.vf_partition_count,
+ fbSize: fb_layout.fb.end - fb_layout.fb.start,
+ vgaWorkspaceOffset: fb_layout.vga_workspace.start,
+ vgaWorkspaceSize: fb_layout.vga_workspace.end - fb_layout.vga_workspace.start,
+ ..Default::default()
+ })
+ }
+}
+
/// Struct containing the arguments required to pass a memory buffer to the GSP
/// for use during initialisation.
///
diff --git a/drivers/gpu/nova-core/gsp/fw/r570_144/bindings.rs b/drivers/gpu/nova-core/gsp/fw/r570_144/bindings.rs
index 6a14cc324391..392b25dc6991 100644
--- a/drivers/gpu/nova-core/gsp/fw/r570_144/bindings.rs
+++ b/drivers/gpu/nova-core/gsp/fw/r570_144/bindings.rs
@@ -9,6 +9,8 @@
pub const GSP_FW_HEAP_SIZE_OVERRIDE_LIBOS2_MAX_MB: u32 = 256;
pub const GSP_FW_HEAP_SIZE_OVERRIDE_LIBOS3_BAREMETAL_MIN_MB: u32 = 88;
pub const GSP_FW_HEAP_SIZE_OVERRIDE_LIBOS3_BAREMETAL_MAX_MB: u32 = 280;
+pub const GSP_FW_WPR_META_REVISION: u32 = 1;
+pub const GSP_FW_WPR_META_MAGIC: i64 = -2577556379034558285;
pub type __u8 = ffi::c_uchar;
pub type __u16 = ffi::c_ushort;
pub type __u32 = ffi::c_uint;
--
2.50.1On Mon Oct 13, 2025 at 3:20 PM JST, Alistair Popple wrote:
> The GSP requires some pieces of metadata to boot. These are passed in a
> struct which the GSP transfers via DMA. Create this struct and get a
> handle to it for future use when booting the GSP.
>
> Signed-off-by: Alistair Popple <apopple@nvidia.com>
>
> ---
>
> Changes for v5:
> - Make member visibility match the struct visibility (thanks Danilo)
>
> Changes for v3:
> - Don't re-export WPR constants (thanks Alex)
>
> Changes for v2:
> - Rebased on Alex's latest version
> ---
> drivers/gpu/nova-core/fb.rs | 1 -
> drivers/gpu/nova-core/firmware/gsp.rs | 3 +-
> drivers/gpu/nova-core/firmware/riscv.rs | 6 +-
> drivers/gpu/nova-core/gsp.rs | 2 +
> drivers/gpu/nova-core/gsp/boot.rs | 7 +++
> drivers/gpu/nova-core/gsp/fw.rs | 55 ++++++++++++++++++-
> .../gpu/nova-core/gsp/fw/r570_144/bindings.rs | 2 +
> 7 files changed, 69 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
> index 4d6a1f452183..5580498ba2fb 100644
> --- a/drivers/gpu/nova-core/fb.rs
> +++ b/drivers/gpu/nova-core/fb.rs
> @@ -87,7 +87,6 @@ pub(crate) fn unregister(&self, bar: &Bar0) {
> ///
> /// Contains ranges of GPU memory reserved for a given purpose during the GSP boot process.
> #[derive(Debug)]
> -#[expect(dead_code)]
> pub(crate) struct FbLayout {
> /// Range of the framebuffer. Starts at `0`.
> pub(crate) fb: Range<u64>,
> diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
> index 3a1cf0607de7..c9ad912a3150 100644
> --- a/drivers/gpu/nova-core/firmware/gsp.rs
> +++ b/drivers/gpu/nova-core/firmware/gsp.rs
> @@ -131,7 +131,7 @@ pub(crate) struct GspFirmware {
> /// Size in bytes of the firmware contained in [`Self::fw`].
> pub size: usize,
> /// Device-mapped GSP signatures matching the GPU's [`Chipset`].
> - signatures: DmaObject,
> + pub signatures: DmaObject,
This needs to be `pub(crate)` or rustc 1.78 emits a warning (I'll also
have to fix the surrounding ones in my own patch).
<snip>
> +impl GspFwWprMeta {
> + pub(crate) fn new(gsp_firmware: &GspFirmware, fb_layout: &FbLayout) -> Self {
> + Self(bindings::GspFwWprMeta {
> + magic: r570_144::GSP_FW_WPR_META_MAGIC as u64,
> + revision: u64::from(r570_144::GSP_FW_WPR_META_REVISION),
> + sysmemAddrOfRadix3Elf: gsp_firmware.radix3_dma_handle(),
> + sizeOfRadix3Elf: gsp_firmware.size as u64,
Very unfortunately I'm afraid we will need to replace the `as` in this
method with `u64::try_from` and make it return a `Result` for now.
On 2025-10-16 at 17:23 +1100, Alexandre Courbot <acourbot@nvidia.com> wrote...
> On Mon Oct 13, 2025 at 3:20 PM JST, Alistair Popple wrote:
> > The GSP requires some pieces of metadata to boot. These are passed in a
> > struct which the GSP transfers via DMA. Create this struct and get a
> > handle to it for future use when booting the GSP.
> >
> > Signed-off-by: Alistair Popple <apopple@nvidia.com>
> >
> > ---
> >
> > Changes for v5:
> > - Make member visibility match the struct visibility (thanks Danilo)
> >
> > Changes for v3:
> > - Don't re-export WPR constants (thanks Alex)
> >
> > Changes for v2:
> > - Rebased on Alex's latest version
> > ---
> > drivers/gpu/nova-core/fb.rs | 1 -
> > drivers/gpu/nova-core/firmware/gsp.rs | 3 +-
> > drivers/gpu/nova-core/firmware/riscv.rs | 6 +-
> > drivers/gpu/nova-core/gsp.rs | 2 +
> > drivers/gpu/nova-core/gsp/boot.rs | 7 +++
> > drivers/gpu/nova-core/gsp/fw.rs | 55 ++++++++++++++++++-
> > .../gpu/nova-core/gsp/fw/r570_144/bindings.rs | 2 +
> > 7 files changed, 69 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
> > index 4d6a1f452183..5580498ba2fb 100644
> > --- a/drivers/gpu/nova-core/fb.rs
> > +++ b/drivers/gpu/nova-core/fb.rs
> > @@ -87,7 +87,6 @@ pub(crate) fn unregister(&self, bar: &Bar0) {
> > ///
> > /// Contains ranges of GPU memory reserved for a given purpose during the GSP boot process.
> > #[derive(Debug)]
> > -#[expect(dead_code)]
> > pub(crate) struct FbLayout {
> > /// Range of the framebuffer. Starts at `0`.
> > pub(crate) fb: Range<u64>,
> > diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
> > index 3a1cf0607de7..c9ad912a3150 100644
> > --- a/drivers/gpu/nova-core/firmware/gsp.rs
> > +++ b/drivers/gpu/nova-core/firmware/gsp.rs
> > @@ -131,7 +131,7 @@ pub(crate) struct GspFirmware {
> > /// Size in bytes of the firmware contained in [`Self::fw`].
> > pub size: usize,
> > /// Device-mapped GSP signatures matching the GPU's [`Chipset`].
> > - signatures: DmaObject,
> > + pub signatures: DmaObject,
>
> This needs to be `pub(crate)` or rustc 1.78 emits a warning (I'll also
> have to fix the surrounding ones in my own patch).
Thanks, missed that one.
> <snip>
> > +impl GspFwWprMeta {
> > + pub(crate) fn new(gsp_firmware: &GspFirmware, fb_layout: &FbLayout) -> Self {
> > + Self(bindings::GspFwWprMeta {
> > + magic: r570_144::GSP_FW_WPR_META_MAGIC as u64,
> > + revision: u64::from(r570_144::GSP_FW_WPR_META_REVISION),
> > + sysmemAddrOfRadix3Elf: gsp_firmware.radix3_dma_handle(),
> > + sizeOfRadix3Elf: gsp_firmware.size as u64,
>
> Very unfortunately I'm afraid we will need to replace the `as` in this
> method with `u64::try_from` and make it return a `Result` for now.
And presumably most of the other `as` keywords in this function dealing with
usize as well? Have made the change but would you mind quickly explaining
why this is needed? Is the concern that usize might be more than 64 bits or
something?
On Fri Oct 17, 2025 at 1:03 AM CEST, Alistair Popple wrote:
> On 2025-10-16 at 17:23 +1100, Alexandre Courbot <acourbot@nvidia.com> wrote...
>> On Mon Oct 13, 2025 at 3:20 PM JST, Alistair Popple wrote:
>> > +impl GspFwWprMeta {
>> > + pub(crate) fn new(gsp_firmware: &GspFirmware, fb_layout: &FbLayout) -> Self {
>> > + Self(bindings::GspFwWprMeta {
>> > + magic: r570_144::GSP_FW_WPR_META_MAGIC as u64,
>> > + revision: u64::from(r570_144::GSP_FW_WPR_META_REVISION),
>> > + sysmemAddrOfRadix3Elf: gsp_firmware.radix3_dma_handle(),
>> > + sizeOfRadix3Elf: gsp_firmware.size as u64,
>>
>> Very unfortunately I'm afraid we will need to replace the `as` in this
>> method with `u64::try_from` and make it return a `Result` for now.
>
> And presumably most of the other `as` keywords in this function dealing with
> usize as well? Have made the change but would you mind quickly explaining
> why this is needed? Is the concern that usize might be more than 64 bits or
> something?
Since nova-core depends on CONFIG_64BIT, I think we want a helper function that
converts usize to u64 infallibly.
This helper function can simply generate a compile time error, when
!CONFIG_64BIT, etc.
We can do this locally in nova-core, but it could also find it's place in the
generic infrastructure. nova-core clearly won't be the last driver running into
this inconvinience.
On Fri Oct 17, 2025 at 8:11 AM JST, Danilo Krummrich wrote:
> On Fri Oct 17, 2025 at 1:03 AM CEST, Alistair Popple wrote:
>> On 2025-10-16 at 17:23 +1100, Alexandre Courbot <acourbot@nvidia.com> wrote...
>>> On Mon Oct 13, 2025 at 3:20 PM JST, Alistair Popple wrote:
>>> > +impl GspFwWprMeta {
>>> > + pub(crate) fn new(gsp_firmware: &GspFirmware, fb_layout: &FbLayout) -> Self {
>>> > + Self(bindings::GspFwWprMeta {
>>> > + magic: r570_144::GSP_FW_WPR_META_MAGIC as u64,
>>> > + revision: u64::from(r570_144::GSP_FW_WPR_META_REVISION),
>>> > + sysmemAddrOfRadix3Elf: gsp_firmware.radix3_dma_handle(),
>>> > + sizeOfRadix3Elf: gsp_firmware.size as u64,
>>>
>>> Very unfortunately I'm afraid we will need to replace the `as` in this
>>> method with `u64::try_from` and make it return a `Result` for now.
>>
>> And presumably most of the other `as` keywords in this function dealing with
>> usize as well? Have made the change but would you mind quickly explaining
>> why this is needed? Is the concern that usize might be more than 64 bits or
>> something?
>
> Since nova-core depends on CONFIG_64BIT, I think we want a helper function that
> converts usize to u64 infallibly.
>
> This helper function can simply generate a compile time error, when
> !CONFIG_64BIT, etc.
>
> We can do this locally in nova-core, but it could also find it's place in the
> generic infrastructure. nova-core clearly won't be the last driver running into
> this inconvinience.
How do you see this taking shape concretely? I have tried writing
extention traits (e.g. `IntoU64`), but unfortunately we also need to use
these in const contexts so that won't until const trait methods are a
thing.
The alternative would be to have const functions like `usize_to_u64`. It
doesn't look as smooth as the extention trait, but can be used in const
contexts.
I can try and provide an implementation to apply before this patch
series if given guidance on which direction we want to go.
On Mon Oct 20, 2025 at 7:40 AM CEST, Alexandre Courbot wrote: > The alternative would be to have const functions like `usize_to_u64`. It > doesn't look as smooth as the extention trait, but can be used in const > contexts. That's what I thought of, exactly for the reason of being usable in const contexts (at least for a quick fix in nova-core). Whether we want an extention trait and a separate temporary const_usize_to_u64() etc. can be discussed in the context of making it common infrastructure.
On Mon Oct 20, 2025 at 7:13 PM JST, Danilo Krummrich wrote: > On Mon Oct 20, 2025 at 7:40 AM CEST, Alexandre Courbot wrote: >> The alternative would be to have const functions like `usize_to_u64`. It >> doesn't look as smooth as the extention trait, but can be used in const >> contexts. > > That's what I thought of, exactly for the reason of being usable in const > contexts (at least for a quick fix in nova-core). > > Whether we want an extention trait and a separate temporary const_usize_to_u64() > etc. can be discussed in the context of making it common infrastructure. Thanks - so IIUC the idea would be to keep this local to nova-core in a first time? If so I guess I can produce this fast (and convert our many uses of `as` in the driver so far).
On Mon Oct 20, 2025 at 12:50 PM CEST, Alexandre Courbot wrote: > On Mon Oct 20, 2025 at 7:13 PM JST, Danilo Krummrich wrote: >> On Mon Oct 20, 2025 at 7:40 AM CEST, Alexandre Courbot wrote: >>> The alternative would be to have const functions like `usize_to_u64`. It >>> doesn't look as smooth as the extention trait, but can be used in const >>> contexts. >> >> That's what I thought of, exactly for the reason of being usable in const >> contexts (at least for a quick fix in nova-core). >> >> Whether we want an extention trait and a separate temporary const_usize_to_u64() >> etc. can be discussed in the context of making it common infrastructure. > > Thanks - so IIUC the idea would be to keep this local to nova-core in a > first time? If so I guess I can produce this fast (and convert our many > uses of `as` in the driver so far). Yeah, I think that's reasonable. We can do both in parallel.
On Fri Oct 17, 2025 at 8:11 AM JST, Danilo Krummrich wrote:
> On Fri Oct 17, 2025 at 1:03 AM CEST, Alistair Popple wrote:
>> On 2025-10-16 at 17:23 +1100, Alexandre Courbot <acourbot@nvidia.com> wrote...
>>> On Mon Oct 13, 2025 at 3:20 PM JST, Alistair Popple wrote:
>>> > +impl GspFwWprMeta {
>>> > + pub(crate) fn new(gsp_firmware: &GspFirmware, fb_layout: &FbLayout) -> Self {
>>> > + Self(bindings::GspFwWprMeta {
>>> > + magic: r570_144::GSP_FW_WPR_META_MAGIC as u64,
>>> > + revision: u64::from(r570_144::GSP_FW_WPR_META_REVISION),
>>> > + sysmemAddrOfRadix3Elf: gsp_firmware.radix3_dma_handle(),
>>> > + sizeOfRadix3Elf: gsp_firmware.size as u64,
>>>
>>> Very unfortunately I'm afraid we will need to replace the `as` in this
>>> method with `u64::try_from` and make it return a `Result` for now.
>>
>> And presumably most of the other `as` keywords in this function dealing with
>> usize as well? Have made the change but would you mind quickly explaining
>> why this is needed? Is the concern that usize might be more than 64 bits or
>> something?
Indeed, the concern is that `as` performs a lossy conversion without
warning, which could catch us off-guard if e.g. some type changes in the
bindings.
>
> Since nova-core depends on CONFIG_64BIT, I think we want a helper function that
> converts usize to u64 infallibly.
>
> This helper function can simply generate a compile time error, when
> !CONFIG_64BIT, etc.
>
> We can do this locally in nova-core, but it could also find it's place in the
> generic infrastructure. nova-core clearly won't be the last driver running into
> this inconvinience.
That would definitely be the correct way to address this.
On 2025-10-17 at 11:43 +1100, Alexandre Courbot <acourbot@nvidia.com> wrote...
> On Fri Oct 17, 2025 at 8:11 AM JST, Danilo Krummrich wrote:
> > On Fri Oct 17, 2025 at 1:03 AM CEST, Alistair Popple wrote:
> >> On 2025-10-16 at 17:23 +1100, Alexandre Courbot <acourbot@nvidia.com> wrote...
> >>> On Mon Oct 13, 2025 at 3:20 PM JST, Alistair Popple wrote:
> >>> > +impl GspFwWprMeta {
> >>> > + pub(crate) fn new(gsp_firmware: &GspFirmware, fb_layout: &FbLayout) -> Self {
> >>> > + Self(bindings::GspFwWprMeta {
> >>> > + magic: r570_144::GSP_FW_WPR_META_MAGIC as u64,
> >>> > + revision: u64::from(r570_144::GSP_FW_WPR_META_REVISION),
> >>> > + sysmemAddrOfRadix3Elf: gsp_firmware.radix3_dma_handle(),
> >>> > + sizeOfRadix3Elf: gsp_firmware.size as u64,
> >>>
> >>> Very unfortunately I'm afraid we will need to replace the `as` in this
> >>> method with `u64::try_from` and make it return a `Result` for now.
> >>
> >> And presumably most of the other `as` keywords in this function dealing with
> >> usize as well? Have made the change but would you mind quickly explaining
> >> why this is needed? Is the concern that usize might be more than 64 bits or
> >> something?
>
> Indeed, the concern is that `as` performs a lossy conversion without
> warning, which could catch us off-guard if e.g. some type changes in the
> bindings.
>
> >
> > Since nova-core depends on CONFIG_64BIT, I think we want a helper function that
> > converts usize to u64 infallibly.
> >
> > This helper function can simply generate a compile time error, when
> > !CONFIG_64BIT, etc.
> >
> > We can do this locally in nova-core, but it could also find it's place in the
> > generic infrastructure. nova-core clearly won't be the last driver running into
> > this inconvinience.
>
> That would definitely be the correct way to address this.
Sure. Although I don't really have a problem with the binding constructors being
fallible as plenty of the others are anyway.
On Fri Oct 17, 2025 at 10:15 AM JST, Alistair Popple wrote:
> On 2025-10-17 at 11:43 +1100, Alexandre Courbot <acourbot@nvidia.com> wrote...
>> On Fri Oct 17, 2025 at 8:11 AM JST, Danilo Krummrich wrote:
>> > On Fri Oct 17, 2025 at 1:03 AM CEST, Alistair Popple wrote:
>> >> On 2025-10-16 at 17:23 +1100, Alexandre Courbot <acourbot@nvidia.com> wrote...
>> >>> On Mon Oct 13, 2025 at 3:20 PM JST, Alistair Popple wrote:
>> >>> > +impl GspFwWprMeta {
>> >>> > + pub(crate) fn new(gsp_firmware: &GspFirmware, fb_layout: &FbLayout) -> Self {
>> >>> > + Self(bindings::GspFwWprMeta {
>> >>> > + magic: r570_144::GSP_FW_WPR_META_MAGIC as u64,
>> >>> > + revision: u64::from(r570_144::GSP_FW_WPR_META_REVISION),
>> >>> > + sysmemAddrOfRadix3Elf: gsp_firmware.radix3_dma_handle(),
>> >>> > + sizeOfRadix3Elf: gsp_firmware.size as u64,
>> >>>
>> >>> Very unfortunately I'm afraid we will need to replace the `as` in this
>> >>> method with `u64::try_from` and make it return a `Result` for now.
>> >>
>> >> And presumably most of the other `as` keywords in this function dealing with
>> >> usize as well? Have made the change but would you mind quickly explaining
>> >> why this is needed? Is the concern that usize might be more than 64 bits or
>> >> something?
>>
>> Indeed, the concern is that `as` performs a lossy conversion without
>> warning, which could catch us off-guard if e.g. some type changes in the
>> bindings.
>>
>> >
>> > Since nova-core depends on CONFIG_64BIT, I think we want a helper function that
>> > converts usize to u64 infallibly.
>> >
>> > This helper function can simply generate a compile time error, when
>> > !CONFIG_64BIT, etc.
>> >
>> > We can do this locally in nova-core, but it could also find it's place in the
>> > generic infrastructure. nova-core clearly won't be the last driver running into
>> > this inconvinience.
>>
>> That would definitely be the correct way to address this.
>
> Sure. Although I don't really have a problem with the binding constructors being
> fallible as plenty of the others are anyway.
I think we can address the non-fallible conversions as a separate task
(as there are many sites that could be similarly optimized in the
nova-core code), so fallible constructors are acceptable imho.
On Fri Oct 17, 2025 at 3:38 AM CEST, Alexandre Courbot wrote:
> On Fri Oct 17, 2025 at 10:15 AM JST, Alistair Popple wrote:
>> On 2025-10-17 at 11:43 +1100, Alexandre Courbot <acourbot@nvidia.com> wrote...
>>> On Fri Oct 17, 2025 at 8:11 AM JST, Danilo Krummrich wrote:
>>> > On Fri Oct 17, 2025 at 1:03 AM CEST, Alistair Popple wrote:
>>> >> On 2025-10-16 at 17:23 +1100, Alexandre Courbot <acourbot@nvidia.com> wrote...
>>> >>> On Mon Oct 13, 2025 at 3:20 PM JST, Alistair Popple wrote:
>>> >>> > +impl GspFwWprMeta {
>>> >>> > + pub(crate) fn new(gsp_firmware: &GspFirmware, fb_layout: &FbLayout) -> Self {
>>> >>> > + Self(bindings::GspFwWprMeta {
>>> >>> > + magic: r570_144::GSP_FW_WPR_META_MAGIC as u64,
>>> >>> > + revision: u64::from(r570_144::GSP_FW_WPR_META_REVISION),
>>> >>> > + sysmemAddrOfRadix3Elf: gsp_firmware.radix3_dma_handle(),
>>> >>> > + sizeOfRadix3Elf: gsp_firmware.size as u64,
>>> >>>
>>> >>> Very unfortunately I'm afraid we will need to replace the `as` in this
>>> >>> method with `u64::try_from` and make it return a `Result` for now.
>>> >>
>>> >> And presumably most of the other `as` keywords in this function dealing with
>>> >> usize as well? Have made the change but would you mind quickly explaining
>>> >> why this is needed? Is the concern that usize might be more than 64 bits or
>>> >> something?
>>>
>>> Indeed, the concern is that `as` performs a lossy conversion without
>>> warning, which could catch us off-guard if e.g. some type changes in the
>>> bindings.
>>>
>>> >
>>> > Since nova-core depends on CONFIG_64BIT, I think we want a helper function that
>>> > converts usize to u64 infallibly.
>>> >
>>> > This helper function can simply generate a compile time error, when
>>> > !CONFIG_64BIT, etc.
>>> >
>>> > We can do this locally in nova-core, but it could also find it's place in the
>>> > generic infrastructure. nova-core clearly won't be the last driver running into
>>> > this inconvinience.
>>>
>>> That would definitely be the correct way to address this.
>>
>> Sure. Although I don't really have a problem with the binding constructors being
>> fallible as plenty of the others are anyway.
>
> I think we can address the non-fallible conversions as a separate task
> (as there are many sites that could be similarly optimized in the
> nova-core code), so fallible constructors are acceptable imho.
The infallible conversion function is trivial to implement.
If you prefer, we can also add it in nova-core first and subsequently move it to
generic infrastructure.
I prefer not to introduce more as-casts or fallible conversions we have to clean
up subsequently.
On Fri, Oct 17, 2025 at 1:11 AM Danilo Krummrich <dakr@kernel.org> wrote: > > Since nova-core depends on CONFIG_64BIT, I think we want a helper function that > converts usize to u64 infallibly. > > This helper function can simply generate a compile time error, when > !CONFIG_64BIT, etc. > > We can do this locally in nova-core, but it could also find it's place in the > generic infrastructure. nova-core clearly won't be the last driver running into > this inconvinience. Indeed. Cheers, Miguel
© 2016 - 2025 Red Hat, Inc.