* [PATCH 1/7] scripts: modpost: detect and report truncated buf_printf() output
2026-04-30 14:55 [PATCH 0/7] gpu: drm: nova: enable calling into nova-core Alexandre Courbot
@ 2026-04-30 14:55 ` Alexandre Courbot
2026-05-05 0:17 ` Claude review: " Claude Code Review Bot
2026-04-30 14:55 ` [PATCH 2/7] scripts: modpost: increase buf_printf's buffer size Alexandre Courbot
` (6 subsequent siblings)
7 siblings, 1 reply; 24+ messages in thread
From: Alexandre Courbot @ 2026-04-30 14:55 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, Joel Fernandes, 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..16f6bc20b058 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 was truncated: %d bytes needed, %d available\n",
+ 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] 24+ messages in thread* Claude review: scripts: modpost: detect and report truncated buf_printf() output
2026-04-30 14:55 ` [PATCH 1/7] scripts: modpost: detect and report truncated buf_printf() output Alexandre Courbot
@ 2026-05-05 0:17 ` Claude Code Review Bot
0 siblings, 0 replies; 24+ messages in thread
From: Claude Code Review Bot @ 2026-05-05 0:17 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Verdict: Good bugfix, should likely be submitted independently.**
This is a genuine pre-existing bug. When `vsnprintf` returns `len >= SZ`, the current code passes that unclamped length to `buf_write`, which reads `len` bytes from a buffer that only has `SZ` bytes — a classic buffer over-read:
```c
len = vsnprintf(tmp, SZ, fmt, ap);
buf_write(buf, tmp, len);
```
The fix correctly checks both error return (`len < 0`) and truncation (`len >= SZ`):
```c
if (len < 0) {
perror("vsnprintf failed");
exit(1);
}
if (len >= SZ)
fatal("buf_printf output was truncated: %d bytes needed, %d available\n",
len + 1, SZ);
```
Minor nits:
1. The error message says `%d available` but prints `SZ` (the buffer size). Since `vsnprintf` reserves one byte for the NUL terminator, `SZ` is the buffer capacity, not the available character count. `len + 1` is the needed bytes *including* NUL, and `SZ` is available *including* NUL, so this is consistent — but it could be clearer. Consider wording like "needed %d bytes but buffer is only %d" without the `+1`.
2. This patch is a standalone bugfix applicable to all of modpost, not Nova-specific. It would benefit from being submitted separately or at minimum noted as such in the cover letter, since it could be picked up independently.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/7] scripts: modpost: increase buf_printf's buffer size
2026-04-30 14:55 [PATCH 0/7] gpu: drm: nova: enable calling into nova-core Alexandre Courbot
2026-04-30 14:55 ` [PATCH 1/7] scripts: modpost: detect and report truncated buf_printf() output Alexandre Courbot
@ 2026-04-30 14:55 ` Alexandre Courbot
2026-05-01 13:02 ` Gary Guo
2026-05-05 0:17 ` Claude review: " Claude Code Review Bot
2026-04-30 14:55 ` [PATCH 3/7] gpu: nova-core: rename module from nova_core to nova-core Alexandre Courbot
` (5 subsequent siblings)
7 siblings, 2 replies; 24+ messages in thread
From: Alexandre Courbot @ 2026-04-30 14:55 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, Joel Fernandes, Timur Tabi,
Zhi Wang, Eliot Courtney, linux-kbuild, linux-kernel,
rust-for-linux, nova-gpu, dri-devel, Alexandre Courbot
Rust tends to produce long symbol names; when trying to export symbols
from nova-core for nova-drm to link to, the 500 bytes of the internal
buffer used for symbol name formatting are not enough, making modpost
fail.
Fix this by increasing the size of the buffer used to format the symbols
to 1024 bytes. It is a stack buffer, but modpost is a user-space program
so that shouldn't be a problem.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
scripts/mod/modpost.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 16f6bc20b058..2ab7aa6edcb4 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1674,7 +1674,7 @@ static void read_symbols_from_files(const char *filename)
fclose(in);
}
-#define SZ 500
+#define SZ 1024
/* We first write the generated file into memory using the
* following helper, then compare to the file on disk and
--
2.54.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 2/7] scripts: modpost: increase buf_printf's buffer size
2026-04-30 14:55 ` [PATCH 2/7] scripts: modpost: increase buf_printf's buffer size Alexandre Courbot
@ 2026-05-01 13:02 ` Gary Guo
2026-05-01 15:31 ` Miguel Ojeda
2026-05-05 0:17 ` Claude review: " Claude Code Review Bot
1 sibling, 1 reply; 24+ messages in thread
From: Gary Guo @ 2026-05-01 13:02 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, Joel Fernandes, Timur Tabi,
Zhi Wang, Eliot Courtney, linux-kbuild, linux-kernel,
rust-for-linux, nova-gpu, dri-devel
On Thu Apr 30, 2026 at 3:55 PM BST, Alexandre Courbot wrote:
> Rust tends to produce long symbol names; when trying to export symbols
> from nova-core for nova-drm to link to, the 500 bytes of the internal
> buffer used for symbol name formatting are not enough, making modpost
> fail.
>
> Fix this by increasing the size of the buffer used to format the symbols
> to 1024 bytes. It is a stack buffer, but modpost is a user-space program
> so that shouldn't be a problem.
I think we should make sure all constants related to symbol names match.
KSYM_NAME_LEN is 512 so this should just be that.
The only case that I've been hit with very long symbol names so far is doc tests.
Can you provide an example of the case where you're hit with very long symbol
names in Nova? In many cases they're just functions that are supposed to be
inlined but isn't.
Best,
Gary
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> scripts/mod/modpost.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 16f6bc20b058..2ab7aa6edcb4 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1674,7 +1674,7 @@ static void read_symbols_from_files(const char *filename)
> fclose(in);
> }
>
> -#define SZ 500
> +#define SZ 1024
>
> /* We first write the generated file into memory using the
> * following helper, then compare to the file on disk and
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/7] scripts: modpost: increase buf_printf's buffer size
2026-05-01 13:02 ` Gary Guo
@ 2026-05-01 15:31 ` Miguel Ojeda
0 siblings, 0 replies; 24+ messages in thread
From: Miguel Ojeda @ 2026-05-01 15:31 UTC (permalink / raw)
To: Gary Guo
Cc: Alexandre Courbot, Miguel Ojeda, Nathan Chancellor,
Nicolas Schier, Boqun Feng, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
David Airlie, Simona Vetter, John Hubbard, Alistair Popple,
Joel Fernandes, Timur Tabi, Zhi Wang, Eliot Courtney,
linux-kbuild, linux-kernel, rust-for-linux, nova-gpu, dri-devel
On Fri, May 1, 2026 at 3:02 PM Gary Guo <gary@garyguo.net> wrote:
>
> The only case that I've been hit with very long symbol names so far is doc tests.
Yeah, nowadays where I have seen them are doctests too, in particular
for UML debug builds.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Claude review: scripts: modpost: increase buf_printf's buffer size
2026-04-30 14:55 ` [PATCH 2/7] scripts: modpost: increase buf_printf's buffer size Alexandre Courbot
2026-05-01 13:02 ` Gary Guo
@ 2026-05-05 0:17 ` Claude Code Review Bot
1 sibling, 0 replies; 24+ messages in thread
From: Claude Code Review Bot @ 2026-05-05 0:17 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Verdict: Acceptable, but the choice of 1024 is arbitrary.**
```c
-#define SZ 500
+#define SZ 1024
```
The change doubles the buffer. 500 was already arbitrary. 1024 is fine for now, but as Rust symbol mangling is verbose, this could be exceeded again in the future. The commit message justifies it well — modpost is userspace, so stack usage isn't a concern.
Worth considering: would it be more robust to `malloc`/`realloc` or use `asprintf` instead of a fixed stack buffer? This would eliminate the truncation problem entirely. That said, this is a minimal fix and the truncation detection in patch 1 would catch future overflow, so this is acceptable as-is.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 3/7] gpu: nova-core: rename module from nova_core to nova-core
2026-04-30 14:55 [PATCH 0/7] gpu: drm: nova: enable calling into nova-core Alexandre Courbot
2026-04-30 14:55 ` [PATCH 1/7] scripts: modpost: detect and report truncated buf_printf() output Alexandre Courbot
2026-04-30 14:55 ` [PATCH 2/7] scripts: modpost: increase buf_printf's buffer size Alexandre Courbot
@ 2026-04-30 14:55 ` Alexandre Courbot
2026-04-30 15:07 ` Joel Fernandes
2026-05-05 0:17 ` Claude review: " Claude Code Review Bot
2026-04-30 14:55 ` [PATCH 4/7] gpu: nova-core: export Rust symbols for dependent modules Alexandre Courbot
` (4 subsequent siblings)
7 siblings, 2 replies; 24+ messages in thread
From: Alexandre Courbot @ 2026-04-30 14:55 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, Joel Fernandes, Timur Tabi,
Zhi Wang, Eliot Courtney, linux-kbuild, linux-kernel,
rust-for-linux, nova-gpu, dri-devel, Alexandre Courbot
Rename the kbuild module target from `nova_core.o` to `nova-core.o`.
This avoids a name collision between the module target and the Rust
crate object (`nova_core.o` from `nova_core.rs`), allowing the driver to
be built as a composite module.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
drivers/gpu/nova-core/Kconfig | 2 +-
drivers/gpu/nova-core/Makefile | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
index d8456f8eaa05..f918f69e0599 100644
--- a/drivers/gpu/nova-core/Kconfig
+++ b/drivers/gpu/nova-core/Kconfig
@@ -14,4 +14,4 @@ config NOVA_CORE
This driver is work in progress and may not be functional.
- If M is selected, the module will be called nova_core.
+ If M is selected, the module will be called nova-core.
diff --git a/drivers/gpu/nova-core/Makefile b/drivers/gpu/nova-core/Makefile
index 2d78c50126e1..1f794baadc86 100644
--- a/drivers/gpu/nova-core/Makefile
+++ b/drivers/gpu/nova-core/Makefile
@@ -1,3 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_NOVA_CORE) += nova_core.o
+obj-$(CONFIG_NOVA_CORE) += nova-core.o
+
+nova-core-y := nova_core.o
--
2.54.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 3/7] gpu: nova-core: rename module from nova_core to nova-core
2026-04-30 14:55 ` [PATCH 3/7] gpu: nova-core: rename module from nova_core to nova-core Alexandre Courbot
@ 2026-04-30 15:07 ` Joel Fernandes
2026-05-01 3:25 ` Alexandre Courbot
2026-05-05 0:17 ` Claude review: " Claude Code Review Bot
1 sibling, 1 reply; 24+ messages in thread
From: Joel Fernandes @ 2026-04-30 15:07 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 4/30/2026 10:55 AM, Alexandre Courbot wrote:
> Rename the kbuild module target from `nova_core.o` to `nova-core.o`.
> This avoids a name collision between the module target and the Rust
> crate object (`nova_core.o` from `nova_core.rs`), allowing the driver to
> be built as a composite module.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> drivers/gpu/nova-core/Kconfig | 2 +-
> drivers/gpu/nova-core/Makefile | 4 +++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
> index d8456f8eaa05..f918f69e0599 100644
> --- a/drivers/gpu/nova-core/Kconfig
> +++ b/drivers/gpu/nova-core/Kconfig
> @@ -14,4 +14,4 @@ config NOVA_CORE
>
> This driver is work in progress and may not be functional.
>
> - If M is selected, the module will be called nova_core.
> + If M is selected, the module will be called nova-core.
> diff --git a/drivers/gpu/nova-core/Makefile b/drivers/gpu/nova-core/Makefile
> index 2d78c50126e1..1f794baadc86 100644
> --- a/drivers/gpu/nova-core/Makefile
> +++ b/drivers/gpu/nova-core/Makefile
> @@ -1,3 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0
>
> -obj-$(CONFIG_NOVA_CORE) += nova_core.o
> +obj-$(CONFIG_NOVA_CORE) += nova-core.o
I think a more descriptive name would be better than relying on dashes versus
versus underscores, that's super confusing, how about:
> +
> +nova-core-y := nova_core.o
>
Lets make this: `nova-core-y := nova-core-mod.o`?
thanks,
--
Joel fernandes
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/7] gpu: nova-core: rename module from nova_core to nova-core
2026-04-30 15:07 ` Joel Fernandes
@ 2026-05-01 3:25 ` Alexandre Courbot
2026-05-01 10:38 ` John Hubbard
2026-05-01 12:31 ` Danilo Krummrich
0 siblings, 2 replies; 24+ messages in thread
From: Alexandre Courbot @ 2026-05-01 3:25 UTC (permalink / raw)
To: Joel Fernandes, Danilo Krummrich
Cc: 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, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, Eliot Courtney, linux-kbuild, linux-kernel,
rust-for-linux, nova-gpu, dri-devel
On Fri May 1, 2026 at 12:07 AM JST, Joel Fernandes wrote:
> On 4/30/2026 10:55 AM, Alexandre Courbot wrote:
>> Rename the kbuild module target from `nova_core.o` to `nova-core.o`.
>> This avoids a name collision between the module target and the Rust
>> crate object (`nova_core.o` from `nova_core.rs`), allowing the driver to
>> be built as a composite module.
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> ---
>> drivers/gpu/nova-core/Kconfig | 2 +-
>> drivers/gpu/nova-core/Makefile | 4 +++-
>> 2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/Kconfig b/drivers/gpu/nova-core/Kconfig
>> index d8456f8eaa05..f918f69e0599 100644
>> --- a/drivers/gpu/nova-core/Kconfig
>> +++ b/drivers/gpu/nova-core/Kconfig
>> @@ -14,4 +14,4 @@ config NOVA_CORE
>>
>> This driver is work in progress and may not be functional.
>>
>> - If M is selected, the module will be called nova_core.
>> + If M is selected, the module will be called nova-core.
>> diff --git a/drivers/gpu/nova-core/Makefile b/drivers/gpu/nova-core/Makefile
>> index 2d78c50126e1..1f794baadc86 100644
>> --- a/drivers/gpu/nova-core/Makefile
>> +++ b/drivers/gpu/nova-core/Makefile
>> @@ -1,3 +1,5 @@
>> # SPDX-License-Identifier: GPL-2.0
>>
>> -obj-$(CONFIG_NOVA_CORE) += nova_core.o
>> +obj-$(CONFIG_NOVA_CORE) += nova-core.o
>
> I think a more descriptive name would be better than relying on dashes versus
> versus underscores, that's super confusing, how about:
>
>> +
>> +nova-core-y := nova_core.o
>>
>
> Lets make this: `nova-core-y := nova-core-mod.o`?
We would have to rename `nova_core.rs` to `nova_core_mod.rs` though.
But your argument has a point generally speaking - if there wasn't
any underscore in the module's name, then we would have to pick a
different name anyway.
Another option would be to use the Rust-idiomatic `lib.rs`. That's what
the kernel crate and others do.
Danilo, WDYT?
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/7] gpu: nova-core: rename module from nova_core to nova-core
2026-05-01 3:25 ` Alexandre Courbot
@ 2026-05-01 10:38 ` John Hubbard
2026-05-01 12:31 ` Danilo Krummrich
1 sibling, 0 replies; 24+ messages in thread
From: John Hubbard @ 2026-05-01 10:38 UTC (permalink / raw)
To: Alexandre Courbot, Joel Fernandes, Danilo Krummrich
Cc: Miguel Ojeda, Nathan Chancellor, Nicolas Schier, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, David Airlie, Simona Vetter,
Alistair Popple, Timur Tabi, Zhi Wang, Eliot Courtney,
linux-kbuild, linux-kernel, rust-for-linux, nova-gpu, dri-devel
On 5/1/26 5:25 AM, Alexandre Courbot wrote:
> On Fri May 1, 2026 at 12:07 AM JST, Joel Fernandes wrote:
>> On 4/30/2026 10:55 AM, Alexandre Courbot wrote:
...
>> Lets make this: `nova-core-y := nova-core-mod.o`?
>
> We would have to rename `nova_core.rs` to `nova_core_mod.rs` though.
>
> But your argument has a point generally speaking - if there wasn't
> any underscore in the module's name, then we would have to pick a
> different name anyway.
>
> Another option would be to use the Rust-idiomatic `lib.rs`. That's what
> the kernel crate and others do.
"lib" (or something like that) is better, because we will want to avoid
"mod" in the name. There is plenty of "mod" naming floating around
already, due to how kbuild names its artifacts, so adding more "mod"
will not increase our happiness. :)
$ ls -a | grep mod | grep nova
nova_core.mod
nova_core.mod.c
.nova_core.mod.cmd
nova_core.mod.o
.nova_core.mod.o.cmd
thanks,
--
John Hubbard
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/7] gpu: nova-core: rename module from nova_core to nova-core
2026-05-01 3:25 ` Alexandre Courbot
2026-05-01 10:38 ` John Hubbard
@ 2026-05-01 12:31 ` Danilo Krummrich
1 sibling, 0 replies; 24+ messages in thread
From: Danilo Krummrich @ 2026-05-01 12:31 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Joel Fernandes, Miguel Ojeda, Nathan Chancellor, Nicolas Schier,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, 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 Fri May 1, 2026 at 5:25 AM CEST, Alexandre Courbot wrote:
> Danilo, WDYT?
I think the patch is fine as is.
However, the commit message should say that it also changes nova_core.ko to
nova-core.ko, which is the reason I proposed this in [1] as well.
I.e. such that the module name (that we still need to change; I can send a patch
since I haven't heard back from Chou) matches the .ko file name.
I'm not a fan of generic names like lib.rs, mod.rs, main.rs, etc., as it messes
with fuzzy finder tools.
If people feel strongly about this, we can probably rename it to
nova_core_main.rs, but I think it is not necessary.
- Danilo
[1] https://lore.kernel.org/all/DH56PG8GLW4X.2QMHJ91J9D29N@kernel.org/
^ permalink raw reply [flat|nested] 24+ messages in thread
* Claude review: gpu: nova-core: rename module from nova_core to nova-core
2026-04-30 14:55 ` [PATCH 3/7] gpu: nova-core: rename module from nova_core to nova-core Alexandre Courbot
2026-04-30 15:07 ` Joel Fernandes
@ 2026-05-05 0:17 ` Claude Code Review Bot
1 sibling, 0 replies; 24+ messages in thread
From: Claude Code Review Bot @ 2026-05-05 0:17 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Verdict: Clean and correct.**
```makefile
-obj-$(CONFIG_NOVA_CORE) += nova_core.o
+obj-$(CONFIG_NOVA_CORE) += nova-core.o
+
+nova-core-y := nova_core.o
```
This is the standard kbuild pattern for composite modules where the crate object name (`nova_core.o` from `nova_core.rs`) would collide with the module target name. The Kconfig help text update is correct. No issues.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/7] gpu: nova-core: export Rust symbols for dependent modules
2026-04-30 14:55 [PATCH 0/7] gpu: drm: nova: enable calling into nova-core Alexandre Courbot
` (2 preceding siblings ...)
2026-04-30 14:55 ` [PATCH 3/7] gpu: nova-core: rename module from nova_core to nova-core Alexandre Courbot
@ 2026-04-30 14:55 ` Alexandre Courbot
2026-04-30 15:22 ` Joel Fernandes
2026-05-05 0:17 ` Claude review: " Claude Code Review Bot
2026-04-30 14:55 ` [PATCH 5/7] gpu: nova-core: emit Rust metadata " Alexandre Courbot
` (3 subsequent siblings)
7 siblings, 2 replies; 24+ messages in thread
From: Alexandre Courbot @ 2026-04-30 14:55 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, Joel Fernandes, Timur Tabi,
Zhi Wang, Eliot Courtney, linux-kbuild, linux-kernel,
rust-for-linux, nova-gpu, dri-devel, Alexandre Courbot
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>
---
drivers/gpu/nova-core/Makefile | 22 +++++++++++++++++++++-
drivers/gpu/nova-core/nova_core_exports.c | 11 +++++++++++
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/nova-core/Makefile b/drivers/gpu/nova-core/Makefile
index 1f794baadc86..f9aaf19f2477 100644
--- a/drivers/gpu/nova-core/Makefile
+++ b/drivers/gpu/nova-core/Makefile
@@ -2,4 +2,24 @@
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..1d3f1544fe8d
--- /dev/null
+++ b/drivers/gpu/nova-core/nova_core_exports.c
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * 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] 24+ messages in thread* Re: [PATCH 4/7] 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-04-30 15:22 ` Joel Fernandes
2026-05-01 3:30 ` Alexandre Courbot
2026-05-01 10:24 ` Miguel Ojeda
2026-05-05 0:17 ` Claude review: " Claude Code Review Bot
1 sibling, 2 replies; 24+ messages in thread
From: Joel Fernandes @ 2026-04-30 15:22 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
Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
One comment below:
On 4/30/2026 10:55 AM, Alexandre Courbot wrote:
> 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>
> ---
> drivers/gpu/nova-core/Makefile | 22 +++++++++++++++++++++-
> drivers/gpu/nova-core/nova_core_exports.c | 11 +++++++++++
> 2 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/nova-core/Makefile b/drivers/gpu/nova-core/Makefile
> index 1f794baadc86..f9aaf19f2477 100644
> --- a/drivers/gpu/nova-core/Makefile
> +++ b/drivers/gpu/nova-core/Makefile
> @@ -2,4 +2,24 @@
>
> 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 \
> + }'
I am curious (in a fun way) how this (Or Miguel's later approach) will work with
generics. I don't think we have such use cases but suppose a module has a
function foo<T> which is expected to be called externally outside the module.
Suppose the module itself does not call foo internally.
How does the rust compiler know that this function can be called externally if
it resolves to no callers within the translation unit?
Further, as a result, how does the above extern get emitted then if foo never
ended up in the object?
Probably Miguel's future infrastructure will address this? If so, probably the
limitation of the above approach should be called out in the code comments and
commit message.
thanks,
--
Joel Fernandes
> +
> +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..1d3f1544fe8d
> --- /dev/null
> +++ b/drivers/gpu/nova-core/nova_core_exports.c
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * 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"
>
I
--
Joel Fernandes
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 4/7] gpu: nova-core: export Rust symbols for dependent modules
2026-04-30 15:22 ` Joel Fernandes
@ 2026-05-01 3:30 ` Alexandre Courbot
2026-05-01 10:24 ` Miguel Ojeda
1 sibling, 0 replies; 24+ messages in thread
From: Alexandre Courbot @ 2026-05-01 3:30 UTC (permalink / raw)
To: Joel Fernandes
Cc: 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, John Hubbard, Alistair Popple, Timur Tabi,
Zhi Wang, Eliot Courtney, linux-kbuild, linux-kernel,
rust-for-linux, nova-gpu, dri-devel
On Fri May 1, 2026 at 12:22 AM JST, Joel Fernandes wrote:
>
> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
> One comment below:
>
> On 4/30/2026 10:55 AM, Alexandre Courbot wrote:
>> 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>
>> ---
>> drivers/gpu/nova-core/Makefile | 22 +++++++++++++++++++++-
>> drivers/gpu/nova-core/nova_core_exports.c | 11 +++++++++++
>> 2 files changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/nova-core/Makefile b/drivers/gpu/nova-core/Makefile
>> index 1f794baadc86..f9aaf19f2477 100644
>> --- a/drivers/gpu/nova-core/Makefile
>> +++ b/drivers/gpu/nova-core/Makefile
>> @@ -2,4 +2,24 @@
>>
>> 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 \
>> + }'
>
> I am curious (in a fun way) how this (Or Miguel's later approach) will work with
> generics. I don't think we have such use cases but suppose a module has a
> function foo<T> which is expected to be called externally outside the module.
> Suppose the module itself does not call foo internally.
>
> How does the rust compiler know that this function can be called externally if
> it resolves to no callers within the translation unit?
>
> Further, as a result, how does the above extern get emitted then if foo never
> ended up in the object?
>
> Probably Miguel's future infrastructure will address this? If so, probably the
> limitation of the above approach should be called out in the code comments and
> commit message.
Short version: it should Just Work (c). The next patch of the series
generates the metadata of the `nova-core` crate, which contains the MIR
representation of all generic elements. If `nova-drm` needs to
monomorphize some generic code, it will do it from that representation -
thus an export of the symbol is not needed.
IIUC this is how inter-crate generics are handled in Rust, so this is
also the way the kernel is also expected to do.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 4/7] gpu: nova-core: export Rust symbols for dependent modules
2026-04-30 15:22 ` Joel Fernandes
2026-05-01 3:30 ` Alexandre Courbot
@ 2026-05-01 10:24 ` Miguel Ojeda
1 sibling, 0 replies; 24+ messages in thread
From: Miguel Ojeda @ 2026-05-01 10:24 UTC (permalink / raw)
To: Joel Fernandes
Cc: 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, John Hubbard,
Alistair Popple, Timur Tabi, Zhi Wang, Eliot Courtney,
linux-kbuild, linux-kernel, rust-for-linux, nova-gpu, dri-devel
On Thu, Apr 30, 2026 at 5:22 PM Joel Fernandes <joelagnelf@nvidia.com> wrote:
>
> How does the rust compiler know that this function can be called externally if
> it resolves to no callers within the translation unit?
Like Alex mentions, that information is what the `.rmeta` files
contain, which are generated by the compiler and consumed by the
compiler later on.
They contain stuff like generics that you point out, in order to
actually use them later on, as well as many other things.
In a way, you can think of them as opaque, generated "C headers" with
all the information needed to build later crates. Other languages take
similar approaches.
Cheers,
Miguel
^ permalink raw reply [flat|nested] 24+ messages in thread
* 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-04-30 15:22 ` Joel Fernandes
@ 2026-05-05 0:17 ` Claude Code Review Bot
1 sibling, 0 replies; 24+ 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] 24+ messages in thread
* [PATCH 5/7] gpu: nova-core: emit Rust metadata for dependent modules
2026-04-30 14:55 [PATCH 0/7] gpu: drm: nova: enable calling into nova-core Alexandre Courbot
` (3 preceding siblings ...)
2026-04-30 14:55 ` [PATCH 4/7] gpu: nova-core: export Rust symbols for dependent modules Alexandre Courbot
@ 2026-04-30 14:55 ` Alexandre Courbot
2026-05-05 0:17 ` Claude review: " Claude Code Review Bot
2026-04-30 14:55 ` [PATCH 6/7] gpu: drm: nova: build after nova-core metadata Alexandre Courbot
` (2 subsequent siblings)
7 siblings, 1 reply; 24+ messages in thread
From: Alexandre Courbot @ 2026-04-30 14:55 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, Joel Fernandes, 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 f9aaf19f2477..fea6d5c68cd6 100644
--- a/drivers/gpu/nova-core/Makefile
+++ b/drivers/gpu/nova-core/Makefile
@@ -23,3 +23,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] 24+ messages in thread* Claude review: gpu: nova-core: emit Rust metadata for dependent modules
2026-04-30 14:55 ` [PATCH 5/7] gpu: nova-core: emit Rust metadata " Alexandre Courbot
@ 2026-05-05 0:17 ` Claude Code Review Bot
0 siblings, 0 replies; 24+ messages in thread
From: Claude Code Review Bot @ 2026-05-05 0:17 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Verdict: Most complex patch in the series, has a subtle concern.**
This adds a separate `rustc --emit=metadata` invocation to produce `.rmeta` without relying on the side-effect of the main object build:
```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
```
Observations:
1. **Why emit both metadata AND an object, then delete the object?** The `--emit=obj=$(basename $@).o` followed by `rm -f` seems wasteful. If only metadata is needed, `--emit=metadata=$@` alone should suffice unless rustc requires an emit target for codegen to complete. If rustc does require it, a comment explaining this would help.
2. **Private directory approach**: Building into `.nova_core_rmeta/` to avoid racing with the main `nova_core.o` build is sound:
```makefile
nova_core_rmeta_private := .nova_core_rmeta/$(nova_core_rmeta)
```
However, the `if_changed_dep` tracking for the private target and then `if_changed,copy` to the stable path means two targets are tracked. This adds complexity but is the right approach for correctness.
3. **The `__modname` and `part-of-builtin`/`part-of-module` private variables** are set correctly, mirroring what kbuild would set for the real module build. Good attention to detail.
4. **The `.rmeta` build invokes `rustc` a second time** on the same source. This doubles compile time for `nova-core`. Acceptable as a temporary workaround, but worth noting the cost.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 6/7] gpu: drm: nova: build after nova-core metadata
2026-04-30 14:55 [PATCH 0/7] gpu: drm: nova: enable calling into nova-core Alexandre Courbot
` (4 preceding siblings ...)
2026-04-30 14:55 ` [PATCH 5/7] gpu: nova-core: emit Rust metadata " Alexandre Courbot
@ 2026-04-30 14:55 ` Alexandre Courbot
2026-05-05 0:17 ` Claude review: " Claude Code Review Bot
2026-04-30 14:55 ` [PATCH POC 7/7] drm: nova: demonstrate interaction with nova-core Alexandre Courbot
2026-05-05 0:17 ` Claude review: gpu: drm: nova: enable calling into nova-core Claude Code Review Bot
7 siblings, 1 reply; 24+ messages in thread
From: Alexandre Courbot @ 2026-04-30 14:55 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, Joel Fernandes, 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 42019bff3173..f1d59c65120f 100644
--- a/drivers/gpu/drm/nova/Makefile
+++ b/drivers/gpu/drm/nova/Makefile
@@ -1,3 +1,18 @@
# SPDX-License-Identifier: GPL-2.0
+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)
+
obj-$(CONFIG_DRM_NOVA) += nova.o
--
2.54.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Claude review: gpu: drm: nova: build after nova-core metadata
2026-04-30 14:55 ` [PATCH 6/7] gpu: drm: nova: build after nova-core metadata Alexandre Courbot
@ 2026-05-05 0:17 ` Claude Code Review Bot
0 siblings, 0 replies; 24+ messages in thread
From: Claude Code Review Bot @ 2026-05-05 0:17 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Verdict: Functional but has the recursive make caveat.**
```makefile
$(objtree)/$(nova_core_rmeta): FORCE
$(Q)$(MAKE) $(build)=drivers/gpu/nova-core $(nova_core_rmeta)
```
This recursive make invocation is the "ugly but temporary" part the comments acknowledge. It works, but:
1. **Recursive make breaks dependency tracking**: The top-level make can't see into the sub-make's dependency graph. If `nova_core.rs` changes, the sub-make will rebuild the rmeta, but the parent make's timestamp-based tracking may not always re-trigger correctly since the target is `FORCE`-ed anyway.
2. **Order-only dependency** on `nova.o`:
```makefile
$(obj)/nova.o: | $(objtree)/$(nova_core_rmeta)
```
The `|` (order-only) prerequisite means make won't rebuild `nova.o` when the rmeta *content* changes, only ensures it exists before first compile. This seems intentional — rustc will detect stale metadata itself — but could lead to missed rebuilds if the rmeta format changes across rustc versions. For a temporary workaround, acceptable.
3. The `rustflags-y += --extern nova_core=$(objtree)/$(nova_core_rmeta)` path construction looks correct.
### PATCH 7 (POC): drm: nova: demonstrate interaction with nova-core
**Verdict: POC only, not for merge — review for completeness.**
This patch demonstrates the cross-module call by adding a `NovaCore::chipset()` method and calling it from `nova-drm`:
```rust
impl NovaCore {
pub fn chipset(adev: &auxiliary::Device<Core>) -> Result<gpu::Chipset> {
let dev = adev.parent();
let drvdata = dev.drvdata::<Self>()?;
Ok(drvdata.gpu.spec.chipset)
}
}
```
Observations:
1. **API design**: The method takes an `auxiliary::Device<Core>` and navigates up to the parent PCI device to extract `drvdata`. This means the caller (`nova-drm`) needs to know that `nova-core`'s data lives on the parent device, which leaks implementation detail. A proper API would likely take `&self` or use a handle/trait abstraction. Fine for POC.
2. **Visibility changes**: `pub(crate)` to `pub` on `NovaCore`, `Chipset`, `Spec.chipset`, `Gpu.spec`, plus `mod driver`/`mod gpu` going from `mod` to `pub mod`. This opens up a lot of internal surface. The real implementation should expose a curated public API, not just broaden all visibility.
3. **The `#[allow(missing_docs)]` on `Chipset`** and the added `//!` module-level docs are there to satisfy clippy. Harmless but indicates the POC is lightly polished.
4. The trailing `//` comment on the import:
```rust
use crate::gpu::{
self,
Gpu, //
};
```
This is the `rustfmt` workaround pattern to force vertical formatting. Stylistically fine.
5. The `use nova_core::driver::NovaCore;` import in `nova-drm`'s `driver.rs` confirms the cross-crate resolution works via the `--extern` flag from patch 6.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH POC 7/7] drm: nova: demonstrate interaction with nova-core
2026-04-30 14:55 [PATCH 0/7] gpu: drm: nova: enable calling into nova-core Alexandre Courbot
` (5 preceding siblings ...)
2026-04-30 14:55 ` [PATCH 6/7] gpu: drm: nova: build after nova-core metadata Alexandre Courbot
@ 2026-04-30 14:55 ` Alexandre Courbot
2026-05-05 0:17 ` Claude review: gpu: drm: nova: enable calling into nova-core Claude Code Review Bot
7 siblings, 0 replies; 24+ messages in thread
From: Alexandre Courbot @ 2026-04-30 14:55 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, Joel Fernandes, 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 | 6 ++++++
drivers/gpu/nova-core/driver.rs | 20 ++++++++++++++++++--
drivers/gpu/nova-core/gpu.rs | 9 ++++++---
drivers/gpu/nova-core/nova_core.rs | 4 ++--
4 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/nova/driver.rs b/drivers/gpu/drm/nova/driver.rs
index b1af0a099551..e18e9fccc067 100644
--- a/drivers/gpu/drm/nova/driver.rs
+++ b/drivers/gpu/drm/nova/driver.rs
@@ -15,6 +15,8 @@
use crate::file::File;
use crate::gem::NovaObject;
+use nova_core::driver::NovaCore;
+
pub(crate) struct NovaDriver {
#[expect(unused)]
drm: ARef<drm::Device<Self>>,
@@ -54,6 +56,10 @@ impl auxiliary::Driver for NovaDriver {
const ID_TABLE: auxiliary::IdTable<Self::IdInfo> = &AUX_TABLE;
fn probe(adev: &auxiliary::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> {
+ let chipset = NovaCore::chipset(adev)?;
+
+ pr_info!("Chipset from nova-core: {}\n", 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 84b0e1703150..985f65e13bd5 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,
@@ -23,13 +25,17 @@
},
};
-use crate::gpu::Gpu;
+use crate::gpu::{
+ self,
+ Gpu, //
+};
/// Counter for generating unique auxiliary device IDs.
static AUXILIARY_ID_COUNTER: Atomic<u32> = Atomic::new(0);
+/// Driver-associated data.
#[pin_data]
-pub(crate) struct NovaCore {
+pub struct NovaCore {
#[pin]
pub(crate) gpu: Gpu,
#[pin]
@@ -112,3 +118,13 @@ fn unbind(pdev: &pci::Device<Core>, this: Pin<&Self>) {
this.gpu.unbind(pdev.as_ref());
}
}
+
+impl NovaCore {
+ /// Returns the chipset of this GPU.
+ pub fn chipset(adev: &auxiliary::Device<Core>) -> Result<gpu::Chipset> {
+ let dev = adev.parent();
+ let drvdata = dev.drvdata::<Self>()?;
+
+ Ok(drvdata.gpu.spec.chipset)
+ }
+}
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 659f6a24ee13..75190ca0693b 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
+//! Declares some core driver types.
+
use kernel::{
device,
devres::Devres,
@@ -31,7 +33,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),*,
}
@@ -185,7 +188,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,
}
@@ -247,7 +250,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 {
- spec: Spec,
+ pub(crate) spec: Spec,
/// MMIO mapping of PCI BAR 0
bar: Arc<Devres<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 3a609f6937e4..ccb98c73457b 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] 24+ messages in thread* Claude review: gpu: drm: nova: enable calling into nova-core
2026-04-30 14:55 [PATCH 0/7] gpu: drm: nova: enable calling into nova-core Alexandre Courbot
` (6 preceding siblings ...)
2026-04-30 14:55 ` [PATCH POC 7/7] drm: nova: demonstrate interaction with nova-core Alexandre Courbot
@ 2026-05-05 0:17 ` Claude Code Review Bot
7 siblings, 0 replies; 24+ messages in thread
From: Claude Code Review Bot @ 2026-05-05 0:17 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: 17
Reviewed: 2026-05-05T10:17:58.324245
---
This series introduces a Nova-local workaround for `nova-drm` to call into `nova-core` by (1) fixing modpost buffer issues with long Rust symbols, (2) renaming the module to enable composite builds, (3) generating EXPORT_SYMBOL_GPL wrappers and .rmeta metadata for Rust cross-crate linking, and (4) demonstrating the interaction with a POC patch.
**The approach is reasonable as a temporary workaround** for the missing Rust cross-crate build system support. The modpost fixes (patches 1-2) are genuine bugfixes that stand on their own. The module rename (patch 3) is a necessary prerequisite. The export/metadata machinery (patches 4-6) is the core of the workaround and is clearly marked as temporary. Patch 7 (POC) is explicitly not for merging.
**Key concerns:**
- The modpost truncation detection (patch 1) is a real bug fix and should be separated from the Nova-specific series, as it affects all kernel module builds.
- The `SZ` bump to 1024 in patch 2 is arbitrary — a more principled approach (or dynamic allocation) would be more robust, though modpost being userspace makes the stack buffer acceptable.
- The rmeta build in patch 5 has a potential race condition concern worth discussing.
- Patch 7 reaches through `drvdata` from the auxiliary child to the PCI parent's private data, which couples `nova-drm` tightly to `nova-core`'s internal layout — fine for a POC, but the real API should be more structured.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 24+ messages in thread