* Claude review: gpu: nova-core: export Rust symbols for dependent modules
2026-04-30 14:55 ` [PATCH 4/7] gpu: nova-core: export Rust symbols for dependent modules Alexandre Courbot
@ 2026-05-05 0:17 ` Claude Code Review Bot
0 siblings, 0 replies; 20+ messages in thread
From: Claude Code Review Bot @ 2026-05-05 0:17 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Verdict: Functional workaround, a few observations.**
The approach — using `nm` to find Rust text symbols (prefixed `_R`) and wrapping them with `EXPORT_SYMBOL_GPL` — is pragmatic:
```makefile
rust_exports = \
$(NM) -p --defined-only $(1) | \
awk '$$2 == "T" && $$3 ~ /^_R/ { \
printf "extern void %s(void); EXPORT_SYMBOL_GPL(%s);\n", $$3, $$3 \
}'
```
Observations:
1. **Over-export risk**: This exports *every* `_R`-prefixed global text symbol in `nova_core.o`. Rust's v0 mangling means that includes all `pub` functions that weren't inlined or elided, including potentially internal helpers, trait impls, drop glue, etc. that consumers shouldn't rely on. The comment should acknowledge this blanket export is intentional and that narrowing it is a future improvement. For a temporary workaround this is acceptable, but if this persists it could create an unstable ABI surface.
2. **Symbol type filter**: Only `T` (global text) symbols are exported. This misses `D`/`B` (global data/BSS) symbols which could also be needed if `nova-core` exposes `pub static` items. For the current POC this works, but worth noting.
3. The `nova_core_exports.c` file is clean and minimal — just includes `<linux/export.h>` and the generated header. The `extern void` declarations for non-void functions are technically wrong C, but for the purpose of `EXPORT_SYMBOL_GPL` it only needs the symbol name, not the actual prototype. This is fine.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 0/7] gpu: drm: nova: enable calling into nova-core
@ 2026-05-27 11:52 Alexandre Courbot
2026-05-27 11:52 ` [PATCH v2 1/7] scripts: modpost: detect and report truncated buf_printf() output Alexandre Courbot
` (7 more replies)
0 siblings, 8 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-05-27 11:52 UTC (permalink / raw)
To: Miguel Ojeda, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kbuild, linux-kernel, rust-for-linux,
nova-gpu, dri-devel, Alexandre Courbot, Joel Fernandes
`nova-drm` is scheduled to expose a user-space API to receive IOCTLs
from user-mode drivers, and to call into `nova-core` to perform the
actual work. We are about to reach the state where we need the ability
to call into `nova-core`, but the current Rust build system does not
currently support this, and the solution will likely take at least a
couple of cycles to be merged.
In the meantime, this series introduces a Nova-local workaround for
`nova-drm` to call into `nova-core`. It generates the `nova-core`
metadata that `nova-drm` can use to resolve references at build-time,
and also builds a list of exported symbols for symbol resolution when
modules are loaded.
Since Rust symbols are long, this work ran into the limits on symbol
sizes `modpost` can handle; however the overflow path was not properly
handled, which resulted in late failures. The first patch makes
`modpost` fail on symbols that are too long for its internal buffer and
report them. It is not strictly necessary for this series and can be
merged independently if preferred.
The second and third patches instruct the compiler to inline some
methods of `pin_init` and Rust basic types to avoid those long symbol
names. Interestingly, this also results in a smaller nova-core binary
size [1].
The rest of the patches enable inter-module calls from nova-drm to
nova-core.
This series is based on `drm-rust-next` with [2] and [3] applied.
[1] https://lore.kernel.org/all/DIN76NTFEU1N.1RT6G4IFD62RG@nvidia.com/
[2] https://lore.kernel.org/all/20260525202921.124698-1-dakr@kernel.org/
[3] https://lore.kernel.org/all/20260525225838.276108-1-dakr@kernel.org/
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Changes in v2:
- Rebase on top of HRT v5.
- Inline some `pin_init` and Rust basic types methods to avoid long
symbol names and optimize code.
- Print truncating modpost symbols and abort upon meeting them.
- Drop increase of `buf_printf`'s buffer.
- Drop obsolete nova-core renaming patch.
- Link to v1: https://patch.msgid.link/20260430-nova-exports-v1-0-7ca31664e983@nvidia.com
---
Alexandre Courbot (7):
scripts: modpost: detect and report truncated buf_printf() output
pin_init: always inline the #ident and #project_ident methods
rust: always inline some init methods
gpu: nova-core: export Rust symbols for dependent modules
gpu: nova-core: emit Rust metadata for dependent modules
gpu: drm: nova: build after nova-core metadata
[POC] drm: nova: demonstrate interaction with nova-core
drivers/gpu/drm/nova/Makefile | 15 ++++++++
drivers/gpu/drm/nova/driver.rs | 9 ++++-
drivers/gpu/nova-core/Makefile | 46 +++++++++++++++++++++++-
drivers/gpu/nova-core/driver.rs | 59 +++++++++++++++++++++++--------
drivers/gpu/nova-core/gpu.rs | 9 +++--
drivers/gpu/nova-core/nova_core.rs | 4 +--
drivers/gpu/nova-core/nova_core_exports.c | 13 +++++++
rust/kernel/alloc/kbox.rs | 2 ++
rust/kernel/init.rs | 1 +
rust/kernel/sync/arc.rs | 2 ++
rust/kernel/types.rs | 3 ++
rust/pin-init/internal/src/pin_data.rs | 2 ++
scripts/mod/modpost.c | 11 +++++-
13 files changed, 154 insertions(+), 22 deletions(-)
---
base-commit: 79c29e27d676a74f9238506a5fe2134637356708
change-id: 20260430-nova-exports-502f996c5aab
Best regards,
--
Alexandre Courbot <acourbot@nvidia.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/7] scripts: modpost: detect and report truncated buf_printf() output
2026-05-27 11:52 [PATCH v2 0/7] gpu: drm: nova: enable calling into nova-core Alexandre Courbot
@ 2026-05-27 11:52 ` Alexandre Courbot
2026-05-27 17:18 ` Nathan Chancellor
2026-05-28 2:21 ` Claude review: " Claude Code Review Bot
2026-05-27 11:52 ` [PATCH v2 2/7] pin_init: always inline the #ident and #project_ident methods Alexandre Courbot
` (6 subsequent siblings)
7 siblings, 2 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-05-27 11:52 UTC (permalink / raw)
To: Miguel Ojeda, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kbuild, linux-kernel, rust-for-linux,
nova-gpu, dri-devel, Alexandre Courbot
buf_printf() uses a fixed-size stack buffer. vsnprintf() returns the
number of bytes that *would* have been written to that buffer, which can
be larger than the size of said buffer if the formatted string is too
long.
The problem is that whenever this happens buf_printf() currently passes
this length, unchecked, to buf_write(), which silently reads past the
stack buffer and copies invalid data into the output buffer.
Fix this by detecting vsnprintf() failures and truncations before
appending to the output buffer, and report a fatal error instead of
producing corrupt symbol names.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
scripts/mod/modpost.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index abbcd3fc1394..0d2f1f09019b 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1689,8 +1689,17 @@ void __attribute__((format(printf, 2, 3))) buf_printf(struct buffer *buf,
va_start(ap, fmt);
len = vsnprintf(tmp, SZ, fmt, ap);
- buf_write(buf, tmp, len);
va_end(ap);
+
+ if (len < 0) {
+ perror("vsnprintf failed");
+ exit(1);
+ }
+ if (len >= SZ)
+ fatal("buf_printf output truncated for string %s: %d bytes needed, %d available\n",
+ tmp, len + 1, SZ);
+
+ buf_write(buf, tmp, len);
}
void buf_write(struct buffer *buf, const char *s, int len)
--
2.54.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/7] pin_init: always inline the #ident and #project_ident methods
2026-05-27 11:52 [PATCH v2 0/7] gpu: drm: nova: enable calling into nova-core Alexandre Courbot
2026-05-27 11:52 ` [PATCH v2 1/7] scripts: modpost: detect and report truncated buf_printf() output Alexandre Courbot
@ 2026-05-27 11:52 ` Alexandre Courbot
2026-05-27 17:49 ` Gary Guo
2026-05-28 2:22 ` Claude review: " Claude Code Review Bot
2026-05-27 11:52 ` [PATCH v2 3/7] rust: always inline some init methods Alexandre Courbot
` (5 subsequent siblings)
7 siblings, 2 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-05-27 11:52 UTC (permalink / raw)
To: Miguel Ojeda, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kbuild, linux-kernel, rust-for-linux,
nova-gpu, dri-devel, Alexandre Courbot
These methods should always be inlined for optimization reasons. Failure
to do so can also produce symbol names larger than what `modpost`
supports.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
rust/pin-init/internal/src/pin_data.rs | 2 ++
1 file changed, 2 insertions(+)
diff --git a/rust/pin-init/internal/src/pin_data.rs b/rust/pin-init/internal/src/pin_data.rs
index 7d871236b49c..945254740f61 100644
--- a/rust/pin-init/internal/src/pin_data.rs
+++ b/rust/pin-init/internal/src/pin_data.rs
@@ -395,6 +395,7 @@ fn handle_field(
/// to deallocate.
#pin_safety
#(#attrs)*
+ #[inline(always)]
#vis unsafe fn #ident<E>(
self,
slot: *mut #ty,
@@ -409,6 +410,7 @@ fn handle_field(
///
#[doc = #slot_safety]
#(#attrs)*
+ #[inline(always)]
#vis unsafe fn #project_ident<'__slot>(
self,
slot: &'__slot mut #ty,
--
2.54.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 3/7] rust: always inline some init methods
2026-05-27 11:52 [PATCH v2 0/7] gpu: drm: nova: enable calling into nova-core Alexandre Courbot
2026-05-27 11:52 ` [PATCH v2 1/7] scripts: modpost: detect and report truncated buf_printf() output Alexandre Courbot
2026-05-27 11:52 ` [PATCH v2 2/7] pin_init: always inline the #ident and #project_ident methods Alexandre Courbot
@ 2026-05-27 11:52 ` Alexandre Courbot
2026-05-27 17:54 ` Gary Guo
2026-05-28 2:22 ` Claude review: " Claude Code Review Bot
2026-05-27 11:52 ` [PATCH v2 4/7] gpu: nova-core: export Rust symbols for dependent modules Alexandre Courbot
` (4 subsequent siblings)
7 siblings, 2 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-05-27 11:52 UTC (permalink / raw)
To: Miguel Ojeda, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kbuild, linux-kernel, rust-for-linux,
nova-gpu, dri-devel, Alexandre Courbot
These methods should always be inlined for optimization reasons. Failure
to do so can also produce symbol names larger than what `modpost`
supports.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
rust/kernel/alloc/kbox.rs | 2 ++
rust/kernel/init.rs | 1 +
rust/kernel/sync/arc.rs | 2 ++
rust/kernel/types.rs | 3 +++
4 files changed, 8 insertions(+)
diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
index 2f8c16473c2c..592fffc72e52 100644
--- a/rust/kernel/alloc/kbox.rs
+++ b/rust/kernel/alloc/kbox.rs
@@ -433,6 +433,7 @@ impl<T, A> InPlaceWrite<T> for Box<MaybeUninit<T>, A>
{
type Initialized = Box<T, A>;
+ #[inline(always)]
fn write_init<E>(mut self, init: impl Init<T, E>) -> Result<Self::Initialized, E> {
let slot = self.as_mut_ptr();
// SAFETY: When init errors/panics, slot will get deallocated but not dropped,
@@ -442,6 +443,7 @@ fn write_init<E>(mut self, init: impl Init<T, E>) -> Result<Self::Initialized, E
Ok(unsafe { Box::assume_init(self) })
}
+ #[inline(always)]
fn write_pin_init<E>(mut self, init: impl PinInit<T, E>) -> Result<Pin<Self::Initialized>, E> {
let slot = self.as_mut_ptr();
// SAFETY: When init errors/panics, slot will get deallocated but not dropped,
diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index 7a0d4559d7b5..36d4bf1959fb 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -151,6 +151,7 @@ fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Self::Pinne
/// type.
///
/// If `T: !Unpin` it will not be able to move afterwards.
+ #[inline(always)]
fn pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> error::Result<Self::PinnedSelf>
where
Error: From<E>,
diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 18d6c0d62ce0..973f15165606 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -712,6 +712,7 @@ fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>
impl<T> InPlaceWrite<T> for UniqueArc<MaybeUninit<T>> {
type Initialized = UniqueArc<T>;
+ #[inline(always)]
fn write_init<E>(mut self, init: impl Init<T, E>) -> Result<Self::Initialized, E> {
let slot = self.as_mut_ptr();
// SAFETY: When init errors/panics, slot will get deallocated but not dropped,
@@ -721,6 +722,7 @@ fn write_init<E>(mut self, init: impl Init<T, E>) -> Result<Self::Initialized, E
Ok(unsafe { self.assume_init() })
}
+ #[inline(always)]
fn write_pin_init<E>(mut self, init: impl PinInit<T, E>) -> Result<Pin<Self::Initialized>, E> {
let slot = self.as_mut_ptr();
// SAFETY: When init errors/panics, slot will get deallocated but not dropped,
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index ac316fd7b538..381a06ee0d59 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -367,6 +367,7 @@ pub const fn zeroed() -> Self {
/// This function is safe, because the `T` inside of an `Opaque` is allowed to be
/// uninitialized. Additionally, access to the inner `T` requires `unsafe`, so the caller needs
/// to verify at that point that the inner value is valid.
+ #[inline(always)]
pub fn ffi_init(init_func: impl FnOnce(*mut T)) -> impl PinInit<Self> {
// SAFETY: We contain a `MaybeUninit`, so it is OK for the `init_func` to not fully
// initialize the `T`.
@@ -386,6 +387,7 @@ pub fn ffi_init(init_func: impl FnOnce(*mut T)) -> impl PinInit<Self> {
/// This function is safe, because the `T` inside of an `Opaque` is allowed to be
/// uninitialized. Additionally, access to the inner `T` requires `unsafe`, so the caller needs
/// to verify at that point that the inner value is valid.
+ #[inline(always)]
pub fn try_ffi_init<E>(
init_func: impl FnOnce(*mut T) -> Result<(), E>,
) -> impl PinInit<Self, E> {
@@ -417,6 +419,7 @@ pub const fn cast_from(this: *const T) -> *const Self {
impl<T> Wrapper<T> for Opaque<T> {
/// Create an opaque pin-initializer from the given pin-initializer.
+ #[inline(always)]
fn pin_init<E>(slot: impl PinInit<T, E>) -> impl PinInit<Self, E> {
Self::try_ffi_init(|ptr: *mut T| {
// SAFETY:
--
2.54.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 4/7] gpu: nova-core: export Rust symbols for dependent modules
2026-05-27 11:52 [PATCH v2 0/7] gpu: drm: nova: enable calling into nova-core Alexandre Courbot
` (2 preceding siblings ...)
2026-05-27 11:52 ` [PATCH v2 3/7] rust: always inline some init methods Alexandre Courbot
@ 2026-05-27 11:52 ` Alexandre Courbot
2026-05-28 2:22 ` Claude review: " Claude Code Review Bot
2026-05-27 11:52 ` [PATCH v2 5/7] gpu: nova-core: emit Rust metadata " Alexandre Courbot
` (3 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Alexandre Courbot @ 2026-05-27 11:52 UTC (permalink / raw)
To: Miguel Ojeda, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kbuild, linux-kernel, rust-for-linux,
nova-gpu, dri-devel, Alexandre Courbot, Joel Fernandes
Export `nova-core`'s Rust symbols so that other loadable modules,
particularly `nova-drm`, can resolve references to `nova-core` at
runtime.
This is done by generating declarations and `EXPORT_SYMBOL_GPL()` calls
for Rust global text symbols using `nm` and compiling them into the
module as `nova_core_exports.o`.
This is a workaround until the build system supports Rust cross-crate
dependencies natively.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
---
drivers/gpu/nova-core/Makefile | 22 +++++++++++++++++++++-
drivers/gpu/nova-core/nova_core_exports.c | 13 +++++++++++++
2 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/nova-core/Makefile b/drivers/gpu/nova-core/Makefile
index 4ae544f808f4..d31bfb04921f 100644
--- a/drivers/gpu/nova-core/Makefile
+++ b/drivers/gpu/nova-core/Makefile
@@ -1,4 +1,24 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_NOVA_CORE) += nova-core.o
-nova-core-y := nova_core.o
+nova-core-y := nova_core.o nova_core_exports.o
+
+# Export Rust symbols so dependent modules can use them at runtime.
+#
+# This is a workaround until the build system supports Rust cross-module
+# dependencies natively.
+# Kbuild uses Rust v0 mangling, whose symbols start with "_R".
+rust_exports = \
+ $(NM) -p --defined-only $(1) | \
+ awk '$$2 == "T" && $$3 ~ /^_R/ { \
+ printf "extern void %s(void); EXPORT_SYMBOL_GPL(%s);\n", $$3, $$3 \
+ }'
+
+define filechk_nova_core_exports
+ $(call rust_exports,$(obj)/nova_core.o)
+endef
+
+$(obj)/exports_nova_core_generated.h: $(obj)/nova_core.o FORCE
+ $(call filechk,nova_core_exports)
+
+$(obj)/nova_core_exports.o: $(obj)/exports_nova_core_generated.h
diff --git a/drivers/gpu/nova-core/nova_core_exports.c b/drivers/gpu/nova-core/nova_core_exports.c
new file mode 100644
index 000000000000..480472b039db
--- /dev/null
+++ b/drivers/gpu/nova-core/nova_core_exports.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+
+/*
+ * Exports Rust symbols from the `nova_core` crate for use by dependent modules.
+ *
+ * This is a workaround until the build system supports Rust cross-module
+ * dependencies natively.
+ */
+
+#include <linux/export.h>
+
+#include "exports_nova_core_generated.h"
--
2.54.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 5/7] gpu: nova-core: emit Rust metadata for dependent modules
2026-05-27 11:52 [PATCH v2 0/7] gpu: drm: nova: enable calling into nova-core Alexandre Courbot
` (3 preceding siblings ...)
2026-05-27 11:52 ` [PATCH v2 4/7] gpu: nova-core: export Rust symbols for dependent modules Alexandre Courbot
@ 2026-05-27 11:52 ` Alexandre Courbot
2026-05-28 2:22 ` Claude review: " Claude Code Review Bot
2026-05-27 11:52 ` [PATCH v2 6/7] gpu: drm: nova: build after nova-core metadata Alexandre Courbot
` (2 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Alexandre Courbot @ 2026-05-27 11:52 UTC (permalink / raw)
To: Miguel Ojeda, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kbuild, linux-kernel, rust-for-linux,
nova-gpu, dri-devel, Alexandre Courbot
Emit nova-core's `.rmeta` crate metadata so dependent Rust modules can
resolve nova-core's types and functions via `--extern`.
The metadata is exposed as an explicit Kbuild target instead of relying
on an untracked side effect of the object build. This lets dependent
modules ask for the metadata directly, including single-target builds
where `nova-core`'s object may already be up to date.
This is a workaround until the build system supports Rust cross-crate
dependencies natively.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/Makefile | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/gpu/nova-core/Makefile b/drivers/gpu/nova-core/Makefile
index d31bfb04921f..7e2c62b9a597 100644
--- a/drivers/gpu/nova-core/Makefile
+++ b/drivers/gpu/nova-core/Makefile
@@ -22,3 +22,27 @@ $(obj)/exports_nova_core_generated.h: $(obj)/nova_core.o FORCE
$(call filechk,nova_core_exports)
$(obj)/nova_core_exports.o: $(obj)/exports_nova_core_generated.h
+
+# Dependent Rust drivers need `nova-core`'s crate metadata.
+# Expose it as an explicit target so they can request it directly.
+nova_core_rmeta := libnova_core.rmeta
+nova_core_rmeta_private := .nova_core_rmeta/$(nova_core_rmeta)
+targets += $(nova_core_rmeta) $(nova_core_rmeta_private)
+
+# Build in a private directory to avoid racing with `nova_core.o` on rustc's
+# intermediate object names during parallel builds.
+quiet_cmd_rustc_rmeta_rs = RUSTC RMETA $@
+ cmd_rustc_rmeta_rs = mkdir -p $(dir $@); \
+ $(rust_common_cmd) --emit=metadata=$@ \
+ --emit=obj=$(basename $@).o $<; \
+ rm -f $(basename $@).o
+
+$(obj)/$(nova_core_rmeta_private): private __modname := nova-core
+$(obj)/$(nova_core_rmeta_private): private part-of-builtin := $(if $(filter y,$(CONFIG_NOVA_CORE)),y)
+$(obj)/$(nova_core_rmeta_private): private part-of-module := $(if $(filter m,$(CONFIG_NOVA_CORE)),y)
+$(obj)/$(nova_core_rmeta_private): $(obj)/nova_core.rs FORCE
+ +$(call if_changed_dep,rustc_rmeta_rs)
+
+# Keep the metadata available at a stable path for consumers.
+$(obj)/$(nova_core_rmeta): $(obj)/$(nova_core_rmeta_private) FORCE
+ $(call if_changed,copy)
--
2.54.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 6/7] gpu: drm: nova: build after nova-core metadata
2026-05-27 11:52 [PATCH v2 0/7] gpu: drm: nova: enable calling into nova-core Alexandre Courbot
` (4 preceding siblings ...)
2026-05-27 11:52 ` [PATCH v2 5/7] gpu: nova-core: emit Rust metadata " Alexandre Courbot
@ 2026-05-27 11:52 ` Alexandre Courbot
2026-05-28 2:22 ` Claude review: " Claude Code Review Bot
2026-05-27 11:52 ` [PATCH POC v2 7/7] drm: nova: demonstrate interaction with nova-core Alexandre Courbot
2026-05-28 2:21 ` Claude review: gpu: drm: nova: enable calling into nova-core Claude Code Review Bot
7 siblings, 1 reply; 20+ messages in thread
From: Alexandre Courbot @ 2026-05-27 11:52 UTC (permalink / raw)
To: Miguel Ojeda, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kbuild, linux-kernel, rust-for-linux,
nova-gpu, dri-devel, Alexandre Courbot
Point rustc to `nova-core`'s `.rmeta` file so `nova-drm` can use
`nova_core`'s types and functions at compile time.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/drm/nova/Makefile | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/nova/Makefile b/drivers/gpu/drm/nova/Makefile
index f8527b2b7b4a..6ed4ccbfbcdc 100644
--- a/drivers/gpu/drm/nova/Makefile
+++ b/drivers/gpu/drm/nova/Makefile
@@ -2,3 +2,18 @@
obj-$(CONFIG_DRM_NOVA) += nova-drm.o
nova-drm-y := nova.o
+
+nova_core_rmeta := drivers/gpu/nova-core/libnova_core.rmeta
+
+rustflags-y += --extern nova_core=$(objtree)/$(nova_core_rmeta)
+
+# `nova-drm` imports `nova-core` at compile time, so request `nova-core`'s
+# explicit metadata target before compiling nova.o.
+$(obj)/nova.o: | $(objtree)/$(nova_core_rmeta)
+
+# Build `nova-core`'s metadata by invoking make.
+#
+# This is ugly but only temporary until the build system natively supports
+# cross-crate dependencies.
+$(objtree)/$(nova_core_rmeta): FORCE
+ $(Q)$(MAKE) $(build)=drivers/gpu/nova-core $(nova_core_rmeta)
--
2.54.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH POC v2 7/7] drm: nova: demonstrate interaction with nova-core
2026-05-27 11:52 [PATCH v2 0/7] gpu: drm: nova: enable calling into nova-core Alexandre Courbot
` (5 preceding siblings ...)
2026-05-27 11:52 ` [PATCH v2 6/7] gpu: drm: nova: build after nova-core metadata Alexandre Courbot
@ 2026-05-27 11:52 ` Alexandre Courbot
2026-05-28 2:22 ` Claude review: " Claude Code Review Bot
2026-05-28 2:21 ` Claude review: gpu: drm: nova: enable calling into nova-core Claude Code Review Bot
7 siblings, 1 reply; 20+ messages in thread
From: Alexandre Courbot @ 2026-05-27 11:52 UTC (permalink / raw)
To: Miguel Ojeda, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kbuild, linux-kernel, rust-for-linux,
nova-gpu, dri-devel, Alexandre Courbot
Export a few items from nova-core and use them from nova-drm in order to
print the chipset of the GPU being probed.
Some documentation items are added to make Clippy happy.
This is only meant for demonstration purposes, and won't be merged.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/drm/nova/driver.rs | 9 +++++-
drivers/gpu/nova-core/driver.rs | 59 +++++++++++++++++++++++++++++---------
drivers/gpu/nova-core/gpu.rs | 9 ++++--
drivers/gpu/nova-core/nova_core.rs | 4 +--
4 files changed, 61 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index 4289df7de01c..2a3f47974079 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -9,12 +9,15 @@
ioctl, //
},
prelude::*,
- sync::aref::ARef, //
+ sync::aref::ARef,
+ types::ForLt, //
};
use crate::file::File;
use crate::gem::NovaObject;
+use nova_core::driver::AuxData;
+
pub(crate) struct NovaDriver;
pub(crate) struct Nova {
@@ -60,6 +63,10 @@ fn probe<'bound>(
adev: &'bound auxiliary::Device<Core<'_>>,
_info: &'bound Self::IdInfo,
) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {
+ let aux_data = adev.registration_data::<ForLt!(AuxData<'_>)>()?;
+
+ pr_info!("Chipset from nova-core: {}\n", aux_data.chipset());
+
let data = try_pin_init!(NovaData { adev: adev.into() });
let drm = drm::Device::<Self>::new(adev.as_ref(), data)?;
diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index cff5034c2dcd..49cb1c046905 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
+//! Main driver module.
+
use kernel::{
auxiliary,
device::Core,
@@ -20,18 +22,36 @@
types::ForLt,
};
-use crate::gpu::Gpu;
+use crate::gpu::{
+ Chipset,
+ Gpu, //
+};
/// Counter for generating unique auxiliary device IDs.
static AUXILIARY_ID_COUNTER: Atomic<u32> = Atomic::new(0);
+/// Data passed to the auxialiary device registration, for the sibling driver to use.
+pub struct AuxData<'bound> {
+ gpu: &'bound Gpu<'bound>,
+}
+
+impl AuxData<'_> {
+ /// Returns the chipset of this GPU.
+ pub fn chipset(&self) -> Chipset {
+ self.gpu.spec.chipset
+ }
+}
+
+/// Driver-associated data.
#[pin_data]
-pub(crate) struct NovaCore<'bound> {
+pub struct NovaCore<'bound> {
+ // Fields are dropped in declaration order: unregister the auxiliary device before dropping
+ // `gpu`, and drop `gpu` before `bar` because `AuxData` borrows `gpu` and `Gpu` borrows `bar`.
+ #[allow(clippy::type_complexity)]
+ _reg: auxiliary::Registration<'bound, ForLt!(AuxData<'_>)>,
#[pin]
pub(crate) gpu: Gpu<'bound>,
bar: pci::Bar<'bound, BAR0_SIZE>,
- #[allow(clippy::type_complexity)]
- _reg: auxiliary::Registration<'bound, ForLt!(())>,
}
pub(crate) struct NovaCoreDriver;
@@ -93,7 +113,7 @@ fn probe<'bound>(
// other threads of execution.
unsafe { pdev.dma_set_mask_and_coherent(DmaMask::new::<GPU_DMA_BITS>())? };
- Ok(try_pin_init!(NovaCore {
+ Ok(try_pin_init!(&this in NovaCore {
bar: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"nova-core/bar0")?,
// TODO: Use `&bar` self-referential pin-init syntax once available.
//
@@ -101,15 +121,26 @@ fn probe<'bound>(
// (`try_pin_init!()` initializes fields in declaration order), lives at a pinned
// stable address, and is dropped after `gpu` (struct field drop order).
gpu <- Gpu::new(pdev, unsafe { &*core::ptr::from_ref(bar) }),
- _reg: auxiliary::Registration::new(
- pdev.as_ref(),
- c"nova-drm",
- // TODO[XARR]: Use XArray or perhaps IDA for proper ID allocation/recycling. For
- // now, use a simple atomic counter that never recycles IDs.
- AUXILIARY_ID_COUNTER.fetch_add(1, Relaxed),
- crate::MODULE_NAME,
- (),
- )?,
+ // SAFETY: `NovaCore` is dropped when the device is unbound; i.e. `mem::forget()` is
+ // never called on it.
+ _reg: unsafe {
+ auxiliary::Registration::new_with_lt(
+ pdev.as_ref(),
+ c"nova-drm",
+ // TODO[XARR]: Use XArray or perhaps IDA for proper ID allocation/recycling.
+ // For now, use a simple atomic counter that never recycles IDs.
+ AUXILIARY_ID_COUNTER.fetch_add(1, Relaxed),
+ crate::MODULE_NAME,
+ AuxData {
+ // TODO: Use `&gpu` self-referential pin-init syntax once available.
+ //
+ // SAFETY: `this.gpu` is initialized before this expression is
+ // evaluated, lives at a pinned stable address, and is dropped after
+ // `_reg` (struct field drop order).
+ gpu: &*core::ptr::from_ref(&this.as_ref().gpu),
+ },
+ )?
+ },
}))
})
}
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index cf134cab49cd..5636659f24a8 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
+//! Core types for the driver.
+
use kernel::{
device,
fmt,
@@ -29,7 +31,8 @@ macro_rules! define_chipset {
{
/// Enum representation of the GPU chipset.
#[derive(fmt::Debug, Copy, Clone, PartialOrd, Ord, PartialEq, Eq)]
- pub(crate) enum Chipset {
+ #[allow(missing_docs)]
+ pub enum Chipset {
$($variant = $value),*,
}
@@ -183,7 +186,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
/// Structure holding a basic description of the GPU: `Chipset` and `Revision`.
#[derive(Clone, Copy)]
pub(crate) struct Spec {
- chipset: Chipset,
+ pub(crate) chipset: Chipset,
revision: Revision,
}
@@ -245,7 +248,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
/// Structure holding the resources required to operate the GPU.
#[pin_data]
pub(crate) struct Gpu<'gpu> {
- spec: Spec,
+ pub(crate) spec: Spec,
/// MMIO mapping of PCI BAR 0.
bar: &'gpu Bar0,
/// System memory page required for flushing all pending GPU-side memory writes done through
diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index 5a260062295f..5166e10ce8a0 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -13,11 +13,11 @@
#[macro_use]
mod bitfield;
-mod driver;
+pub mod driver;
mod falcon;
mod fb;
mod firmware;
-mod gpu;
+pub mod gpu;
mod gsp;
#[macro_use]
mod num;
--
2.54.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/7] scripts: modpost: detect and report truncated buf_printf() output
2026-05-27 11:52 ` [PATCH v2 1/7] scripts: modpost: detect and report truncated buf_printf() output Alexandre Courbot
@ 2026-05-27 17:18 ` Nathan Chancellor
2026-05-28 2:21 ` Claude review: " Claude Code Review Bot
1 sibling, 0 replies; 20+ messages in thread
From: Nathan Chancellor @ 2026-05-27 17:18 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Miguel Ojeda, Nicolas Schier, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, David Airlie, Simona Vetter,
John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kbuild, linux-kernel, rust-for-linux,
nova-gpu, dri-devel
On Wed, May 27, 2026 at 08:52:17PM +0900, Alexandre Courbot wrote:
> buf_printf() uses a fixed-size stack buffer. vsnprintf() returns the
> number of bytes that *would* have been written to that buffer, which can
> be larger than the size of said buffer if the formatted string is too
> long.
>
> The problem is that whenever this happens buf_printf() currently passes
> this length, unchecked, to buf_write(), which silently reads past the
> stack buffer and copies invalid data into the output buffer.
>
> Fix this by detecting vsnprintf() failures and truncations before
> appending to the output buffer, and report a fatal error instead of
> producing corrupt symbol names.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Acked-by: Nathan Chancellor <nathan@kernel.org>
> ---
> scripts/mod/modpost.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index abbcd3fc1394..0d2f1f09019b 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1689,8 +1689,17 @@ void __attribute__((format(printf, 2, 3))) buf_printf(struct buffer *buf,
>
> va_start(ap, fmt);
> len = vsnprintf(tmp, SZ, fmt, ap);
> - buf_write(buf, tmp, len);
> va_end(ap);
> +
> + if (len < 0) {
> + perror("vsnprintf failed");
> + exit(1);
> + }
> + if (len >= SZ)
> + fatal("buf_printf output truncated for string %s: %d bytes needed, %d available\n",
> + tmp, len + 1, SZ);
> +
> + buf_write(buf, tmp, len);
> }
>
> void buf_write(struct buffer *buf, const char *s, int len)
>
> --
> 2.54.0
>
--
Cheers,
Nathan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/7] pin_init: always inline the #ident and #project_ident methods
2026-05-27 11:52 ` [PATCH v2 2/7] pin_init: always inline the #ident and #project_ident methods Alexandre Courbot
@ 2026-05-27 17:49 ` Gary Guo
2026-05-28 2:22 ` Claude review: " Claude Code Review Bot
1 sibling, 0 replies; 20+ messages in thread
From: Gary Guo @ 2026-05-27 17:49 UTC (permalink / raw)
To: Alexandre Courbot, Miguel Ojeda, Nathan Chancellor,
Nicolas Schier, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kbuild, linux-kernel, rust-for-linux,
nova-gpu, dri-devel
On Wed May 27, 2026 at 12:52 PM BST, Alexandre Courbot wrote:
> These methods should always be inlined for optimization reasons. Failure
> to do so can also produce symbol names larger than what `modpost`
> supports.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> rust/pin-init/internal/src/pin_data.rs | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/rust/pin-init/internal/src/pin_data.rs b/rust/pin-init/internal/src/pin_data.rs
> index 7d871236b49c..945254740f61 100644
> --- a/rust/pin-init/internal/src/pin_data.rs
> +++ b/rust/pin-init/internal/src/pin_data.rs
> @@ -395,6 +395,7 @@ fn handle_field(
> /// to deallocate.
> #pin_safety
> #(#attrs)*
> + #[inline(always)]
> #vis unsafe fn #ident<E>(
> self,
> slot: *mut #ty,
> @@ -409,6 +410,7 @@ fn handle_field(
> ///
> #[doc = #slot_safety]
> #(#attrs)*
> + #[inline(always)]
> #vis unsafe fn #project_ident<'__slot>(
> self,
> slot: &'__slot mut #ty,
This code doesn't exist anymore on pin-init-next.
Best,
Gary
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/7] rust: always inline some init methods
2026-05-27 11:52 ` [PATCH v2 3/7] rust: always inline some init methods Alexandre Courbot
@ 2026-05-27 17:54 ` Gary Guo
2026-05-28 2:22 ` Claude review: " Claude Code Review Bot
1 sibling, 0 replies; 20+ messages in thread
From: Gary Guo @ 2026-05-27 17:54 UTC (permalink / raw)
To: Alexandre Courbot, Miguel Ojeda, Nathan Chancellor,
Nicolas Schier, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, Zhi Wang,
Eliot Courtney, linux-kbuild, linux-kernel, rust-for-linux,
nova-gpu, dri-devel
On Wed May 27, 2026 at 12:52 PM BST, Alexandre Courbot wrote:
> These methods should always be inlined for optimization reasons. Failure
> to do so can also produce symbol names larger than what `modpost`
> supports.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> rust/kernel/alloc/kbox.rs | 2 ++
> rust/kernel/init.rs | 1 +
> rust/kernel/sync/arc.rs | 2 ++
> rust/kernel/types.rs | 3 +++
> 4 files changed, 8 insertions(+)
>
> diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
> index 2f8c16473c2c..592fffc72e52 100644
> --- a/rust/kernel/alloc/kbox.rs
> +++ b/rust/kernel/alloc/kbox.rs
> @@ -433,6 +433,7 @@ impl<T, A> InPlaceWrite<T> for Box<MaybeUninit<T>, A>
> {
> type Initialized = Box<T, A>;
>
> + #[inline(always)]
> fn write_init<E>(mut self, init: impl Init<T, E>) -> Result<Self::Initialized, E> {
> let slot = self.as_mut_ptr();
> // SAFETY: When init errors/panics, slot will get deallocated but not dropped,
> @@ -442,6 +443,7 @@ fn write_init<E>(mut self, init: impl Init<T, E>) -> Result<Self::Initialized, E
> Ok(unsafe { Box::assume_init(self) })
> }
>
> + #[inline(always)]
> fn write_pin_init<E>(mut self, init: impl PinInit<T, E>) -> Result<Pin<Self::Initialized>, E> {
> let slot = self.as_mut_ptr();
> // SAFETY: When init errors/panics, slot will get deallocated but not dropped,
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index 7a0d4559d7b5..36d4bf1959fb 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -151,6 +151,7 @@ fn try_pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> Result<Self::Pinne
> /// type.
> ///
> /// If `T: !Unpin` it will not be able to move afterwards.
> + #[inline(always)]
> fn pin_init<E>(init: impl PinInit<T, E>, flags: Flags) -> error::Result<Self::PinnedSelf>
> where
> Error: From<E>,
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 18d6c0d62ce0..973f15165606 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -712,6 +712,7 @@ fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>
> impl<T> InPlaceWrite<T> for UniqueArc<MaybeUninit<T>> {
> type Initialized = UniqueArc<T>;
>
> + #[inline(always)]
> fn write_init<E>(mut self, init: impl Init<T, E>) -> Result<Self::Initialized, E> {
> let slot = self.as_mut_ptr();
> // SAFETY: When init errors/panics, slot will get deallocated but not dropped,
> @@ -721,6 +722,7 @@ fn write_init<E>(mut self, init: impl Init<T, E>) -> Result<Self::Initialized, E
> Ok(unsafe { self.assume_init() })
> }
>
> + #[inline(always)]
> fn write_pin_init<E>(mut self, init: impl PinInit<T, E>) -> Result<Pin<Self::Initialized>, E> {
> let slot = self.as_mut_ptr();
> // SAFETY: When init errors/panics, slot will get deallocated but not dropped,
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index ac316fd7b538..381a06ee0d59 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -367,6 +367,7 @@ pub const fn zeroed() -> Self {
> /// This function is safe, because the `T` inside of an `Opaque` is allowed to be
> /// uninitialized. Additionally, access to the inner `T` requires `unsafe`, so the caller needs
> /// to verify at that point that the inner value is valid.
> + #[inline(always)]
> pub fn ffi_init(init_func: impl FnOnce(*mut T)) -> impl PinInit<Self> {
> // SAFETY: We contain a `MaybeUninit`, so it is OK for the `init_func` to not fully
> // initialize the `T`.
Are you sure this (and other `impl PinInit`-returning functions) are actually
causing issue? They just return an initializer and the actual init code is not
marked as `#[inline]`.
Best,
Gary
> @@ -386,6 +387,7 @@ pub fn ffi_init(init_func: impl FnOnce(*mut T)) -> impl PinInit<Self> {
> /// This function is safe, because the `T` inside of an `Opaque` is allowed to be
> /// uninitialized. Additionally, access to the inner `T` requires `unsafe`, so the caller needs
> /// to verify at that point that the inner value is valid.
> + #[inline(always)]
> pub fn try_ffi_init<E>(
> init_func: impl FnOnce(*mut T) -> Result<(), E>,
> ) -> impl PinInit<Self, E> {
> @@ -417,6 +419,7 @@ pub const fn cast_from(this: *const T) -> *const Self {
>
> impl<T> Wrapper<T> for Opaque<T> {
> /// Create an opaque pin-initializer from the given pin-initializer.
> + #[inline(always)]
> fn pin_init<E>(slot: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> Self::try_ffi_init(|ptr: *mut T| {
> // SAFETY:
^ permalink raw reply [flat|nested] 20+ messages in thread
* Claude review: gpu: drm: nova: enable calling into nova-core
2026-05-27 11:52 [PATCH v2 0/7] gpu: drm: nova: enable calling into nova-core Alexandre Courbot
` (6 preceding siblings ...)
2026-05-27 11:52 ` [PATCH POC v2 7/7] drm: nova: demonstrate interaction with nova-core Alexandre Courbot
@ 2026-05-28 2:21 ` Claude Code Review Bot
7 siblings, 0 replies; 20+ messages in thread
From: Claude Code Review Bot @ 2026-05-28 2:21 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: gpu: drm: nova: enable calling into nova-core
Author: Alexandre Courbot <acourbot@nvidia.com>
Patches: 11
Reviewed: 2026-05-28T12:21:59.491666
---
This is a well-structured 7-patch series enabling `nova-drm` to call into `nova-core` as an interim workaround until the Rust build system natively supports cross-crate module dependencies. The approach is pragmatic: export Rust symbols via generated C `EXPORT_SYMBOL_GPL()` calls, generate `.rmeta` metadata for compile-time type resolution, and wire up the Makefile dependencies. The first three patches are preparatory fixes (modpost truncation detection, inlining to shrink symbol names) and the final patch is an explicitly-labeled POC not intended for merge.
**Key concerns:**
1. The modpost fix (patch 1) has a correctness issue with printing `tmp` after truncation — the content is already truncated/partial and may confuse developers.
2. The `#[inline(always)]` patches are reasonable workarounds but scatter hints across core Rust infrastructure to fix a Nova-specific symbol length problem — this should be discussed with the Rust-for-Linux maintainers since it affects all kernel Rust users.
3. The POC patch (7/7) has significant issues with self-referential struct field ordering and `SAFETY` comments that don't fully address the concerns.
Overall, patches 1-6 are in reasonable shape for a stated workaround. Patch 7 needs rework if it were ever to be merged, but it's explicitly labeled as a POC.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 20+ messages in thread
* Claude review: scripts: modpost: detect and report truncated buf_printf() output
2026-05-27 11:52 ` [PATCH v2 1/7] scripts: modpost: detect and report truncated buf_printf() output Alexandre Courbot
2026-05-27 17:18 ` Nathan Chancellor
@ 2026-05-28 2:21 ` Claude Code Review Bot
1 sibling, 0 replies; 20+ messages in thread
From: Claude Code Review Bot @ 2026-05-28 2:21 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Correctness fix is needed.** The patch correctly identifies that `vsnprintf()` returning `len >= SZ` means the output was truncated and `buf_write()` would read `len` bytes from a `SZ`-sized stack buffer — a buffer over-read.
However, the error message has a problem:
```c
if (len >= SZ)
fatal("buf_printf output truncated for string %s: %d bytes needed, %d available\n",
tmp, len + 1, SZ);
```
When `len >= SZ`, `tmp` contains a **truncated**, null-terminated string. Printing it via `%s` is technically safe (it's still null-terminated by `vsnprintf()`), but the message says "for string %s" which shows the *truncated* output, not the full intended string. This is probably good enough for debugging — you'll see the start of the symbol name — but the message could be slightly misleading. Consider adding a note like `(truncated)` to the output.
Also, the `va_end(ap)` was moved before the truncation check, which is correct — `va_end` should be called before `exit(1)` or `fatal()` to ensure proper cleanup. Good.
Minor style: the kernel modpost code doesn't seem to use braces around single-statement `if` bodies elsewhere, but consistency with the existing `if (len < 0)` block (which does use braces) is fine.
**No blocking issues.**
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 20+ messages in thread
* Claude review: pin_init: always inline the #ident and #project_ident methods
2026-05-27 11:52 ` [PATCH v2 2/7] pin_init: always inline the #ident and #project_ident methods Alexandre Courbot
2026-05-27 17:49 ` Gary Guo
@ 2026-05-28 2:22 ` Claude Code Review Bot
1 sibling, 0 replies; 20+ messages in thread
From: Claude Code Review Bot @ 2026-05-28 2:22 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Small, mechanical patch adding `#[inline(always)]` to two generated methods in `pin_data.rs`:
```rust
#[inline(always)]
#vis unsafe fn #ident<E>(
```
```rust
#[inline(always)]
#vis unsafe fn #project_ident<'__slot>(
```
These are proc-macro-generated methods that form part of the pin initialization infrastructure. Adding `#[inline(always)]` is reasonable for methods that are small delegation wrappers and whose mangled names would otherwise consume the 500-byte `buf_printf` buffer.
**No issues.** This should be reviewed/acked by the pin-init maintainer (Benno Lossin).
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 20+ messages in thread
* Claude review: rust: always inline some init methods
2026-05-27 11:52 ` [PATCH v2 3/7] rust: always inline some init methods Alexandre Courbot
2026-05-27 17:54 ` Gary Guo
@ 2026-05-28 2:22 ` Claude Code Review Bot
1 sibling, 0 replies; 20+ messages in thread
From: Claude Code Review Bot @ 2026-05-28 2:22 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Adds `#[inline(always)]` to 8 methods across 4 files:
- `Box::write_init`, `Box::write_pin_init` in `kbox.rs`
- `InPlaceInit::pin_init` in `init.rs`
- `UniqueArc::write_init`, `UniqueArc::write_pin_init` in `arc.rs`
- `Opaque::ffi_init`, `Opaque::try_ffi_init`, `Opaque::pin_init` in `types.rs`
These are all thin wrapper methods that forward to inner initialization logic. Forcing inlining is justified both for optimization (avoiding monomorphization explosion of trivial wrappers) and for the symbol length constraint.
**No issues.** Same maintainer coordination needed as patch 2.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 20+ messages in thread
* Claude review: gpu: nova-core: export Rust symbols for dependent modules
2026-05-27 11:52 ` [PATCH v2 4/7] gpu: nova-core: export Rust symbols for dependent modules Alexandre Courbot
@ 2026-05-28 2:22 ` Claude Code Review Bot
0 siblings, 0 replies; 20+ messages in thread
From: Claude Code Review Bot @ 2026-05-28 2:22 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
This is the core workaround. The approach:
1. Use `nm -p --defined-only` to extract global text symbols starting with `_R` (Rust v0 mangling prefix)
2. Generate `extern void sym(void); EXPORT_SYMBOL_GPL(sym);` for each
3. Compile the generated header into `nova_core_exports.o`
```makefile
rust_exports = \
$(NM) -p --defined-only $(1) | \
awk '$$2 == "T" && $$3 ~ /^_R/ { \
printf "extern void %s(void); EXPORT_SYMBOL_GPL(%s);\n", $$3, $$3 \
}'
```
**Observations:**
- Filtering on `$$2 == "T"` (global text) and `$$3 ~ /^_R/` is correct for Rust v0 mangled symbols. This won't accidentally pick up C symbols.
- The `extern void sym(void)` declarations are technically type-incorrect (the actual functions have different signatures), but this is only used for `EXPORT_SYMBOL_GPL()` which just needs the symbol address. This is the same pattern used elsewhere in the kernel for cross-language symbol exports.
- Using `filechk` ensures the header is regenerated when the object changes.
- `nova_core_exports.c` is clean and minimal.
**Potential concern:** This exports *all* global Rust text symbols from `nova_core.o`, not just the ones `nova-drm` actually needs. This is a broad export surface. The cover letter acknowledges this is a workaround, so this is acceptable for now, but a comment noting that a more targeted approach would be preferable long-term might be useful.
**No blocking issues.**
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 20+ messages in thread
* Claude review: gpu: nova-core: emit Rust metadata for dependent modules
2026-05-27 11:52 ` [PATCH v2 5/7] gpu: nova-core: emit Rust metadata " Alexandre Courbot
@ 2026-05-28 2:22 ` Claude Code Review Bot
0 siblings, 0 replies; 20+ messages in thread
From: Claude Code Review Bot @ 2026-05-28 2:22 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Generates `.rmeta` (Rust crate metadata) so `nova-drm` can resolve types at compile time.
```makefile
quiet_cmd_rustc_rmeta_rs = RUSTC RMETA $@
cmd_rustc_rmeta_rs = mkdir -p $(dir $@); \
$(rust_common_cmd) --emit=metadata=$@ \
--emit=obj=$(basename $@).o $<; \
rm -f $(basename $@).o
```
Key design decisions:
- Builds in a private directory (`.nova_core_rmeta/`) to avoid racing with the main `nova_core.o` build during parallel make. This is a sensible precaution.
- The intermediate `.o` file from the metadata build is immediately deleted since only the `.rmeta` is needed.
- A final `if_changed,copy` step places the metadata at a stable path.
**Minor observation:** The `--emit=obj=$(basename $@).o` plus `rm -f` is needed because rustc insists on emitting an object when also emitting metadata. The `rm -f` cleanup is fine.
**No blocking issues.**
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 20+ messages in thread
* Claude review: gpu: drm: nova: build after nova-core metadata
2026-05-27 11:52 ` [PATCH v2 6/7] gpu: drm: nova: build after nova-core metadata Alexandre Courbot
@ 2026-05-28 2:22 ` Claude Code Review Bot
0 siblings, 0 replies; 20+ messages in thread
From: Claude Code Review Bot @ 2026-05-28 2:22 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Wires up `nova-drm`'s Makefile to depend on `nova-core`'s metadata:
```makefile
rustflags-y += --extern nova_core=$(objtree)/$(nova_core_rmeta)
$(obj)/nova.o: | $(objtree)/$(nova_core_rmeta)
$(objtree)/$(nova_core_rmeta): FORCE
$(Q)$(MAKE) $(build)=drivers/gpu/nova-core $(nova_core_rmeta)
```
The order-only prerequisite (`|`) is correct — `nova.o` needs the metadata to exist before compilation starts, but doesn't need to be rebuilt just because the metadata timestamp changed (rustc will check content).
The recursive `$(MAKE)` call to build `nova-core`'s metadata from `nova-drm`'s Makefile is explicitly acknowledged as ugly but temporary.
**No issues.**
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 20+ messages in thread
* Claude review: drm: nova: demonstrate interaction with nova-core
2026-05-27 11:52 ` [PATCH POC v2 7/7] drm: nova: demonstrate interaction with nova-core Alexandre Courbot
@ 2026-05-28 2:22 ` Claude Code Review Bot
0 siblings, 0 replies; 20+ messages in thread
From: Claude Code Review Bot @ 2026-05-28 2:22 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
This is explicitly labeled as a POC that won't be merged. Even so, there are several issues worth noting for when the real implementation is built:
**1. Self-referential struct field ordering change:**
The patch reorders `NovaCore`'s fields to put `_reg` first:
```rust
pub struct NovaCore<'bound> {
_reg: auxiliary::Registration<'bound, ForLt!(AuxData<'_>)>,
#[pin]
pub(crate) gpu: Gpu<'bound>,
bar: pci::Bar<'bound, BAR0_SIZE>,
}
```
The comment says "Fields are dropped in declaration order: unregister the auxiliary device before dropping `gpu`". This is correct for Rust's drop order (fields are dropped in declaration order). However, this conflicts with the current upstream code which uses `Devres<auxiliary::Registration>` — the `Devres` wrapper handles device-managed lifetime differently. The POC has regressed to a non-`Devres` approach (no `#[pin]` on `_reg`, direct `Registration` instead of `Devres<Registration>`), which may not be the right pattern for the final implementation.
**2. Unsafe self-referential pointer construction:**
```rust
gpu: &*core::ptr::from_ref(&this.as_ref().gpu),
```
The `SAFETY` comment says "`this.gpu` is initialized before this expression is evaluated", but this relies on `try_pin_init!(&this in ...)` guaranteeing field initialization order. This is correct for `pin_init`'s current semantics but is subtle. The comment should also note that `AuxData` must not outlive `NovaCore` (which it can't since `_reg` is dropped first).
**3. Changed struct to non-`#[pin]` bar field:**
The upstream code uses `Arc<Devres<Bar0>>` for the bar (shared ownership with device-managed cleanup), but the POC switches to `pci::Bar<'bound, BAR0_SIZE>` (borrowed, lifetime-bound). This is a significant API difference that makes the POC not representative of the final design.
**4. Public visibility changes:**
Making `driver` and `gpu` modules `pub` and exposing `Spec.chipset` and `Gpu.spec` as `pub(crate)` is reasonable for the cross-crate use case. The `AuxData` wrapper providing a `chipset()` method is a clean API.
**No blocking issues** (this is a POC and labeled as such).
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2026-05-28 2:22 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-27 11:52 [PATCH v2 0/7] gpu: drm: nova: enable calling into nova-core Alexandre Courbot
2026-05-27 11:52 ` [PATCH v2 1/7] scripts: modpost: detect and report truncated buf_printf() output Alexandre Courbot
2026-05-27 17:18 ` Nathan Chancellor
2026-05-28 2:21 ` Claude review: " Claude Code Review Bot
2026-05-27 11:52 ` [PATCH v2 2/7] pin_init: always inline the #ident and #project_ident methods Alexandre Courbot
2026-05-27 17:49 ` Gary Guo
2026-05-28 2:22 ` Claude review: " Claude Code Review Bot
2026-05-27 11:52 ` [PATCH v2 3/7] rust: always inline some init methods Alexandre Courbot
2026-05-27 17:54 ` Gary Guo
2026-05-28 2:22 ` Claude review: " Claude Code Review Bot
2026-05-27 11:52 ` [PATCH v2 4/7] gpu: nova-core: export Rust symbols for dependent modules Alexandre Courbot
2026-05-28 2:22 ` Claude review: " Claude Code Review Bot
2026-05-27 11:52 ` [PATCH v2 5/7] gpu: nova-core: emit Rust metadata " Alexandre Courbot
2026-05-28 2:22 ` Claude review: " Claude Code Review Bot
2026-05-27 11:52 ` [PATCH v2 6/7] gpu: drm: nova: build after nova-core metadata Alexandre Courbot
2026-05-28 2:22 ` Claude review: " Claude Code Review Bot
2026-05-27 11:52 ` [PATCH POC v2 7/7] drm: nova: demonstrate interaction with nova-core Alexandre Courbot
2026-05-28 2:22 ` Claude review: " Claude Code Review Bot
2026-05-28 2:21 ` Claude review: gpu: drm: nova: enable calling into nova-core Claude Code Review Bot
-- strict thread matches above, loose matches on Subject: below --
2026-04-30 14:55 [PATCH 0/7] " Alexandre Courbot
2026-04-30 14:55 ` [PATCH 4/7] gpu: nova-core: export Rust symbols for dependent modules Alexandre Courbot
2026-05-05 0:17 ` Claude review: " Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox