| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <9da480a7-f8ff-4b58-b474-378485762c7c@nvidia.com>
Date: Tue, 14 Oct 2025 19:08:51 -0700
From: John Hubbard <jhubbard@...dia.com>
To: Joel Fernandes <joelagnelf@...dia.com>, linux-kernel@...r.kernel.org,
rust-for-linux@...r.kernel.org, dri-devel@...ts.freedesktop.org,
dakr@...nel.org, acourbot@...dia.com
Cc: Alistair Popple <apopple@...dia.com>, Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>,
Gary Guo <gary@...yguo.net>, bjorn3_gh@...tonmail.com,
Benno Lossin <lossin@...nel.org>, Andreas Hindborg <a.hindborg@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
Timur Tabi <ttabi@...dia.com>, joel@...lfernandes.org,
Elle Rhumsaa <elle@...thered-steel.dev>,
Daniel Almeida <daniel.almeida@...labora.com>,
Andrea Righi <arighi@...dia.com>, nouveau@...ts.freedesktop.org
Subject: Re: [PATCH RFC] nova-core: mm: Add initial structures for page table
management
On 10/6/25 2:09 PM, Joel Fernandes wrote:
> Add initial structures and helpers for page table management. Will
> expand and build on this in later patches. Uses bitstruct for clean
> code.
>
> Signed-off-by: Joel Fernandes <joelagnelf@...dia.com>
> ---
> Rebased on rust-drm-next + bitfield patches [1].
>
> [1] https://lore.kernel.org/all/20251003154748.1687160-1-joelagnelf@nvidia.com/
>
> drivers/gpu/nova-core/falcon/gsp.rs | 2 +-
> drivers/gpu/nova-core/mm/mod.rs | 3 +
> drivers/gpu/nova-core/mm/types.rs | 382 ++++++++++++++++++++++++++++
> drivers/gpu/nova-core/nova_core.rs | 1 +
> 4 files changed, 387 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/nova-core/mm/mod.rs
> create mode 100644 drivers/gpu/nova-core/mm/types.rs
>
Hi Joel,
This will be easier to provide a meaningful review for, once there are
some callers to look at. Structures alone are very difficult to review,
in the absence of behaviors (callers).
thanks,
John Hubbard
> diff --git a/drivers/gpu/nova-core/falcon/gsp.rs b/drivers/gpu/nova-core/falcon/gsp.rs
> index cd4960e997c8..839c803001b5 100644
> --- a/drivers/gpu/nova-core/falcon/gsp.rs
> +++ b/drivers/gpu/nova-core/falcon/gsp.rs
> @@ -5,7 +5,7 @@
> use crate::{
> driver::Bar0,
> falcon::{Falcon, FalconEngine, PFalcon2Base, PFalconBase},
> - regs::self,
> + regs,
> };
>
> /// Type specifying the `Gsp` falcon engine. Cannot be instantiated.
> diff --git a/drivers/gpu/nova-core/mm/mod.rs b/drivers/gpu/nova-core/mm/mod.rs
> new file mode 100644
> index 000000000000..4cebfa3cf184
> --- /dev/null
> +++ b/drivers/gpu/nova-core/mm/mod.rs
> @@ -0,0 +1,3 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +pub(crate) mod types;
> diff --git a/drivers/gpu/nova-core/mm/types.rs b/drivers/gpu/nova-core/mm/types.rs
> new file mode 100644
> index 000000000000..6eca8bcb24a6
> --- /dev/null
> +++ b/drivers/gpu/nova-core/mm/types.rs
> @@ -0,0 +1,382 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//! Page table data management for NVIDIA GPUs
> +
> +#![expect(dead_code)]
> +
> +use kernel::bitfield;
> +
> +/// Memory size constants
> +pub(crate) const KB: usize = 1024;
> +pub(crate) const MB: usize = KB * 1024;
> +
> +/// Page size: 4 KiB
> +pub(crate) const PAGE_SIZE: usize = 4 * KB;
> +
> +/// Page Table Level hierarchy
> +#[derive(Debug, Clone, Copy, PartialEq)]
> +pub(crate) enum PageTableLevel {
> + Pdb, // Level 0 - Page Directory Base
> + L1, // Level 1
> + L2, // Level 2
> + L3, // Level 3 - Dual PDE (128-bit entries)
> + L4, // Level 4 - PTEs
> +}
> +
> +impl PageTableLevel {
> + /// Get the entry size for this level
> + pub(crate) fn entry_size(&self) -> usize {
> + match self {
> + Self::L3 => 16, // 128-bit dual PDE
> + _ => 8, // 64-bit PDE/PTE
> + }
> + }
> +
> + /// PDE levels constant array for iteration
> + const PDE_LEVELS: [PageTableLevel; 4] = [
> + PageTableLevel::Pdb,
> + PageTableLevel::L1,
> + PageTableLevel::L2,
> + PageTableLevel::L3,
> + ];
> +
> + /// Get iterator over PDE levels
> + pub(crate) fn pde_levels() -> impl Iterator<Item = PageTableLevel> {
> + Self::PDE_LEVELS.into_iter()
> + }
> +}
> +
> +/// Memory aperture for Page Directory Entries (PDEs)
> +#[repr(u8)]
> +#[derive(Debug, Clone, Copy, PartialEq, Default)]
> +pub(crate) enum AperturePde {
> + #[default]
> + Invalid = 0,
> + VideoMemory = 1,
> + SystemCoherent = 2,
> + SystemNonCoherent = 3,
> +}
> +
> +impl From<u8> for AperturePde {
> + fn from(val: u8) -> Self {
> + match val {
> + 1 => Self::VideoMemory,
> + 2 => Self::SystemCoherent,
> + 3 => Self::SystemNonCoherent,
> + _ => Self::Invalid,
> + }
> + }
> +}
> +
> +impl From<AperturePde> for u8 {
> + fn from(val: AperturePde) -> Self {
> + val as u8
> + }
> +}
> +
> +impl From<AperturePde> for u64 {
> + fn from(val: AperturePde) -> Self {
> + val as u64
> + }
> +}
> +
> +/// Memory aperture for Page Table Entries (PTEs)
> +#[repr(u8)]
> +#[derive(Debug, Clone, Copy, PartialEq, Default)]
> +pub(crate) enum AperturePte {
> + #[default]
> + VideoMemory = 0,
> + PeerVideoMemory = 1,
> + SystemCoherent = 2,
> + SystemNonCoherent = 3,
> +}
> +
> +impl From<u8> for AperturePte {
> + fn from(val: u8) -> Self {
> + match val {
> + 0 => Self::VideoMemory,
> + 1 => Self::PeerVideoMemory,
> + 2 => Self::SystemCoherent,
> + 3 => Self::SystemNonCoherent,
> + _ => Self::VideoMemory,
> + }
> + }
> +}
> +
> +impl From<AperturePte> for u8 {
> + fn from(val: AperturePte) -> Self {
> + val as u8
> + }
> +}
> +
> +impl From<AperturePte> for u64 {
> + fn from(val: AperturePte) -> Self {
> + val as u64
> + }
> +}
> +
> +/// Common trait for address types
> +pub(crate) trait Address {
> + /// Get raw u64 value
> + fn raw(&self) -> u64;
> +
> + /// Get the frame number (upper 52 bits)
> + fn frame_number(&self) -> u64 {
> + self.raw() >> 12
> + }
> +
> + /// Get the offset within the frame (lower 12 bits)
> + fn frame_offset(&self) -> u16 {
> + (self.raw() & 0xFFF) as u16
> + }
> +
> + /// Convert to usize
> + fn to_usize(&self) -> usize {
> + self.raw() as usize
> + }
> +}
> +
> +// VRAM address representation - used with Pfn for GPU video memory allocation
> +// and page table entries.
> +bitfield! {
> + pub(crate) struct VramAddress(u64) {
> + 11:0 offset as u16; // Offset within 4KB page
> + 63:12 frame_number as u64; // Frame number (52 bits)
> + }
> +}
> +
> +impl Address for VramAddress {
> + fn raw(&self) -> u64 {
> + self.0
> + }
> +}
> +
> +impl From<u64> for VramAddress {
> + fn from(val: u64) -> Self {
> + VramAddress(val)
> + }
> +}
> +
> +impl From<usize> for VramAddress {
> + fn from(val: usize) -> Self {
> + VramAddress(val as u64)
> + }
> +}
> +
> +impl From<Pfn> for VramAddress {
> + fn from(pfn: Pfn) -> VramAddress {
> + VramAddress::default().set_frame_number(pfn.raw())
> + }
> +}
> +
> +// Virtual address
> +bitfield! {
> + pub(crate) struct VirtualAddress(u64) {
> + 11:0 offset as u16; // Offset within 4KB page
> + 20:12 l4_index as u16; // Level 4 index (PTE)
> + 29:21 l3_index as u16; // Level 3 index
> + 38:30 l2_index as u16; // Level 2 index
> + 47:39 l1_index as u16; // Level 1 index
> + 56:48 l0_index as u16; // Level 0 index (PDB)
> + }
> +}
> +
> +impl VirtualAddress {
> + /// Get index for a specific page table level
> + pub(crate) fn level_index(&self, level: PageTableLevel) -> u16 {
> + match level {
> + PageTableLevel::Pdb => self.l0_index(),
> + PageTableLevel::L1 => self.l1_index(),
> + PageTableLevel::L2 => self.l2_index(),
> + PageTableLevel::L3 => self.l3_index(),
> + PageTableLevel::L4 => self.l4_index(),
> + }
> + }
> +}
> +
> +impl Address for VirtualAddress {
> + fn raw(&self) -> u64 {
> + self.0
> + }
> +}
> +
> +impl From<u64> for VirtualAddress {
> + fn from(addr: u64) -> Self {
> + Self(addr)
> + }
> +}
> +
> +impl From<Vfn> for VirtualAddress {
> + fn from(vfn: Vfn) -> VirtualAddress {
> + VirtualAddress::from(vfn.raw() << 12)
> + }
> +}
> +
> +// Page Table Entry (PTE) - 64-bit entry at level 4
> +// Note: PTE has V=1 for valid, and includes aperture bits
> +bitfield! {
> + pub(crate) struct Pte(u64) {
> + 0:0 valid as bool; // (1 = valid for PTEs)
> + 1:1 privilege as bool; // P - Privileged/kernel-only access
> + 2:2 read_only as bool; // RO - Write protection
> + 3:3 atomic_disable as bool; // AD - Disable atomic ops
> + 4:4 encrypted as bool; // E - Encryption enabled
> + 39:8 frame_number as u64; // PA[39:8] - Physical frame number (32 bits)
> + 41:40 aperture as u8 => AperturePte; // Memory aperture type.
> + 42:42 volatile as bool; // VOL - Volatile flag
> + 50:43 kind as u8; // K[7:0] - Compression/tiling kind
> + 63:51 comptag_line as u16; // CTL[12:0] - Compression tag line (partial, shared with PA)
> + }
> +}
> +
> +impl Pte {
> + /// Set the address for this PTE
> + pub(crate) fn set_address(&mut self, addr: VramAddress) {
> + self.set_frame_number(addr.frame_number());
> + }
> +
> + /// Get the address from this PTE
> + pub(crate) fn address(&self) -> VramAddress {
> + VramAddress::default().set_frame_number(self.frame_number())
> + }
> +
> + /// Get raw u64 value
> + pub(crate) fn raw(&self) -> u64 {
> + self.0
> + }
> +}
> +
> +// Page Directory Entry (PDE) - 64-bit entry at levels 0-2
> +// Note: V=0 means valid for PDEs (inverted), aperture=0 means invalid
> +bitfield! {
> + pub(crate) struct Pde(u64) {
> + 0:0 valid_inverted as bool; // V - Valid bit (0=valid for PDEs)
> + 2:1 aperture as u8 => AperturePde; // Memory aperture type
> + 3:3 volatile as bool; // VOL - Volatile flag
> + 39:8 table_frame_number as u64; // PA[39:8] - Table frame number (32 bits)
> + }
> +}
> +
> +impl Pde {
> + /// Check if PDE is valid
> + pub(crate) fn is_valid(&self) -> bool {
> + !self.valid_inverted() && self.aperture() != AperturePde::Invalid
> + }
> +
> + /// The valid bit is inverted so add an accessor to flip it
> + pub(crate) fn set_valid(&self, value: bool) -> Pde {
> + self.set_valid_inverted(!value)
> + }
> +
> + /// Set the table address for this PDE
> + pub(crate) fn set_table_address(&mut self, addr: VramAddress) {
> + self.set_table_frame_number(addr.frame_number());
> + }
> +
> + /// Get the table address from this PDE
> + pub(crate) fn table_address(&self) -> VramAddress {
> + VramAddress::default().set_frame_number(self.table_frame_number())
> + }
> +
> + /// Get raw u64 value
> + pub(crate) fn raw(&self) -> u64 {
> + self.0
> + }
> +}
> +
> +/// Dual PDE at Level 3 - 128-bit entry containing both LPT and SPT pointers
> +/// Lower 64 bits = big/large page, upper 64 bits = small page
> +#[repr(C)]
> +#[derive(Debug, Clone, Copy)]
> +pub(crate) struct DualPde {
> + pub lpt: Pde, // Large/Big Page Table pointer (2MB pages) - bits 63:0 (lower)
> + pub spt: Pde, // Small Page Table pointer (4KB pages) - bits 127:64 (upper)
> +}
> +
> +impl DualPde {
> + /// Create a new empty dual PDE
> + pub(crate) fn new() -> Self {
> + Self {
> + spt: Pde::default(),
> + lpt: Pde::default(),
> + }
> + }
> +
> + /// Set the Small Page Table address with aperture
> + pub(crate) fn set_small_pt_address(&mut self, addr: VramAddress, aperture: AperturePde) {
> + self.spt = Pde::default()
> + .set_valid(true)
> + .set_table_frame_number(addr.frame_number())
> + .set_aperture(aperture);
> + }
> +
> + /// Set the Large Page Table address with aperture
> + pub(crate) fn set_large_pt_address(&mut self, addr: VramAddress, aperture: AperturePde) {
> + self.lpt = Pde::default()
> + .set_valid(true)
> + .set_table_frame_number(addr.frame_number())
> + .set_aperture(aperture);
> + }
> +
> + /// Check if has valid Small Page Table
> + pub(crate) fn has_small_pt_address(&self) -> bool {
> + self.spt.is_valid()
> + }
> +
> + /// Check if has valid Large Page Table
> + pub(crate) fn has_large_pt_address(&self) -> bool {
> + self.lpt.is_valid()
> + }
> +
> + /// Set SPT (Small Page Table) using Pfn
> + pub(crate) fn set_spt(&mut self, pfn: Pfn, aperture: AperturePde) {
> + self.spt = Pde::default()
> + .set_valid(true)
> + .set_aperture(aperture)
> + .set_table_frame_number(pfn.raw());
> + }
> +}
> +
> +/// Virtual Frame Number - virtual address divided by 4KB
> +#[derive(Debug, Clone, Copy, PartialEq, Eq)]
> +pub(crate) struct Vfn(u64);
> +
> +impl Vfn {
> + /// Create a new VFN from a frame number
> + pub(crate) const fn new(frame_number: u64) -> Self {
> + Self(frame_number)
> + }
> +
> + /// Get raw frame number
> + pub(crate) const fn raw(&self) -> u64 {
> + self.0
> + }
> +}
> +
> +impl From<VirtualAddress> for Vfn {
> + fn from(vaddr: VirtualAddress) -> Self {
> + Self(vaddr.frame_number())
> + }
> +}
> +
> +/// Physical Frame Number - physical address divided by 4KB
> +#[derive(Debug, Clone, Copy, PartialEq, Eq)]
> +pub(crate) struct Pfn(u64);
> +
> +impl Pfn {
> + /// Create a new PFN from a frame number
> + pub(crate) const fn new(frame_number: u64) -> Self {
> + Self(frame_number)
> + }
> +
> + /// Get raw frame number
> + pub(crate) const fn raw(&self) -> u64 {
> + self.0
> + }
> +}
> +
> +impl From<VramAddress> for Pfn {
> + fn from(addr: VramAddress) -> Self {
> + let frame = addr.frame_number();
> + Self(frame)
> + }
> +}
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> index fffcaee2249f..ff994f1ec9b9 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -10,6 +10,7 @@
> mod gfw;
> mod gpu;
> mod gsp;
> +mod mm;
> mod regs;
> mod util;
> mod vbios;
Powered by blists - more mailing lists