* [PATCH v8 1/7] rust: device: add device name method
2026-03-10 21:59 [PATCH v8 0/7] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
@ 2026-03-10 21:59 ` Timur Tabi
2026-03-10 22:05 ` Alice Ryhl
2026-03-13 2:10 ` Alexandre Courbot
2026-03-10 21:59 ` [PATCH v8 2/7] rust: uaccess: add write_dma() for copying from DMA buffers to userspace Timur Tabi
` (7 subsequent siblings)
8 siblings, 2 replies; 21+ messages in thread
From: Timur Tabi @ 2026-03-10 21:59 UTC (permalink / raw)
To: Gary Guo, Alice Ryhl, mmaurer, Danilo Krummrich,
Alexandre Courbot, John Hubbard, Joel Fernandes, 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>
---
rust/helpers/device.c | 5 +++++
rust/kernel/device.rs | 10 ++++++++++
2 files changed, 15 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..6be93425fa0b 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -489,6 +489,16 @@ 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.
+ 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.53.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v8 1/7] rust: device: add device name method
2026-03-10 21:59 ` [PATCH v8 1/7] rust: device: add device name method Timur Tabi
@ 2026-03-10 22:05 ` Alice Ryhl
2026-03-13 2:10 ` Alexandre Courbot
1 sibling, 0 replies; 21+ messages in thread
From: Alice Ryhl @ 2026-03-10 22:05 UTC (permalink / raw)
To: Timur Tabi
Cc: Gary Guo, mmaurer, Danilo Krummrich, Alexandre Courbot,
John Hubbard, Joel Fernandes, rust-for-linux, nouveau
On Tue, Mar 10, 2026 at 11:00 PM Timur Tabi <ttabi@nvidia.com> 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>
with below nits fixed:
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> ---
> rust/helpers/device.c | 5 +++++
> rust/kernel/device.rs | 10 ++++++++++
> 2 files changed, 15 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..6be93425fa0b 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -489,6 +489,16 @@ 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.
> + pub fn name(&self) -> &CStr {
Missing #[inline]
We want 'strlen' to be inlined into the caller so that it can be optimized.
Alice
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v8 1/7] rust: device: add device name method
2026-03-10 21:59 ` [PATCH v8 1/7] rust: device: add device name method Timur Tabi
2026-03-10 22:05 ` Alice Ryhl
@ 2026-03-13 2:10 ` Alexandre Courbot
1 sibling, 0 replies; 21+ messages in thread
From: Alexandre Courbot @ 2026-03-13 2:10 UTC (permalink / raw)
To: Timur Tabi
Cc: Gary Guo, Alice Ryhl, mmaurer, Danilo Krummrich, John Hubbard,
Joel Fernandes, rust-for-linux, nouveau
On Wed Mar 11, 2026 at 6:59 AM JST, 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: Alexandre Courbot <acourbot@nvidia.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v8 2/7] rust: uaccess: add write_dma() for copying from DMA buffers to userspace
2026-03-10 21:59 [PATCH v8 0/7] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
2026-03-10 21:59 ` [PATCH v8 1/7] rust: device: add device name method Timur Tabi
@ 2026-03-10 21:59 ` Timur Tabi
2026-03-11 5:48 ` kernel test robot
2026-03-13 2:11 ` Alexandre Courbot
2026-03-10 21:59 ` [PATCH v8 3/7] rust: dma: implement BinaryWriter for CoherentAllocation<u8> Timur Tabi
` (6 subsequent siblings)
8 siblings, 2 replies; 21+ messages in thread
From: Timur Tabi @ 2026-03-10 21:59 UTC (permalink / raw)
To: Gary Guo, Alice Ryhl, mmaurer, Danilo Krummrich,
Alexandre Courbot, John Hubbard, Joel Fernandes, 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>
---
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..3f569acc3718 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,25 @@ 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 and is properly aligned).
+ 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 +487,64 @@ 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.53.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v8 2/7] rust: uaccess: add write_dma() for copying from DMA buffers to userspace
2026-03-10 21:59 ` [PATCH v8 2/7] rust: uaccess: add write_dma() for copying from DMA buffers to userspace Timur Tabi
@ 2026-03-11 5:48 ` kernel test robot
2026-03-13 2:11 ` Alexandre Courbot
1 sibling, 0 replies; 21+ messages in thread
From: kernel test robot @ 2026-03-11 5:48 UTC (permalink / raw)
To: Timur Tabi, Gary Guo, Alice Ryhl, mmaurer, Danilo Krummrich,
Alexandre Courbot, John Hubbard, Joel Fernandes, rust-for-linux,
nouveau
Cc: oe-kbuild-all
Hi Timur,
kernel test robot noticed the following build errors:
[auto build test ERROR on a544873ce0575b2fd8285a1364d3e09929d9a3ba]
url: https://github.com/intel-lab-lkp/linux/commits/Timur-Tabi/rust-device-add-device-name-method/20260311-060450
base: a544873ce0575b2fd8285a1364d3e09929d9a3ba
patch link: https://lore.kernel.org/r/20260310220000.1897166-3-ttabi%40nvidia.com
patch subject: [PATCH v8 2/7] rust: uaccess: add write_dma() for copying from DMA buffers to userspace
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20260311/202603110649.PeH4YNQx-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260311/202603110649.PeH4YNQx-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603110649.PeH4YNQx-lkp@intel.com/
All errors (new ones prefixed by >>):
PATH=/opt/cross/clang-20/bin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
INFO PATH=/opt/cross/rustc-1.88.0-bindgen-0.72.1/cargo/bin:/opt/cross/clang-20/bin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
/usr/bin/timeout -k 100 12h /usr/bin/make KCFLAGS=\ -fno-crash-diagnostics\ -Wno-error=return-type\ -Wreturn-type\ -funsigned-char\ -Wundef\ -falign-functions=64 W=1 --keep-going LLVM=1 -j32 -C source O=/kbuild/obj/consumer/x86_64-rhel-9.4-rust ARCH=x86_64 SHELL=/bin/bash rustfmtcheck
make: Entering directory '/kbuild/src/consumer'
make[1]: Entering directory '/kbuild/obj/consumer/x86_64-rhel-9.4-rust'
>> Diff in rust/kernel/uaccess.rs:476:
// - `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)
- };
+ let res =
+ unsafe { bindings::copy_to_user(self.ptr.as_mut_ptr(), ptr.cast::<c_void>(), len) };
if res != 0 {
return Err(EFAULT);
}
>> Diff in rust/kernel/uaccess.rs:476:
// - `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)
- };
+ let res =
+ unsafe { bindings::copy_to_user(self.ptr.as_mut_ptr(), ptr.cast::<c_void>(), len) };
if res != 0 {
return Err(EFAULT);
}
make[2]: *** [Makefile:1912: rustfmt] Error 123
make[2]: Target 'rustfmtcheck' not remade because of errors.
make[1]: Leaving directory '/kbuild/obj/consumer/x86_64-rhel-9.4-rust'
make[1]: *** [Makefile:248: __sub-make] Error 2
make[1]: Target 'rustfmtcheck' not remade because of errors.
make: *** [Makefile:248: __sub-make] Error 2
make: Target 'rustfmtcheck' not remade because of errors.
make: Leaving directory '/kbuild/src/consumer'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v8 2/7] rust: uaccess: add write_dma() for copying from DMA buffers to userspace
2026-03-10 21:59 ` [PATCH v8 2/7] rust: uaccess: add write_dma() for copying from DMA buffers to userspace Timur Tabi
2026-03-11 5:48 ` kernel test robot
@ 2026-03-13 2:11 ` Alexandre Courbot
1 sibling, 0 replies; 21+ messages in thread
From: Alexandre Courbot @ 2026-03-13 2:11 UTC (permalink / raw)
To: Timur Tabi
Cc: Gary Guo, Alice Ryhl, mmaurer, Danilo Krummrich, John Hubbard,
Joel Fernandes, rust-for-linux, nouveau
On Wed Mar 11, 2026 at 6:59 AM JST, 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>
> ---
> 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..3f569acc3718 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,25 @@ 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 and is properly aligned).
Bytes arrays are supposed to be byte-aligned, so I am not sure the
"properly aligned" adds something (it's also not technically incorrect
so fine to keep it).
> + 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 +487,64 @@ 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
Nit: missing empty line.
Other thank that (and the test robot warnings),
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v8 3/7] rust: dma: implement BinaryWriter for CoherentAllocation<u8>
2026-03-10 21:59 [PATCH v8 0/7] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
2026-03-10 21:59 ` [PATCH v8 1/7] rust: device: add device name method Timur Tabi
2026-03-10 21:59 ` [PATCH v8 2/7] rust: uaccess: add write_dma() for copying from DMA buffers to userspace Timur Tabi
@ 2026-03-10 21:59 ` Timur Tabi
2026-03-13 2:11 ` Alexandre Courbot
2026-03-10 21:59 ` [PATCH v8 4/7] gpu: nova-core: Replace module_pci_driver! with explicit module init Timur Tabi
` (5 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Timur Tabi @ 2026-03-10 21:59 UTC (permalink / raw)
To: Gary Guo, Alice Ryhl, mmaurer, Danilo Krummrich,
Alexandre Courbot, John Hubbard, Joel Fernandes, 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 | 36 +++++++++++++++++++++++++++++++++++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 909d56fd5118..b1cc39756dd8 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,38 @@ 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);
+ }
+
+ let offset_val: usize = (*offset).try_into().map_err(|_| EINVAL)?;
+ let len = self.count();
+
+ if offset_val >= len {
+ return Ok(0);
+ }
+
+ let count = (len - 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.53.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v8 3/7] rust: dma: implement BinaryWriter for CoherentAllocation<u8>
2026-03-10 21:59 ` [PATCH v8 3/7] rust: dma: implement BinaryWriter for CoherentAllocation<u8> Timur Tabi
@ 2026-03-13 2:11 ` Alexandre Courbot
2026-03-14 2:05 ` Timur Tabi
0 siblings, 1 reply; 21+ messages in thread
From: Alexandre Courbot @ 2026-03-13 2:11 UTC (permalink / raw)
To: Timur Tabi
Cc: Gary Guo, Alice Ryhl, mmaurer, Danilo Krummrich, John Hubbard,
Joel Fernandes, rust-for-linux, nouveau
On Wed Mar 11, 2026 at 6:59 AM 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>
> ---
> rust/kernel/dma.rs | 36 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index 909d56fd5118..b1cc39756dd8 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,38 @@ 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);
> + }
Won't the `try_into` right below also reject negative values?
> +
> + let offset_val: usize = (*offset).try_into().map_err(|_| EINVAL)?;
> + let len = self.count();
> +
> + if offset_val >= len {
> + return Ok(0);
> + }
> +
> + let count = (len - offset_val).min(writer.len());
We can simplify a bit:
let count = self.count().saturating_sub(offset_val).min(writer.len());
That way you can get rid of the `offset_val >= len`` test and the rest
of the code will work with `len == 0`, which will have the desired
effect.
Regardless,
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v8 3/7] rust: dma: implement BinaryWriter for CoherentAllocation<u8>
2026-03-13 2:11 ` Alexandre Courbot
@ 2026-03-14 2:05 ` Timur Tabi
2026-03-15 5:11 ` Alexandre Courbot
0 siblings, 1 reply; 21+ messages in thread
From: Timur Tabi @ 2026-03-14 2:05 UTC (permalink / raw)
To: Alexandre Courbot
Cc: John Hubbard, gary@garyguo.net, mmaurer@google.com,
rust-for-linux@vger.kernel.org, nouveau@lists.freedesktop.org,
dakr@kernel.org, Joel Fernandes, aliceryhl@google.com
On Fri, 2026-03-13 at 11:11 +0900, Alexandre Courbot wrote:
> > + if offset.is_negative() {
> > + return Err(EINVAL);
> > + }
>
> Won't the `try_into` right below also reject negative values?
>
> > +
> > + let offset_val: usize = (*offset).try_into().map_err(|_| EINVAL)?;
Well, yes, but there's actually a bigger problem, now that you point this out. If usize is 32
bits, the try_into() could fail if *offset is too large. In that case, it should return Ok(0)
instead of EINVAL.
So actually, we need to keep the is_negative() check, but should modify the try_into() to return
Ok(0) instead of EINVAL.
let Ok(offset_val) = usize::try_from(*offset) else {
return Ok(0); // offset too large for usize = past EOF
};
Are we going to assume CONFIG_64BIT forever, and just ignore any possible errors if usize is 32
bits?
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v8 3/7] rust: dma: implement BinaryWriter for CoherentAllocation<u8>
2026-03-14 2:05 ` Timur Tabi
@ 2026-03-15 5:11 ` Alexandre Courbot
2026-03-15 18:57 ` Timur Tabi
0 siblings, 1 reply; 21+ messages in thread
From: Alexandre Courbot @ 2026-03-15 5:11 UTC (permalink / raw)
To: Timur Tabi
Cc: John Hubbard, gary@garyguo.net, mmaurer@google.com,
rust-for-linux@vger.kernel.org, nouveau@lists.freedesktop.org,
dakr@kernel.org, Joel Fernandes, aliceryhl@google.com
On Sat Mar 14, 2026 at 11:05 AM JST, Timur Tabi wrote:
> On Fri, 2026-03-13 at 11:11 +0900, Alexandre Courbot wrote:
>> > + if offset.is_negative() {
>> > + return Err(EINVAL);
>> > + }
>>
>> Won't the `try_into` right below also reject negative values?
>>
>> > +
>> > + let offset_val: usize = (*offset).try_into().map_err(|_| EINVAL)?;
>
> Well, yes, but there's actually a bigger problem, now that you point this out. If usize is 32
> bits, the try_into() could fail if *offset is too large. In that case, it should return Ok(0)
> instead of EINVAL.
>
> So actually, we need to keep the is_negative() check, but should modify the try_into() to return
> Ok(0) instead of EINVAL.
>
> let Ok(offset_val) = usize::try_from(*offset) else {
> return Ok(0); // offset too large for usize = past EOF
> };
>
> Are we going to assume CONFIG_64BIT forever, and just ignore any possible errors if usize is 32
> bits?
This is not Nova, so we cannot assume CONFIG_64BIT. I guess we have to
do as you described above.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v8 3/7] rust: dma: implement BinaryWriter for CoherentAllocation<u8>
2026-03-15 5:11 ` Alexandre Courbot
@ 2026-03-15 18:57 ` Timur Tabi
2026-03-16 3:44 ` Alexandre Courbot
0 siblings, 1 reply; 21+ messages in thread
From: Timur Tabi @ 2026-03-15 18:57 UTC (permalink / raw)
To: Alexandre Courbot
Cc: John Hubbard, gary@garyguo.net, mmaurer@google.com,
rust-for-linux@vger.kernel.org, nouveau@lists.freedesktop.org,
dakr@kernel.org, Joel Fernandes, aliceryhl@google.com
On Sun, 2026-03-15 at 14:11 +0900, Alexandre Courbot wrote:
> > Well, yes, but there's actually a bigger problem, now that you point this out. If usize is
> > 32
> > bits, the try_into() could fail if *offset is too large. In that case, it should return
> > Ok(0)
> > instead of EINVAL.
> >
> > So actually, we need to keep the is_negative() check, but should modify the try_into() to
> > return
> > Ok(0) instead of EINVAL.
> >
> > let Ok(offset_val) = usize::try_from(*offset) else {
> > return Ok(0); // offset too large for usize = past EOF
> > };
> >
> > Are we going to assume CONFIG_64BIT forever, and just ignore any possible errors if usize is
> > 32
> > bits?
>
> This is not Nova, so we cannot assume CONFIG_64BIT. I guess we have to
> do as you described above.
Yes, you're right. I forgot that this was common code.
Still, I wonder. It seems incongruous to me that, on a 32-bit platform, offsets (file::Offset)
are 64-bit but sizes (usize) are only 32-bit.
I'll post a v9 later today with my above changes.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v8 3/7] rust: dma: implement BinaryWriter for CoherentAllocation<u8>
2026-03-15 18:57 ` Timur Tabi
@ 2026-03-16 3:44 ` Alexandre Courbot
0 siblings, 0 replies; 21+ messages in thread
From: Alexandre Courbot @ 2026-03-16 3:44 UTC (permalink / raw)
To: Timur Tabi
Cc: John Hubbard, gary@garyguo.net, mmaurer@google.com,
rust-for-linux@vger.kernel.org, nouveau@lists.freedesktop.org,
dakr@kernel.org, Joel Fernandes, aliceryhl@google.com
On Mon Mar 16, 2026 at 3:57 AM JST, Timur Tabi wrote:
> On Sun, 2026-03-15 at 14:11 +0900, Alexandre Courbot wrote:
>> > Well, yes, but there's actually a bigger problem, now that you point this out. If usize is
>> > 32
>> > bits, the try_into() could fail if *offset is too large. In that case, it should return
>> > Ok(0)
>> > instead of EINVAL.
>> >
>> > So actually, we need to keep the is_negative() check, but should modify the try_into() to
>> > return
>> > Ok(0) instead of EINVAL.
>> >
>> > let Ok(offset_val) = usize::try_from(*offset) else {
>> > return Ok(0); // offset too large for usize = past EOF
>> > };
>> >
>> > Are we going to assume CONFIG_64BIT forever, and just ignore any possible errors if usize is
>> > 32
>> > bits?
>>
>> This is not Nova, so we cannot assume CONFIG_64BIT. I guess we have to
>> do as you described above.
>
> Yes, you're right. I forgot that this was common code.
>
> Still, I wonder. It seems incongruous to me that, on a 32-bit platform, offsets (file::Offset)
> are 64-bit but sizes (usize) are only 32-bit.
`file::Offset` maps to the C's `loff_t` which is a version of `off_t`
that is guaranteed to be 64-bit. The kernel core APIs (like
`file_operations` seem to all use `loff_t`, so it makes sense for Rust
to follow suit.
I suppose the kernel uses `loff_t` everywhere where files are involved
because without that it wouldn't be able to manage files larger than
2GB. With that in mind, the extra check when converting to a 32-bit
pointer makes more sense.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v8 4/7] gpu: nova-core: Replace module_pci_driver! with explicit module init
2026-03-10 21:59 [PATCH v8 0/7] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
` (2 preceding siblings ...)
2026-03-10 21:59 ` [PATCH v8 3/7] rust: dma: implement BinaryWriter for CoherentAllocation<u8> Timur Tabi
@ 2026-03-10 21:59 ` Timur Tabi
2026-03-10 21:59 ` [PATCH v8 5/7] gpu: nova-core: use pin projection in method boot() Timur Tabi
` (4 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Timur Tabi @ 2026-03-10 21:59 UTC (permalink / raw)
To: Gary Guo, Alice Ryhl, mmaurer, Danilo Krummrich,
Alexandre Courbot, John Hubbard, Joel Fernandes, 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.53.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v8 5/7] gpu: nova-core: use pin projection in method boot()
2026-03-10 21:59 [PATCH v8 0/7] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
` (3 preceding siblings ...)
2026-03-10 21:59 ` [PATCH v8 4/7] gpu: nova-core: Replace module_pci_driver! with explicit module init Timur Tabi
@ 2026-03-10 21:59 ` Timur Tabi
2026-03-13 2:13 ` Alexandre Courbot
2026-03-10 21:59 ` [PATCH v8 6/7] gpu: nova-core: create debugfs root in module init Timur Tabi
` (3 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Timur Tabi @ 2026-03-10 21:59 UTC (permalink / raw)
To: Gary Guo, Alice Ryhl, mmaurer, Danilo Krummrich,
Alexandre Courbot, John Hubbard, Joel Fernandes, rust-for-linux,
nouveau
Struct Gsp in gsp.rs is tagged with #[pin_data], which allows any of its
fields to be pinned (i.e. with #[pin]). When #[pin] is added to any
field in a #[pin_data] struct, fields can no longer be directly accessed
via normal field access. Instead, pin projection must be used to access
those fields.
Currently, no fields are pinned, but that will change. The boot() method
receives self: Pin<&mut Self>. When struct Gsp contains any pinned
fields, direct field access like self.cmdq is not allowed through
Pin<&mut Self>, as Pin prevents obtaining &mut Self to protect pinned
data from being moved.
Use pin projection via self.as_mut().project() to access struct fields.
The project() method, generated by #[pin_data], returns a projection
struct providing &mut references to non-pinned fields, enabling mutable
access while preserving pin invariants.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
drivers/gpu/nova-core/gsp/boot.rs | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index d278ce620c24..425ec8bfd17e 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -168,12 +168,14 @@ pub(crate) fn boot(
CoherentAllocation::<GspFwWprMeta>::alloc_coherent(dev, 1, GFP_KERNEL | __GFP_ZERO)?;
dma_write!(wpr_meta[0] = GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
- self.cmdq
+ let this = self.as_mut().project();
+
+ this.cmdq
.send_command(bar, commands::SetSystemInfo::new(pdev))?;
- self.cmdq.send_command(bar, commands::SetRegistry::new())?;
+ this.cmdq.send_command(bar, commands::SetRegistry::new())?;
gsp_falcon.reset(bar)?;
- let libos_handle = self.libos.dma_handle();
+ let libos_handle = this.libos.dma_handle();
let (mbox0, mbox1) = gsp_falcon.boot(
bar,
Some(libos_handle as u32),
@@ -222,13 +224,13 @@ pub(crate) fn boot(
dev: pdev.as_ref().into(),
bar,
};
- GspSequencer::run(&mut self.cmdq, seq_params)?;
+ GspSequencer::run(this.cmdq, seq_params)?;
// Wait until GSP is fully initialized.
- commands::wait_gsp_init_done(&mut self.cmdq)?;
+ commands::wait_gsp_init_done(this.cmdq)?;
// Obtain and display basic GPU information.
- let info = commands::get_gsp_info(&mut self.cmdq, bar)?;
+ let info = commands::get_gsp_info(this.cmdq, bar)?;
match info.gpu_name() {
Ok(name) => dev_info!(pdev, "GPU name: {}\n", name),
Err(e) => dev_warn!(pdev, "GPU name unavailable: {:?}\n", e),
--
2.53.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v8 5/7] gpu: nova-core: use pin projection in method boot()
2026-03-10 21:59 ` [PATCH v8 5/7] gpu: nova-core: use pin projection in method boot() Timur Tabi
@ 2026-03-13 2:13 ` Alexandre Courbot
2026-03-14 2:20 ` Timur Tabi
0 siblings, 1 reply; 21+ messages in thread
From: Alexandre Courbot @ 2026-03-13 2:13 UTC (permalink / raw)
To: Timur Tabi
Cc: Gary Guo, Alice Ryhl, mmaurer, Danilo Krummrich, John Hubbard,
Joel Fernandes, rust-for-linux, nouveau
On Wed Mar 11, 2026 at 6:59 AM JST, Timur Tabi wrote:
> Struct Gsp in gsp.rs is tagged with #[pin_data], which allows any of its
> fields to be pinned (i.e. with #[pin]). When #[pin] is added to any
> field in a #[pin_data] struct, fields can no longer be directly accessed
> via normal field access. Instead, pin projection must be used to access
> those fields.
>
> Currently, no fields are pinned, but that will change. The boot() method
> receives self: Pin<&mut Self>. When struct Gsp contains any pinned
> fields, direct field access like self.cmdq is not allowed through
> Pin<&mut Self>, as Pin prevents obtaining &mut Self to protect pinned
> data from being moved.
>
> Use pin projection via self.as_mut().project() to access struct fields.
> The project() method, generated by #[pin_data], returns a projection
> struct providing &mut references to non-pinned fields, enabling mutable
> access while preserving pin invariants.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
This is going to clash with the command queue locking series [1], which
we will want to merge first. Can you rebase on top of it? I suspect you
might even just need to drop this patch and things will work.
[1] https://lore.kernel.org/all/20260310-cmdq-locking-v4-0-4e5c4753c408@nvidia.com/
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v8 5/7] gpu: nova-core: use pin projection in method boot()
2026-03-13 2:13 ` Alexandre Courbot
@ 2026-03-14 2:20 ` Timur Tabi
0 siblings, 0 replies; 21+ messages in thread
From: Timur Tabi @ 2026-03-14 2:20 UTC (permalink / raw)
To: Alexandre Courbot
Cc: John Hubbard, gary@garyguo.net, mmaurer@google.com,
rust-for-linux@vger.kernel.org, nouveau@lists.freedesktop.org,
dakr@kernel.org, Joel Fernandes, aliceryhl@google.com
On Fri, 2026-03-13 at 11:13 +0900, Alexandre Courbot wrote:
> This is going to clash with the command queue locking series [1], which
> we will want to merge first. Can you rebase on top of it? I suspect you
> might even just need to drop this patch and things will work.
>
> [1] https://lore.kernel.org/all/20260310-cmdq-locking-v4-0-4e5c4753c408@nvidia.com/
Yes, Eliot's set does allow me to drop this patch.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v8 6/7] gpu: nova-core: create debugfs root in module init
2026-03-10 21:59 [PATCH v8 0/7] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
` (4 preceding siblings ...)
2026-03-10 21:59 ` [PATCH v8 5/7] gpu: nova-core: use pin projection in method boot() Timur Tabi
@ 2026-03-10 21:59 ` Timur Tabi
2026-03-10 22:00 ` [PATCH v8 7/7] gpu: nova-core: create GSP-RM logging buffers debugfs entries Timur Tabi
` (2 subsequent siblings)
8 siblings, 0 replies; 21+ messages in thread
From: Timur Tabi @ 2026-03-10 21:59 UTC (permalink / raw)
To: Gary Guo, Alice Ryhl, mmaurer, Danilo Krummrich,
Alexandre Courbot, John Hubbard, Joel Fernandes, 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.53.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v8 7/7] gpu: nova-core: create GSP-RM logging buffers debugfs entries
2026-03-10 21:59 [PATCH v8 0/7] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
` (5 preceding siblings ...)
2026-03-10 21:59 ` [PATCH v8 6/7] gpu: nova-core: create debugfs root in module init Timur Tabi
@ 2026-03-10 22:00 ` Timur Tabi
2026-03-10 22:20 ` [PATCH v8 0/7] gpu: nova-core: expose the logging buffers via debugfs John Hubbard
2026-03-12 3:50 ` John Hubbard
8 siblings, 0 replies; 21+ messages in thread
From: Timur Tabi @ 2026-03-10 22:00 UTC (permalink / raw)
To: Gary Guo, Alice Ryhl, mmaurer, Danilo Krummrich,
Alexandre Courbot, John Hubbard, Joel Fernandes, 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 174feaca0a6b..863a75b93c30 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.
pub(crate) cmdq: Cmdq,
/// RM arguments.
@@ -123,15 +132,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,
@@ -152,6 +163,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.53.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v8 0/7] gpu: nova-core: expose the logging buffers via debugfs
2026-03-10 21:59 [PATCH v8 0/7] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
` (6 preceding siblings ...)
2026-03-10 22:00 ` [PATCH v8 7/7] gpu: nova-core: create GSP-RM logging buffers debugfs entries Timur Tabi
@ 2026-03-10 22:20 ` John Hubbard
2026-03-12 3:50 ` John Hubbard
8 siblings, 0 replies; 21+ messages in thread
From: John Hubbard @ 2026-03-10 22:20 UTC (permalink / raw)
To: Timur Tabi, Gary Guo, Alice Ryhl, mmaurer, Danilo Krummrich,
Alexandre Courbot, Joel Fernandes, rust-for-linux, nouveau
On 3/10/26 2:59 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.
I'll give this series a quick re-test and if it continues to work
well, a tested-by tag at least.
>
> 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.
>
> 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.
>
> Changes since v7:
> 1. Fixed rustfmt complaints
> 2. Marked write_raw() as unsafe
> 3. rebased onto drm-rust-next
>
> Timur Tabi (7):
> 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: use pin projection in method boot()
> 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/gsp/boot.rs | 14 ++---
> drivers/gpu/nova-core/nova_core.rs | 50 +++++++++++++++++-
> rust/helpers/device.c | 5 ++
> rust/kernel/device.rs | 10 ++++
> rust/kernel/dma.rs | 36 ++++++++++++-
> rust/kernel/uaccess.rs | 84 ++++++++++++++++++++++++++----
> 7 files changed, 221 insertions(+), 27 deletions(-)
>
>
> base-commit: a544873ce0575b2fd8285a1364d3e09929d9a3ba
thanks,
--
John Hubbard
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v8 0/7] gpu: nova-core: expose the logging buffers via debugfs
2026-03-10 21:59 [PATCH v8 0/7] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
` (7 preceding siblings ...)
2026-03-10 22:20 ` [PATCH v8 0/7] gpu: nova-core: expose the logging buffers via debugfs John Hubbard
@ 2026-03-12 3:50 ` John Hubbard
8 siblings, 0 replies; 21+ messages in thread
From: John Hubbard @ 2026-03-12 3:50 UTC (permalink / raw)
To: Timur Tabi, Gary Guo, Alice Ryhl, mmaurer, Danilo Krummrich,
Alexandre Courbot, Joel Fernandes, rust-for-linux, nouveau
On 3/10/26 2:59 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.
Hi Timur,
I've booted this up on Ampere, and my scripts are still able to
extract the logs and decode them, so everything is still working
nicely.
For the series, please feel free to add:
Tested-by: John Hubbard <jhubbard@nvidia.com>
thanks,
--
John Hubbard
^ permalink raw reply [flat|nested] 21+ messages in thread