gpu: nova-core: expose the logging buffers via debugfs
Subsystems (6)
v7 cover gpu: nova-core: expose the logging buffers via debugfs
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.
These entries are gated on a CONFIG_NOVA_CORE_DEBUGFS Kconfig option.
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 v6:
1. Removed NOVA_CORE_DEBUGFS Kconfig
2. Expanded some SAFETY and PANIC comments
3. Added write_dma() and write_raw()
4. Renamed BinaryWriter patch
5. Moved misplaced diff in BinaryWriter patch
6. Miscellaneous minor changes.
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 | 74 +++++++++++++++++++++++++----- 7 files changed, 210 insertions(+), 28 deletions(-)
base-commit: cea7b66a80412e2a5b74627b89ae25f1d0110a4b
Signature · 1 line
2.52.0
PATCH v7 1/7 rust: device: add device name method
Add a name() method to the Device type, which returns a CStr that contains the device name.
--- rust/helpers/device.c | 5 +++++ rust/kernel/device.rs | 10 ++++++++++ 2 files changed, 15 insertions(+)
rust/helpers/device.c +5 -0
@@ -25,3 +25,8 @@ 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); +}
rust/kernel/device.rs +10 -0
@@ -482,6 +482,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
Signature · 1 line
2.52.0
PATCH v7 2/7 rust: uaccess: add write_dma() for copying from DMA buffers to userspace
In reply to Timur TabiAdd 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.
--- rust/kernel/uaccess.rs | 74 +++++++++++++++++++++++++++++++++++------- 1 file changed, 63 insertions(+), 11 deletions(-)
rust/kernel/uaccess.rs +63 -11
@@ -7,6 +7,7 @@use crate ::{ alloc::{Allocator ,Flags }, bindings, + dma::CoherentAllocation , error::Result , ffi::{c_char, c_void}, fs::file,
@@ -459,20 +460,20 @@ pub fn is_empty(&self) -> bool { self.length ==0 } -/// 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 { -let len = data.len (); -let data_ptr = data.as_ptr ().cast ::<c_void >(); +/// Low-level write from a raw pointer. Caller must ensure ptr is valid for `len` bytes. +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 +482,57 @@ 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 { + 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) }; + + 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
Signature · 1 line
2.52.0
kernel test robot
build botRe: [PATCH v7 2/7] rust: uaccess: add write_dma() for copying from DMA buffers to userspace
kernel test robot noticed the following build errors:
Hi Timur,
kernel test robot noticed the following build errors:
[auto build test ERROR on cea7b66a80412e2a5b74627b89ae25f1d0110a4b]
url: https://github.com/intel-lab-lkp/linux/commits/Timur-Tabi/rust-device-add-device-name-method/20260204-065057 base: cea7b66a80412e2a5b74627b89ae25f1d0110a4b patch link: https://lore.kernel.org/r/20260203224757.871729-3-ttabi%40nvidia.com patch subject: [PATCH v7 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/20260204/[email protected]/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/20260204/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
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:471:
// - `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:471:
// - `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[1]: Leaving directory '/kbuild/obj/consumer/x86_64-rhel-9.4-rust' make[2]: *** [Makefile:1871: rustfmt] Error 123
make[2]: Target 'rustfmtcheck' not remade because of errors. make[1]: *** [Makefile:248: __sub-make] Error 2
make: *** [Makefile:248: __sub-make] Error 2
make[1]: Target 'rustfmtcheck' not remade because of errors.
make: Target 'rustfmtcheck' not remade because of errors.
make: Leaving directory '/kbuild/src/consumer'
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Re: [PATCH v7 2/7] rust: uaccess: add write_dma() for copying from DMA buffers to userspace
Quoted 15 lines · kernel test robot
Quoted 1 line
Quoted 1 line
Diff in
rust/kernel/uaccess.rs:471:// -
ptris valid for readinglenbytes as required by this function's safety contract.
// -copy_to_uservalidates 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);
}Quoted 1 line
Quoted 1 line
Diff in
rust/kernel/uaccess.rs:471:
Quoted 13 lines
// -
ptris valid for readinglenbytes as required by this function's safety contract.
// -copy_to_uservalidates 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);
}
Why does rustfmtcheck display the same diff twice?
Danilo Krummrich
maintainerRe: [PATCH v7 2/7] rust: uaccess: add write_dma() for copying from DMA buffers to userspace
In reply to Timur TabiQuoted 31 lines · Timur Tabi
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()andwrite_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 <[email protected]>--- rust/kernel/uaccess.rs | 74 +++++++++++++++++++++++++++++++++++------- 1 file changed, 63 insertions(+), 11 deletions(-)rust/kernel/uaccess.rs +1
-0@@ -7,6 +7,7 @@use crate ::{ alloc::{Allocator ,Flags }, bindings, + dma::CoherentAllocation , error::Result , ffi::{c_char, c_void}, fs::file,@@ -459,20 +460,20 @@ pub fn is_empty(&self) -> bool { self.length ==0 }
rust/kernel/uaccess.rs pub fn is_empty(&self) -> bool { · line 464
Quoted 10 lines
rust/kernel/uaccess.rs +2
-8-/// 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 { -let len = data.len (); -let data_ptr = data.as_ptr ().cast ::<c_void >(); +/// Low-level write from a raw pointer. Caller must ensure ptr is valid for `len` bytes. +fn write_raw (&mut self, ptr: *const u8 , len:usize ) ->Result {
The method has to be unsafe as the caller has to promise that ptr is indeed a slice with len elements.
Another option would be to pass a fat pointer, i.e. *const [u8]. write_dma() would then need to use ptr::slice_from_raw_parts() and the safety requirement of this function becomes that ptr simply has to be valid.
Re: [PATCH v7 2/7] rust: uaccess: add write_dma() for copying from DMA buffers to userspace
Quoted 5 lines · Danilo Krummrich
Quoted 2 lines
+ /// Low-level write from a raw pointer. Caller must ensure ptr is valid for
lenbytes. + fnwrite_raw(&mut self, ptr: *const u8, len: usize)-> Result {The method has to be unsafe as the caller has to promise that ptr is indeed a slice with len elements.
Ok.
Quoted 3 lines
Another option would be to pass a fat pointer, i.e. *const [u8].
write_dma()would then need to useptr::slice_from_raw_parts()and the safety requirement of this function becomes that ptr simply has to be valid.
So I tried this approach, but the end result was that write_raw() and write_slice() were practically identical. At this point, why bother with write_raw() -- just have write_dma() call write_slice():
let src_ptr = unsafe { alloc.start_ptr().add(offset) };
let slice = unsafe { core::slice::from_raw_parts(src_ptr, count) };
self.write_slice(slice)I think this is better, but I wanted to get your opinion before I posted a v8 with this change.
Danilo Krummrich
maintainerRe: [PATCH v7 2/7] rust: uaccess: add write_dma() for copying from DMA buffers to userspace
Quoted 8 lines · Timur Tabi
Quoted 5 lines · Danilo Krummrich
Quoted 2 lines
+ /// Low-level write from a raw pointer. Caller must ensure ptr is valid for
lenbytes. + fnwrite_raw(&mut self, ptr: *const u8, len: usize)-> Result {The method has to be unsafe as the caller has to promise that ptr is indeed a slice with len elements.
Quoted 8 lines
Ok.
Quoted 3 lines
Another option would be to pass a fat pointer, i.e. *const [u8].
write_dma()would then need to useptr::slice_from_raw_parts()and the safety requirement of this function becomes that ptr simply has to be valid.So I tried this approach, but the end result was that
write_raw()andwrite_slice()were practically identical.
That sounds wrong, the only thing that write_slice() needs to do is to forward to write_raw(), that's a one-liner and not different to what you have in your patch currently?
Quoted 7 lines
At this point, why bother with
write_raw()-- just havewrite_dma()callwrite_slice():let src_ptr = unsafe { alloc.start_ptr().add(offset) }; let slice = unsafe { core::slice::from_raw_parts(src_ptr, count) }; self.write_slice(slice)I think this is better, but I wanted to get your opinion before I posted a v8 with this change.
The problem is that creating a slice of a DMA buffer, that is used by a device concurrently, is undefined behavior. Hence the indirection through write_raw().
Re: [PATCH v7 2/7] rust: uaccess: add write_dma() for copying from DMA buffers to userspace
Quoted 8 lines · Danilo Krummrich
Quoted 4 lines
So I tried this approach, but the end result was that
write_raw()andwrite_slice()were practically
identical.That sounds wrong, the only thing that
write_slice()needs to do is to forward towrite_raw(), that's a one-liner and not different to what you have in your patch currently?
Sorry, I guess I was unclear. What I meant is that when write_slice() calls write_raw(), it passes a parameter that was similar to what it received:
pub fn write_slice(&mut self, data: &[u8]) -> Result {
self.write_raw(core::ptr::from_ref(data))
}Quoted 11 lines
Quoted 8 lines
At this point, why bother with
write_raw()-- just havewrite_dma()callwrite_slice():let
src_ptr= unsafe { alloc.start_ptr().add(offset) }; let slice = unsafe {core::slice::from_raw_parts(src_ptr, count)};self.write_slice(slice)I think this is better, but I wanted to get your opinion before I posted a v8 with this change.
The problem is that creating a slice of a DMA buffer, that is used by a device concurrently, is undefined behavior. Hence the indirection through
write_raw().
Ok, I had forgotten about that. I'll add a comment to that effect.
Alice Ryhl
maintainerRe: [PATCH v7 2/7] rust: uaccess: add write_dma() for copying from DMA buffers to userspace
In reply to Timur TabiQuoted 14 lines · Timur Tabi
Quoted 5 lines · Danilo Krummrich
Quoted 2 lines
+ /// Low-level write from a raw pointer. Caller must ensure ptr is valid for
lenbytes. + fnwrite_raw(&mut self, ptr: *const u8, len: usize)-> Result {The method has to be unsafe as the caller has to promise that ptr is indeed a slice with len elements.
Ok.
Quoted 3 lines
Another option would be to pass a fat pointer, i.e. *const [u8].
write_dma()would then need to useptr::slice_from_raw_parts()and the safety requirement of this function becomes that ptr simply has to be valid.
Quoted 8 lines
So I tried this approach, but the end result was that
write_raw()andwrite_slice()were practically identical. At this point, why bother withwrite_raw()-- just havewrite_dma()callwrite_slice():let src_ptr = unsafe { alloc.start_ptr().add(offset) }; let slice = unsafe { core::slice::from_raw_parts(src_ptr, count) }; self.write_slice(slice)I think this is better, but I wanted to get your opinion before I posted a v8 with this change.
Unfortunately, the creation of a reference into volatile memory is not legal. Here, 'reference' includes slices. You can implement write_slice in terms of write_raw, but the reverse will not work.
Alice
PATCH v7 3/7 rust: dma: implement BinaryWriter for CoherentAllocation<u8>
In reply to Timur TabiImplement 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().
--- rust/kernel/dma.rs | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-)
rust/kernel/dma.rs +35 -1
@@ -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 ;
@@ -651,6 +653,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 += countas i64 ; +Ok (count) + } +} +/// Reads a field of an item from an allocated region of structs. /// /// # Examples
Signature · 1 line
2.52.0
PATCH v7 4/7 gpu: nova-core: Replace module_pci_driver! with explicit module init
In reply to Timur TabiReplace 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.
--- drivers/gpu/nova-core/nova_core.rs | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-)
drivers/gpu/nova-core/nova_core.rs +23 -2
@@ -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 : &kernel::str::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" ,
Signature · 1 line
2.52.0
PATCH v7 5/7 gpu: nova-core: use pin projection in method boot()
In reply to Timur TabiStruct 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.
--- drivers/gpu/nova-core/gsp/boot.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
drivers/gpu/nova-core/gsp/boot.rs +8 -6
@@ -159,12 +159,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),
@@ -231,13 +233,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.as_ref (),"GPU name: {}\n" , name),Err (e) =>dev_warn! (pdev.as_ref (),"GPU name unavailable: {:?}\n" , e),
Signature · 1 line
2.52.0
PATCH v7 6/7 gpu: nova-core: create debugfs root in module init
In reply to Timur TabiCreate 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.
--- drivers/gpu/nova-core/nova_core.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
drivers/gpu/nova-core/nova_core.rs +25 -0
@@ -3,6 +3,7 @@//! Nova Core GPU Driver use kernel::{ + debugfs, driver::Registration , pci, prelude::*,
@@ -27,16 +28,40 @@pub (crate )const MODULE_NAME : &kernel::str::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 , }) } }
Signature · 1 line
2.52.0
PATCH v7 7/7 gpu: nova-core: create GSP-RM logging buffers debugfs entries
In reply to Timur TabiCreate 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.
--- drivers/gpu/nova-core/gsp.rs | 49 ++++++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 8 deletions(-)
drivers/gpu/nova-core/gsp.rs +41 -8
@@ -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, Errordma_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 ); + }) + }, })) }) }
Signature · 1 line
2.52.0
On 2/3/26 2:47 PM, Timur Tabi wrote:
...
Quoted 9 lines
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 | 74 +++++++++++++++++++++++++----- 7 files changed, 210 insertions(+), 28 deletions(-)
I've been using this on my multi-GPU setup, and it has been working just fine, although there is a somewhat related patch [1] that you'll want in order to use multi-GPU with this.
So for the series:
[1] https://lore.kernel.org/[email protected]
thanks,
Signature · 1 line
John Hubbard