[PATCH 1/2] gpu: nova-core: add initial driver stub

[PATCH 1/2] gpu: nova-core: add initial driver stub

[Danilo Krummrich]

Add the initial nova-core driver stub.

nova-core is intended to serve as a common base for nova-drm (the
corresponding DRM driver) and the vGPU manager VFIO driver, serving as a
hard- and firmware abstraction layer for GSP-based NVIDIA GPUs.

The Nova project, including nova-core and nova-drm, in the long term,
is intended to serve as the successor of Nouveau for all GSP-based GPUs.

The motivation for both, starting a successor project for Nouveau and
doing so using the Rust programming language, is documented in detail
through a previous post on the mailing list [1], an LWN article [2] and a
talk from LPC '24.

In order to avoid the chicken and egg problem to require a user to
upstream Rust abstractions, but at the same time require the Rust
abstractions to implement the driver, nova-core kicks off as a driver
stub and is subsequently developed upstream.

Link: https://lore.kernel.org/dri-devel/Zfsj0_tb-0-tNrJy@cassiopeiae/T/#u [1]
Link: https://lwn.net/Articles/990736/ [2]
Link: https://youtu.be/3Igmx28B3BQ?si=sBdSEer4tAPKGpOs [3]
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 MAINTAINERS                        |  10 ++
 drivers/gpu/Makefile               |   1 +
 drivers/gpu/nova-core/Kconfig      |  13 +++
 drivers/gpu/nova-core/Makefile     |   3 +
 drivers/gpu/nova-core/driver.rs    |  47 ++++++++
 drivers/gpu/nova-core/gpu.rs       | 171 +++++++++++++++++++++++++++++
 drivers/gpu/nova-core/nova_core.rs |  14 +++
 drivers/video/Kconfig              |   1 +
 8 files changed, 260 insertions(+)
 create mode 100644 drivers/gpu/nova-core/Kconfig
 create mode 100644 drivers/gpu/nova-core/Makefile
 create mode 100644 drivers/gpu/nova-core/driver.rs
 create mode 100644 drivers/gpu/nova-core/gpu.rs
 create mode 100644 drivers/gpu/nova-core/nova_core.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index d1086e53a317..f7ddca7de0ef 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7446,6 +7446,16 @@ T:	git https://gitlab.freedesktop.org/drm/nouveau.git
 F:	drivers/gpu/drm/nouveau/
 F:	include/uapi/drm/nouveau_drm.h
 
+CORE DRIVER FOR NVIDIA GPUS [RUST]
+M:	Danilo Krummrich <dakr@kernel.org>
+L:	nouveau@lists.freedesktop.org
+S:	Supported
+Q:	https://patchwork.freedesktop.org/project/nouveau/
+B:	https://gitlab.freedesktop.org/drm/nova/-/issues
+C:	irc://irc.oftc.net/nouveau
+T:	git https://gitlab.freedesktop.org/drm/nova.git nova-next
+F:	drivers/gpu/nova-core/
+
 DRM DRIVER FOR OLIMEX LCD-OLINUXINO PANELS
 M:	Stefan Mavrodiev <stefan@olimex.com>
 S:	Maintained
diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
index 8997f0096545..36a54d456630 100644
--- a/drivers/gpu/Makefile
+++ b/drivers/gpu/Makefile
@@ -5,3 +5,4 @@
 obj-y			+= host1x/ drm/ vga/
 obj-$(CONFIG_IMX_IPUV3_CORE)	+= ipu-v3/
 obj-$(CONFIG_TRACE_GPU_MEM)		+= trace/
+obj-$(CONFIG_NOVA_CORE)		+= nova-core/
diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
new file mode 100644
index 000000000000..33ac937b244a
--- /dev/null
+++ b/drivers/gpu/nova-core/Kconfig
@@ -0,0 +1,13 @@
+config NOVA_CORE
+	tristate "Nova Core GPU driver"
+	depends on PCI
+	depends on RUST
+	depends on RUST_FW_LOADER_ABSTRACTIONS
+	default n
+	help
+	  Choose this if you want to build the Nova Core driver for Nvidia
+	  GSP-based GPUs.
+
+	  This driver is work in progress and may not be functional.
+
+	  If M is selected, the module will be called nova-core.
diff --git a/drivers/gpu/nova-core/Makefile b/drivers/gpu/nova-core/Makefile
new file mode 100644
index 000000000000..2d78c50126e1
--- /dev/null
+++ b/drivers/gpu/nova-core/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_NOVA_CORE) += nova_core.o
diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
new file mode 100644
index 000000000000..2a2aa9b0630b
--- /dev/null
+++ b/drivers/gpu/nova-core/driver.rs
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use kernel::{bindings, c_str, pci, prelude::*};
+
+use crate::gpu::Gpu;
+
+#[pin_data]
+pub(crate) struct NovaCore {
+    #[pin]
+    pub(crate) gpu: Gpu,
+}
+
+const BAR0_SIZE: usize = 8;
+pub(crate) type Bar0 = pci::Bar<BAR0_SIZE>;
+
+kernel::pci_device_table!(
+    PCI_TABLE,
+    MODULE_PCI_TABLE,
+    <NovaCore as pci::Driver>::IdInfo,
+    [(
+        pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_NVIDIA, bindings::PCI_ANY_ID as _),
+        ()
+    )]
+);
+
+impl pci::Driver for NovaCore {
+    type IdInfo = ();
+    const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
+
+    fn probe(pdev: &mut pci::Device, _info: &Self::IdInfo) -> Result<Pin<KBox<Self>>> {
+        dev_dbg!(pdev.as_ref(), "Probe Nova Core GPU driver.\n");
+
+        pdev.enable_device_mem()?;
+        pdev.set_master();
+
+        let bar = pdev.iomap_region_sized::<BAR0_SIZE>(0, c_str!("nova-core"))?;
+
+        let this = KBox::pin_init(
+            try_pin_init!(Self {
+                gpu <- Gpu::new(pdev, bar)?,
+            }),
+            GFP_KERNEL,
+        )?;
+
+        Ok(this)
+    }
+}
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
new file mode 100644
index 000000000000..cf62390e72eb
--- /dev/null
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use kernel::{
+    device, devres::Devres, error::code::*, firmware, fmt, pci, prelude::*, str::CString,
+};
+
+use crate::driver::Bar0;
+use core::fmt::Debug;
+
+/// Enum representation of the GPU chipset.
+#[derive(Debug)]
+pub(crate) enum Chipset {
+    TU102 = 0x162,
+    TU104 = 0x164,
+    TU106 = 0x166,
+    TU117 = 0x167,
+    TU116 = 0x168,
+    GA102 = 0x172,
+    GA103 = 0x173,
+    GA104 = 0x174,
+    GA106 = 0x176,
+    GA107 = 0x177,
+    AD102 = 0x192,
+    AD103 = 0x193,
+    AD104 = 0x194,
+    AD106 = 0x196,
+    AD107 = 0x197,
+}
+
+/// Enum representation of the GPU generation.
+#[derive(Debug)]
+pub(crate) enum CardType {
+    /// Turing
+    TU100 = 0x160,
+    /// Ampere
+    GA100 = 0x170,
+    /// Ada Lovelace
+    AD100 = 0x190,
+}
+
+/// Structure holding the metadata of the GPU.
+#[allow(dead_code)]
+pub(crate) struct GpuSpec {
+    /// Contents of the boot0 register.
+    boot0: u64,
+    card_type: CardType,
+    chipset: Chipset,
+    /// The revision of the chipset.
+    chiprev: u8,
+}
+
+/// Structure encapsulating the firmware blobs required for the GPU to operate.
+#[allow(dead_code)]
+pub(crate) struct Firmware {
+    booter_load: firmware::Firmware,
+    booter_unload: firmware::Firmware,
+    gsp: firmware::Firmware,
+}
+
+/// Structure holding the resources required to operate the GPU.
+#[allow(dead_code)]
+#[pin_data]
+pub(crate) struct Gpu {
+    spec: GpuSpec,
+    /// MMIO mapping of PCI BAR 0
+    bar: Devres<Bar0>,
+    fw: Firmware,
+}
+
+// TODO replace with something like derive(FromPrimitive)
+impl Chipset {
+    fn from_u32(value: u32) -> Option<Chipset> {
+        match value {
+            0x162 => Some(Chipset::TU102),
+            0x164 => Some(Chipset::TU104),
+            0x166 => Some(Chipset::TU106),
+            0x167 => Some(Chipset::TU117),
+            0x168 => Some(Chipset::TU116),
+            0x172 => Some(Chipset::GA102),
+            0x173 => Some(Chipset::GA103),
+            0x174 => Some(Chipset::GA104),
+            0x176 => Some(Chipset::GA106),
+            0x177 => Some(Chipset::GA107),
+            0x192 => Some(Chipset::AD102),
+            0x193 => Some(Chipset::AD103),
+            0x194 => Some(Chipset::AD104),
+            0x196 => Some(Chipset::AD106),
+            0x197 => Some(Chipset::AD107),
+            _ => None,
+        }
+    }
+}
+
+// TODO replace with something like derive(FromPrimitive)
+impl CardType {
+    fn from_u32(value: u32) -> Option<CardType> {
+        match value {
+            0x160 => Some(CardType::TU100),
+            0x170 => Some(CardType::GA100),
+            0x190 => Some(CardType::AD100),
+            _ => None,
+        }
+    }
+}
+
+impl GpuSpec {
+    fn new(bar: &Devres<Bar0>) -> Result<GpuSpec> {
+        let bar = bar.try_access().ok_or(ENXIO)?;
+        let boot0 = u64::from_le(bar.readq(0));
+        let chip = ((boot0 & 0x1ff00000) >> 20) as u32;
+
+        if boot0 & 0x1f000000 == 0 {
+            return Err(ENODEV);
+        }
+
+        let Some(chipset) = Chipset::from_u32(chip) else {
+            return Err(ENODEV);
+        };
+
+        let Some(card_type) = CardType::from_u32(chip & 0x1f0) else {
+            return Err(ENODEV);
+        };
+
+        Ok(Self {
+            boot0,
+            card_type,
+            chipset,
+            chiprev: (boot0 & 0xff) as u8,
+        })
+    }
+}
+
+impl Firmware {
+    fn new(dev: &device::Device, spec: &GpuSpec, ver: &str) -> Result<Firmware> {
+        let mut chip_name = CString::try_from_fmt(fmt!("{:?}", spec.chipset))?;
+        chip_name.make_ascii_lowercase();
+
+        let fw_booter_load_path =
+            CString::try_from_fmt(fmt!("nvidia/{}/gsp/booter_load-{}.bin", &*chip_name, ver))?;
+        let fw_booter_unload_path =
+            CString::try_from_fmt(fmt!("nvidia/{}/gsp/booter_unload-{}.bin", &*chip_name, ver))?;
+        let fw_gsp_path =
+            CString::try_from_fmt(fmt!("nvidia/{}/gsp/gsp-{}.bin", &*chip_name, ver))?;
+
+        let booter_load = firmware::Firmware::request(&fw_booter_load_path, dev)?;
+        let booter_unload = firmware::Firmware::request(&fw_booter_unload_path, dev)?;
+        let gsp = firmware::Firmware::request(&fw_gsp_path, dev)?;
+
+        Ok(Firmware {
+            booter_load,
+            booter_unload,
+            gsp,
+        })
+    }
+}
+
+impl Gpu {
+    pub(crate) fn new(pdev: &pci::Device, bar: Devres<Bar0>) -> Result<impl PinInit<Self>> {
+        let spec = GpuSpec::new(&bar)?;
+        let fw = Firmware::new(pdev.as_ref(), &spec, "535.113.01")?;
+
+        dev_info!(
+            pdev.as_ref(),
+            "NVIDIA {:?} ({:#x})",
+            spec.chipset,
+            spec.boot0
+        );
+
+        Ok(pin_init!(Self { spec, bar, fw }))
+    }
+}
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
new file mode 100644
index 000000000000..b130d9ca6a0f
--- /dev/null
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Nova Core GPU Driver
+
+mod driver;
+mod gpu;
+
+kernel::module_pci_driver! {
+    type: driver::NovaCore,
+    name: "NovaCore",
+    author: "Danilo Krummrich",
+    description: "Nova Core GPU driver",
+    license: "GPL v2",
+}
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 44c9ef1435a2..5df981920a94 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -39,6 +39,7 @@ source "drivers/gpu/vga/Kconfig"
 
 source "drivers/gpu/host1x/Kconfig"
 source "drivers/gpu/ipu-v3/Kconfig"
+source "drivers/gpu/nova-core/Kconfig"
 
 source "drivers/gpu/drm/Kconfig"
 

base-commit: 69b8923f5003664e3ffef102e73333edfa2abdcf
-- 
2.48.1

[PATCH 2/2] gpu: nova-core: add initial documentation

[Danilo Krummrich]

Add the initial documentation of the Nova project.

The initial project documentation consists out of a brief introduction
of the project, as well as project guidelines both general and nova-core
specific and a task list for nova-core specifically.

The task list is divided into tasks for general Rust infrastructure
required by the project, tasks regarding GSP enablement and firmware
abstraction, general GPU driver tasks as well as tasks related to
external API design and test infrastructure.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 Documentation/gpu/drivers.rst              |   1 +
 Documentation/gpu/nova/core/guidelines.rst |  24 ++
 Documentation/gpu/nova/core/todo.rst       | 382 +++++++++++++++++++++
 Documentation/gpu/nova/guidelines.rst      |  73 ++++
 Documentation/gpu/nova/index.rst           |  30 ++
 MAINTAINERS                                |   1 +
 6 files changed, 511 insertions(+)
 create mode 100644 Documentation/gpu/nova/core/guidelines.rst
 create mode 100644 Documentation/gpu/nova/core/todo.rst
 create mode 100644 Documentation/gpu/nova/guidelines.rst
 create mode 100644 Documentation/gpu/nova/index.rst

diff --git a/Documentation/gpu/drivers.rst b/Documentation/gpu/drivers.rst
index 1f17ad0790d7..7c2c5dcb5fd4 100644
--- a/Documentation/gpu/drivers.rst
+++ b/Documentation/gpu/drivers.rst
@@ -24,6 +24,7 @@ GPU Driver Documentation
    panfrost
    panthor
    zynqmp
+   nova/index
 
 .. only::  subproject and html
 
diff --git a/Documentation/gpu/nova/core/guidelines.rst b/Documentation/gpu/nova/core/guidelines.rst
new file mode 100644
index 000000000000..a389d65d7982
--- /dev/null
+++ b/Documentation/gpu/nova/core/guidelines.rst
@@ -0,0 +1,24 @@
+.. SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+==========
+Guidelines
+==========
+
+This documents contains the guidelines for nova-core. Additionally, all common
+guidelines of the Nova project do apply.
+
+Driver API
+==========
+
+One main purpose of nova-core is to implement the abstraction around the
+firmware interface of GSP and provide a firmware (version) independent API for
+2nd level drivers, such as nova-drm or the vGPU manager VFIO driver.
+
+Therefore, it is not permitted to leak firmware (version) specifics, through the
+driver API, to 2nd level drivers.
+
+Acceptance Criteria
+===================
+
+- To the extend possible, patches submitted to nova-core must be tested for
+  regressions with all 2nd level drivers.
diff --git a/Documentation/gpu/nova/core/todo.rst b/Documentation/gpu/nova/core/todo.rst
new file mode 100644
index 000000000000..87a43302f03e
--- /dev/null
+++ b/Documentation/gpu/nova/core/todo.rst
@@ -0,0 +1,382 @@
+.. SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+=========
+Task List
+=========
+
+Tasks may have the following fields:
+
+- ``Complexity``: Describes the required familiarity with Rust and / or the
+  corresponding kernel APIs or subsystems. There are four different complexities,
+  ``Beginner``, ``Intermediate``, ``Advanced`` and ``Expert``.
+- ``Reference``: References to other tasks.
+- ``Link``: Links to external resources.
+- ``Contact``: The person that can be contacted for further information about
+  the task.
+
+Enablement (Rust)
+=================
+
+Tasks that are not directly related to nova-core, but are preconditions in terms
+of required APIs.
+
+FromPrimitive API
+-----------------
+
+Sometimes the need arises to convert a number to a value of an enum or a
+structure.
+
+A good example from nova-core would be the ``Chipset`` enum type, which defines
+the value ``AD102``. When probing the GPU the value ``0x192`` can be read from a
+certain register indication the chipset AD102. Hence, the enum value ``AD102``
+should be derived from the number ``0x192``. Currently, nova-core uses a custom
+implementation (``Chipset::from_u32`` for this.
+
+Instead, it would be desirable to have something like the ``FromPrimitive``
+trait [1] from the num crate.
+
+Having this generalization also helps with implementing a generic macro that
+automatically generates the corresponding mappings between a value and a number.
+
+| Complexity: Beginner
+| Link: https://docs.rs/num/latest/num/trait.FromPrimitive.html
+
+Delay / Sleep abstractions
+--------------------------
+
+Rust abstractions for the kernel's delay() and sleep() functions.
+
+There is some ongoing work from FUJITA Tomonori [1], which has not seen any updates
+since Oct. 24.
+
+| Complexity: Beginner
+| Link: https://lore.kernel.org/netdev/20241001112512.4861-2-fujita.tomonori@gmail.com/ [1]
+
+IRQ abstractions
+----------------
+
+Rust abstractions for IRQ handling.
+
+There is active ongoing work from Daniel Almeida [1] for the "core" abstractions
+to request IRQs.
+
+Besides optional review and testing work, the required ``pci::Device`` code
+around those core abstractions needs to be worked out.
+
+| Complexity: Intermediate
+| Link: https://lore.kernel.org/lkml/20250122163932.46697-1-daniel.almeida@collabora.com/ [1]
+| Contact: Daniel Almeida
+
+Page abstraction for foreign pages
+----------------------------------
+
+Rust abstractions for pages not created by the Rust page abstraction without
+direct ownership.
+
+There is active onging work from Abdiel Janulgue [1].
+
+| Complexity: Advanced
+| Link: https://lore.kernel.org/linux-mm/20241119112408.779243-1-abdiel.janulgue@gmail.com/ [1]
+
+Scatterlist / sg_table abstractions
+-----------------------------------
+
+Rust abstractions for scatterlist / sg_table.
+
+There is preceding work from Abdiel Janulgue, which hasn't made it to the
+mailing list yet.
+
+| Complexity: Intermediate
+| Contact: Abdiel Janulgue
+
+ELF utils
+---------
+
+Rust implementation of ELF header representation to retrieve section header
+tables, names, and data from an ELF-formatted images.
+
+There is preceding work from Abdiel Janulgue, which hasn't made it to the
+mailing list yet.
+
+| Complexity: Beginner
+| Contact: Abdiel Janulgue
+
+PCI MISC APIs
+-------------
+
+Extend the existing PCI device / driver abstractions by SR-IOV, config space,
+capability, MSI API abstractions.
+
+| Complexity: Beginner
+
+Auxiliary bus abstractions
+--------------------------
+
+Rust abstraction for the auxiliary bus APIs.
+
+This is needed to connect nova-core to the nova-drm driver.
+
+| Complexity: Intermediate
+
+Debugfs abstractions
+--------------------
+
+Rust abstraction for debugfs APIs.
+
+| Reference: Export GSP log buffers
+| Complexity: Beginner
+
+Vec extensions
+--------------
+
+Implement ``Vec::truncate`` and ``Vec::resize``.
+
+Currently this is used for some experimental code to parse the vBIOS.
+
+| Reference vBIOS support
+| Complexity: Beginner
+
+GPU (general)
+=============
+
+Parse firmware headers
+----------------------
+
+Parse ELF headers from the firmware files loaded from the filesystem.
+
+| Reference: ELF utils
+| Complexity: Beginner
+| Contact: Abdiel Janulgue
+
+Build radix3 page table
+-----------------------
+
+Build the radix3 page table to map the firmware.
+
+| Complexity: Intermediate
+| Contact: Abdiel Janulgue
+
+vBIOS support
+-------------
+
+Parse the vBIOS and probe the structures required for driver initialization.
+
+| Contact: Dave Airlie
+| Reference: Vec extensions
+| Complexity: Intermediate
+
+Initial Devinit support
+-----------------------
+
+Implement BIOS Device Initialization, i.e. memory sizing, waiting, PLL
+configuration.
+
+| Contact: Dave Airlie
+| Complexity: Beginner
+
+Boot Falcon controller
+----------------------
+
+Infrastructure to load and execute falcon (sec2) firmware images; handle the
+GSP falcon processor and fwsec loading.
+
+| Complexity: Advanced
+| Contact: Dave Airlie
+
+GPU Timer support
+-----------------
+
+Support for the GPU's internal timer peripheral.
+
+| Complexity: Beginner
+| Contact: Dave Airlie
+
+MMU / PT management
+-------------------
+
+Work out the architecture for MMU / page table management.
+
+We need to consider that nova-drm will need rather fine-grained control,
+especially in terms of locking, in order to be able to implement asynchronous
+Vulkan queues.
+
+While generally sharing the corresponding code is desirable, it needs to be
+evaluated how (and if at all) sharing the corresponding code is expedient.
+
+| Complexity: Expert
+
+VRAM memory allocator
+---------------------
+
+Investigate options for a VRAM memory allocator.
+
+Some possible options:
+  - Rust abstractions for
+    - RB tree (interval tree) / drm_mm
+    - maple_tree
+  - native Rust collections
+
+| Complexity: Advanced
+
+Instance Memory
+---------------
+
+Implement support for instmem (bar2) used to store page tables.
+
+| Complexity: Intermediate
+| Contact: Dave Airlie
+
+GPU System Processor (GSP)
+==========================
+
+Export GSP log buffers
+----------------------
+
+Recent patches from Timur Tabi [1] added support to expose GSP-RM log buffers
+(even after failure to probe the driver) through debugfs.
+
+This is also an interesting feature for nova-core, especially in the early days.
+
+| Link: https://lore.kernel.org/nouveau/20241030202952.694055-2-ttabi@nvidia.com/ [1]
+| Reference: Debugfs abstractions
+| Complexity: Intermediate
+
+GSP firmware abstraction
+------------------------
+
+The GSP-RM firmware API is unstable and may incompatibly change from version to
+version, in terms of data structures and semantics.
+
+This problem is one of the big motivations for using Rust for nova-core, since
+it turns out that Rust's procedural macro feature provides a rather elegant way
+to address this issue:
+
+1. generate Rust structures from the C headers in a separate namespace per version
+2. build abstraction structures (within a generic namespace) that implement the
+   firmware interfaces; annotate the differences in implementation with version
+   identifiers
+3. use a procedural macro to generate the actual per version implementation out
+   of this abstraction
+4. instantiate the correct version type one on runtime (can be sure that all
+   have the same interface because it's defined by a common trait)
+
+There is a PoC implementation of this pattern, in the context of the nova-core
+PoC driver.
+
+This task aims at refining the feature and ideally generalize it, to be usable
+by other drivers as well.
+
+| Complexity: Expert
+
+GSP message queue
+-----------------
+
+Implement low level GSP message queue (command, status) for communication
+between the kernel driver and GSP.
+
+| Complexity: Advanced
+| Contact: Dave Airlie
+
+Bootstrap GSP
+-------------
+
+Call the boot firmware to boot the GSP processor; execute initial control
+messages.
+
+| Complexity: Intermediate
+| Contact: Dave Airlie
+
+Client / Device APIs
+--------------------
+
+Implement the GSP message interface for client / device allocation and the
+corresponding client and device allocation APIs.
+
+| Complexity: Intermediate
+| Contact: Dave Airlie
+
+Bar PDE handling
+----------------
+
+Synchronize page table handling for BARs between the kernel driver and GSP.
+
+| Complexity: Beginner
+| Contact: Dave Airlie
+
+FIFO engine
+-----------
+
+Implement support for the FIFO engine, i.e. the corresponding GSP message
+interface and provide an API for chid allocation and channel handling.
+
+| Complexity: Advanced
+| Contact: Dave Airlie
+
+GR engine
+---------
+
+Implement support for the graphics engine, i.e. the corresponding GSP message
+interface and provide an API for (golden) context creation and promotion.
+
+| Complexity: Advanced
+| Contact: Dave Airlie
+
+CE engine
+---------
+
+Implement support for the copy engine, i.e. the corresponding GSP message
+interface.
+
+| Complexity: Intermediate
+| Contact: Dave Airlie
+
+VFN IRQ controller
+------------------
+
+Support for the VFN interrupt controller.
+
+| Complexity: Intermediate
+| Contact: Dave Airlie
+
+External APIs
+=============
+
+nova-core base API
+------------------
+
+Work out the common pieces of the API to connect 2nd level drivers, i.e. vGPU
+manager and nova-drm.
+
+| Complexity: Advanced
+
+vGPU manager API
+----------------
+
+Work out the API parts required by the vGPU manager, which are not covered by
+the base API.
+
+| Complexity: Advanced
+
+nova-core C API
+---------------
+
+Implement a C wrapper for the APIs required by the vGPU manager driver.
+
+| Complexity: Intermediate
+
+Testing
+=======
+
+CI pipeline
+-----------
+
+Investigate option for continuous integration testing.
+
+This can go from as simple as running KUnit tests over running (graphics) CTS to
+booting up (multiple) guest VMs to test VFIO use-cases.
+
+It might also be worth to consider the introduction of a new test suite directly
+sitting on top of the uAPI for more targeted testing and debugging. There may be
+options for collaboration / shared code with the Mesa project.
+
+| Complexity: Advanced
diff --git a/Documentation/gpu/nova/guidelines.rst b/Documentation/gpu/nova/guidelines.rst
new file mode 100644
index 000000000000..28a959f51c2c
--- /dev/null
+++ b/Documentation/gpu/nova/guidelines.rst
@@ -0,0 +1,73 @@
+.. SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+==========
+Guidelines
+==========
+
+This document describes the general project guidelines that apply to nova-core
+and nova-drm.
+
+Language
+========
+
+The Nova project uses the Rust programming language. In this context, the
+following rules apply.
+
+- Unless technically necessary otherwise (e.g. uAPI), any driver code is written
+  in Rust.
+
+- Direct FFI calls to C kernel APIs must be avoided; instead C kernel APIs
+  should be accessed through shared Rust abstractions.
+
+- Unless technically necessary, unsafe Rust code must be avoided. In case of
+  technical necessity, unsafe code should be isolated in a separate component
+  providing a safe API for other driver code to use.
+
+Style
+-----
+
+All rules of the Rust for Linux project as documented in
+:doc:`../../rust/coding-guidelines` apply. Additionally, the following rules
+apply.
+
+- Code must be formatted with the ``rustfmt`` make target.
+
+- Code submitted for inclusion into the Nova driver project must pass the Rust
+  linter, which can be enabled with ``CLIPPY=1``.
+
+Documentation
+=============
+
+The availability of proper documentation is essential in terms of scalability,
+accessability for new contributors and maintainability of a project in general,
+but especially for a driver running as complex hardware as Nova is targeting.
+
+Hence, adding documentation of any kind is very much encouraged by the project.
+
+Besides that, there are some minimum requirements.
+
+- Every non-private structure needs at least a brief doc comment explaining the
+  semantical sense of the structure, as well as potential locking and lifetime
+  requirements. It is encouraged to have the same minimum documentation for
+  non-trivial private structures.
+
+- uAPIs must be fully documented with kernel-doc comments; additionally, the
+  semantical behavior must be explained including potential special or corner
+  cases.
+
+- The APIs connecting the 1st level driver (nova-core) with 2nd level drivers
+  must be fully documented. This includes doc comments, potential locking and
+  lifetime requirements, as well as example code if applicable.
+
+- Abbreviations must be explained when introduced; terminology must be uniquely
+  defined.
+
+- Register addresses, layouts, shift values and masks must be defined properly;
+  unless obvious, the semantical sense must be documented. This only applies if
+  the author is able to obtain the corresponding information.
+
+Acceptance Criteria
+===================
+
+- Patches must only be applied if reviewed by at least one other person on the
+  mailing list; this also applies for maintainers.
diff --git a/Documentation/gpu/nova/index.rst b/Documentation/gpu/nova/index.rst
new file mode 100644
index 000000000000..2701b3f4af35
--- /dev/null
+++ b/Documentation/gpu/nova/index.rst
@@ -0,0 +1,30 @@
+.. SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+=======================
+nova NVIDIA GPU drivers
+=======================
+
+The nova driver project consists out of two separate drivers nova-core and
+nova-drm and intends to supersede the nouveau driver for NVIDIA GPUs based on
+the GPU System Processor (GSP).
+
+The following documents apply to both nova-core and nova-drm.
+
+.. toctree::
+   :titlesonly:
+
+   guidelines
+
+nova-core
+=========
+
+The nova-core driver is the core driver for NVIDIA GPUs based on GSP. nova-core,
+as the 1st level driver, provides an abstraction around the GPUs hard- and
+firmware interfaces providing a common base for 2nd level drivers, such as the
+vGPU manager VFIO driver and the nova-drm driver.
+
+.. toctree::
+   :titlesonly:
+
+   core/guidelines
+   core/todo
diff --git a/MAINTAINERS b/MAINTAINERS
index f7ddca7de0ef..07455c945834 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7454,6 +7454,7 @@ Q:	https://patchwork.freedesktop.org/project/nouveau/
 B:	https://gitlab.freedesktop.org/drm/nova/-/issues
 C:	irc://irc.oftc.net/nouveau
 T:	git https://gitlab.freedesktop.org/drm/nova.git nova-next
+F:	Documentation/gpu/nova/
 F:	drivers/gpu/nova-core/
 
 DRM DRIVER FOR OLIMEX LCD-OLINUXINO PANELS
-- 
2.48.1

Re: [PATCH 1/2] gpu: nova-core: add initial driver stub

[John Hubbard]

On 1/31/25 2:04 PM, Danilo Krummrich wrote:
> Add the initial nova-core driver stub.
> 
> nova-core is intended to serve as a common base for nova-drm (the
> corresponding DRM driver) and the vGPU manager VFIO driver, serving as a
> hard- and firmware abstraction layer for GSP-based NVIDIA GPUs.
> 
> The Nova project, including nova-core and nova-drm, in the long term,
> is intended to serve as the successor of Nouveau for all GSP-based GPUs.
> 
> The motivation for both, starting a successor project for Nouveau and
> doing so using the Rust programming language, is documented in detail
> through a previous post on the mailing list [1], an LWN article [2] and a
> talk from LPC '24.
> 
> In order to avoid the chicken and egg problem to require a user to
> upstream Rust abstractions, but at the same time require the Rust
> abstractions to implement the driver, nova-core kicks off as a driver
> stub and is subsequently developed upstream.
> 
> Link: https://lore.kernel.org/dri-devel/Zfsj0_tb-0-tNrJy@cassiopeiae/T/#u [1]
> Link: https://lwn.net/Articles/990736/ [2]
> Link: https://youtu.be/3Igmx28B3BQ?si=sBdSEer4tAPKGpOs [3]
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>   MAINTAINERS                        |  10 ++
>   drivers/gpu/Makefile               |   1 +
>   drivers/gpu/nova-core/Kconfig      |  13 +++
>   drivers/gpu/nova-core/Makefile     |   3 +
>   drivers/gpu/nova-core/driver.rs    |  47 ++++++++
>   drivers/gpu/nova-core/gpu.rs       | 171 +++++++++++++++++++++++++++++
>   drivers/gpu/nova-core/nova_core.rs |  14 +++
>   drivers/video/Kconfig              |   1 +
>   8 files changed, 260 insertions(+)
>   create mode 100644 drivers/gpu/nova-core/Kconfig
>   create mode 100644 drivers/gpu/nova-core/Makefile
>   create mode 100644 drivers/gpu/nova-core/driver.rs
>   create mode 100644 drivers/gpu/nova-core/gpu.rs
>   create mode 100644 drivers/gpu/nova-core/nova_core.rs

Hi Danilo,

This is pleasantly clean, and even elegant. I was pleasantly surprised at
the level of firmware loading support in Rust, and how this approach takes
advantage of our r535 firmware snapshot that is in Turing, Ampere, and Ada.

It loads up on my GA104 system and I've been poking around at it.

Some minor comments below, but this looks like a very good starting "stub"
to get merged.

> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d1086e53a317..f7ddca7de0ef 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7446,6 +7446,16 @@ T:	git https://gitlab.freedesktop.org/drm/nouveau.git
>   F:	drivers/gpu/drm/nouveau/
>   F:	include/uapi/drm/nouveau_drm.h
>   
> +CORE DRIVER FOR NVIDIA GPUS [RUST]
> +M:	Danilo Krummrich <dakr@kernel.org>
> +L:	nouveau@lists.freedesktop.org
> +S:	Supported
> +Q:	https://patchwork.freedesktop.org/project/nouveau/

Are you sure? I'm not sure how patchwork things work, but it seems
unfortunate to confuse Nova and nouveau here.

> +B:	https://gitlab.freedesktop.org/drm/nova/-/issues
> +C:	irc://irc.oftc.net/nouveau
> +T:	git https://gitlab.freedesktop.org/drm/nova.git nova-next
> +F:	drivers/gpu/nova-core/
> +
>   DRM DRIVER FOR OLIMEX LCD-OLINUXINO PANELS
>   M:	Stefan Mavrodiev <stefan@olimex.com>
>   S:	Maintained
> diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
> index 8997f0096545..36a54d456630 100644
> --- a/drivers/gpu/Makefile
> +++ b/drivers/gpu/Makefile
> @@ -5,3 +5,4 @@
>   obj-y			+= host1x/ drm/ vga/
>   obj-$(CONFIG_IMX_IPUV3_CORE)	+= ipu-v3/
>   obj-$(CONFIG_TRACE_GPU_MEM)		+= trace/
> +obj-$(CONFIG_NOVA_CORE)		+= nova-core/
> diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
> new file mode 100644
> index 000000000000..33ac937b244a
> --- /dev/null
> +++ b/drivers/gpu/nova-core/Kconfig
> @@ -0,0 +1,13 @@
> +config NOVA_CORE
> +	tristate "Nova Core GPU driver"
> +	depends on PCI
> +	depends on RUST
> +	depends on RUST_FW_LOADER_ABSTRACTIONS
> +	default n
> +	help
> +	  Choose this if you want to build the Nova Core driver for Nvidia
> +	  GSP-based GPUs.

Maybe a little note about what "GSP" is and how you know if you have it,
would help. Turing and later architectures have GSP firmware. All the
user might know is the GPU architecture, I wouldn't expect the user
to know if it has a "GSP".

> +
> +	  This driver is work in progress and may not be functional.
> +
> +	  If M is selected, the module will be called nova-core.

Or nova_core? I realize the driver core translates between "-" and "_",
just trying to be consistent.

It does show up as /sys/module/nova_core .


> diff --git a/drivers/gpu/nova-core/Makefile b/drivers/gpu/nova-core/Makefile
> new file mode 100644
> index 000000000000..2d78c50126e1
> --- /dev/null
> +++ b/drivers/gpu/nova-core/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_NOVA_CORE) += nova_core.o
> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> new file mode 100644
> index 000000000000..2a2aa9b0630b
> --- /dev/null
> +++ b/drivers/gpu/nova-core/driver.rs
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use kernel::{bindings, c_str, pci, prelude::*};
> +
> +use crate::gpu::Gpu;
> +
> +#[pin_data]
> +pub(crate) struct NovaCore {
> +    #[pin]
> +    pub(crate) gpu: Gpu,
> +}
> +
> +const BAR0_SIZE: usize = 8;
> +pub(crate) type Bar0 = pci::Bar<BAR0_SIZE>;
> +
> +kernel::pci_device_table!(
> +    PCI_TABLE,
> +    MODULE_PCI_TABLE,
> +    <NovaCore as pci::Driver>::IdInfo,
> +    [(
> +        pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_NVIDIA, bindings::PCI_ANY_ID as _),
> +        ()
> +    )]
> +);
> +
> +impl pci::Driver for NovaCore {
> +    type IdInfo = ();
> +    const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
> +
> +    fn probe(pdev: &mut pci::Device, _info: &Self::IdInfo) -> Result<Pin<KBox<Self>>> {
> +        dev_dbg!(pdev.as_ref(), "Probe Nova Core GPU driver.\n");
> +
> +        pdev.enable_device_mem()?;
> +        pdev.set_master();
> +
> +        let bar = pdev.iomap_region_sized::<BAR0_SIZE>(0, c_str!("nova-core"))?;

Another question about whether it should be nova-core or nova_core.

> +
> +        let this = KBox::pin_init(
> +            try_pin_init!(Self {
> +                gpu <- Gpu::new(pdev, bar)?,
> +            }),
> +            GFP_KERNEL,
> +        )?;
> +
> +        Ok(this)
> +    }
> +}
> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> new file mode 100644
> index 000000000000..cf62390e72eb
> --- /dev/null
> +++ b/drivers/gpu/nova-core/gpu.rs
> @@ -0,0 +1,171 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use kernel::{
> +    device, devres::Devres, error::code::*, firmware, fmt, pci, prelude::*, str::CString,
> +};
> +
> +use crate::driver::Bar0;
> +use core::fmt::Debug;
> +
> +/// Enum representation of the GPU chipset.
> +#[derive(Debug)]
> +pub(crate) enum Chipset {
> +    TU102 = 0x162,
> +    TU104 = 0x164,
> +    TU106 = 0x166,
> +    TU117 = 0x167,
> +    TU116 = 0x168,
> +    GA102 = 0x172,
> +    GA103 = 0x173,
> +    GA104 = 0x174,
> +    GA106 = 0x176,
> +    GA107 = 0x177,
> +    AD102 = 0x192,
> +    AD103 = 0x193,
> +    AD104 = 0x194,
> +    AD106 = 0x196,
> +    AD107 = 0x197,
> +}
> +
> +/// Enum representation of the GPU generation.
> +#[derive(Debug)]
> +pub(crate) enum CardType {
> +    /// Turing
> +    TU100 = 0x160,
> +    /// Ampere
> +    GA100 = 0x170,
> +    /// Ada Lovelace
> +    AD100 = 0x190,
> +}
> +
> +/// Structure holding the metadata of the GPU.
> +#[allow(dead_code)]
> +pub(crate) struct GpuSpec {
> +    /// Contents of the boot0 register.
> +    boot0: u64,

It is redundant to store boot0, when all of the following fields
are deduced from boot0.

> +    card_type: CardType,
> +    chipset: Chipset,
> +    /// The revision of the chipset.
> +    chiprev: u8,
> +}
> +
> +/// Structure encapsulating the firmware blobs required for the GPU to operate.
> +#[allow(dead_code)]
> +pub(crate) struct Firmware {
> +    booter_load: firmware::Firmware,
> +    booter_unload: firmware::Firmware,
> +    gsp: firmware::Firmware,
> +}
> +
> +/// Structure holding the resources required to operate the GPU.
> +#[allow(dead_code)]
> +#[pin_data]
> +pub(crate) struct Gpu {
> +    spec: GpuSpec,
> +    /// MMIO mapping of PCI BAR 0
> +    bar: Devres<Bar0>,
> +    fw: Firmware,
> +}
> +
> +// TODO replace with something like derive(FromPrimitive)
> +impl Chipset {
> +    fn from_u32(value: u32) -> Option<Chipset> {
> +        match value {
> +            0x162 => Some(Chipset::TU102),
> +            0x164 => Some(Chipset::TU104),
> +            0x166 => Some(Chipset::TU106),
> +            0x167 => Some(Chipset::TU117),
> +            0x168 => Some(Chipset::TU116),
> +            0x172 => Some(Chipset::GA102),
> +            0x173 => Some(Chipset::GA103),
> +            0x174 => Some(Chipset::GA104),
> +            0x176 => Some(Chipset::GA106),
> +            0x177 => Some(Chipset::GA107),
> +            0x192 => Some(Chipset::AD102),
> +            0x193 => Some(Chipset::AD103),
> +            0x194 => Some(Chipset::AD104),
> +            0x196 => Some(Chipset::AD106),
> +            0x197 => Some(Chipset::AD107),
> +            _ => None,
> +        }
> +    }
> +}
> +
> +// TODO replace with something like derive(FromPrimitive)
> +impl CardType {
> +    fn from_u32(value: u32) -> Option<CardType> {
> +        match value {
> +            0x160 => Some(CardType::TU100),
> +            0x170 => Some(CardType::GA100),
> +            0x190 => Some(CardType::AD100),

Is this how nouveau does it too? I mean, classifying cards as GA100,
and variants as TU102. It feels wrong to me, because we have for example
GA100 GPUs. I mean, GA100 is the same kind of thing as a GA102: each is
a GPU.

If I were naming card types, I'd calling them by their architecture names:
Turing, Ampere, Ada.

> +            _ => None,
> +        }
> +    }
> +}
> +
> +impl GpuSpec {
> +    fn new(bar: &Devres<Bar0>) -> Result<GpuSpec> {
> +        let bar = bar.try_access().ok_or(ENXIO)?;
> +        let boot0 = u64::from_le(bar.readq(0));
> +        let chip = ((boot0 & 0x1ff00000) >> 20) as u32;
> +
> +        if boot0 & 0x1f000000 == 0 {
> +            return Err(ENODEV);
> +        }
> +
> +        let Some(chipset) = Chipset::from_u32(chip) else {
> +            return Err(ENODEV);
> +        };
> +
> +        let Some(card_type) = CardType::from_u32(chip & 0x1f0) else {
> +            return Err(ENODEV);
> +        };
> +
> +        Ok(Self {
> +            boot0,
> +            card_type,
> +            chipset,
> +            chiprev: (boot0 & 0xff) as u8,
> +        })
> +    }
> +}
> +
> +impl Firmware {
> +    fn new(dev: &device::Device, spec: &GpuSpec, ver: &str) -> Result<Firmware> {
> +        let mut chip_name = CString::try_from_fmt(fmt!("{:?}", spec.chipset))?;
> +        chip_name.make_ascii_lowercase();
> +
> +        let fw_booter_load_path =
> +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/booter_load-{}.bin", &*chip_name, ver))?;
> +        let fw_booter_unload_path =
> +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/booter_unload-{}.bin", &*chip_name, ver))?;
> +        let fw_gsp_path =
> +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/gsp-{}.bin", &*chip_name, ver))?;
> +
> +        let booter_load = firmware::Firmware::request(&fw_booter_load_path, dev)?;
> +        let booter_unload = firmware::Firmware::request(&fw_booter_unload_path, dev)?;
> +        let gsp = firmware::Firmware::request(&fw_gsp_path, dev)?;
> +
> +        Ok(Firmware {
> +            booter_load,
> +            booter_unload,
> +            gsp,
> +        })
> +    }
> +}
> +
> +impl Gpu {
> +    pub(crate) fn new(pdev: &pci::Device, bar: Devres<Bar0>) -> Result<impl PinInit<Self>> {
> +        let spec = GpuSpec::new(&bar)?;
> +        let fw = Firmware::new(pdev.as_ref(), &spec, "535.113.01")?;

lol there it is: our one, "stable" set of GSP firmware. Maybe a one line comment
above might be appropriate, to mention that this is hardcoded, but new firmware
versions will not be. On the other hand, that's obvious. :)

> +
> +        dev_info!(
> +            pdev.as_ref(),
> +            "NVIDIA {:?} ({:#x})",
> +            spec.chipset,
> +            spec.boot0
> +        );
> +
> +        Ok(pin_init!(Self { spec, bar, fw }))
> +    }
> +}
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> new file mode 100644
> index 000000000000..b130d9ca6a0f
> --- /dev/null
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Nova Core GPU Driver
> +
> +mod driver;
> +mod gpu;
> +
> +kernel::module_pci_driver! {
> +    type: driver::NovaCore,
> +    name: "NovaCore",
> +    author: "Danilo Krummrich",
> +    description: "Nova Core GPU driver",
> +    license: "GPL v2",
> +}
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 44c9ef1435a2..5df981920a94 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -39,6 +39,7 @@ source "drivers/gpu/vga/Kconfig"
>   
>   source "drivers/gpu/host1x/Kconfig"
>   source "drivers/gpu/ipu-v3/Kconfig"
> +source "drivers/gpu/nova-core/Kconfig"
>   
>   source "drivers/gpu/drm/Kconfig"
>   
> 
> base-commit: 69b8923f5003664e3ffef102e73333edfa2abdcf

I'm always grateful when anyone uses "git format-patch --base", it makes
life simpler.


thanks,
-- 
John Hubbard

Re: [PATCH 1/2] gpu: nova-core: add initial driver stub

[Karol Herbst]

On Fri, Jan 31, 2025 at 11:04 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> Add the initial nova-core driver stub.
>
> nova-core is intended to serve as a common base for nova-drm (the
> corresponding DRM driver) and the vGPU manager VFIO driver, serving as a
> hard- and firmware abstraction layer for GSP-based NVIDIA GPUs.
>
> The Nova project, including nova-core and nova-drm, in the long term,
> is intended to serve as the successor of Nouveau for all GSP-based GPUs.
>
> The motivation for both, starting a successor project for Nouveau and
> doing so using the Rust programming language, is documented in detail
> through a previous post on the mailing list [1], an LWN article [2] and a
> talk from LPC '24.
>
> In order to avoid the chicken and egg problem to require a user to
> upstream Rust abstractions, but at the same time require the Rust
> abstractions to implement the driver, nova-core kicks off as a driver
> stub and is subsequently developed upstream.
>
> Link: https://lore.kernel.org/dri-devel/Zfsj0_tb-0-tNrJy@cassiopeiae/T/#u [1]
> Link: https://lwn.net/Articles/990736/ [2]
> Link: https://youtu.be/3Igmx28B3BQ?si=sBdSEer4tAPKGpOs [3]
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  MAINTAINERS                        |  10 ++
>  drivers/gpu/Makefile               |   1 +
>  drivers/gpu/nova-core/Kconfig      |  13 +++
>  drivers/gpu/nova-core/Makefile     |   3 +
>  drivers/gpu/nova-core/driver.rs    |  47 ++++++++
>  drivers/gpu/nova-core/gpu.rs       | 171 +++++++++++++++++++++++++++++
>  drivers/gpu/nova-core/nova_core.rs |  14 +++
>  drivers/video/Kconfig              |   1 +
>  8 files changed, 260 insertions(+)
>  create mode 100644 drivers/gpu/nova-core/Kconfig
>  create mode 100644 drivers/gpu/nova-core/Makefile
>  create mode 100644 drivers/gpu/nova-core/driver.rs
>  create mode 100644 drivers/gpu/nova-core/gpu.rs
>  create mode 100644 drivers/gpu/nova-core/nova_core.rs
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d1086e53a317..f7ddca7de0ef 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7446,6 +7446,16 @@ T:       git https://gitlab.freedesktop.org/drm/nouveau.git
>  F:     drivers/gpu/drm/nouveau/
>  F:     include/uapi/drm/nouveau_drm.h
>
> +CORE DRIVER FOR NVIDIA GPUS [RUST]
> +M:     Danilo Krummrich <dakr@kernel.org>
> +L:     nouveau@lists.freedesktop.org
> +S:     Supported
> +Q:     https://patchwork.freedesktop.org/project/nouveau/
> +B:     https://gitlab.freedesktop.org/drm/nova/-/issues
> +C:     irc://irc.oftc.net/nouveau
> +T:     git https://gitlab.freedesktop.org/drm/nova.git nova-next
> +F:     drivers/gpu/nova-core/
> +
>  DRM DRIVER FOR OLIMEX LCD-OLINUXINO PANELS
>  M:     Stefan Mavrodiev <stefan@olimex.com>
>  S:     Maintained
> diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
> index 8997f0096545..36a54d456630 100644
> --- a/drivers/gpu/Makefile
> +++ b/drivers/gpu/Makefile
> @@ -5,3 +5,4 @@
>  obj-y                  += host1x/ drm/ vga/
>  obj-$(CONFIG_IMX_IPUV3_CORE)   += ipu-v3/
>  obj-$(CONFIG_TRACE_GPU_MEM)            += trace/
> +obj-$(CONFIG_NOVA_CORE)                += nova-core/
> diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
> new file mode 100644
> index 000000000000..33ac937b244a
> --- /dev/null
> +++ b/drivers/gpu/nova-core/Kconfig
> @@ -0,0 +1,13 @@
> +config NOVA_CORE
> +       tristate "Nova Core GPU driver"
> +       depends on PCI
> +       depends on RUST
> +       depends on RUST_FW_LOADER_ABSTRACTIONS
> +       default n
> +       help
> +         Choose this if you want to build the Nova Core driver for Nvidia
> +         GSP-based GPUs.
> +
> +         This driver is work in progress and may not be functional.
> +
> +         If M is selected, the module will be called nova-core.
> diff --git a/drivers/gpu/nova-core/Makefile b/drivers/gpu/nova-core/Makefile
> new file mode 100644
> index 000000000000..2d78c50126e1
> --- /dev/null
> +++ b/drivers/gpu/nova-core/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_NOVA_CORE) += nova_core.o
> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> new file mode 100644
> index 000000000000..2a2aa9b0630b
> --- /dev/null
> +++ b/drivers/gpu/nova-core/driver.rs
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use kernel::{bindings, c_str, pci, prelude::*};
> +
> +use crate::gpu::Gpu;
> +
> +#[pin_data]
> +pub(crate) struct NovaCore {
> +    #[pin]
> +    pub(crate) gpu: Gpu,
> +}
> +
> +const BAR0_SIZE: usize = 8;
> +pub(crate) type Bar0 = pci::Bar<BAR0_SIZE>;
> +
> +kernel::pci_device_table!(
> +    PCI_TABLE,
> +    MODULE_PCI_TABLE,
> +    <NovaCore as pci::Driver>::IdInfo,
> +    [(
> +        pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_NVIDIA, bindings::PCI_ANY_ID as _),
> +        ()
> +    )]
> +);
> +
> +impl pci::Driver for NovaCore {
> +    type IdInfo = ();
> +    const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
> +
> +    fn probe(pdev: &mut pci::Device, _info: &Self::IdInfo) -> Result<Pin<KBox<Self>>> {
> +        dev_dbg!(pdev.as_ref(), "Probe Nova Core GPU driver.\n");
> +
> +        pdev.enable_device_mem()?;
> +        pdev.set_master();
> +
> +        let bar = pdev.iomap_region_sized::<BAR0_SIZE>(0, c_str!("nova-core"))?;

I'm curious about the c_str! macro here. Since rust 1.78 one can do
c"nova-core" to get a &CStr, is this not available in the r4l project
yet or other reasons why this can't be used? Might make sense to clean
it up kernel wide (outside this patch set) if it's guaranteed to be
available.

> +
> +        let this = KBox::pin_init(
> +            try_pin_init!(Self {
> +                gpu <- Gpu::new(pdev, bar)?,
> +            }),
> +            GFP_KERNEL,
> +        )?;
> +
> +        Ok(this)
> +    }
> +}
> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> new file mode 100644
> index 000000000000..cf62390e72eb
> --- /dev/null
> +++ b/drivers/gpu/nova-core/gpu.rs
> @@ -0,0 +1,171 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use kernel::{
> +    device, devres::Devres, error::code::*, firmware, fmt, pci, prelude::*, str::CString,
> +};
> +
> +use crate::driver::Bar0;
> +use core::fmt::Debug;
> +
> +/// Enum representation of the GPU chipset.
> +#[derive(Debug)]
> +pub(crate) enum Chipset {
> +    TU102 = 0x162,
> +    TU104 = 0x164,
> +    TU106 = 0x166,
> +    TU117 = 0x167,
> +    TU116 = 0x168,
> +    GA102 = 0x172,
> +    GA103 = 0x173,
> +    GA104 = 0x174,
> +    GA106 = 0x176,
> +    GA107 = 0x177,
> +    AD102 = 0x192,
> +    AD103 = 0x193,
> +    AD104 = 0x194,
> +    AD106 = 0x196,
> +    AD107 = 0x197,
> +}
> +
> +/// Enum representation of the GPU generation.
> +#[derive(Debug)]
> +pub(crate) enum CardType {
> +    /// Turing
> +    TU100 = 0x160,
> +    /// Ampere
> +    GA100 = 0x170,
> +    /// Ada Lovelace
> +    AD100 = 0x190,
> +}
> +
> +/// Structure holding the metadata of the GPU.
> +#[allow(dead_code)]
> +pub(crate) struct GpuSpec {
> +    /// Contents of the boot0 register.
> +    boot0: u64,
> +    card_type: CardType,
> +    chipset: Chipset,
> +    /// The revision of the chipset.
> +    chiprev: u8,
> +}
> +
> +/// Structure encapsulating the firmware blobs required for the GPU to operate.
> +#[allow(dead_code)]
> +pub(crate) struct Firmware {
> +    booter_load: firmware::Firmware,
> +    booter_unload: firmware::Firmware,
> +    gsp: firmware::Firmware,
> +}
> +
> +/// Structure holding the resources required to operate the GPU.
> +#[allow(dead_code)]
> +#[pin_data]
> +pub(crate) struct Gpu {
> +    spec: GpuSpec,
> +    /// MMIO mapping of PCI BAR 0
> +    bar: Devres<Bar0>,
> +    fw: Firmware,
> +}
> +
> +// TODO replace with something like derive(FromPrimitive)
> +impl Chipset {
> +    fn from_u32(value: u32) -> Option<Chipset> {
> +        match value {
> +            0x162 => Some(Chipset::TU102),
> +            0x164 => Some(Chipset::TU104),
> +            0x166 => Some(Chipset::TU106),
> +            0x167 => Some(Chipset::TU117),
> +            0x168 => Some(Chipset::TU116),
> +            0x172 => Some(Chipset::GA102),
> +            0x173 => Some(Chipset::GA103),
> +            0x174 => Some(Chipset::GA104),
> +            0x176 => Some(Chipset::GA106),
> +            0x177 => Some(Chipset::GA107),
> +            0x192 => Some(Chipset::AD102),
> +            0x193 => Some(Chipset::AD103),
> +            0x194 => Some(Chipset::AD104),
> +            0x196 => Some(Chipset::AD106),
> +            0x197 => Some(Chipset::AD107),
> +            _ => None,
> +        }
> +    }
> +}
> +
> +// TODO replace with something like derive(FromPrimitive)
> +impl CardType {
> +    fn from_u32(value: u32) -> Option<CardType> {
> +        match value {
> +            0x160 => Some(CardType::TU100),
> +            0x170 => Some(CardType::GA100),
> +            0x190 => Some(CardType::AD100),
> +            _ => None,
> +        }
> +    }
> +}
> +
> +impl GpuSpec {
> +    fn new(bar: &Devres<Bar0>) -> Result<GpuSpec> {
> +        let bar = bar.try_access().ok_or(ENXIO)?;
> +        let boot0 = u64::from_le(bar.readq(0));
> +        let chip = ((boot0 & 0x1ff00000) >> 20) as u32;
> +
> +        if boot0 & 0x1f000000 == 0 {
> +            return Err(ENODEV);
> +        }
> +
> +        let Some(chipset) = Chipset::from_u32(chip) else {
> +            return Err(ENODEV);
> +        };
> +
> +        let Some(card_type) = CardType::from_u32(chip & 0x1f0) else {
> +            return Err(ENODEV);
> +        };
> +
> +        Ok(Self {
> +            boot0,
> +            card_type,
> +            chipset,
> +            chiprev: (boot0 & 0xff) as u8,
> +        })
> +    }
> +}
> +
> +impl Firmware {
> +    fn new(dev: &device::Device, spec: &GpuSpec, ver: &str) -> Result<Firmware> {
> +        let mut chip_name = CString::try_from_fmt(fmt!("{:?}", spec.chipset))?;
> +        chip_name.make_ascii_lowercase();
> +
> +        let fw_booter_load_path =
> +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/booter_load-{}.bin", &*chip_name, ver))?;
> +        let fw_booter_unload_path =
> +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/booter_unload-{}.bin", &*chip_name, ver))?;
> +        let fw_gsp_path =
> +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/gsp-{}.bin", &*chip_name, ver))?;
> +
> +        let booter_load = firmware::Firmware::request(&fw_booter_load_path, dev)?;
> +        let booter_unload = firmware::Firmware::request(&fw_booter_unload_path, dev)?;
> +        let gsp = firmware::Firmware::request(&fw_gsp_path, dev)?;
> +
> +        Ok(Firmware {
> +            booter_load,
> +            booter_unload,
> +            gsp,
> +        })
> +    }
> +}
> +
> +impl Gpu {
> +    pub(crate) fn new(pdev: &pci::Device, bar: Devres<Bar0>) -> Result<impl PinInit<Self>> {
> +        let spec = GpuSpec::new(&bar)?;
> +        let fw = Firmware::new(pdev.as_ref(), &spec, "535.113.01")?;
> +
> +        dev_info!(
> +            pdev.as_ref(),
> +            "NVIDIA {:?} ({:#x})",
> +            spec.chipset,
> +            spec.boot0
> +        );
> +
> +        Ok(pin_init!(Self { spec, bar, fw }))
> +    }
> +}
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> new file mode 100644
> index 000000000000..b130d9ca6a0f
> --- /dev/null
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Nova Core GPU Driver
> +
> +mod driver;
> +mod gpu;
> +
> +kernel::module_pci_driver! {
> +    type: driver::NovaCore,
> +    name: "NovaCore",
> +    author: "Danilo Krummrich",
> +    description: "Nova Core GPU driver",
> +    license: "GPL v2",
> +}
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 44c9ef1435a2..5df981920a94 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -39,6 +39,7 @@ source "drivers/gpu/vga/Kconfig"
>
>  source "drivers/gpu/host1x/Kconfig"
>  source "drivers/gpu/ipu-v3/Kconfig"
> +source "drivers/gpu/nova-core/Kconfig"
>
>  source "drivers/gpu/drm/Kconfig"
>
>
> base-commit: 69b8923f5003664e3ffef102e73333edfa2abdcf
> --
> 2.48.1
>

Re: [PATCH 1/2] gpu: nova-core: add initial driver stub

[Karol Herbst]

On Sat, Feb 1, 2025 at 9:14 AM Karol Herbst <kherbst@redhat.com> wrote:
>
> On Fri, Jan 31, 2025 at 11:04 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >
> > Add the initial nova-core driver stub.
> >
> > nova-core is intended to serve as a common base for nova-drm (the
> > corresponding DRM driver) and the vGPU manager VFIO driver, serving as a
> > hard- and firmware abstraction layer for GSP-based NVIDIA GPUs.
> >
> > The Nova project, including nova-core and nova-drm, in the long term,
> > is intended to serve as the successor of Nouveau for all GSP-based GPUs.
> >
> > The motivation for both, starting a successor project for Nouveau and
> > doing so using the Rust programming language, is documented in detail
> > through a previous post on the mailing list [1], an LWN article [2] and a
> > talk from LPC '24.
> >
> > In order to avoid the chicken and egg problem to require a user to
> > upstream Rust abstractions, but at the same time require the Rust
> > abstractions to implement the driver, nova-core kicks off as a driver
> > stub and is subsequently developed upstream.
> >
> > Link: https://lore.kernel.org/dri-devel/Zfsj0_tb-0-tNrJy@cassiopeiae/T/#u [1]
> > Link: https://lwn.net/Articles/990736/ [2]
> > Link: https://youtu.be/3Igmx28B3BQ?si=sBdSEer4tAPKGpOs [3]
> > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> > ---
> >  MAINTAINERS                        |  10 ++
> >  drivers/gpu/Makefile               |   1 +
> >  drivers/gpu/nova-core/Kconfig      |  13 +++
> >  drivers/gpu/nova-core/Makefile     |   3 +
> >  drivers/gpu/nova-core/driver.rs    |  47 ++++++++
> >  drivers/gpu/nova-core/gpu.rs       | 171 +++++++++++++++++++++++++++++
> >  drivers/gpu/nova-core/nova_core.rs |  14 +++
> >  drivers/video/Kconfig              |   1 +
> >  8 files changed, 260 insertions(+)
> >  create mode 100644 drivers/gpu/nova-core/Kconfig
> >  create mode 100644 drivers/gpu/nova-core/Makefile
> >  create mode 100644 drivers/gpu/nova-core/driver.rs
> >  create mode 100644 drivers/gpu/nova-core/gpu.rs
> >  create mode 100644 drivers/gpu/nova-core/nova_core.rs
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index d1086e53a317..f7ddca7de0ef 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -7446,6 +7446,16 @@ T:       git https://gitlab.freedesktop.org/drm/nouveau.git
> >  F:     drivers/gpu/drm/nouveau/
> >  F:     include/uapi/drm/nouveau_drm.h
> >
> > +CORE DRIVER FOR NVIDIA GPUS [RUST]
> > +M:     Danilo Krummrich <dakr@kernel.org>
> > +L:     nouveau@lists.freedesktop.org
> > +S:     Supported
> > +Q:     https://patchwork.freedesktop.org/project/nouveau/
> > +B:     https://gitlab.freedesktop.org/drm/nova/-/issues
> > +C:     irc://irc.oftc.net/nouveau
> > +T:     git https://gitlab.freedesktop.org/drm/nova.git nova-next
> > +F:     drivers/gpu/nova-core/
> > +
> >  DRM DRIVER FOR OLIMEX LCD-OLINUXINO PANELS
> >  M:     Stefan Mavrodiev <stefan@olimex.com>
> >  S:     Maintained
> > diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
> > index 8997f0096545..36a54d456630 100644
> > --- a/drivers/gpu/Makefile
> > +++ b/drivers/gpu/Makefile
> > @@ -5,3 +5,4 @@
> >  obj-y                  += host1x/ drm/ vga/
> >  obj-$(CONFIG_IMX_IPUV3_CORE)   += ipu-v3/
> >  obj-$(CONFIG_TRACE_GPU_MEM)            += trace/
> > +obj-$(CONFIG_NOVA_CORE)                += nova-core/
> > diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
> > new file mode 100644
> > index 000000000000..33ac937b244a
> > --- /dev/null
> > +++ b/drivers/gpu/nova-core/Kconfig
> > @@ -0,0 +1,13 @@
> > +config NOVA_CORE
> > +       tristate "Nova Core GPU driver"
> > +       depends on PCI
> > +       depends on RUST
> > +       depends on RUST_FW_LOADER_ABSTRACTIONS
> > +       default n
> > +       help
> > +         Choose this if you want to build the Nova Core driver for Nvidia
> > +         GSP-based GPUs.
> > +
> > +         This driver is work in progress and may not be functional.
> > +
> > +         If M is selected, the module will be called nova-core.
> > diff --git a/drivers/gpu/nova-core/Makefile b/drivers/gpu/nova-core/Makefile
> > new file mode 100644
> > index 000000000000..2d78c50126e1
> > --- /dev/null
> > +++ b/drivers/gpu/nova-core/Makefile
> > @@ -0,0 +1,3 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +obj-$(CONFIG_NOVA_CORE) += nova_core.o
> > diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> > new file mode 100644
> > index 000000000000..2a2aa9b0630b
> > --- /dev/null
> > +++ b/drivers/gpu/nova-core/driver.rs
> > @@ -0,0 +1,47 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +use kernel::{bindings, c_str, pci, prelude::*};
> > +
> > +use crate::gpu::Gpu;
> > +
> > +#[pin_data]
> > +pub(crate) struct NovaCore {
> > +    #[pin]
> > +    pub(crate) gpu: Gpu,
> > +}
> > +
> > +const BAR0_SIZE: usize = 8;
> > +pub(crate) type Bar0 = pci::Bar<BAR0_SIZE>;
> > +
> > +kernel::pci_device_table!(
> > +    PCI_TABLE,
> > +    MODULE_PCI_TABLE,
> > +    <NovaCore as pci::Driver>::IdInfo,
> > +    [(
> > +        pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_NVIDIA, bindings::PCI_ANY_ID as _),
> > +        ()
> > +    )]
> > +);
> > +
> > +impl pci::Driver for NovaCore {
> > +    type IdInfo = ();
> > +    const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
> > +
> > +    fn probe(pdev: &mut pci::Device, _info: &Self::IdInfo) -> Result<Pin<KBox<Self>>> {
> > +        dev_dbg!(pdev.as_ref(), "Probe Nova Core GPU driver.\n");
> > +
> > +        pdev.enable_device_mem()?;
> > +        pdev.set_master();
> > +
> > +        let bar = pdev.iomap_region_sized::<BAR0_SIZE>(0, c_str!("nova-core"))?;
>
> I'm curious about the c_str! macro here. Since rust 1.78 one can do
> c"nova-core" to get a &CStr, is this not available in the r4l project
> yet or other reasons why this can't be used? Might make sense to clean
> it up kernel wide (outside this patch set) if it's guaranteed to be
> available.
>

1.77 actually, but in mesa we moved to 1.78, so I got that confused.

> > +
> > +        let this = KBox::pin_init(
> > +            try_pin_init!(Self {
> > +                gpu <- Gpu::new(pdev, bar)?,
> > +            }),
> > +            GFP_KERNEL,
> > +        )?;
> > +
> > +        Ok(this)
> > +    }
> > +}
> > diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> > new file mode 100644
> > index 000000000000..cf62390e72eb
> > --- /dev/null
> > +++ b/drivers/gpu/nova-core/gpu.rs
> > @@ -0,0 +1,171 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +use kernel::{
> > +    device, devres::Devres, error::code::*, firmware, fmt, pci, prelude::*, str::CString,
> > +};
> > +
> > +use crate::driver::Bar0;
> > +use core::fmt::Debug;
> > +
> > +/// Enum representation of the GPU chipset.
> > +#[derive(Debug)]
> > +pub(crate) enum Chipset {
> > +    TU102 = 0x162,
> > +    TU104 = 0x164,
> > +    TU106 = 0x166,
> > +    TU117 = 0x167,
> > +    TU116 = 0x168,
> > +    GA102 = 0x172,
> > +    GA103 = 0x173,
> > +    GA104 = 0x174,
> > +    GA106 = 0x176,
> > +    GA107 = 0x177,
> > +    AD102 = 0x192,
> > +    AD103 = 0x193,
> > +    AD104 = 0x194,
> > +    AD106 = 0x196,
> > +    AD107 = 0x197,
> > +}
> > +
> > +/// Enum representation of the GPU generation.
> > +#[derive(Debug)]
> > +pub(crate) enum CardType {
> > +    /// Turing
> > +    TU100 = 0x160,
> > +    /// Ampere
> > +    GA100 = 0x170,
> > +    /// Ada Lovelace
> > +    AD100 = 0x190,
> > +}
> > +
> > +/// Structure holding the metadata of the GPU.
> > +#[allow(dead_code)]
> > +pub(crate) struct GpuSpec {
> > +    /// Contents of the boot0 register.
> > +    boot0: u64,
> > +    card_type: CardType,
> > +    chipset: Chipset,
> > +    /// The revision of the chipset.
> > +    chiprev: u8,
> > +}
> > +
> > +/// Structure encapsulating the firmware blobs required for the GPU to operate.
> > +#[allow(dead_code)]
> > +pub(crate) struct Firmware {
> > +    booter_load: firmware::Firmware,
> > +    booter_unload: firmware::Firmware,
> > +    gsp: firmware::Firmware,
> > +}
> > +
> > +/// Structure holding the resources required to operate the GPU.
> > +#[allow(dead_code)]
> > +#[pin_data]
> > +pub(crate) struct Gpu {
> > +    spec: GpuSpec,
> > +    /// MMIO mapping of PCI BAR 0
> > +    bar: Devres<Bar0>,
> > +    fw: Firmware,
> > +}
> > +
> > +// TODO replace with something like derive(FromPrimitive)
> > +impl Chipset {
> > +    fn from_u32(value: u32) -> Option<Chipset> {
> > +        match value {
> > +            0x162 => Some(Chipset::TU102),
> > +            0x164 => Some(Chipset::TU104),
> > +            0x166 => Some(Chipset::TU106),
> > +            0x167 => Some(Chipset::TU117),
> > +            0x168 => Some(Chipset::TU116),
> > +            0x172 => Some(Chipset::GA102),
> > +            0x173 => Some(Chipset::GA103),
> > +            0x174 => Some(Chipset::GA104),
> > +            0x176 => Some(Chipset::GA106),
> > +            0x177 => Some(Chipset::GA107),
> > +            0x192 => Some(Chipset::AD102),
> > +            0x193 => Some(Chipset::AD103),
> > +            0x194 => Some(Chipset::AD104),
> > +            0x196 => Some(Chipset::AD106),
> > +            0x197 => Some(Chipset::AD107),
> > +            _ => None,
> > +        }
> > +    }
> > +}
> > +
> > +// TODO replace with something like derive(FromPrimitive)
> > +impl CardType {
> > +    fn from_u32(value: u32) -> Option<CardType> {
> > +        match value {
> > +            0x160 => Some(CardType::TU100),
> > +            0x170 => Some(CardType::GA100),
> > +            0x190 => Some(CardType::AD100),
> > +            _ => None,
> > +        }
> > +    }
> > +}
> > +
> > +impl GpuSpec {
> > +    fn new(bar: &Devres<Bar0>) -> Result<GpuSpec> {
> > +        let bar = bar.try_access().ok_or(ENXIO)?;
> > +        let boot0 = u64::from_le(bar.readq(0));
> > +        let chip = ((boot0 & 0x1ff00000) >> 20) as u32;
> > +
> > +        if boot0 & 0x1f000000 == 0 {
> > +            return Err(ENODEV);
> > +        }
> > +
> > +        let Some(chipset) = Chipset::from_u32(chip) else {
> > +            return Err(ENODEV);
> > +        };
> > +
> > +        let Some(card_type) = CardType::from_u32(chip & 0x1f0) else {
> > +            return Err(ENODEV);
> > +        };
> > +
> > +        Ok(Self {
> > +            boot0,
> > +            card_type,
> > +            chipset,
> > +            chiprev: (boot0 & 0xff) as u8,
> > +        })
> > +    }
> > +}
> > +
> > +impl Firmware {
> > +    fn new(dev: &device::Device, spec: &GpuSpec, ver: &str) -> Result<Firmware> {
> > +        let mut chip_name = CString::try_from_fmt(fmt!("{:?}", spec.chipset))?;
> > +        chip_name.make_ascii_lowercase();
> > +
> > +        let fw_booter_load_path =
> > +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/booter_load-{}.bin", &*chip_name, ver))?;
> > +        let fw_booter_unload_path =
> > +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/booter_unload-{}.bin", &*chip_name, ver))?;
> > +        let fw_gsp_path =
> > +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/gsp-{}.bin", &*chip_name, ver))?;
> > +
> > +        let booter_load = firmware::Firmware::request(&fw_booter_load_path, dev)?;
> > +        let booter_unload = firmware::Firmware::request(&fw_booter_unload_path, dev)?;
> > +        let gsp = firmware::Firmware::request(&fw_gsp_path, dev)?;
> > +
> > +        Ok(Firmware {
> > +            booter_load,
> > +            booter_unload,
> > +            gsp,
> > +        })
> > +    }
> > +}
> > +
> > +impl Gpu {
> > +    pub(crate) fn new(pdev: &pci::Device, bar: Devres<Bar0>) -> Result<impl PinInit<Self>> {
> > +        let spec = GpuSpec::new(&bar)?;
> > +        let fw = Firmware::new(pdev.as_ref(), &spec, "535.113.01")?;
> > +
> > +        dev_info!(
> > +            pdev.as_ref(),
> > +            "NVIDIA {:?} ({:#x})",
> > +            spec.chipset,
> > +            spec.boot0
> > +        );
> > +
> > +        Ok(pin_init!(Self { spec, bar, fw }))
> > +    }
> > +}
> > diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> > new file mode 100644
> > index 000000000000..b130d9ca6a0f
> > --- /dev/null
> > +++ b/drivers/gpu/nova-core/nova_core.rs
> > @@ -0,0 +1,14 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Nova Core GPU Driver
> > +
> > +mod driver;
> > +mod gpu;
> > +
> > +kernel::module_pci_driver! {
> > +    type: driver::NovaCore,
> > +    name: "NovaCore",
> > +    author: "Danilo Krummrich",
> > +    description: "Nova Core GPU driver",
> > +    license: "GPL v2",
> > +}
> > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > index 44c9ef1435a2..5df981920a94 100644
> > --- a/drivers/video/Kconfig
> > +++ b/drivers/video/Kconfig
> > @@ -39,6 +39,7 @@ source "drivers/gpu/vga/Kconfig"
> >
> >  source "drivers/gpu/host1x/Kconfig"
> >  source "drivers/gpu/ipu-v3/Kconfig"
> > +source "drivers/gpu/nova-core/Kconfig"
> >
> >  source "drivers/gpu/drm/Kconfig"
> >
> >
> > base-commit: 69b8923f5003664e3ffef102e73333edfa2abdcf
> > --
> > 2.48.1
> >

Re: [PATCH 1/2] gpu: nova-core: add initial driver stub

[Greg KH]

On Fri, Jan 31, 2025 at 11:04:24PM +0100, Danilo Krummrich wrote:
> +impl Gpu {
> +    pub(crate) fn new(pdev: &pci::Device, bar: Devres<Bar0>) -> Result<impl PinInit<Self>> {
> +        let spec = GpuSpec::new(&bar)?;
> +        let fw = Firmware::new(pdev.as_ref(), &spec, "535.113.01")?;
> +
> +        dev_info!(
> +            pdev.as_ref(),
> +            "NVIDIA {:?} ({:#x})",
> +            spec.chipset,
> +            spec.boot0
> +        );

When drivers work properly, they should be quiet, so can you move this
to dev_dbg()?

thanks,

greg k-h

Re: [PATCH 1/2] gpu: nova-core: add initial driver stub

[Danilo Krummrich]

On Sat, Feb 01, 2025 at 09:14:48AM +0100, Karol Herbst wrote:
> On Fri, Jan 31, 2025 at 11:04 PM Danilo Krummrich <dakr@kernel.org> wrote:
> > +impl pci::Driver for NovaCore {
> > +    type IdInfo = ();
> > +    const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
> > +
> > +    fn probe(pdev: &mut pci::Device, _info: &Self::IdInfo) -> Result<Pin<KBox<Self>>> {
> > +        dev_dbg!(pdev.as_ref(), "Probe Nova Core GPU driver.\n");
> > +
> > +        pdev.enable_device_mem()?;
> > +        pdev.set_master();
> > +
> > +        let bar = pdev.iomap_region_sized::<BAR0_SIZE>(0, c_str!("nova-core"))?;
> 
> I'm curious about the c_str! macro here. Since rust 1.78 one can do
> c"nova-core" to get a &CStr, is this not available in the r4l project
> yet or other reasons why this can't be used?

The kernel is still using kernel::str::CStr instead of core::ffi::CStr.

> Might make sense to clean
> it up kernel wide (outside this patch set) if it's guaranteed to be
> available.

Indeed, there's already an entry in the R4L issue tracker about this [1].
There's also a patch series [2] addressing it, but it seems that the series
didn't get an update for quite a while.

[1] https://github.com/Rust-for-Linux/linux/issues/1075
[2] https://lore.kernel.org/rust-for-linux/20240819153656.28807-2-vadorovsky@protonmail.com/

Re: [PATCH 1/2] gpu: nova-core: add initial driver stub

[Danilo Krummrich]

On Sat, Feb 01, 2025 at 09:33:28AM +0100, Greg KH wrote:
> On Fri, Jan 31, 2025 at 11:04:24PM +0100, Danilo Krummrich wrote:
> > +impl Gpu {
> > +    pub(crate) fn new(pdev: &pci::Device, bar: Devres<Bar0>) -> Result<impl PinInit<Self>> {
> > +        let spec = GpuSpec::new(&bar)?;
> > +        let fw = Firmware::new(pdev.as_ref(), &spec, "535.113.01")?;
> > +
> > +        dev_info!(
> > +            pdev.as_ref(),
> > +            "NVIDIA {:?} ({:#x})",
> > +            spec.chipset,
> > +            spec.boot0
> > +        );
> 
> When drivers work properly, they should be quiet, so can you move this
> to dev_dbg()?

Sure, the only reason I made this dev_info!() is because, as an initial
skeleton, the driver isn't doing anything else for now. So, I thought it would
be nice to have some sign of life.

Of course, the intention was to remove this, once there's any other sign of
life.

Re: [PATCH 1/2] gpu: nova-core: add initial driver stub

[Danilo Krummrich]

On Fri, Jan 31, 2025 at 08:01:00PM -0800, John Hubbard wrote:
> On 1/31/25 2:04 PM, Danilo Krummrich wrote:
> > Add the initial nova-core driver stub.
> > 
> > nova-core is intended to serve as a common base for nova-drm (the
> > corresponding DRM driver) and the vGPU manager VFIO driver, serving as a
> > hard- and firmware abstraction layer for GSP-based NVIDIA GPUs.
> > 
> > The Nova project, including nova-core and nova-drm, in the long term,
> > is intended to serve as the successor of Nouveau for all GSP-based GPUs.
> > 
> > The motivation for both, starting a successor project for Nouveau and
> > doing so using the Rust programming language, is documented in detail
> > through a previous post on the mailing list [1], an LWN article [2] and a
> > talk from LPC '24.
> > 
> > In order to avoid the chicken and egg problem to require a user to
> > upstream Rust abstractions, but at the same time require the Rust
> > abstractions to implement the driver, nova-core kicks off as a driver
> > stub and is subsequently developed upstream.
> > 
> > Link: https://lore.kernel.org/dri-devel/Zfsj0_tb-0-tNrJy@cassiopeiae/T/#u [1]
> > Link: https://lwn.net/Articles/990736/ [2]
> > Link: https://youtu.be/3Igmx28B3BQ?si=sBdSEer4tAPKGpOs [3]
> > Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> > ---
> >   MAINTAINERS                        |  10 ++
> >   drivers/gpu/Makefile               |   1 +
> >   drivers/gpu/nova-core/Kconfig      |  13 +++
> >   drivers/gpu/nova-core/Makefile     |   3 +
> >   drivers/gpu/nova-core/driver.rs    |  47 ++++++++
> >   drivers/gpu/nova-core/gpu.rs       | 171 +++++++++++++++++++++++++++++
> >   drivers/gpu/nova-core/nova_core.rs |  14 +++
> >   drivers/video/Kconfig              |   1 +
> >   8 files changed, 260 insertions(+)
> >   create mode 100644 drivers/gpu/nova-core/Kconfig
> >   create mode 100644 drivers/gpu/nova-core/Makefile
> >   create mode 100644 drivers/gpu/nova-core/driver.rs
> >   create mode 100644 drivers/gpu/nova-core/gpu.rs
> >   create mode 100644 drivers/gpu/nova-core/nova_core.rs
> 
> Hi Danilo,
> 
> This is pleasantly clean, and even elegant. I was pleasantly surprised at
> the level of firmware loading support in Rust, and how this approach takes
> advantage of our r535 firmware snapshot that is in Turing, Ampere, and Ada.
> 
> It loads up on my GA104 system and I've been poking around at it.
> 
> Some minor comments below, but this looks like a very good starting "stub"
> to get merged.
> 
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index d1086e53a317..f7ddca7de0ef 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -7446,6 +7446,16 @@ T:	git https://gitlab.freedesktop.org/drm/nouveau.git
> >   F:	drivers/gpu/drm/nouveau/
> >   F:	include/uapi/drm/nouveau_drm.h
> > +CORE DRIVER FOR NVIDIA GPUS [RUST]
> > +M:	Danilo Krummrich <dakr@kernel.org>
> > +L:	nouveau@lists.freedesktop.org
> > +S:	Supported
> > +Q:	https://patchwork.freedesktop.org/project/nouveau/
> 
> Are you sure? I'm not sure how patchwork things work, but it seems
> unfortunate to confuse Nova and nouveau here.

It's the nouveau patchwork, because we're also using the nouveau mailing list
for now.

Using the nouveau mailing makes it easier to reach people interested in the
project in the beginning.

In the medium-term I think it would make sense to introduce some separate
infrastructure.

> 
> > +B:	https://gitlab.freedesktop.org/drm/nova/-/issues
> > +C:	irc://irc.oftc.net/nouveau
> > +T:	git https://gitlab.freedesktop.org/drm/nova.git nova-next
> > +F:	drivers/gpu/nova-core/
> > +
> >   DRM DRIVER FOR OLIMEX LCD-OLINUXINO PANELS
> >   M:	Stefan Mavrodiev <stefan@olimex.com>
> >   S:	Maintained
> > diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
> > index 8997f0096545..36a54d456630 100644
> > --- a/drivers/gpu/Makefile
> > +++ b/drivers/gpu/Makefile
> > @@ -5,3 +5,4 @@
> >   obj-y			+= host1x/ drm/ vga/
> >   obj-$(CONFIG_IMX_IPUV3_CORE)	+= ipu-v3/
> >   obj-$(CONFIG_TRACE_GPU_MEM)		+= trace/
> > +obj-$(CONFIG_NOVA_CORE)		+= nova-core/
> > diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
> > new file mode 100644
> > index 000000000000..33ac937b244a
> > --- /dev/null
> > +++ b/drivers/gpu/nova-core/Kconfig
> > @@ -0,0 +1,13 @@
> > +config NOVA_CORE
> > +	tristate "Nova Core GPU driver"
> > +	depends on PCI
> > +	depends on RUST
> > +	depends on RUST_FW_LOADER_ABSTRACTIONS
> > +	default n
> > +	help
> > +	  Choose this if you want to build the Nova Core driver for Nvidia
> > +	  GSP-based GPUs.
> 
> Maybe a little note about what "GSP" is and how you know if you have it,
> would help. Turing and later architectures have GSP firmware. All the
> user might know is the GPU architecture, I wouldn't expect the user
> to know if it has a "GSP".

That's a good idea, gonna add some notes.

> 
> > +
> > +	  This driver is work in progress and may not be functional.
> > +
> > +	  If M is selected, the module will be called nova-core.
> 
> Or nova_core? I realize the driver core translates between "-" and "_",
> just trying to be consistent.
> 
> It does show up as /sys/module/nova_core .

Yes, this should be 'nova_core'. I already noticed this typo myself and forgot
to fix it - dang!

> 
> 
> > diff --git a/drivers/gpu/nova-core/Makefile b/drivers/gpu/nova-core/Makefile
> > new file mode 100644
> > index 000000000000..2d78c50126e1
> > --- /dev/null
> > +++ b/drivers/gpu/nova-core/Makefile
> > @@ -0,0 +1,3 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +obj-$(CONFIG_NOVA_CORE) += nova_core.o
> > diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> > new file mode 100644
> > index 000000000000..2a2aa9b0630b
> > --- /dev/null
> > +++ b/drivers/gpu/nova-core/driver.rs
> > @@ -0,0 +1,47 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +use kernel::{bindings, c_str, pci, prelude::*};
> > +
> > +use crate::gpu::Gpu;
> > +
> > +#[pin_data]
> > +pub(crate) struct NovaCore {
> > +    #[pin]
> > +    pub(crate) gpu: Gpu,
> > +}
> > +
> > +const BAR0_SIZE: usize = 8;
> > +pub(crate) type Bar0 = pci::Bar<BAR0_SIZE>;
> > +
> > +kernel::pci_device_table!(
> > +    PCI_TABLE,
> > +    MODULE_PCI_TABLE,
> > +    <NovaCore as pci::Driver>::IdInfo,
> > +    [(
> > +        pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_NVIDIA, bindings::PCI_ANY_ID as _),
> > +        ()
> > +    )]
> > +);
> > +
> > +impl pci::Driver for NovaCore {
> > +    type IdInfo = ();
> > +    const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
> > +
> > +    fn probe(pdev: &mut pci::Device, _info: &Self::IdInfo) -> Result<Pin<KBox<Self>>> {
> > +        dev_dbg!(pdev.as_ref(), "Probe Nova Core GPU driver.\n");
> > +
> > +        pdev.enable_device_mem()?;
> > +        pdev.set_master();
> > +
> > +        let bar = pdev.iomap_region_sized::<BAR0_SIZE>(0, c_str!("nova-core"))?;
> 
> Another question about whether it should be nova-core or nova_core.

This string is not related to the module name (which btw. is only 'nova_core'
because the build system doesn't like 'nova-core.rs'), hence I used the correct
spelling of the driver name.

This string ends up as the name for the underlying struct resource [1]. So, if
we'd want to be super correct, it should probably be something like
"nova-core BAR0".

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/ioport.h#n24

> 
> > +
> > +        let this = KBox::pin_init(
> > +            try_pin_init!(Self {
> > +                gpu <- Gpu::new(pdev, bar)?,
> > +            }),
> > +            GFP_KERNEL,
> > +        )?;
> > +
> > +        Ok(this)
> > +    }
> > +}
> > diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> > new file mode 100644
> > index 000000000000..cf62390e72eb
> > --- /dev/null
> > +++ b/drivers/gpu/nova-core/gpu.rs
> > @@ -0,0 +1,171 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +use kernel::{
> > +    device, devres::Devres, error::code::*, firmware, fmt, pci, prelude::*, str::CString,
> > +};
> > +
> > +use crate::driver::Bar0;
> > +use core::fmt::Debug;
> > +
> > +/// Enum representation of the GPU chipset.
> > +#[derive(Debug)]
> > +pub(crate) enum Chipset {
> > +    TU102 = 0x162,
> > +    TU104 = 0x164,
> > +    TU106 = 0x166,
> > +    TU117 = 0x167,
> > +    TU116 = 0x168,
> > +    GA102 = 0x172,
> > +    GA103 = 0x173,
> > +    GA104 = 0x174,
> > +    GA106 = 0x176,
> > +    GA107 = 0x177,
> > +    AD102 = 0x192,
> > +    AD103 = 0x193,
> > +    AD104 = 0x194,
> > +    AD106 = 0x196,
> > +    AD107 = 0x197,
> > +}
> > +
> > +/// Enum representation of the GPU generation.
> > +#[derive(Debug)]
> > +pub(crate) enum CardType {
> > +    /// Turing
> > +    TU100 = 0x160,
> > +    /// Ampere
> > +    GA100 = 0x170,
> > +    /// Ada Lovelace
> > +    AD100 = 0x190,
> > +}
> > +
> > +/// Structure holding the metadata of the GPU.
> > +#[allow(dead_code)]
> > +pub(crate) struct GpuSpec {
> > +    /// Contents of the boot0 register.
> > +    boot0: u64,
> 
> It is redundant to store boot0, when all of the following fields
> are deduced from boot0.

Yes, I think we can probably remove it, I only use it to print it in Gpu::new()
as a sign of life and because I don't know if boot0 contains any other useful
information than chipset and chiprev.

But maybe you can help me out here? :) That is, share the register layout and
field names. This way I could also get rid of those magic numbers, and put in
proper naming for fields, masks and shifts.

> 
> > +    card_type: CardType,
> > +    chipset: Chipset,
> > +    /// The revision of the chipset.
> > +    chiprev: u8,
> > +}
> > +
> > +/// Structure encapsulating the firmware blobs required for the GPU to operate.
> > +#[allow(dead_code)]
> > +pub(crate) struct Firmware {
> > +    booter_load: firmware::Firmware,
> > +    booter_unload: firmware::Firmware,
> > +    gsp: firmware::Firmware,
> > +}
> > +
> > +/// Structure holding the resources required to operate the GPU.
> > +#[allow(dead_code)]
> > +#[pin_data]
> > +pub(crate) struct Gpu {
> > +    spec: GpuSpec,
> > +    /// MMIO mapping of PCI BAR 0
> > +    bar: Devres<Bar0>,
> > +    fw: Firmware,
> > +}
> > +
> > +// TODO replace with something like derive(FromPrimitive)
> > +impl Chipset {
> > +    fn from_u32(value: u32) -> Option<Chipset> {
> > +        match value {
> > +            0x162 => Some(Chipset::TU102),
> > +            0x164 => Some(Chipset::TU104),
> > +            0x166 => Some(Chipset::TU106),
> > +            0x167 => Some(Chipset::TU117),
> > +            0x168 => Some(Chipset::TU116),
> > +            0x172 => Some(Chipset::GA102),
> > +            0x173 => Some(Chipset::GA103),
> > +            0x174 => Some(Chipset::GA104),
> > +            0x176 => Some(Chipset::GA106),
> > +            0x177 => Some(Chipset::GA107),
> > +            0x192 => Some(Chipset::AD102),
> > +            0x193 => Some(Chipset::AD103),
> > +            0x194 => Some(Chipset::AD104),
> > +            0x196 => Some(Chipset::AD106),
> > +            0x197 => Some(Chipset::AD107),
> > +            _ => None,
> > +        }
> > +    }
> > +}
> > +
> > +// TODO replace with something like derive(FromPrimitive)
> > +impl CardType {
> > +    fn from_u32(value: u32) -> Option<CardType> {
> > +        match value {
> > +            0x160 => Some(CardType::TU100),
> > +            0x170 => Some(CardType::GA100),
> > +            0x190 => Some(CardType::AD100),
> 
> Is this how nouveau does it too? I mean, classifying cards as GA100,
> and variants as TU102. It feels wrong to me, because we have for example
> GA100 GPUs. I mean, GA100 is the same kind of thing as a GA102: each is
> a GPU.

Yes, that's what nouveau came up with and it's meant as e.g. 'GA1xx'. But yes,
I agree it's a bit confusing.

OOC, what about the first digit in this example? For Blackwell it would seem to
be 'GB2xx'. Can you shed some light on this?

> 
> If I were naming card types, I'd calling them by their architecture names:
> Turing, Ampere, Ada.

Yeah, that is probably less cryptic. Plus, we should probably name it something
around the term "architecture" rather than "CardType".

> 
> > +            _ => None,
> > +        }
> > +    }
> > +}
> > +
> > +impl GpuSpec {
> > +    fn new(bar: &Devres<Bar0>) -> Result<GpuSpec> {
> > +        let bar = bar.try_access().ok_or(ENXIO)?;
> > +        let boot0 = u64::from_le(bar.readq(0));
> > +        let chip = ((boot0 & 0x1ff00000) >> 20) as u32;
> > +
> > +        if boot0 & 0x1f000000 == 0 {
> > +            return Err(ENODEV);
> > +        }
> > +
> > +        let Some(chipset) = Chipset::from_u32(chip) else {
> > +            return Err(ENODEV);
> > +        };
> > +
> > +        let Some(card_type) = CardType::from_u32(chip & 0x1f0) else {
> > +            return Err(ENODEV);
> > +        };
> > +
> > +        Ok(Self {
> > +            boot0,
> > +            card_type,
> > +            chipset,
> > +            chiprev: (boot0 & 0xff) as u8,
> > +        })
> > +    }
> > +}
> > +
> > +impl Firmware {
> > +    fn new(dev: &device::Device, spec: &GpuSpec, ver: &str) -> Result<Firmware> {
> > +        let mut chip_name = CString::try_from_fmt(fmt!("{:?}", spec.chipset))?;
> > +        chip_name.make_ascii_lowercase();
> > +
> > +        let fw_booter_load_path =
> > +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/booter_load-{}.bin", &*chip_name, ver))?;
> > +        let fw_booter_unload_path =
> > +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/booter_unload-{}.bin", &*chip_name, ver))?;
> > +        let fw_gsp_path =
> > +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/gsp-{}.bin", &*chip_name, ver))?;
> > +
> > +        let booter_load = firmware::Firmware::request(&fw_booter_load_path, dev)?;
> > +        let booter_unload = firmware::Firmware::request(&fw_booter_unload_path, dev)?;
> > +        let gsp = firmware::Firmware::request(&fw_gsp_path, dev)?;
> > +
> > +        Ok(Firmware {
> > +            booter_load,
> > +            booter_unload,
> > +            gsp,
> > +        })
> > +    }
> > +}
> > +
> > +impl Gpu {
> > +    pub(crate) fn new(pdev: &pci::Device, bar: Devres<Bar0>) -> Result<impl PinInit<Self>> {
> > +        let spec = GpuSpec::new(&bar)?;
> > +        let fw = Firmware::new(pdev.as_ref(), &spec, "535.113.01")?;
> 
> lol there it is: our one, "stable" set of GSP firmware. Maybe a one line comment
> above might be appropriate, to mention that this is hardcoded, but new firmware
> versions will not be. On the other hand, that's obvious. :)

Well, I guess we'll have to probe what the distribution provides us with and see
if that's supported and sufficient for the chipset we try to initialize.

> 
> > +
> > +        dev_info!(
> > +            pdev.as_ref(),
> > +            "NVIDIA {:?} ({:#x})",
> > +            spec.chipset,
> > +            spec.boot0
> > +        );
> > +
> > +        Ok(pin_init!(Self { spec, bar, fw }))
> > +    }
> > +}
> > diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> > new file mode 100644
> > index 000000000000..b130d9ca6a0f
> > --- /dev/null
> > +++ b/drivers/gpu/nova-core/nova_core.rs
> > @@ -0,0 +1,14 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Nova Core GPU Driver
> > +
> > +mod driver;
> > +mod gpu;
> > +
> > +kernel::module_pci_driver! {
> > +    type: driver::NovaCore,
> > +    name: "NovaCore",
> > +    author: "Danilo Krummrich",
> > +    description: "Nova Core GPU driver",
> > +    license: "GPL v2",
> > +}
> > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > index 44c9ef1435a2..5df981920a94 100644
> > --- a/drivers/video/Kconfig
> > +++ b/drivers/video/Kconfig
> > @@ -39,6 +39,7 @@ source "drivers/gpu/vga/Kconfig"
> >   source "drivers/gpu/host1x/Kconfig"
> >   source "drivers/gpu/ipu-v3/Kconfig"
> > +source "drivers/gpu/nova-core/Kconfig"
> >   source "drivers/gpu/drm/Kconfig"
> > 
> > base-commit: 69b8923f5003664e3ffef102e73333edfa2abdcf
> 
> I'm always grateful when anyone uses "git format-patch --base", it makes
> life simpler.
> 
> 
> thanks,
> -- 
> John Hubbard
> 

Re: [PATCH 1/2] gpu: nova-core: add initial driver stub

[Greg KH]

On Sat, Feb 01, 2025 at 01:12:35PM +0100, Danilo Krummrich wrote:
> On Sat, Feb 01, 2025 at 09:33:28AM +0100, Greg KH wrote:
> > On Fri, Jan 31, 2025 at 11:04:24PM +0100, Danilo Krummrich wrote:
> > > +impl Gpu {
> > > +    pub(crate) fn new(pdev: &pci::Device, bar: Devres<Bar0>) -> Result<impl PinInit<Self>> {
> > > +        let spec = GpuSpec::new(&bar)?;
> > > +        let fw = Firmware::new(pdev.as_ref(), &spec, "535.113.01")?;
> > > +
> > > +        dev_info!(
> > > +            pdev.as_ref(),
> > > +            "NVIDIA {:?} ({:#x})",
> > > +            spec.chipset,
> > > +            spec.boot0
> > > +        );
> > 
> > When drivers work properly, they should be quiet, so can you move this
> > to dev_dbg()?
> 
> Sure, the only reason I made this dev_info!() is because, as an initial
> skeleton, the driver isn't doing anything else for now. So, I thought it would
> be nice to have some sign of life.
> 
> Of course, the intention was to remove this, once there's any other sign of
> life.

Ok, that's fine, just a constant reminder I send everyone for new
drivers when I see this happen :)

Re: [PATCH 1/2] gpu: nova-core: add initial driver stub

[John Hubbard]

On 2/1/25 5:52 AM, Danilo Krummrich wrote:
> On Fri, Jan 31, 2025 at 08:01:00PM -0800, John Hubbard wrote:
>> On 1/31/25 2:04 PM, Danilo Krummrich wrote:
...
>>> +/// Enum representation of the GPU generation.
>>> +#[derive(Debug)]
>>> +pub(crate) enum CardType {
>>> +    /// Turing
>>> +    TU100 = 0x160,
>>> +    /// Ampere
>>> +    GA100 = 0x170,
>>> +    /// Ada Lovelace
>>> +    AD100 = 0x190,
>>> +}
>>> +
>>> +/// Structure holding the metadata of the GPU.
>>> +#[allow(dead_code)]
>>> +pub(crate) struct GpuSpec {
>>> +    /// Contents of the boot0 register.
>>> +    boot0: u64,
>>
>> It is redundant to store boot0, when all of the following fields
>> are deduced from boot0.
> 
> Yes, I think we can probably remove it, I only use it to print it in Gpu::new()
> as a sign of life and because I don't know if boot0 contains any other useful
> information than chipset and chiprev.
> 
> But maybe you can help me out here? :) That is, share the register layout and
> field names. This way I could also get rid of those magic numbers, and put in
> proper naming for fields, masks and shifts.
> 

We've open sourced those, starting around 2017, and for example the Ampere
fields for boot0 are here:

https://github.com/NVIDIA/open-gpu-doc/blob/master/manuals/ampere/ga100/dev_boot.ref.txt

If more information is needed in this area, let me know and we might be able
to open source it, depending of course on what it is. It's just 
explaining what
the chip is, after all.

...
>>> +// TODO replace with something like derive(FromPrimitive)
>>> +impl CardType {
>>> +    fn from_u32(value: u32) -> Option<CardType> {
>>> +        match value {
>>> +            0x160 => Some(CardType::TU100),
>>> +            0x170 => Some(CardType::GA100),
>>> +            0x190 => Some(CardType::AD100),
>>
>> Is this how nouveau does it too? I mean, classifying cards as GA100,
>> and variants as TU102. It feels wrong to me, because we have for example
>> GA100 GPUs. I mean, GA100 is the same kind of thing as a GA102: each is
>> a GPU.
> 
> Yes, that's what nouveau came up with and it's meant as e.g. 'GA1xx'. But yes,
> I agree it's a bit confusing.
> 
> OOC, what about the first digit in this example? For Blackwell it would seem to
> be 'GB2xx'. Can you shed some light on this?

I'll take a homework assignment to go dig that up. After GSP and Open RM 
came
out, we stopped explicitly publishing to 
https://github.com/NVIDIA/open-gpu-doc ,
because the same information was being published in
https://github.com/NVIDIA/open-gpu-kernel-modules .


> 
>>
>> If I were naming card types, I'd calling them by their architecture names:
>> Turing, Ampere, Ada.
> 
> Yeah, that is probably less cryptic. Plus, we should probably name it something
> around the term "architecture" rather than "CardType".

Yes, "architecture" is accurate.

> 
>>
>>> +            _ => None,
>>> +        }
>>> +    }
>>> +}
>>> +
>>> +impl GpuSpec {
>>> +    fn new(bar: &Devres<Bar0>) -> Result<GpuSpec> {
>>> +        let bar = bar.try_access().ok_or(ENXIO)?;
>>> +        let boot0 = u64::from_le(bar.readq(0));
>>> +        let chip = ((boot0 & 0x1ff00000) >> 20) as u32;
>>> +
>>> +        if boot0 & 0x1f000000 == 0 {
>>> +            return Err(ENODEV);
>>> +        }
>>> +
>>> +        let Some(chipset) = Chipset::from_u32(chip) else {
>>> +            return Err(ENODEV);
>>> +        };
>>> +
>>> +        let Some(card_type) = CardType::from_u32(chip & 0x1f0) else {
>>> +            return Err(ENODEV);
>>> +        };
>>> +
>>> +        Ok(Self {
>>> +            boot0,
>>> +            card_type,
>>> +            chipset,
>>> +            chiprev: (boot0 & 0xff) as u8,
>>> +        })
>>> +    }
>>> +}
>>> +
>>> +impl Firmware {
>>> +    fn new(dev: &device::Device, spec: &GpuSpec, ver: &str) -> Result<Firmware> {
>>> +        let mut chip_name = CString::try_from_fmt(fmt!("{:?}", spec.chipset))?;
>>> +        chip_name.make_ascii_lowercase();
>>> +
>>> +        let fw_booter_load_path =
>>> +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/booter_load-{}.bin", &*chip_name, ver))?;
>>> +        let fw_booter_unload_path =
>>> +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/booter_unload-{}.bin", &*chip_name, ver))?;
>>> +        let fw_gsp_path =
>>> +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/gsp-{}.bin", &*chip_name, ver))?;
>>> +
>>> +        let booter_load = firmware::Firmware::request(&fw_booter_load_path, dev)?;
>>> +        let booter_unload = firmware::Firmware::request(&fw_booter_unload_path, dev)?;
>>> +        let gsp = firmware::Firmware::request(&fw_gsp_path, dev)?;
>>> +
>>> +        Ok(Firmware {
>>> +            booter_load,
>>> +            booter_unload,
>>> +            gsp,
>>> +        })
>>> +    }
>>> +}
>>> +
>>> +impl Gpu {
>>> +    pub(crate) fn new(pdev: &pci::Device, bar: Devres<Bar0>) -> Result<impl PinInit<Self>> {
>>> +        let spec = GpuSpec::new(&bar)?;
>>> +        let fw = Firmware::new(pdev.as_ref(), &spec, "535.113.01")?;
>>
>> lol there it is: our one, "stable" set of GSP firmware. Maybe a one line comment
>> above might be appropriate, to mention that this is hardcoded, but new firmware
>> versions will not be. On the other hand, that's obvious. :)
> 
> Well, I guess we'll have to probe what the distribution provides us with and see
> if that's supported and sufficient for the chipset we try to initialize.

Oh, I think we can leave it alone for a moment. We're working hard on a
firmware plan to make things better.


thanks,
-- 
John Hubbard

Re: [PATCH 1/2] gpu: nova-core: add initial driver stub

[Joel Fernandes]

Hi Danilo,

On Fri, Jan 31, 2025 at 11:04:24PM +0100, Danilo Krummrich wrote:
> Add the initial nova-core driver stub.
> 
> nova-core is intended to serve as a common base for nova-drm (the
> corresponding DRM driver) and the vGPU manager VFIO driver, serving as a
> hard- and firmware abstraction layer for GSP-based NVIDIA GPUs.
> 
> The Nova project, including nova-core and nova-drm, in the long term,
> is intended to serve as the successor of Nouveau for all GSP-based GPUs.
> 
> The motivation for both, starting a successor project for Nouveau and
> doing so using the Rust programming language, is documented in detail
> through a previous post on the mailing list [1], an LWN article [2] and a
> talk from LPC '24.
> 
> In order to avoid the chicken and egg problem to require a user to
> upstream Rust abstractions, but at the same time require the Rust
> abstractions to implement the driver, nova-core kicks off as a driver
> stub and is subsequently developed upstream.
> 
> Link: https://lore.kernel.org/dri-devel/Zfsj0_tb-0-tNrJy@cassiopeiae/T/#u [1]
> Link: https://lwn.net/Articles/990736/ [2]
> Link: https://youtu.be/3Igmx28B3BQ?si=sBdSEer4tAPKGpOs [3]
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  MAINTAINERS                        |  10 ++
>  drivers/gpu/Makefile               |   1 +
[..]
> diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
> index 8997f0096545..36a54d456630 100644
> --- a/drivers/gpu/Makefile
> +++ b/drivers/gpu/Makefile
> @@ -5,3 +5,4 @@
>  obj-y			+= host1x/ drm/ vga/
>  obj-$(CONFIG_IMX_IPUV3_CORE)	+= ipu-v3/
>  obj-$(CONFIG_TRACE_GPU_MEM)		+= trace/
> +obj-$(CONFIG_NOVA_CORE)		+= nova-core/
> diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
> new file mode 100644
> index 000000000000..33ac937b244a
> --- /dev/null
> +++ b/drivers/gpu/nova-core/Kconfig
> @@ -0,0 +1,13 @@
> +config NOVA_CORE
> +	tristate "Nova Core GPU driver"
> +	depends on PCI
> +	depends on RUST
> +	depends on RUST_FW_LOADER_ABSTRACTIONS
> +	default n
> +	help
> +	  Choose this if you want to build the Nova Core driver for Nvidia
> +	  GSP-based GPUs.
> +
> +	  This driver is work in progress and may not be functional.
> +
> +	  If M is selected, the module will be called nova-core.
> diff --git a/drivers/gpu/nova-core/Makefile b/drivers/gpu/nova-core/Makefile
> new file mode 100644
> index 000000000000..2d78c50126e1
> --- /dev/null
> +++ b/drivers/gpu/nova-core/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_NOVA_CORE) += nova_core.o
> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> new file mode 100644
> index 000000000000..2a2aa9b0630b
> --- /dev/null
> +++ b/drivers/gpu/nova-core/driver.rs
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use kernel::{bindings, c_str, pci, prelude::*};
> +
> +use crate::gpu::Gpu;
> +
> +#[pin_data]
> +pub(crate) struct NovaCore {
> +    #[pin]
> +    pub(crate) gpu: Gpu,
> +}

I am curious what is the need for pinning here in the patch in its current
form, is it for future-proofing?

I looked through the sample PCI driver example you had posted and did not see
pinning there [1]. Thanks for the clarification.
[1] https://lore.kernel.org/all/20241219170425.12036-12-dakr@kernel.org/

> +
> +const BAR0_SIZE: usize = 8;
> +pub(crate) type Bar0 = pci::Bar<BAR0_SIZE>;
> +
> +kernel::pci_device_table!(
> +    PCI_TABLE,
> +    MODULE_PCI_TABLE,
> +    <NovaCore as pci::Driver>::IdInfo,
> +    [(
> +        pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_NVIDIA, bindings::PCI_ANY_ID as _),

Does this mean it will match even non-GSP Nvidia devices?

> +        ()
> +    )]
> +);
> +
> +impl pci::Driver for NovaCore {
> +    type IdInfo = ();
> +    const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
> +
> +    fn probe(pdev: &mut pci::Device, _info: &Self::IdInfo) -> Result<Pin<KBox<Self>>> {
> +        dev_dbg!(pdev.as_ref(), "Probe Nova Core GPU driver.\n");
> +
> +        pdev.enable_device_mem()?;
> +        pdev.set_master();
> +
> +        let bar = pdev.iomap_region_sized::<BAR0_SIZE>(0, c_str!("nova-core"))?;
> +
> +        let this = KBox::pin_init(
> +            try_pin_init!(Self {
> +                gpu <- Gpu::new(pdev, bar)?,
> +            }),
> +            GFP_KERNEL,
> +        )?;
> +
> +        Ok(this)
> +    }
> +}
> diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
> new file mode 100644
> index 000000000000..cf62390e72eb
> --- /dev/null
> +++ b/drivers/gpu/nova-core/gpu.rs
> @@ -0,0 +1,171 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use kernel::{
> +    device, devres::Devres, error::code::*, firmware, fmt, pci, prelude::*, str::CString,
> +};
> +
> +use crate::driver::Bar0;
> +use core::fmt::Debug;
> +
> +/// Enum representation of the GPU chipset.
> +#[derive(Debug)]
> +pub(crate) enum Chipset {
> +    TU102 = 0x162,
> +    TU104 = 0x164,
> +    TU106 = 0x166,
> +    TU117 = 0x167,
> +    TU116 = 0x168,
> +    GA102 = 0x172,
> +    GA103 = 0x173,
> +    GA104 = 0x174,
> +    GA106 = 0x176,
> +    GA107 = 0x177,
> +    AD102 = 0x192,
> +    AD103 = 0x193,
> +    AD104 = 0x194,
> +    AD106 = 0x196,
> +    AD107 = 0x197,
> +}
> +
> +/// Enum representation of the GPU generation.
> +#[derive(Debug)]
> +pub(crate) enum CardType {
> +    /// Turing
> +    TU100 = 0x160,
> +    /// Ampere
> +    GA100 = 0x170,
> +    /// Ada Lovelace
> +    AD100 = 0x190,
> +}
> +
> +/// Structure holding the metadata of the GPU.
> +#[allow(dead_code)]
> +pub(crate) struct GpuSpec {
> +    /// Contents of the boot0 register.
> +    boot0: u64,
> +    card_type: CardType,
> +    chipset: Chipset,
> +    /// The revision of the chipset.
> +    chiprev: u8,
> +}
> +
> +/// Structure encapsulating the firmware blobs required for the GPU to operate.
> +#[allow(dead_code)]
> +pub(crate) struct Firmware {
> +    booter_load: firmware::Firmware,
> +    booter_unload: firmware::Firmware,
> +    gsp: firmware::Firmware,
> +}
> +
> +/// Structure holding the resources required to operate the GPU.
> +#[allow(dead_code)]
> +#[pin_data]
> +pub(crate) struct Gpu {
> +    spec: GpuSpec,
> +    /// MMIO mapping of PCI BAR 0
> +    bar: Devres<Bar0>,
> +    fw: Firmware,
> +}

Here, #[pin_data] is used on top of struct Gpu, but no #[pin] is used?

According to [1]
Place this macro on a struct definition and then #[pin] in front of the
attributes of each field you want to structurally pin.

[1]
https://rust.docs.kernel.org/macros/attr.pin_data.html

> +
> +// TODO replace with something like derive(FromPrimitive)
> +impl Chipset {
> +    fn from_u32(value: u32) -> Option<Chipset> {
> +        match value {
> +            0x162 => Some(Chipset::TU102),
> +            0x164 => Some(Chipset::TU104),
> +            0x166 => Some(Chipset::TU106),
> +            0x167 => Some(Chipset::TU117),
> +            0x168 => Some(Chipset::TU116),
> +            0x172 => Some(Chipset::GA102),
> +            0x173 => Some(Chipset::GA103),
> +            0x174 => Some(Chipset::GA104),
> +            0x176 => Some(Chipset::GA106),
> +            0x177 => Some(Chipset::GA107),
> +            0x192 => Some(Chipset::AD102),
> +            0x193 => Some(Chipset::AD103),
> +            0x194 => Some(Chipset::AD104),
> +            0x196 => Some(Chipset::AD106),
> +            0x197 => Some(Chipset::AD107),
> +            _ => None,
> +        }
> +    }
> +}
> +
> +// TODO replace with something like derive(FromPrimitive)
> +impl CardType {
> +    fn from_u32(value: u32) -> Option<CardType> {
> +        match value {
> +            0x160 => Some(CardType::TU100),
> +            0x170 => Some(CardType::GA100),
> +            0x190 => Some(CardType::AD100),
> +            _ => None,
> +        }
> +    }
> +}
> +
> +impl GpuSpec {
> +    fn new(bar: &Devres<Bar0>) -> Result<GpuSpec> {
> +        let bar = bar.try_access().ok_or(ENXIO)?;
> +        let boot0 = u64::from_le(bar.readq(0));
> +        let chip = ((boot0 & 0x1ff00000) >> 20) as u32;
> +
> +        if boot0 & 0x1f000000 == 0 {
> +            return Err(ENODEV);
> +        }
> +
> +        let Some(chipset) = Chipset::from_u32(chip) else {
> +            return Err(ENODEV);
> +        };
> +
> +        let Some(card_type) = CardType::from_u32(chip & 0x1f0) else {
> +            return Err(ENODEV);
> +        };

Can use ok_or() here as well?

let chipset = Chipset::from_u32(chip).ok_or(ENODEV)?;
let card_type = CardType::from_u32(chip & 0x1f0).ok_or(ENODEV)?;

Or does it not work for some reason?

thanks,

 - Joel


> +
> +        Ok(Self {
> +            boot0,
> +            card_type,
> +            chipset,
> +            chiprev: (boot0 & 0xff) as u8,
> +        })
> +    }
> +}
> +
> +impl Firmware {
> +    fn new(dev: &device::Device, spec: &GpuSpec, ver: &str) -> Result<Firmware> {
> +        let mut chip_name = CString::try_from_fmt(fmt!("{:?}", spec.chipset))?;
> +        chip_name.make_ascii_lowercase();
> +
> +        let fw_booter_load_path =
> +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/booter_load-{}.bin", &*chip_name, ver))?;
> +        let fw_booter_unload_path =
> +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/booter_unload-{}.bin", &*chip_name, ver))?;
> +        let fw_gsp_path =
> +            CString::try_from_fmt(fmt!("nvidia/{}/gsp/gsp-{}.bin", &*chip_name, ver))?;
> +
> +        let booter_load = firmware::Firmware::request(&fw_booter_load_path, dev)?;
> +        let booter_unload = firmware::Firmware::request(&fw_booter_unload_path, dev)?;
> +        let gsp = firmware::Firmware::request(&fw_gsp_path, dev)?;
> +
> +        Ok(Firmware {
> +            booter_load,
> +            booter_unload,
> +            gsp,
> +        })
> +    }
> +}
> +
> +impl Gpu {
> +    pub(crate) fn new(pdev: &pci::Device, bar: Devres<Bar0>) -> Result<impl PinInit<Self>> {
> +        let spec = GpuSpec::new(&bar)?;
> +        let fw = Firmware::new(pdev.as_ref(), &spec, "535.113.01")?;
> +
> +        dev_info!(
> +            pdev.as_ref(),
> +            "NVIDIA {:?} ({:#x})",
> +            spec.chipset,
> +            spec.boot0
> +        );
> +
> +        Ok(pin_init!(Self { spec, bar, fw }))
> +    }
> +}
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> new file mode 100644
> index 000000000000..b130d9ca6a0f
> --- /dev/null
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Nova Core GPU Driver
> +
> +mod driver;
> +mod gpu;
> +
> +kernel::module_pci_driver! {
> +    type: driver::NovaCore,
> +    name: "NovaCore",
> +    author: "Danilo Krummrich",
> +    description: "Nova Core GPU driver",
> +    license: "GPL v2",
> +}
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 44c9ef1435a2..5df981920a94 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -39,6 +39,7 @@ source "drivers/gpu/vga/Kconfig"
>  
>  source "drivers/gpu/host1x/Kconfig"
>  source "drivers/gpu/ipu-v3/Kconfig"
> +source "drivers/gpu/nova-core/Kconfig"
>  
>  source "drivers/gpu/drm/Kconfig"
>  
> 
> base-commit: 69b8923f5003664e3ffef102e73333edfa2abdcf
> -- 
> 2.48.1
> 

Re: [PATCH 1/2] gpu: nova-core: add initial driver stub

[John Hubbard]

On 2/3/25 12:24 PM, Joel Fernandes wrote:
> Hi Danilo,
> On Fri, Jan 31, 2025 at 11:04:24PM +0100, Danilo Krummrich wrote:
...
>> +const BAR0_SIZE: usize = 8;
>> +pub(crate) type Bar0 = pci::Bar<BAR0_SIZE>;
>> +
>> +kernel::pci_device_table!(
>> +    PCI_TABLE,
>> +    MODULE_PCI_TABLE,
>> +    <NovaCore as pci::Driver>::IdInfo,
>> +    [(
>> +        pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_NVIDIA, bindings::PCI_ANY_ID as _),
> 
> Does this mean it will match even non-GSP Nvidia devices?
> 

Yes, it does. However, the Gpu construction a little further down
will fail on pre-GSP GPUs, thus failing the probe(), so all is well.

More below:

>> +        ()
>> +    )]
>> +);
>> +
>> +impl pci::Driver for NovaCore {
>> +    type IdInfo = ();
>> +    const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
>> +
>> +    fn probe(pdev: &mut pci::Device, _info: &Self::IdInfo) -> Result<Pin<KBox<Self>>> {
>> +        dev_dbg!(pdev.as_ref(), "Probe Nova Core GPU driver.\n");
>> +
>> +        pdev.enable_device_mem()?;
>> +        pdev.set_master();
>> +
>> +        let bar = pdev.iomap_region_sized::<BAR0_SIZE>(0, c_str!("nova-core"))?;
>> +
>> +        let this = KBox::pin_init(
>> +            try_pin_init!(Self {
>> +                gpu <- Gpu::new(pdev, bar)?,

Here. Try to construct a Gpu, which tries to construct a GpuSpec, which
fails out if Chipset is not listed, or if CardType (which should be
renamed to Architecture) is not listed.

And only Turing+ GPUs are listed. Turing is the first GPU that has a
GSP unit.

By the way, I have loaded this on a system with a Kepler GPU (pre-Turing),
and an Ampere GPU, and traced through actually loading NovaCore, and it
behaves as described above.

thanks,
-- 
John Hubbard

Re: [PATCH 1/2] gpu: nova-core: add initial driver stub

[Joel Fernandes]

On Mon, Feb 3, 2025 at 4:00 PM John Hubbard <jhubbard@nvidia.com> wrote:
[..]
>
> >> +        ()
> >> +    )]
> >> +);
> >> +
> >> +impl pci::Driver for NovaCore {
> >> +    type IdInfo = ();
> >> +    const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
> >> +
> >> +    fn probe(pdev: &mut pci::Device, _info: &Self::IdInfo) -> Result<Pin<KBox<Self>>> {
> >> +        dev_dbg!(pdev.as_ref(), "Probe Nova Core GPU driver.\n");
> >> +
> >> +        pdev.enable_device_mem()?;
> >> +        pdev.set_master();
> >> +
> >> +        let bar = pdev.iomap_region_sized::<BAR0_SIZE>(0, c_str!("nova-core"))?;
> >> +
> >> +        let this = KBox::pin_init(
> >> +            try_pin_init!(Self {
> >> +                gpu <- Gpu::new(pdev, bar)?,
>
> Here. Try to construct a Gpu, which tries to construct a GpuSpec, which
> fails out if Chipset is not listed, or if CardType (which should be
> renamed to Architecture) is not listed.
>
> And only Turing+ GPUs are listed. Turing is the first GPU that has a
> GSP unit.
>
> By the way, I have loaded this on a system with a Kepler GPU (pre-Turing),
> and an Ampere GPU, and traced through actually loading NovaCore, and it
> behaves as described above.


Ah that makes sense, Thanks John!

 - Joel

Re: [PATCH 2/2] gpu: nova-core: add initial documentation

[Timur Tabi]

On Fri, 2025-01-31 at 23:04 +0100, Danilo Krummrich wrote:
> +Rust abstraction for debugfs APIs.
> +
> +| Reference: Export GSP log buffers
> +| Complexity: Beginner

Seriously?

Re: [PATCH 1/2] gpu: nova-core: add initial driver stub

[Timur Tabi]

On Fri, 2025-01-31 at 23:04 +0100, Danilo Krummrich wrote:
> +/// Structure encapsulating the firmware blobs required for the GPU to operate.
> +#[allow(dead_code)]
> +pub(crate) struct Firmware {
> +    booter_load: firmware::Firmware,
> +    booter_unload: firmware::Firmware,
> +    gsp: firmware::Firmware,

What about the bootloader?

Re: [PATCH 1/2] gpu: nova-core: add initial driver stub

[Danilo Krummrich]

On Mon, Feb 03, 2025 at 03:24:10PM -0500, Joel Fernandes wrote:
> Hi Danilo,
> 
> On Fri, Jan 31, 2025 at 11:04:24PM +0100, Danilo Krummrich wrote:
> > +#[pin_data]
> > +pub(crate) struct NovaCore {
> > +    #[pin]
> > +    pub(crate) gpu: Gpu,
> > +}
> 
> I am curious what is the need for pinning here in the patch in its current
> form, is it for future-proofing?

Yes, it is.

It's not always needed, but since I know that further down the road we'll need
at least a few mutexes, it seemed reasonable to already consider it.

> 
> I looked through the sample PCI driver example you had posted and did not see
> pinning there [1]. Thanks for the clarification.
> [1] https://lore.kernel.org/all/20241219170425.12036-12-dakr@kernel.org/

Re: [PATCH 2/2] gpu: nova-core: add initial documentation

[Danilo Krummrich]

On Tue, Feb 04, 2025 at 06:40:41PM +0000, Timur Tabi wrote:
> On Fri, 2025-01-31 at 23:04 +0100, Danilo Krummrich wrote:
> > +Rust abstraction for debugfs APIs.
> > +
> > +| Reference: Export GSP log buffers
> > +| Complexity: Beginner
> 
> Seriously?

Well, that seems indeed a bit optimistic. :-)

Re: [PATCH 1/2] gpu: nova-core: add initial driver stub

[Danilo Krummrich]

On Tue, Feb 04, 2025 at 06:42:46PM +0000, Timur Tabi wrote:
> On Fri, 2025-01-31 at 23:04 +0100, Danilo Krummrich wrote:
> > +/// Structure encapsulating the firmware blobs required for the GPU to operate.
> > +#[allow(dead_code)]
> > +pub(crate) struct Firmware {
> > +    booter_load: firmware::Firmware,
> > +    booter_unload: firmware::Firmware,
> > +    gsp: firmware::Firmware,
> 
> What about the bootloader?

Gonna add it.

Re: [PATCH 1/2] gpu: nova-core: add initial driver stub

[Joel Fernandes]

On Tue, Feb 4, 2025 at 1:46 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Mon, Feb 03, 2025 at 03:24:10PM -0500, Joel Fernandes wrote:
> > Hi Danilo,
> >
> > On Fri, Jan 31, 2025 at 11:04:24PM +0100, Danilo Krummrich wrote:
> > > +#[pin_data]
> > > +pub(crate) struct NovaCore {
> > > +    #[pin]
> > > +    pub(crate) gpu: Gpu,
> > > +}
> >
> > I am curious what is the need for pinning here in the patch in its current
> > form, is it for future-proofing?
>
> Yes, it is.
>
> It's not always needed, but since I know that further down the road we'll need
> at least a few mutexes, it seemed reasonable to already consider it.

It seems reasonable to me as well, I would probably also add a code
comment there about what is expected to be unmovable in the future
(Just in case any code readers don't raise the same question I was
raising).

The source of the confusion for a reader could be the documentation
expecting a #[pin].

 - Joel