* [PATCH v9 0/6] gpu: nova-core: expose the logging buffers via debugfs
@ 2026-03-16 5:57 Timur Tabi
2026-03-16 5:57 ` [PATCH v9 1/6] rust: device: add device name method Timur Tabi
` (7 more replies)
0 siblings, 8 replies; 21+ messages in thread
From: Timur Tabi @ 2026-03-16 5:57 UTC (permalink / raw)
To: Miguel Ojeda, Danilo Krummrich, Alice Ryhl, Gary Guo, mmaurer,
Alexandre Courbot, John Hubbard, Joel Fernandes, ecourtney,
rust-for-linux, nouveau
GSP-RM writes its printf message to "logging buffers", which are blocks
memory allocated by the driver. The messages are encoded, so exposing
the buffers as debugfs entries allows the buffers to be extracted and
decoded by a special application.
When the driver loads, a /sys/kernel/debug/nova_core root entry is
created. To do this, the normal module_pci_driver! macro call is
replaced with an explicit initialization function, as this allows
that debugfs entry to be created once for all GPUs.
Then in each GPU's initialization, a subdirectory based on the PCI
BDF name is created, and the logging buffer entries are created under
that.
This series depends on Eliot Courtney's Cmdq patch set [1].
Note: the debugfs entry has a file size of 0, because debugfs defaults
a 0 size and the Rust abstractions do not adjust it for the same of
the object. Nouveau makes this adjustment manually in the driver.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Tested-by: John Hubbard <jhubbard@nvidia.com>
[1] https://lore.kernel.org/all/20260310-cmdq-locking-v4-0-4e5c4753c408@nvidia.com/
Changes since v8:
1. Added #inline to Device::name()
2. Fixed rustfmt issues again
3. Updated write_to_slice()
4. Dropped "use pin projection in method boot()"
5. Now depends on Eliot's Cmdq patch set
6. Misc nits fixed
Timur Tabi (6):
rust: device: add device name method
rust: uaccess: add write_dma() for copying from DMA buffers to
userspace
rust: dma: implement BinaryWriter for CoherentAllocation<u8>
gpu: nova-core: Replace module_pci_driver! with explicit module init
gpu: nova-core: create debugfs root in module init
gpu: nova-core: create GSP-RM logging buffers debugfs entries
drivers/gpu/nova-core/gsp.rs | 49 ++++++++++++++---
drivers/gpu/nova-core/nova_core.rs | 50 +++++++++++++++++-
rust/helpers/device.c | 5 ++
rust/kernel/device.rs | 11 ++++
rust/kernel/dma.rs | 35 ++++++++++++-
rust/kernel/uaccess.rs | 84 ++++++++++++++++++++++++++----
6 files changed, 213 insertions(+), 21 deletions(-)
base-commit: a544873ce0575b2fd8285a1364d3e09929d9a3ba
prerequisite-patch-id: fefd403caf8af386276351dd12397dda8ae8553f
prerequisite-patch-id: 3e02192944c4dde97e6895a28371479aa49ddc96
prerequisite-patch-id: de85fb30b65233210512b8295d7fe811f485f03f
prerequisite-patch-id: 4232928e15093c74aa6d2a6519a4d7ac5e109607
prerequisite-patch-id: d4f34f7eda8f5dcad2464f1bf3789e78556eb51c
--
2.51.0
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v9 1/6] rust: device: add device name method
2026-03-16 5:57 [PATCH v9 0/6] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
@ 2026-03-16 5:57 ` Timur Tabi
2026-03-16 11:49 ` Gary Guo
2026-03-16 5:57 ` [PATCH v9 2/6] rust: uaccess: add write_dma() for copying from DMA buffers to userspace Timur Tabi
` (6 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Timur Tabi @ 2026-03-16 5:57 UTC (permalink / raw)
To: Miguel Ojeda, Danilo Krummrich, Alice Ryhl, Gary Guo, mmaurer,
Alexandre Courbot, John Hubbard, Joel Fernandes, ecourtney,
rust-for-linux, nouveau
Add a name() method to the `Device` type, which returns a CStr that
contains the device name.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
---
rust/helpers/device.c | 5 +++++
rust/kernel/device.rs | 11 +++++++++++
2 files changed, 16 insertions(+)
diff --git a/rust/helpers/device.c b/rust/helpers/device.c
index a8ab931a9bd1..3be4ee590784 100644
--- a/rust/helpers/device.c
+++ b/rust/helpers/device.c
@@ -25,3 +25,8 @@ __rust_helper void rust_helper_dev_set_drvdata(struct device *dev, void *data)
{
dev_set_drvdata(dev, data);
}
+
+__rust_helper const char *rust_helper_dev_name(const struct device *dev)
+{
+ return dev_name(dev);
+}
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 94e0548e7687..8f6ad4f53101 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -489,6 +489,17 @@ pub fn fwnode(&self) -> Option<&property::FwNode> {
// defined as a `#[repr(transparent)]` wrapper around `fwnode_handle`.
Some(unsafe { &*fwnode_handle.cast() })
}
+
+ /// Returns the name of the device.
+ ///
+ /// This is the kobject name of the device, or its initial name if the kobject is not yet
+ /// available.
+ #[inline]
+ pub fn name(&self) -> &CStr {
+ // SAFETY: By its type invariant `self.as_raw()` is a valid pointer to a `struct device`.
+ // The returned string is valid for the lifetime of the device.
+ unsafe { CStr::from_char_ptr(bindings::dev_name(self.as_raw())) }
+ }
}
// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v9 2/6] rust: uaccess: add write_dma() for copying from DMA buffers to userspace
2026-03-16 5:57 [PATCH v9 0/6] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
2026-03-16 5:57 ` [PATCH v9 1/6] rust: device: add device name method Timur Tabi
@ 2026-03-16 5:57 ` Timur Tabi
2026-03-16 20:42 ` Alice Ryhl
2026-03-17 21:43 ` Miguel Ojeda
2026-03-16 5:57 ` [PATCH v9 3/6] rust: dma: implement BinaryWriter for CoherentAllocation<u8> Timur Tabi
` (5 subsequent siblings)
7 siblings, 2 replies; 21+ messages in thread
From: Timur Tabi @ 2026-03-16 5:57 UTC (permalink / raw)
To: Miguel Ojeda, Danilo Krummrich, Alice Ryhl, Gary Guo, mmaurer,
Alexandre Courbot, John Hubbard, Joel Fernandes, ecourtney,
rust-for-linux, nouveau
Add UserSliceWriter::write_dma() to copy data from a CoherentAllocation<u8>
to userspace. This provides a safe interface for copying DMA buffer
contents to userspace without requiring callers to work with raw pointers.
Because write_dma() and write_slice() have common code, factor that code
out into a helper function, write_raw().
The method handles bounds checking and offset calculation internally,
wrapping the unsafe copy_to_user() call.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
---
rust/kernel/uaccess.rs | 84 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 74 insertions(+), 10 deletions(-)
diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
index f989539a31b4..6be7392ca9f7 100644
--- a/rust/kernel/uaccess.rs
+++ b/rust/kernel/uaccess.rs
@@ -7,6 +7,7 @@
use crate::{
alloc::{Allocator, Flags},
bindings,
+ dma::CoherentAllocation,
error::Result,
ffi::{c_char, c_void},
fs::file,
@@ -459,20 +460,24 @@ pub fn is_empty(&self) -> bool {
self.length == 0
}
- /// Writes raw data to this user pointer from a kernel buffer.
+ /// Low-level write from a raw pointer.
///
- /// Fails with [`EFAULT`] if the write happens on a bad address, or if the write goes out of
- /// bounds of this [`UserSliceWriter`]. This call may modify the associated userspace slice even
- /// if it returns an error.
- pub fn write_slice(&mut self, data: &[u8]) -> Result {
- let len = data.len();
- let data_ptr = data.as_ptr().cast::<c_void>();
+ /// # Safety
+ ///
+ /// The caller must ensure that `ptr` points to a valid slice of `len` bytes (i.e., it is
+ /// valid for reads of `len` bytes).
+ unsafe fn write_raw(&mut self, ptr: *const u8, len: usize) -> Result {
if len > self.length {
return Err(EFAULT);
}
- // SAFETY: `data_ptr` points into an immutable slice of length `len`, so we may read
- // that many bytes from it.
- let res = unsafe { bindings::copy_to_user(self.ptr.as_mut_ptr(), data_ptr, len) };
+ // SAFETY:
+ // - `self.ptr` is a userspace pointer, and `len <= self.length` is checked above to
+ // ensure we don't exceed the caller-specified bounds.
+ // - `ptr` is valid for reading `len` bytes as required by this function's safety contract.
+ // - `copy_to_user` validates the userspace address at runtime and returns non-zero on
+ // failure (e.g., bad address or unmapped memory).
+ let res =
+ unsafe { bindings::copy_to_user(self.ptr.as_mut_ptr(), ptr.cast::<c_void>(), len) };
if res != 0 {
return Err(EFAULT);
}
@@ -481,6 +486,65 @@ pub fn write_slice(&mut self, data: &[u8]) -> Result {
Ok(())
}
+ /// Writes raw data to this user pointer from a kernel buffer.
+ ///
+ /// Fails with [`EFAULT`] if the write happens on a bad address, or if the write goes out of
+ /// bounds of this [`UserSliceWriter`]. This call may modify the associated userspace slice even
+ /// if it returns an error.
+ pub fn write_slice(&mut self, data: &[u8]) -> Result {
+ // SAFETY: `data` is a valid slice, so `data.as_ptr()` is valid for
+ // reading `data.len()` bytes.
+ unsafe { self.write_raw(data.as_ptr(), data.len()) }
+ }
+
+ /// Writes raw data to this user pointer from a DMA coherent allocation.
+ ///
+ /// # Arguments
+ ///
+ /// * `data` - The DMA coherent allocation to copy from.
+ /// * `offset` - The byte offset into `data` to start copying from.
+ /// * `count` - The number of bytes to copy.
+ ///
+ /// # Errors
+ ///
+ /// Returns [`EOVERFLOW`] if `offset + count` overflows.
+ /// Returns [`ERANGE`] if `offset + count` exceeds the size of `data`, or `count` exceeds
+ /// the size of the user-space buffer.
+ /// Returns [`EFAULT`] if the write happens on a bad address, or if the write goes out of
+ /// bounds of this [`UserSliceWriter`].
+ ///
+ /// This call may modify the associated userspace slice even if it returns an error.
+ ///
+ /// Note: The memory may be concurrently modified by hardware (e.g., DMA). In such cases,
+ /// the copied data may be inconsistent, but this does not cause undefined behavior.
+ pub fn write_dma(
+ &mut self,
+ alloc: &CoherentAllocation<u8>,
+ offset: usize,
+ count: usize,
+ ) -> Result {
+ let len = alloc.size();
+ if offset.checked_add(count).ok_or(EOVERFLOW)? > len {
+ return Err(ERANGE);
+ }
+
+ if count > self.len() {
+ return Err(ERANGE);
+ }
+
+ // SAFETY: `start_ptr()` returns a valid pointer to a memory region of `count()` bytes,
+ // as guaranteed by the `CoherentAllocation` invariants. The check above ensures
+ // `offset + count <= len`.
+ let src_ptr = unsafe { alloc.start_ptr().add(offset) };
+
+ // Note: Use `write_raw` instead of `write_slice` because the allocation is coherent
+ // memory that hardware may modify (e.g., DMA); we cannot form a `&[u8]` slice over
+ // such volatile memory.
+ //
+ // SAFETY: `src_ptr` points into the allocation and is valid for `count` bytes (see above).
+ unsafe { self.write_raw(src_ptr, count) }
+ }
+
/// Writes raw data to this user pointer from a kernel buffer partially.
///
/// This is the same as [`Self::write_slice`] but considers the given `offset` into `data` and
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v9 3/6] rust: dma: implement BinaryWriter for CoherentAllocation<u8>
2026-03-16 5:57 [PATCH v9 0/6] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
2026-03-16 5:57 ` [PATCH v9 1/6] rust: device: add device name method Timur Tabi
2026-03-16 5:57 ` [PATCH v9 2/6] rust: uaccess: add write_dma() for copying from DMA buffers to userspace Timur Tabi
@ 2026-03-16 5:57 ` Timur Tabi
2026-03-16 20:46 ` Alice Ryhl
2026-03-17 4:22 ` Alexandre Courbot
2026-03-16 5:57 ` [PATCH v9 4/6] gpu: nova-core: Replace module_pci_driver! with explicit module init Timur Tabi
` (4 subsequent siblings)
7 siblings, 2 replies; 21+ messages in thread
From: Timur Tabi @ 2026-03-16 5:57 UTC (permalink / raw)
To: Miguel Ojeda, Danilo Krummrich, Alice Ryhl, Gary Guo, mmaurer,
Alexandre Courbot, John Hubbard, Joel Fernandes, ecourtney,
rust-for-linux, nouveau
Implement the BinaryWriter trait for CoherentAllocation<u8>, enabling
DMA coherent allocations to be exposed as readable binary files.
The implementation handles offset tracking and bounds checking, copying
data from the coherent allocation to userspace via write_dma().
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
rust/kernel/dma.rs | 35 ++++++++++++++++++++++++++++++++++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 909d56fd5118..b38f16fc6c66 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -5,12 +5,14 @@
//! C header: [`include/linux/dma-mapping.h`](srctree/include/linux/dma-mapping.h)
use crate::{
- bindings, build_assert, device,
+ bindings, build_assert, debugfs, device,
device::{Bound, Core},
error::{to_result, Result},
+ fs::file,
prelude::*,
sync::aref::ARef,
transmute::{AsBytes, FromBytes},
+ uaccess::UserSliceWriter,
};
use core::ptr::NonNull;
@@ -668,6 +670,37 @@ fn drop(&mut self) {
// can be sent to another thread.
unsafe impl<T: AsBytes + FromBytes + Send> Send for CoherentAllocation<T> {}
+// SAFETY: Sharing `&CoherentAllocation` across threads is safe if `T` is `Sync`, because all
+// methods that access the buffer contents (`field_read`, `field_write`, `as_slice`,
+// `as_slice_mut`) are `unsafe`, and callers are responsible for ensuring no data races occur.
+// The safe methods only return metadata or raw pointers whose use requires `unsafe`.
+unsafe impl<T: AsBytes + FromBytes + Sync> Sync for CoherentAllocation<T> {}
+
+impl debugfs::BinaryWriter for CoherentAllocation<u8> {
+ fn write_to_slice(
+ &self,
+ writer: &mut UserSliceWriter,
+ offset: &mut file::Offset,
+ ) -> Result<usize> {
+ if offset.is_negative() {
+ return Err(EINVAL);
+ }
+
+ // If the offset is too large for a usize (e.g. on 32-bit platforms),
+ // then consider that as past EOF and just return 0 bytes.
+ let Ok(offset_val) = usize::try_from(*offset) else {
+ return Ok(0);
+ };
+
+ let count = self.count().saturating_sub(offset_val).min(writer.len());
+
+ writer.write_dma(self, offset_val, count)?;
+
+ *offset += count as i64;
+ Ok(count)
+ }
+}
+
/// Reads a field of an item from an allocated region of structs.
///
/// # Examples
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v9 4/6] gpu: nova-core: Replace module_pci_driver! with explicit module init
2026-03-16 5:57 [PATCH v9 0/6] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
` (2 preceding siblings ...)
2026-03-16 5:57 ` [PATCH v9 3/6] rust: dma: implement BinaryWriter for CoherentAllocation<u8> Timur Tabi
@ 2026-03-16 5:57 ` Timur Tabi
2026-03-16 16:28 ` Gary Guo
2026-03-16 5:57 ` [PATCH v9 5/6] gpu: nova-core: create debugfs root in " Timur Tabi
` (3 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Timur Tabi @ 2026-03-16 5:57 UTC (permalink / raw)
To: Miguel Ojeda, Danilo Krummrich, Alice Ryhl, Gary Guo, mmaurer,
Alexandre Courbot, John Hubbard, Joel Fernandes, ecourtney,
rust-for-linux, nouveau
Replace the module_pci_driver! macro with an explicit module
initialization using the standard module! macro and InPlaceModule
trait implementation. No functional change intended, with the
exception that the driver now prints a message when loaded.
This change is necessary so that we can create a top-level "nova_core"
debugfs entry when the driver is loaded.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
drivers/gpu/nova-core/nova_core.rs | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index b5caf1044697..0114a59825aa 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -2,6 +2,13 @@
//! Nova Core GPU Driver
+use kernel::{
+ driver::Registration,
+ pci,
+ prelude::*,
+ InPlaceModule, //
+};
+
#[macro_use]
mod bitfield;
@@ -20,8 +27,22 @@
pub(crate) const MODULE_NAME: &core::ffi::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
-kernel::module_pci_driver! {
- type: driver::NovaCore,
+#[pin_data]
+struct NovaCoreModule {
+ #[pin]
+ _driver: Registration<pci::Adapter<driver::NovaCore>>,
+}
+
+impl InPlaceModule for NovaCoreModule {
+ fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
+ try_pin_init!(Self {
+ _driver <- Registration::new(MODULE_NAME, module),
+ })
+ }
+}
+
+module! {
+ type: NovaCoreModule,
name: "NovaCore",
authors: ["Danilo Krummrich"],
description: "Nova Core GPU driver",
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v9 5/6] gpu: nova-core: create debugfs root in module init
2026-03-16 5:57 [PATCH v9 0/6] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
` (3 preceding siblings ...)
2026-03-16 5:57 ` [PATCH v9 4/6] gpu: nova-core: Replace module_pci_driver! with explicit module init Timur Tabi
@ 2026-03-16 5:57 ` Timur Tabi
2026-03-16 16:28 ` Gary Guo
2026-03-16 5:57 ` [PATCH v9 6/6] gpu: nova-core: create GSP-RM logging buffers debugfs entries Timur Tabi
` (2 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Timur Tabi @ 2026-03-16 5:57 UTC (permalink / raw)
To: Miguel Ojeda, Danilo Krummrich, Alice Ryhl, Gary Guo, mmaurer,
Alexandre Courbot, John Hubbard, Joel Fernandes, ecourtney,
rust-for-linux, nouveau
Create the 'nova_core' root debugfs entry when the driver loads.
Normally, non-const global variables need to be protected by a
mutex. Instead, we use unsafe code, as we know the entry is never
modified after the driver is loaded. This solves the lifetime
issue of the mutex guard, which would otherwise have required the
use of `pin_init_scope`.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
drivers/gpu/nova-core/nova_core.rs | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index 0114a59825aa..b7e6c6274aa8 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -3,6 +3,7 @@
//! Nova Core GPU Driver
use kernel::{
+ debugfs,
driver::Registration,
pci,
prelude::*,
@@ -27,16 +28,40 @@
pub(crate) const MODULE_NAME: &core::ffi::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
+// FIXME: Move this into per-module data once that exists
+static mut DEBUGFS_ROOT: Option<debugfs::Dir> = None;
+
+/// Guard that clears DEBUGFS_ROOT when dropped.
+struct DebugfsRootGuard;
+
+impl Drop for DebugfsRootGuard {
+ fn drop(&mut self) {
+ // SAFETY: This guard is dropped after _driver (due to field order),
+ // so the driver is unregistered and no probe() can be running.
+ unsafe { DEBUGFS_ROOT = None };
+ }
+}
+
#[pin_data]
struct NovaCoreModule {
+ // Fields are dropped in declaration order, so _driver is dropped first,
+ // then _debugfs_guard clears DEBUGFS_ROOT.
#[pin]
_driver: Registration<pci::Adapter<driver::NovaCore>>,
+ _debugfs_guard: DebugfsRootGuard,
}
impl InPlaceModule for NovaCoreModule {
fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
+ let dir = debugfs::Dir::new(kernel::c_str!("nova_core"));
+
+ // SAFETY: We are the only driver code running during init, so there
+ // cannot be any concurrent access to `DEBUGFS_ROOT`.
+ unsafe { DEBUGFS_ROOT = Some(dir) };
+
try_pin_init!(Self {
_driver <- Registration::new(MODULE_NAME, module),
+ _debugfs_guard: DebugfsRootGuard,
})
}
}
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v9 6/6] gpu: nova-core: create GSP-RM logging buffers debugfs entries
2026-03-16 5:57 [PATCH v9 0/6] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
` (4 preceding siblings ...)
2026-03-16 5:57 ` [PATCH v9 5/6] gpu: nova-core: create debugfs root in " Timur Tabi
@ 2026-03-16 5:57 ` Timur Tabi
2026-03-16 16:29 ` Gary Guo
2026-03-16 22:05 ` [PATCH v9 0/6] gpu: nova-core: expose the logging buffers via debugfs John Hubbard
2026-03-17 1:53 ` Eliot Courtney
7 siblings, 1 reply; 21+ messages in thread
From: Timur Tabi @ 2026-03-16 5:57 UTC (permalink / raw)
To: Miguel Ojeda, Danilo Krummrich, Alice Ryhl, Gary Guo, mmaurer,
Alexandre Courbot, John Hubbard, Joel Fernandes, ecourtney,
rust-for-linux, nouveau
Create read-only debugfs entries for LOGINIT, LOGRM, and LOGINTR, which
are the three primary printf logging buffers from GSP-RM. LOGPMU will
be added at a later date, as it requires it support for its RPC message
first.
This patch uses the `pin_init_scope` feature to create the entries.
`pin_init_scope` solves the lifetime issue over the `DEBUGFS_ROOT`
reference by delaying its acquisition until the time the entry is
actually initialized.
Co-developed-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
drivers/gpu/nova-core/gsp.rs | 49 ++++++++++++++++++++++++++++++------
1 file changed, 41 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index a6f3918c20b1..484e6aa48bf3 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -3,6 +3,8 @@
mod boot;
use kernel::{
+ c_str,
+ debugfs,
device,
dma::{
CoherentAllocation,
@@ -100,17 +102,24 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
}
}
-/// GSP runtime data.
-#[pin_data]
-pub(crate) struct Gsp {
- /// Libos arguments.
- pub(crate) libos: CoherentAllocation<LibosMemoryRegionInitArgument>,
+/// Log buffers used by GSP-RM for debug logging.
+struct LogBuffers {
/// Init log buffer.
loginit: LogBuffer,
/// Interrupts log buffer.
logintr: LogBuffer,
/// RM log buffer.
logrm: LogBuffer,
+}
+
+/// GSP runtime data.
+#[pin_data]
+pub(crate) struct Gsp {
+ /// Libos arguments.
+ pub(crate) libos: CoherentAllocation<LibosMemoryRegionInitArgument>,
+ /// Log buffers, optionally exposed via debugfs.
+ #[pin]
+ logs: debugfs::Scope<LogBuffers>,
/// Command queue.
#[pin]
pub(crate) cmdq: Cmdq,
@@ -124,15 +133,17 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> impl PinInit<Self, Error
pin_init::pin_init_scope(move || {
let dev = pdev.as_ref();
+ // Create log buffers before try_pin_init! so they're accessible throughout
+ let loginit = LogBuffer::new(dev)?;
+ let logintr = LogBuffer::new(dev)?;
+ let logrm = LogBuffer::new(dev)?;
+
Ok(try_pin_init!(Self {
libos: CoherentAllocation::<LibosMemoryRegionInitArgument>::alloc_coherent(
dev,
GSP_PAGE_SIZE / size_of::<LibosMemoryRegionInitArgument>(),
GFP_KERNEL | __GFP_ZERO,
)?,
- loginit: LogBuffer::new(dev)?,
- logintr: LogBuffer::new(dev)?,
- logrm: LogBuffer::new(dev)?,
cmdq <- Cmdq::new(dev),
rmargs: CoherentAllocation::<GspArgumentsPadded>::alloc_coherent(
dev,
@@ -153,6 +164,28 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> impl PinInit<Self, Error
dma_write!(rmargs[0].inner = fw::GspArgumentsCached::new(&cmdq))?;
dma_write!(libos[3] = LibosMemoryRegionInitArgument::new("RMARGS", rmargs))?;
},
+ logs <- {
+ let log_buffers = LogBuffers {
+ loginit,
+ logintr,
+ logrm,
+ };
+
+ #[allow(static_mut_refs)]
+ // SAFETY: `DEBUGFS_ROOT` is created before driver registration and cleared
+ // after driver unregistration, so no probe() can race with its modification.
+ // PANIC: `DEBUGFS_ROOT` cannot be `None` here. It is set before driver
+ // registration and cleared after driver unregistration, so it is always
+ // `Some` for the entire lifetime that probe() can be called.
+ let log_parent: &debugfs::Dir = unsafe { crate::DEBUGFS_ROOT.as_ref() }
+ .expect("DEBUGFS_ROOT not initialized");
+
+ log_parent.scope(log_buffers, dev.name(), |logs, dir| {
+ dir.read_binary_file(c_str!("loginit"), &logs.loginit.0);
+ dir.read_binary_file(c_str!("logintr"), &logs.logintr.0);
+ dir.read_binary_file(c_str!("logrm"), &logs.logrm.0);
+ })
+ },
}))
})
}
--
2.51.0
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v9 1/6] rust: device: add device name method
2026-03-16 5:57 ` [PATCH v9 1/6] rust: device: add device name method Timur Tabi
@ 2026-03-16 11:49 ` Gary Guo
0 siblings, 0 replies; 21+ messages in thread
From: Gary Guo @ 2026-03-16 11:49 UTC (permalink / raw)
To: Timur Tabi, Miguel Ojeda, Danilo Krummrich, Alice Ryhl, Gary Guo,
mmaurer, Alexandre Courbot, John Hubbard, Joel Fernandes,
ecourtney, rust-for-linux, nouveau
On Mon Mar 16, 2026 at 5:57 AM GMT, Timur Tabi wrote:
> Add a name() method to the `Device` type, which returns a CStr that
> contains the device name.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
> ---
> rust/helpers/device.c | 5 +++++
> rust/kernel/device.rs | 11 +++++++++++
> 2 files changed, 16 insertions(+)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v9 4/6] gpu: nova-core: Replace module_pci_driver! with explicit module init
2026-03-16 5:57 ` [PATCH v9 4/6] gpu: nova-core: Replace module_pci_driver! with explicit module init Timur Tabi
@ 2026-03-16 16:28 ` Gary Guo
0 siblings, 0 replies; 21+ messages in thread
From: Gary Guo @ 2026-03-16 16:28 UTC (permalink / raw)
To: Timur Tabi, Miguel Ojeda, Danilo Krummrich, Alice Ryhl, Gary Guo,
mmaurer, Alexandre Courbot, John Hubbard, Joel Fernandes,
ecourtney, rust-for-linux, nouveau
On Mon Mar 16, 2026 at 5:57 AM GMT, Timur Tabi wrote:
> Replace the module_pci_driver! macro with an explicit module
> initialization using the standard module! macro and InPlaceModule
> trait implementation. No functional change intended, with the
> exception that the driver now prints a message when loaded.
>
> This change is necessary so that we can create a top-level "nova_core"
> debugfs entry when the driver is loaded.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
> ---
> drivers/gpu/nova-core/nova_core.rs | 25 +++++++++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v9 5/6] gpu: nova-core: create debugfs root in module init
2026-03-16 5:57 ` [PATCH v9 5/6] gpu: nova-core: create debugfs root in " Timur Tabi
@ 2026-03-16 16:28 ` Gary Guo
0 siblings, 0 replies; 21+ messages in thread
From: Gary Guo @ 2026-03-16 16:28 UTC (permalink / raw)
To: Timur Tabi, Miguel Ojeda, Danilo Krummrich, Alice Ryhl, Gary Guo,
mmaurer, Alexandre Courbot, John Hubbard, Joel Fernandes,
ecourtney, rust-for-linux, nouveau
On Mon Mar 16, 2026 at 5:57 AM GMT, Timur Tabi wrote:
> Create the 'nova_core' root debugfs entry when the driver loads.
>
> Normally, non-const global variables need to be protected by a
> mutex. Instead, we use unsafe code, as we know the entry is never
> modified after the driver is loaded. This solves the lifetime
> issue of the mutex guard, which would otherwise have required the
> use of `pin_init_scope`.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
> ---
> drivers/gpu/nova-core/nova_core.rs | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v9 6/6] gpu: nova-core: create GSP-RM logging buffers debugfs entries
2026-03-16 5:57 ` [PATCH v9 6/6] gpu: nova-core: create GSP-RM logging buffers debugfs entries Timur Tabi
@ 2026-03-16 16:29 ` Gary Guo
0 siblings, 0 replies; 21+ messages in thread
From: Gary Guo @ 2026-03-16 16:29 UTC (permalink / raw)
To: Timur Tabi, Miguel Ojeda, Danilo Krummrich, Alice Ryhl, Gary Guo,
mmaurer, Alexandre Courbot, John Hubbard, Joel Fernandes,
ecourtney, rust-for-linux, nouveau
On Mon Mar 16, 2026 at 5:57 AM GMT, Timur Tabi wrote:
> Create read-only debugfs entries for LOGINIT, LOGRM, and LOGINTR, which
> are the three primary printf logging buffers from GSP-RM. LOGPMU will
> be added at a later date, as it requires it support for its RPC message
> first.
>
> This patch uses the `pin_init_scope` feature to create the entries.
> `pin_init_scope` solves the lifetime issue over the `DEBUGFS_ROOT`
> reference by delaying its acquisition until the time the entry is
> actually initialized.
>
> Co-developed-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
> drivers/gpu/nova-core/gsp.rs | 49 ++++++++++++++++++++++++++++++------
> 1 file changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
> index a6f3918c20b1..484e6aa48bf3 100644
> --- a/drivers/gpu/nova-core/gsp.rs
> +++ b/drivers/gpu/nova-core/gsp.rs
> @@ -3,6 +3,8 @@
> mod boot;
>
> use kernel::{
> + c_str,
> + debugfs,
> device,
> dma::{
> CoherentAllocation,
> @@ -100,17 +102,24 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
> }
> }
>
> -/// GSP runtime data.
> -#[pin_data]
> -pub(crate) struct Gsp {
> - /// Libos arguments.
> - pub(crate) libos: CoherentAllocation<LibosMemoryRegionInitArgument>,
> +/// Log buffers used by GSP-RM for debug logging.
> +struct LogBuffers {
> /// Init log buffer.
> loginit: LogBuffer,
> /// Interrupts log buffer.
> logintr: LogBuffer,
> /// RM log buffer.
> logrm: LogBuffer,
> +}
> +
> +/// GSP runtime data.
> +#[pin_data]
> +pub(crate) struct Gsp {
> + /// Libos arguments.
> + pub(crate) libos: CoherentAllocation<LibosMemoryRegionInitArgument>,
> + /// Log buffers, optionally exposed via debugfs.
> + #[pin]
> + logs: debugfs::Scope<LogBuffers>,
> /// Command queue.
> #[pin]
> pub(crate) cmdq: Cmdq,
> @@ -124,15 +133,17 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> impl PinInit<Self, Error
> pin_init::pin_init_scope(move || {
> let dev = pdev.as_ref();
>
> + // Create log buffers before try_pin_init! so they're accessible throughout
> + let loginit = LogBuffer::new(dev)?;
> + let logintr = LogBuffer::new(dev)?;
> + let logrm = LogBuffer::new(dev)?;
> +
> Ok(try_pin_init!(Self {
> libos: CoherentAllocation::<LibosMemoryRegionInitArgument>::alloc_coherent(
> dev,
> GSP_PAGE_SIZE / size_of::<LibosMemoryRegionInitArgument>(),
> GFP_KERNEL | __GFP_ZERO,
> )?,
> - loginit: LogBuffer::new(dev)?,
> - logintr: LogBuffer::new(dev)?,
> - logrm: LogBuffer::new(dev)?,
> cmdq <- Cmdq::new(dev),
> rmargs: CoherentAllocation::<GspArgumentsPadded>::alloc_coherent(
> dev,
> @@ -153,6 +164,28 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> impl PinInit<Self, Error
> dma_write!(rmargs[0].inner = fw::GspArgumentsCached::new(&cmdq))?;
> dma_write!(libos[3] = LibosMemoryRegionInitArgument::new("RMARGS", rmargs))?;
> },
> + logs <- {
> + let log_buffers = LogBuffers {
> + loginit,
> + logintr,
> + logrm,
> + };
> +
> + #[allow(static_mut_refs)]
> + // SAFETY: `DEBUGFS_ROOT` is created before driver registration and cleared
> + // after driver unregistration, so no probe() can race with its modification.
> + // PANIC: `DEBUGFS_ROOT` cannot be `None` here. It is set before driver
> + // registration and cleared after driver unregistration, so it is always
> + // `Some` for the entire lifetime that probe() can be called.
> + let log_parent: &debugfs::Dir = unsafe { crate::DEBUGFS_ROOT.as_ref() }
> + .expect("DEBUGFS_ROOT not initialized");
> +
> + log_parent.scope(log_buffers, dev.name(), |logs, dir| {
> + dir.read_binary_file(c_str!("loginit"), &logs.loginit.0);
This needs to be c"loginit", same below.
Best,
Gary
> + dir.read_binary_file(c_str!("logintr"), &logs.logintr.0);
> + dir.read_binary_file(c_str!("logrm"), &logs.logrm.0);
> + })
> + },
> }))
> })
> }
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v9 2/6] rust: uaccess: add write_dma() for copying from DMA buffers to userspace
2026-03-16 5:57 ` [PATCH v9 2/6] rust: uaccess: add write_dma() for copying from DMA buffers to userspace Timur Tabi
@ 2026-03-16 20:42 ` Alice Ryhl
2026-03-16 21:42 ` Timur Tabi
2026-03-17 21:43 ` Miguel Ojeda
1 sibling, 1 reply; 21+ messages in thread
From: Alice Ryhl @ 2026-03-16 20:42 UTC (permalink / raw)
To: Timur Tabi
Cc: Miguel Ojeda, Danilo Krummrich, Gary Guo, mmaurer,
Alexandre Courbot, John Hubbard, Joel Fernandes, ecourtney,
rust-for-linux, nouveau
On Mon, Mar 16, 2026 at 12:57:32AM -0500, Timur Tabi wrote:
> Add UserSliceWriter::write_dma() to copy data from a CoherentAllocation<u8>
> to userspace. This provides a safe interface for copying DMA buffer
> contents to userspace without requiring callers to work with raw pointers.
>
> Because write_dma() and write_slice() have common code, factor that code
> out into a helper function, write_raw().
>
> The method handles bounds checking and offset calculation internally,
> wrapping the unsafe copy_to_user() call.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> + /// # Safety
> + ///
> + /// The caller must ensure that `ptr` points to a valid slice of `len` bytes (i.e., it is
> + /// valid for reads of `len` bytes).
I don't know how I feel about this 'valid slice' thing. The memory might
be dma memory after all ... just say it's valid for reads of `len`
bytes.
After all, in below SAFETY: comments, you also argue that it's valid for
reads, not that it points at a valid slice.
> + unsafe fn write_raw(&mut self, ptr: *const u8, len: usize) -> Result {
> if len > self.length {
> return Err(EFAULT);
> }
> - // SAFETY: `data_ptr` points into an immutable slice of length `len`, so we may read
> - // that many bytes from it.
> - let res = unsafe { bindings::copy_to_user(self.ptr.as_mut_ptr(), data_ptr, len) };
> + // SAFETY:
> + // - `self.ptr` is a userspace pointer, and `len <= self.length` is checked above to
> + // ensure we don't exceed the caller-specified bounds.
> + // - `ptr` is valid for reading `len` bytes as required by this function's safety contract.
> + // - `copy_to_user` validates the userspace address at runtime and returns non-zero on
> + // failure (e.g., bad address or unmapped memory).
> + let res =
> + unsafe { bindings::copy_to_user(self.ptr.as_mut_ptr(), ptr.cast::<c_void>(), len) };
Points 1 and 3 in this bulleted list do not seem to address any actual
safety requirements of `copy_to_user`. Yes, the function validates
userspace addresses at runtime, and this is part of why its
implementation is safe, but it's not a precondition on the caller.
There's no way to call `copy_to_user` where it would not validate the
userspace address at runtime.
> + /// Writes raw data to this user pointer from a DMA coherent allocation.
> + ///
> + /// # Arguments
> + ///
> + /// * `data` - The DMA coherent allocation to copy from.
> + /// * `offset` - The byte offset into `data` to start copying from.
> + /// * `count` - The number of bytes to copy.
This is not the usual Rust style for documentation. We would generally
just write it in words:
Copies `count` bytes from `alloc` starting from `offset` into
this userspace slice.
Yes, argument lists are *occassionally* used, but it's rare.
> + /// # Errors
> + ///
> + /// Returns [`EOVERFLOW`] if `offset + count` overflows.
> + /// Returns [`ERANGE`] if `offset + count` exceeds the size of `data`, or `count` exceeds
> + /// the size of the user-space buffer.
> + /// Returns [`EFAULT`] if the write happens on a bad address, or if the write goes out of
> + /// bounds of this [`UserSliceWriter`].
Using a list to format the possible errors seems sensible to me. But I
don't think you intended for this to be rendered like this in the final
docs:
Returns `EOVERFLOW` if `offset + count` overflows. Returns
`ERANGE` if `offset + count` exceeds the size of `data`, or
`count` exceeds the size of the user-space buffer. Returns
`EFAULT` if the write happens on a bad address, or if the
write goes out of bounds of this `UserSliceWriter`.
Please use a markdown list to format this, and check the generated html
docs.
Alice
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v9 3/6] rust: dma: implement BinaryWriter for CoherentAllocation<u8>
2026-03-16 5:57 ` [PATCH v9 3/6] rust: dma: implement BinaryWriter for CoherentAllocation<u8> Timur Tabi
@ 2026-03-16 20:46 ` Alice Ryhl
2026-03-16 21:57 ` Gary Guo
2026-03-17 4:22 ` Alexandre Courbot
1 sibling, 1 reply; 21+ messages in thread
From: Alice Ryhl @ 2026-03-16 20:46 UTC (permalink / raw)
To: Timur Tabi
Cc: Miguel Ojeda, Danilo Krummrich, Gary Guo, mmaurer,
Alexandre Courbot, John Hubbard, Joel Fernandes, ecourtney,
rust-for-linux, nouveau
On Mon, Mar 16, 2026 at 12:57:33AM -0500, Timur Tabi wrote:
> Implement the BinaryWriter trait for CoherentAllocation<u8>, enabling
> DMA coherent allocations to be exposed as readable binary files.
> The implementation handles offset tracking and bounds checking, copying
> data from the coherent allocation to userspace via write_dma().
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> + let count = self.count().saturating_sub(offset_val).min(writer.len());
It's kind of weird to call the length method self.count() rather than
self.len(). All other methods called count() in the stdlib are cases
where you get a length by literally counting with a for loop.
Anyway, not a problem of this patch. LGTM.
Alice
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v9 2/6] rust: uaccess: add write_dma() for copying from DMA buffers to userspace
2026-03-16 20:42 ` Alice Ryhl
@ 2026-03-16 21:42 ` Timur Tabi
2026-03-16 21:51 ` Alice Ryhl
0 siblings, 1 reply; 21+ messages in thread
From: Timur Tabi @ 2026-03-16 21:42 UTC (permalink / raw)
To: aliceryhl@google.com
Cc: John Hubbard, gary@garyguo.net, mmaurer@google.com,
rust-for-linux@vger.kernel.org, Eliot Courtney,
nouveau@lists.freedesktop.org, Joel Fernandes, dakr@kernel.org,
ojeda@kernel.org, Alexandre Courbot
On Mon, 2026-03-16 at 20:42 +0000, Alice Ryhl wrote:
> On Mon, Mar 16, 2026 at 12:57:32AM -0500, Timur Tabi wrote:
> > Add UserSliceWriter::write_dma() to copy data from a CoherentAllocation<u8>
> > to userspace. This provides a safe interface for copying DMA buffer
> > contents to userspace without requiring callers to work with raw pointers.
> >
> > Because write_dma() and write_slice() have common code, factor that code
> > out into a helper function, write_raw().
> >
> > The method handles bounds checking and offset calculation internally,
> > wrapping the unsafe copy_to_user() call.
> >
> > Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> > Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
>
> > + /// # Safety
> > + ///
> > + /// The caller must ensure that `ptr` points to a valid slice of `len` bytes (i.e., it is
> > + /// valid for reads of `len` bytes).
>
> I don't know how I feel about this 'valid slice' thing. The memory might
> be dma memory after all ... just say it's valid for reads of `len`
> bytes.
Shouldn't the safety comment also say that `len` is valid for writes to self.ptr, not just reads for
`ptr`? I know there is a "if len > self.length" check. Does that check obviate the need for a
safety comment that references self.ptr?
Maybe I should rename `ptr`, to avoid confusion with self.ptr. Would be okay to rename it to
`from`, since it's copying from that kernel address?
> After all, in below SAFETY: comments, you also argue that it's valid for
> reads, not that it points at a valid slice.
>
> > + unsafe fn write_raw(&mut self, ptr: *const u8, len: usize) -> Result {
> > if len > self.length {
> > return Err(EFAULT);
> > }
> > - // SAFETY: `data_ptr` points into an immutable slice of length `len`, so we may read
> > - // that many bytes from it.
> > - let res = unsafe { bindings::copy_to_user(self.ptr.as_mut_ptr(), data_ptr, len) };
> > + // SAFETY:
> > + // - `self.ptr` is a userspace pointer, and `len <= self.length` is checked above to
> > + // ensure we don't exceed the caller-specified bounds.
> > + // - `ptr` is valid for reading `len` bytes as required by this function's safety
> > contract.
> > + // - `copy_to_user` validates the userspace address at runtime and returns non-zero on
> > + // failure (e.g., bad address or unmapped memory).
> > + let res =
> > + unsafe { bindings::copy_to_user(self.ptr.as_mut_ptr(), ptr.cast::<c_void>(), len)
> > };
>
> Points 1 and 3 in this bulleted list do not seem to address any actual
> safety requirements of `copy_to_user`. Yes, the function validates
> userspace addresses at runtime, and this is part of why its
> implementation is safe, but it's not a precondition on the caller.
> There's no way to call `copy_to_user` where it would not validate the
> userspace address at runtime.
Fair enough, but I'm struggling to come up with a safety comment that doesn't just repeat what's in
the function comment:
/// The caller must ensure that `ptr` is valid for reads of `len` bytes.
(and maybe also valid for writes to self.ptr).
It seems to me that we're just extending the safety conditions copy_to_user() directly to
write_raw(). That is, the reason write_raw() is unsafe is because copy_to_user() is unsafe.
Therefore, shouldn't the safety comment for copy_to_user just be:
// SAFETY: Caller guarantees `ptr` is valid for `len` bytes (see this function's safety contract).
> > + /// Writes raw data to this user pointer from a DMA coherent allocation.
> > + ///
> > + /// # Arguments
> > + ///
> > + /// * `data` - The DMA coherent allocation to copy from.
> > + /// * `offset` - The byte offset into `data` to start copying from.
> > + /// * `count` - The number of bytes to copy.
>
> This is not the usual Rust style for documentation. We would generally
> just write it in words:
>
> Copies `count` bytes from `alloc` starting from `offset` into
> this userspace slice.
>
> Yes, argument lists are *occassionally* used, but it's rare.
Ok.
> > + /// # Errors
> > + ///
> > + /// Returns [`EOVERFLOW`] if `offset + count` overflows.
> > + /// Returns [`ERANGE`] if `offset + count` exceeds the size of `data`, or `count` exceeds
> > + /// the size of the user-space buffer.
> > + /// Returns [`EFAULT`] if the write happens on a bad address, or if the write goes out of
> > + /// bounds of this [`UserSliceWriter`].
>
> Using a list to format the possible errors seems sensible to me. But I
> don't think you intended for this to be rendered like this in the final
> docs:
>
> Returns `EOVERFLOW` if `offset + count` overflows. Returns
> `ERANGE` if `offset + count` exceeds the size of `data`, or
> `count` exceeds the size of the user-space buffer. Returns
> `EFAULT` if the write happens on a bad address, or if the
> write goes out of bounds of this `UserSliceWriter`.
>
> Please use a markdown list to format this, and check the generated html
> docs.
Ok.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v9 2/6] rust: uaccess: add write_dma() for copying from DMA buffers to userspace
2026-03-16 21:42 ` Timur Tabi
@ 2026-03-16 21:51 ` Alice Ryhl
0 siblings, 0 replies; 21+ messages in thread
From: Alice Ryhl @ 2026-03-16 21:51 UTC (permalink / raw)
To: Timur Tabi
Cc: John Hubbard, gary@garyguo.net, mmaurer@google.com,
rust-for-linux@vger.kernel.org, Eliot Courtney,
nouveau@lists.freedesktop.org, Joel Fernandes, dakr@kernel.org,
ojeda@kernel.org, Alexandre Courbot
On Mon, Mar 16, 2026 at 09:42:44PM +0000, Timur Tabi wrote:
> On Mon, 2026-03-16 at 20:42 +0000, Alice Ryhl wrote:
> > On Mon, Mar 16, 2026 at 12:57:32AM -0500, Timur Tabi wrote:
> > > Add UserSliceWriter::write_dma() to copy data from a CoherentAllocation<u8>
> > > to userspace. This provides a safe interface for copying DMA buffer
> > > contents to userspace without requiring callers to work with raw pointers.
> > >
> > > Because write_dma() and write_slice() have common code, factor that code
> > > out into a helper function, write_raw().
> > >
> > > The method handles bounds checking and offset calculation internally,
> > > wrapping the unsafe copy_to_user() call.
> > >
> > > Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> > > Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> >
> > > + /// # Safety
> > > + ///
> > > + /// The caller must ensure that `ptr` points to a valid slice of `len` bytes (i.e., it is
> > > + /// valid for reads of `len` bytes).
> >
> > I don't know how I feel about this 'valid slice' thing. The memory might
> > be dma memory after all ... just say it's valid for reads of `len`
> > bytes.
>
> Shouldn't the safety comment also say that `len` is valid for writes to self.ptr, not just reads for
> `ptr`? I know there is a "if len > self.length" check. Does that check obviate the need for a
> safety comment that references self.ptr?
You do not need the caller to do anything special about the userspace
part of the write. That's just safe because 'copy_to_user' either
succeeds safely or returns an error. Only reading from `ptr` is
dangerous here.
> Maybe I should rename `ptr`, to avoid confusion with self.ptr. Would be okay to rename it to
> `from`, since it's copying from that kernel address?
Yes, that is a good suggestion.
> > After all, in below SAFETY: comments, you also argue that it's valid for
> > reads, not that it points at a valid slice.
> >
> > > + unsafe fn write_raw(&mut self, ptr: *const u8, len: usize) -> Result {
> > > if len > self.length {
> > > return Err(EFAULT);
> > > }
> > > - // SAFETY: `data_ptr` points into an immutable slice of length `len`, so we may read
> > > - // that many bytes from it.
> > > - let res = unsafe { bindings::copy_to_user(self.ptr.as_mut_ptr(), data_ptr, len) };
> > > + // SAFETY:
> > > + // - `self.ptr` is a userspace pointer, and `len <= self.length` is checked above to
> > > + // ensure we don't exceed the caller-specified bounds.
> > > + // - `ptr` is valid for reading `len` bytes as required by this function's safety
> > > contract.
> > > + // - `copy_to_user` validates the userspace address at runtime and returns non-zero on
> > > + // failure (e.g., bad address or unmapped memory).
> > > + let res =
> > > + unsafe { bindings::copy_to_user(self.ptr.as_mut_ptr(), ptr.cast::<c_void>(), len)
> > > };
> >
> > Points 1 and 3 in this bulleted list do not seem to address any actual
> > safety requirements of `copy_to_user`. Yes, the function validates
> > userspace addresses at runtime, and this is part of why its
> > implementation is safe, but it's not a precondition on the caller.
> > There's no way to call `copy_to_user` where it would not validate the
> > userspace address at runtime.
>
> Fair enough, but I'm struggling to come up with a safety comment that doesn't just repeat what's in
> the function comment:
>
> /// The caller must ensure that `ptr` is valid for reads of `len` bytes.
>
> (and maybe also valid for writes to self.ptr).
>
> It seems to me that we're just extending the safety conditions copy_to_user() directly to
> write_raw(). That is, the reason write_raw() is unsafe is because copy_to_user() is unsafe.
> Therefore, shouldn't the safety comment for copy_to_user just be:
>
> // SAFETY: Caller guarantees `ptr` is valid for `len` bytes (see this function's safety contract).
Yes, that's a good safety comment for copy_to_user(). I think most
commonly it is worded like this: "By this methods safety requirements,
the caller guarantees that `ptr` is valid for reading `len` bytes."
Alice
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v9 3/6] rust: dma: implement BinaryWriter for CoherentAllocation<u8>
2026-03-16 20:46 ` Alice Ryhl
@ 2026-03-16 21:57 ` Gary Guo
0 siblings, 0 replies; 21+ messages in thread
From: Gary Guo @ 2026-03-16 21:57 UTC (permalink / raw)
To: Alice Ryhl, Timur Tabi
Cc: Miguel Ojeda, Danilo Krummrich, Gary Guo, mmaurer,
Alexandre Courbot, John Hubbard, Joel Fernandes, ecourtney,
rust-for-linux, nouveau
On Mon Mar 16, 2026 at 8:46 PM GMT, Alice Ryhl wrote:
> On Mon, Mar 16, 2026 at 12:57:33AM -0500, Timur Tabi wrote:
>> Implement the BinaryWriter trait for CoherentAllocation<u8>, enabling
>> DMA coherent allocations to be exposed as readable binary files.
>> The implementation handles offset tracking and bounds checking, copying
>> data from the coherent allocation to userspace via write_dma().
>>
>> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>
>> + let count = self.count().saturating_sub(offset_val).min(writer.len());
>
> It's kind of weird to call the length method self.count() rather than
> self.len(). All other methods called count() in the stdlib are cases
> where you get a length by literally counting with a for loop.
This will be updated in
https://lore.kernel.org/rust-for-linux/20260303162314.94363-3-dakr@kernel.org/.
Best,
Gary
>
> Anyway, not a problem of this patch. LGTM.
>
> Alice
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v9 0/6] gpu: nova-core: expose the logging buffers via debugfs
2026-03-16 5:57 [PATCH v9 0/6] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
` (5 preceding siblings ...)
2026-03-16 5:57 ` [PATCH v9 6/6] gpu: nova-core: create GSP-RM logging buffers debugfs entries Timur Tabi
@ 2026-03-16 22:05 ` John Hubbard
2026-03-17 1:53 ` Eliot Courtney
7 siblings, 0 replies; 21+ messages in thread
From: John Hubbard @ 2026-03-16 22:05 UTC (permalink / raw)
To: Timur Tabi, Miguel Ojeda, Danilo Krummrich, Alice Ryhl, Gary Guo,
mmaurer, Alexandre Courbot, Joel Fernandes, ecourtney,
rust-for-linux, nouveau
On 3/15/26 10:57 PM, Timur Tabi wrote:
> GSP-RM writes its printf message to "logging buffers", which are blocks
> memory allocated by the driver. The messages are encoded, so exposing
> the buffers as debugfs entries allows the buffers to be extracted and
> decoded by a special application.
>
> When the driver loads, a /sys/kernel/debug/nova_core root entry is
> created. To do this, the normal module_pci_driver! macro call is
> replaced with an explicit initialization function, as this allows
> that debugfs entry to be created once for all GPUs.
>
> Then in each GPU's initialization, a subdirectory based on the PCI
> BDF name is created, and the logging buffer entries are created under
> that.
>
> This series depends on Eliot Courtney's Cmdq patch set [1].
>
> Note: the debugfs entry has a file size of 0, because debugfs defaults
> a 0 size and the Rust abstractions do not adjust it for the same of
> the object. Nouveau makes this adjustment manually in the driver.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> Tested-by: John Hubbard <jhubbard@nvidia.com>
Thanks for adding the tag, but b4(1) won't pick it up unless it
sees another reply to this specific series, or if there are per-patch
tags applied.
With that in mind, and because it doesn't hurt to have more testing
done, I've applied this and re-tested on Ampere, and it passed, so:
For the series:
Tested-by: John Hubbard <jhubbard@nvidia.com>
thanks,
--
John Hubbard
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v9 0/6] gpu: nova-core: expose the logging buffers via debugfs
2026-03-16 5:57 [PATCH v9 0/6] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
` (6 preceding siblings ...)
2026-03-16 22:05 ` [PATCH v9 0/6] gpu: nova-core: expose the logging buffers via debugfs John Hubbard
@ 2026-03-17 1:53 ` Eliot Courtney
7 siblings, 0 replies; 21+ messages in thread
From: Eliot Courtney @ 2026-03-17 1:53 UTC (permalink / raw)
To: Timur Tabi, Miguel Ojeda, Danilo Krummrich, Alice Ryhl, Gary Guo,
mmaurer, Alexandre Courbot, John Hubbard, Joel Fernandes,
ecourtney, rust-for-linux, nouveau
On Mon Mar 16, 2026 at 2:57 PM JST, Timur Tabi wrote:
> GSP-RM writes its printf message to "logging buffers", which are blocks
> memory allocated by the driver. The messages are encoded, so exposing
> the buffers as debugfs entries allows the buffers to be extracted and
> decoded by a special application.
>
> When the driver loads, a /sys/kernel/debug/nova_core root entry is
> created. To do this, the normal module_pci_driver! macro call is
> replaced with an explicit initialization function, as this allows
> that debugfs entry to be created once for all GPUs.
>
> Then in each GPU's initialization, a subdirectory based on the PCI
> BDF name is created, and the logging buffer entries are created under
> that.
>
> This series depends on Eliot Courtney's Cmdq patch set [1].
>
> Note: the debugfs entry has a file size of 0, because debugfs defaults
> a 0 size and the Rust abstractions do not adjust it for the same of
> the object. Nouveau makes this adjustment manually in the driver.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> Tested-by: John Hubbard <jhubbard@nvidia.com>
>
> [1] https://lore.kernel.org/all/20260310-cmdq-locking-v4-0-4e5c4753c408@nvidia.com/
>
> Changes since v8:
> 1. Added #inline to Device::name()
> 2. Fixed rustfmt issues again
> 3. Updated write_to_slice()
> 4. Dropped "use pin projection in method boot()"
> 5. Now depends on Eliot's Cmdq patch set
> 6. Misc nits fixed
>
> Timur Tabi (6):
> rust: device: add device name method
> rust: uaccess: add write_dma() for copying from DMA buffers to
> userspace
> rust: dma: implement BinaryWriter for CoherentAllocation<u8>
> gpu: nova-core: Replace module_pci_driver! with explicit module init
> gpu: nova-core: create debugfs root in module init
> gpu: nova-core: create GSP-RM logging buffers debugfs entries
>
> drivers/gpu/nova-core/gsp.rs | 49 ++++++++++++++---
> drivers/gpu/nova-core/nova_core.rs | 50 +++++++++++++++++-
> rust/helpers/device.c | 5 ++
> rust/kernel/device.rs | 11 ++++
> rust/kernel/dma.rs | 35 ++++++++++++-
> rust/kernel/uaccess.rs | 84 ++++++++++++++++++++++++++----
> 6 files changed, 213 insertions(+), 21 deletions(-)
>
>
> base-commit: a544873ce0575b2fd8285a1364d3e09929d9a3ba
> prerequisite-patch-id: fefd403caf8af386276351dd12397dda8ae8553f
> prerequisite-patch-id: 3e02192944c4dde97e6895a28371479aa49ddc96
> prerequisite-patch-id: de85fb30b65233210512b8295d7fe811f485f03f
> prerequisite-patch-id: 4232928e15093c74aa6d2a6519a4d7ac5e109607
> prerequisite-patch-id: d4f34f7eda8f5dcad2464f1bf3789e78556eb51c
Tested-by: Eliot Courtney <ecourtney@nvidia.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v9 3/6] rust: dma: implement BinaryWriter for CoherentAllocation<u8>
2026-03-16 5:57 ` [PATCH v9 3/6] rust: dma: implement BinaryWriter for CoherentAllocation<u8> Timur Tabi
2026-03-16 20:46 ` Alice Ryhl
@ 2026-03-17 4:22 ` Alexandre Courbot
1 sibling, 0 replies; 21+ messages in thread
From: Alexandre Courbot @ 2026-03-17 4:22 UTC (permalink / raw)
To: Timur Tabi
Cc: Miguel Ojeda, Danilo Krummrich, Alice Ryhl, Gary Guo, mmaurer,
John Hubbard, Joel Fernandes, ecourtney, rust-for-linux, nouveau
On Mon Mar 16, 2026 at 2:57 PM JST, Timur Tabi wrote:
> Implement the BinaryWriter trait for CoherentAllocation<u8>, enabling
> DMA coherent allocations to be exposed as readable binary files.
> The implementation handles offset tracking and bounds checking, copying
> data from the coherent allocation to userspace via write_dma().
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v9 2/6] rust: uaccess: add write_dma() for copying from DMA buffers to userspace
2026-03-16 5:57 ` [PATCH v9 2/6] rust: uaccess: add write_dma() for copying from DMA buffers to userspace Timur Tabi
2026-03-16 20:42 ` Alice Ryhl
@ 2026-03-17 21:43 ` Miguel Ojeda
2026-03-17 23:02 ` Timur Tabi
1 sibling, 1 reply; 21+ messages in thread
From: Miguel Ojeda @ 2026-03-17 21:43 UTC (permalink / raw)
To: Timur Tabi
Cc: Miguel Ojeda, Danilo Krummrich, Alice Ryhl, Gary Guo, mmaurer,
Alexandre Courbot, John Hubbard, Joel Fernandes, ecourtney,
rust-for-linux, nouveau
On Mon, Mar 16, 2026 at 6:58 AM Timur Tabi <ttabi@nvidia.com> wrote:
>
> Add UserSliceWriter::write_dma() to copy data from a CoherentAllocation<u8>
> to userspace. This provides a safe interface for copying DMA buffer
> contents to userspace without requiring callers to work with raw pointers.
>
> Because write_dma() and write_slice() have common code, factor that code
> out into a helper function, write_raw().
>
> The method handles bounds checking and offset calculation internally,
> wrapping the unsafe copy_to_user() call.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Since I think Danilo wants to pick this one up, with the documentation fixed:
Acked-by: Miguel Ojeda <ojeda@kernel.org>
By the way, there is a typo in both the arguments and errors -- there
is no `data` parameter (which I guess came from the other methods).
Since I imagine you will send a new version, please consider adding an
example, even if it doesn't run (like the others). If it did run, then
I would suggest exercising the error paths.
Thanks!
Cheers,
Miguel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v9 2/6] rust: uaccess: add write_dma() for copying from DMA buffers to userspace
2026-03-17 21:43 ` Miguel Ojeda
@ 2026-03-17 23:02 ` Timur Tabi
0 siblings, 0 replies; 21+ messages in thread
From: Timur Tabi @ 2026-03-17 23:02 UTC (permalink / raw)
To: miguel.ojeda.sandonis@gmail.com
Cc: John Hubbard, gary@garyguo.net, mmaurer@google.com,
rust-for-linux@vger.kernel.org, Eliot Courtney,
nouveau@lists.freedesktop.org, dakr@kernel.org, Joel Fernandes,
aliceryhl@google.com, ojeda@kernel.org, Alexandre Courbot
On Tue, 2026-03-17 at 22:43 +0100, Miguel Ojeda wrote:
> By the way, there is a typo in both the arguments and errors -- there
> is no `data` parameter (which I guess came from the other methods).
Ah yes, thanks. That should be 'alloc' not 'data'.
> Since I imagine you will send a new version, please consider adding an
> example, even if it doesn't run (like the others). If it did run, then
> I would suggest exercising the error paths.
Ok, I added a rudimentary example.
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2026-03-17 23:02 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-16 5:57 [PATCH v9 0/6] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
2026-03-16 5:57 ` [PATCH v9 1/6] rust: device: add device name method Timur Tabi
2026-03-16 11:49 ` Gary Guo
2026-03-16 5:57 ` [PATCH v9 2/6] rust: uaccess: add write_dma() for copying from DMA buffers to userspace Timur Tabi
2026-03-16 20:42 ` Alice Ryhl
2026-03-16 21:42 ` Timur Tabi
2026-03-16 21:51 ` Alice Ryhl
2026-03-17 21:43 ` Miguel Ojeda
2026-03-17 23:02 ` Timur Tabi
2026-03-16 5:57 ` [PATCH v9 3/6] rust: dma: implement BinaryWriter for CoherentAllocation<u8> Timur Tabi
2026-03-16 20:46 ` Alice Ryhl
2026-03-16 21:57 ` Gary Guo
2026-03-17 4:22 ` Alexandre Courbot
2026-03-16 5:57 ` [PATCH v9 4/6] gpu: nova-core: Replace module_pci_driver! with explicit module init Timur Tabi
2026-03-16 16:28 ` Gary Guo
2026-03-16 5:57 ` [PATCH v9 5/6] gpu: nova-core: create debugfs root in " Timur Tabi
2026-03-16 16:28 ` Gary Guo
2026-03-16 5:57 ` [PATCH v9 6/6] gpu: nova-core: create GSP-RM logging buffers debugfs entries Timur Tabi
2026-03-16 16:29 ` Gary Guo
2026-03-16 22:05 ` [PATCH v9 0/6] gpu: nova-core: expose the logging buffers via debugfs John Hubbard
2026-03-17 1:53 ` Eliot Courtney
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox