FWSEC-FRTS is run with the desired address of the FRTS region as
parameter, which we need to compute depending on some hardware
parameters.
Do this in a `FbLayout` structure, that will be later extended to
describe more memory regions used to boot the GSP.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/gpu.rs | 4 ++
drivers/gpu/nova-core/gsp.rs | 3 ++
drivers/gpu/nova-core/gsp/fb.rs | 108 +++++++++++++++++++++++++++++++++++++
drivers/gpu/nova-core/nova_core.rs | 1 +
drivers/gpu/nova-core/regs.rs | 27 ++++++++++
5 files changed, 143 insertions(+)
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 4bf7f72247e5320935a517270b5a0e1ec2becfec..a3d96639706e808305cce66416778d2bf6e7e683 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -7,6 +7,7 @@
use crate::driver::Bar0;
use crate::falcon::{gsp::Gsp, sec2::Sec2, Falcon};
use crate::firmware::Firmware;
+use crate::gsp::fb::FbLayout;
use crate::regs;
use crate::util;
use crate::vbios::Vbios;
@@ -239,6 +240,9 @@ pub(crate) fn new(
let _sec2_falcon = Falcon::<Sec2>::new(pdev.as_ref(), spec.chipset, bar, true)?;
+ let fb_layout = FbLayout::new(spec.chipset, bar)?;
+ dev_dbg!(pdev.as_ref(), "{:#x?}\n", fb_layout);
+
let _bios = Vbios::new(pdev, bar)?;
Ok(pin_init!(Self {
diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
new file mode 100644
index 0000000000000000000000000000000000000000..27616a9d2b7069b18661fc97811fa1cac285b8f8
--- /dev/null
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -0,0 +1,3 @@
+// SPDX-License-Identifier: GPL-2.0
+
+pub(crate) mod fb;
diff --git a/drivers/gpu/nova-core/gsp/fb.rs b/drivers/gpu/nova-core/gsp/fb.rs
new file mode 100644
index 0000000000000000000000000000000000000000..f28ded59469d52daf39e5d19c09efd7bf08fee92
--- /dev/null
+++ b/drivers/gpu/nova-core/gsp/fb.rs
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use core::ops::Range;
+
+use kernel::prelude::*;
+
+use crate::driver::Bar0;
+use crate::gpu::Chipset;
+use crate::regs;
+
+fn align_down(value: u64, align: u64) -> u64 {
+ value & !(align - 1)
+}
+
+/// Layout of the GPU framebuffer memory.
+///
+/// Contains ranges of GPU memory reserved for a given purpose during the GSP bootup process.
+#[derive(Debug)]
+#[expect(dead_code)]
+pub(crate) struct FbLayout {
+ pub fb: Range<u64>,
+
+ pub vga_workspace: Range<u64>,
+ pub bios: Range<u64>,
+
+ pub frts: Range<u64>,
+}
+
+impl FbLayout {
+ pub(crate) fn new(chipset: Chipset, bar: &Bar0) -> Result<Self> {
+ let fb = {
+ let fb_size = vidmem_size(bar, chipset);
+
+ 0..fb_size
+ };
+ let fb_len = fb.end - fb.start;
+
+ let vga_workspace = {
+ let vga_base = vga_workspace_addr(bar, fb_len, chipset);
+
+ vga_base..fb.end
+ };
+
+ let bios = vga_workspace.clone();
+
+ let frts = {
+ const FRTS_DOWN_ALIGN: u64 = 0x20000;
+ const FRTS_SIZE: u64 = 0x100000;
+ let frts_base = align_down(vga_workspace.start, FRTS_DOWN_ALIGN) - FRTS_SIZE;
+
+ frts_base..frts_base + FRTS_SIZE
+ };
+
+ Ok(Self {
+ fb,
+ vga_workspace,
+ bios,
+ frts,
+ })
+ }
+}
+
+/// Returns `true` if the display is disabled.
+fn display_disabled(bar: &Bar0, chipset: Chipset) -> bool {
+ if chipset >= Chipset::GA100 {
+ regs::NV_FUSE_STATUS_OPT_DISPLAY_MAXWELL::read(bar).display_disabled()
+ } else {
+ regs::NV_FUSE_STATUS_OPT_DISPLAY_AMPERE::read(bar).display_disabled()
+ }
+}
+
+/// Returns the video memory size in bytes.
+fn vidmem_size(bar: &Bar0, chipset: Chipset) -> u64 {
+ if chipset >= Chipset::GA102 {
+ (regs::NV_PGC6_AON_SECURE_SCRATCH_GROUP_42::read(bar).value() as u64) << 20
+ } else {
+ let local_mem_range = regs::NV_PFB_PRI_MMU_LOCAL_MEMORY_RANGE::read(bar);
+ let size =
+ (local_mem_range.lower_mag() as u64) << ((local_mem_range.lower_scale() as u64) + 20);
+
+ if local_mem_range.ecc_mode_enabled() {
+ size / 16 * 15
+ } else {
+ size
+ }
+ }
+}
+
+/// Returns the vga workspace address.
+fn vga_workspace_addr(bar: &Bar0, fb_size: u64, chipset: Chipset) -> u64 {
+ let base = fb_size - 0x100000;
+ let vga_workspace_base = if display_disabled(bar, chipset) {
+ regs::NV_PDISP_VGA_WORKSPACE_BASE::read(bar)
+ } else {
+ return base;
+ };
+
+ if !vga_workspace_base.status_valid() {
+ return base;
+ }
+
+ let addr = (vga_workspace_base.addr() as u64) << 16;
+ if addr < base {
+ fb_size - 0x20000
+ } else {
+ addr
+ }
+}
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index ff6d0b40c18f36af4c7e2d5c839fdf77dba23321..202e978e56f024de3ae8b178e65b63c2cea244e1 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -8,6 +8,7 @@
mod falcon;
mod firmware;
mod gpu;
+mod gsp;
mod regs;
mod util;
mod vbios;
diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
index b5c6eeb6ed873a06b4aefcb375f4944eb0b20597..15ec9b7e69694ff198b5353d562fc1aff5eefd3f 100644
--- a/drivers/gpu/nova-core/regs.rs
+++ b/drivers/gpu/nova-core/regs.rs
@@ -53,6 +53,12 @@ pub(crate) fn chipset(self) -> Result<Chipset, Error> {
23:0 adr_63_40 as u32;
});
+register!(NV_PFB_PRI_MMU_LOCAL_MEMORY_RANGE @ 0x00100ce0 {
+ 3:0 lower_scale as u8;
+ 9:4 lower_mag as u8;
+ 30:30 ecc_mode_enabled as bool;
+});
+
/* PGC6 */
register!(NV_PGC6_AON_SECURE_SCRATCH_GROUP_05_PRIV_LEVEL_MASK @ 0x00118128 {
@@ -64,6 +70,27 @@ pub(crate) fn chipset(self) -> Result<Chipset, Error> {
31:0 value as u32;
});
+register!(NV_PGC6_AON_SECURE_SCRATCH_GROUP_42 @ 0x001183a4 {
+ 31:0 value as u32;
+});
+
+/* PDISP */
+
+register!(NV_PDISP_VGA_WORKSPACE_BASE @ 0x00625f04 {
+ 3:3 status_valid as bool;
+ 31:8 addr as u32;
+});
+
+/* FUSE */
+
+register!(NV_FUSE_STATUS_OPT_DISPLAY_MAXWELL @ 0x00021c04 {
+ 0:0 display_disabled as bool;
+});
+
+register!(NV_FUSE_STATUS_OPT_DISPLAY_AMPERE @ 0x00820c04 {
+ 0:0 display_disabled as bool;
+});
+
/* PFALCON */
register!(NV_PFALCON_FALCON_IRQSCLR @ +0x00000004 {
--
2.49.0On Wed, May 07, 2025 at 10:52:44PM +0900, Alexandre Courbot wrote:
> FWSEC-FRTS is run with the desired address of the FRTS region as
> parameter, which we need to compute depending on some hardware
> parameters.
>
> Do this in a `FbLayout` structure, that will be later extended to
> describe more memory regions used to boot the GSP.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> drivers/gpu/nova-core/gpu.rs | 4 ++
> drivers/gpu/nova-core/gsp.rs | 3 ++
> drivers/gpu/nova-core/gsp/fb.rs | 108 +++++++++++++++++++++++++++++++++++++
> drivers/gpu/nova-core/nova_core.rs | 1 +
> drivers/gpu/nova-core/regs.rs | 27 ++++++++++
> 5 files changed, 143 insertions(+)
>
> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> index 4bf7f72247e5320935a517270b5a0e1ec2becfec..a3d96639706e808305cce66416778d2bf6e7e683 100644
> --- a/drivers/gpu/nova-core/gpu.rs
> +++ b/drivers/gpu/nova-core/gpu.rs
> @@ -7,6 +7,7 @@
> use crate::driver::Bar0;
> use crate::falcon::{gsp::Gsp, sec2::Sec2, Falcon};
> use crate::firmware::Firmware;
> +use crate::gsp::fb::FbLayout;
> use crate::regs;
> use crate::util;
> use crate::vbios::Vbios;
> @@ -239,6 +240,9 @@ pub(crate) fn new(
>
> let _sec2_falcon = Falcon::<Sec2>::new(pdev.as_ref(), spec.chipset, bar, true)?;
>
> + let fb_layout = FbLayout::new(spec.chipset, bar)?;
> + dev_dbg!(pdev.as_ref(), "{:#x?}\n", fb_layout);
> +
> let _bios = Vbios::new(pdev, bar)?;
>
> Ok(pin_init!(Self {
> diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..27616a9d2b7069b18661fc97811fa1cac285b8f8
> --- /dev/null
> +++ b/drivers/gpu/nova-core/gsp.rs
> @@ -0,0 +1,3 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +pub(crate) mod fb;
> diff --git a/drivers/gpu/nova-core/gsp/fb.rs b/drivers/gpu/nova-core/gsp/fb.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..f28ded59469d52daf39e5d19c09efd7bf08fee92
> --- /dev/null
> +++ b/drivers/gpu/nova-core/gsp/fb.rs
> @@ -0,0 +1,108 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use core::ops::Range;
> +
> +use kernel::prelude::*;
> +
> +use crate::driver::Bar0;
> +use crate::gpu::Chipset;
> +use crate::regs;
> +
> +fn align_down(value: u64, align: u64) -> u64 {
> + value & !(align - 1)
> +}
Can this go in the previous patch, i.e. "rust: num: Add an upward alignment
helper for usize"?
> +
> +/// Layout of the GPU framebuffer memory.
> +///
> +/// Contains ranges of GPU memory reserved for a given purpose during the GSP bootup process.
> +#[derive(Debug)]
> +#[expect(dead_code)]
> +pub(crate) struct FbLayout {
> + pub fb: Range<u64>,
> +
> + pub vga_workspace: Range<u64>,
> + pub bios: Range<u64>,
> +
> + pub frts: Range<u64>,
Please remove the empty lines.
> +}
> +
> +impl FbLayout {
> + pub(crate) fn new(chipset: Chipset, bar: &Bar0) -> Result<Self> {
> + let fb = {
> + let fb_size = vidmem_size(bar, chipset);
> +
> + 0..fb_size
> + };
> + let fb_len = fb.end - fb.start;
Isn't this the same as fb_size? Why not just write it as
let fb_size = vidmem_size(bar, chipset);
let fb = 0..fb_size;
> +
> + let vga_workspace = {
> + let vga_base = vga_workspace_addr(bar, fb_len, chipset);
> +
> + vga_base..fb.end
> + };
> +
> + let bios = vga_workspace.clone();
Why? And why store the same thing twice in FbLayout? If it's really needed,
clone it in the constructor below and add a comment why it's the same.
> +
> + let frts = {
> + const FRTS_DOWN_ALIGN: u64 = 0x20000;
> + const FRTS_SIZE: u64 = 0x100000;
> + let frts_base = align_down(vga_workspace.start, FRTS_DOWN_ALIGN) - FRTS_SIZE;
> +
> + frts_base..frts_base + FRTS_SIZE
> + };
> +
> + Ok(Self {
> + fb,
> + vga_workspace,
> + bios,
> + frts,
> + })
> + }
> +}
I'd probably wrap those helpers below in
mod hal { ... }
or even a new file fb/hal.rs to make their purpose obvious.
> +/// Returns `true` if the display is disabled.
> +fn display_disabled(bar: &Bar0, chipset: Chipset) -> bool {
> + if chipset >= Chipset::GA100 {
> + regs::NV_FUSE_STATUS_OPT_DISPLAY_MAXWELL::read(bar).display_disabled()
> + } else {
> + regs::NV_FUSE_STATUS_OPT_DISPLAY_AMPERE::read(bar).display_disabled()
> + }
> +}
> +
> +/// Returns the video memory size in bytes.
> +fn vidmem_size(bar: &Bar0, chipset: Chipset) -> u64 {
> + if chipset >= Chipset::GA102 {
Is GA102 intentional or should this also be GA100?
> + (regs::NV_PGC6_AON_SECURE_SCRATCH_GROUP_42::read(bar).value() as u64) << 20
Why isn't this shift part of the register abstraction?
> + } else {
> + let local_mem_range = regs::NV_PFB_PRI_MMU_LOCAL_MEMORY_RANGE::read(bar);
> + let size =
> + (local_mem_range.lower_mag() as u64) << ((local_mem_range.lower_scale() as u64) + 20);
Same here. Please make this part of the register abstraction as it is done in
previous patches.
> +
> + if local_mem_range.ecc_mode_enabled() {
> + size / 16 * 15
> + } else {
> + size
> + }
> + }
> +}
> +
> +/// Returns the vga workspace address.
> +fn vga_workspace_addr(bar: &Bar0, fb_size: u64, chipset: Chipset) -> u64 {
> + let base = fb_size - 0x100000;
What's this offset? How do you guarantee that this never underflows?
> + let vga_workspace_base = if display_disabled(bar, chipset) {
> + regs::NV_PDISP_VGA_WORKSPACE_BASE::read(bar)
> + } else {
> + return base;
> + };
> +
> + if !vga_workspace_base.status_valid() {
> + return base;
> + }
> +
> + let addr = (vga_workspace_base.addr() as u64) << 16;
Where does this shift come from? Shouldn't this be part of the register
abstraction?
> + if addr < base {
> + fb_size - 0x20000
What's this offset? Can this ever underflow?
> + } else {
> + addr
> + }
> +}
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> index ff6d0b40c18f36af4c7e2d5c839fdf77dba23321..202e978e56f024de3ae8b178e65b63c2cea244e1 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -8,6 +8,7 @@
> mod falcon;
> mod firmware;
> mod gpu;
> +mod gsp;
> mod regs;
> mod util;
> mod vbios;
> diff --git a/drivers/gpu/nova-core/regs.rs b/drivers/gpu/nova-core/regs.rs
> index b5c6eeb6ed873a06b4aefcb375f4944eb0b20597..15ec9b7e69694ff198b5353d562fc1aff5eefd3f 100644
> --- a/drivers/gpu/nova-core/regs.rs
> +++ b/drivers/gpu/nova-core/regs.rs
> @@ -53,6 +53,12 @@ pub(crate) fn chipset(self) -> Result<Chipset, Error> {
> 23:0 adr_63_40 as u32;
> });
>
> +register!(NV_PFB_PRI_MMU_LOCAL_MEMORY_RANGE @ 0x00100ce0 {
> + 3:0 lower_scale as u8;
> + 9:4 lower_mag as u8;
> + 30:30 ecc_mode_enabled as bool;
> +});
> +
> /* PGC6 */
>
> register!(NV_PGC6_AON_SECURE_SCRATCH_GROUP_05_PRIV_LEVEL_MASK @ 0x00118128 {
> @@ -64,6 +70,27 @@ pub(crate) fn chipset(self) -> Result<Chipset, Error> {
> 31:0 value as u32;
> });
>
> +register!(NV_PGC6_AON_SECURE_SCRATCH_GROUP_42 @ 0x001183a4 {
> + 31:0 value as u32;
> +});
> +
> +/* PDISP */
> +
> +register!(NV_PDISP_VGA_WORKSPACE_BASE @ 0x00625f04 {
> + 3:3 status_valid as bool;
> + 31:8 addr as u32;
> +});
> +
> +/* FUSE */
> +
> +register!(NV_FUSE_STATUS_OPT_DISPLAY_MAXWELL @ 0x00021c04 {
> + 0:0 display_disabled as bool;
> +});
> +
> +register!(NV_FUSE_STATUS_OPT_DISPLAY_AMPERE @ 0x00820c04 {
> + 0:0 display_disabled as bool;
> +});
> +
> /* PFALCON */
>
> register!(NV_PFALCON_FALCON_IRQSCLR @ +0x00000004 {
>
> --
> 2.49.0
>
On Wed May 14, 2025 at 1:41 AM JST, Danilo Krummrich wrote:
<snip>
>> diff --git a/drivers/gpu/nova-core/gsp/fb.rs b/drivers/gpu/nova-core/gsp/fb.rs
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..f28ded59469d52daf39e5d19c09efd7bf08fee92
>> --- /dev/null
>> +++ b/drivers/gpu/nova-core/gsp/fb.rs
>> @@ -0,0 +1,108 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +use core::ops::Range;
>> +
>> +use kernel::prelude::*;
>> +
>> +use crate::driver::Bar0;
>> +use crate::gpu::Chipset;
>> +use crate::regs;
>> +
>> +fn align_down(value: u64, align: u64) -> u64 {
>> + value & !(align - 1)
>> +}
>
> Can this go in the previous patch, i.e. "rust: num: Add an upward alignment
> helper for usize"?
Yes, let me try to consolidate things around the `num` module. Not sure
why it didn't occur to me to add that one there.
>
>> +
>> +/// Layout of the GPU framebuffer memory.
>> +///
>> +/// Contains ranges of GPU memory reserved for a given purpose during the GSP bootup process.
>> +#[derive(Debug)]
>> +#[expect(dead_code)]
>> +pub(crate) struct FbLayout {
>> + pub fb: Range<u64>,
>> +
>> + pub vga_workspace: Range<u64>,
>> + pub bios: Range<u64>,
>> +
>> + pub frts: Range<u64>,
>
> Please remove the empty lines.
>
>> +}
>> +
>> +impl FbLayout {
>> + pub(crate) fn new(chipset: Chipset, bar: &Bar0) -> Result<Self> {
>> + let fb = {
>> + let fb_size = vidmem_size(bar, chipset);
>> +
>> + 0..fb_size
>> + };
>> + let fb_len = fb.end - fb.start;
>
> Isn't this the same as fb_size? Why not just write it as
>
> let fb_size = vidmem_size(bar, chipset);
> let fb = 0..fb_size;
It is the same indeed, and fb_size and fb_len are semantically the same
thing so no need to have both.
>
>> +
>> + let vga_workspace = {
>> + let vga_base = vga_workspace_addr(bar, fb_len, chipset);
>> +
>> + vga_base..fb.end
>> + };
>> +
>> + let bios = vga_workspace.clone();
>
> Why? And why store the same thing twice in FbLayout? If it's really needed,
> clone it in the constructor below and add a comment why it's the same.
The bios field does not seem to be used at the moment anyway, so let me
remove it.
>
>> +
>> + let frts = {
>> + const FRTS_DOWN_ALIGN: u64 = 0x20000;
>> + const FRTS_SIZE: u64 = 0x100000;
>> + let frts_base = align_down(vga_workspace.start, FRTS_DOWN_ALIGN) - FRTS_SIZE;
>> +
>> + frts_base..frts_base + FRTS_SIZE
>> + };
>> +
>> + Ok(Self {
>> + fb,
>> + vga_workspace,
>> + bios,
>> + frts,
>> + })
>> + }
>> +}
>
> I'd probably wrap those helpers below in
>
> mod hal { ... }
>
> or even a new file fb/hal.rs to make their purpose obvious.
Do we want a module here? I'm fine with it, but these methods are
already private anyway and putting them under a module would require
them to have `pub(super)` visibility.
... or maybe we should have an actual HAL here with dynamic dispatch,
similar to what we have in the falcon module. That's what OpenRM does
actually. Let me look into that.
>
>> +/// Returns `true` if the display is disabled.
>> +fn display_disabled(bar: &Bar0, chipset: Chipset) -> bool {
>> + if chipset >= Chipset::GA100 {
>> + regs::NV_FUSE_STATUS_OPT_DISPLAY_MAXWELL::read(bar).display_disabled()
>> + } else {
>> + regs::NV_FUSE_STATUS_OPT_DISPLAY_AMPERE::read(bar).display_disabled()
>> + }
>> +}
>> +
>> +/// Returns the video memory size in bytes.
>> +fn vidmem_size(bar: &Bar0, chipset: Chipset) -> u64 {
>> + if chipset >= Chipset::GA102 {
>
> Is GA102 intentional or should this also be GA100?
After double-checking with OpenRM GA102 is indeed correct.
>
>> + (regs::NV_PGC6_AON_SECURE_SCRATCH_GROUP_42::read(bar).value() as u64) << 20
>
> Why isn't this shift part of the register abstraction?
This value came from a scratch register, which interpretation depends on
context so I did not abstract the values. But OpenRM does, so let me add
a way to create alias registers so we can get adequate definitions here
as well.
>
>> + } else {
>> + let local_mem_range = regs::NV_PFB_PRI_MMU_LOCAL_MEMORY_RANGE::read(bar);
>> + let size =
>> + (local_mem_range.lower_mag() as u64) << ((local_mem_range.lower_scale() as u64) + 20);
>
> Same here. Please make this part of the register abstraction as it is done in
> previous patches.
Ack.
>
>> +
>> + if local_mem_range.ecc_mode_enabled() {
>> + size / 16 * 15
>> + } else {
>> + size
>> + }
>> + }
>> +}
>> +
>> +/// Returns the vga workspace address.
>> +fn vga_workspace_addr(bar: &Bar0, fb_size: u64, chipset: Chipset) -> u64 {
>> + let base = fb_size - 0x100000;
>
> What's this offset? How do you guarantee that this never underflows?
Looked it up in OpenRM, it is the size of PRAMIN. I'll add a constant.
>
>> + let vga_workspace_base = if display_disabled(bar, chipset) {
>> + regs::NV_PDISP_VGA_WORKSPACE_BASE::read(bar)
>> + } else {
>> + return base;
>> + };
>> +
>> + if !vga_workspace_base.status_valid() {
>> + return base;
>> + }
>> +
>> + let addr = (vga_workspace_base.addr() as u64) << 16;
>
> Where does this shift come from? Shouldn't this be part of the register
> abstraction?
Yes. Also added documentation in the field to explain that the field's
lowest 16 bits are truncated.
>
>> + if addr < base {
>> + fb_size - 0x20000
>
> What's this offset? Can this ever underflow?
This one is also defined as a constant in OpenRM. Let me replicate it
here.
© 2016 - 2025 Red Hat, Inc.