* [PATCH 1/7] rust: pci: add PCI device name method
2025-12-12 20:49 [PATCH 0/7] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
@ 2025-12-12 20:49 ` Timur Tabi
2025-12-15 11:35 ` Gary Guo
2025-12-15 16:49 ` lyude
2025-12-12 20:49 ` [PATCH 2/7] gpu: nova-core: Replace module_pci_driver! with explicit module init Timur Tabi
` (6 subsequent siblings)
7 siblings, 2 replies; 25+ messages in thread
From: Timur Tabi @ 2025-12-12 20:49 UTC (permalink / raw)
To: Danilo Krummrich, Alexandre Courbot, John Hubbard, joelagnelf,
lyude, nouveau, rust-for-linux
Add a name() method to the PCI `Device` type, which returns a CStr
that contains the device name, typically the BDF address.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
rust/helpers/pci.c | 5 +++++
rust/kernel/pci.rs | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+)
diff --git a/rust/helpers/pci.c b/rust/helpers/pci.c
index bf8173979c5e..411bc743fcc2 100644
--- a/rust/helpers/pci.c
+++ b/rust/helpers/pci.c
@@ -2,6 +2,11 @@
#include <linux/pci.h>
+const char *rust_helper_pci_name(const struct pci_dev *pdev)
+{
+ return pci_name(pdev);
+}
+
u16 rust_helper_pci_dev_id(struct pci_dev *dev)
{
return PCI_DEVID(dev->bus->number, dev->devfn);
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 82e128431f08..d0c0c2f6aa32 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -427,6 +427,43 @@ pub fn pci_class(&self) -> Class {
// SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
Class::from_raw(unsafe { (*self.as_raw()).class })
}
+
+ /// Returns the PCI device name.
+ ///
+ /// This returns the device name in the format "DDDD:BB:DD.F" where:
+ /// - DDDD is the PCI domain (4 hex digits)
+ /// - BB is the bus number (2 hex digits)
+ /// - DD is the device number (2 hex digits)
+ /// - F is the function number (1 hex digit)
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// # use kernel::{c_str, debugfs::Dir, device::Core, pci, prelude::*};
+ /// fn create_debugfs(pdev: &pci::Device<Core>) -> Result {
+ /// let dir = Dir::new(pdev.name());
+ /// Ok(())
+ /// }
+ /// ```
+ #[inline]
+ pub fn name(&self) -> &CStr {
+ // SAFETY: By its type invariant `self.as_raw` is always a valid pointer to a
+ // `struct pci_dev`, which contains a `struct device dev` member.
+ unsafe {
+ let pci_dev = self.as_raw();
+ let dev = addr_of_mut!((*pci_dev).dev);
+
+ // If init_name is set, use it; otherwise use the kobject name
+ let init_name = (*dev).init_name;
+ let name_ptr = if !init_name.is_null() {
+ init_name
+ } else {
+ (*dev).kobj.name
+ };
+
+ CStr::from_char_ptr(name_ptr)
+ }
+ }
}
impl Device<device::Core> {
--
2.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 1/7] rust: pci: add PCI device name method
2025-12-12 20:49 ` [PATCH 1/7] rust: pci: add PCI device name method Timur Tabi
@ 2025-12-15 11:35 ` Gary Guo
2025-12-15 16:49 ` lyude
1 sibling, 0 replies; 25+ messages in thread
From: Gary Guo @ 2025-12-15 11:35 UTC (permalink / raw)
To: Timur Tabi
Cc: Danilo Krummrich, Alexandre Courbot, John Hubbard, joelagnelf,
lyude, nouveau, rust-for-linux
On Fri, 12 Dec 2025 14:49:46 -0600
Timur Tabi <ttabi@nvidia.com> wrote:
> Add a name() method to the PCI `Device` type, which returns a CStr
> that contains the device name, typically the BDF address.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
> rust/helpers/pci.c | 5 +++++
> rust/kernel/pci.rs | 37 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 42 insertions(+)
>
> diff --git a/rust/helpers/pci.c b/rust/helpers/pci.c
> index bf8173979c5e..411bc743fcc2 100644
> --- a/rust/helpers/pci.c
> +++ b/rust/helpers/pci.c
> @@ -2,6 +2,11 @@
>
> #include <linux/pci.h>
>
> +const char *rust_helper_pci_name(const struct pci_dev *pdev)
> +{
> + return pci_name(pdev);
> +}
> +
You're adding a helper function while having a Rust reimplementation of
the added function. Please pick one approach and stick with it.
Best,
Gary
> u16 rust_helper_pci_dev_id(struct pci_dev *dev)
> {
> return PCI_DEVID(dev->bus->number, dev->devfn);
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 82e128431f08..d0c0c2f6aa32 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -427,6 +427,43 @@ pub fn pci_class(&self) -> Class {
> // SAFETY: `self.as_raw` is a valid pointer to a `struct pci_dev`.
> Class::from_raw(unsafe { (*self.as_raw()).class })
> }
> +
> + /// Returns the PCI device name.
> + ///
> + /// This returns the device name in the format "DDDD:BB:DD.F" where:
> + /// - DDDD is the PCI domain (4 hex digits)
> + /// - BB is the bus number (2 hex digits)
> + /// - DD is the device number (2 hex digits)
> + /// - F is the function number (1 hex digit)
> + ///
> + /// # Examples
> + ///
> + /// ```
> + /// # use kernel::{c_str, debugfs::Dir, device::Core, pci, prelude::*};
> + /// fn create_debugfs(pdev: &pci::Device<Core>) -> Result {
> + /// let dir = Dir::new(pdev.name());
> + /// Ok(())
> + /// }
> + /// ```
> + #[inline]
> + pub fn name(&self) -> &CStr {
> + // SAFETY: By its type invariant `self.as_raw` is always a valid pointer to a
> + // `struct pci_dev`, which contains a `struct device dev` member.
> + unsafe {
> + let pci_dev = self.as_raw();
> + let dev = addr_of_mut!((*pci_dev).dev);
> +
> + // If init_name is set, use it; otherwise use the kobject name
> + let init_name = (*dev).init_name;
> + let name_ptr = if !init_name.is_null() {
> + init_name
> + } else {
> + (*dev).kobj.name
> + };
> +
> + CStr::from_char_ptr(name_ptr)
> + }
> + }
> }
>
> impl Device<device::Core> {
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 1/7] rust: pci: add PCI device name method
2025-12-12 20:49 ` [PATCH 1/7] rust: pci: add PCI device name method Timur Tabi
2025-12-15 11:35 ` Gary Guo
@ 2025-12-15 16:49 ` lyude
1 sibling, 0 replies; 25+ messages in thread
From: lyude @ 2025-12-15 16:49 UTC (permalink / raw)
To: Timur Tabi, Danilo Krummrich, Alexandre Courbot, John Hubbard,
joelagnelf, nouveau, rust-for-linux
On Fri, 2025-12-12 at 14:49 -0600, Timur Tabi wrote:
> + // `struct pci_dev`, which contains a `struct device dev`
> member.
> + unsafe {
> + let pci_dev = self.as_raw();
> + let dev = addr_of_mut!((*pci_dev).dev);
> +
JFYI - addr_of!/addr_of_mut! shouldn't be used in new code, we're
moving to using &raw const and &raw mut respectively (which pretty much
do the same thing without a macro).
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/7] gpu: nova-core: Replace module_pci_driver! with explicit module init
2025-12-12 20:49 [PATCH 0/7] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
2025-12-12 20:49 ` [PATCH 1/7] rust: pci: add PCI device name method Timur Tabi
@ 2025-12-12 20:49 ` Timur Tabi
2025-12-13 23:46 ` Joel Fernandes
2025-12-15 11:36 ` Gary Guo
2025-12-12 20:49 ` [PATCH 3/7] gpu: nova-core: create debugfs root in PCI init closure Timur Tabi
` (5 subsequent siblings)
7 siblings, 2 replies; 25+ messages in thread
From: Timur Tabi @ 2025-12-12 20:49 UTC (permalink / raw)
To: Danilo Krummrich, Alexandre Courbot, John Hubbard, joelagnelf,
lyude, nouveau, rust-for-linux
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.
Also add a no-op module exit function as a template.
This change is necessary so that we can create a top-level "novacore"
debugfs entry when the driver is loaded.
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
drivers/gpu/nova-core/nova_core.rs | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index b98a1c03f13d..7d7b75650b04 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -2,6 +2,9 @@
//! Nova Core GPU Driver
+use kernel::{error::Error, pci, prelude::*, InPlaceModule};
+use pin_init::{PinInit, pinned_drop};
+
#[macro_use]
mod bitfield;
@@ -21,13 +24,33 @@
pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
-kernel::module_pci_driver! {
- type: driver::NovaCore,
+#[pin_data(PinnedDrop)]
+struct NovaCoreModule {
+ #[pin]
+ _driver: kernel::driver::Registration<pci::Adapter<driver::NovaCore>>,
+}
+
+impl InPlaceModule for NovaCoreModule {
+ fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
+ pr_info!("NovaCore GPU driver loaded\n");
+ try_pin_init!(Self {
+ _driver <- kernel::driver::Registration::new(MODULE_NAME, module),
+ })
+ }
+}
+
+#[pinned_drop]
+impl PinnedDrop for NovaCoreModule {
+ fn drop(self: Pin<&mut Self>) {
+ }
+}
+
+module! {
+ type: NovaCoreModule,
name: "NovaCore",
authors: ["Danilo Krummrich"],
description: "Nova Core GPU driver",
license: "GPL v2",
- firmware: [],
}
kernel::module_firmware!(firmware::ModInfoBuilder);
--
2.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 2/7] gpu: nova-core: Replace module_pci_driver! with explicit module init
2025-12-12 20:49 ` [PATCH 2/7] gpu: nova-core: Replace module_pci_driver! with explicit module init Timur Tabi
@ 2025-12-13 23:46 ` Joel Fernandes
2025-12-15 11:36 ` Gary Guo
1 sibling, 0 replies; 25+ messages in thread
From: Joel Fernandes @ 2025-12-13 23:46 UTC (permalink / raw)
To: Timur Tabi
Cc: Danilo Krummrich, Alexandre Courbot, John Hubbard,
lyude@redhat.com, nouveau@lists.freedesktop.org,
rust-for-linux@vger.kernel.org
> On Dec 13, 2025, at 6:00 AM, Timur Tabi <ttabi@nvidia.com> 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.
>
> Also add a no-op module exit function as a template.
>
> This change is necessary so that we can create a top-level "novacore"
> debugfs entry when the driver is loaded.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
> drivers/gpu/nova-core/nova_core.rs | 29 ++++++++++++++++++++++++++---
> 1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> index b98a1c03f13d..7d7b75650b04 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -2,6 +2,9 @@
>
> //! Nova Core GPU Driver
>
> +use kernel::{error::Error, pci, prelude::*, InPlaceModule};
> +use pin_init::{PinInit, pinned_drop};
> +
Import style errors (one per line with final //) needs fixing in this and other patches.
- Joel
> #[macro_use]
> mod bitfield;
>
> @@ -21,13 +24,33 @@
>
> pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
>
> -kernel::module_pci_driver! {
> - type: driver::NovaCore,
> +#[pin_data(PinnedDrop)]
> +struct NovaCoreModule {
> + #[pin]
> + _driver: kernel::driver::Registration<pci::Adapter<driver::NovaCore>>,
> +}
> +
> +impl InPlaceModule for NovaCoreModule {
> + fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
> + pr_info!("NovaCore GPU driver loaded\n");
> + try_pin_init!(Self {
> + _driver <- kernel::driver::Registration::new(MODULE_NAME, module),
> + })
> + }
> +}
> +
> +#[pinned_drop]
> +impl PinnedDrop for NovaCoreModule {
> + fn drop(self: Pin<&mut Self>) {
> + }
> +}
> +
> +module! {
> + type: NovaCoreModule,
> name: "NovaCore",
> authors: ["Danilo Krummrich"],
> description: "Nova Core GPU driver",
> license: "GPL v2",
> - firmware: [],
> }
>
> kernel::module_firmware!(firmware::ModInfoBuilder);
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 2/7] gpu: nova-core: Replace module_pci_driver! with explicit module init
2025-12-12 20:49 ` [PATCH 2/7] gpu: nova-core: Replace module_pci_driver! with explicit module init Timur Tabi
2025-12-13 23:46 ` Joel Fernandes
@ 2025-12-15 11:36 ` Gary Guo
1 sibling, 0 replies; 25+ messages in thread
From: Gary Guo @ 2025-12-15 11:36 UTC (permalink / raw)
To: Timur Tabi
Cc: Danilo Krummrich, Alexandre Courbot, John Hubbard, joelagnelf,
lyude, nouveau, rust-for-linux
On Fri, 12 Dec 2025 14:49:47 -0600
Timur Tabi <ttabi@nvidia.com> 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.
>
> Also add a no-op module exit function as a template.
>
> This change is necessary so that we can create a top-level "novacore"
> debugfs entry when the driver is loaded.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
> drivers/gpu/nova-core/nova_core.rs | 29 ++++++++++++++++++++++++++---
> 1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> index b98a1c03f13d..7d7b75650b04 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -2,6 +2,9 @@
>
> //! Nova Core GPU Driver
>
> +use kernel::{error::Error, pci, prelude::*, InPlaceModule};
> +use pin_init::{PinInit, pinned_drop};
> +
> #[macro_use]
> mod bitfield;
>
> @@ -21,13 +24,33 @@
>
> pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
>
> -kernel::module_pci_driver! {
> - type: driver::NovaCore,
> +#[pin_data(PinnedDrop)]
> +struct NovaCoreModule {
> + #[pin]
> + _driver: kernel::driver::Registration<pci::Adapter<driver::NovaCore>>,
> +}
> +
> +impl InPlaceModule for NovaCoreModule {
> + fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
> + pr_info!("NovaCore GPU driver loaded\n");
> + try_pin_init!(Self {
> + _driver <- kernel::driver::Registration::new(MODULE_NAME, module),
> + })
> + }
> +}
> +
> +#[pinned_drop]
> +impl PinnedDrop for NovaCoreModule {
> + fn drop(self: Pin<&mut Self>) {
> + }
> +}
I don't see any subsequent patch that makes use of this added drop impl.
Why is it needed?
Best,
Gary
> +
> +module! {
> + type: NovaCoreModule,
> name: "NovaCore",
> authors: ["Danilo Krummrich"],
> description: "Nova Core GPU driver",
> license: "GPL v2",
> - firmware: [],
> }
>
> kernel::module_firmware!(firmware::ModInfoBuilder);
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 3/7] gpu: nova-core: create debugfs root in PCI init closure
2025-12-12 20:49 [PATCH 0/7] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
2025-12-12 20:49 ` [PATCH 1/7] rust: pci: add PCI device name method Timur Tabi
2025-12-12 20:49 ` [PATCH 2/7] gpu: nova-core: Replace module_pci_driver! with explicit module init Timur Tabi
@ 2025-12-12 20:49 ` Timur Tabi
2025-12-15 11:40 ` Gary Guo
2025-12-12 20:49 ` [PATCH 4/7] gpu: nova-core: implement BinaryWriter for LogBuffer Timur Tabi
` (4 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Timur Tabi @ 2025-12-12 20:49 UTC (permalink / raw)
To: Danilo Krummrich, Alexandre Courbot, John Hubbard, joelagnelf,
lyude, nouveau, rust-for-linux
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 | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index 7d7b75650b04..591edede4376 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -2,7 +2,7 @@
//! Nova Core GPU Driver
-use kernel::{error::Error, pci, prelude::*, InPlaceModule};
+use kernel::{error::Error, pci, prelude::*, InPlaceModule, debugfs::Dir};
use pin_init::{PinInit, pinned_drop};
#[macro_use]
@@ -24,6 +24,8 @@
pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
+static mut DEBUGFS_ROOT: Option<Dir> = None;
+
#[pin_data(PinnedDrop)]
struct NovaCoreModule {
#[pin]
@@ -33,6 +35,13 @@ struct NovaCoreModule {
impl InPlaceModule for NovaCoreModule {
fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
pr_info!("NovaCore GPU driver loaded\n");
+
+ let dir = Dir::new(kernel::c_str!("nova_core"));
+
+ // SAFETY: we are the only driver code running, so there cannot be any concurrent access to
+ // `DEBUGFS_ROOT`.
+ unsafe { DEBUGFS_ROOT = Some(dir) };
+
try_pin_init!(Self {
_driver <- kernel::driver::Registration::new(MODULE_NAME, module),
})
@@ -42,6 +51,9 @@ fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
#[pinned_drop]
impl PinnedDrop for NovaCoreModule {
fn drop(self: Pin<&mut Self>) {
+ // SAFETY: we are the only driver code running, so there cannot be any concurrent access to
+ // `DEBUGFS_ROOT`.
+ unsafe { DEBUGFS_ROOT = None };
}
}
--
2.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 3/7] gpu: nova-core: create debugfs root in PCI init closure
2025-12-12 20:49 ` [PATCH 3/7] gpu: nova-core: create debugfs root in PCI init closure Timur Tabi
@ 2025-12-15 11:40 ` Gary Guo
2025-12-15 16:45 ` Timur Tabi
0 siblings, 1 reply; 25+ messages in thread
From: Gary Guo @ 2025-12-15 11:40 UTC (permalink / raw)
To: Timur Tabi
Cc: Danilo Krummrich, Alexandre Courbot, John Hubbard, joelagnelf,
lyude, nouveau, rust-for-linux
On Fri, 12 Dec 2025 14:49:48 -0600
Timur Tabi <ttabi@nvidia.com> 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>
> ---
> drivers/gpu/nova-core/nova_core.rs | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
> index 7d7b75650b04..591edede4376 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -2,7 +2,7 @@
>
> //! Nova Core GPU Driver
>
> -use kernel::{error::Error, pci, prelude::*, InPlaceModule};
> +use kernel::{error::Error, pci, prelude::*, InPlaceModule, debugfs::Dir};
> use pin_init::{PinInit, pinned_drop};
>
> #[macro_use]
> @@ -24,6 +24,8 @@
>
> pub(crate) const MODULE_NAME: &kernel::str::CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
>
> +static mut DEBUGFS_ROOT: Option<Dir> = None;
Please don't use `static mut`. If you need anything associcated with the
lifetime of the driver, please just put it into the module.
> +
> #[pin_data(PinnedDrop)]
> struct NovaCoreModule {
> #[pin]
> @@ -33,6 +35,13 @@ struct NovaCoreModule {
> impl InPlaceModule for NovaCoreModule {
> fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
> pr_info!("NovaCore GPU driver loaded\n");
> +
> + let dir = Dir::new(kernel::c_str!("nova_core"));
> +
> + // SAFETY: we are the only driver code running, so there cannot be any concurrent access to
> + // `DEBUGFS_ROOT`.
> + unsafe { DEBUGFS_ROOT = Some(dir) };
> +
> try_pin_init!(Self {
> _driver <- kernel::driver::Registration::new(MODULE_NAME, module),
> })
> @@ -42,6 +51,9 @@ fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
> #[pinned_drop]
> impl PinnedDrop for NovaCoreModule {
> fn drop(self: Pin<&mut Self>) {
> + // SAFETY: we are the only driver code running, so there cannot be any concurrent access to
> + // `DEBUGFS_ROOT`.
> + unsafe { DEBUGFS_ROOT = None };
Sorry, I missed this from my previous email. But if you put this struct
inside module itself, the dtor will automatically be run on module unload.
Best,
Gary
> }
> }
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 3/7] gpu: nova-core: create debugfs root in PCI init closure
2025-12-15 11:40 ` Gary Guo
@ 2025-12-15 16:45 ` Timur Tabi
2025-12-16 10:04 ` John Hubbard
0 siblings, 1 reply; 25+ messages in thread
From: Timur Tabi @ 2025-12-15 16:45 UTC (permalink / raw)
To: gary@garyguo.net
Cc: nouveau@lists.freedesktop.org, Alexandre Courbot, dakr@kernel.org,
lyude@redhat.com, Joel Fernandes, John Hubbard,
rust-for-linux@vger.kernel.org
On Mon, 2025-12-15 at 11:40 +0000, Gary Guo wrote:
> >
> > +static mut DEBUGFS_ROOT: Option<Dir> = None;
>
> Please don't use `static mut`. If you need anything associcated with the
> lifetime of the driver, please just put it into the module.
If I do that, then how do I access it from gsp.rs (the last patch in this series)?
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/7] gpu: nova-core: create debugfs root in PCI init closure
2025-12-15 16:45 ` Timur Tabi
@ 2025-12-16 10:04 ` John Hubbard
2025-12-16 20:28 ` Timur Tabi
0 siblings, 1 reply; 25+ messages in thread
From: John Hubbard @ 2025-12-16 10:04 UTC (permalink / raw)
To: Timur Tabi, gary@garyguo.net
Cc: nouveau@lists.freedesktop.org, Alexandre Courbot, dakr@kernel.org,
lyude@redhat.com, Joel Fernandes, rust-for-linux@vger.kernel.org
On 12/16/25 1:45 AM, Timur Tabi wrote:
> On Mon, 2025-12-15 at 11:40 +0000, Gary Guo wrote:
>>>
>>> +static mut DEBUGFS_ROOT: Option<Dir> = None;
>>
>> Please don't use `static mut`. If you need anything associcated with the
>> lifetime of the driver, please just put it into the module.
>
> If I do that, then how do I access it from gsp.rs (the last patch in this series)?
Given that the current PCI .probe() doesn't pass in module data,
I looked around and saw that Binder is using global_lock!()
instead of passing around module data. It seems like this area
is still new in Rust for Linux.
I wonder if global_lock!() is what Gary had in mind? That is still
effectively global access, but at least it's synchronized,
unlike static mut.
thanks,
--
John Hubbard
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/7] gpu: nova-core: create debugfs root in PCI init closure
2025-12-16 10:04 ` John Hubbard
@ 2025-12-16 20:28 ` Timur Tabi
2025-12-16 22:01 ` Joel Fernandes
0 siblings, 1 reply; 25+ messages in thread
From: Timur Tabi @ 2025-12-16 20:28 UTC (permalink / raw)
To: gary@garyguo.net, John Hubbard
Cc: nouveau@lists.freedesktop.org, Alexandre Courbot, dakr@kernel.org,
lyude@redhat.com, Joel Fernandes, rust-for-linux@vger.kernel.org
On Tue, 2025-12-16 at 19:04 +0900, John Hubbard wrote:
> > If I do that, then how do I access it from gsp.rs (the last patch in this series)?
>
> Given that the current PCI .probe() doesn't pass in module data,
> I looked around and saw that Binder is using global_lock!()
> instead of passing around module data. It seems like this area
> is still new in Rust for Linux.
>
> I wonder if global_lock!() is what Gary had in mind? That is still
> effectively global access, but at least it's synchronized,
> unlike static mut.
It's only 'mut' in that it needs to be initialized when the driver loads. After that, it's only
referenced as non-mut:
#[allow(static_mut_refs)]
// SAFETY: `DEBUGFS_ROOT` is never modified after initialization, so it is safe to
// create a shared reference to it.
let novacore_dir = unsafe { crate::DEBUGFS_ROOT.as_ref() }.ok_or(ENOENT)?;
The first internal version of this code did have global_lock!, but it caused problems. That's why
Alex recommended to drop the lock and just have an unsafe initialization:
https://github.com/Gnurou/linux/commit/d5435f662b8677545a93373b4c4c80d8b4be40c9
It would be nice if Rust had a concept of a variable that was initialized at runtime once (before
any concurrent access could occur), and was then read-only everywhere else.
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 3/7] gpu: nova-core: create debugfs root in PCI init closure
2025-12-16 20:28 ` Timur Tabi
@ 2025-12-16 22:01 ` Joel Fernandes
2025-12-16 22:29 ` Timur Tabi
0 siblings, 1 reply; 25+ messages in thread
From: Joel Fernandes @ 2025-12-16 22:01 UTC (permalink / raw)
To: Timur Tabi, gary@garyguo.net, John Hubbard
Cc: nouveau@lists.freedesktop.org, Alexandre Courbot, dakr@kernel.org,
lyude@redhat.com, rust-for-linux@vger.kernel.org
On 12/16/2025 3:28 PM, Timur Tabi wrote:
> On Tue, 2025-12-16 at 19:04 +0900, John Hubbard wrote:
>>> If I do that, then how do I access it from gsp.rs (the last patch in this series)?
>>
>> Given that the current PCI .probe() doesn't pass in module data,
>> I looked around and saw that Binder is using global_lock!()
>> instead of passing around module data. It seems like this area
>> is still new in Rust for Linux.
>>
>> I wonder if global_lock!() is what Gary had in mind? That is still
>> effectively global access, but at least it's synchronized,
>> unlike static mut.
>
> It's only 'mut' in that it needs to be initialized when the driver loads. After that, it's only
> referenced as non-mut:
>
> #[allow(static_mut_refs)]
> // SAFETY: `DEBUGFS_ROOT` is never modified after initialization, so it is safe to
> // create a shared reference to it.
> let novacore_dir = unsafe { crate::DEBUGFS_ROOT.as_ref() }.ok_or(ENOENT)?;
>
> The first internal version of this code did have global_lock!, but it caused problems. That's why
> Alex recommended to drop the lock and just have an unsafe initialization:
>
> https://github.com/Gnurou/linux/commit/d5435f662b8677545a93373b4c4c80d8b4be40c9
>
> It would be nice if Rust had a concept of a variable that was initialized at runtime once (before
> any concurrent access could occur), and was then read-only everywhere else.
>
Could the debugfs Rust abstraction add a directory lookup function
`Dir::lookup(name)` method and allow creating sub directories under a known
path? This would let `probe()` find the module-created root directory without
needing global state right? Then store references to this on a per-GPU basis
that the GSP code can access. Alternatively, the first probe can create the root
directory, and subsequent probes reuse it without requiring module-level root.
thanks,
- Joel
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 3/7] gpu: nova-core: create debugfs root in PCI init closure
2025-12-16 22:01 ` Joel Fernandes
@ 2025-12-16 22:29 ` Timur Tabi
2025-12-16 23:17 ` Joel Fernandes
0 siblings, 1 reply; 25+ messages in thread
From: Timur Tabi @ 2025-12-16 22:29 UTC (permalink / raw)
To: gary@garyguo.net, Joel Fernandes, John Hubbard
Cc: nouveau@lists.freedesktop.org, Alexandre Courbot, dakr@kernel.org,
lyude@redhat.com, rust-for-linux@vger.kernel.org
On Tue, 2025-12-16 at 17:01 -0500, Joel Fernandes wrote:
> Could the debugfs Rust abstraction add a directory lookup function
> `Dir::lookup(name)` method and allow creating sub directories under a known
> path?
Actually, that's an excellent idea.
> This would let `probe()` find the module-created root directory without
> needing global state right? Then store references to this on a per-GPU basis
> that the GSP code can access.
Yes, that would work. I will try to implement this, but I suspect I will need help.
> Alternatively, the first probe can create the root
> directory, and subsequent probes reuse it without requiring module-level root.
How would the second probe know that it's not the first, and where would it look up the root Dir?
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/7] gpu: nova-core: create debugfs root in PCI init closure
2025-12-16 22:29 ` Timur Tabi
@ 2025-12-16 23:17 ` Joel Fernandes
2025-12-16 23:29 ` Timur Tabi
0 siblings, 1 reply; 25+ messages in thread
From: Joel Fernandes @ 2025-12-16 23:17 UTC (permalink / raw)
To: Timur Tabi, gary@garyguo.net, John Hubbard
Cc: nouveau@lists.freedesktop.org, Alexandre Courbot, dakr@kernel.org,
lyude@redhat.com, rust-for-linux@vger.kernel.org
On 12/16/2025 5:29 PM, Timur Tabi wrote:
> On Tue, 2025-12-16 at 17:01 -0500, Joel Fernandes wrote:
>> Could the debugfs Rust abstraction add a directory lookup function
>> `Dir::lookup(name)` method and allow creating sub directories under a known
>> path?
>
> Actually, that's an excellent idea.
Thanks.
>> This would let `probe()` find the module-created root directory without
>> needing global state right? Then store references to this on a per-GPU basis
>> that the GSP code can access.
>
> Yes, that would work. I will try to implement this, but I suspect I will need help.
Cool!
>> Alternatively, the first probe can create the root
>> directory, and subsequent probes reuse it without requiring module-level root.
>
> How would the second probe know that it's not the first, and where would it look up the root Dir?
Since driver_attach() attaches devices to drivers serially AFAIK, simply look up
the debugfs nova root path in probe(), if it does not exist create it. If it
does exist, then you know you're not the first one.
In C, debugfs_lookup() can lookup a name from the root if parent passed is NULL.
Or we can have a Dir::find_or_create() in the debugfs Rust abstraction which
abstracts this which does both.
Alternatively, create nova_core root directory at module init, and then probe()
can assume it exists. That's also robust against any possible concurrent
multiple device probes I think (basically the case where 2 look ups happen
simultaneously and then 2 creates happen simultaneously - one of them error'ing
out and printing a nastygram in dmesg).
thanks,
- Joel
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/7] gpu: nova-core: create debugfs root in PCI init closure
2025-12-16 23:17 ` Joel Fernandes
@ 2025-12-16 23:29 ` Timur Tabi
0 siblings, 0 replies; 25+ messages in thread
From: Timur Tabi @ 2025-12-16 23:29 UTC (permalink / raw)
To: gary@garyguo.net, Joel Fernandes, John Hubbard
Cc: nouveau@lists.freedesktop.org, Alexandre Courbot, dakr@kernel.org,
lyude@redhat.com, rust-for-linux@vger.kernel.org
On Tue, 2025-12-16 at 18:17 -0500, Joel Fernandes wrote:
> > How would the second probe know that it's not the first, and where would it look up the root
> > Dir?
>
> Since driver_attach() attaches devices to drivers serially AFAIK, simply look up
> the debugfs nova root path in probe(), if it does not exist create it. If it
> does exist, then you know you're not the first one.
Oh, you meant to use lookup() in both cases.
> In C, debugfs_lookup() can lookup a name from the root if parent passed is NULL.
>
> Or we can have a Dir::find_or_create() in the debugfs Rust abstraction which
> abstracts this which does both.
>
> Alternatively, create nova_core root directory at module init, and then probe()
> can assume it exists. That's also robust against any possible concurrent
> multiple device probes I think (basically the case where 2 look ups happen
> simultaneously and then 2 creates happen simultaneously - one of them error'ing
> out and printing a nastygram in dmesg).
I like this approach better.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 4/7] gpu: nova-core: implement BinaryWriter for LogBuffer
2025-12-12 20:49 [PATCH 0/7] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
` (2 preceding siblings ...)
2025-12-12 20:49 ` [PATCH 3/7] gpu: nova-core: create debugfs root in PCI init closure Timur Tabi
@ 2025-12-12 20:49 ` Timur Tabi
2025-12-12 20:49 ` [PATCH 5/7] gpu: nova-core: use pin projection in method boot() Timur Tabi
` (3 subsequent siblings)
7 siblings, 0 replies; 25+ messages in thread
From: Timur Tabi @ 2025-12-12 20:49 UTC (permalink / raw)
To: Danilo Krummrich, Alexandre Courbot, John Hubbard, joelagnelf,
lyude, nouveau, rust-for-linux
From: Alexandre Courbot <acourbot@nvidia.com>
`LogBuffer` is the entity we ultimately want to dump through debugfs.
Provide a simple implementation of `BinaryWriter` for it, albeit it
might not cut the safety requirements.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
drivers/gpu/nova-core/gsp.rs | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index fb6f74797178..860674dac31e 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -3,6 +3,7 @@
mod boot;
use kernel::{
+ debugfs,
device,
dma::{
CoherentAllocation,
@@ -117,6 +118,29 @@ pub(crate) struct Gsp {
rmargs: CoherentAllocation<GspArgumentsCached>,
}
+impl debugfs::BinaryWriter for LogBuffer {
+ fn write_to_slice(
+ &self,
+ writer: &mut kernel::uaccess::UserSliceWriter,
+ offset: &mut kernel::fs::file::Offset,
+ ) -> Result<usize> {
+ // SAFETY: This is a debug log buffer. GSP may write concurrently, so the
+ // snapshot may contain partially-written entries. This is acceptable for
+ // debugging purposes - users should be aware logs may be slightly garbled
+ // if read while GSP is actively logging.
+ let slice = unsafe { self.0.as_slice(0, self.0.count()) }?;
+
+ writer.write_slice_file(slice, offset)
+ }
+}
+
+// SAFETY: `LogBuffer` only provides shared access to the underlying `CoherentAllocation`.
+// GSP may write to the buffer concurrently regardless of CPU access, so concurrent reads
+// from multiple CPU threads do not introduce any additional races beyond what already
+// exists with the device. Reads may observe partially-written log entries, which is
+// acceptable for debug logging purposes.
+unsafe impl Sync for LogBuffer {}
+
impl Gsp {
// Creates an in-place initializer for a `Gsp` manager for `pdev`.
pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> Result<impl PinInit<Self, Error>> {
--
2.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 5/7] gpu: nova-core: use pin projection in method boot()
2025-12-12 20:49 [PATCH 0/7] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
` (3 preceding siblings ...)
2025-12-12 20:49 ` [PATCH 4/7] gpu: nova-core: implement BinaryWriter for LogBuffer Timur Tabi
@ 2025-12-12 20:49 ` Timur Tabi
2025-12-12 20:49 ` [PATCH 6/7] gpu: nova-core: create loginit debugfs entry Timur Tabi
` (2 subsequent siblings)
7 siblings, 0 replies; 25+ messages in thread
From: Timur Tabi @ 2025-12-12 20:49 UTC (permalink / raw)
To: Danilo Krummrich, Alexandre Courbot, John Hubbard, joelagnelf,
lyude, nouveau, rust-for-linux
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 | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 54937606b5b0..1db1099bd344 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -162,12 +162,13 @@ 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
- .send_command(bar, commands::SetSystemInfo::new(pdev))?;
- self.cmdq.send_command(bar, commands::SetRegistry::new())?;
+ let this = self.as_mut().project();
+
+ this.cmdq.send_command(bar, commands::SetSystemInfo::new(pdev))?;
+ 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),
@@ -234,13 +235,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)?;
dev_info!(
pdev.as_ref(),
"GPU name: {}\n",
--
2.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 6/7] gpu: nova-core: create loginit debugfs entry
2025-12-12 20:49 [PATCH 0/7] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
` (4 preceding siblings ...)
2025-12-12 20:49 ` [PATCH 5/7] gpu: nova-core: use pin projection in method boot() Timur Tabi
@ 2025-12-12 20:49 ` Timur Tabi
2025-12-15 16:45 ` Timur Tabi
2025-12-12 20:49 ` [PATCH 7/7] gpu: nova-core: create GSP-RM logging buffers debugfs entries Timur Tabi
2025-12-13 19:19 ` [PATCH 0/7] gpu: nova-core: expose the logging buffers via debugfs Joel Fernandes
7 siblings, 1 reply; 25+ messages in thread
From: Timur Tabi @ 2025-12-12 20:49 UTC (permalink / raw)
To: Danilo Krummrich, Alexandre Courbot, John Hubbard, joelagnelf,
lyude, nouveau, rust-for-linux
From: Alexandre Courbot <acourbot@nvidia.com>
Use the `pin_init_scope` feature to create the debugfs entry for
loginit.
`pin_init_scope` solves the lifetime issue over the `DEBUGFS_ROOT`
reference by delaying its acquisition until the time the entry is
actually initialized.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
drivers/gpu/nova-core/gsp.rs | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index 860674dac31e..ba4f789d8ac1 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -107,7 +107,8 @@ pub(crate) struct Gsp {
/// Libos arguments.
pub(crate) libos: CoherentAllocation<LibosMemoryRegionInitArgument>,
/// Init log buffer.
- loginit: LogBuffer,
+ #[pin]
+ pub loginit: debugfs::File<LogBuffer>,
/// Interrupts log buffer.
logintr: LogBuffer,
/// RM log buffer.
@@ -143,7 +144,9 @@ unsafe impl Sync for LogBuffer {}
impl Gsp {
// Creates an in-place initializer for a `Gsp` manager for `pdev`.
- pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> Result<impl PinInit<Self, Error>> {
+ pub(crate) fn new<'a>(
+ pdev: &'a pci::Device<device::Bound>,
+ ) -> Result<impl PinInit<Self, Error> + 'a> {
let dev = pdev.as_ref();
let libos = CoherentAllocation::<LibosMemoryRegionInitArgument>::alloc_coherent(
dev,
@@ -173,9 +176,17 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> Result<impl PinInit<Self
dma_write!(rmargs[0] = fw::GspArgumentsCached::new(&cmdq))?;
dma_write!(libos[3] = LibosMemoryRegionInitArgument::new("RMARGS", &rmargs))?;
+ #[allow(static_mut_refs)]
+ let debugfs_dir =
+ // SAFETY: `DEBUGFS_ROOT` is never modified after initialization, so it is safe to
+ // create a shared reference to it.
+ unsafe { crate::DEBUGFS_ROOT.as_ref() }
+ .map(|root| root.subdir(pdev.name()))
+ .ok_or(ENOENT)?;
+
Ok(try_pin_init!(Self {
libos,
- loginit,
+ loginit <- debugfs_dir.read_binary_file(kernel::c_str!("loginit"), loginit),
logintr,
logrm,
rmargs,
--
2.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 6/7] gpu: nova-core: create loginit debugfs entry
2025-12-12 20:49 ` [PATCH 6/7] gpu: nova-core: create loginit debugfs entry Timur Tabi
@ 2025-12-15 16:45 ` Timur Tabi
0 siblings, 0 replies; 25+ messages in thread
From: Timur Tabi @ 2025-12-15 16:45 UTC (permalink / raw)
To: nouveau@lists.freedesktop.org, Alexandre Courbot, dakr@kernel.org,
lyude@redhat.com, Joel Fernandes, John Hubbard,
rust-for-linux@vger.kernel.org
On Fri, 2025-12-12 at 14:49 -0600, Timur Tabi wrote:
> From: Alexandre Courbot <acourbot@nvidia.com>
>
> Use the `pin_init_scope` feature to create the debugfs entry for
> loginit.
>
> `pin_init_scope` solves the lifetime issue over the `DEBUGFS_ROOT`
> reference by delaying its acquisition until the time the entry is
> actually initialized.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
Sorry, this patch is not supposed to be here. It should have been merged into patch 7/7.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 7/7] gpu: nova-core: create GSP-RM logging buffers debugfs entries
2025-12-12 20:49 [PATCH 0/7] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
` (5 preceding siblings ...)
2025-12-12 20:49 ` [PATCH 6/7] gpu: nova-core: create loginit debugfs entry Timur Tabi
@ 2025-12-12 20:49 ` Timur Tabi
2025-12-13 23:44 ` Joel Fernandes
2025-12-13 19:19 ` [PATCH 0/7] gpu: nova-core: expose the logging buffers via debugfs Joel Fernandes
7 siblings, 1 reply; 25+ messages in thread
From: Timur Tabi @ 2025-12-12 20:49 UTC (permalink / raw)
To: Danilo Krummrich, Alexandre Courbot, John Hubbard, joelagnelf,
lyude, nouveau, rust-for-linux
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: Timur Tabi <ttabi@nvidia.com>
---
drivers/gpu/nova-core/gsp.rs | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index ba4f789d8ac1..2267ec3391dd 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -108,11 +108,13 @@ pub(crate) struct Gsp {
pub(crate) libos: CoherentAllocation<LibosMemoryRegionInitArgument>,
/// Init log buffer.
#[pin]
- pub loginit: debugfs::File<LogBuffer>,
+ loginit: debugfs::File<LogBuffer>,
/// Interrupts log buffer.
- logintr: LogBuffer,
+ #[pin]
+ logintr: debugfs::File<LogBuffer>,
/// RM log buffer.
- logrm: LogBuffer,
+ #[pin]
+ logrm: debugfs::File<LogBuffer>,
/// Command queue.
pub(crate) cmdq: Cmdq,
/// RM arguments.
@@ -177,18 +179,17 @@ pub(crate) fn new<'a>(
dma_write!(libos[3] = LibosMemoryRegionInitArgument::new("RMARGS", &rmargs))?;
#[allow(static_mut_refs)]
- let debugfs_dir =
- // SAFETY: `DEBUGFS_ROOT` is never modified after initialization, so it is safe to
- // create a shared reference to it.
- unsafe { crate::DEBUGFS_ROOT.as_ref() }
- .map(|root| root.subdir(pdev.name()))
- .ok_or(ENOENT)?;
+ // SAFETY: `DEBUGFS_ROOT` is never modified after initialization, so it is safe to
+ // create a shared reference to it.
+ let novacore_dir = unsafe { crate::DEBUGFS_ROOT.as_ref() }.ok_or(ENOENT)?;
+
+ let root = novacore_dir.subdir(pdev.name());
Ok(try_pin_init!(Self {
libos,
- loginit <- debugfs_dir.read_binary_file(kernel::c_str!("loginit"), loginit),
- logintr,
- logrm,
+ loginit <- root.read_binary_file(kernel::c_str!("loginit"), loginit),
+ logintr <- root.read_binary_file(kernel::c_str!("logintr"), logintr),
+ logrm <- root.read_binary_file(kernel::c_str!("logrm"), logrm),
rmargs,
cmdq,
}))
--
2.52.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 7/7] gpu: nova-core: create GSP-RM logging buffers debugfs entries
2025-12-12 20:49 ` [PATCH 7/7] gpu: nova-core: create GSP-RM logging buffers debugfs entries Timur Tabi
@ 2025-12-13 23:44 ` Joel Fernandes
0 siblings, 0 replies; 25+ messages in thread
From: Joel Fernandes @ 2025-12-13 23:44 UTC (permalink / raw)
To: Timur Tabi
Cc: Danilo Krummrich, Alexandre Courbot, John Hubbard,
lyude@redhat.com, nouveau@lists.freedesktop.org,
rust-for-linux@vger.kernel.org
> On Dec 13, 2025, at 6:00 AM, Timur Tabi <ttabi@nvidia.com> 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>
Co-developed tag needs a sign off tag with it from the developer.
- Joel
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
> drivers/gpu/nova-core/gsp.rs | 25 +++++++++++++------------
> 1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
> index ba4f789d8ac1..2267ec3391dd 100644
> --- a/drivers/gpu/nova-core/gsp.rs
> +++ b/drivers/gpu/nova-core/gsp.rs
> @@ -108,11 +108,13 @@ pub(crate) struct Gsp {
> pub(crate) libos: CoherentAllocation<LibosMemoryRegionInitArgument>,
> /// Init log buffer.
> #[pin]
> - pub loginit: debugfs::File<LogBuffer>,
> + loginit: debugfs::File<LogBuffer>,
> /// Interrupts log buffer.
> - logintr: LogBuffer,
> + #[pin]
> + logintr: debugfs::File<LogBuffer>,
> /// RM log buffer.
> - logrm: LogBuffer,
> + #[pin]
> + logrm: debugfs::File<LogBuffer>,
> /// Command queue.
> pub(crate) cmdq: Cmdq,
> /// RM arguments.
> @@ -177,18 +179,17 @@ pub(crate) fn new<'a>(
> dma_write!(libos[3] = LibosMemoryRegionInitArgument::new("RMARGS", &rmargs))?;
>
> #[allow(static_mut_refs)]
> - let debugfs_dir =
> - // SAFETY: `DEBUGFS_ROOT` is never modified after initialization, so it is safe to
> - // create a shared reference to it.
> - unsafe { crate::DEBUGFS_ROOT.as_ref() }
> - .map(|root| root.subdir(pdev.name()))
> - .ok_or(ENOENT)?;
> + // SAFETY: `DEBUGFS_ROOT` is never modified after initialization, so it is safe to
> + // create a shared reference to it.
> + let novacore_dir = unsafe { crate::DEBUGFS_ROOT.as_ref() }.ok_or(ENOENT)?;
> +
> + let root = novacore_dir.subdir(pdev.name());
>
> Ok(try_pin_init!(Self {
> libos,
> - loginit <- debugfs_dir.read_binary_file(kernel::c_str!("loginit"), loginit),
> - logintr,
> - logrm,
> + loginit <- root.read_binary_file(kernel::c_str!("loginit"), loginit),
> + logintr <- root.read_binary_file(kernel::c_str!("logintr"), logintr),
> + logrm <- root.read_binary_file(kernel::c_str!("logrm"), logrm),
> rmargs,
> cmdq,
> }))
> --
> 2.52.0
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/7] gpu: nova-core: expose the logging buffers via debugfs
2025-12-12 20:49 [PATCH 0/7] gpu: nova-core: expose the logging buffers via debugfs Timur Tabi
` (6 preceding siblings ...)
2025-12-12 20:49 ` [PATCH 7/7] gpu: nova-core: create GSP-RM logging buffers debugfs entries Timur Tabi
@ 2025-12-13 19:19 ` Joel Fernandes
2025-12-13 20:47 ` Timur Tabi
7 siblings, 1 reply; 25+ messages in thread
From: Joel Fernandes @ 2025-12-13 19:19 UTC (permalink / raw)
To: Timur Tabi
Cc: Danilo Krummrich, Alexandre Courbot, John Hubbard,
lyude@redhat.com, nouveau@lists.freedesktop.org,
rust-for-linux@vger.kernel.org
> On Dec 13, 2025, at 6:00 AM, Timur Tabi <ttabi@nvidia.com> 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.
>
> 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.
>
> Summary of changes:
>
> 1. Replace module_pci_driver! with explicit init function.
> 2. Creates root, per-gpu, and individual buffer debugfs entries.
> 3. Adds a pci::name() method to generate a PCI device name string.
>
> Alexandre Courbot (2):
> gpu: nova-core: implement BinaryWriter for LogBuffer
> gpu: nova-core: create loginit debugfs entry
>
> Timur Tabi (5):
> rust: pci: add PCI device name method
> gpu: nova-core: Replace module_pci_driver! with explicit module init
> gpu: nova-core: create debugfs root in PCI init closure
> gpu: nova-core: use pin projection in method boot()
> gpu: nova-core: create GSP-RM logging buffers debugfs entries
>
> drivers/gpu/nova-core/gsp.rs | 50 +++++++++++++++++++++++++-----
> drivers/gpu/nova-core/gsp/boot.rs | 15 ++++-----
> drivers/gpu/nova-core/nova_core.rs | 41 ++++++++++++++++++++++--
> rust/helpers/pci.c | 5 +++
> rust/kernel/pci.rs | 37 ++++++++++++++++++++++
> 5 files changed, 131 insertions(+), 17 deletions(-)
>
>
> base-commit: 187d0801404f415f22c0b31531982c7ea97fa341
I could not find this base commit myself in any branch. Without any mention of a git tree in the cover letter, how do we know which tree or branch this applies against? I am new to the base-commit flow hence asking.
Thanks.
> --
> 2.52.0
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 0/7] gpu: nova-core: expose the logging buffers via debugfs
2025-12-13 19:19 ` [PATCH 0/7] gpu: nova-core: expose the logging buffers via debugfs Joel Fernandes
@ 2025-12-13 20:47 ` Timur Tabi
2025-12-13 21:49 ` Joel Fernandes
0 siblings, 1 reply; 25+ messages in thread
From: Timur Tabi @ 2025-12-13 20:47 UTC (permalink / raw)
To: Joel Fernandes
Cc: nouveau@lists.freedesktop.org, Alexandre Courbot, dakr@kernel.org,
lyude@redhat.com, John Hubbard, rust-for-linux@vger.kernel.org
On Sat, 2025-12-13 at 19:19 +0000, Joel Fernandes wrote:
> > base-commit: 187d0801404f415f22c0b31531982c7ea97fa341
>
> I could not find this base commit myself in any branch. Without any mention of a git tree in
> the cover letter, how do we know which tree or branch this applies against? I am new to the
> base-commit flow hence asking.
That's in Linus' tree, where the debugfs patches from Danilo live.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?qt=range&q=187d0801404f415f22c0b31531982c7ea97fa341
drm-rust-next needs to rebase on top of it.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/7] gpu: nova-core: expose the logging buffers via debugfs
2025-12-13 20:47 ` Timur Tabi
@ 2025-12-13 21:49 ` Joel Fernandes
0 siblings, 0 replies; 25+ messages in thread
From: Joel Fernandes @ 2025-12-13 21:49 UTC (permalink / raw)
To: Timur Tabi
Cc: nouveau@lists.freedesktop.org, Alexandre Courbot, dakr@kernel.org,
lyude@redhat.com, John Hubbard, rust-for-linux@vger.kernel.org
> On Dec 14, 2025, at 5:47 AM, Timur Tabi <ttabi@nvidia.com> wrote:
>
> On Sat, 2025-12-13 at 19:19 +0000, Joel Fernandes wrote:
>>> base-commit: 187d0801404f415f22c0b31531982c7ea97fa341
>>
>> I could not find this base commit myself in any branch. Without any mention of a git tree in
>> the cover letter, how do we know which tree or branch this applies against? I am new to the
>> base-commit flow hence asking.
>
> That's in Linus' tree, where the debugfs patches from Danilo live.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/log/?qt=range&q=187d0801404f415f22c0b31531982c7ea97fa341
>
> drm-rust-next needs to rebase on top of it.
Got it. FWIW, I think b4 does not know which tree the base commit comes from, so unless this deviates from what is the tree used for the project, it may be best to either provide a link to a git tree with all the patches, or mention the base git tree used for the patches. At least that is what I will do.
Thanks.
^ permalink raw reply [flat|nested] 25+ messages in thread